Skip to content

Commit 6409517

Browse files
committed
hscontrol: gate /key on supported capability version
/key handed out the Noise public key for any v>=39, a floor unrelated to the handshake's capver.MinSupportedCapabilityVersion. Reject below the supported floor, matching /ts2021, and drop the stale constant. Fixes #3380
1 parent f20f1f1 commit 6409517

2 files changed

Lines changed: 66 additions & 22 deletions

File tree

hscontrol/handlers.go

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,6 @@ import (
1919
)
2020

2121
const (
22-
// NoiseCapabilityVersion is used by Tailscale clients to indicate
23-
// their codebase version. Tailscale clients can communicate over TS2021
24-
// from CapabilityVersion 28, but we only have good support for it
25-
// since https://github.com/tailscale/tailscale/pull/4323 (Noise in any HTTPS port).
26-
//
27-
// Related to this change, there is https://github.com/tailscale/tailscale/pull/5379,
28-
// where CapabilityVersion 39 is introduced to indicate #4323 was merged.
29-
//
30-
// See also https://github.com/tailscale/tailscale/blob/main/tailcfg/tailcfg.go
31-
NoiseCapabilityVersion = 39
32-
3322
reservedResponseHeaderSize = 4
3423
)
3524

@@ -199,20 +188,27 @@ func (h *Headscale) KeyHandler(
199188
return
200189
}
201190

202-
// TS2021 (Tailscale v2 protocol) requires to have a different key
203-
if capVer >= NoiseCapabilityVersion {
204-
resp := tailcfg.OverTLSPublicKeyResponse{
205-
PublicKey: h.noisePrivateKey.Public(),
206-
}
191+
// Only disclose the Noise public key to clients this server can
192+
// actually complete a handshake with. Gating on the same floor the
193+
// Noise handshake enforces (capver.MinSupportedCapabilityVersion, see
194+
// isSupportedVersion in noise.go) keeps /key consistent with /ts2021:
195+
// versions the handshake would reject get a clear rejection here
196+
// instead of a key that only serves as a version-boundary oracle.
197+
// See https://github.com/juanfont/headscale/issues/3380.
198+
if !isSupportedVersion(capVer) {
199+
httpError(writer, NewHTTPError(http.StatusBadRequest, "unsupported client version", unsupportedClientError(capVer)))
200+
return
201+
}
207202

208-
writer.Header().Set("Content-Type", "application/json")
203+
resp := tailcfg.OverTLSPublicKeyResponse{
204+
PublicKey: h.noisePrivateKey.Public(),
205+
}
209206

210-
err := json.NewEncoder(writer).Encode(resp)
211-
if err != nil {
212-
log.Error().Err(err).Msg("failed to encode public key response")
213-
}
207+
writer.Header().Set("Content-Type", "application/json")
214208

215-
return
209+
err = json.NewEncoder(writer).Encode(resp)
210+
if err != nil {
211+
log.Error().Err(err).Msg("failed to encode public key response")
216212
}
217213
}
218214

hscontrol/handlers_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@ import (
55
"context"
66
"encoding/json"
77
"errors"
8+
"fmt"
89
"net/http"
910
"net/http/httptest"
1011
"net/netip"
1112
"strings"
1213
"testing"
1314

15+
"github.com/juanfont/headscale/hscontrol/capver"
1416
"github.com/juanfont/headscale/hscontrol/types"
1517
"github.com/stretchr/testify/assert"
1618
"github.com/stretchr/testify/require"
@@ -116,6 +118,52 @@ func TestVerifyHandler_SuccessSetsJSONContentType(t *testing.T) {
116118
"successful /verify response must advertise application/json")
117119
}
118120

121+
// TestKeyHandler_UnsupportedCapVerDoesNotLeakKey reproduces
122+
// https://github.com/juanfont/headscale/issues/3380. The /key handler
123+
// must gate key disclosure on the same floor the Noise handshake
124+
// enforces (capver.MinSupportedCapabilityVersion). A capability version
125+
// below that floor can never complete a handshake, so it must be
126+
// rejected rather than handed the server's Noise public key, which would
127+
// otherwise serve only as a fingerprint / version-boundary oracle.
128+
func TestKeyHandler_UnsupportedCapVerDoesNotLeakKey(t *testing.T) {
129+
t.Parallel()
130+
131+
noise := key.NewMachine()
132+
h := &Headscale{noisePrivateKey: &noise}
133+
134+
unsupported := capver.MinSupportedCapabilityVersion - 1
135+
136+
rec := httptest.NewRecorder()
137+
req := httptest.NewRequestWithContext(
138+
context.Background(),
139+
http.MethodGet,
140+
fmt.Sprintf("/key?v=%d", unsupported),
141+
nil,
142+
)
143+
144+
h.KeyHandler(rec, req)
145+
146+
assert.Equal(t, http.StatusBadRequest, rec.Code,
147+
"a client below the supported floor must be rejected")
148+
assert.NotContains(t, rec.Body.String(), noise.Public().String(),
149+
"must not disclose Noise public key to a client below the supported floor")
150+
151+
// A supported client still receives the key.
152+
recOK := httptest.NewRecorder()
153+
reqOK := httptest.NewRequestWithContext(
154+
context.Background(),
155+
http.MethodGet,
156+
fmt.Sprintf("/key?v=%d", capver.MinSupportedCapabilityVersion),
157+
nil,
158+
)
159+
160+
h.KeyHandler(recOK, reqOK)
161+
162+
assert.Equal(t, http.StatusOK, recOK.Code)
163+
assert.Contains(t, recOK.Body.String(), noise.Public().String(),
164+
"a supported client must receive the Noise public key")
165+
}
166+
119167
// errorAsHTTPError is a small local helper that unwraps an [HTTPError]
120168
// from an error chain.
121169
func errorAsHTTPError(err error) (HTTPError, bool) {

0 commit comments

Comments
 (0)