quicwg / base-drafts

Internet-Drafts that make up the base QUIC specification
https://quicwg.org
1.63k stars 204 forks source link

Specify AEAD-based protection of cleartext packets #693

Closed ekr closed 7 years ago

ekr commented 7 years ago

Based on discussion in Prague:

ianswett commented 7 years ago

I think this obsoletes #554

mikkelfj commented 7 years ago

I have no major issue with AES-GCM and the benefit is that it is already there - but please also consider AES-CMAC - it is much lighter and does the job for clear text - and would thus be more version neutral - e.g. implementations are not forced to carry GCM machinery if they want other crypto in a specific version. Notably, middle-boxes would have an easier time.

https://tools.ietf.org/html/rfc4493 https://tools.ietf.org/html/rfc4494

ekr commented 7 years ago

The rationale for GCM was that Ian wanted to encrypt the plaintext.

ianswett commented 7 years ago

Yes, I'm worried about the day when QUIC version N+1 wants to support TLS > 1.3 and I don't want to deal with all the old middleboxes that have ossified QUIC==TLS 1.3.

mikkelfj commented 7 years ago

ah I see

huitema commented 7 years ago

How does this apply to the Version Negotiation packet?

ekr commented 7 years ago

I had in mind the same way. You would use the client's version there too

ekr commented 7 years ago

This is assigned to me, but I'm waiting on doing it until @martinthomson does some refactoring. Trying to avoid merge conflicts

huitema commented 7 years ago

So for version negotiation, we would have:

1) Client sends an Initial packet with version unknown to server. 2) Server sees only the header and cannot decrypt the content because it does not know the version specific key. 3) Server replies with version negotiation packet, which is not encrypted.

Correct?

ekr commented 7 years ago

Ian was suggesting that the version-specific key be predictable from the version, so no, you would just decrypt it.

davidben commented 7 years ago

If it's predictable, won't the middleboxes just derive the key, decrypt it, and then do their usual ossifying thing?

RyanTheOptimist commented 7 years ago

On Sun, Jul 30, 2017 at 4:37 PM, ekr notifications@github.com wrote:

Ian was suggesting that the version-specific key be predictable from the version, so no, you would just decrypt it.

​Decrypt which? The version negotiation packet? The version negotiation packet has N versions in it, right?

huitema commented 7 years ago

I don't see the point of going to AEAD encryption if the key is entirely predictable from the packet. Doing so would not allow us to replace AES-GCM by something else in the future. It would also be a mere speed bump for ossifying NATs. I would rather stick to FNV1A-64.

I see the point of a key that is specified as part of the document specifying a specific version. That mean that a NAT developer could not decrypt the data for version X without having read the spec for version X. It is not an iron-proof guarantee, because the NAT developers may decide to just read the part of the spec that specifies the key and ignore the other parts. But people who do that can be pointed out as misbehaving. The document that specifies the version would also specify the algorithm, which will allow for evolution.

But then, that means that servers receiving an packet with the wrong version will only be able to understand the packet header.

martinthomson commented 7 years ago

The idea here is to make the key (and algorithm) version-specific. Then you absolutely can't pretend to support a version you don't understand.

The risk, as always, is that NATs latch on to one version and block all other versions. But that's preferable to the range of subtle and noxious stuff that they might otherwise do.

@ekr, if the server sends a Version Negotiation packet, it can't know which version to choose - the only version it knows that the client supports is one that it doesn't. If I'm right, then Version Negotiation has to be unencrypted. I think that for consistency we should define version 0 for this purpose with a fixed algorithm, which might be f(x)=x.

huitema commented 7 years ago

@martinthomson, I think we agree. But I don't think we need the version 0 hack. The base spec says that the clear text packets are integrity protected, but it does not mention anything of the kind for the version negotiation packet. The way I read the spec, the version negotiation packets are not protected at all.

This means that the version can be spoofed. But then, the transport parameters repeat the content and results of the renegotiation, and the spoofing will be detected. So we are probably fine without any protection.

martinthomson commented 7 years ago

:) The only hazard here is what you get when the Version Negotiation is indistinguishable from Internet-noise. I think that we can and should just decide that we're comfortable with that.

huitema commented 7 years ago

Yes, we should be OK with the "Internet Noise" issue. Version Negotiation is sent to clients. It will only be accepted if the connection ID and Sequence Number match what the client expects. That, and the IP source address and UDP source port...

ekr commented 7 years ago

There are two objectives here:

  1. Preventing off-path attackers from being able to inject packets. AEADing all packets with a key derived from the client CONN_ID does this (regardless of whether or not you mix in some version-specific key). It also doesn't actually matter for this whether you encrypt, but it's convenient because it means no new transform.

  2. Preventing middleboxes from trying to read the content of versions they don't know. Having a strong defense against this obviously requires an unpredictable per-version key, but Ian seemed to think there was some value in a predictable key and I'm fine with going along, but i'm also fine not doing this at all.

WRT to the various suggestions made above: It's important that VN be protected from off-path attackers as well because otherwise attackers can inject it and cause connection failure just as with other packets (remember: all of this is about protecting injection of packets pre-crypto, because we need to have a negotiation protocol that protects against full on-path attackers). Thus, it needs to also be tied to the ClientInitial and the whole point of this issue is to do that in a generic way rather than in a per-packet specific way, as @huitema proposes directly above. Hence, the AEAD (or, as I originally proposed, HMAC). So, I don't think it can be unprotected.

I think the right approach here is a variant of @martinthomson's suggestion above.

  1. Use a version-specific key label that's unpredictable and baked into the version when it's published. I.e., Key = HKDF(ConnID, version_label)
  2. VN always uses some predictable label (say, all 0s)

The impact of this would be that:

  1. VN is integrity protected, as expected and you can always send VN
  2. Servers which receive packets for versions they totally don't know will send VN even if the packet is actually invalid. this seems acceptable because any attacker can produce valid packets and elicit a VN.
huitema commented 7 years ago

Works for me.

janaiyengar commented 7 years ago

I like ekr's proposed approach. The VN packet currently echoes pieces of the client packet, which seems unnecessary with the "encrypted" VN. We could clean that up too if we want, but that should be a separate issue to deal with once this one is done..

martinthomson commented 7 years ago

I agree with Jana, adding Server Stateless Retry to that bucket of things that we might re-examine after this.

MikeBishop commented 7 years ago

The risk of an encrypted VN packet is that it moves the specifics of encrypting at least that packet into the "can't change between versions" realm. Do we want to codify something that requires keeping old moldy crypto around just to express what QUIC versions you support that no longer use that crypto?

Whatever we do, Version Negotiation winds up being a one-off -- if we're consistent now, then it's a one-off when we eventually change. If we don't encrypt, it's a one-off from the start, which might be more honest. Though I agree the protection against off-path attackers is also interesting.

RyanTheOptimist commented 7 years ago

On Wed, Aug 2, 2017 at 3:15 PM, Mike Bishop notifications@github.com wrote:

The risk of an encrypted VN packet is that it moves the specifics of encrypting at least that packet into the "can't change between versions" realm. Do we want to codify something that requires keeping old moldy crypto around just to express what QUIC versions you support that no longer use that crypto?

​I completely agree with this​ point and think that a minimal VN packet format is probably best.

janaiyengar commented 7 years ago

Mike, Ryan: Are you suggesting that we treat VN as a special snowflake (which it undoubtedly is) and leave it unencrypted with an FNV-128a hash? We have nominal protection against off-path attackers already with a connection ID echo, so that's covered without the encryption.

I am sympathetic to the idea of not wanting to have old crypto sitting around just to handle VN packets. I'd be happy to treat VN packets as special, and leave them unencrypted. I don't think I care enough about bit flips with the VN packet to need an FNV hash for this alone... we'll still have the (weak) UDP checksum to cover that.

RyanTheOptimist commented 7 years ago

I suggest that it have no encryption and no fnv-128 hash.

On Wed, Aug 2, 2017 at 4:07 PM, janaiyengar notifications@github.com wrote:

Mike, Ryan: Are you suggesting that we treat VN as a special snowflake (which it undoubtedly is) and leave it unencrypted with an FNV-128a hash? We have nominal protection against off-path attackers already with a connection ID echo, so that's covered without the encryption.

I am sympathetic to the idea of not wanting to have old crypto sitting around just to handle VN packets. I'd be happy to treat VN packets as special, and leave them unencrypted. I don't think I care enough about bit flips with the VN packet to need an FNV hash for this alone... we'll still have the (weak) UDP checksum to cover that.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/quicwg/base-drafts/issues/693#issuecomment-319823016, or mute the thread https://github.com/notifications/unsubscribe-auth/ASp6ymfKvman1rx2Ffo8rnM9meurafUgks5sUQEpgaJpZM4OfFhh .

janaiyengar commented 7 years ago

SGTM.

ianswett commented 7 years ago

I'm also happy with VN having no encryption and no fnv-128 hash and being a special snowflake.

MikeBishop commented 7 years ago

Likewise, no encryption and no hash. There are exactly two packet types that are version-independent, which makes them both inherently special snowflakes. (That argues for not using this scheme in stateless reset, as well.)

ianswett commented 7 years ago

The other being stateless(or connection) reset?

MikeBishop commented 7 years ago

Yep.

huitema commented 7 years ago

Since everybody agrees, let's add my vote: no FNV, no encryption for VN.

-- Christian Huitema

On Aug 2, 2017, at 4:54 PM, Mike Bishop notifications@github.com wrote:

Yep.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

janaiyengar commented 7 years ago

What vote? This is the IETF Christian, we don't vote here.

On Thu, Aug 3, 2017 at 12:21 AM, Christian Huitema <notifications@github.com

wrote:

Since everybody agrees, let's add my vote: no FNV, no encryption for VN.

-- Christian Huitema

On Aug 2, 2017, at 4:54 PM, Mike Bishop notifications@github.com wrote:

Yep.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/quicwg/base-drafts/issues/693#issuecomment-319833634, or mute the thread https://github.com/notifications/unsubscribe-auth/AKjg1KR87ZRLXhoNnSeh_ExSWnwgy2Toks5sURKYgaJpZM4OfFhh .

huitema commented 7 years ago

Jana, do you realize that right now I am finishing a nice bottle of wine, in an island in which I just moored my boat? So yes of course I can pretend to vote!

-- Christian Huitema

On Aug 2, 2017, at 7:20 PM, janaiyengar notifications@github.com wrote:

What vote? This is the IETF Christian, we don't vote here.

On Thu, Aug 3, 2017 at 12:21 AM, Christian Huitema <notifications@github.com

wrote:

Since everybody agrees, let's add my vote: no FNV, no encryption for VN.

-- Christian Huitema

On Aug 2, 2017, at 4:54 PM, Mike Bishop notifications@github.com wrote:

Yep.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/quicwg/base-drafts/issues/693#issuecomment-319833634, or mute the thread https://github.com/notifications/unsubscribe-auth/AKjg1KR87ZRLXhoNnSeh_ExSWnwgy2Toks5sURKYgaJpZM4OfFhh .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

martinthomson commented 7 years ago

We still need proof that the VN packet didn't come from off-path. How should we do that? We currently echo the header, is that still the right answer?

janaiyengar commented 7 years ago

That was my thinking. The protection is as good as the packet encrypted with the connection ID, since the information required to generate the VN is the same in both cases.

RyanTheOptimist commented 7 years ago

Agreed. Echoing the connection id seems to provide sufficient entropy to defend against off-path attackers.

On Wed, Aug 2, 2017 at 7:34 PM, janaiyengar notifications@github.com wrote:

That was my thinking. The protection is as good as the packet encrypted with the connection ID, since the information required to generate the VN is the same in both cases.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/quicwg/base-drafts/issues/693#issuecomment-319851408, or mute the thread https://github.com/notifications/unsubscribe-auth/ASp6yl2Lr-8KHUakdZ49tqGUtdBhlPk4ks5sUTG-gaJpZM4OfFhh .

RyanTheOptimist commented 7 years ago

On Wed, Aug 2, 2017 at 7:27 PM, Christian Huitema notifications@github.com wrote:

Jana, do you realize that right now I am finishing a nice bottle of wine, in an island in which I just moored my boat? So yes of course I can pretend to vote!

​I vote that we have the next interim on Christian's boat moored at his island!

janaiyengar commented 7 years ago

I believe Christian isn't too far from Seattle...

ekr commented 7 years ago

But now this is the only thing with some special snowflake anti-injection. I don't see how that makes things better.

On Wed, Aug 2, 2017 at 7:57 PM, Ryan Hamilton notifications@github.com wrote:

Agreed. Echoing the connection id seems to provide sufficient entropy to defend against off-path attackers.

On Wed, Aug 2, 2017 at 7:34 PM, janaiyengar notifications@github.com wrote:

That was my thinking. The protection is as good as the packet encrypted with the connection ID, since the information required to generate the VN is the same in both cases.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/quicwg/base-drafts/issues/693#issuecomment-319851408 , or mute the thread https://github.com/notifications/unsubscribe-auth/ASp6yl2Lr- 8KHUakdZ49tqGUtdBhlPk4ks5sUTG-gaJpZM4OfFhh

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/quicwg/base-drafts/issues/693#issuecomment-319854352, or mute the thread https://github.com/notifications/unsubscribe-auth/ABD1oRI2adgeAAd4bq4m3CijJmmlabgGks5sUTcHgaJpZM4OfFhh .

janaiyengar commented 7 years ago

As Mike pointed out, VN and Stateless Reset are special snowflakes, no matter what. The point is that since whatever we do now with them is going to become stale sooner or later, leaving it as plaintext seems like the cleanest approach. That makes sense to me, especially since there's no additional protection that you get for encrypting them with a forever-fixed key.

On Thu, Aug 3, 2017 at 3:52 PM, ekr notifications@github.com wrote:

But now this is the only thing with some special snowflake anti-injection. I don't see how that makes things better.

On Wed, Aug 2, 2017 at 7:57 PM, Ryan Hamilton notifications@github.com wrote:

Agreed. Echoing the connection id seems to provide sufficient entropy to defend against off-path attackers.

On Wed, Aug 2, 2017 at 7:34 PM, janaiyengar notifications@github.com wrote:

That was my thinking. The protection is as good as the packet encrypted with the connection ID, since the information required to generate the VN is the same in both cases.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/quicwg/base-drafts/issues/693# issuecomment-319851408 , or mute the thread https://github.com/notifications/unsubscribe-auth/ASp6yl2Lr- 8KHUakdZ49tqGUtdBhlPk4ks5sUTG-gaJpZM4OfFhh

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/quicwg/base-drafts/issues/693#issuecomment-319854352 , or mute the thread https://github.com/notifications/unsubscribe-auth/ ABD1oRI2adgeAAd4bq4m3CijJmmlabgGks5sUTcHgaJpZM4OfFhh .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/quicwg/base-drafts/issues/693#issuecomment-320010951, or mute the thread https://github.com/notifications/unsubscribe-auth/AKjg1FDaaPr7OVpcDH9U1b0tGDo5LKYRks5sUezGgaJpZM4OfFhh .

ekr commented 7 years ago

I think it's valuable to have a consistent set of defenses that are easy to reason about.

However, it seems that we have consensus to do this for all packets other than VN and Stateless Reset, and VN doesn't have any integrity check at all now. So why don't I send a PR that does this for the other packets and then we can discuss VN in Seattle?

On Thu, Aug 3, 2017 at 5:34 PM, janaiyengar notifications@github.com wrote:

As Mike pointed out, VN and Stateless Reset are special snowflakes, no matter what. The point is that since whatever we do now with them is going to become stale sooner or later, leaving it as plaintext seems like the cleanest approach. That makes sense to me, especially since there's no additional protection that you get for encrypting them with a forever-fixed key.

On Thu, Aug 3, 2017 at 3:52 PM, ekr notifications@github.com wrote:

But now this is the only thing with some special snowflake anti-injection. I don't see how that makes things better.

On Wed, Aug 2, 2017 at 7:57 PM, Ryan Hamilton notifications@github.com wrote:

Agreed. Echoing the connection id seems to provide sufficient entropy to defend against off-path attackers.

On Wed, Aug 2, 2017 at 7:34 PM, janaiyengar notifications@github.com wrote:

That was my thinking. The protection is as good as the packet encrypted with the connection ID, since the information required to generate the VN is the same in both cases.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/quicwg/base-drafts/issues/693# issuecomment-319851408 , or mute the thread https://github.com/notifications/unsubscribe-auth/ASp6yl2Lr- 8KHUakdZ49tqGUtdBhlPk4ks5sUTG-gaJpZM4OfFhh

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/quicwg/base-drafts/issues/693# issuecomment-319854352 , or mute the thread https://github.com/notifications/unsubscribe-auth/ ABD1oRI2adgeAAd4bq4m3CijJmmlabgGks5sUTcHgaJpZM4OfFhh .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/quicwg/base-drafts/issues/693#issuecomment-320010951 , or mute the thread https://github.com/notifications/unsubscribe-auth/ AKjg1FDaaPr7OVpcDH9U1b0tGDo5LKYRks5sUezGgaJpZM4OfFhh

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/quicwg/base-drafts/issues/693#issuecomment-320125396, or mute the thread https://github.com/notifications/unsubscribe-auth/ABD1oQWsZvGLuMCKTAF6k9E48X5mBEcAks5sUmcVgaJpZM4OfFhh .

janaiyengar commented 7 years ago

Sorry - I thought I had responded to this, but I dropped it. Yes, your plan SGTM.

janaiyengar commented 7 years ago

From discussion in Seattle: AEAD protect all cleartext packet types, leave Stateless Reset and Version Negotiation as they are. EKR to produce PR for AEAD protection.

larseggert commented 7 years ago

Could @ekr in the process also add some text that makes it clear(er) that Stateless Reset and Version Negotiation are special? See #840.

mcmanus commented 7 years ago

does this mean we can get rid of the initial packet number randomization now that this serves as proof-of-onpath?

On Mon, Oct 9, 2017 at 8:02 PM, Martin Thomson notifications@github.com wrote:

Closed #693 https://github.com/quicwg/base-drafts/issues/693 via a0c571b https://github.com/quicwg/base-drafts/commit/a0c571b42e22bdbf2f8e49f9fec03c2be6816ab5 .

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/quicwg/base-drafts/issues/693#event-1285102574, or mute the thread https://github.com/notifications/unsubscribe-auth/AAP5s4o7Ge52eXAZOknJn8IRD65rB7xHks5sqrQngaJpZM4OfFhh .

RyanTheOptimist commented 7 years ago

On Tue, Oct 10, 2017 at 7:26 AM, Patrick McManus notifications@github.com wrote:

does this mean we can get rid of the initial packet number randomization now that this serves as proof-of-onpath?

​I love it!​

huitema commented 7 years ago

But if we remove packet number randomization, do we start with SN=0 or SN=1?

ianswett commented 7 years ago
  1. We really should ban 0. It's a pain.
janaiyengar commented 7 years ago

Ha, interesting. Removing randomization SGTM. And yes, please, please SN=1. 0 is special.