tlsfuzzer / tlsfuzzer

SSL and TLS protocol test suite and fuzzer
GNU General Public License v2.0
549 stars 113 forks source link

test-tls13-certificate-verify.py: does not tolerate empty status request extension by server #633

Open nmav opened 4 years ago

nmav commented 4 years ago

Bug Report

According to TLS1.3:

A server MAY request that a client present an OCSP response with its
certificate by sending an empty "status_request" extension in its
CertificateRequest message.

However when the server sends this extension to tlsfuzzer it errors with:

test-tls13-certificate-verify.py:stdout:Traceback (most recent call last):
test-tls13-certificate-verify.py:stdout:  File "scripts/test-tls13-certificate-verify.py", line 586, in main
test-tls13-certificate-verify.py:stdout:    runner.run()
test-tls13-certificate-verify.py:stdout:  File "/home/nmav/cvs/gnutls-mine/tests/suite/tls-fuzzer/tlsfuzzer/tlsfuzzer/runner.py", line 237, in run
test-tls13-certificate-verify.py:stdout:    node.process(self.state, msg)
test-tls13-certificate-verify.py:stdout:  File "/home/nmav/cvs/gnutls-mine/tests/suite/tls-fuzzer/tlsfuzzer/tlsfuzzer/expect.py", line 1304, in process
test-tls13-certificate-verify.py:stdout:    self._process_extensions(state, cert_request)
test-tls13-certificate-verify.py:stdout:  File "/home/nmav/cvs/gnutls-mine/tests/suite/tls-fuzzer/tlsfuzzer/tlsfuzzer/expect.py", line 1257, in _process_extensions
test-tls13-certificate-verify.py:stdout:    handler(state, ext)
test-tls13-certificate-verify.py:stdout:  File "/home/nmav/cvs/gnutls-mine/tests/suite/tls-fuzzer/tlsfuzzer/tlsfuzzer/expect.py", line 419, in clnt_ext_handler_status_request
test-tls13-certificate-verify.py:stdout:    .format(CertificateStatusType.toStr(extension.status_type)))
test-tls13-certificate-verify.py:stdout:AssertionError: Unexpected status_type in status_request extension: None
test-tls13-certificate-verify.py:stdout:
tomato42 commented 4 years ago

yes, in "check sigalgs in cert request" we are specifying signature algorithms, which effectively sets expected set of extensions, changing the CertificateRequest parser into strict mode

we'll need to add support for -E option, similar to test-tls13-certificate-request.py script

nmav commented 4 years ago

From the error message, I think it tries to parser the status_request extension as a client_hello status request which has different form despite the same code point.

nmav commented 4 years ago

That also makes me wonder whether there are other client implementations doing the same.

tomato42 commented 4 years ago

From the error message, I think it tries to parser the status_request extension as a client_hello status request which has different form despite the same code point.

yes, RFC 8446, section 4.4.2.1

   A server MAY request that a client present an OCSP response with its
   certificate by sending an empty "status_request" extension in its
   CertificateRequest message.  If the client opts to send an OCSP
   response, the body of its "status_request" extension MUST be a
   CertificateStatus structure as defined in [RFC6066].

while the code is expecting the payload to be the same as the one expected in ClientHello message, it should instead verify that the extension does not have a payload

That also makes me wonder whether there are other client implementations doing the same.

I'm not aware of any implementation that supports status_request extension in client's Certificate: I'm sure OpenSSL doesn't support it, I almost sure that NSS doesn't