Repository navigation
TLS: use rsa_pss_pss_* with id-RSASSA-PSS keys - #12004
jackctj117 wants to merge 8 commits into
Conversation
|
There was a problem hiding this comment.
🟡 Changes recommended
RSA-PSS handling can bypass key restrictions and ignores MGF constraints, while new tests fail on supported OpenSSL versions and hash configurations.
7 open issues
What changed in this PR
This PR addresses #11988 by aligning wolfSSL’s RSA-PSS signature selection and validation with certificate key types across TLS 1.2 and 1.3, including shared DTLS paths.
Changes:
- Distinguishes
rsa_pss_rsae_*fromrsa_pss_pss_*during negotiation, encoding, and verification. - Carries certificate PSS hash and salt parameters into TLS state for compatibility checks.
- Adds API regression tests, expands OpenSSL interoperability cases, and registers
rsapss.testwithmake check.
| File | Description |
|---|---|
| wolfssl/wolfcrypt/asn.h | Adds decoded PSS key parameter fields. |
| wolfssl/internal.h | Adds TLS PSS state and a peer-validation declaration. |
| wolfcrypt/src/asn.c | Retains parsed PSS hash and salt parameters. |
| tests/api.c | Adds RSA-PSS negotiation and mutual-authentication tests. |
| src/tls13.c | Updates TLS 1.3 scheme encoding and peer validation. |
| src/ssl_load.c | Copies certificate PSS parameters into TLS state. |
| src/internal.c | Updates scheme selection, signing, and verification. |
| scripts/rsapss.test | Expands TLS/DTLS interoperability and rejection cases. |
| scripts/include.am | Registers RSA-PSS interoperability tests. |
Review effort: Balanced
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if (ssl->peerRsaPssHash != 0 && HashAlgoToType(hashAlgo) != | ||
| (enum wc_HashType)ssl->peerRsaPssHash) { | ||
| WOLFSSL_MSG("Peer RSA-PSS hash differs from key parameters"); | ||
| WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER); | ||
| return INVALID_PARAMETER; | ||
| } | ||
| if (ssl->peerRsaPssSaltLen != 0 && (int)ssl->peerRsaPssSaltLen != | ||
| wc_HashGetDigestSize(HashAlgoToType(hashAlgo))) { |
There was a problem hiding this comment.
🔴 TLS 1.3-only builds (--disable-tlsv12, RSA on by default) will fail to link once this merges, with an undefined reference to HashAlgoToType. Both new RSA-PSS key checks call HashAlgoToType() at internal.c:6397/6404 and 34836/34840. Its only definition, at internal.c:5816-5858, sits inside #ifndef WOLFSSL_NO_TLS12. Fix: make HashAlgoToType available in every build that compiles these callers, e.g. widen its guard to !WOLFSSL_NO_TLS12 || (!NO_RSA && WC_RSA_PSS). That covers both sites: CheckPeerRsaSigAlgo and MatchSigAlgo.
Why this was flagged
The trigger is a build with WOLFSSL_NO_TLS12 and WC_RSA_PSS. Running configure with --disable-tlsv12 and RSA left on gives that, because configure.ac:7193-7195 forces RSAPSS=yes whenever TLS 1.3 is enabled. CI runs this build in os-check-linux.json:504 with --disable-tlsv12 --enable-ocspstapling --enable-opensslextra. In that build, CheckPeerRsaSigAlgo (internal.c:6384-6417) is guarded only by #if !defined(NO_RSA) && defined(WC_RSA_PSS), and tls13.c:12938 calls it from the TLS 1.3 CertificateVerify handler. MatchSigAlgo (guarded by !NO_TLS && (!NO_WOLFSSL_SERVER || !NO_CERTS) at 34723) is used by TLS 1.3 PickHashSigAlgo. Both now call HashAlgoToType. Its only definition, at internal.c:5821, is wrapped in #ifndef WOLFSSL_NO_TLS12 (5816/5858). The prototype at internal.h:8151 is unguarded, so compilation succeeds but linking libwolfssl into the examples, unit tests or applications fails. On the base branch, every HashAlgoToType caller (38575, 41782, 44952) was TLS 1.2-only code, so this configuration linked.
Verification: Trigger: any build with WOLFSSL_NO_TLS12 plus RSA and WC_RSA_PSS, for example ./configure --disable-tlsv12. The only definition of HashAlgoToType is at src/internal.c:5821, inside #ifndef WOLFSSL_NO_TLS12. The new calls at internal.c:6397, 6404, 34836 and 34840 have no TLS 1.2 guard. Every other existing caller is in TLS 1.2-only code, so the base branch linked cleanly.
| if (ssl->peerRsaPssSaltLen != 0 && (int)ssl->peerRsaPssSaltLen != | ||
| wc_HashGetDigestSize(HashAlgoToType(hashAlgo))) { | ||
| WOLFSSL_MSG("Peer RSA-PSS salt length differs from key parameters"); | ||
| WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER); | ||
| return INVALID_PARAMETER; | ||
| } |
There was a problem hiding this comment.
🔴 Valid id-RSASSA-PSS certificates whose key parameters declare a salt length shorter than the hash size can no longer be used or verified in TLS. The salt check needs peerRsaPssSaltLen/rsaPssSaltLen to equal the digest size. RFC 4055, and GetCertKey's own saltLen > sigSaltLen check at asn.c:14536, treat the key's saltLength as a minimum. Fix: in both checks, reject only when the declared salt length is greater than wc_HashGetDigestSize(HashAlgoToType(hashAlgo)). That keeps 0 working, as the no-constraint value. Same pattern at 2 sites (src/internal.c:6403, src/internal.c:34839).
Why this was flagged
Trigger: a cert whose RSASSA-PSS-params set hashAlgorithm sha256 and either leave out saltLength (DecodeRsaPssParams then sets the ASN.1 DEFAULT of 20, asn.c:8745) or set it to 20. GetCertKey stores 20 in keyPssSaltLen (asn.c:14542). When that cert is the peer's, ProcessPeerCertDecodeKey copies it into peerRsaPssSaltLen. CheckPeerRsaSigAlgo (internal.c:6403) then returns INVALID_PARAMETER for rsa_pss_pss_sha256, because TLS salts are always 32 bytes and 20 != 32. The ServerKeyExchange or CertificateVerify fails in both TLS 1.2 and TLS 1.3. When the cert is the local one, MatchSigAlgo (internal.c:34839) rejects every scheme, so PickHashSigAlgo finds nothing and the handshake fails. On the base branch these keys negotiated and verified normally. Nothing else covers this case: the parser itself only rejects a key salt length larger than the signature's, which treats it as a minimum. The rsapss.test certs declare exactly 32 or no parameters, so the tests never hit it.
Verification: GetCertKey treats the key's salt length as a minimum (asn.c:14536). CheckPeerRsaSigAlgo (internal.c:6403-6407) instead returns INVALID_PARAMETER when peerRsaPssSaltLen is non-zero and not equal to the digest size, so sha256 with saltLength 20 fails the handshake. MatchSigAlgo (internal.c:34839-34841) does the same check. On the base, these keys worked.
| ssl->useRsaPss = cert->keyOID == RSAPSSk; | ||
| ssl->rsaPssHash = cert->keyPssHash; | ||
| ssl->rsaPssSaltLen = cert->keyPssSaltLen; | ||
| } | ||
| if (ctx) { | ||
| ctx->useRsaPss = cert->keyOID == RSAPSSk; | ||
| ctx->rsaPssHash = cert->keyPssHash; | ||
| ctx->rsaPssSaltLen = cert->keyPssSaltLen; |
There was a problem hiding this comment.
🔴 Servers that install certificates through the OpenSSL-compat APIs can pick the wrong RSA-PSS family and fail handshakes that work on the base branch. useRsaPss, rsaPssHash and rsaPssSaltLen are only written by ProcessBufferCertPublicKey and SetSSL_CTX. Two install paths skip both, so the flags stay stale while MatchSigAlgo and the peer check now enforce them. Fix: keep the three fields in sync on every path that installs or swaps the local certificate, for example a shared helper called from each, which covers the 2 sites listed. Same pattern at 2 sites (src/ssl.c:9133, src/ssl_load.c:5454).
Why this was flagged
wolfSSL_set_SSL_CTX (src/ssl.c:9133), the SNI-callback API, swaps the cert, key and keyType but never copies ctx->useRsaPss/rsaPssHash/rsaPssSaltLen. wolfSSL_CTX_use_certificate(ctx, X509*) (src/ssl_load.c:5454) bypasses ProcessBuffer and sets only haveRSA. Case 1: the initial CTX has an rsaEncryption cert and SNI switches to an id-RSASSA-PSS cert, or a PSS cert is installed from an X509. useRsaPss stays 0, so the server still signs rsa_pss_rsae_. The new CheckPeerRsaSigAlgo (internal.c:6391) in a wolfSSL client sees peerRsaPss=1 and returns INVALID_PARAMETER, where the base client accepted. Case 2: an SNI switch from a PSS CTX to an rsaEncryption CTX leaves useRsaPss=1 and the old rsaPssHash. The new MatchSigAlgo branch (internal.c:34831) then allows only rsa_pss_pss_ with that stale hash. TLS 1.2 SKE gets labelled pss_pss over an rsaEncryption key, or no sigalg matches, so clients reject a handshake that worked on base. CheckSSL_CTX_CertsAndKeys only checks buffer staleness, not these flags.
Verification: wolfSSL_set_SSL_CTX (src/ssl.c:9007 onward) never copies ctx->useRsaPss, rsaPssHash or rsaPssSaltLen. wolfSSL_CTX_use_certificate (src/ssl_load.c:5454) does not update useRsaPss. MatchSigAlgo (internal.c:34831-34841) now gates on these fields, so a stale flag picks the wrong PSS family, and CheckPeerRsaSigAlgo (internal.c:6391-6396) rejects handshakes the base accepted.
| if (ssl->hsType == DYNAMIC_TYPE_RSA) { | ||
| #ifdef WC_RSA_PSS | ||
| /* id-RSASSA-PSS key: rsa_pss_pss_* (RFC 8446 4.2.3) */ | ||
| if (ssl->useRsaPss) | ||
| args->sigAlgo = rsa_pss_pss_algo; | ||
| else | ||
| #endif | ||
| args->sigAlgo = rsa_pss_sa_algo; |
There was a problem hiding this comment.
🔴 With dual-algorithm certs, a TLS 1.3 endpoint whose native key is id-RSASSA-PSS can no longer send a CertificateVerify when the peer requests both signatures. The send path now sets args->sigAlgo to rsa_pss_pss_algo when useRsaPss is set. EncodeDualSigAlg (tls13.c:9748, 9764) only maps rsa_pss_sa_algo, so it writes 0x0000 and the handshake fails with ALGO_ID_E. Fix: in the CKS-BOTH branch, have EncodeDualSigAlg accept rsa_pss_pss_algo as the native algorithm wherever it accepts rsa_pss_sa_algo (or pass it the rsae value).
Why this was flagged
The trigger needs a build with WOLFSSL_DUAL_ALG_CERTS, a native id-RSASSA-PSS key (so ssl->useRsaPss is set by ssl_load.c:1862) and a peer that negotiates WOLFSSL_CKS_SIGSPEC_BOTH. The path is SendTls13CertificateVerify. At tls13.c:11634-11635 the changed code sets args->sigAlgo = rsa_pss_pss_algo. At tls13.c:11686 EncodeDualSigAlg(args->sigAlgo, args->altSigAlgo, args->verify) only matches sigAlg == rsa_pss_sa_algo for the RSA3072 hybrid code points (tls13.c:9748, 9764). It leaves args->verify[0] == 0, so tls13.c:11687-11688 does ERROR_OUT(ALGO_ID_E, exit_scv). On the base branch args->sigAlgo was always rsa_pss_sa_algo for RSA keys, so the hybrid code point was encoded and the dual-signature handshake completed.
Verification: ssl_load.c:1862 sets useRsaPss for an id-RSASSA-PSS native key, and tls13.c:11634-11635 then sets args->sigAlgo = rsa_pss_pss_algo, where the base used rsa_pss_sa_algo. EncodeDualSigAlg only matches rsa_pss_sa_algo (tls13.c:9748, 9764), so output stays 0 and tls13.c:11687-11688 fails with ALGO_ID_E. On the base, the hybrid RSA3072 code point was written.
| else if (sigAlgo == rsa_pss_pss_algo) { | ||
| /* Only the id-RSASSA-PSS key scheme. */ | ||
| ADD_HASH_SIG_ALGO(hashSigAlgo, inOutIdx, rsa_pss_sa_algo, | ||
| PSS_RSAE_TO_PSS_PSS(macAlgo)); |
There was a problem hiding this comment.
🟡 (optional) Users who set "RSA-PSS-PSS+SHA1" or "RSA-PSS-PSS+SHA224" advertise Ed25519 (0x0807) or Ed448 (0x0808) instead of an RSA-PSS scheme. The new rsa_pss_pss_algo branch applies PSS_RSAE_TO_PSS_PSS to any macAlgo, and only sha256..sha512 map to 0x0809-0x080b. Fix: emit the pss_pss entry only when macAlgo is sha256_mac..sha512_mac. Also make SetSuitesHashSigAlgo reject RSA-PSS-PSS with any other hash. Apply the same range check to the rsa_pss_sa_algo branch at internal.c:4134. Without TLS 1.3 compiled in, that branch now emits this second entry for every hash.
Why this was flagged
Trigger: wolfSSL_set1_sigalgs_list or SetSuitesHashSigAlgo given the new names "RSA-PSS-PSS"/"PSS-PSS" from internal.c:34603 with SHA1 or SHA224. GetMacAlgFromName accepts both (internal.c:34565, 34572). sha_mac + (pss_sha256 - sha256_mac) = 7, and sha224_mac gives 8. AddSuiteHashSigAlgo (internal.c:4139) therefore writes 0x08 0x07 or 0x08 0x08. In signature_algorithms that advertises Ed25519 or Ed448, and SupportedHashSigAlgo accepts those from a peer. No RSA-PSS scheme is advertised. The old tls13.c EncodeSigAlg guarded this mapping with (sha256_mac <= hashAlgo) && (hashAlgo <= sha512_mac), and the new code has no such guard. The removed #ifdef WOLFSSL_TLS13 at internal.c:4133 means "RSA-PSS+SHA1" now emits the same bogus 0x0807 entry in TLS 1.2-only builds too.
Verification: The new branch at src/internal.c:4139-4142 applies the offset to whatever macAlgo it is given. PSS_RSAE_TO_PSS_PSS is (macAlgo) + (pss_sha256 - sha256_mac) (wolfssl/internal.h:5099-5100). SetSuitesHashSigAlgo (internal.c:34660-34701) checks nothing beyond a non-zero mac_alg, so the call succeeds and only the Ed25519/Ed448 codepoint is advertised, with no RSA-PSS scheme.
| if (ssl->options.sigAlgo == rsa_sa_algo) { | ||
| if (ssl->useRsaPss) { | ||
| /* id-RSASSA-PSS key: rsa_pss_pss_* only (RFC 8446 4.2.3), with | ||
| * the hash and salt length its parameters declare (RFC 4055 1.2). */ | ||
| if (sigAlgo != rsa_pss_pss_algo) |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: Pre-existing: a TLS 1.2 server with an id-RSASSA-PSS key still signs ServerKeyExchange with rsa_pkcs1 when the ClientHello has no signature_algorithms. The new rsa_pss_pss_* binding in MatchSigAlgo never runs then. PickHashSigAlgo returns early at internal.c:34997 with sigAlgo = rsa_sa_algo and hashAlgo set to sha256. Fix: when useRsaPss is set and the peer sent no list, fail the handshake (or select rsa_pss_pss_algo), so a PSS-OID key never makes a PKCS#1 v1.5 signature.
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
The trigger is a TLS 1.2 (or DTLS 1.2) ClientHello without the signature_algorithms extension. That leaves clSuites->hashSigAlgoSz = 0 (internal.c:44228, 43435), and the server's cert is id-RSASSA-PSS. internal.c:43294 calls PickHashSigAlgo. It sets ssl->options.sigAlgo = ssl->specs.sig_algo (rsa_sa_algo) and returns 0 at internal.c:34997-34998, before any MatchSigAlgo call, so the new useRsaPss checks at internal.c:34831-34841 are never reached. SendServerKeyExchange then encodes and makes an RSASSA-PKCS1-v1_5 signature with the PSS-restricted key. This breaks the RFC 4055/8446 binding this change enforces everywhere else, and compliant peers, including this PR's own CheckPeerRsaSigAlgo, reject it. The base branch behaves the same way, so this is a gap the partial fix leaves open, not a regression.
Verification: Pre-existing. Trigger: a TLS 1.2 or DTLS 1.2 client sends a ClientHello without signature_algorithms to a server whose certificate key is id-RSASSA-PSS. PickHashSigAlgo returns at if (hashSigAlgoSz == 0) return 0; with rsa_sa_algo, so the binding at internal.c:34831-34841 never runs. The base has the same early return at line 34927.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #12004
Scan targets checked: wolfcrypt-src, wolfcrypt-bugs, wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): scripts/include.am, scripts/rsapss.test
Coverage: 6 of 7 in-scope changed file(s) opened by the reviewer; not opened: wolfssl/wolfcrypt/asn.h
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
Review tier: Max
| WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER); | ||
| return INVALID_PARAMETER; | ||
| } | ||
| if (ssl->peerRsaPssHash != 0 && HashAlgoToType(hashAlgo) != |
There was a problem hiding this comment.
TLS 1.3-only builds fail to link: HashAlgoToType is undefined · Logic errors
CheckPeerRsaSigAlgo and the new MatchSigAlgo branch (line 34836) call HashAlgoToType. That function is only defined inside #ifndef WOLFSSL_NO_TLS12 (line 5816), so WOLFSSL_NO_TLS12 + WC_RSA_PSS builds (e.g. --disable-tlsv12) fail to link on an undefined reference.
Suggested fix: Widen or move the guard so HashAlgoToType is defined whenever !NO_RSA && WC_RSA_PSS, including WOLFSSL_NO_TLS12 builds.
Basis: ISO C11 6.9p5: an identifier with external linkage that is used in an expression must have exactly one external definition in the program.
| WOLFSSL_ERROR_VERBOSE(INVALID_PARAMETER); | ||
| return INVALID_PARAMETER; | ||
| } | ||
| if (ssl->peerRsaPssSaltLen != 0 && (int)ssl->peerRsaPssSaltLen != |
There was a problem hiding this comment.
Exact salt-length match rejects valid id-RSASSA-PSS keys · Logic errors
The key's saltLength must equal the digest size here and in MatchSigAlgo (line 34839), but GetCertKey treats it as a minimum. A key declaring SHA-256 with salt 20 (the DER default) can never sign locally, and peers using one are rejected.
Suggested fix: Reject only when the key's saltLength is larger than the digest size, in both CheckPeerRsaSigAlgo and MatchSigAlgo.
Basis: RFC 4055 section 3.1: unlike the other RSASSA-PSS-params fields, saltLength need not be fixed for a key pair; RFC 8446 section 4.2.3 fixes the TLS salt length at the digest length.
Related known findings (similar but distinct; listed for context, not part of this finding)
| client_cbf.ssl_ready = test_rsa_pss_pss_sha384_ssl_ready; | ||
| client_cbf.on_result = test_rsa_pss_pss_on_result; | ||
| server_cbf.on_result = test_rsa_pss_pss_on_result; | ||
| server_cbf.certPemFile = RSAPSS3072_SVR_CERT; |
There was a problem hiding this comment.
3072-bit server test case cannot verify its chain against the 2048-bit root · Logic errors
server-3072-rsapss.pem chains via ca-3072 to root-3072-rsapss.pem (AKID AA:71...), but the client still trusts only RSAPSS_ROOT (SKID 64:D5...). The client verifies by default, so the handshake fails and the TEST_SUCCESS check at line 39221 fails in OPENSSL_EXTRA builds.
Suggested fix: Set client_cbf.caPemFile to ./certs/rsapss/root-3072-rsapss.pem for this case.
Basis: RFC 5280 section 6.1: certification path validation must start from a trust anchor that issued the first certificate in the path.
Pick, label and check RSA-PSS signature schemes by the key's OID in TLS 1.2 and 1.3, honour the key's PSS parameters, and accept rsa_pss_pss_* in a TLS 1.2 CertificateVerify. Wire rsapss.test into make check and add a unit test. Fixes wolfSSL#11988
Check the hash, MGF and minimum salt length (RFC 4055 3.3) of each candidate scheme, not only the first match, through ConvertHashPss so TLS 1.3-only builds link. Read the scheme off the wire in rsapss.test and gate its SHA-384 and SHA-512 cases; trust the 3072-bit root in the unit test.
Copy them on a context swap and when a certificate is installed as an X509 object, map the key to the RSA hybrid code points, list RSA-PSS schemes for SHA-256 to SHA-512 only, and refuse a TLS 1.2 ClientHello without signature_algorithms when the key must not sign rsa_pkcs1 (RFC 4055 1.2).
8ccebf8 to
5286e64
Compare
Such a key only signs (RFC 4055 1.2), so the server passes over the static RSA suites. Resolve the test certificates from the script's own directory for out-of-tree make check, fail cases that never connected, and keep the RSA-PSS checks out of NO_RSA builds.
|
Jenkins restest please. |
MatchRsaPssParams takes the hash as an int, as MatchSigAlgo passes it (MSVC C4244). The script follows the examples onto the IPv6 loopback, lets s_client accept a server without renegotiation_info, treats a refused UDP write as no connection, and leaves out the client certificate cases where the build has no client auth or verifies through Apple's framework. The unit test needs PEM, client auth and wolfSSL's own chain verification.
Without WOLFSSL_EXTRA_ALERTS the server rejects a ClientHello silently, so s_client retransmits into a closed port and logs the same write error as when no server was ever there. Count it as no connection only when nothing was read.
A fixed pause is not enough on a loaded machine running make check in parallel. Poll ss or netstat for the port, with the old pause as the fallback when neither exists.
shellcheck reads $port[[:space:]] as an array subscript (SC1087).
Pick, label and check RSA-PSS signature schemes by the key's OID in TLS 1.2 and 1.3, honour the key's PSS parameters, and accept rsa_pss_pss_* in a TLS 1.2 CertificateVerify. Wire rsapss.test into make check and add a unit test.
Fixes #11988
Description
Please describe the scope of the fix or feature addition.
Fixes zd#
Testing
How did you test?
Checklist