Add error when RPK is used with DANE stub#10486
Open
embhorn wants to merge 1 commit into
Open
Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10486
Scan targets checked: wolfssl-bugs, wolfssl-src
No new issues found in the changed files. ✅
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts wolfSSL’s OpenSSL-compat verification reporting for TLS Raw Public Keys (RPK, RFC 7250) so that connections using RPK without an out-of-band trust mechanism do not report WOLFSSL_X509_V_OK via wolfSSL_get_verify_result().
Changes:
- Introduces a new verification result code for “RPK not trusted” and exposes it in both wolfSSL and OpenSSL-compat headers.
- Updates peer-certificate processing to set
peerVerifyRetto the new RPK-untrusted code when appropriate. - Extends
test_tls13_rpk_handshaketo assert the expected verify-result behavior for RPK vs X.509 handshakes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
wolfssl/ssl.h |
Adds a new wolfSSL verification-result enum value for untrusted RPK. |
wolfssl/openssl/x509.h |
Exposes the corresponding OpenSSL-compat X509_V_ERR_RPK_UNTRUSTED define. |
src/internal.c |
Sets peerVerifyRet for RPK handshakes and adds an error-string mapping for the new code. |
tests/api/test_tls13.c |
Adds assertions that RPK handshakes produce a non-OK verify result while X.509 handshakes remain OK. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10486
Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
No new issues found in the changed files. ✅
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
src/internal.c:1
- Correct the typo in the comment: “skip following x509 version check” should be “skip following X.509 version check” (or at least “skip following x509 version check” with proper spacing was already addressed, but please also standardize “X.509” capitalization to match surrounding terminology).
src/internal.c:1 - The handshake failure path uses ASN_NO_SIGNER_E (an X.509-oriented error) as the transport-level error for an untrusted RPK, while also introducing a distinct verify-result code (WOLFSSL_X509_V_ERR_RPK_UNTRUSTED) under OPENSSL_EXTRA*. This can be confusing for API consumers diagnosing failures (they may see ASN_NO_SIGNER_E rather than an RPK-specific failure unless they also inspect verify_result). Recommendation: consider propagating an RPK-specific error code at the handshake error layer as well (or otherwise ensure the mapping from ASN_NO_SIGNER_E to the new RPK_UNTRUSTED reason is discoverable), to make diagnostics consistent across builds (with/without OPENSSL_EXTRA).
Comment on lines
+2847
to
+2860
| *ctx_c = wolfSSL_CTX_new(method_c()); | ||
| *ctx_s = wolfSSL_CTX_new(method_s()); | ||
| if ((*ctx_c == NULL) || (*ctx_s == NULL)) | ||
| return -1; | ||
|
|
||
| if (wolfSSL_CTX_load_verify_locations(*ctx_c, caCertFile, 0) | ||
| != WOLFSSL_SUCCESS) | ||
| return -1; | ||
| if (wolfSSL_CTX_use_certificate_file(*ctx_c, clntRpkCertFile, | ||
| WOLFSSL_FILETYPE_ASN1) != WOLFSSL_SUCCESS) | ||
| return -1; | ||
| if (wolfSSL_CTX_use_PrivateKey_file(*ctx_c, cliKeyFile, CERT_FILETYPE) | ||
| != WOLFSSL_SUCCESS) | ||
| return -1; |
Comment on lines
+3009
to
+3010
| ExpectIntEQ(load_file(svrRpkCertFile, &svrSpki, &svrSpkiSz), 0); | ||
| ExpectIntEQ(load_file(clntRpkCertFile, &cliSpki, &cliSpkiSz), 0); |
Comment on lines
+2779
to
+2781
| /* Codes at or above WC_OSSL_V509_V_ERR_MAX are intentionally outside the | ||
| * contiguous range swept by the error-string test in tests/api.c. */ | ||
| WOLFSSL_X509_V_ERR_RPK_UNTRUSTED = 95, |
Comment on lines
+6252
to
+6257
| WOLFSSL_API int wolfSSL_CTX_set_expected_rpk(WOLFSSL_CTX* ctx, | ||
| const unsigned char* spki, | ||
| unsigned int spkiSz); | ||
| WOLFSSL_API int wolfSSL_set_expected_rpk(WOLFSSL* ssl, | ||
| const unsigned char* spki, | ||
| unsigned int spkiSz); |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10486
Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
No new issues found in the changed files. ✅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
DANE support was stubbed for the compatibility layer. Ensure an actual connection errors if it is used.
Fixes zd21825
Testing
Added test cases in
test_tls13_rpk_handshakeChecklist