Closed Bug 1353750 Opened 3 years ago Closed 3 years ago

malformed extended master secret extension causes incorrect alert

Categories

(NSS :: Libraries, enhancement)

3.29
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hkario, Assigned: mt)

Details

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
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Assignee: nobody → martin.thomson
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)
(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.
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.
I agree with MT. The support is already there, it should be correct.
https://hg.mozilla.org/projects/nss/rev/dba170cbc3b4
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.32
You need to log in before you can comment on or make changes to this bug.