Skip to content

sha256: support delegating to wolfSSL API#21078

Closed
vszakats wants to merge 1 commit intocurl:masterfrom
vszakats:sha256wolfssl
Closed

sha256: support delegating to wolfSSL API#21078
vszakats wants to merge 1 commit intocurl:masterfrom
vszakats:sha256wolfssl

Conversation

@vszakats
Copy link
Copy Markdown
Member

@vszakats vszakats commented Mar 23, 2026

Offered by wolfSSL v3.11.0+ (2017-05-04).


(Can be made work in v3.10.2 (2017-02-13) by including <wolfssl/wolfcrypt/types.h>, which looks like a wolfSSL early implementation issue.)

  • figure out which wolfSSL version it requires. v3.10.2 (2017-02-13). Check for it, or bump minimum. → a higher version is already required without this patch.
  • use the native wolfcrypt API instead? following lib/vtls/wolfssl.c? [FUTURE] → sha256, sha512_256: switch to wolfCrypt API #21090

@bagder
Copy link
Copy Markdown
Member

bagder commented Mar 23, 2026

Might as well bump the minimum version IMHO!

@vszakats
Copy link
Copy Markdown
Member Author

According to tests, 5.0.0 is the current minimum required → PR #21080

@vszakats vszakats closed this in 988b352 Mar 24, 2026
@vszakats vszakats deleted the sha256wolfssl branch March 24, 2026 18:30
@vszakats vszakats requested a review from Copilot March 24, 2026 21:52
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates lib/sha256.c to delegate SHA-256 hashing to wolfSSL’s API when building with USE_WOLFSSL, aligning wolfSSL behavior more closely with the existing OpenSSL-backed implementation.

Changes:

  • Add a wolfSSL-backed SHA-256 implementation path (via the OpenSSL-compat EVP interface) alongside the existing OpenSSL EVP path.
  • Update the documented SSL-backend branch ordering to include USE_WOLFSSL.
  • Split EVP context allocation / digest init-update-final between OpenSSL and wolfSSL APIs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +53 to +59
#if defined(USE_OPENSSL) || defined(USE_WOLFSSL)
#ifdef USE_OPENSSL
#include <openssl/evp.h>
#else
#include <wolfssl/options.h>
#include <wolfssl/openssl/evp.h>
#endif
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new USE_WOLFSSL path unconditionally includes <wolfssl/openssl/evp.h> and calls wolfSSL_EVP_* APIs. wolfSSL’s OpenSSL-compat/EVP layer is not guaranteed to be enabled in all supported wolfSSL builds (e.g., builds without --enable-opensslextra), and curl’s configure currently only feature-detects a few OpenSSL-compat symbols (DES/BIO/etc). This risks build/link failures for otherwise-supported wolfSSL configurations.

Suggestion: add configure/CMake feature checks for the specific wolfSSL_EVP_* symbols used here (or a LIBWOLFSSL_VERSION_HEX guard), and fall back to the internal SHA-256 implementation when those symbols are unavailable (or switch to the native wolfCrypt SHA-256 API which is always present).

Copilot uses AI. Check for mistakes.
Comment on lines 61 to +66
struct ossl_sha256_ctx {
#ifdef USE_OPENSSL
EVP_MD_CTX *openssl_ctx;
#else
WOLFSSL_EVP_MD_CTX *openssl_ctx;
#endif
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the OpenSSL/wolfSSL shared branch, the field name openssl_ctx is now misleading for the USE_WOLFSSL build where it actually holds a wolfSSL EVP context. Consider renaming it to something backend-neutral (e.g., evp_ctx/md_ctx) to avoid confusion when maintaining the dual-backend code.

Copilot uses AI. Check for mistakes.
vszakats added a commit that referenced this pull request Mar 25, 2026
Replacing the OpenSSL-like compatibility interface, and syncing with
existing API use within lib/wolfssl.c for SHA-256.

Ref: https://www.wolfssl.com/documentation/manuals/wolfssl/group__SHA.html

Follow-up to 28f0932 #21077
Follow-up to 988b352 #21078

Closes #21090
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants