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

Document why one might want to pad ACKS #4252

Closed
wants to merge 2 commits into from

Conversation

ianswett
Copy link
Contributor

...or alter the idle timeout behavior when unpadded ACK-only packets are received.

Fixes #4183

...or alter the idle timeout behavior when unpadded ACK-only packets are received.
@ianswett ianswett added editorial An issue that does not affect the design of the protocol; does not require consensus. -transport labels Oct 19, 2020
@martinthomson
Copy link
Member

As I said privately, I don't think that we need to say anything more on this point. The case we are interested in is Initial packets from a server that contain only ACK. If those are getting through, then they will be in response to Initial packets from the client. Given that the client has received an acknowledgement, it will only be sending Initial packets on PTO. As the PTO count doesn't reset at this stage, the PTO timeout increases exponentially. Eventually the PTO will be larger than the idle timeout. So - worst case - the idle timeout is doubled. If you are relying on idle timeout to abandon the connection attempt, then this isn't great, but you can still rely on that being hit.

@janaiyengar
Copy link
Contributor

I agree with @martinthomson here. If we are going to add a note, I'd rather add one about the fact that smaller-than-1200-byte non-ack-eliciting packets can cause unpredicted side-effects. Given that we don't have anything actionable here for an implementer however, I don't see the value in this.

@ianswett
Copy link
Contributor Author

Possibly this is not the right text, but @marten-seemann wanted to pad ACKs and I think there are valid reasons, so I'm attempting to describe the one possible issue with not doing so.

I'm happy to close this and accept someone else's PR instead.

Copy link
Member

@kazuho kazuho left a comment

Choose a reason for hiding this comment

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

I am concerned that the note is a bit too specific regarding behaviors around timeout. We do assume endpoints to have timeouts other than idle timeouts (e.g. handshake timeout).

@janaiyengar

I'd rather add one about the fact that smaller-than-1200-byte non-ack-eliciting packets can cause unpredicted side-effects.

I'm not sure if giving such advice makes sense. As pointed out in #4253, size of datagram is not authenticated, and therefore endpoints should be prepared against MOTS attackers injecting spoofed datagrams that do not meet the padding requirement.

It might be true that we can have exceptions for Initial packets, but I do not think having such exception is worthwhile when we cannot have such an exception for connection migration.

@ianswett
Copy link
Contributor Author

Not a lot of interest in this, so closing.

@ianswett ianswett closed this Oct 20, 2020
@martinthomson martinthomson deleted the ianswett-unpadded-acks branch November 5, 2020 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handshake failure (infinite ping-pong) when path MTU is asymmetric
4 participants