[GH-ISSUE #2796] Inconsistent error handling in SHA256 digest functions (openssl_auth.cpp) #1300

New issue

Open

opened 2026-03-04 01:52:56 +03:00 by kerem · 1 comment

kerem commented

2026-03-04 01:52:56 +03:00

Owner

Originally created by @CarstenGrohmann on GitHub (Jan 31, 2026).
Original GitHub issue: https://github.com/s3fs-fuse/s3fs-fuse/issues/2796

The digest functions using the old OpenSSL API (s3fs_md5() and s3fs_sha256()) lack error handling. This creates silent failures where callers expect error detection but never receive it.

The modern API usage was only added to the MD5 functions for OpenSSL 3.0.

Issues

No nullptr checks

EVP_get_digestbyname() and EVP_MD_CTX_create() can return nullptr on memory exhaustion. The code does not check for this, leading to undefined behavior.

github.com/s3fs-fuse/s3fs-fuse@85cc411aaa/src/openssl_auth.cpp (L346-L347)
No return value checks

The return values of EVP_DigestInit_ex(), EVP_DigestUpdate(), and EVP_DigestFinal_ex() are ignored. These functions return 1 on success and 0 on failure.

github.com/s3fs-fuse/s3fs-fuse@85cc411aaa/src/openssl_auth.cpp (L348-L351)
No RAII

Manual resource management is used instead of std::unique_ptr with custom deleter, as done in the OpenSSL 3.0 version of s3fs_md5().
Deprecated API / Inconsistency

The MD5 functions have an OpenSSL 3.0 variant using modern API (EVP_MD_CTX_new()/EVP_MD_CTX_free()), RAII, and proper error handling. The SHA256 functions still use the old API (EVP_MD_CTX_create()/EVP_MD_CTX_destroy()).

github.com/s3fs-fuse/s3fs-fuse@85cc411aaa/src/openssl_auth.cpp (L347)
Silent failure

The functions always return true, even when OpenSSL operations fail. All callers have implemented error handling that checks the return value, but this code path is never executed:

github.com/s3fs-fuse/s3fs-fuse@85cc411aaa/src/openssl_auth.cpp (L344-L355)

Callers with dead error handling code:

Question

API compatibility: Is OpenSSL < 1.1.0 still a supported target?

Proposed Solution

Update s3fs_sha256() and s3fs_sha256_fd() to match the style of the OpenSSL 3.0 MD5 functions:

Use std::unique_ptr with custom deleter for RAII
Check return values of all EVP_Digest*() functions
Add nullptr checks for context creation
Use modern API (EVP_MD_CTX_new()/EVP_MD_CTX_free())

If you agree, I can create a PR for that.

Originally created by @CarstenGrohmann on GitHub (Jan 31, 2026). Original GitHub issue: https://github.com/s3fs-fuse/s3fs-fuse/issues/2796 The digest functions using the old OpenSSL API (`s3fs_md5()` and `s3fs_sha256()`) lack error handling. This creates silent failures where callers expect error detection but never receive it. The modern API usage was only added to the MD5 functions for OpenSSL 3.0. ## Issues 1. No nullptr checks `EVP_get_digestbyname()` and `EVP_MD_CTX_create()` can return `nullptr` on memory exhaustion. The code does not check for this, leading to undefined behavior. https://github.com/s3fs-fuse/s3fs-fuse/blob/85cc411aaa52f9848de6c17a9205d78ed3b4bad9/src/openssl_auth.cpp#L346-L347 2. No return value checks The return values of `EVP_DigestInit_ex()`, `EVP_DigestUpdate()`, and `EVP_DigestFinal_ex()` are ignored. These functions return 1 on success and 0 on failure. https://github.com/s3fs-fuse/s3fs-fuse/blob/85cc411aaa52f9848de6c17a9205d78ed3b4bad9/src/openssl_auth.cpp#L348-L351 3. No RAII Manual resource management is used instead of `std::unique_ptr` with custom deleter, as done in the OpenSSL 3.0 version of `s3fs_md5()`. 4. Deprecated API / Inconsistency The MD5 functions have an OpenSSL 3.0 variant using modern API (`EVP_MD_CTX_new()`/`EVP_MD_CTX_free()`), RAII, and proper error handling. The SHA256 functions still use the old API (`EVP_MD_CTX_create()`/`EVP_MD_CTX_destroy()`). https://github.com/s3fs-fuse/s3fs-fuse/blob/85cc411aaa52f9848de6c17a9205d78ed3b4bad9/src/openssl_auth.cpp#L347 5. Silent failure The functions always return `true`, even when OpenSSL operations fail. All callers have implemented error handling that checks the return value, but this code path is never executed: https://github.com/s3fs-fuse/s3fs-fuse/blob/85cc411aaa52f9848de6c17a9205d78ed3b4bad9/src/openssl_auth.cpp#L344-L355 Callers with dead error handling code: - https://github.com/s3fs-fuse/s3fs-fuse/blob/85cc411aaa52f9848de6c17a9205d78ed3b4bad9/src/curl.cpp#L2284-L2286 - https://github.com/s3fs-fuse/s3fs-fuse/blob/85cc411aaa52f9848de6c17a9205d78ed3b4bad9/src/curl.cpp#L2350-L2352 - https://github.com/s3fs-fuse/s3fs-fuse/blob/85cc411aaa52f9848de6c17a9205d78ed3b4bad9/src/common_auth.cpp#L43-L46 ## Question 1. API compatibility: Is OpenSSL < 1.1.0 still a supported target? ## Proposed Solution Update `s3fs_sha256()` and `s3fs_sha256_fd()` to match the style of the OpenSSL 3.0 MD5 functions: - Use `std::unique_ptr` with custom deleter for RAII - Check return values of all `EVP_Digest*()` functions - Add `nullptr` checks for context creation - Use modern API (`EVP_MD_CTX_new()`/`EVP_MD_CTX_free()`) If you agree, I can create a PR for that.

kerem commented

2026-03-04 01:52:57 +03:00

Author

Owner

@ggtakec commented on GitHub (Feb 7, 2026):

@CarstenGrohmann Thank you.
I still think it's best to maintain support for OpenSSL 1.1.x.

I also checked openssl_auth.cpp, and in addition to the code you pointed out, I think it would be better to fix the calls to ERR_load_crypto_strings, EVP_cleanup, ERR_free_strings, and OpenSSL_add_all_algorithms.
github.com/s3fs-fuse/s3fs-fuse@90324ad669/src/openssl_auth.cpp (L55-L76)
Calling these functions is no longer necessary in OpenSSL 1.1.0 and later.
I think it would be best to make the following changes. (This is for versions prior to 1.1.0, so you may want to remove it.)

bool s3fs_init_global_ssl()
{
#if OPENSSL_VERSION_NUMBER < 0x10100000L
    ERR_load_crypto_strings();
    ERR_load_BIO_strings();
    OpenSSL_add_all_algorithms();
#endif
    return true;
}

bool s3fs_destroy_global_ssl()
{
#if OPENSSL_VERSION_NUMBER < 0x10100000L
    EVP_cleanup();
    ERR_free_strings();
#endif
    return true;
}

I will wait for your PR, including the above.
Thanks in advance for your help.

@ggtakec commented on GitHub (Feb 7, 2026): @CarstenGrohmann Thank you. I still think it's best to maintain support for OpenSSL 1.1.x. I also checked `openssl_auth.cpp`, and in addition to the code you pointed out, I think it would be better to fix the calls to `ERR_load_crypto_strings`, `EVP_cleanup`, `ERR_free_strings`, and `OpenSSL_add_all_algorithms`. https://github.com/s3fs-fuse/s3fs-fuse/blob/90324ad6694dd7b135411b15cda8712b226ab5fb/src/openssl_auth.cpp#L55-L76 Calling these functions is no longer necessary in OpenSSL 1.1.0 and later. I think it would be best to make the following changes. (This is for versions prior to 1.1.0, so you may want to remove it.) ``` bool s3fs_init_global_ssl() { #if OPENSSL_VERSION_NUMBER < 0x10100000L ERR_load_crypto_strings(); ERR_load_BIO_strings(); OpenSSL_add_all_algorithms(); #endif return true; } bool s3fs_destroy_global_ssl() { #if OPENSSL_VERSION_NUMBER < 0x10100000L EVP_cleanup(); ERR_free_strings(); #endif return true; } ``` I will wait for your PR, including the above. Thanks in advance for your help.