Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.41k stars 2.58k forks source link

Message Sequence Non-conformance Issue #4376

Open bathooman opened 3 years ago

bathooman commented 3 years ago

Description


Non-conformance Bug

mbed TLS build:
Version: MbedTLS 2.22.0

Expected behavior
The DTLS RFC specifies the following requirement regarding the message sequence number:

The first message each side transmits in each handshake always has message_seq = 0. Whenever each new message is generated, the message_seq value is incremented by one.

Actual behavior

However, Our tests show that the mentioned version does not comply with the RFC and you can complete a handshake with the following values for the message_sequence:

CH0.message_seq: 1 HVR.message_seq: 1 CH2.message_seq: 2 SH.message_seq: 2 SHD.message_seq: 3 CKE.message_seq: 3

Clearly, in both cases of the client and the server, the message sequence has not started with 0. I have attached a handshake trace (PCAP) that shows the mentioned non-conformance issue.

mbedtls_invalid_mseq.zip

gilles-peskine-arm commented 3 years ago

Could you clarify whether the problem is that Mbed TLS is accepting manifestly incorrect sequence numbers in incoming messages, or that Mbed TLS is sending incorrect sequence numbers? Is Mbed TLS the client, the server or both?

hanno-becker commented 3 years ago

Could you share steps to reproduce? I ran a sample DTLS handshake with Mbed TLS client and server and see the sequence number starting at 0.

As for the server replying with sequence number 1 in the HVR to a ClientHello with sequence number 1, this is standards-compliant. Quoting Section 4.2.1 of the DTLS RFC:

When responding to a HelloVerifyRequest, the client MUST use the same parameter values (version, random, session_id, cipher_suites, compression_method) as it did in the original ClientHello. The server SHOULD use those values to generate its cookie and verify that they are correct upon cookie receipt. The server MUST use the same version number in the HelloVerifyRequest that it would use when sending a ServerHello. Upon receipt of the ServerHello, the client MUST verify that the server version values match. In order to avoid sequence number duplication in case of multiple HelloVerifyRequests, the server MUST use the record sequence number in the ClientHello as the record sequence number in the HelloVerifyRequest.

bathooman commented 3 years ago

Could you clarify whether the problem is that Mbed TLS is accepting manifestly incorrect sequence numbers in incoming messages, or that Mbed TLS is sending incorrect sequence numbers? Is Mbed TLS the client, the server or both?

The problem is that the MbedTLS Server accepts a client_hello whose message_sequence is not 0 and it is a violation of the mentioned requirement. I will provide a reproduce script shortly.

bathooman commented 3 years ago

Could you share steps to reproduce? I ran a sample DTLS handshake with Mbed TLS client and server and see the sequence number starting at 0.

As for the server replying with sequence number 1 in the HVR to a ClientHello with sequence number 1, this is standards-compliant. Quoting Section 4.2.1 of the DTLS RFC:

When responding to a HelloVerifyRequest, the client MUST use the same parameter values (version, random, session_id, cipher_suites, compression_method) as it did in the original ClientHello. The server SHOULD use those values to generate its cookie and verify that they are correct upon cookie receipt. The server MUST use the same version number in the HelloVerifyRequest that it would use when sending a ServerHello. Upon receipt of the ServerHello, the client MUST verify that the server version values match. In order to avoid sequence number duplication in case of multiple HelloVerifyRequests, the server MUST use the record sequence number in the ClientHello as the record sequence number in the HelloVerifyRequest.

The mentioned requirement in Section 4.2.1 is about the record_sequence_number. The mentioned non-conformance issue is affecting the message_sequence.

pfg666 commented 3 years ago

Hi,

I thought I would post here reproduction steps for this problem and for a related problem we found. As previously mentioned, MbedTLS does not check message sequence numbers in the ClientHello initiating the handshake. Moreover, MbedTLS allows a ClientHello with invalid message sequence numbers to restart the handshake in the middle of a current one. Attached is reproduction material for both problems using DTLS-fuzzer. Java 8 JDK is needed to run the reproduction.

Just unpack, cd to the resulting directory and, to reproduce the reported bug run:

bash reproduce.sh invalid_mseq_start.test

To reproduce the handshake restart by way of a ClientHello with invalid message sequence numbers, run:

bash reproduce.sh invalid_mseq_restart.test

I am guessing the two are related, which is why I did not open an additional issue. Could you confirm reproduction on your end?

Thanks. invalid_message_sequence.tar.gz

gilles-peskine-arm commented 3 years ago

MbedTLS allows a ClientHello with invalid message sequence numbers to restart the handshake in the middle of a current one.

This would raise the severity from non-conformance to a potential denial-of-service from an adversary that can inject but not suppress packets, right?

pfg666 commented 3 years ago

The scenarios I have given are ones where the restarting ClientHello contains the cookie the server generates via HelloVerifyRequest. An attacker that somehow manages to sniff this cookie/predict it, can indeed cause denial of service, unless the cookie also authenticates the IP address. However, based on the below quote, the RFC only appears to only use the cookie exchange as DoS prevention mechanism for when getting the cookie is not possible.

This mechanism forces the attacker/client to be able to receive the cookie, which makes DoS attacks with spoofed IP addresses difficult. This mechanism does not provide any defense against DoS attacks mounted from valid IP addresses.

pfg666 commented 3 years ago

I opened a similar issue for Scandium, which exhibited similar behavior. Please check the discussion. The following paragraphs from the RFC are particularly relevant for this issue:

If a server receives a ClientHello with an invalid cookie, it SHOULD treat it the same as a ClientHello with no cookie. This avoids race/deadlock conditions if the client somehow gets a bad cookie (e.g., because the server changes its cookie signing key).

Note to implementors: This may result in clients receiving multiple HelloVerifyRequest messages with different cookies. Clients SHOULD handle this by sending a new ClientHello with a cookie in response to the new HelloVerifyRequest.

The above paragraphs make it possible for the server to receive ClientHello messages with increased message_seq even in cases where the client is conforming. This happens in the (albeit unlikely) event that the server, having recently updated its secret, receives a ClientHello with an old cookie, to which it responds by a HelloVerifyRequest with a new cookie. The client will then produce a new ClientHello which carries this new cookie, but has an incremented message_seq relative to the old ClientHello. The server may accept this new ClientHello.

You should decide whether you intend for the server to accept ClientHello messages with increased message_seq.

A funny thing is that @hanno-arm is actually correct though not based on the quote he gives, but the errata fix to that quote which replaces record sequence numbers by message sequence numbers. The server setting the message_seq of HelloVerifyRequest to that of ClientHello is essential for the corner case described earlier (if it did not replay the message_seq, and only generated HelloVerifyRequest with message_seq 0, the client would discard subsequent HelloVerifyRequest messages which may contain updated cookies).