-
Notifications
You must be signed in to change notification settings - Fork 203
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
Add initial threat model to security considerations #2925
Add initial threat model to security considerations #2925
Conversation
Upcoming change to terminology here as well, just as a note. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll wait until this is revised before doing the hard stuff. Some easy comments though to keep things motivated.
As commented on the issue, this should fix #2387 as well. |
Added a few paragraphs describing general active and passive attacks in the context of RFC 3552 and more clearly defined off-path and on-path attackers to follow the definition established by that document. This will likely still take some iteration, but it should be mostly ready for that, comments welcome! |
@gorryfair Please take a look, would love your feedback on this! |
I just had a quick read, and this looks fine (maybe there is an option to remove some redundancy but not sure). However, I would have expected that this would also talk about likability. Or do we already cover hat somewhere else? More generally I think it would be helpful to add some text at the beginning to not only define the attackers capabilities but also its intention, so the (goal of) the attack itself. |
Thanks @mirjak! There's a section, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments from Hackathon - basically heading in correct direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WIP
Co-Authored-By: Eric Kinnear <32474881+erickinnear@users.noreply.github.com>
First cut at handshake guarantees
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great. Minor editorial nitpicks, and one which might turn into a separate issue.
datagrams, causing multiple packets to be coalesced into a single datagram, or | ||
splitting coalesced packets into multiple datagrams. Such modification has no | ||
functional effect on a QUIC connection, although it might change the performance | ||
characteristics exhibited by the receiving endpoint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good observation. I remember having discussions about heuristics that assume packets sent in a single datagram arrived together. This means those heuristics aren't reliable. I don't recall whether we endorsed such approaches in the document (PMTU probing, maybe?), but this suggests that we might want to explicitly caution against them. (Not necessarily here.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of note here is that we allow Initial packets to be smaller than 1200 bytes, as long as they are coalesced with other stuff to make up a 1200 byte UDP datagram. That too is vulnerable to this sort of composition problem. I think that it's fine to make these assumptions and risk modification as long as they are recognized as such. For Initial, we are doing so on the understanding that elements on the path are able to disrupt the handshake at that point anyway. For PMTUD, we might make a different assessment as the conditions are different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good thinking, it might be worth a note around those rules in the Initial that such a thing is possible (and might be more likely than if you just padded the initial packet?). PMTUD is probably worth calling out a little bit more clearly -- I'll file an editorial issue for each, since that's way elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MikeBishop In terms of relying on how packets were coalesced, I don't believe there's any reliable information to be obtained, and relying on these hueristics is already prone to cause problems. For example, there's no guarantee that a receiver buffers the currently undecryptable packets in a datagram if it can decrypt at least one packet. So yes, we should caution against that if we don't already.
Co-Authored-By: Mike Bishop <mbishop@evequefou.be>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @MikeBishop! Applied suggestions and rewrapping.
datagrams, causing multiple packets to be coalesced into a single datagram, or | ||
splitting coalesced packets into multiple datagrams. Such modification has no | ||
functional effect on a QUIC connection, although it might change the performance | ||
characteristics exhibited by the receiving endpoint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good thinking, it might be worth a note around those rules in the Initial that such a thing is possible (and might be more likely than if you just padded the initial packet?). PMTUD is probably worth calling out a little bit more clearly -- I'll file an editorial issue for each, since that's way elsewhere.
Anything else we want to do before this is ready to land? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing the amended stuff. All goodness, though I have a suggestion or two.
I think that @janaiyengar promised to take another look at this, but for me I'm getting tired of staring at this. I want to merge this very soon.
|
||
An on-path attacker can force the QUIC handshake to fail by replacing either the | ||
client or server Initial messages with invalid messages. An off-path attacker | ||
can also mount this attack by racing the Initials. Once valid Initial messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said elsewhere, this could be simplied to:
An on-path or off-path attacker can force the QUIC handshake to fail by replacing or racing Initial packets. Once valid Initial packets [...]
Note: "packet" and not "message".
### Handshake {#handshake-properties} | ||
|
||
The QUIC handshake incorporates the TLS 1.3 handshake and enjoys the | ||
cryptographic properties described in {{?RFC8446}}; Appendix E.1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cryptographic properties described in {{?RFC8446}}; Appendix E.1. | |
cryptographic properties described in {{?TLS13=RFC8446}}; Appendix E.1. |
QUIC server's first flight may be sent to a client whose address it cannot | ||
validate. This flight may be long and therefore potentially allows the server | ||
to be used as a DoS reflector/amplifier. The mechanisms described in | ||
{{validate-handshake}} restrict the amplification to a factor of three. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead:
Address validation ({{address-validation}}) is used to verify that an entity that claims a given address is able to receive packets at that address. Address validation limits amplification attack targets to addresses for which an attacker is either on-path or off-path.
Prior to validation, endpoints are limited in what they are able to send. During the handshake, a server cannot send more than three times the data it receives; clients that initiate new connections or migrate to a new network path are limited to [um, I can't find a concrete limit in the document].
|
||
An attacker can also modify the boundaries between QUIC packets and UDP | ||
datagrams, causing multiple packets to be coalesced into a single datagram, or | ||
splitting coalesced packets into multiple datagrams. Such modification has no |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
splitting coalesced packets into multiple datagrams. Such modification has no | |
splitting coalesced packets into multiple datagrams. Aside from datagrams containing Initial packets, which require padding, modification has no |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions, but nothing concerning, so LG.
|
||
The entire handshake is cryptographically protected, with the Initial packets | ||
being encrypted with per-version keys and the Handshake and later packets being | ||
encrypted with keys derived from the TLS key exchange. Further, parameter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Is it a key exchange or message exchange?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Key exchange is correct (the key exchange depends on the message exchange, but it's the key exchange that matters here).
parameter negotiation. | ||
|
||
Connection IDs are unencrypted but integrity protected in all messages. They | ||
are not incorporated in the TLS handshake transcript. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is potentially misleading, since there is the original_connection_id transport param, but I'm not sure if it's worth mentioning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this second sentence can be dropped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without further elaboration, I have to agree.
Separately, I think it was Antoine who made some (good) points about authenticating connection IDs more thoroughly that we might want to consider. Separately though (reminds self to open an issue).
datagrams, causing multiple packets to be coalesced into a single datagram, or | ||
splitting coalesced packets into multiple datagrams. Such modification has no | ||
functional effect on a QUIC connection, although it might change the performance | ||
characteristics exhibited by the receiving endpoint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MikeBishop In terms of relying on how packets were coalesced, I don't believe there's any reliable information to be obtained, and relying on these hueristics is already prone to cause problems. For example, there's no guarantee that a receiver buffers the currently undecryptable packets in a datagram if it can decrypt at least one packet. So yes, we should caution against that if we don't already.
#### On-Path Active Attacks | ||
|
||
An attacker that can cause a packet it observes to no longer reach its intended | ||
destination is considered an on-path attacker. Such an attacker generally is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally is present is both a somewhat strong(implying >50% of the time) and quite vague statement. Maybe this clause should be changed to: "When an attacker is present between the QUIC client and server, QUIC endpoints are required to send packets through the attacker to establish connectivity on a given path."?
properties: | ||
|
||
1. An on-path attacker can prevent use of a path for a QUIC connection, causing | ||
it to fail if it cannot migrate to a new path that does not contain the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it's necessary to use the word 'migrate' here. If the handshake hasn't completed, you can't migrate yet. If you take my suggestion, it probably makes sense to update the other bullets.
it to fail if it cannot migrate to a new path that does not contain the | |
it to fail if it cannot use a different path that does not contain the |
In the presence of an off-path attacker, QUIC aims to provide the following | ||
properties: | ||
|
||
1. An off-path attacker can race packets and attempt to become a "limited" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word race is used a few times in this section, but I don't believe it's defined and I'm not sure it's clear to everyone what it means in this context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's first use would seem to work well enough to be definitional. I'm happy with this as it is.
packets with the source address listed as the off-path attacker as long as it | ||
can provide improved connectivity between the client and the server. | ||
|
||
3. An off-path attacker cannot cause a connection to close. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3. An off-path attacker cannot cause a connection to close. | |
3. An off-path attacker cannot cause a connection to close once the handshake has completed. |
In the presence of a limited on-path attacker, QUIC aims to provide the | ||
following properties: | ||
|
||
1. A limited on-path attacker cannot cause an active connection to close. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. A limited on-path attacker cannot cause an active connection to close. | |
1. A limited on-path attacker cannot cause an established connection to close. |
parameter negotiation. | ||
|
||
Connection IDs are unencrypted but integrity protected in all messages. They | ||
are not incorporated in the TLS handshake transcript. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this second sentence can be dropped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just so everyone is clear on what is happening here, I've made a few changes and landed this. This is obviously a lot of text and it could benefit from more review, but the editors all agreed that it would better to make incremental changes rather than continue to maintain our oldest open PR further.
Just to add to that, thanks very much to Eric (and Eric) for this text. It is really valuable work, and very fiddly.
parameter negotiation. | ||
|
||
Connection IDs are unencrypted but integrity protected in all messages. They | ||
are not incorporated in the TLS handshake transcript. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without further elaboration, I have to agree.
Separately, I think it was Antoine who made some (good) points about authenticating connection IDs more thoroughly that we might want to consider. Separately though (reminds self to open an issue).
|
||
The entire handshake is cryptographically protected, with the Initial packets | ||
being encrypted with per-version keys and the Handshake and later packets being | ||
encrypted with keys derived from the TLS key exchange. Further, parameter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Key exchange is correct (the key exchange depends on the message exchange, but it's the key exchange that matters here).
In the presence of an off-path attacker, QUIC aims to provide the following | ||
properties: | ||
|
||
1. An off-path attacker can race packets and attempt to become a "limited" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's first use would seem to work well enough to be definitional. I'm happy with this as it is.
Thanks Martin, and to all who reviewed! |
can also mount this attack by racing the Initials. Once valid Initial messages | ||
have been exchanged, the remaining handshake messages are protected with the | ||
handshake keys and an on-path attacker cannot force handshake failure, though | ||
they can produce a handshake timeout by dropping packets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The on path attacker can also submit a MITM attack on the handshake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That could be a great thing to cover, can you file an issue to discuss whatever text should be here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@erickinnear added #3512
This adds a section for a QUIC threat model, filling in only the migration section (for #2143, some of the rest of the threat model will come in #2387).
Some wordsmithing can be applied here, and we may want additional issues to fill out detailed descriptions of some of the worst attacks and outline heuristics that might help, but for now I'm focusing this purely on documenting what's possible/not possible for an attacker to do today.
Addresses #2143.