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

Increase resilience to spoofed Version Negotiation packets #524

Closed
wants to merge 2 commits into from

Conversation

martinthomson
Copy link
Member

There's two layers of defense here:

  1. Version Negotiation echoes 31-bits of entropy (maybe more if you consider the version to be at all unpredictable, or the client's source address to contain any entropy).

  2. If validation fails and version negotiation happened, encourage the client to try again ignoring any Version Negotiation packets that match the failed profile.

Closes #523.

@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels May 12, 2017
Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First part seems good; second part makes me a little sad.


Note:

: The client cannot rely on the version list from the transport parameters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rephrase this -- the client certainly can rely on that list -- once the handshake has completed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The client has to rely on the list; it just cannot validate the list until the end of the handshake.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack, I'll correct that.


If the server does not list an acceptable version, the client MAY ignore the
Version Negotiation packet. This might reduce the likelihood that a spoofed
Version Negotiation packet can be used to disrupt connection establishment.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems very painful if there is a real mismatch. We've already blocked out injected packets, and I suspect that legitimate failures will be more common.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. I would expect the common case to be mismatch, since an attacker can easily generate a VN packet that supports a large number of known versions (minus the client's chosen).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll cut this paragraph.

@@ -1228,6 +1235,18 @@ client MUST terminate with a QUIC_VERSION_NEGOTIATION_MISMATCH error code if
version negotiation occurred but it would have selected a different version
based on the value of the supported_versions list.

If the client receives a Version Negotiation packet and these validation checks
subsequently fail, it is likely that the client received a spoofed Version
Negotiation packet. A client MAY attempt to create a new connection and ignore
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we know this to be likely a result of spoofing, so I wouldn't say it yet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is a disagreement between transport parameters and version negotiation, then something is broken. I don't think that it matters if it is due to spoofing or a bug.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely agree that something is broken in this case, but the text doesn't say that. It says "it is likely that the client received a spoofed Version Negotiation packet."

There's two layers of defense here:

1. Version Negotiation echoes 31-bits of entropy (maybe more if you consider the version to be at all unpredictable, or the client's source address to contain any entropy).

2. If validation fails and version negotiation happened, encourage the client to try again ignoring any Version Negotiation packets that match the failed profile.

Closes #523.
@martinthomson
Copy link
Member Author

I ended up dropping this for some reason, but I've updated it now.

@mikkelfj
Copy link
Contributor

What makes version negotiation special compared to all other first packets from a server?

@martinthomson
Copy link
Member Author

OBE: we decided not to worry about spoofed version negotiation too much. #724 covers the bit we want to keep here.

@martinthomson martinthomson deleted the validate_vn branch August 11, 2017 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants