Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLS-RFC compliance #469

Open
mmaehren opened this issue Jun 3, 2021 · 4 comments
Open

TLS-RFC compliance #469

mmaehren opened this issue Jun 3, 2021 · 4 comments
Labels
bug unintented behaviour in tlslite-ng code
Milestone

Comments

@mmaehren
Copy link

mmaehren commented Jun 3, 2021

Hi,
we (@jurajsomorovsky @ic0ns @mmaehren @XoMEX @Kavakuo) are performing an analysis of the RFC-compliance of open-source TLS implementations. Below we list our findings for this implementation. We admit that some are rather nit-picky, but we added them for the sake of completeness. We tried to keep the descriptions brief and didn’t want to spam the issues section so feel free to split up the list into individual issues as you see fit.
If you disagree with our interpretation of certain RFC statements, please leave feedback as we’re interested in your view.

Our results apply to the default configuration of version 0.8.0-alpha39. We used the example implementations for client and server.

[S] = Applies to server
[C] = Applies to client
[C+S] = Applies to both

Misc

  • [S] tlslite-ng allows to resume a session even if the previous session was terminated with a fatal close_notify

    • This is a corner case as an RFC-compliant peer should never send a 'fatal' close_notify
    • This only holds for a fatal_close notify. Other fatal alerts are handled properly
    • RFC 5246 - 7.2.2 Error Alerts

      Description: Thus, any connection terminated with a fatal alert MUST NOT be resumed.

  • [S] tlslite-ng accepts records with a deprecated version

    • RFC 8446 - D5 Security Restrictions Related to Backward Compatibility

      Implementations MUST NOT send any records with a version less than 0x0300. Implementations SHOULD NOT accept any records with a version less than 0x0300 (but may inadvertently do so if the record version number is ignored completely).

Session not aborted

  • [C] upon receiving a ServerHello with a SupportedVersions extension containing 0x0303 (TLS 1.2), tlslite_ng negotiates TLS 1.2

    • RFC 8446 - 4.2.1 Supported Versions

      If the "supported_versions" extension in the ServerHello contains a version not offered by the client or contains a version prior to TLS 1.3, the client MUST abort the handshake with an "illegal_parameter" alert.

  • [C] upon receiving a ServerHello that contains unproposed extensions (besides the TLS 1.3 Cookie extension)

    • RFC 8446 - 7.4.1.4. Hello Extensions

      If a client receives an extension type in ServerHello that it did not request in the associated ClientHello, it MUST abort the handshake with an unsupported_extension fatal alert.

  • [C] upon receiving a ServerHello that negotiates an AEAD cipher suite and encrypt-then-MAC at the same time

    • RFC 7366 - 3. Applying Encrypt-then-MAC

      If a server receives an encrypt-then-MAC request extension from a client and then selects a stream or Authenticated Encryption with Associated Data (AEAD) ciphersuite, it MUST NOT send an encrypt-then-MAC response extension back to the client.

  • [C] upon receiving a TLS 1.3 HelloRetryRequest with invalid content(cipher suite, compression value, session id)

    • RFC 8446 - 4.1.4. Hello Retry Request

      Upon receipt of a HelloRetryRequest, the client MUST check the legacy_version, legacy_session_id_echo, cipher_suite, and legacy_compression_method as specified in Section 4.1.3

  • [C] upon receiving an EncryptedExtensions message that contains an extension that must not appear in this message

    • Tested with Padding and SupportedVersions extension
    • SupportedVersions requires the client-side structure to be accepted
    • RFC 8446 - 4.3.1. Encrypted Extensions

      The client MUST check EncryptedExtensions for the presence of any forbidden extensions and if any are found MUST abort the handshake with an "illegal_parameter" alert.

  • [C] upon receiving a ServerHello that negotiates TLS 1.3 but does not echo the client's session ID

    • RFC 8446 - 4.1.3 Server Hello

      A client which receives a legacy_session_id_echo field that does not match what it sent in the ClientHello MUST abort the handshake with an "illegal_parameter" alert.

  • [C] upon receiving an encrypted Finished message without receiving a ChangeCipherSpec first. tlslite-ng attempts to parse the record as unencrypted - for cipher suites with a random IV at the beginning of the record, tlslite_ng parses the first bytes of this IV as handshake message type and message length fields. As this 'length' is usually not within the bounds of the record, tlslite_ng waits for more records

    • the record should be rejected based on the content type or, if a session ticket message is expected, based on the handshake message type field of the wrongfully parsed record
  • [C+S] upon receiving an encrypted CCS or a CCS after a completed handshake when TLS 1.3 is used

    • Judging by the commits this may have been fixed already
    • RFC: 8446, Section: 5. Record Protocol

      An implementation which receives any other change_cipher_spec value or which receives a protected change_cipher_spec record MUST abort the handshake with an "unexpected_message" alert.
      If an implementation detects a change_cipher_spec record received before the first ClientHello message or after the peer's Finished message, it MUST be treated as an unexpected record type (though stateless servers may not be able to distinguish these cases from allowed cases).

  • [S] upon receiving a Padding extension that contains bytes other than 0x00

    • Please leave a comment if your implementation does not support this extension and hence ignores the content
    • RFC 7685 - 3. Padding Extension

      The client MUST fill the padding extension completely with zero bytes, although the padding extension_data field may be empty.

  • [S] upon receiving a SupportedPointFormat extension that only accepts compressed points or an invalid format

    • tlslite-ng ignores the listed formats
    • 8422 - 5.1. Client Hello Extensions

      If the client sends the extension and the extension does not contain the uncompressed point format, and the client has used the Supported Groups extension to indicate support for any of the curves defined in this specification, then the server MUST abort the handshake and return an illegal_parameter alert.

  • [S] upon receiving a ClientHello that contains elliptic curve extensions but no ECC cipher suite

    • 8422 - 4. TLS Extensions for ECC

      The client MUST NOT include these extensions in the ClientHello message if it does not propose any ECC cipher suites.

  • [S] upon receiving a ClientHello that negotiates TLS 1.3 but does not set the legacy version field to 0x0303

    • RFC 8446 - 4.1.2 Client Hello

      In TLS 1.3, the client indicates its version preferences in the "supported_versions" extension (Section 4.2.1) and the legacy_version field MUST be set to 0x0303, which is the version number for TLS 1.2.

Only session closed but no alert sent

  • [C] upon receiving an invalid verify_data in a TLS 1.3 Finished message

    • RFC 8446 - 4.4.4. Finished

      Recipients of Finished messages MUST verify that the contents are correct and if incorrect MUST terminate the connection with a "decrypt_error" alert.

  • [C] upon receiving an invalid CertificateVerify message (invalid signature scheme / signature or missing signature)

  • [C] upon receiving a second TLS 1.3 HelloRetryRequest

    • RFC 8446 - 4.1.4 Hello Retry Request

      If a client receives a second HelloRetryRequest in the same connection (i.e., where the ClientHello was itself in response to a HelloRetryRequest), it MUST abort the handshake with an "unexpected_message" alert.

  • [C] upon receiving a ServerHello with a KeyShare extension that contains a NamedGroup for which the client did not send a KeyShare (e.g X448)

Different alert sent than defined by the RFC

  • [C] upon receiving a ServerHello with an SSL2 protocol version set in the version field, tlslite_ng replies with 0x80 02 02 46 which is an SSL2 record header carrying a TLS alert (fatal, protocol_version)

  • [S] when there are no overlapping parameters. tlslite-ng sends an 'illegal parameter' alert

    • RFC 8446 - Section 4.1.1 Cryptographic Negotiation

      If the server is unable to negotiate a supported set of parameters (i.e., there is no overlap between the client and server parameters), it MUST abort the handshake with either a "handshake_failure" or "insufficient_security" fatal alert (see Section 6).

  • [C+S] when another record type interleaves handshake messages

    • Tested using various alerts interleaving CH fragments
    • tlslite-ng closes the connection using a close_notify and TCP FIN
    • We'd expect a fatal alert to be sent
    • RFC 8446 - Section 5.1 Record Layer

      Handshake messages MUST NOT be interleaved with other record types. That is, if a handshake message is split over two or more records, there MUST NOT be any other records between them.

  • [C] upon receiving a record with an undefined record content type. tlslite-ng sends a 'record overflow' alert

    • This seems to be caused by SSL2 parsing code as our test uses the content type 0xFF
    • RFC 5246 - 6. The TLS Record Protocol

      If a TLS implementation receives an unexpected record type, it MUST send an unexpected_message alert.

@tomato42 tomato42 added the bug unintented behaviour in tlslite-ng code label Jun 4, 2021
@tomato42 tomato42 added this to the v0.8.0 milestone Jun 4, 2021
@tomato42
Copy link
Member

tomato42 commented Jun 4, 2021

Thanks for the report!
I'll go through them one by one later, two items caught my eye though:

when there are no overlapping parameters. tlslite-ng sends an 'illegal parameter' alert

Could you explain in what situation it happens? In majority of cases it should send the correct handshake_failure or insufficient_security.

upon receiving a ClientHello that contains elliptic curve extensions but no ECC cipher suite

I don't think enforcing that is useful; the extension may have been included because of an ECC ciphersuite that's unknown to the server, and even if all the ciphersuites are known, that's rather brittle code that does not increase interoperability or security.

@mmaehren
Copy link
Author

mmaehren commented Jun 4, 2021

Thank you for the feedback. We agree that enforcing this ECC cipher suite check may cause problems - that's a good point! I think the alert description for the lack of overlapping parameters may be caused by deprecated groups we include in the ClientHello we send in the test.

@tomato42
Copy link
Member

tomato42 commented Jun 4, 2021

I think the alert description for the lack of overlapping parameters may be caused by deprecated groups we include in the ClientHello we send in the test.

RFC 8446 says that deprecated groups "MUST NOT be offered or negotiated by TLS 1.3 implementations."; so that's what causing the rejection: if the ClientHello is recognised as a TLS 1.3 CH, and includes those obsolete curves then technically the message is malformed, so illegal_parameter is appropriate here. If you send them in TLS 1.2 you should see the expected handshake_failure, similar behaviour should be observed when using the unassigned curves (like from GREASE range).

@mmaehren
Copy link
Author

mmaehren commented Jun 4, 2021

Yes, it's a false positive on our end - sorry if my last comment didn't convey this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unintented behaviour in tlslite-ng code
Projects
None yet
Development

No branches or pull requests

2 participants