control/controlbase: make the protocol version number selectable. #4370
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bradfitz
reviewed
Apr 7, 2022
tailcfg/tailcfg.go
Outdated
| @@ -34,7 +34,7 @@ import ( | |||
| // Previously (prior to 2022-03-06), it was known as the "MapRequest | |||
| // version" or "mapVer" or "map cap" and that name and usage persists | |||
| // in places. | |||
| type CapabilityVersion int | |||
| type CapabilityVersion uint16 | |||
There was a problem hiding this comment.
I don't think this will be worth the pain.
Just explode elsewhere if this ever goes over uint16 in size. (or write some test that fails if it goes over 64k)
There was a problem hiding this comment.
Switched tailcfg type back to int. I kept uint16 in controlbase and controlhttp, you okay with that?
Added a test that will fail when tailcfg version exceeds 60k, as an early warning to revise our wire protocol. Also added a hard panic() in controlclient if we ever try to start a session with a >uint16 version. Hard panic because the test should give us very early warning that the panic is coming, so something's very wrong if that codepath trips.
danderson
force-pushed
the
danderson/noise-capability
branch
2 times, most recently
from
c4916de
to
1c9ad7e
Compare
bradfitz
approved these changes
Apr 7, 2022
danderson
force-pushed
the
danderson/noise-capability
branch
from
1c9ad7e
to
14ce4cc
Compare
This is so that we can plumb our client capability version through the protocol as the Noise version. The capability version increments more frequently than strictly required (the Noise version only needs to change when cryptographically-significant changes are made to the protocol, whereas the capability version also indicates changes in non-cryptographically-significant parts of the protocol), but this gives us a safe pre-auth way to determine if the client supports future protocol features, while still relying on Noise's strong assurance that the client and server have agreed on the same version. Currently, the server executes the same protocol regardless of the version number, and just presents the version to the caller so they can do capability-based things in the upper RPC protocol. In future, we may add a ratchet to disallow obsolete protocols, or vary the Noise handshake behavior based on requested version. Updates #3488 Signed-off-by: David Anderson <danderson@tailscale.com>
danderson
force-pushed
the
danderson/noise-capability
branch
from
14ce4cc
to
e44caf9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is so that we can plumb our client capability version through
the protocol as the Noise version. The capability version increments
more frequently than strictly required (the Noise version only needs
to change when cryptographically-significant changes are made to
the protocol, whereas the capability version also indicates changes
in non-cryptographically-significant parts of the protocol), but this
gives us a safe pre-auth way to determine if the client supports
future protocol features, while still relying on Noise's strong
assurance that the client and server have agreed on the same version.
Currently, the server executes the same protocol regardless of the
version number, and just presents the version to the caller so they
can do capability-based things in the upper RPC protocol. In future,
we may add a ratchet to disallow obsolete protocols, or vary the
Noise handshake behavior based on requested version.
Signed-off-by: David Anderson danderson@tailscale.com