-
Notifications
You must be signed in to change notification settings - Fork 4
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
Cookie of c2c handshake is set late. #135
Comments
This is clearly a bug and the security impact is actually quite hard to estimate and I'll have to think hard about that. I'm not sure if the cookie is also checked elsewhere in the code. It also clearly shows that always sending the cookie is in hindsight a mistake in the spec, encouraging such errors. |
I couldn't really think of a plausible attack scenario so far but due to its complexity, we should fix this ASAP nevertheless. |
Same for me 👍 I'm not a security expert so I didn't add any speculations about security implications to the issue when I opened it 😉 |
Uhm how 😱 Sorry, I'm not sure what happened. I just wanted to comment that for the s2c handshake the cookie is set correctly, but then though it's unnecessary and deleted the comment. I have no idea how this closed the thread. |
No worries. 🙂 |
The line(s) below sets the cookie of the c2c peer when receiving the
auth
message.For initiator:
saltyrtc-client-java/src/main/java/org/saltyrtc/client/signaling/InitiatorSignaling.java
Line 503 in d01e553
For responder:
saltyrtc-client-java/src/main/java/org/saltyrtc/client/signaling/ResponderSignaling.java
Line 310 in d01e553
But as far as I understand the cookie should be set when first receiving a message from a client (independent of weather it's from/as the Server, Responder or Initiator). But before receiving the
auth
the Initiator/Responder do receive aKey
and potentialToken
message, so I think the cookie should be set when receivingToken
(orKey
if noToken
message is send/expected).Details
Spec
The text was updated successfully, but these errors were encountered: