Repository navigation
TLS: use rsa_pss_pss_* with id-RSASSA-PSS keys - #12004
jackctj117 wants to merge 4 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.
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