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

Handshake: Post-Handshake Operation missing end-of-stream marker to protect against stream truncation #17

Open
ahdinosaur opened this issue Nov 28, 2023 · 6 comments

Comments

@ahdinosaur
Copy link

hi @hackergrrl,

i was looking at the new "handshake.md" draft, however i noticed one important feature missing in the "Post-Handshake Operation": an end-of-stream marker to protect against stream truncation.

from the Noise spec section 13: "Application Responsibilities":

Session termination: Applications must consider that a sequence of Noise transport messages could be truncated by an attacker. Applications should include explicit length fields or termination signals inside of transport payloads to signal the end of an interactive session, or the end of a one-way stream of transport messages.

i've been working on a similar post-handshake stream: "Secret Channel".

for Cable, i can recommend what i did in "Secret Channel":

  • disallow a content length of 0, since that doesn't make sense anyways
  • interpret a length of 0 to actually mean an end-of-stream

then Cable should be protected against stream truncation, since end-of-stream messages are authenticated.

an figurative attack to illustrator is described in pull-box-stream:

Alice: hey bob, just calling to say that I think TLS is really great, really elegant protocol, and that I love everything about it.

Mallory (man in the middle): (SNIP! ...terminates connection...)

Alice: NOT!!!!! (Bob never receives this!)

Bob... WTF, I thought Alice had taste!

Bob never gets the punchline, so thinks that Alice's childish humor was
actually her sincere belief.

@hackergrrl
Copy link
Member

hackergrrl commented Nov 29, 2023

Hey @ahdinosaur, thanks for the note. Cool module. :)

So the idea is that by having an explicit end-of-stream marker, Bob can tell the difference between a lost connection and Alice intentionally shutting down her side?

I'm trying to think of a case where this is helpful. The example attack you wrote doesn't apply as-is for Cable, since chat message posts are signed and are either fully received or not. And since the protocol is eventually consistent, it's sort of "expected behaviour" to potentially not receive intermediary posts until possibly MUCH later. And of course Alice/Bob could have lost their connection to each other for many valid reasons as well. If we had stream truncation detection in place, it would be hard -- impractical? -- to find a way to surface that to the client interface in a way that's helpful.

Yeah, I think I'm feeling a bit confused around those points & I'd like to hear more!

@ahdinosaur
Copy link
Author

ahdinosaur commented Nov 29, 2023

So the idea is that by having an explicit end-of-stream marker, Bob can tell the difference between a lost connection and Alice intentionally shutting down her side?

yes 👍

I'm trying to think of a case where this is helpful.

🤷 i reckon if you don't think it's necessary for Cable, then it probably isn't, as you'd know best. 💜

for me, i feel the difference between a lost connection and an intentional end-of-stream is important to communicate in a secure transport protocol, but you're right for eventually consistent p2p protocols it probably doesn't make an ounce of difference. like does it matter if you request a given set of messages and their response is truncated, but you don't know that? sounds like not.

@hackergrrl
Copy link
Member

@ahdinosaur I feel ok about the handshake protocol being explicit about length=0 meaning end-of-stream, if you have capacity to send a patch in. 👍🏻

@hackergrrl hackergrrl reopened this Jan 17, 2024
@mycognosist
Copy link
Contributor

Thanks for raising this issue, @ahdinosaur 🐱

While some of the protective benefits of an end-of-stream marker may not apply to cable due to its design considerations (raised by @hackergrrl), I still think it's good practice to include the behaviour - even if only for the added clarity when debugging network issues; I'm currently working on a Scuttlebutt EBT implementation, debugging premature session conclusions, and any extra data about the termination context is super helpful when things aren't going according to plan.

Added to that, the addition of an end-of-stream marker is trivial - given that we already have support for length fields.

@mycognosist
Copy link
Contributor

I've submitted a PR: #24

@hackergrrl
Copy link
Member

hackergrrl commented Feb 7, 2024

I've thought a lot more about this, and have come to really appreciate learning about this vulnerability! Thank you. ☺️

I also realized that both parties are only protected against stream truncation if the end-of-stream behaviour is for both parties to send an encrypted+authenticated end-of-stream marker. If only Alice sends hers and then closes the TCP connection (or whatever transport), Bob will know he wasn't a victim of a truncation attack, but maybe Bob has one more thing to send before he wants to end the stream? That could get truncated and Alice wouldn't know.

TLS 1.3 does this too, having both sides send aclose_notify alert, although it's not a MUST that the other party need wait to receive it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants