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

Compatible version upgrade #1901

Closed
wants to merge 30 commits into from
Closed

Compatible version upgrade #1901

wants to merge 30 commits into from

Conversation

martinthomson
Copy link
Member

This introduces the design that proposed by ekr prior to the NY interim.
In this design, the client advertises the list of versions it supports
in its transport parameters. The server is permitted to select a
"compatible" version and proceed as though the client selected that
version.

The main advantage of this approach is seamless upgrade between
compatible versions without the extra round-trip imposed by Version
Negotiation. This will be especially useful when we move from
pre-release versions (0xff0000xx) to the final version.

Closes #1773, #1755.

This introduces the design that proposed by ekr prior to the NY interim.
In this design, the client advertises the list of versions it supports
in its transport parameters.  The server is permitted to select a
"compatible" version and proceed as though the client selected that
version.

The main advantage of this approach is seamless upgrade between
compatible versions without the extra round-trip imposed by Version
Negotiation.  This will be especially useful when we move from
pre-release versions (0xff0000xx) to the final version.

Closes #1773, #1755.
@martinthomson martinthomson added design An issue that affects the design of the protocol; resolution requires consensus. -transport labels Oct 24, 2018

A server MUST NOT send a Version Negotiation packet if it prefers a version that
is not compatible with the version the client initially chose; a server has to
allow the client to choose between versions that are not compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if the client wants to use a specific other version if possible, but it does not want to do it in-flight within the current connection, for example to keep the client simple? It cannot omit that version from TP because then the current version will continue. It cannot include that version in TP because then it might get in-flight version change.

Copy link
Contributor

Choose a reason for hiding this comment

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

What’s the use case for this? I don’t really see the point of “keeping the client simple” here, this is not one of the complex areas of the protocol.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you implement each version as a separate binary or in a separate process. You could send on the that other version instead and hope the best, but that could result in many unnecessary version negotations if that version is not (yet) widely supported.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you were to implement each version in a different process, then this would not work, yes. Unless you wanted to complicate the hand-off from a simple exec quic-vY --disable-vn to something more like exec quic-vY --handoff=<translated packet+necessary keys>. The consequence is that you probably want to have compatible versions handled by the same instance.

2. The first message MUST include the transport parameters chosen by the client.
This enables upgrade to a compatible version (see {{version-upgrade}}).

3. The first message MUST fit within a 1232 octet QUIC packet payload. This
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 this already implied by 1.?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, and I think that we agreed that the 1232 thing was wrong. I'll cite 4.3 of the -tls doc instead.

is sent by an attacker.

To protect against these attacks, the transport parameters includes the complete
list of versions that a client is willing to use, with the version they used for
Copy link
Contributor

Choose a reason for hiding this comment

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

s/they/it


The client MUST include the first QUIC version it attempts to use as the first
entry of the supported_versions list in the transport parameters. A server MUST
close the connection attempt with a VERSION_NEGOTIATION_ERROR if it supports the
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence has some leftover parts, I assume.

Copy link
Member

@kazuho kazuho Oct 25, 2018

Choose a reason for hiding this comment

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

My understanding is that the intent of the text is to detect version downgrade attack at the server-side instead of doing on the client-side. IMO we need to make the change; see #1810.

However, I am not sure if the proposed design is sufficient, because it only detects a repression of only "one version" (being identified the first entry of supported_versions), while an attacker can inject VN to repress multiple versions.

Consider the case where a client supports three versions, v3, v2, v1 talking to a server that supports v2 and v1. Assume that the use of newer versions are preferable than the older ones. The client uses v3 in it's initial attempt. The attacker intercepts the Initial packet and responds with a fake VN that advertises v1 only. In such case, the server will not notice the downgrade attack and the connection will be established using v1.

IMO, it might make sense to send a list of versions that the client preferred but was unable to use due to the values included in VN that it received.

PS. To confess, I also used to believe that detecting downgrade of only one version was sufficient.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, so that would be the case where the v2 and v1 are not compatible, that is, the server has to respect the client's choice between v1 and v2. In this case, we're not respecting the client's preferences. I think that I'll have to take @ekr's design here to fix that. That is, the client advertises the version it attempts first, then it provides a list in preference order. Then validation at the server goes thus:

original version is supported, but not the current version = error
client prefers an incompatible version over the current version and the preferred version is supported by the server = error

Does that sound right?

about their interpretation. One way that a new format could be introduced is to
define a TLS extension with a different codepoint.

Transport parmeters for QUIC versions that are compatible with this QUIC version
Copy link
Contributor

Choose a reason for hiding this comment

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

Parameters.


A QUIC version is compatible with this version if the cryptographic handshake
message sent in the first packet can be used in both versions. A compatible
version is also able to identifying and acknowledge the first packet sent by the
Copy link
Contributor

Choose a reason for hiding this comment

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

s/identifying/identify


Upgrading in this manner allows a server to upgrade without incurring the round
trip imposed by sending a Version Negotiation packet. It also allows clients to
send their first packet using a widely deployed version, without the risk of
Copy link
Contributor

Choose a reason for hiding this comment

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

There’s no incentive to use a widely deployed version any more. A client can use the highest (compatible) version, and the server would just downgrade it to the highest version it supports.

Copy link
Contributor

Choose a reason for hiding this comment

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

The server might not know that there is a newer compatible version - compatible is both a spec thing and an implementation thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the incentive remains the same - the client offers what it believes to be most widely deployed. This design only allows for a seamless upgrade to a compatible version. If the client were to advertise the new, less-widely-deployed version, it risks getting a Version Negotiation packet.

Copy link
Member

Choose a reason for hiding this comment

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

A client can use the highest (compatible) version, and the server would just downgrade it to the highest version it supports.

It needs to be the lowest version for maximum flexibility.

Consider the case where we would be using 0x81 as the first octet for the Initial packet in QUIC v2, and that we have a client that wants to connect to a server using either v1 or v2. The way to provide room for such possibility is to let the client send a v1 Initial packet with an indication that the server can respond using a v2 Initial packet.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thinking of versions as being ordered is helpful, but might be constraining. The point is that the client will use the most widely deployed to start with and accept that the server is able to choose a compatible version. The client hopes that the server will choose a version that it prefers more, but leaves that to the server to decide.

In the case of monotonically increasing versions where A >> B >> C, this might seem obvious, but it's still possible that the client will advertise B and get an apparent downgrade to C. That's a property of the design that I don't think we'll see that often, but it's nonetheless theoretically possible.

But as @kazuho says, the expected case is a client advertising vN, then being opportunistically upgraded to vN+1. Like today, you can rely on TLS 1.0 being available widely, but you might prefer TLS 1.3.

Copy link
Contributor

Choose a reason for hiding this comment

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

The TLS comparison is somewhat inapt, since in TLS you actually do offer your highest (most preferred) version and let the server select a compatible, less-preferred version if it doesn't support that one. QUIC can't do that because the contents of the Initial aren't invariant.


A server MUST NOT send a Version Negotiation packet if it prefers a version that
is not compatible with the version the client initially chose; a server has to
allow the client to choose between versions that are not compatible.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need some text what a client is supposed to do when it receives a VNP that includes a compatible version to the one it initially selected?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. Because the server might not support every compatible version. If the client offers A, where B is compatible, the server can still send Version Negotiation in response with B because it doesn't understand A.


A client also includes a list of supported versions along with its transport
parameters. Transport parameters are carried in the first packet the client
sends. Sending the list of supported versions enables upgrade between
Copy link
Contributor

Choose a reason for hiding this comment

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

s/upgrade/upgrades

This complicates things slightly more.  The client has to order its
supported versions strictly (and respect that ordering in its selection
of versions).  The server has to validate things more carefully.

I have an algorithm for validation in comments, I'm interested in
whether that is worth putting in the draft.
@martinthomson
Copy link
Member Author

Thanks @kazuho for finding another hole. That validated the design that @ekr had for this, which separated the first attempted version from the supported versions list. It turns out that the preference order in the supported versions was necessary to prevent that. The client needs to tell the server how it would select between common versions so that the choice between incompatible versions can be validated.

This is slightly more complicated, but not much. Please review the algorithm I put in comments and let me know if you think that would be helpful in documenting the validation process.

@mikkelfj
Copy link
Contributor

Are we sure we want to do this? Usually I'm all for doing things smarter, but it seems like there is a huge attack vector here and a great opportunity for implementations to get things wrong, especially across versions.

Copy link
Member

@nibanks nibanks left a comment

Choose a reason for hiding this comment

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

Overall looks really good, and I think is a great improvement. Just a few minor comments.

The client then attempts to create a connection using the version it selects.
Though the content of the Initial packet the client sends might not change in
response to version negotiation, a client MUST increase the packet number it
uses on every packet it sends. Packets MUST continue to use long headers
Copy link
Member

Choose a reason for hiding this comment

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

Is this saying that even if the client creates an entirely new connection state it needs to increase the packet number still? I assume that wasn't intended. Perhaps Though the content of the Initial packet the client sends might not change in response to version negotiation, a client MUST increase the packet number it uses on every packet it sends. should be changed to something like If the content of the Initial packet the client sends does not change in response to version negotiation, a client MUST increase the packet number it uses on every packet it sends.?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll take the suggestion, which is better than what we have. In practice, the packet will probably change - it's a different version of the protocol after all.

packet from the server which is not a Version Negotiation packet, it MUST
discard other Version Negotiation packets on the same connection. Similarly, a
client MUST ignore a Version Negotiation packet if it has already received and
acted on a Version Negotiation packet.
Copy link
Member

Choose a reason for hiding this comment

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

Is it not theoretically possible that a client (that supports A,B,C) sends version A, and the server supports version {B, C} and responds with a version negotiation packet. THEN the server is updated to support only {C}. The client responds to the VN with B, and then gets a new VN in response, and would want to then try C? It's a pretty crappy experience and extremely small edge case, but I think possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is exactly the type of edge case I had in mind when talking about #504 (now quicwg/ops-drafts#28).

I don't think that we can fix that here, though this compatible version upgrade will make that a lot easier: if B and C are compatible, you can retain enough support for B that you can upgrade from B to C. It's not perfect, because you still have to send Version Negotiation in response to B if the client doesn't support C, but it could get you further in an incremental roll-back scenario.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, in that case the client should abort and possibly make a completely new connection attempt a limited number of times. Otherwise you can get into a loop between two round robin load balanced servers where you never terminate version negotiation.

Copy link
Contributor

Choose a reason for hiding this comment

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

The client is not allowed to respond to version {C} because it is only allow to handle one VN packet (which is important to avoid loops). But as I wrote, a client could make a new connection attempt which amounts to the same thing, so stricly speaking the connection would fail on timeout as the second VN packet is ignored.

Copy link
Member

Choose a reason for hiding this comment

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

Generally speaking, a server cluster will indicate a common subset of versions that it supports in VN.

When adding support for a new version, each of the server start accepting the new version when found in Initial. After that completes, the next would be to modify each server to start indicating the use of the new version in VN.

In case of removing the support for an old version, we will remove that version from the VN list, then stop accepting Initial with that version.

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
A server MUST NOT send a Version Negotiation packet if it prefers a version that
is not compatible with the version the client initially chose; a server has to
allow the client to choose between versions that are not compatible.

Copy link
Member

Choose a reason for hiding this comment

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

Should we have some text that says what to do if the server's supported list doesn't overlap at all with the client's? Should it still send a VN, even though it knows the client won't be able to continue the connection? I didn't see any text talking about that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's caught by the generic version negotiation text. But I note that we don't have that. #1917.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since you wrote this, new text appeared requiring a connection abort if a VN with no overlap is received. If the server has implicit knowledge about what the client supports it doesn't change anything - either it goes forward with a compatible version or it sends a VN packet.

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
version it supports, unless that version also matches the original_version
field.

<!-- Editor's note: should I include this algorithm?
Copy link
Member

Choose a reason for hiding this comment

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

👍 I find it useful.

nibanks and others added 6 commits October 26, 2018 06:28
Co-Authored-By: martinthomson <martin.thomson@gmail.com>
Co-Authored-By: martinthomson <martin.thomson@gmail.com>
Co-Authored-By: martinthomson <martin.thomson@gmail.com>
@kazuho
Copy link
Member

kazuho commented Oct 25, 2018

@martinthomson

Thanks @kazuho for finding another hole. That validated the design that @ekr had for this, which separated the first attempted version from the supported versions list. It turns out that the preference order in the supported versions was necessary to prevent that. The client needs to tell the server how it would select between common versions so that the choice between incompatible versions can be validated.

I think we agree, though I might say that we do not need a list that contains every version in the preference order. Just having a way to separate the supported versions into two groups in fine.

IMO, a workable design with minimal change would be use the packet version as a sentinel to divide the supported versions into two groups: a list that contains the versions that the client was not able to choose (due to VN) and a list that contains the other supported versions.

Consider the case where client supports five versions: v1, v1.1, v1.2, v2, v3. Compatible version upgrade is possible between different minor versions but not between major versions.

The client, first attempts to connect using v3. The first packet will be a v3 packet. V1.x server responds with a VN that contains v1, v1.1, v1.2.

Then, the client sends an Initial packet in v1, that contains a supported list of (v3, v2, v1, v1.1, v1.2). The server can determine which versions that the client assumed was impossible to use (i.e. v3, v2), because they are placed before v1 which is the packet version.

The benefit of using this "grouping" approach instead of the "total preference order" approach is that it allows the use of the lowest version as the packet format when there are more than one compatible versions. That is generally preferable, because a server can perform a compatible version upgrade only if it can parse the packet format, and using the lowest version provides the maximum chance.

@martinthomson Do my comments sound sensible?

@martinthomson
Copy link
Member Author

Relying on grouping might work, but it's probably more complicated than necessary. Maybe something simpler again. Two lists:

  • unsupported_versions is everything the client thinks that the server doesn't support, empty if there wasn't a version negotiation, filled if there was

  • supported_versions is every (other) version the client supports

The server validates by seeing if unsupported_versions contains any version that it supports.

supported_versions could be every version the client supports, but that just wastes space, I will expressly allow the client to move items from supported_versions to unsupported_versions to save space.

The nice thing with that is that it forces the client to regenerate the Initial if it gets a Version Negotiation packet (because at least one version will always be added to the unsupported_versions bucket).

@marten-seemann
Copy link
Contributor

@kazuho How does this solve #1810?

Consider the case where client supports five versions: v1, v1.1, v1.2, v2, v3. Compatible version upgrade is possible between different minor versions but not between major versions.
The client, first attempts to connect using v3. The first packet will be a v3 packet. V1.x server responds with a VN that contains v1, v1.1, v1.2.
Then, the client sends an Initial packet in v1, that contains a supported list of (v3, v2, v1, v1.1, v1.2). The server can determine which versions that the client assumed was impossible to use (i.e. v3, v2), because they are placed before v1 which is the packet version.

The equivalent to the case you describe in #1810 is the server being upgraded to support v2 just after sending the VNP. When receiving the v1 Initial packet from the client, containing v3 and v2 as impossible versions, it will conclude that an attacker injected the VNP, right?

@martinthomson
Copy link
Member Author

@marten-seemann, I assume that the server would upgrade to support v2 in stages similar to what @kazuho suggests in this comment:

That is, the server first starts accepting v2, but not advertising support for it. In the revised design I proposed, it would also not object if v2 were to appear in unsupported versions during this phase of rollout.

Once all server instances have started accepting v2, then the server can start sending it in Version Negotiation packets.

With the proposed design there is a third phase - once there are no Version Negotiation packets in flight that don't advertise v2, the server can generate errors if unsupported_versions contains v2.

@kazuho
Copy link
Member

kazuho commented Oct 26, 2018

@martinthomson:

Relying on grouping might work, but it's probably more complicated than necessary. Maybe something simpler again. Two lists:
(snip)

Exactly. I agree that using two lists is the most intuitive way to send two groups of versions.

Two minor things that we might want to clarify:

  • Does the version number included in the packet header need to appear in supported_versions? If we are to allow omitting it we should be clear about that.
  • Is the server allowed to perform a compatible version upgrade against a version included in unsupported_versions? This could happen during the upgrade process of a server cluster. My slight preference goes to allowing that, because that simplifies the design by making the server the sole entity that detects version downgrade. But people might have different opinions.

EDIT. By rereading the comment right above I notice that we share the same preference for the second point.

@martinthomson
Copy link
Member Author

martinthomson commented Oct 26, 2018

Good questions.

Does the version number included in the packet header need to appear in supported_versions?

Yes. We want this under integrity protection. We want the union of both lists to include every version the client supports and the client won't ever attempt to connect with a version that it just put in unsupported_versions.

Is the server allowed to perform a compatible version upgrade against a version included in unsupported_versions?

Yes, and we should allow that to happen. The client won't care. It is only splitting the versions into two lists for purposes of detecting a downgrade attack. This should make the upgrade process easier. I will call this out explicitly.

Having just written up the text for upgrades, I think that this works.

@martinthomson
Copy link
Member Author

OK, change implemented.

I realized that we're now relying on different pieces for different capabilities. We use unsupported versions to protect against a spoofed Version Negotiation packet. That might lead a client to believe that the server doesn't support a version that it does.

We use the supported versions list and the compatible version upgrade to protect against an attacker rewriting the version of the Initial packet. That suggests that the server needs to choose, it can't just stay with a version that it doesn't prefer (of course, not choosing is a form of choosing, a server could easily not care).

Then I realized that there is a weakening of our guarantees if we don't record the final version in the transcript, so I put that back. This is mainly from a surplus of caution. An attacker could maybe rewrite the version in the server Initial packet and convince the client that the version the server chose was different. Of course, the version is under integrity protection for later packet types, so this would fail at that point, but it's not great that we don't have the final selection covered by the handshake, so I added it back. It's not that hard and it makes the integrity protection far more direct.


The format of the packet that a client sends might be different in the new
version. In this case, the client generates a new packet that conforms to the
selected version. Though different versions might convey information about
Copy link
Member

Choose a reason for hiding this comment

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

Aren't we requiring the client to always regenerate a packet, now that unsupported_versions change from an empty list to a non-empty list when receiving a VN?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh, I missed that one. I squashed the other text, but missed that one.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah! It's a gift to proof-readers 👍

be empty unless the client has received and processed a Version Negotiation
packet. A client MUST NOT omit a version that it supports from
`unsupported_versions`, unless the Version Negotiation packet it processed
listed that version.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this unless clause contradicts with the following sentence added above: Though different versions might convey information about versions differently, a client MUST NOT change the set of versions it claims to support.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, the text isn't wrong, but I'm not sure how to fix it. The set of versions that a client claims to support is the union of supported_versions and unsupported_versions. I've said that, but I think that there's a better way to structure the text to avoid having to say that.

@martinduke
Copy link
Contributor

As I see it, there are two separable changes here:

  1. Moving validation from client to server. The necessary change here is to move the transport parameter from EE to ClientHello. IMO this is no biggie, and addresses the server upgrade problem (i.e. clients can't distinguish between a server pool rolling upgrade and a downgrade attack).

  2. Allowing seamless transition between "compatible" versions. This certainly reduces the latency penalty. However this creates a more complicated hierarchy of versions, which is difficult to understand. I also do not feel great that we have though through the implications of taking a v1 packet and acking it in v2. For instance, what if v2 requires a different set of client transport parameters?

I would suggest a separate PR to move the TPs over, which should be merged relatively quickly, and a more extended discussion on seamless upgrade.

If we separate out item (2), however, I think we should retain supported_versions in Encrypted Extensions purely for next-connection informational purposes (and not for validation).

@RyanTheOptimist
Copy link
Contributor

@martinduke, I agree with your framing. Change 1 seems non-controversial. (Interestingly, we've had client-side QUIC downgrade detection since day 1 and have not had problems with spurious detection, but nonetheless this seems like a fine change.) Change 2 is a different story.

@DavidSchinazi
Copy link
Contributor

I agree with @martinduke. Making downgrade prevention more secure is important to QUICv1. However, I'm not yet convinced that "compatible version upgrade" makes sense as part of QUICv1. I think there are two use-cases for it:

  1. Upgrade from QUICv1-draft to QUICv1-RFC
    In this scenario we can safely assume that the relevant QUIC versions are compatible. However, we can also safely assume that the only deployments of QUIC will be HTTP. Therefore we can rely on Alt-Svc, and have no strong motivation for this mechanism.
  2. Upgrade from QUICv1 to QUICv2
    In this scenario we're not sure the versions will be compatible, and we also don't have a need to add this to QUICv1. If during design of QUICv2 we want this feature (and I personally believe we will), then we can add a QUICv1 extension indicating support for seamless upgrade to v2, and have browsers try v1 first.

To be clear, I really like this solution - I think it's elegant and efficient. However, it also adds complexity - do we really need it in v1?

@marten-seemann
Copy link
Contributor

  1. Upgrade from QUICv1-draft to QUICv1-RFC
    In this scenario we can safely assume that the relevant QUIC versions are compatible. However, we can also safely assume that the only deployments of QUIC will be HTTP. Therefore we can rely on Alt-Svc, and have no strong motivation for this mechanism.

This is not true. I'm deploying QUIC draft versions in IPFS already, and who knows what other people are doing...

@DavidSchinazi
Copy link
Contributor

Thanks @marten-seemann, that's interesting (and news to me). I'm not familiar with IPFS at all but, out of curiosity, how do your IPFS endpoints let each other know that they support QUIC? Does that mechanism have a way to communicate versions?

@mikkelfj
Copy link
Contributor

mikkelfj commented Nov 1, 2018

QUIC is useful for all sorts of custom data sync protocols that currently use TCP+TLS.

@marten-seemann
Copy link
Contributor

@DavidSchinazi Every endpoints announces a list of addresses it believes it is reachable at. We currently don't encode the QUIC version number into that address. I'd prefer to not do that, but instead have an efficient QUIC version negotiation mechanism take care of selecting the best version.

@nibanks
Copy link
Member

nibanks commented Nov 1, 2018

Microsoft also has a number of other non-HTTP projects under way using QUIC that I think would benefit from this.

@DavidSchinazi
Copy link
Contributor

@mikkelfj I agree that this is definitely useful. The only question I'm asking is which document this should land in and when.
@nibanks, can you comment on which use-case you would benefit from? (draft-to-RFC or v1-to-v2?)

@nibanks
Copy link
Member

nibanks commented Nov 1, 2018

@DavidSchinazi possibly both. We will likely have limited deployment of some of the projects before QUIC hits RFC status and we may end up using a draft version number for those. Beyond that, since those protocols won't necessarily have an alt-svc design, the v1-to-v2 would likely be beneficial as well.

@martinduke
Copy link
Contributor

Can we land the bit where we add supported-versions to client TPs and argue about seamless upgrade in Bangkok?

@DavidSchinazi
Copy link
Contributor

@martinduke +1

@kazuho
Copy link
Member

kazuho commented Nov 2, 2018

In my view, one of the potential issues regarding the current design of "compatible version upgrade" is that it states that the server "MAY" upgrade, while not giving the client a way to opt out.

Would it help the people not interested in implementing compatible version upgrade if the feature was made optional from the client's viewpoint?

@mikkelfj
Copy link
Contributor

mikkelfj commented Nov 2, 2018

@kazuho I think the objection has more to do with interop and unforeseen cornercases and vulnerabilities than it has to do with ones own implementation.

@martinduke
Copy link
Contributor

After discussing this with @kazuho, @janaiyengar, and @ianswett in Bangkok, I'm not sure the server upgrade problem has been correctly reasoned.

Downgrade attack detection happens in the context of a single connection. After receiving VN, the client uses the same CID and increments the packet number. Non-pathological load balancers will direct this to the same server. People have repeatedly pointed out to me that the LB config could flip over in that moment, but that is an edge case of an edge case, and the only consequence is a fatal connection error and a possible security alert.

It might we worth adding somewhere that in the event of a downgrade alert, the client SHOULD NOT cache the contents of VN for future use. Also, if the server's TPs don't match the contents of a client version cache from a different connection, that is not a downgrade attack.

Furthermore, @ianswett points out that downgrade attack detection traditionally happens at the client. We haven't fully thought through the incentives and other implications of moving this responsibility to the server.

In summary, I no longer support moving downgrade detection to the server. I am open to seamless upgrade in v2 after we do some more analysis to make sure there are no gotchas.

@martinduke
Copy link
Contributor

In Bangkok @kazuho short-circuited my objection at the mic by pointing out that the min DCID length might be different in other versions.

Upon further review, this requires a minor tweak to the spec: the client MUST select an initial DCID long enough for every version it supports. This implies that the maximum initial DCID length is invariant across versions, but I can't see a use case where we would drop that below 18.

@mikkelfj
Copy link
Contributor

mikkelfj commented Nov 7, 2018

I must admit I don’t quite follow why min DCID is important.

Regardless, forcing a higher min DCID is unfortunate because a system that relies on small DCIDs in its default operational mode will be forced to not support other versions with larger DCIDs.

@kazuho
Copy link
Member

kazuho commented Nov 7, 2018

@martinduke:

In Bangkok @kazuho short-circuited my objection at the mic by pointing out that the min DCID length might be different in other versions.

Upon further review, this requires a minor tweak to the spec: the client MUST select an initial DCID long enough for every version it supports. This implies that the maximum initial DCID length is invariant across versions, but I can't see a use case where we would drop that below 18.

FWIW, let me note that in the current design, it is not impossible for a server cluster with mixed supported versions to respond in a way that does not cause the clients to see "downgrade attacks", even in case the resent Initial lands on a different server within the cluster.

This is how you would do in case you are upgrading a cluster to support v2:

  1. Some of the servers start accepting v2, but continues to claim support only for v1 in the VN packets.
  2. During this period, when the server receives a v1 Initial, it responds with supported_versions that only contains v1. When the server receives a v2 Initial, it responds with supported_versions that contains both v1 and v2.
  3. After all servers start accepting v2, they start sending v2 in the VN packet and also in supported_versions.

The difference from what you would do when we have "compatible version upgrade" is step 2; we need to send different supported_versions based on the version number of the client's Initial. This looks strange, but I think it would work.

@igorlord
Copy link
Contributor

igorlord commented Nov 7, 2018

Upon further review, this requires a minor tweak to the spec: the client MUST select an initial DCID long enough for every version it supports. This implies that the maximum initial DCID length is invariant across versions, but I can't see a use case where we would drop that below 18.

@martinduke, I think it is not only an issue with max DCID but also min DCID. The client MUST select an initial DCID whose length falls within the range of valid DCID lengths for each version it supports. What this means is that if versions A and B require DCIDs to be between (Xa and Ya) and (Xb and Yb) long respectively, Xa-Ya and Xb-Yb ranges would better overlap, or no client would be able to support both A and B.

The Invariants could specify the largest-allowed-min and smallest-allowed-max DCID lengths. But it is also possible to leave the matter to future version specs, which would need to take care of ensuring an overlap in DCID length with all prior versions they wish to "co-exist" with.

@martinduke
Copy link
Contributor

@igorlord I agree, although I cannot see a use case that would really require an upper limit on initial DCID len. So I think it is simpler just to hold the max length constant and allow versions to vary min length (as this value is needed to provide minimum entropy for Retry and VN packets).

@mikkelfj
Copy link
Contributor

mikkelfj commented Nov 7, 2018

How about separating VN / Retry entropy from DCID lengths if this is the only thing that blocks short DCIDs from coexisting with "standard" DCID min lengths?

@martinduke
Copy link
Contributor

How would you do that? This is only relevant to the Initial DCID, not the server-proposed one.

@mikkelfj
Copy link
Contributor

mikkelfj commented Nov 7, 2018

If the version dependent DCID length really only is a concern for the intial packet, then I suggest that this length is hardcoded to 128-bits, independent of SCID/DCID length requirements of any given version.

@mikkelfj
Copy link
Contributor

mikkelfj commented Nov 7, 2018

Note that the initial DCID is expected to be random and not contain any load balancer coordination etc.

@martinthomson
Copy link
Member Author

After talking to several people, I think that we will close this PR for now while we consider our options.

Note that @kazuho's suggested design isn't perfect. It still has a problem around the transition from step 2 to 3.

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.