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

DTLS server allows insecure renegotiation without indicating renegotiation options in ClientHello #99

Closed
jerrytesting opened this issue Aug 5, 2021 · 8 comments
Labels
please retest Please retest the related PR or commit, if that works for you

Comments

@jerrytesting
Copy link

jerrytesting commented Aug 5, 2021

Hello,

The dtls-server application allows completing the second handshake without indicating renegotiation options in the first handshake message, which is prohibited according to RFC 5746.

We could use Wireshark to capture these two handshake processes shown as follows.
renegotiation

This creates the opportunity for an attack in which the attacker who can intercept a client's transport layer connection can inject traffic of his own as a prefix to the client's interaction with the server(cited from RFC 5746).

Could you kindly have a check? Thanks a lot.

Kind Regards,
Jerry

@boaks
Copy link
Contributor

boaks commented Aug 5, 2021

Unfortunately, RFC6347 doesn't really mention such kinds of TCP/TLS specific attacks.
And RFC5746 only mentions, that the extension can be used for DTLS, not that the attack applies.

That attack is mainly based on "buffering" of TCP/TLS for "stream oriented" processing.
For the most UDP stuff, that doesn't apply. When the received data is passed to the tinydtls stack, it's processed synchronous and when the callback handler for read is called, the dtls_context_t is exactly that of this record and not of that before nor after.

@jerrytesting
Copy link
Author

RFC7925 also refers that TLS/DTLS allows a client and a server that already have a TLS/DTLS connection to negotiate some aspects by using the renegotiation feature. But yes, it also doesn't indicate that using renegotiation options is necessary for DTLS.

Except for this, I am also confused why the server state is always in the DTLS_STATE_WAIT_CLIENTHELLO state, thereby the server can always accept ClientHello requests from clients. After replying ServerHelloDone, could the state of the server move to the following one? This can be reproducible if not using clients from TinyDTLS.

@jerrytesting
Copy link
Author

jerrytesting commented Aug 8, 2021

Again:( Besides the negotiation problem, I also found other two specifications that the TinyDTLS server violates according to RFC6347. The following picture shows packets captured by Wireshark.
11

The first violation is that, the server is able to complete a handshake with clients from epoch 1, while RFC6347 indicates that the epoch number is initially zero.

The second violation is that, the second handshake could be completed with the same epoch as the first one. However, RFC6347 prohibits this way, saying "In order to ensure that any given sequence/epoch pair is unique, implementations MUST NOT allow the same epoch value to be reused within two times the TCP maximum segment lifetime."

Hope you could have a check. Thanks for your patience.

@boaks
Copy link
Contributor

boaks commented Aug 8, 2021

Yes, there are some issue with the handshakes. I also created two issue (#84 and #93) and provided a PR to fix them (#89). I guess, if you retest with that PR (or even better with PR #94) your issue with epoch 1 is also fixed.

The same developers of this library are also develop libcoap, and so I think we currently need more patience.

@boaks
Copy link
Contributor

boaks commented Aug 8, 2021

Except for this, I am also confused why the server state is always in the DTLS_STATE_WAIT_CLIENTHELLO state, thereby the server can always accept ClientHello requests from clients.

RFC6347 -
4.2.8. Establishing New Associations with Existing Parameters

If a DTLS client-server pair is configured in such a way that
repeated connections happen on the same host/port quartet, then it is
possible that a client will silently abandon one connection and then
initiate another with the same parameters (e.g., after a reboot).
This will appear to the server as a new handshake with epoch=0. In
cases where a server believes it has an existing association on a
given host/port quartet and it receives an epoch=0 ClientHello, it
SHOULD proceed with a new handshake but MUST NOT destroy the existing
association until the client has demonstrated reachability either by
completing a cookie exchange or by completing a complete handshake
including delivering a verifiable Finished message. After a correct
Finished message is received, the server MUST abandon the previous
association to avoid confusion between two valid associations with
overlapping epochs. The reachability requirement prevents
off-path/blind attackers from destroying associations merely by
sending forged ClientHellos.

According that, at least this SHOULD be the behavior. I recently also tried to get some more clarity for that, also because discussing issue #95 showed very different interpretations. My e-mail to the tls-mailing list wasn't answered, and DTLS 1.3 issue wasn't successful either. So, for now, a system should either go with that 4.2.8, or should have a specified "session timeout". Otherwise, assuming clients with changing addresses (maybe NATed) or crashes will no work that reliable, because the "close" is also not reliable (and assuming crashs, "close" even can't be reliable).

@boaks
Copy link
Contributor

boaks commented Aug 9, 2022

In the meantime a lot of fixes have been merged.
Maybe you can spend some time into retest main and give us your feedback or close this issue.

@boaks boaks added the please retest Please retest the related PR or commit, if that works for you label Aug 9, 2022
@boaks
Copy link
Contributor

boaks commented Nov 2, 2022

In the meantime we decide to implement a minimal version of RFC5746, see discussion in issue #175 .

@boaks
Copy link
Contributor

boaks commented Feb 28, 2023

Thanks for retesting and closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please retest Please retest the related PR or commit, if that works for you
Projects
None yet
Development

No branches or pull requests

2 participants