Skip to content

Commit

Permalink
reject unsupported capability versions
Browse files Browse the repository at this point in the history
Signed-off-by: Kristoffer Dalby <[email protected]>
  • Loading branch information
kradalby committed Nov 19, 2023
1 parent b77e752 commit 9ba6582
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 32 deletions.
12 changes: 12 additions & 0 deletions hscontrol/auth_noise.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion hscontrol/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
19 changes: 8 additions & 11 deletions hscontrol/mapper/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -85,8 +82,6 @@ func NewMapper(
uid, _ := util.GenerateRandomStringDNSSafe(mapperIDLength)

return &Mapper{
capVer: capVer,

derpMap: derpMap,
baseDomain: baseDomain,
dnsCfg: dnsCfg,
Expand Down Expand Up @@ -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
}
Expand All @@ -216,7 +212,7 @@ func (m *Mapper) fullMapResponse(
resp,
pol,
node,
m.capVer,
capVer,
peers,
peers,
m.baseDomain,
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -309,7 +305,7 @@ func (m *Mapper) PeerChangedResponse(
&resp,
pol,
node,
m.capVer,
mapRequest.Version,
nodeMapToList(m.peers),
changed,
m.baseDomain,
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion hscontrol/mapper/mapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,6 @@ func Test_fullMapResponse(t *testing.T) {
mappy := NewMapper(
tt.node,
tt.peers,
0,
tt.derpMap,
tt.baseDomain,
tt.dnsConfig,
Expand All @@ -486,6 +485,7 @@ func Test_fullMapResponse(t *testing.T) {
got, err := mappy.fullMapResponse(
tt.node,
tt.pol,
0,
)

if (err != nil) != tt.wantErr {
Expand Down
7 changes: 2 additions & 5 deletions hscontrol/poll.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -156,7 +156,6 @@ func (h *Headscale) handlePoll(
mapp := mapper.NewMapper(
node,
peers,
capVer,
h.DERPMap,
h.cfg.BaseDomain,
h.cfg.DNSConfig,
Expand Down Expand Up @@ -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)

Expand All @@ -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,
Expand Down
32 changes: 18 additions & 14 deletions hscontrol/poll_noise.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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)
}

0 comments on commit 9ba6582

Please sign in to comment.