tlsfuzzer icon indicating copy to clipboard operation
tlsfuzzer copied to clipboard

Draft: tls13-certificate-compression: smoke-test RFC 8879

Open t184256 opened this issue 2 years ago • 4 comments

Description

Smoke-test RFC 8879 TLS Certificate Compression. Depends on https://github.com/tlsfuzzer/tlslite-ng/pull/484

Motivation and Context

GnuTLS has support for it now, I want it to test it against tlsfuzzer.

Checklist

  • [ ] I have read the CONTRIBUTING.md document and my PR follows change requirements therein
  • [ ] the changes are also reflected in documentation and code comments
  • [ ] all new and existing tests pass (see CI results)
  • [ ] test script checklist was followed for new scripts
  • [ ] new test script added to tlslite-ng.json and tlslite-ng-random-subset.json
  • [ ] new and modified scripts were ran against popular TLS implementations:
    • [ ] OpenSSL
    • [ ] NSS
    • [ ] GnuTLS
  • [ ] required version of tlslite-ng updated in requirements.txt and README.md

This change is Reviewable

t184256 avatar Oct 11 '22 13:10 t184256

that doesn't seem right... If the client asks for an algorithm that the server doesn't support, then the server should simply continue with no compression, not abort the connection

It's less a negotiation, and more of a notification. The client (and server if we flip it), is saying "these are the compression algorithms I support". It's up to the peer to decide if they actually want to (or can!) send a compressed certificate. But yes, unsupported algorithms ought to be ignored.

tmshort avatar Nov 16 '22 14:11 tmshort

scripts/test-tls13-certificate-compression-verify.py line 589 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

well, that should be covered by unit tests, shouldn't it?

no, I meant, that I haven't messed up calculating base_uncompressed_length earlier in this script

t184256 avatar Dec 06 '22 18:12 t184256

scripts/test-tls13-certificate-compression-verify.py line 770 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

then I think those should be special dedicated test cases, not the default (like I wrote elsewhere, some libraries have hard limits for Handshake message sizes, so by using max size we may end up testing different code than we intend to)

as in, moved "bomb" ones to a separate file?

t184256 avatar Dec 06 '22 18:12 t184256

scripts/test-tls13-certificate-compression-verify.py line 770 at r4 (raw file):

Previously, tomato42 (Hubert Kario) wrote…

no, they're fine in this file Consider this: you have an implementation that won't accept a Certificate message that's bigger than 1 MiB (entirely reasonable, even for RSA keys). That check, if the Certificate is larger than that may be implemented right there on record layer. Then instead of testing if the compressed certificate handling is correct, we hit the hard limit on Certificate message length, not the handling of a compressed message that decompresses to an empty string.

ah. I failed to notice this is the empty case, not the bomb case

t184256 avatar Dec 08 '22 10:12 t184256