1330615 - malformed ALPN extension is rejected with incorrect alert messages

Status: NEW

(NSS :: Libraries, defect, P3)

Description

[Hubert Kario]

Description of problem:

The extension is defined as follows:

   The "extension_data" field of the
   ("application_layer_protocol_negotiation(16)") extension SHALL
   contain a "ProtocolNameList" value.

   opaque ProtocolName<1..2^8-1>;

   struct {
       ProtocolName protocol_name_list<2..2^16-1>
   } ProtocolNameList;

When the ProtocolName length is 0 or the protocol_name_list length is 0, NSS doesn't send the expected decode_error alert:

   decode_error
      A message could not be decoded because some field was out of the
      specified range or the length of the message was incorrect.  This
      message is always fatal and should never be observed in
      communication between proper implementations (except when messages
      were corrupted in the network).

Instead it sends illegal_parameter and no_application_protocol alerts.

Version-Release number of selected component (if applicable):
nss-3.27.1-12.el6.x86_64

How reproducible:
always

Steps to Reproduce:
git clone https://github.com/tomato42/tlsfuzzer.git
pushd tlsfuzzer
git checkout alpn-test # won't be necessary in future
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
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 -d sql:./nssdb -p 4433 -V tls1.0: -H 1 -n localhost -Q
# in another terminal, same directory
PYTHONPATH=tlsfuzzer python tlsfuzzer/scripts/test-alpn-negotiation.py 'empty extension' 'empty list'

Actual results:
empty extension ...
Error encountered while processing node <tlsfuzzer.expect.ExpectAlert object at 0x2dd9790> (child: <tlsfuzzer.expect.ExpectClose object at 0x2dd97d0>) with last message being: <tlslite.messages.Message object at 0x2dd9f90>
Error while processing
Traceback (most recent call last):
  File "test-alpn-negotiation.py", line 218, in main
    runner.run()
  File "/tmp/tmp.pejrU9l8lU/tlsfuzzer/tlsfuzzer/runner.py", line 168, in run
    node.process(self.state, msg)
  File "/tmp/tmp.pejrU9l8lU/tlsfuzzer/tlsfuzzer/expect.py", line 542, in process
    raise AssertionError(problem_desc)
AssertionError: Alert description 47 != 50

empty list ...
Error encountered while processing node <tlsfuzzer.expect.ExpectAlert object at 0x2dd9650> (child: <tlsfuzzer.expect.ExpectClose object at 0x2dd9690>) with last message being: <tlslite.messages.Message object at 0x2ddaf10>
Error while processing
Traceback (most recent call last):
  File "test-alpn-negotiation.py", line 218, in main
    runner.run()
  File "/tmp/tmp.pejrU9l8lU/tlsfuzzer/tlsfuzzer/runner.py", line 168, in run
    node.process(self.state, msg)
  File "/tmp/tmp.pejrU9l8lU/tlsfuzzer/tlsfuzzer/expect.py", line 542, in process
    raise AssertionError(problem_desc)
AssertionError: Alert description 120 != 50

Expected results:
empty extension ...
OK

empty list ...
OK

Comment 1

[Eric Rescorla (:ekr)]

I agree that this is a bug in TLS 1.3 mode.

The alert categories in TLS 1.2 were more vague, so I think it's at least arguable it's compliant with RFC 5246. I imagine you will find this kind of alert defect in a number of places.

Comment 2

[Hubert Kario]

(In reply to Hubert Kario from comment #0)
> Version-Release number of selected component (if applicable):
> nss-3.27.1-12.el6.x86_64

while it's problematic there too, I have tested it with vanilla 3.28.0 with the exact same result.

(In reply to Eric Rescorla (:ekr) from comment #1)
> I agree that this is a bug in TLS 1.3 mode.
> 
> The alert categories in TLS 1.2 were more vague, so I think it's at least
> arguable it's compliant with RFC 5246. I imagine you will find this kind of
> alert defect in a number of places.

that's true, but I find it in far fewer places than I expected, and not investigating where the difference comes from already made me miss the CVE-2016-5285 first time 'round I was looking at DHE handshakes so it has value (not to mention that it also makes fingerprinting harder if all implementations follow RFC's to the letter)

Comment 3

[Andy bentley]

Thunderbird 52.1.0  w/  nss.x86_64 3.30.2-1.1 Omits both the Extension : server_name section and Extension : Application Layer Protocol Negotiation section from the Client Hello.  Config max TLS = TLS1.2 min TLSv1.0 Works fine in Firefox 53.0.2, and evolution 3.22.6-2
Results in a TLSv1 Alert Fatal  Description: Protocol Version Alert 21, Level Fatal (2), Description: Protocol Version (70)