Closed
Bug 1414931
Opened 8 years ago
Closed 7 years ago
NSS accepts PKCS#1 v1.5 signatures made with RSA-PSS keys
Categories
(NSS :: Libraries, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
3.39
People
(Reporter: hkario, Assigned: mt)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
When NSS receives PKCS#1 v1.5 signature made using RSA-PSS key, it continues the connection. This happens both for Server Key Exchange and Certificate Verify.
Reproducer:
Generate rsa-pss key, sign it by a CA trusted by database in ./nssdb. Export to unencrypted files the key (host.key) and certificate (host.crt).
git clone https://github.com/tomato42/tlslite-ng.git
pushd tlslite-ng
git clone https://github.com/warner/python-ecdsa.git .python-ecdsa
ln -s .python-ecdsa/ecdsa ecdsa
popd
PYTHONPATH=tlslite-ng cert-verify-server.py -k host.key -c host.crt
connect to it using tstclnt:
tstclnt -d sql:nssdb -4 -h localhost -p 4433
Result:
connection established
Expected result:
connection aborted by client after receiving Server Key Exchange message
Comment 1•8 years ago
|
||
Hubert: From a design question, it appears you believe these should not be accepted.
I'm curious how/why you've reached that conclusion, and how you're defining "RSA-PSS key" - since, after all, both EMSA-PSS-Encode and EMSA-PKCS1-v1_5 are just transformations done before the RSA operation.
I mention this because there's considerable debate on how to restrict key usages (RFC 3447, RFC 4055, RFC 5756 capture those nuances)
| Reporter | ||
Comment 2•8 years ago
|
||
by rsa-pss key I understand a certificate that has a Public Key type of RSA-PSS - i.e. key that is restricted to performing only rsa-pss signatures according to RFC 4055:
When the RSA private key owner wishes to limit the use of the public
key exclusively to RSASSA-PSS, then the id-RSASSA-PSS object
identifier MUST be used in the algorithm field within the subject
public key information, and, if present, the parameters field MUST
contain RSASSA-PSS-params.
Comment 3•8 years ago
|
||
There's not a particularly compelling reason to limit, and further discussion captured in https://tools.ietf.org/html/rfc5756#section-1 highlights where aspects of this may be unwise.
In terms of other implementations, are you aware of implementations that limit in this way? Neither BoringSSL nor CryptoAPI do.
| Reporter | ||
Comment 4•8 years ago
|
||
(In reply to Ryan Sleevi from comment #3)
> There's not a particularly compelling reason to limit, and further
> discussion captured in https://tools.ietf.org/html/rfc5756#section-1
> highlights where aspects of this may be unwise.
Section 1 of RFC 5756 does not talk about RSA-PSS at all...
The generic statement is about negotiation of KDFs. RSA-PSS is a _padding_ format.
Section 2 only talks about when the rsa-pss parameters need to be included or not. Not when the RSA-PSS key _type_ needs to be used in the first place.
To reiterate, I am not talking about a situation where we have rsa-pss key limited to using a single hash and MGF.
> In terms of other implementations, are you aware of implementations that
> limit in this way? Neither BoringSSL nor CryptoAPI do.
CryptoAPI does support RSA-PSS keys?
BoringSSL developers are not infallible - just recently we have discovered that they expect SHA-1 fallback in Certificate Verify when the RFC explicitly forbids that behaviour.
| Reporter | ||
Comment 5•8 years ago
|
||
I've tested OpenSSL 1.1.1-dev (master fb9163ba4d) and it rejects such signatures by sending a decode_error and printing
139899071163200:error:1414D172:SSL routines:tls12_check_peer_sigalg:wrong signature type:ssl/t1_lib.c:913:
BoringSSL (master d5dda9b803f) and it rejects it too with a decode_error and prints:
40160216:error:06000066:public key routines:OPENSSL_internal:DECODE_ERROR:/home/hkario/dev/boringssl/crypto/evp/evp_asn1.c:110:
so it doesn't look to me like they don't limit rsa-pss key usage, exactly as the RFC requires them to do
Comment 6•8 years ago
|
||
Updated•8 years ago
|
Priority: -- → P3
Updated•7 years ago
|
Attachment #8960980 -
Attachment is obsolete: true
Comment 7•7 years ago
|
||
This seems to be fixed as part of bug 1428208 (thank you Martin!). Closing.
Assignee: nobody → martin.thomson
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.39
| Reporter | ||
Comment 8•7 years ago
|
||
I have verified client side using 187c81f83011
- NSS rejects PKSC#1 v1.5 signature in ServerKeyExchange message in TLS 1.2 when the server has a rsa-pss key (including sending the correct alert - illegal_parameter)
but the server side still has incorrect behaviour:
- it doesn't ask for rsa_pss_pss_* signatures, just rsa_pss_rsae_*
- when it receives rsa_pss_pss_sha256 signature from rsa-pss certificate it rejects it with decrypt_error instead illegal_parameter
- same if a rsa-pss certificate is used to make rsa_pss_rsae_sha256 signature, rejected with decrypt_error instead of illegal_parameter
there may be more issues, I haven't updated test-rsa-pss-sigs-on-certificate-verify.py to draft-28 yet (https://github.com/tomato42/tlsfuzzer/issues/449)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
| Assignee | ||
Comment 9•7 years ago
|
||
OK, so we got the main one right. There are probably some client certificate tests to add (and code to fix).
> it doesn't ask for rsa_pss_pss_* signatures, just rsa_pss_rsae_*
This is the default configuration. You need to enable the pss_pss signature schemes with SSL_SignatureSchemePrefSet(). If you did call that function, then we have a serious problem.
> when it receives rsa_pss_pss_sha256 signature from rsa-pss certificate it rejects it with decrypt_error instead illegal_parameter
> [same for the reverse]
Seems simple enough to fix. There are multiple paths, but I assume that you are talking about a PSS client certificate. That code is here:
https://searchfox.org/nss/rev/397d9b45e399c0035d510d7af48b1aec38a6ec05/lib/ssl/ssl3con.c#9573
I found a bug with DH server key exchange when checking on this: We don't seem to be sending an alert at all (ECDH sends illegal_parameter):
https://searchfox.org/nss/rev/397d9b45e399c0035d510d7af48b1aec38a6ec05/lib/ssl/ssl3con.c#6966
Daiki, do you think that you could look at fixing these? I could do this, but it would take a while.
Flags: needinfo?(dueno)
Comment 10•7 years ago
|
||
Thank you for the pointers; yes, I will look into them.
Flags: needinfo?(dueno)
| Reporter | ||
Comment 11•7 years ago
|
||
> This is the default configuration. You need to enable the pss_pss signature schemes with SSL_SignatureSchemePrefSet(). If you did call that function, then we have a serious problem.
I'm using tstclnt, strsclnt and selfserv, can I make them do that? I'm testing interoperability, so using gtests is not a good fit for it.
| Assignee | ||
Comment 12•7 years ago
|
||
We do not have a switch for that. Sorry. Opened bug 1482747.
Comment 13•7 years ago
|
||
This fixes the corner cases where incorrect alert is sent (or even no alert is sent):
- when the client's CertificateVerify is signed with rsa_pss_pss_*, while the certificate is RSA
- when the client's CertificateVerify is signed with rsa_pss_rsae_*, while the certificate is RSA-PSS
- when ServerKeyExchange is signed with an inconsistent signature scheme with the server certificate
| Assignee | ||
Comment 14•7 years ago
|
||
Comment on attachment 8999893 [details]
Bug 1414931, send correct alert on inconsistent signature scheme
Martin Thomson [:mt:] has approved the revision.
Attachment #8999893 -
Flags: review+
Comment 15•7 years ago
|
||
Pushed as:
https://hg.mozilla.org/projects/nss/rev/6349fa699c3b
Let's close this bug once we know bug 1482747 sufficiently cover the first issue in comment 8.
| Reporter | ||
Comment 16•7 years ago
|
||
verified as fixed in nss 3.41.0
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•