1353750 - malformed extended master secret extension causes incorrect alert

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement)

Description

[Hubert Kario]

When client sends Client Hello that includes a malformed Extended Master Secret extension (one that includes some payload), the server replies with incorrect alert (handshake_failure) instead of a decode_error alert.

Version:
nss-3.29.3-1.0.fc24.x86_64

Reproducer:
git clone https://github.com/tomato42/tlsfuzzer.git
pushd tlsfuzzer
git checkout extended-master-secret-parameterise
git clone https://github.com/warner/python-ecdsa .python-ecdsa
ln -s .python-ecdsa/ecdsa ecdsa
git clone https://github.com/tomato42/tlslite-ng.git .tlslite-ng
pushd .tlslite-ng
git checkout ems-fixup
popd
ln -s .tlslite-ng/tlslite tlslite
popd
openssl req -x509 -newkey rsa -keyout localhost.key -out localhost.crt -nodes -batch -subj /CN=localhost
openssl pkcs12 -export -passout pass:  -out localhost.p12 -inkey localhost.key -in localhost.crt
mkdir nssdb
certutil -N -d sql:nssdb --empty-password
pk12util -i localhost.p12 -d sql:nssdb -W ''
selfserv -n localhost -p 4433 -d sql:./nssdb -V tls1.0: -H 1 -U 0 -G 

# in another terminal, same directory
PYTHONPATH=tlsfuzzer python tlsfuzzer/scripts/test-extended-master-secret-extension.py 'malformed extended master secret ext'


Result:
Server prints
selfserv: HDX PR_Read returned error -12260:
SSL received a malformed Client Hello handshake message.

Client prints:
AssertionError: Alert description 40 != 50

Comment 1

[Martin Thomson [:mt:]]

https://nss-review.dev.mozaws.net/D337

I found that we don't even validate the body of the signed certificate timestamps extension just today.  I'm going to lump that in together with this if that's OK with you.

Comment 2

[Hubert Kario]

I'm not sure if implementing support for experimental extension that google already is abandoning is a good use of time; unless you mean https://tools.ietf.org/html/draft-ietf-trans-rfc6962-bis-24 and not https://tools.ietf.org/html/rfc6962 (that is bug 1281469)

Comment 3

[Ryan Sleevi]

(In reply to Hubert Kario from comment #2)
> I'm not sure if implementing support for experimental extension that google
> already is abandoning is a good use of time; unless you mean
> https://tools.ietf.org/html/draft-ietf-trans-rfc6962-bis-24 and not
> https://tools.ietf.org/html/rfc6962 (that is bug 1281469)

It wouldn't be fair to characterize it as "abandoning support"

Chrome has offered no timeline for 6962-bis adoption, and clearly communicated strong support for 6962 for at least the next 3-5 years, as part of transitioning the WebPKI to CT.

Comment 4

[Martin Thomson [:mt:]]

Until we explicitly decide to remove the code, I intend for it to work correctly.  Given how inexpensive this fix is, I don't see an issue.

Comment 5

[Eric Rescorla (:ekr)]

I agree with MT. The support is already there, it should be correct.

Comment 6

[Martin Thomson [:mt:]]

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