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

Connection abort during handshake #597

Closed
mikkelfj opened this issue Jun 6, 2017 · 10 comments
Closed

Connection abort during handshake #597

mikkelfj opened this issue Jun 6, 2017 · 10 comments
Labels
invalid A duplicate, overcome-by-events, ill-formed, or off-topic issue, or a question better asked on-list.

Comments

@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 6, 2017

The transport document is not very clear on how to handle errors during handshake. TLS 1.3 section 6.2 says to abort connection and send a simple error. This can be done in client or server cleartext packets.

However, a server, or a client, may be jammed with error messages from an adversary. For servers, this can be mitigated by veryfing the Connection Id before accepting the TLS error message. For clients this is not so simple because not all server clear text messages contain the CCID. In a traditional browser client this may not be a big deal, but for peer2peer between two servers, this could be a problem.

A solution could be have all server cleartext packets contain CCID in addition to SSID, similar to how Stateless Retry packets are protected, except that Stateless Retry does not also have a SSID.

The draft does not mention explicitly that cleartext packets with CONNECTION_CLOSE or GO_AWAY should be dropped, but if they are not, the same issue may happen during handshake. It would be helpful with some very explicit text about how to ignore packets and close connections when unexpected payload appears.

@mikkelfj
Copy link
Contributor Author

mikkelfj commented Jun 6, 2017

Related: QUIC_INVALID_VERSION_NEGOTIATION_PACKET could be sent by an adversary if CONNECTION_CLOSE frames are permitted in cleartext, and if not, the error is useless unless forwarded to TLS.

@chocis
Copy link

chocis commented Jun 7, 2017

Well thats why I think this is another reason for using Server Stateless Retry, which returns client's selected random packet number and can be validated by client. It would be hard for attacker to guess that value (2^31).
But protecting against that when server can change CID in ServerCleartext in current design doesn't seem to be possible.

@mikkelfj
Copy link
Contributor Author

mikkelfj commented Jun 7, 2017

Well thats why I think this is another reason for using Server Stateless Retry, which returns client's selected random packet number and can be validated by client. It would be hard for attacker to guess that value (2^31).

On using packet numbers for validation: I think it would be simpler to validate on a client and a server chosen nonce (aka Connection Id) because it requires less state, but I might be missing something. For example if failing on a second Client ClearText packet you need to remember all packet numbers and while retransmission does this, it is a different machinery. Having the nonces also simplifies state lookup.

@mikkelfj
Copy link
Contributor Author

mikkelfj commented Jun 7, 2017

I guess one counter argument to my issue is that the 5-tuple is hard to guess for an observer, but it really is only one port number that is hard to guess, and that port number might not be generated as random as one would hope on all platforms, notably through some NAT translations.

@mcmanus
Copy link
Contributor

mcmanus commented Jun 7, 2017

The draft does not mention explicitly that cleartext packets with CONNECTION_CLOSE or GO_AWAY should be dropped, but if they are not, the same issue may happen during handshake.

the draft does whitelist which frame types are valid in the various cleartext packets (generally some combination of stream ack and padding) which I think covers this particularly (quoted) concern.

@martinthomson
Copy link
Member

Is #608 a more succinct description of this issue?

Note that CONNECTION_CLOSE isn't available to us right now, though we could easily permit it.

@mikkelfj
Copy link
Contributor Author

mikkelfj commented Jun 7, 2017

Is #608 a more succinct description of this issue?

More succinct, but does not capture pitfalls.

@martinthomson
Copy link
Member

I think that we need to maintain some degree of clarity about issues rather than try to burden them with every imaginable related problem. Can we try to reduce complexity rather than increase it?

@mikkelfj
Copy link
Contributor Author

mikkelfj commented Jun 7, 2017

See TCP RST attack : https://www.utc.edu/center-information-security-assurance/pdfs/course-paper-5620-attacktcpip.pdf

Observations-
Going through Netwox to continually transmit TCP packets flagged as RST,
the attack successfully prevented establishing a connection on the indicated
port (telnet).
Explanations-
In the case of this attack, the misuse of trust and lack of
authentication allow an attacker to continually send TCP RST packets to a
target IP and port number which will effectively prevent any communication on
that port. A small added difficulty with this attack is that a port number
should be known to send the RST message to. However, common uses have standard
port number such as 21, 22, 23, 80; with a sufficient system, an attacker
could cyclically send RST messages over 10,000 ports for a more effective DoS.

@mnot
Copy link
Member

mnot commented Jun 20, 2017

Closing in favour of #608. Folks there can refer to discussion here as necessary.

@mnot mnot closed this as completed Jun 20, 2017
@mnot mnot added the duplicate label Sep 25, 2017
@mnot mnot added the invalid A duplicate, overcome-by-events, ill-formed, or off-topic issue, or a question better asked on-list. label Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid A duplicate, overcome-by-events, ill-formed, or off-topic issue, or a question better asked on-list.
Projects
None yet
Development

No branches or pull requests

5 participants