ssl3_CipherSuiteAllowedForVersionRange draft-TLS-1.3 version checks look off

RESOLVED FIXED in 3.28

Status

NSS
Libraries
P3
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: David Benjamin, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2351.3 Safari/537.36

Steps to reproduce:

I noticed this while I was reviewing the diff to import NSS 3.18 into Chromium:

https://hg.mozilla.org/projects/nss/file/60c35f8c4e0f/lib/ssl/ssl3con.c#l640

The version checks in ssl3_CipherSuiteAllowedForVersionRange look off. If I'm reading them right, if you even *advertise* draft-TLS-1.3, the non-PFS+AEAD SHA256 ciphers are removed from the ClientHello. I would think this check should only happen if the server selects TLS 1.3.

I believe the check should actually be:

  return vrange->max >= SSL_LIBRARY_VERSION_TLS_1_2 &&
            vrange->min < SSL_LIBRARY_VERSION_TLS_1_3;

which is almost, but not quite, the same.

Updated

3 years ago
Blocks: 1057463
(In reply to David Benjamin from comment #0)
> If I'm reading them right, if you even *advertise* draft-TLS-1.3, the
> non-PFS+AEAD SHA256 ciphers are removed from the ClientHello.

That's about right (though it's used only for checking if cipher suite configs are valid). But I'd prefer not to allow non-PFS cipher suites if not necessary. Since we didn't run into any problems with this so far I'd tend to close this as WONTFIX. But if we want to advertise non-PFS cipher suites if TLS 1.3 is enabled we should change it as proposed.
Flags: needinfo?(ekr)
Priority: -- → P3
Fixed in bug 1304832.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.28
Flags: needinfo?(ekr)
You need to log in before you can comment on or make changes to this bug.