From 9ba6582f71567b4fbcfbdaa5e693f4a97637db7b Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 19 Nov 2023 20:56:31 +0100 Subject: [PATCH] reject unsupported capability versions Signed-off-by: Kristoffer Dalby --- hscontrol/auth_noise.go | 12 ++++++++++++ hscontrol/handlers.go | 2 +- hscontrol/mapper/mapper.go | 19 ++++++++----------- hscontrol/mapper/mapper_test.go | 2 +- hscontrol/poll.go | 7 ++----- hscontrol/poll_noise.go | 32 ++++++++++++++++++-------------- 6 files changed, 42 insertions(+), 32 deletions(-) diff --git a/hscontrol/auth_noise.go b/hscontrol/auth_noise.go index 4fb4bb05e6b..323a49b0936 100644 --- a/hscontrol/auth_noise.go +++ b/hscontrol/auth_noise.go @@ -39,6 +39,18 @@ func (ns *noiseServer) NoiseRegistrationHandler( return } + // Reject unsupported versions + if registerRequest.Version < MinimumCapVersion { + log.Info(). + Caller(). + Int("min_version", int(MinimumCapVersion)). + Int("client_version", int(registerRequest.Version)). + Msg("unsupported client connected") + http.Error(writer, "Internal error", http.StatusBadRequest) + + return + } + ns.nodeKey = registerRequest.NodeKey ns.headscale.handleRegister(writer, req, registerRequest, ns.conn.Peer()) diff --git a/hscontrol/handlers.go b/hscontrol/handlers.go index 437b9d49b2c..ee670733a0d 100644 --- a/hscontrol/handlers.go +++ b/hscontrol/handlers.go @@ -80,7 +80,7 @@ func (h *Headscale) KeyHandler( log.Debug(). Str("handler", "/key"). - Int("v", int(capVer)). + Int("cap_ver", int(capVer)). Msg("New noise client") if err != nil { writer.Header().Set("Content-Type", "text/plain; charset=utf-8") diff --git a/hscontrol/mapper/mapper.go b/hscontrol/mapper/mapper.go index 736c459f891..66ecb4fe61c 100644 --- a/hscontrol/mapper/mapper.go +++ b/hscontrol/mapper/mapper.go @@ -47,8 +47,6 @@ var debugDumpMapResponsePath = envknob.String("HEADSCALE_DEBUG_DUMP_MAPRESPONSE_ // - Create a "minifier" that removes info not needed for the node type Mapper struct { - capVer tailcfg.CapabilityVersion - // Configuration // TODO(kradalby): figure out if this is the format we want this in derpMap *tailcfg.DERPMap @@ -70,7 +68,6 @@ type Mapper struct { func NewMapper( node *types.Node, peers types.Nodes, - capVer tailcfg.CapabilityVersion, derpMap *tailcfg.DERPMap, baseDomain string, dnsCfg *tailcfg.DNSConfig, @@ -85,8 +82,6 @@ func NewMapper( uid, _ := util.GenerateRandomStringDNSSafe(mapperIDLength) return &Mapper{ - capVer: capVer, - derpMap: derpMap, baseDomain: baseDomain, dnsCfg: dnsCfg, @@ -204,10 +199,11 @@ func addNextDNSMetadata(resolvers []*dnstype.Resolver, node *types.Node) { func (m *Mapper) fullMapResponse( node *types.Node, pol *policy.ACLPolicy, + capVer tailcfg.CapabilityVersion, ) (*tailcfg.MapResponse, error) { peers := nodeMapToList(m.peers) - resp, err := m.baseWithConfigMapResponse(node, pol) + resp, err := m.baseWithConfigMapResponse(node, pol, capVer) if err != nil { return nil, err } @@ -216,7 +212,7 @@ func (m *Mapper) fullMapResponse( resp, pol, node, - m.capVer, + capVer, peers, peers, m.baseDomain, @@ -239,7 +235,7 @@ func (m *Mapper) FullMapResponse( m.mu.Lock() defer m.mu.Unlock() - resp, err := m.fullMapResponse(node, pol) + resp, err := m.fullMapResponse(node, pol, mapRequest.Version) if err != nil { return nil, err } @@ -255,7 +251,7 @@ func (m *Mapper) LiteMapResponse( node *types.Node, pol *policy.ACLPolicy, ) ([]byte, error) { - resp, err := m.baseWithConfigMapResponse(node, pol) + resp, err := m.baseWithConfigMapResponse(node, pol, mapRequest.Version) if err != nil { return nil, err } @@ -309,7 +305,7 @@ func (m *Mapper) PeerChangedResponse( &resp, pol, node, - m.capVer, + mapRequest.Version, nodeMapToList(m.peers), changed, m.baseDomain, @@ -473,10 +469,11 @@ func (m *Mapper) baseMapResponse() tailcfg.MapResponse { func (m *Mapper) baseWithConfigMapResponse( node *types.Node, pol *policy.ACLPolicy, + capVer tailcfg.CapabilityVersion, ) (*tailcfg.MapResponse, error) { resp := m.baseMapResponse() - tailnode, err := tailNode(node, m.capVer, pol, m.dnsCfg, m.baseDomain, m.randomClientPort) + tailnode, err := tailNode(node, capVer, pol, m.dnsCfg, m.baseDomain, m.randomClientPort) if err != nil { return nil, err } diff --git a/hscontrol/mapper/mapper_test.go b/hscontrol/mapper/mapper_test.go index d286a894f60..865372fee78 100644 --- a/hscontrol/mapper/mapper_test.go +++ b/hscontrol/mapper/mapper_test.go @@ -475,7 +475,6 @@ func Test_fullMapResponse(t *testing.T) { mappy := NewMapper( tt.node, tt.peers, - 0, tt.derpMap, tt.baseDomain, tt.dnsConfig, @@ -486,6 +485,7 @@ func Test_fullMapResponse(t *testing.T) { got, err := mappy.fullMapResponse( tt.node, tt.pol, + 0, ) if (err != nil) != tt.wantErr { diff --git a/hscontrol/poll.go b/hscontrol/poll.go index f3651aad7c3..a827c7d664f 100644 --- a/hscontrol/poll.go +++ b/hscontrol/poll.go @@ -58,7 +58,6 @@ func (h *Headscale) handlePoll( ctx context.Context, node *types.Node, mapRequest tailcfg.MapRequest, - capVer tailcfg.CapabilityVersion, ) { logInfo, logErr := logPollFunc(mapRequest, node) @@ -79,6 +78,7 @@ func (h *Headscale) handlePoll( Str("node_key", node.NodeKey.ShortString()). Str("node", node.Hostname). Strs("endpoints", node.Endpoints). + Int("cap_ver", int(mapRequest.Version)). Msg("Received endpoint update") now := time.Now().UTC() @@ -125,7 +125,7 @@ func (h *Headscale) handlePoll( // The intended use is for clients to discover the DERP map at // start-up before their first real endpoint update. } else if mapRequest.OmitPeers && !mapRequest.Stream && mapRequest.ReadOnly { - h.handleLiteRequest(writer, node, mapRequest, capVer) + h.handleLiteRequest(writer, node, mapRequest) return } else if mapRequest.OmitPeers && mapRequest.Stream { @@ -156,7 +156,6 @@ func (h *Headscale) handlePoll( mapp := mapper.NewMapper( node, peers, - capVer, h.DERPMap, h.cfg.BaseDomain, h.cfg.DNSConfig, @@ -375,7 +374,6 @@ func (h *Headscale) handleLiteRequest( writer http.ResponseWriter, node *types.Node, mapRequest tailcfg.MapRequest, - capVer tailcfg.CapabilityVersion, ) { logInfo, logErr := logPollFunc(mapRequest, node) @@ -384,7 +382,6 @@ func (h *Headscale) handleLiteRequest( // TODO(kradalby): It might not be acceptable to send // an empty peer list here. types.Nodes{}, - capVer, h.DERPMap, h.cfg.BaseDomain, h.cfg.DNSConfig, diff --git a/hscontrol/poll_noise.go b/hscontrol/poll_noise.go index 67585d11737..ee1b67f9691 100644 --- a/hscontrol/poll_noise.go +++ b/hscontrol/poll_noise.go @@ -12,6 +12,10 @@ import ( "tailscale.com/types/key" ) +const ( + MinimumCapVersion tailcfg.CapabilityVersion = 36 +) + // NoisePollNetMapHandler takes care of /machine/:id/map using the Noise protocol // // This is the busiest endpoint, as it keeps the HTTP long poll that updates @@ -47,6 +51,18 @@ func (ns *noiseServer) NoisePollNetMapHandler( return } + // Reject unsupported versions + if mapRequest.Version < MinimumCapVersion { + log.Info(). + Caller(). + Int("min_version", int(MinimumCapVersion)). + Int("client_version", int(mapRequest.Version)). + Msg("unsupported client connected") + http.Error(writer, "Internal error", http.StatusBadRequest) + + return + } + ns.nodeKey = mapRequest.NodeKey node, err := ns.headscale.db.GetNodeByAnyKey( @@ -73,20 +89,8 @@ func (ns *noiseServer) NoisePollNetMapHandler( log.Debug(). Str("handler", "NoisePollNetMap"). Str("node", node.Hostname). + Int("cap_ver", int(mapRequest.Version)). Msg("A node sending a MapRequest with Noise protocol") - capVer, err := parseCabailityVersion(req) - if err != nil && !errors.Is(err, ErrNoCapabilityVersion) { - log.Error(). - Caller(). - Err(err). - Msg("failed to parse capVer") - http.Error(writer, "Internal error", http.StatusInternalServerError) - - return - } - - // TODO(kradalby): since we are now passing capVer, we could arguably stop passing - // isNoise, and rather have a isNoise function that takes capVer - ns.headscale.handlePoll(writer, req.Context(), node, mapRequest, capVer) + ns.headscale.handlePoll(writer, req.Context(), node, mapRequest) }