tlsfuzzer / tlsfuzzer

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

The test-version-numbers script shouldn't offer DTLS #169

Open randombit opened 6 years ago

randombit commented 6 years ago

The script tests with large version numbers, specifically (254,254), to ensure version tolerance. However major=254 corresponds to DTLS (https://tools.ietf.org/html/rfc6347) so an implementation that shares client hello parsing between TLS and DTLS will attempt to parse it as a DTLS hello (which has a different format, due to the replay cookie) and return a decoding error alert rather than negotiating TLS v1.2

Its certainly debatable if it's a good idea to share the parsing code in this way, but in any case it seems like while TLS might theoretically use a larger version in the future (though given TLS v1.3 I'd say not likely at this point) it will certainly never use a major version that conflicts with another deployed TLS variant.

For my purposes I just changed the major version in the test to 253.

nmav commented 6 years ago

Actually for the exact same reason you give, it may make sense to have this test in duplicate. One with the DTLS numbers to ensure that the implementation doesn't use the same parser when not parsing DTLS (it is not only the transport layer, the protocols are different), and another with a non-DTLS allocated value.

That way you could disable the test that you don't support and only enable the other.

tomato42 commented 6 years ago

major=254 is defined only for DTLS, so if you use TCP as the transport (the only one that tlsfuzzer can use) 254 should not be treated specially

that being said, I haven't read the DTLS RFCs in detail, so I may be wrong - references to RFC sections that show this is the case welcome

randombit commented 6 years ago

Quoting DTLS RFC 6347 section 4.1.1.1 discussing PMTU issues "For DTLS over TCP or SCTP, ..." so clearly it was anticipated that some use of DTLS over TCP would occur.

randombit commented 6 years ago

draft-ietf-tls-dtls13-22 has the same language

tomato42 commented 6 years ago

you're of course right

could you be so kind and propose your local change in a PR?

nmav commented 6 years ago

Quoting DTLS RFC 6347 section 4.1.1.1 discussing PMTU issues "For DTLS over TCP or SCTP, ..." so clearly it was anticipated that some use of DTLS over TCP would occur.

That is right, you could use DTLS over TCP but you wouldn't expect a configured as TLS server to negotiate DTLS, or parse the TLS messages as DTLS messages. If you have such a server I suspect that this is an exception rather than the rule.

tomato42 commented 6 years ago

I don't see why you couldn't offer both TLS and DTLS on the same port...

yes, detecting it and reporting that DTLS over TLS is enabled probably should happen in tlsfuzzer, but I don't think we're there just yet

nmav commented 6 years ago

I don't see why you couldn't offer both TLS and DTLS on the same port...

I don't say that you couldn't under any conditions. I say that it is a condition that would be worth detecting, and that's why I proposed using two different tests. To check the behavior if a DTLS version is seen, and a test to check the behavior when an unassigned version is seen.

randombit commented 6 years ago

I agree with nmav there should be two tests because detecting such dual-stack servers is useful for diagnostic purposes. I think (from my limited understanding of how tlsfuzzer works based on reading the code so far) it would be easy to add a DTLS-over-TCP test that actually attempted a DTLS client hello and checked for a response (not necessarily doing the full handshake but just sending client hello and seeing if the server replies with a DTLS hello). It "should" just be a simple extension to tlslite-ng. I can try writing up a patch this weekend if there is interest.

tomato42 commented 6 years ago

I have DTLS in plans, so DTLS over TCP definitely could be the first step of it