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

Repeat tokens in all Initial packets #2089

Merged
merged 5 commits into from
Dec 4, 2018
Merged

Conversation

ianswett
Copy link
Contributor

@ianswett ianswett commented Dec 1, 2018

The client repeats the token in all Initial packets. Also clarifies that the token is sent in Initial in a few spots.

https://www.ietf.org/mail-archive/web/quic/current/msg05071.html

The client repeats the token in all Initial packets.  Also clarifies that the token is sent in Initial in a few spots.

https://www.ietf.org/mail-archive/web/quic/current/msg05071.html
draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
believes that its point of network attachment has changed since the token was
last used; that is, if there is a change in its local IP address or network
interface. A client needs to start the connection process over if it migrates
prior to completing the handshake.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is getting ahead of other discussions, but a client SHOULD not repeat also because the server MAY reject repeated tokens.

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that the text from PR #2064 will be inserted after this paragraph. Again, it is the flip side of the "client SHOULD NOT" clause. Client should not reuse the same token for multiple connections, and servers MAY (or SHOULD?) reject connections that attempt to reuse a token.

Copy link
Member

Choose a reason for hiding this comment

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

@huitema has it right here. We are adding text in #2064 that should help.

SHOULD attempt to validate it, unless it has already completed address
validation. If the token is invalid then the server SHOULD proceed as if
the client did not have a validated address, including potentially sending
a Retry. If the validation succeeds, the server SHOULD then allow the
handshake to proceed.
Copy link
Contributor

Choose a reason for hiding this comment

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

And here text might be placed on server MAY reject repeat tokens?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

@huitema huitema left a comment

Choose a reason for hiding this comment

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

I assume that we will process PR #2064 after this one. I am fine with the text, but I would like something said about server behavior. Client recommendations are mostly enforced when breaking them causes servers to reject packets...


As long as it is not possible for an attacker to generate a valid token for
its own address (see {{token-integrity}}) and the client is able to return
that token, it proves to the server that it received the token.

Copy link
Contributor

Choose a reason for hiding this comment

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

Define "proves". The token mechanism may be vulnerable to replay attacks.

Copy link
Member

Choose a reason for hiding this comment

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

It only proves that it received it, not when. I think that we can probably defer discussion of the reuse/replay aspects of this.

address validation in a future connection. The client MUST include the
token in all Initial packets it sends, unless a Retry replaces the token
with a newer token. The client MUST NOT use the token provided in a Retry
for future connections.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, "all Initial packets" is what we want. At least it is not ambiguous. There is however a general problem with such recommendations: they are not enforced unless servers behave that way. I would add something at the end:

with a newer token. The client MUST NOT use the token provided in a Retry
for future connections. Servers MAY reject any Initial packet that does not
carry the expected token.

I am not sure about the use of the plural in "future connections". Allowing the same token to be reused in multiple connections is potentially dangerous. But that may be work for another PR.

Copy link
Member

Choose a reason for hiding this comment

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

Is the desired action "reject" or "ignore/discard"? I would say either is OK, though I might be inclined more toward the latter (on the basis that spoofing implies DoS).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, discard is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated.

believes that its point of network attachment has changed since the token was
last used; that is, if there is a change in its local IP address or network
interface. A client needs to start the connection process over if it migrates
prior to completing the handshake.
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that the text from PR #2064 will be inserted after this paragraph. Again, it is the flip side of the "client SHOULD NOT" clause. Client should not reuse the same token for multiple connections, and servers MAY (or SHOULD?) reject connections that attempt to reuse a token.

Copy link
Member

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

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

This is fine, pending the small suggested tweaks. Though we need to settle on "reject" vs. "discard" to close this one.

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved

As long as it is not possible for an attacker to generate a valid token for
its own address (see {{token-integrity}}) and the client is able to return
that token, it proves to the server that it received the token.

Copy link
Member

Choose a reason for hiding this comment

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

It only proves that it received it, not when. I think that we can probably defer discussion of the reuse/replay aspects of this.

address validation in a future connection. The client MUST include the
token in all Initial packets it sends, unless a Retry replaces the token
with a newer token. The client MUST NOT use the token provided in a Retry
for future connections.

Copy link
Member

Choose a reason for hiding this comment

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

Is the desired action "reject" or "ignore/discard"? I would say either is OK, though I might be inclined more toward the latter (on the basis that spoofing implies DoS).

believes that its point of network attachment has changed since the token was
last used; that is, if there is a change in its local IP address or network
interface. A client needs to start the connection process over if it migrates
prior to completing the handshake.
Copy link
Member

Choose a reason for hiding this comment

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

@huitema has it right here. We are adding text in #2064 that should help.

SHOULD attempt to validate it, unless it has already completed address
validation. If the token is invalid then the server SHOULD proceed as if
the client did not have a validated address, including potentially sending
a Retry. If the validation succeeds, the server SHOULD then allow the
handshake to proceed.
Copy link
Member

Choose a reason for hiding this comment

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

martinthomson and others added 2 commits December 2, 2018 14:32
Co-Authored-By: ianswett <ianswett@users.noreply.github.com>
can either abort the connection or permit it to proceed.
token MUST be repeated by the client in all Initial packets it sends after it
receives the Retry packet. In response to receiving a token in an Initial
packet, a server can either abort the connection or permit it to proceed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't there a third option to ignore a packet rather then abort or permit to proceed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the 3rd option?

Copy link
Contributor

Choose a reason for hiding this comment

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

The third option is to "drop/ignore". The mentioned options are "abort" and "proceed", but proceed implies the packet is processed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SG, changed from receiving to processing, because otherwise I think this sentence is not really correct.

@martinthomson martinthomson merged commit fc92ac6 into master Dec 4, 2018
@martinthomson
Copy link
Member

@ianswett, does this fix an issue?

@martinthomson martinthomson deleted the ianswett-repeat-tokens branch December 4, 2018 21:13
@ianswett
Copy link
Contributor Author

ianswett commented Dec 4, 2018

The only issue I'm aware of is the thread I linked to.

@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Dec 13, 2018
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

5 participants