1720235 - SSL handling of signature algorithms ignores environmental invalid algorithms.

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Robert Relyea]

Attachment: nss-3.67-fix-ssl-alerts.patch

Our QA is quite extensive on handling of alert corner cases. Our code that checks if a signature algorithm is supported ignores the role of policy. If SHA1 is turned off by policy, for instance, we only detect that late in the game. This shows up in our test cases as decrypt_alerts rather than illegal_parameter or handshake_error alerts. It also shows up in us apparently accepting a client auth request which only has invalid alerts.

We also don't handle filtering out signature algorithms that are illegal in tls 13 mode.

This patch not only fixes these issues, but also issues where we proposing signature algorithms in server mode that we don't support by policy.

This patch is for reference. It still requires 1) new test cases, and 2) handling of signing algorithm policy, not just hash policies.

Comment 1

[Benjamin Beurdouche [:beurdouche]]

Bob, can you please set severity for this? Thanks! : )

Comment 2

[Robert Relyea]

Attachment: Bug 1720235 SSL handling of signature algorithms ignores environmental invalid algorithms.

Our QA is quite extensive on handling of alert corner cases. Our code that checks if a signature algorithm is supported ignores the role of policy. If SHA1 is turned off by policy, for instance, we only detect that late in the game. This shows up in our test cases as decrypt_alerts rather than illegal_parameter or handshake_error alerts. It also shows up in us apparently accepting a client auth request which only has invalid alerts.

We also don't handle filtering out signature algorithms that are illegal in tls 13 mode.

This patch not only fixes these issues, but also issues where we proposing signature algorithms in server mode that we don't support by policy.

This patch includes:

In gtests:

1. adding support for policy in ssl_gtests. Currently both the server an client will run with the same policy. The patch allows us to set policy on one and keeping the old policy on the other.

2. Update extension tests which failed in tls 1.3 because the patch now correctly rejects illegal tls 1.3 auth values. The test was updated to use a legal auth value in tls 1.3 (so we are correctly testing the format issue.

3. Update extension tests to handle the case where we try to use an illegal value for tls 1.3.

4. add tests to ssl_auth_unittests.cc to make sure we can properly connect even when several auth methods are turned off by policy (make sure we don't advertize them on the client side, and that the server doesn't select them when the client doesn't advertize them).

5. add tests to ssl_auth_unittests.cc to make sure we don't send empty client auth requests when the requester only sends invalid auth requests.

patch itself:

1. The handling of policy checks for ssl schemes were scattered in various locations. I've consolidated them into a single function. That function now checks for NSS_ALG_USE_IN_ANY_SIGNATURE as if this is off by policy, we will fail if we try to use the algorithm in a signature in any case. NSS now supports policy on all signature algorithms, not just DSA, so we need to check the policy of all the algorithms.

2. to support the policy check on the signature algorithms, I added a new ssl_AuthTypeToOID, which also replaces our switch in checking if the SPKI matches our auth type.

3. ssl_SignatureSchemeValid now accepts an spkiOid of SEC_OID_UNKNOWN. To allow us to filter signature schemes based on version and policy restrictions before we try to select a certificate. This prevents us from sending empty client auth messages when we are presented with only invalid signature schemes.

4. We filter supported algorithms against policy early, preventing us from sending, or even setting invalid algorithms if they are turned off by policy.

5. ssl ConsumeSignatureScheme was handling alerts inconsistently. The Consume could send an allert in it's failure case, but the check of scheme validity wouldn't sent an alert. The collers were inconstent as well. Now ssl_ConsumeSignatureScheme always sends and alert on failure, and the callers do not.

Comment 3

[Robert Relyea]

https://hg.mozilla.org/projects/nss/rev/c71bb1bedf7d9d45b34cd483097d30c8a3f01602