1280439 - TLS 1.3: Allow RSA-PSS signatures in CertificateVerify messages

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1280439-Use-RSA-PSS-for-signatures-in-TLS-1.3-Ce.patch

(I can't upload patches to Rietveld for some strange reason, but this shouldn't be too hard to review here.)

What this patch does is implicitly use RSA-PSS for CertificateVerify messages in 1.3, ssl_gtests are passing.

I think that the next patch I'll be working on should announce support for RSA-PSS in the "signature_algorithms" extension, and we should be able to parse those too. I'm not completely sure yet where we use that data and what other parts might need fixing though.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1280439-Use-RSA-PSS-for-signatures-in-TLS-1.3-Ce.patch, v2

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Just noticed that the signature is RSA-PSS but it's announced as PKCS#1 v1.5 signature in tls13_SendCertificateVerify(). Also we need some negotiations as Ekr mentioned on IRC, so that we don't break everyone.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8770577 [details] [diff] [review]
0001-Bug-1280439-Use-RSA-PSS-for-signatures-in-TLS-1.3-Ce.patch, v2

Maybe let's cancel this for now.

Comment 5

[Martin Thomson [:mt:]]

(In reply to Tim Taubert [:ttaubert] from comment #3)
> Just noticed that the signature is RSA-PSS but it's announced as PKCS#1 v1.5
> signature in tls13_SendCertificateVerify(). Also we need some negotiations
> as Ekr mentioned on IRC, so that we don't break everyone.

That's right.  I think that we need more here for other reasons as well.

I would like to see TLS 1.3 use the new code point that I established for PSS certificates as well.  That means looking at the signature_algorithms^Wsignature_schemes^Wauthentication_schemes and then picking a certificate based on that.

Here's a few other places to look at:

Changing this would allow you to configure a PKCS#1.5 cert so that it can be used for PSS:
http://searchfox.org/nss/rev/36a2402180174ff0095fa9b206cac75eb4c2074d/lib/ssl/sslcert.c#416-435
Basically, if the cert is a signature cert and the authType is set to ssl_auth_rsa_pss, then put it in that slot.

Then you need to look at ssl3_SelectServerCert and add a carve-out for TLS 1.3.
http://searchfox.org/nss/rev/36a2402180174ff0095fa9b206cac75eb4c2074d/lib/ssl/ssl3con.c#8795
Or, maybe better, add a wrapper in tls13con.c that searches for ssl_auth_rsa_pss if ss->ssl3.hs.kea_def->authKeyType == ssl_auth_rsa_sign.

Then you only have to add a switch based on ss->serverCert->certType.authType and invoke ssl3_SignHashes or tls13_SignHashes appropriately.

I hope that this helps.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0003-Bug-1280439-TLS-1.3-Allow-RSA-PSS-signatures-in-Cert.patch

Current WIP patch. This works but has no tests yet and I need to do some more plumbing.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

The cipher suite info offers an SSLAuthType, used to denote the "auth type" for a cipher suite. Should/can we remove "ssl_auth_rsa_pss"? It seems that even when using PSS to sign, ssl_auth_rsa_sign would be the correct "auth type". And that's as much as the cipher suite info should be able to give us.

We could then have the channel info offer a signature scheme field, that allows to look up what signature scheme was used, and to differentiate between PKCS#1 v1.5 and PSS. If the scheme is null then the cipher suite auth is static RSA or PSK, or null.

The problem is that SignatureScheme is defined in ssl3prot.h and thus not exposed, we'd have to put it in sslt.h and I'm not sure we want that. We could also have a separate type SSLAuthScheme or something that basically is basically SSLSignType with differentiation between PKCS1 and PSS.

mt, what do you think?

Another thing we need to fix is that currently, when calling SSL_ConfigServerCert() with ssl_auth_rsa_pss and an RSA cert, we would disable RSA cipher suites because kea_def->authType is ssl_auth_rsa_sign, and that's what we compare. Could we have only a single slot for RSA certs if we remove ssl_auth_rsa_pss from SSLAuthType? Does it make sense to be able to configure an RSA cert that is used for only PKCS1, or only PSS?

Comment 8

[Martin Thomson [:mt:]]

Let me try to separate this into desiderata:
1. We want to signal that we have PSS support
2. We want to use PSS whenever we have an RSA certificate and the peer indicates that they have PSS support
3. We want to know whether PSS has been used, though it might only be necessary to know that the peer used PSS

Now based on that I sort of realize that we are in a sticky situation with the PSS slot that I created because all of our configuration for cipher suites includes a generic "RSA signature" label (ssl_auth_rsa_sign), which also more specifically identifies PKCS#1 v1.5.  In a way, it's unfortunate that PSS was exposed in the API because, as you note, just using the rsa signature slots is probably best.  But it's also somewhat nice in the sense that we have a slot that can be restricted to ONLY using PSS.

Here's what I think we can do for each of these:

1. indicate that we support PSS (easy)

2a. when picking a certificate, add a special check: if this is an rsa certificate and pss is supported, look for the _rsa_pss cert first and pick that in preference to the _rsa_sign one

2b. when signing, check the type of the certificate and use PSS if that is the type; for client certs (though maybe for 1.2 only) there is a little extra hacking and plumbing needed, but you could defer support for PSS on the client side in 1.2.

2c. when filtering suites based on available certs, add another special check

3. Translate the signature scheme on the peer's signature into SSLAuthType and save that (or save the scheme and translate when SSL_GetChannelInfo is called).

Now, the alternative is - as I think that you were suggesting - to just plumb PSS into the _rsa_sign slot and switch hit when it comes to signing.  That's simpler for the selection and filtering code, but it has two drawbacks: firstly, we don't get to say that a cert is only good for PSS, and we would then have a vestigal code point that we never use.  I don't think that I mind either way: keeping the code simple seems like a real benefit, and I don't care that much for restricting certificates to PSS-only.

I don't think that we need to expose the signature scheme, mainly on the grounds that exposing more things just means more work.  If we take your proposed option and remove ssl_auth_rsa_pss, then it might be necessary.

Note that ekr is looking at implementing the new orthogonal negotiation stuff, which means that there will be a need to expose SSLKEAType and SSLAuthType on SSL_GetChannelInfo() anyway.

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Patch at: https://codereview.appspot.com/305460043

https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=7e739a51e82e6f9a314f4fb1cf0e72b12937211e

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/10b045de68e6

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/992fdbf20cf8

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/2ac96fe5bcf6