1342471 - malformed key shares cause incorrect alert messages

Status: NEW

(NSS :: Libraries, defect, P3)

Description

[Hubert Kario]

When a client sends invalid x25519 key share to the server, the server replies with incorrect alert messages.

When the key share is longer or shorter than 32 bytes, the server replies with handshake_failure instead of illegal_parameter.

When the key share has invalid x25519 key share (the highest bit of the most significant byte is set) the server continues the connection and in the end replies with bad_record_mac instead of illegal_parameter.

Reproducer:
git clone https://github.com/tomato42/tlsfuzzer.git
pushd tlsfuzzer
git checkout basic-x25519
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 rfc7748
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-x25519.py 'too big x25519 key share' 'too small x25519 key share' 'x25519 key share with high bit set' 'sanity - negotiate x25519'


Additional info:
draft-ietf-tls-rfc4492bis-12:
   Since there are some implementation of the X25519 function that
   impose this restriction on their input and others that don't,
   implementations of X25519 in TLS SHOULD reject public keys when the
   high-order bit of the final byte is set (in other words, when the
   value of the rightmost byte is greater than 0x7F) in order to prevent
   implementation fingerprinting.

RFC 5246:
   illegal_parameter
      A field in the handshake was out of range or inconsistent with
      other fields.  This message is always fatal.

Comment 1

[Franziskus Kiefer [:franziskus]]

We send handshake_failure for all malformed client key exchange messages.
https://nss-review.dev.mozaws.net/D267

Comment 2

[Franziskus Kiefer [:franziskus]]

The high-bit check was removed from rfc4492bis [1]. But the other issue is still valid.

[1] https://github.com/tlswg/rfc4492bis/pull/38

Comment 3

[Hubert Kario]

Empty key share is also a problem:

$ python test-x25519.py 'empty x25519 key share'
Error encountered while processing node <tlsfuzzer.expect.ExpectAlert object at 0x1df1790> (child: <tlsfuzzer.expect.ExpectClose object at 0x1df17d0>) with last message being: <tlslite.messages.Message object at 0x1df4090>
Error while processing
Traceback (most recent call last):
  File "test-x25519.py", line 673, in main
    runner.run()
  File "/tmp/tmp.Euj5MSlvEv/tlsfuzzer/tlsfuzzer/runner.py", line 168, in run
    node.process(self.state, msg)
  File "/tmp/tmp.Euj5MSlvEv/tlsfuzzer/tlsfuzzer/expect.py", line 543, in process
    raise AssertionError(problem_desc)
AssertionError: Alert description 47 != 50


Since the RFC specifies the minimal size of the key share to be 1 byte, an empty one is a malformed message, so it needs to be rejected with decode_error alert (50).

Comment 4

[Hubert Kario]

Script updated to remove the high-bit check.
I also added test cases with key shares that should result in all-zero shared secret.
(Also, use of rfc7748 branch in tlslite-ng is no longer necessary).