From c58ce6f60cd0e85750228acc30f24a3dda93b619 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 27 Feb 2022 18:40:10 +0100 Subject: [PATCH 01/32] Generalise the registration method to DRY stuff up --- cli_test.go | 2 ++ machine.go | 75 ++++++++++++++++++++++++++++------------------------- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/cli_test.go b/cli_test.go index 71f2bea5..2eedb5b4 100644 --- a/cli_test.go +++ b/cli_test.go @@ -32,6 +32,8 @@ func (s *Suite) TestRegisterMachine(c *check.C) { machineAfterRegistering, err := app.RegisterMachine( machine.MachineKey, namespace.Name, + RegisterMethodCLI, + nil, nil, nil, nil, ) c.Assert(err, check.IsNil) c.Assert(machineAfterRegistering.Registered, check.Equals, true) diff --git a/machine.go b/machine.go index dac44591..3415b408 100644 --- a/machine.go +++ b/machine.go @@ -44,8 +44,10 @@ type Machine struct { Registered bool // temp RegisterMethod string - AuthKeyID uint - AuthKey *PreAuthKey + + // TODO(kradalby): This seems like irrelevant information? + AuthKeyID uint + AuthKey *PreAuthKey LastSeen *time.Time LastSuccessfulUpdate *time.Time @@ -686,6 +688,13 @@ func (machine *Machine) toProto() *v1.Machine { func (h *Headscale) RegisterMachine( machineKeyStr string, namespaceName string, + registrationMethod string, + + // Optionals + expiry *time.Time, + authKey *PreAuthKey, + nodePublicKey *string, + lastSeen *time.Time, ) (*Machine, error) { namespace, err := h.GetNamespace(namespaceName) if err != nil { @@ -709,27 +718,13 @@ func (h *Headscale) RegisterMachine( return nil, err } - // TODO(kradalby): Currently, if it fails to find a requested expiry, non will be set - // This means that if a user is to slow with register a machine, it will possibly not - // have the correct expiry. - requestedTime := time.Time{} - if requestedTimeIf, found := h.requestedExpiryCache.Get(machineKey.String()); found { - log.Trace(). - Caller(). - Str("machine", machine.Name). - Msg("Expiry time found in cache, assigning to node") - if reqTime, ok := requestedTimeIf.(time.Time); ok { - requestedTime = reqTime - } - } - if machine.isRegistered() { log.Trace(). Caller(). Str("machine", machine.Name). Msg("machine already registered, reauthenticating") - h.RefreshMachine(machine, requestedTime) + h.RefreshMachine(machine, *expiry) return machine, nil } @@ -739,17 +734,6 @@ func (h *Headscale) RegisterMachine( Str("machine", machine.Name). Msg("Attempting to register machine") - if machine.isRegistered() { - err := errMachineAlreadyRegistered - log.Error(). - Caller(). - Err(err). - Str("machine", machine.Name). - Msg("Attempting to register machine") - - return nil, err - } - h.ipAllocationMutex.Lock() defer h.ipAllocationMutex.Unlock() @@ -764,17 +748,36 @@ func (h *Headscale) RegisterMachine( return nil, err } - log.Trace(). - Caller(). - Str("machine", machine.Name). - Str("ip", strings.Join(ips.ToStringSlice(), ",")). - Msg("Found IP for host") - machine.IPAddresses = ips + + if expiry != nil { + machine.Expiry = expiry + } + + if authKey != nil { + machine.AuthKeyID = uint(authKey.ID) + } + + if nodePublicKey != nil { + machine.NodeKey = *nodePublicKey + } + + if lastSeen != nil { + machine.LastSeen = lastSeen + } + machine.NamespaceID = namespace.ID + + // TODO(kradalby): This field is uneccessary metadata, + // move it to tags instead of having a column. + machine.RegisterMethod = registrationMethod + + // TODO(kradalby): Registered is a very frustrating value + // to keep up to date, and it makes is have to care if a + // machine is registered, authenticated and expired. + // Let us simplify the model, a machine is _only_ saved if + // it is registered. machine.Registered = true - machine.RegisterMethod = RegisterMethodCLI - machine.Expiry = &requestedTime h.db.Save(&machine) log.Trace(). From acb945841c6878c1af6e66522f146265bd7feaa3 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 27 Feb 2022 18:42:15 +0100 Subject: [PATCH 02/32] Generalise registration for pre auth keys --- api.go | 53 +++++++++++++++++++++++------------------------------ 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/api.go b/api.go index bb5495ae..810271cf 100644 --- a/api.go +++ b/api.go @@ -22,7 +22,7 @@ import ( const ( reservedResponseHeaderSize = 4 - RegisterMethodAuthKey = "authKey" + RegisterMethodAuthKey = RegisterMethodAuthKey RegisterMethodOIDC = "oidc" RegisterMethodCLI = "cli" ErrRegisterMethodCLIDoesNotSupportExpire = Error( @@ -545,7 +545,7 @@ func (h *Headscale) handleAuthKey( Err(err). Msg("Cannot encode message") ctx.String(http.StatusInternalServerError, "") - machineRegistrations.WithLabelValues("new", "authkey", "error", machine.Namespace.Name). + machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", machine.Namespace.Name). Inc() return @@ -556,7 +556,7 @@ func (h *Headscale) handleAuthKey( Str("func", "handleAuthKey"). Str("machine", machine.Name). Msg("Failed authentication via AuthKey") - machineRegistrations.WithLabelValues("new", "authkey", "error", machine.Namespace.Name). + machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", machine.Namespace.Name). Inc() return @@ -575,38 +575,31 @@ func (h *Headscale) handleAuthKey( Str("machine", machine.Name). Msg("Authentication key was valid, proceeding to acquire IP addresses") - h.ipAllocationMutex.Lock() + nodeKey := NodePublicKeyStripPrefix(registerRequest.NodeKey) + now := time.Now().UTC() - ips, err := h.getAvailableIPs() + _, err = h.RegisterMachine( + machine.Name, + machine.Namespace.Name, + RegisterMethodAuthKey, + ®isterRequest.Expiry, + pak, + &nodeKey, + &now, + ) if err != nil { log.Error(). Caller(). - Str("func", "handleAuthKey"). - Str("machine", machine.Name). - Msg("Failed to find an available IP address") - machineRegistrations.WithLabelValues("new", "authkey", "error", machine.Namespace.Name). - Inc() + Err(err). + Msg("could not register machine") + machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", machine.Namespace.Name).Inc() + ctx.String( + http.StatusInternalServerError, + "could not register machine", + ) return } - log.Info(). - Str("func", "handleAuthKey"). - Str("machine", machine.Name). - Str("ips", strings.Join(ips.ToStringSlice(), ",")). - Msgf("Assigning %s to %s", strings.Join(ips.ToStringSlice(), ","), machine.Name) - - machine.Expiry = ®isterRequest.Expiry - machine.AuthKeyID = uint(pak.ID) - machine.IPAddresses = ips - machine.NamespaceID = pak.NamespaceID - - machine.NodeKey = NodePublicKeyStripPrefix(registerRequest.NodeKey) - // we update it just in case - machine.Registered = true - machine.RegisterMethod = RegisterMethodAuthKey - h.db.Save(&machine) - - h.ipAllocationMutex.Unlock() } pak.Used = true @@ -622,13 +615,13 @@ func (h *Headscale) handleAuthKey( Str("machine", machine.Name). Err(err). Msg("Cannot encode message") - machineRegistrations.WithLabelValues("new", "authkey", "error", machine.Namespace.Name). + machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", machine.Namespace.Name). Inc() ctx.String(http.StatusInternalServerError, "Extremely sad!") return } - machineRegistrations.WithLabelValues("new", "authkey", "success", machine.Namespace.Name). + machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "success", machine.Namespace.Name). Inc() ctx.Data(http.StatusOK, "application/json; charset=utf-8", respBody) log.Info(). From fd1e4a1dcd4b08868f70f221eee4f2524a69ffdf Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 27 Feb 2022 18:42:24 +0100 Subject: [PATCH 03/32] Generalise registration for openid --- oidc.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/oidc.go b/oidc.go index edea32a8..5207c64e 100644 --- a/oidc.go +++ b/oidc.go @@ -328,27 +328,28 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { return } - ips, err := h.getAvailableIPs() + _, err = h.RegisterMachine( + machineKeyStr, + namespace.Name, + RegisterMethodOIDC, + &requestedTime, + nil, + nil, + &now, + ) if err != nil { log.Error(). Caller(). Err(err). - Msg("could not get an IP from the pool") + Msg("could not register machine") ctx.String( http.StatusInternalServerError, - "could not get an IP from the pool", + "could not register machine", ) return } - machine.IPAddresses = ips - machine.NamespaceID = namespace.ID - machine.Registered = true - machine.RegisterMethod = RegisterMethodOIDC - machine.LastSuccessfulUpdate = &now - machine.Expiry = &requestedTime - h.db.Save(&machine) } var content bytes.Buffer From caffbd8956776e97e54bbeb6b18ea30a4c287b40 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 27 Feb 2022 18:42:43 +0100 Subject: [PATCH 04/32] Update cli registration with new method --- grpcv1.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/grpcv1.go b/grpcv1.go index 60e181df..b1396bc8 100644 --- a/grpcv1.go +++ b/grpcv1.go @@ -159,9 +159,27 @@ func (api headscaleV1APIServer) RegisterMachine( Str("namespace", request.GetNamespace()). Str("machine_key", request.GetKey()). Msg("Registering machine") + + // TODO(kradalby): Currently, if it fails to find a requested expiry, non will be set + // This means that if a user is to slow with register a machine, it will possibly not + // have the correct expiry. + requestedTime := time.Time{} + if requestedTimeIf, found := api.h.requestedExpiryCache.Get(request.GetKey()); found { + log.Trace(). + Caller(). + Str("machine", request.Key). + Msg("Expiry time found in cache, assigning to node") + if reqTime, ok := requestedTimeIf.(time.Time); ok { + requestedTime = reqTime + } + } + machine, err := api.h.RegisterMachine( request.GetKey(), request.GetNamespace(), + RegisterMethodCLI, + &requestedTime, + nil, nil, nil, ) if err != nil { return nil, err From ecc26432fd64c19d3b314536fcfee634d53e3517 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 27 Feb 2022 18:48:12 +0100 Subject: [PATCH 05/32] Fix excessive replace --- api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api.go b/api.go index 810271cf..7d535322 100644 --- a/api.go +++ b/api.go @@ -22,7 +22,7 @@ import ( const ( reservedResponseHeaderSize = 4 - RegisterMethodAuthKey = RegisterMethodAuthKey + RegisterMethodAuthKey = "authkey" RegisterMethodOIDC = "oidc" RegisterMethodCLI = "cli" ErrRegisterMethodCLIDoesNotSupportExpire = Error( From 1caa6f5d6915b762cad4c9d5b28654ae0d2efe67 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 27 Feb 2022 18:48:25 +0100 Subject: [PATCH 06/32] Add todo for JSON datatype --- machine.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/machine.go b/machine.go index 3415b408..b805de67 100644 --- a/machine.go +++ b/machine.go @@ -53,6 +53,8 @@ type Machine struct { LastSuccessfulUpdate *time.Time Expiry *time.Time + // TODO(kradalby): Figure out a way to use tailcfg datatypes + // here and have gorm serialise them. HostInfo datatypes.JSON Endpoints datatypes.JSON EnabledRoutes datatypes.JSON From 469551bc5df6dd8483af1246560f2f358a2ed943 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 08:06:39 +0000 Subject: [PATCH 07/32] Register new machines needing callback in memory This commit stores temporary registration data in cache, instead of memory allowing us to only have actually registered machines in the database. --- api.go | 153 +++++++++++++++++++++++------------------------- app.go | 9 +++ cli_test.go | 1 + grpcv1.go | 3 +- machine.go | 52 +++++++++++++--- oidc.go | 7 +-- preauth_keys.go | 6 ++ 7 files changed, 136 insertions(+), 95 deletions(-) diff --git a/api.go b/api.go index 7d535322..60ca63f2 100644 --- a/api.go +++ b/api.go @@ -125,25 +125,40 @@ func (h *Headscale) RegistrationHandler(ctx *gin.Context) { machine, err := h.GetMachineByMachineKey(machineKey) if errors.Is(err, gorm.ErrRecordNotFound) { log.Info().Str("machine", req.Hostinfo.Hostname).Msg("New machine") - newMachine := Machine{ - Expiry: &time.Time{}, - MachineKey: MachinePublicKeyStripPrefix(machineKey), - Name: req.Hostinfo.Hostname, - } - if err := h.db.Create(&newMachine).Error; err != nil { - log.Error(). - Caller(). - Err(err). - Msg("Could not create row") - machineRegistrations.WithLabelValues("unknown", "web", "error", machine.Namespace.Name). - Inc() + + machineKeyStr := MachinePublicKeyStripPrefix(machineKey) + + // If the machine has AuthKey set, handle registration via PreAuthKeys + if req.Auth.AuthKey != "" { + h.handleAuthKey(ctx, machineKey, req) return } - machine = &newMachine + + // The machine did not have a key to authenticate, which means + // that we rely on a method that calls back some how (OpenID or CLI) + // We create the machine and then keep it around until a callback + // happens + newMachine := Machine{ + Expiry: &time.Time{}, + MachineKey: machineKeyStr, + Name: req.Hostinfo.Hostname, + NodeKey: NodePublicKeyStripPrefix(req.NodeKey), + LastSeen: &now, + } + + h.registrationCache.Set( + machineKeyStr, + newMachine, + requestedExpiryCacheExpiration, + ) + + h.handleMachineRegistrationNew(ctx, machineKey, req) + return } - if machine.Registered { + // The machine is already registered, so we need to pass through reauth or key update. + if machine != nil { // If the NodeKey stored in headscale is the same as the key presented in a registration // request, then we have a node that is either: // - Trying to log out (sending a expiry in the past) @@ -180,15 +195,6 @@ func (h *Headscale) RegistrationHandler(ctx *gin.Context) { return } - - // If the machine has AuthKey set, handle registration via PreAuthKeys - if req.Auth.AuthKey != "" { - h.handleAuthKey(ctx, machineKey, req, *machine) - - return - } - - h.handleMachineRegistrationNew(ctx, machineKey, req, *machine) } func (h *Headscale) getMapResponse( @@ -402,7 +408,7 @@ func (h *Headscale) handleMachineExpired( Msg("Machine registration has expired. Sending a authurl to register") if registerRequest.Auth.AuthKey != "" { - h.handleAuthKey(ctx, machineKey, registerRequest, machine) + h.handleAuthKey(ctx, machineKey, registerRequest) return } @@ -465,13 +471,12 @@ func (h *Headscale) handleMachineRegistrationNew( ctx *gin.Context, machineKey key.MachinePublic, registerRequest tailcfg.RegisterRequest, - machine Machine, ) { resp := tailcfg.RegisterResponse{} // The machine registration is new, redirect the client to the registration URL log.Debug(). - Str("machine", machine.Name). + Str("machine", registerRequest.Hostinfo.Hostname). Msg("The node is sending us a new NodeKey, sending auth url") if h.cfg.OIDC.Issuer != "" { resp.AuthURL = fmt.Sprintf( @@ -487,7 +492,7 @@ func (h *Headscale) handleMachineRegistrationNew( if !registerRequest.Expiry.IsZero() { log.Trace(). Caller(). - Str("machine", machine.Name). + Str("machine", registerRequest.Hostinfo.Hostname). Time("expiry", registerRequest.Expiry). Msg("Non-zero expiry time requested, adding to cache") h.requestedExpiryCache.Set( @@ -497,11 +502,6 @@ func (h *Headscale) handleMachineRegistrationNew( ) } - machine.NodeKey = NodePublicKeyStripPrefix(registerRequest.NodeKey) - - // save the NodeKey - h.db.Save(&machine) - respBody, err := encode(resp, &machineKey, h.privateKey) if err != nil { log.Error(). @@ -520,19 +520,21 @@ func (h *Headscale) handleAuthKey( ctx *gin.Context, machineKey key.MachinePublic, registerRequest tailcfg.RegisterRequest, - machine Machine, ) { + machineKeyStr := MachinePublicKeyStripPrefix(machineKey) + log.Debug(). Str("func", "handleAuthKey"). Str("machine", registerRequest.Hostinfo.Hostname). Msgf("Processing auth key for %s", registerRequest.Hostinfo.Hostname) resp := tailcfg.RegisterResponse{} + pak, err := h.checkKeyValidity(registerRequest.Auth.AuthKey) if err != nil { log.Error(). Caller(). Str("func", "handleAuthKey"). - Str("machine", machine.Name). + Str("machine", registerRequest.Hostinfo.Hostname). Err(err). Msg("Failed authentication via AuthKey") resp.MachineAuthorized = false @@ -541,69 +543,62 @@ func (h *Headscale) handleAuthKey( log.Error(). Caller(). Str("func", "handleAuthKey"). - Str("machine", machine.Name). + Str("machine", registerRequest.Hostinfo.Hostname). Err(err). Msg("Cannot encode message") ctx.String(http.StatusInternalServerError, "") - machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", machine.Namespace.Name). + machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", pak.Namespace.Name). Inc() return } + ctx.Data(http.StatusUnauthorized, "application/json; charset=utf-8", respBody) log.Error(). Caller(). Str("func", "handleAuthKey"). - Str("machine", machine.Name). + Str("machine", registerRequest.Hostinfo.Hostname). Msg("Failed authentication via AuthKey") - machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", machine.Namespace.Name). + machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", pak.Namespace.Name). Inc() return } - if machine.isRegistered() { - log.Trace(). + log.Debug(). + Str("func", "handleAuthKey"). + Str("machine", registerRequest.Hostinfo.Hostname). + Msg("Authentication key was valid, proceeding to acquire IP addresses") + + nodeKey := NodePublicKeyStripPrefix(registerRequest.NodeKey) + now := time.Now().UTC() + + machine, err := h.RegisterMachine( + registerRequest.Hostinfo.Hostname, + machineKeyStr, + pak.Namespace.Name, + RegisterMethodAuthKey, + ®isterRequest.Expiry, + pak, + &nodeKey, + &now, + ) + if err != nil { + log.Error(). Caller(). - Str("machine", machine.Name). - Msg("machine already registered, reauthenticating") - - h.RefreshMachine(&machine, registerRequest.Expiry) - } else { - log.Debug(). - Str("func", "handleAuthKey"). - Str("machine", machine.Name). - Msg("Authentication key was valid, proceeding to acquire IP addresses") - - nodeKey := NodePublicKeyStripPrefix(registerRequest.NodeKey) - now := time.Now().UTC() - - _, err = h.RegisterMachine( - machine.Name, - machine.Namespace.Name, - RegisterMethodAuthKey, - ®isterRequest.Expiry, - pak, - &nodeKey, - &now, + Err(err). + Msg("could not register machine") + machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", pak.Namespace.Name). + Inc() + ctx.String( + http.StatusInternalServerError, + "could not register machine", ) - if err != nil { - log.Error(). - Caller(). - Err(err). - Msg("could not register machine") - machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", machine.Namespace.Name).Inc() - ctx.String( - http.StatusInternalServerError, - "could not register machine", - ) - return - } + return } - pak.Used = true - h.db.Save(&pak) + h.UsePreAuthKey(pak) resp.MachineAuthorized = true resp.User = *pak.Namespace.toUser() @@ -612,21 +607,21 @@ func (h *Headscale) handleAuthKey( log.Error(). Caller(). Str("func", "handleAuthKey"). - Str("machine", machine.Name). + Str("machine", registerRequest.Hostinfo.Hostname). Err(err). Msg("Cannot encode message") - machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", machine.Namespace.Name). + machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "error", pak.Namespace.Name). Inc() ctx.String(http.StatusInternalServerError, "Extremely sad!") return } - machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "success", machine.Namespace.Name). + machineRegistrations.WithLabelValues("new", RegisterMethodAuthKey, "success", pak.Namespace.Name). Inc() ctx.Data(http.StatusOK, "application/json; charset=utf-8", respBody) log.Info(). Str("func", "handleAuthKey"). - Str("machine", machine.Name). + Str("machine", registerRequest.Hostinfo.Hostname). Str("ips", strings.Join(machine.IPAddresses.ToStringSlice(), ", ")). Msg("Successfully authenticated via AuthKey") } diff --git a/app.go b/app.go index eda670ed..63ca10e4 100644 --- a/app.go +++ b/app.go @@ -154,6 +154,8 @@ type Headscale struct { requestedExpiryCache *cache.Cache + registrationCache *cache.Cache + ipAllocationMutex sync.Mutex } @@ -207,6 +209,12 @@ func NewHeadscale(cfg Config) (*Headscale, error) { requestedExpiryCacheCleanupInterval, ) + registrationCache := cache.New( + // TODO(kradalby): Add unified cache expiry config options + requestedExpiryCacheExpiration, + requestedExpiryCacheCleanupInterval, + ) + app := Headscale{ cfg: cfg, dbType: cfg.DBtype, @@ -214,6 +222,7 @@ func NewHeadscale(cfg Config) (*Headscale, error) { privateKey: privKey, aclRules: tailcfg.FilterAllowAll, // default allowall requestedExpiryCache: requestedExpiryCache, + registrationCache: registrationCache, } err = app.initDB() diff --git a/cli_test.go b/cli_test.go index 2eedb5b4..fab8201f 100644 --- a/cli_test.go +++ b/cli_test.go @@ -30,6 +30,7 @@ func (s *Suite) TestRegisterMachine(c *check.C) { c.Assert(err, check.IsNil) machineAfterRegistering, err := app.RegisterMachine( + "testmachine", machine.MachineKey, namespace.Name, RegisterMethodCLI, diff --git a/grpcv1.go b/grpcv1.go index b1396bc8..3a2e19ce 100644 --- a/grpcv1.go +++ b/grpcv1.go @@ -174,12 +174,11 @@ func (api headscaleV1APIServer) RegisterMachine( } } - machine, err := api.h.RegisterMachine( + machine, err := api.h.RegisterMachineFromAuthCallback( request.GetKey(), request.GetNamespace(), RegisterMethodCLI, &requestedTime, - nil, nil, nil, ) if err != nil { return nil, err diff --git a/machine.go b/machine.go index b805de67..74251553 100644 --- a/machine.go +++ b/machine.go @@ -20,11 +20,15 @@ import ( ) const ( - errMachineNotFound = Error("machine not found") - errMachineAlreadyRegistered = Error("machine already registered") - errMachineRouteIsNotAvailable = Error("route is not available on machine") - errMachineAddressesInvalid = Error("failed to parse machine addresses") - errHostnameTooLong = Error("Hostname too long") + errMachineNotFound = Error("machine not found") + errMachineAlreadyRegistered = Error("machine already registered") + errMachineRouteIsNotAvailable = Error("route is not available on machine") + errMachineAddressesInvalid = Error("failed to parse machine addresses") + errMachineNotFoundRegistrationCache = Error( + "machine not found in registration cache", + ) + errCouldNotConvertMachineInterface = Error("failed to convert machine interface") + errHostnameTooLong = Error("Hostname too long") ) const ( @@ -686,14 +690,44 @@ func (machine *Machine) toProto() *v1.Machine { return machineProto } -// RegisterMachine is executed from the CLI to register a new Machine using its MachineKey. -func (h *Headscale) RegisterMachine( +func (h *Headscale) RegisterMachineFromAuthCallback( machineKeyStr string, namespaceName string, registrationMethod string, + expiry *time.Time, +) (*Machine, error) { + if machineInterface, ok := h.registrationCache.Get(machineKeyStr); ok { + if registrationMachine, ok := machineInterface.(Machine); ok { + machine, err := h.RegisterMachine( + registrationMachine.Name, + machineKeyStr, + namespaceName, + registrationMethod, + expiry, + nil, + ®istrationMachine.NodeKey, + registrationMachine.LastSeen, + ) + + return machine, err + + } else { + return nil, errCouldNotConvertMachineInterface + } + } + + return nil, errMachineNotFoundRegistrationCache +} + +// RegisterMachine is executed from the CLI to register a new Machine using its MachineKey. +func (h *Headscale) RegisterMachine( + machineName string, + machineKeyStr string, + namespaceName string, + registrationMethod string, + expiry *time.Time, // Optionals - expiry *time.Time, authKey *PreAuthKey, nodePublicKey *string, lastSeen *time.Time, @@ -768,6 +802,7 @@ func (h *Headscale) RegisterMachine( machine.LastSeen = lastSeen } + machine.Name = machineName machine.NamespaceID = namespace.ID // TODO(kradalby): This field is uneccessary metadata, @@ -780,6 +815,7 @@ func (h *Headscale) RegisterMachine( // Let us simplify the model, a machine is _only_ saved if // it is registered. machine.Registered = true + h.db.Save(&machine) log.Trace(). diff --git a/oidc.go b/oidc.go index 5207c64e..48664aed 100644 --- a/oidc.go +++ b/oidc.go @@ -279,8 +279,6 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { return } - now := time.Now().UTC() - namespaceName, err := NormalizeNamespaceName( claims.Email, h.cfg.OIDC.StripEmaildomain, @@ -328,14 +326,11 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { return } - _, err = h.RegisterMachine( + _, err = h.RegisterMachineFromAuthCallback( machineKeyStr, namespace.Name, RegisterMethodOIDC, &requestedTime, - nil, - nil, - &now, ) if err != nil { log.Error(). diff --git a/preauth_keys.go b/preauth_keys.go index 50bc4746..55f62226 100644 --- a/preauth_keys.go +++ b/preauth_keys.go @@ -113,6 +113,12 @@ func (h *Headscale) ExpirePreAuthKey(k *PreAuthKey) error { return nil } +// UsePreAuthKey marks a PreAuthKey as used. +func (h *Headscale) UsePreAuthKey(k *PreAuthKey) { + k.Used = true + h.db.Save(k) +} + // checkKeyValidity does the heavy lifting for validation of the PreAuthKey coming from a node // If returns no error and a PreAuthKey, it can be used. func (h *Headscale) checkKeyValidity(k string) (*PreAuthKey, error) { From 402a76070f501bf490dd75391b72ef1743db66e8 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 16:34:28 +0000 Subject: [PATCH 08/32] Reuse machine structure for parameters, named parameters --- machine.go | 89 +++++++++++++++++------------------------------------- 1 file changed, 27 insertions(+), 62 deletions(-) diff --git a/machine.go b/machine.go index 74251553..6e5b62d9 100644 --- a/machine.go +++ b/machine.go @@ -21,7 +21,6 @@ import ( const ( errMachineNotFound = Error("machine not found") - errMachineAlreadyRegistered = Error("machine already registered") errMachineRouteIsNotAvailable = Error("route is not available on machine") errMachineAddressesInvalid = Error("failed to parse machine addresses") errMachineNotFoundRegistrationCache = Error( @@ -698,19 +697,23 @@ func (h *Headscale) RegisterMachineFromAuthCallback( ) (*Machine, error) { if machineInterface, ok := h.registrationCache.Get(machineKeyStr); ok { if registrationMachine, ok := machineInterface.(Machine); ok { + namespace, err := h.GetNamespace(namespaceName) + if err != nil { + return nil, fmt.Errorf( + "failed to find namespace in register machine from auth callback, %w", + err, + ) + } + + registrationMachine.NamespaceID = namespace.ID + registrationMachine.RegisterMethod = registrationMethod + registrationMachine.Expiry = expiry + machine, err := h.RegisterMachine( - registrationMachine.Name, - machineKeyStr, - namespaceName, - registrationMethod, - expiry, - nil, - ®istrationMachine.NodeKey, - registrationMachine.LastSeen, + registrationMachine, ) return machine, err - } else { return nil, errCouldNotConvertMachineInterface } @@ -720,49 +723,30 @@ func (h *Headscale) RegisterMachineFromAuthCallback( } // RegisterMachine is executed from the CLI to register a new Machine using its MachineKey. -func (h *Headscale) RegisterMachine( - machineName string, - machineKeyStr string, - namespaceName string, - registrationMethod string, - expiry *time.Time, - - // Optionals - authKey *PreAuthKey, - nodePublicKey *string, - lastSeen *time.Time, +func (h *Headscale) RegisterMachine(machine Machine, ) (*Machine, error) { - namespace, err := h.GetNamespace(namespaceName) - if err != nil { - return nil, err - } - - var machineKey key.MachinePublic - err = machineKey.UnmarshalText([]byte(MachinePublicKeyEnsurePrefix(machineKeyStr))) - if err != nil { - return nil, err - } - log.Trace(). Caller(). - Str("machine_key_str", machineKeyStr). - Str("machine_key", machineKey.String()). + Str("machine_key", machine.MachineKey). Msg("Registering machine") - machine, err := h.GetMachineByMachineKey(machineKey) - if err != nil { - return nil, err - } - + // If the machine is already in the database, it is seeking + // reauthentication, and by reaching this step, has been authenticated + // and need to have an updated expiry. + var machineKey key.MachinePublic + _ = machineKey.UnmarshalText( + []byte(MachinePublicKeyEnsurePrefix(machine.MachineKey)), + ) + machineFromDatabase, _ := h.GetMachineByMachineKey(machineKey) if machine.isRegistered() { log.Trace(). Caller(). Str("machine", machine.Name). Msg("machine already registered, reauthenticating") - h.RefreshMachine(machine, *expiry) + h.RefreshMachine(machineFromDatabase, *machine.Expiry) - return machine, nil + return machineFromDatabase, nil } log.Trace(). @@ -786,28 +770,9 @@ func (h *Headscale) RegisterMachine( machine.IPAddresses = ips - if expiry != nil { - machine.Expiry = expiry - } - - if authKey != nil { - machine.AuthKeyID = uint(authKey.ID) - } - - if nodePublicKey != nil { - machine.NodeKey = *nodePublicKey - } - - if lastSeen != nil { - machine.LastSeen = lastSeen - } - - machine.Name = machineName - machine.NamespaceID = namespace.ID - // TODO(kradalby): This field is uneccessary metadata, // move it to tags instead of having a column. - machine.RegisterMethod = registrationMethod + // machine.RegisterMethod = registrationMethod // TODO(kradalby): Registered is a very frustrating value // to keep up to date, and it makes is have to care if a @@ -824,7 +789,7 @@ func (h *Headscale) RegisterMachine( Str("ip", strings.Join(ips.ToStringSlice(), ",")). Msg("Machine registered with the database") - return machine, nil + return &machine, nil } func (machine *Machine) GetAdvertisedRoutes() ([]netaddr.IPPrefix, error) { From 54cc3c067ffabf2cae5339a615598ab82feb02fd Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 16:34:50 +0000 Subject: [PATCH 09/32] Implement new machine register parameter --- api.go | 21 +++++++++++++-------- grpcv1.go | 10 +++++----- oidc.go | 1 - 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/api.go b/api.go index 60ca63f2..f831cacf 100644 --- a/api.go +++ b/api.go @@ -154,6 +154,7 @@ func (h *Headscale) RegistrationHandler(ctx *gin.Context) { ) h.handleMachineRegistrationNew(ctx, machineKey, req) + return } @@ -573,15 +574,19 @@ func (h *Headscale) handleAuthKey( nodeKey := NodePublicKeyStripPrefix(registerRequest.NodeKey) now := time.Now().UTC() + machineToRegister := Machine{ + Name: registerRequest.Hostinfo.Hostname, + NamespaceID: pak.Namespace.ID, + MachineKey: machineKeyStr, + RegisterMethod: RegisterMethodAuthKey, + Expiry: ®isterRequest.Expiry, + NodeKey: nodeKey, + LastSeen: &now, + AuthKeyID: uint(pak.ID), + } + machine, err := h.RegisterMachine( - registerRequest.Hostinfo.Hostname, - machineKeyStr, - pak.Namespace.Name, - RegisterMethodAuthKey, - ®isterRequest.Expiry, - pak, - &nodeKey, - &now, + machineToRegister, ) if err != nil { log.Error(). diff --git a/grpcv1.go b/grpcv1.go index 3a2e19ce..75732b1d 100644 --- a/grpcv1.go +++ b/grpcv1.go @@ -415,11 +415,11 @@ func (api headscaleV1APIServer) DebugCreateMachine( HostInfo: datatypes.JSON(hostinfoJson), } - // log.Trace().Caller().Interface("machine", newMachine).Msg("") - - if err := api.h.db.Create(&newMachine).Error; err != nil { - return nil, err - } + api.h.registrationCache.Set( + request.GetKey(), + newMachine, + requestedExpiryCacheExpiration, + ) return &v1.DebugCreateMachineResponse{Machine: newMachine.toProto()}, nil } diff --git a/oidc.go b/oidc.go index 48664aed..952e2600 100644 --- a/oidc.go +++ b/oidc.go @@ -344,7 +344,6 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { return } - } var content bytes.Buffer From 50053e616a42109b22a1aa15cba56461c04035eb Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 16:35:08 +0000 Subject: [PATCH 10/32] Ignore complexity linter --- .golangci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yaml b/.golangci.yaml index 153cd7c3..56fdc28e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -29,6 +29,7 @@ linters: - wrapcheck - dupl - makezero + - maintidx # We might want to enable this, but it might be a lot of work - cyclop From c6b87de95901193b7a72494832fdfc94456dceae Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 16:36:16 +0000 Subject: [PATCH 11/32] Remove poorly aged test --- cli_test.go | 44 -------------------------------------------- 1 file changed, 44 deletions(-) delete mode 100644 cli_test.go diff --git a/cli_test.go b/cli_test.go deleted file mode 100644 index fab8201f..00000000 --- a/cli_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package headscale - -import ( - "time" - - "gopkg.in/check.v1" - "inet.af/netaddr" -) - -func (s *Suite) TestRegisterMachine(c *check.C) { - namespace, err := app.CreateNamespace("test") - c.Assert(err, check.IsNil) - - now := time.Now().UTC() - - machine := Machine{ - ID: 0, - MachineKey: "8ce002a935f8c394e55e78fbbb410576575ff8ec5cfa2e627e4b807f1be15b0e", - NodeKey: "bar", - DiscoKey: "faa", - Name: "testmachine", - NamespaceID: namespace.ID, - IPAddresses: []netaddr.IP{netaddr.MustParseIP("10.0.0.1")}, - Expiry: &now, - } - err = app.db.Save(&machine).Error - c.Assert(err, check.IsNil) - - _, err = app.GetMachine(namespace.Name, machine.Name) - c.Assert(err, check.IsNil) - - machineAfterRegistering, err := app.RegisterMachine( - "testmachine", - machine.MachineKey, - namespace.Name, - RegisterMethodCLI, - nil, nil, nil, nil, - ) - c.Assert(err, check.IsNil) - c.Assert(machineAfterRegistering.Registered, check.Equals, true) - - _, err = machineAfterRegistering.GetHostInfo() - c.Assert(err, check.IsNil) -} From e7bef567184622a7e5374023ae5238e2fccd00ed Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 16:36:29 +0000 Subject: [PATCH 12/32] Remove reference to registered in integration test --- integration_cli_test.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/integration_cli_test.go b/integration_cli_test.go index aae80cb0..7ce0758f 100644 --- a/integration_cli_test.go +++ b/integration_cli_test.go @@ -621,12 +621,6 @@ func (s *IntegrationCLITestSuite) TestNodeCommand() { assert.Equal(s.T(), "machine-4", listAll[3].Name) assert.Equal(s.T(), "machine-5", listAll[4].Name) - assert.True(s.T(), listAll[0].Registered) - assert.True(s.T(), listAll[1].Registered) - assert.True(s.T(), listAll[2].Registered) - assert.True(s.T(), listAll[3].Registered) - assert.True(s.T(), listAll[4].Registered) - otherNamespaceMachineKeys := []string{ "b5b444774186d4217adcec407563a1223929465ee2c68a4da13af0d0185b4f8e", "dc721977ac7415aafa87f7d4574cbe07c6b171834a6d37375782bdc1fb6b3584", @@ -710,9 +704,6 @@ func (s *IntegrationCLITestSuite) TestNodeCommand() { assert.Equal(s.T(), "otherNamespace-machine-1", listAllWithotherNamespace[5].Name) assert.Equal(s.T(), "otherNamespace-machine-2", listAllWithotherNamespace[6].Name) - assert.True(s.T(), listAllWithotherNamespace[5].Registered) - assert.True(s.T(), listAllWithotherNamespace[6].Registered) - // Test list all nodes after added otherNamespace listOnlyotherNamespaceMachineNamespaceResult, err := ExecuteCommand( &s.headscale, @@ -752,9 +743,6 @@ func (s *IntegrationCLITestSuite) TestNodeCommand() { listOnlyotherNamespaceMachineNamespace[1].Name, ) - assert.True(s.T(), listOnlyotherNamespaceMachineNamespace[0].Registered) - assert.True(s.T(), listOnlyotherNamespaceMachineNamespace[1].Registered) - // Delete a machines _, err = ExecuteCommand( &s.headscale, @@ -979,7 +967,6 @@ func (s *IntegrationCLITestSuite) TestRouteCommand() { assert.Equal(s.T(), uint64(1), machine.Id) assert.Equal(s.T(), "route-machine", machine.Name) - assert.True(s.T(), machine.Registered) listAllResult, err := ExecuteCommand( &s.headscale, From 35616eb8611ab7514e45ab3a7eb01a5abe0c52f3 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 16:41:28 +0000 Subject: [PATCH 13/32] Fix oidc error were namespace isnt created #365 --- oidc.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/oidc.go b/oidc.go index 952e2600..f291e8c9 100644 --- a/oidc.go +++ b/oidc.go @@ -17,7 +17,6 @@ import ( "github.com/patrickmn/go-cache" "github.com/rs/zerolog/log" "golang.org/x/oauth2" - "gorm.io/gorm" "tailscale.com/types/key" ) @@ -297,7 +296,7 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { log.Debug().Msg("Registering new machine after successful callback") namespace, err := h.GetNamespace(namespaceName) - if errors.Is(err, gorm.ErrRecordNotFound) { + if errors.Is(err, errNamespaceNotFound) { namespace, err = h.CreateNamespace(namespaceName) if err != nil { From 16b21e8158d7a32d1ed1b093f05268a633a5bd17 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 16:55:57 +0000 Subject: [PATCH 14/32] Remove all references to Machine.Registered --- machine.go | 24 +++------------- oidc.go | 81 +++++++++++++++++++++++++++--------------------------- 2 files changed, 44 insertions(+), 61 deletions(-) diff --git a/machine.go b/machine.go index 6e5b62d9..16d6d86f 100644 --- a/machine.go +++ b/machine.go @@ -72,11 +72,6 @@ type ( MachinesP []*Machine ) -// For the time being this method is rather naive. -func (machine Machine) isRegistered() bool { - return machine.Registered -} - type MachineAddresses []netaddr.IP func (ma MachineAddresses) ToStringSlice() []string { @@ -221,7 +216,7 @@ func (h *Headscale) ListPeers(machine *Machine) (Machines, error) { Msg("Finding direct peers") machines := Machines{} - if err := h.db.Preload("AuthKey").Preload("AuthKey.Namespace").Preload("Namespace").Where("machine_key <> ? AND registered", + if err := h.db.Preload("AuthKey").Preload("AuthKey.Namespace").Preload("Namespace").Where("machine_key <> ?", machine.MachineKey).Find(&machines).Error; err != nil { log.Error().Err(err).Msg("Error accessing db") @@ -284,7 +279,7 @@ func (h *Headscale) getValidPeers(machine *Machine) (Machines, error) { } for _, peer := range peers { - if peer.isRegistered() && !peer.isExpired() { + if !peer.isExpired() { validPeers = append(validPeers, peer) } } @@ -373,8 +368,6 @@ func (h *Headscale) RefreshMachine(machine *Machine, expiry time.Time) { // DeleteMachine softs deletes a Machine from the database. func (h *Headscale) DeleteMachine(machine *Machine) error { - machine.Registered = false - h.db.Save(&machine) // we mark it as unregistered, just in case if err := h.db.Delete(&machine).Error; err != nil { return err } @@ -642,7 +635,7 @@ func (machine Machine) toNode( LastSeen: machine.LastSeen, KeepAlive: true, - MachineAuthorized: machine.Registered, + MachineAuthorized: !machine.isExpired(), Capabilities: []string{tailcfg.CapabilityFileSharing}, } @@ -660,8 +653,6 @@ func (machine *Machine) toProto() *v1.Machine { Name: machine.Name, Namespace: machine.Namespace.toProto(), - Registered: machine.Registered, - // TODO(kradalby): Implement register method enum converter // RegisterMethod: , @@ -738,7 +729,7 @@ func (h *Headscale) RegisterMachine(machine Machine, []byte(MachinePublicKeyEnsurePrefix(machine.MachineKey)), ) machineFromDatabase, _ := h.GetMachineByMachineKey(machineKey) - if machine.isRegistered() { + if machineFromDatabase != nil { log.Trace(). Caller(). Str("machine", machine.Name). @@ -774,13 +765,6 @@ func (h *Headscale) RegisterMachine(machine Machine, // move it to tags instead of having a column. // machine.RegisterMethod = registrationMethod - // TODO(kradalby): Registered is a very frustrating value - // to keep up to date, and it makes is have to care if a - // machine is registered, authenticated and expired. - // Let us simplify the model, a machine is _only_ saved if - // it is registered. - machine.Registered = true - h.db.Save(&machine) log.Trace(). diff --git a/oidc.go b/oidc.go index f291e8c9..e745236b 100644 --- a/oidc.go +++ b/oidc.go @@ -248,7 +248,7 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { return } - if machine.isRegistered() { + if machine != nil { log.Trace(). Caller(). Str("machine", machine.Name). @@ -291,58 +291,57 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { return } + // register the machine if it's new - if !machine.Registered { - log.Debug().Msg("Registering new machine after successful callback") + log.Debug().Msg("Registering new machine after successful callback") - namespace, err := h.GetNamespace(namespaceName) - if errors.Is(err, errNamespaceNotFound) { - namespace, err = h.CreateNamespace(namespaceName) + namespace, err := h.GetNamespace(namespaceName) + if errors.Is(err, errNamespaceNotFound) { + namespace, err = h.CreateNamespace(namespaceName) - if err != nil { - log.Error(). - Err(err). - Caller(). - Msgf("could not create new namespace '%s'", namespaceName) - ctx.String( - http.StatusInternalServerError, - "could not create new namespace", - ) - - return - } - } else if err != nil { - log.Error(). - Caller(). - Err(err). - Str("namespace", namespaceName). - Msg("could not find or create namespace") - ctx.String( - http.StatusInternalServerError, - "could not find or create namespace", - ) - - return - } - - _, err = h.RegisterMachineFromAuthCallback( - machineKeyStr, - namespace.Name, - RegisterMethodOIDC, - &requestedTime, - ) if err != nil { log.Error(). - Caller(). Err(err). - Msg("could not register machine") + Caller(). + Msgf("could not create new namespace '%s'", namespaceName) ctx.String( http.StatusInternalServerError, - "could not register machine", + "could not create new namespace", ) return } + } else if err != nil { + log.Error(). + Caller(). + Err(err). + Str("namespace", namespaceName). + Msg("could not find or create namespace") + ctx.String( + http.StatusInternalServerError, + "could not find or create namespace", + ) + + return + } + + _, err = h.RegisterMachineFromAuthCallback( + machineKeyStr, + namespace.Name, + RegisterMethodOIDC, + &requestedTime, + ) + if err != nil { + log.Error(). + Caller(). + Err(err). + Msg("could not register machine") + ctx.String( + http.StatusInternalServerError, + "could not register machine", + ) + + return } var content bytes.Buffer From a8649d83c4b655eae469d556eacafe8c81985a40 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 17:42:03 +0000 Subject: [PATCH 15/32] Remove all references to Machine.Registered from tests --- acls_test.go | 7 ------- dns_test.go | 8 -------- machine_test.go | 7 ------- namespaces_test.go | 5 ----- preauth_keys_test.go | 3 --- routes_test.go | 2 -- utils_test.go | 3 --- 7 files changed, 35 deletions(-) diff --git a/acls_test.go b/acls_test.go index 5534257d..edea854a 100644 --- a/acls_test.go +++ b/acls_test.go @@ -119,7 +119,6 @@ func (s *Suite) TestValidExpandTagOwnersInUsers(c *check.C) { Name: "testmachine", IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), HostInfo: datatypes.JSON(hostInfo), @@ -163,7 +162,6 @@ func (s *Suite) TestValidExpandTagOwnersInPorts(c *check.C) { Name: "testmachine", IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), HostInfo: datatypes.JSON(hostInfo), @@ -207,7 +205,6 @@ func (s *Suite) TestInvalidTagValidNamespace(c *check.C) { Name: "testmachine", IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), HostInfo: datatypes.JSON(hostInfo), @@ -250,7 +247,6 @@ func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { Name: "webserver", IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.1")}, NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), HostInfo: datatypes.JSON(hostInfo), @@ -267,7 +263,6 @@ func (s *Suite) TestValidTagInvalidNamespace(c *check.C) { Name: "user", IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), HostInfo: datatypes.JSON(hostInfo), @@ -345,7 +340,6 @@ func (s *Suite) TestPortNamespace(c *check.C) { DiscoKey: "faa", Name: "testmachine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: ips, AuthKeyID: uint(pak.ID), @@ -388,7 +382,6 @@ func (s *Suite) TestPortGroup(c *check.C) { DiscoKey: "faa", Name: "testmachine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: ips, AuthKeyID: uint(pak.ID), diff --git a/dns_test.go b/dns_test.go index 23a34ca7..5fd30d3b 100644 --- a/dns_test.go +++ b/dns_test.go @@ -164,7 +164,6 @@ func (s *Suite) TestDNSConfigMapResponseWithMagicDNS(c *check.C) { Name: "test_get_shared_nodes_1", NamespaceID: namespaceShared1.ID, Namespace: *namespaceShared1, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.1")}, AuthKeyID: uint(preAuthKeyInShared1.ID), @@ -182,7 +181,6 @@ func (s *Suite) TestDNSConfigMapResponseWithMagicDNS(c *check.C) { Name: "test_get_shared_nodes_2", NamespaceID: namespaceShared2.ID, Namespace: *namespaceShared2, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.2")}, AuthKeyID: uint(preAuthKeyInShared2.ID), @@ -200,7 +198,6 @@ func (s *Suite) TestDNSConfigMapResponseWithMagicDNS(c *check.C) { Name: "test_get_shared_nodes_3", NamespaceID: namespaceShared3.ID, Namespace: *namespaceShared3, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.3")}, AuthKeyID: uint(preAuthKeyInShared3.ID), @@ -218,7 +215,6 @@ func (s *Suite) TestDNSConfigMapResponseWithMagicDNS(c *check.C) { Name: "test_get_shared_nodes_4", NamespaceID: namespaceShared1.ID, Namespace: *namespaceShared1, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.4")}, AuthKeyID: uint(PreAuthKey2InShared1.ID), @@ -311,7 +307,6 @@ func (s *Suite) TestDNSConfigMapResponseWithoutMagicDNS(c *check.C) { Name: "test_get_shared_nodes_1", NamespaceID: namespaceShared1.ID, Namespace: *namespaceShared1, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.1")}, AuthKeyID: uint(preAuthKeyInShared1.ID), @@ -329,7 +324,6 @@ func (s *Suite) TestDNSConfigMapResponseWithoutMagicDNS(c *check.C) { Name: "test_get_shared_nodes_2", NamespaceID: namespaceShared2.ID, Namespace: *namespaceShared2, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.2")}, AuthKeyID: uint(preAuthKeyInShared2.ID), @@ -347,7 +341,6 @@ func (s *Suite) TestDNSConfigMapResponseWithoutMagicDNS(c *check.C) { Name: "test_get_shared_nodes_3", NamespaceID: namespaceShared3.ID, Namespace: *namespaceShared3, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.3")}, AuthKeyID: uint(preAuthKeyInShared3.ID), @@ -365,7 +358,6 @@ func (s *Suite) TestDNSConfigMapResponseWithoutMagicDNS(c *check.C) { Name: "test_get_shared_nodes_4", NamespaceID: namespaceShared1.ID, Namespace: *namespaceShared1, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.4")}, AuthKeyID: uint(preAuthKey2InShared1.ID), diff --git a/machine_test.go b/machine_test.go index e9c91f8b..733270e4 100644 --- a/machine_test.go +++ b/machine_test.go @@ -29,7 +29,6 @@ func (s *Suite) TestGetMachine(c *check.C) { DiscoKey: "faa", Name: "testmachine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), } @@ -59,7 +58,6 @@ func (s *Suite) TestGetMachineByID(c *check.C) { DiscoKey: "faa", Name: "testmachine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), } @@ -82,7 +80,6 @@ func (s *Suite) TestDeleteMachine(c *check.C) { DiscoKey: "faa", Name: "testmachine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(1), } @@ -105,7 +102,6 @@ func (s *Suite) TestHardDeleteMachine(c *check.C) { DiscoKey: "faa", Name: "testmachine3", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(1), } @@ -136,7 +132,6 @@ func (s *Suite) TestListPeers(c *check.C) { DiscoKey: "faa" + strconv.Itoa(index), Name: "testmachine" + strconv.Itoa(index), NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), } @@ -188,7 +183,6 @@ func (s *Suite) TestGetACLFilteredPeers(c *check.C) { }, Name: "testmachine" + strconv.Itoa(index), NamespaceID: stor[index%2].namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(stor[index%2].key.ID), } @@ -258,7 +252,6 @@ func (s *Suite) TestExpireMachine(c *check.C) { DiscoKey: "faa", Name: "testmachine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), Expiry: &time.Time{}, diff --git a/namespaces_test.go b/namespaces_test.go index 585ba938..1710f7d6 100644 --- a/namespaces_test.go +++ b/namespaces_test.go @@ -54,7 +54,6 @@ func (s *Suite) TestDestroyNamespaceErrors(c *check.C) { DiscoKey: "faa", Name: "testmachine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), } @@ -146,7 +145,6 @@ func (s *Suite) TestGetMapResponseUserProfiles(c *check.C) { Name: "test_get_shared_nodes_1", NamespaceID: namespaceShared1.ID, Namespace: *namespaceShared1, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.1")}, AuthKeyID: uint(preAuthKeyShared1.ID), @@ -164,7 +162,6 @@ func (s *Suite) TestGetMapResponseUserProfiles(c *check.C) { Name: "test_get_shared_nodes_2", NamespaceID: namespaceShared2.ID, Namespace: *namespaceShared2, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.2")}, AuthKeyID: uint(preAuthKeyShared2.ID), @@ -182,7 +179,6 @@ func (s *Suite) TestGetMapResponseUserProfiles(c *check.C) { Name: "test_get_shared_nodes_3", NamespaceID: namespaceShared3.ID, Namespace: *namespaceShared3, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.3")}, AuthKeyID: uint(preAuthKeyShared3.ID), @@ -200,7 +196,6 @@ func (s *Suite) TestGetMapResponseUserProfiles(c *check.C) { Name: "test_get_shared_nodes_4", NamespaceID: namespaceShared1.ID, Namespace: *namespaceShared1, - Registered: true, RegisterMethod: RegisterMethodAuthKey, IPAddresses: []netaddr.IP{netaddr.MustParseIP("100.64.0.4")}, AuthKeyID: uint(preAuthKey2Shared1.ID), diff --git a/preauth_keys_test.go b/preauth_keys_test.go index f8cf276d..6f233a94 100644 --- a/preauth_keys_test.go +++ b/preauth_keys_test.go @@ -80,7 +80,6 @@ func (*Suite) TestAlreadyUsedKey(c *check.C) { DiscoKey: "faa", Name: "testest", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), } @@ -105,7 +104,6 @@ func (*Suite) TestReusableBeingUsedKey(c *check.C) { DiscoKey: "faa", Name: "testest", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), } @@ -143,7 +141,6 @@ func (*Suite) TestEphemeralKey(c *check.C) { DiscoKey: "faa", Name: "testest", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, LastSeen: &now, AuthKeyID: uint(pak.ID), diff --git a/routes_test.go b/routes_test.go index 94bda45b..1c24bf26 100644 --- a/routes_test.go +++ b/routes_test.go @@ -35,7 +35,6 @@ func (s *Suite) TestGetRoutes(c *check.C) { DiscoKey: "faa", Name: "test_get_route_machine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), HostInfo: datatypes.JSON(hostinfo), @@ -89,7 +88,6 @@ func (s *Suite) TestGetEnableRoutes(c *check.C) { DiscoKey: "faa", Name: "test_enable_route_machine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), HostInfo: datatypes.JSON(hostinfo), diff --git a/utils_test.go b/utils_test.go index 271013a1..7c72c09a 100644 --- a/utils_test.go +++ b/utils_test.go @@ -36,7 +36,6 @@ func (s *Suite) TestGetUsedIps(c *check.C) { DiscoKey: "faa", Name: "testmachine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), IPAddresses: ips, @@ -85,7 +84,6 @@ func (s *Suite) TestGetMultiIp(c *check.C) { DiscoKey: "faa", Name: "testmachine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), IPAddresses: ips, @@ -176,7 +174,6 @@ func (s *Suite) TestGetAvailableIpMachineWithoutIP(c *check.C) { DiscoKey: "faa", Name: "testmachine", NamespaceID: namespace.ID, - Registered: true, RegisterMethod: RegisterMethodAuthKey, AuthKeyID: uint(pak.ID), } From 78251ce8ec5e37295e2498dabb87170c3f5e1cce Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 18:05:03 +0000 Subject: [PATCH 16/32] Remove registrated field This commit removes the field from the database and does a DB migration **removing** all unregistered machines from headscale. This means that from this version, all machines in the database is considered registered. --- db.go | 33 +++++++++++++++++++++++++++++++++ machine.go | 1 - 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/db.go b/db.go index 8db7bc69..e87cb6d3 100644 --- a/db.go +++ b/db.go @@ -5,6 +5,7 @@ import ( "time" "github.com/glebarez/sqlite" + "github.com/rs/zerolog/log" "gorm.io/driver/postgres" "gorm.io/gorm" "gorm.io/gorm/logger" @@ -34,6 +35,38 @@ func (h *Headscale) initDB() error { _ = db.Migrator().RenameColumn(&Machine{}, "ip_address", "ip_addresses") + // If the Machine table has a column for registered, + // find all occourences of "false" and drop them. Then + // remove the column. + if db.Migrator().HasColumn(&Machine{}, "registered") { + log.Info(). + Msg(`Database has legacy "registered" column in machine, removing...`) + + machines := Machines{} + if err := h.db.Not("registered").Find(&machines).Error; err != nil { + log.Error().Err(err).Msg("Error accessing db") + } + + for _, machine := range machines { + log.Info(). + Str("machine", machine.Name). + Str("machine_key", machine.MachineKey). + Msg("Deleting unregistered machine") + if err := h.db.Delete(&Machine{}, machine.ID).Error; err != nil { + log.Error(). + Err(err). + Str("machine", machine.Name). + Str("machine_key", machine.MachineKey). + Msg("Error deleting unregistered machine") + } + } + + err := db.Migrator().DropColumn(&Machine{}, "registered") + if err != nil { + log.Error().Err(err).Msg("Error dropping registered column") + } + } + err = db.AutoMigrate(&Machine{}) if err != nil { return err diff --git a/machine.go b/machine.go index 16d6d86f..ac1a1c84 100644 --- a/machine.go +++ b/machine.go @@ -45,7 +45,6 @@ type Machine struct { NamespaceID uint Namespace Namespace `gorm:"foreignKey:NamespaceID"` - Registered bool // temp RegisterMethod string // TODO(kradalby): This seems like irrelevant information? From eea8e7ba6f2d69d41a065959d566e69013480e67 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 22:11:31 +0000 Subject: [PATCH 17/32] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d44db566..20c23c99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ **Changes**: - Fix a bug were the same IP could be assigned to multiple hosts if joined in quick succession [#346](https://github.com/juanfont/headscale/pull/346) +- Simplify the code behind registration of machines [#366](https://github.com/juanfont/headscale/pull/366) + - Nodes are now only written to database if they are registrated successfully **0.14.0 (2022-02-24):** From 5e1b12948e305dca78eeccdce826784092174cd0 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 22:21:06 +0000 Subject: [PATCH 18/32] Remove registered field from proto --- proto/headscale/v1/machine.proto | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/proto/headscale/v1/machine.proto b/proto/headscale/v1/machine.proto index 7451db83..75552d78 100644 --- a/proto/headscale/v1/machine.proto +++ b/proto/headscale/v1/machine.proto @@ -22,16 +22,16 @@ message Machine { string name = 6; Namespace namespace = 7; - bool registered = 8; - RegisterMethod register_method = 9; - google.protobuf.Timestamp last_seen = 10; - google.protobuf.Timestamp last_successful_update = 11; - google.protobuf.Timestamp expiry = 12; + google.protobuf.Timestamp last_seen = 8; + google.protobuf.Timestamp last_successful_update = 9; + google.protobuf.Timestamp expiry = 10; - PreAuthKey pre_auth_key = 13; + PreAuthKey pre_auth_key = 11; - google.protobuf.Timestamp created_at = 14; + google.protobuf.Timestamp created_at = 12; + + RegisterMethod register_method = 13; // google.protobuf.Timestamp updated_at = 14; // google.protobuf.Timestamp deleted_at = 15; From e64bee778f4e925553c4045b6087ef933bff3779 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 22:21:14 +0000 Subject: [PATCH 19/32] Regenerate proto --- gen/go/headscale/v1/machine.pb.go | 100 ++++++++---------- .../headscale/v1/headscale.swagger.json | 9 +- 2 files changed, 48 insertions(+), 61 deletions(-) diff --git a/gen/go/headscale/v1/machine.pb.go b/gen/go/headscale/v1/machine.pb.go index 9a5064fe..b25db290 100644 --- a/gen/go/headscale/v1/machine.pb.go +++ b/gen/go/headscale/v1/machine.pb.go @@ -85,13 +85,12 @@ type Machine struct { IpAddresses []string `protobuf:"bytes,5,rep,name=ip_addresses,json=ipAddresses,proto3" json:"ip_addresses,omitempty"` Name string `protobuf:"bytes,6,opt,name=name,proto3" json:"name,omitempty"` Namespace *Namespace `protobuf:"bytes,7,opt,name=namespace,proto3" json:"namespace,omitempty"` - Registered bool `protobuf:"varint,8,opt,name=registered,proto3" json:"registered,omitempty"` - RegisterMethod RegisterMethod `protobuf:"varint,9,opt,name=register_method,json=registerMethod,proto3,enum=headscale.v1.RegisterMethod" json:"register_method,omitempty"` - LastSeen *timestamppb.Timestamp `protobuf:"bytes,10,opt,name=last_seen,json=lastSeen,proto3" json:"last_seen,omitempty"` - LastSuccessfulUpdate *timestamppb.Timestamp `protobuf:"bytes,11,opt,name=last_successful_update,json=lastSuccessfulUpdate,proto3" json:"last_successful_update,omitempty"` - Expiry *timestamppb.Timestamp `protobuf:"bytes,12,opt,name=expiry,proto3" json:"expiry,omitempty"` - PreAuthKey *PreAuthKey `protobuf:"bytes,13,opt,name=pre_auth_key,json=preAuthKey,proto3" json:"pre_auth_key,omitempty"` - CreatedAt *timestamppb.Timestamp `protobuf:"bytes,14,opt,name=created_at,json=createdAt,proto3" json:"created_at,omitempty"` + LastSeen *timestamppb.Timestamp `protobuf:"bytes,8,opt,name=last_seen,json=lastSeen,proto3" json:"last_seen,omitempty"` + LastSuccessfulUpdate *timestamppb.Timestamp `protobuf:"bytes,9,opt,name=last_successful_update,json=lastSuccessfulUpdate,proto3" json:"last_successful_update,omitempty"` + Expiry *timestamppb.Timestamp `protobuf:"bytes,10,opt,name=expiry,proto3" json:"expiry,omitempty"` + PreAuthKey *PreAuthKey `protobuf:"bytes,11,opt,name=pre_auth_key,json=preAuthKey,proto3" json:"pre_auth_key,omitempty"` + CreatedAt *timestamppb.Timestamp `protobuf:"bytes,12,opt,name=created_at,json=createdAt,proto3" json:"created_at,omitempty"` + RegisterMethod RegisterMethod `protobuf:"varint,13,opt,name=register_method,json=registerMethod,proto3,enum=headscale.v1.RegisterMethod" json:"register_method,omitempty"` } func (x *Machine) Reset() { @@ -175,20 +174,6 @@ func (x *Machine) GetNamespace() *Namespace { return nil } -func (x *Machine) GetRegistered() bool { - if x != nil { - return x.Registered - } - return false -} - -func (x *Machine) GetRegisterMethod() RegisterMethod { - if x != nil { - return x.RegisterMethod - } - return RegisterMethod_REGISTER_METHOD_UNSPECIFIED -} - func (x *Machine) GetLastSeen() *timestamppb.Timestamp { if x != nil { return x.LastSeen @@ -224,6 +209,13 @@ func (x *Machine) GetCreatedAt() *timestamppb.Timestamp { return nil } +func (x *Machine) GetRegisterMethod() RegisterMethod { + if x != nil { + return x.RegisterMethod + } + return RegisterMethod_REGISTER_METHOD_UNSPECIFIED +} + type RegisterMachineRequest struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -822,7 +814,7 @@ var file_headscale_v1_machine_proto_rawDesc = []byte{ 0x64, 0x73, 0x63, 0x61, 0x6c, 0x65, 0x2f, 0x76, 0x31, 0x2f, 0x6e, 0x61, 0x6d, 0x65, 0x73, 0x70, 0x61, 0x63, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x1a, 0x1d, 0x68, 0x65, 0x61, 0x64, 0x73, 0x63, 0x61, 0x6c, 0x65, 0x2f, 0x76, 0x31, 0x2f, 0x70, 0x72, 0x65, 0x61, 0x75, 0x74, 0x68, 0x6b, - 0x65, 0x79, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xfd, 0x04, 0x0a, 0x07, 0x4d, 0x61, 0x63, + 0x65, 0x79, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xdd, 0x04, 0x0a, 0x07, 0x4d, 0x61, 0x63, 0x68, 0x69, 0x6e, 0x65, 0x12, 0x0e, 0x0a, 0x02, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x04, 0x52, 0x02, 0x69, 0x64, 0x12, 0x1f, 0x0a, 0x0b, 0x6d, 0x61, 0x63, 0x68, 0x69, 0x6e, 0x65, 0x5f, 0x6b, 0x65, 0x79, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0a, 0x6d, 0x61, 0x63, 0x68, 0x69, @@ -836,33 +828,31 @@ var file_headscale_v1_machine_proto_rawDesc = []byte{ 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x35, 0x0a, 0x09, 0x6e, 0x61, 0x6d, 0x65, 0x73, 0x70, 0x61, 0x63, 0x65, 0x18, 0x07, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x17, 0x2e, 0x68, 0x65, 0x61, 0x64, 0x73, 0x63, 0x61, 0x6c, 0x65, 0x2e, 0x76, 0x31, 0x2e, 0x4e, 0x61, 0x6d, 0x65, 0x73, 0x70, 0x61, 0x63, 0x65, - 0x52, 0x09, 0x6e, 0x61, 0x6d, 0x65, 0x73, 0x70, 0x61, 0x63, 0x65, 0x12, 0x1e, 0x0a, 0x0a, 0x72, - 0x65, 0x67, 0x69, 0x73, 0x74, 0x65, 0x72, 0x65, 0x64, 0x18, 0x08, 0x20, 0x01, 0x28, 0x08, 0x52, - 0x0a, 0x72, 0x65, 0x67, 0x69, 0x73, 0x74, 0x65, 0x72, 0x65, 0x64, 0x12, 0x45, 0x0a, 0x0f, 0x72, - 0x65, 0x67, 0x69, 0x73, 0x74, 0x65, 0x72, 0x5f, 0x6d, 0x65, 0x74, 0x68, 0x6f, 0x64, 0x18, 0x09, - 0x20, 0x01, 0x28, 0x0e, 0x32, 0x1c, 0x2e, 0x68, 0x65, 0x61, 0x64, 0x73, 0x63, 0x61, 0x6c, 0x65, - 0x2e, 0x76, 0x31, 0x2e, 0x52, 0x65, 0x67, 0x69, 0x73, 0x74, 0x65, 0x72, 0x4d, 0x65, 0x74, 0x68, - 0x6f, 0x64, 0x52, 0x0e, 0x72, 0x65, 0x67, 0x69, 0x73, 0x74, 0x65, 0x72, 0x4d, 0x65, 0x74, 0x68, - 0x6f, 0x64, 0x12, 0x37, 0x0a, 0x09, 0x6c, 0x61, 0x73, 0x74, 0x5f, 0x73, 0x65, 0x65, 0x6e, 0x18, - 0x0a, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, - 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, - 0x70, 0x52, 0x08, 0x6c, 0x61, 0x73, 0x74, 0x53, 0x65, 0x65, 0x6e, 0x12, 0x50, 0x0a, 0x16, 0x6c, - 0x61, 0x73, 0x74, 0x5f, 0x73, 0x75, 0x63, 0x63, 0x65, 0x73, 0x73, 0x66, 0x75, 0x6c, 0x5f, 0x75, - 0x70, 0x64, 0x61, 0x74, 0x65, 0x18, 0x0b, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, - 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, - 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x52, 0x14, 0x6c, 0x61, 0x73, 0x74, 0x53, 0x75, 0x63, - 0x63, 0x65, 0x73, 0x73, 0x66, 0x75, 0x6c, 0x55, 0x70, 0x64, 0x61, 0x74, 0x65, 0x12, 0x32, 0x0a, - 0x06, 0x65, 0x78, 0x70, 0x69, 0x72, 0x79, 0x18, 0x0c, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, - 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, - 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x52, 0x06, 0x65, 0x78, 0x70, 0x69, 0x72, - 0x79, 0x12, 0x3a, 0x0a, 0x0c, 0x70, 0x72, 0x65, 0x5f, 0x61, 0x75, 0x74, 0x68, 0x5f, 0x6b, 0x65, - 0x79, 0x18, 0x0d, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x18, 0x2e, 0x68, 0x65, 0x61, 0x64, 0x73, 0x63, - 0x61, 0x6c, 0x65, 0x2e, 0x76, 0x31, 0x2e, 0x50, 0x72, 0x65, 0x41, 0x75, 0x74, 0x68, 0x4b, 0x65, - 0x79, 0x52, 0x0a, 0x70, 0x72, 0x65, 0x41, 0x75, 0x74, 0x68, 0x4b, 0x65, 0x79, 0x12, 0x39, 0x0a, - 0x0a, 0x63, 0x72, 0x65, 0x61, 0x74, 0x65, 0x64, 0x5f, 0x61, 0x74, 0x18, 0x0e, 0x20, 0x01, 0x28, - 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, - 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x52, 0x09, 0x63, - 0x72, 0x65, 0x61, 0x74, 0x65, 0x64, 0x41, 0x74, 0x22, 0x48, 0x0a, 0x16, 0x52, 0x65, 0x67, 0x69, + 0x52, 0x09, 0x6e, 0x61, 0x6d, 0x65, 0x73, 0x70, 0x61, 0x63, 0x65, 0x12, 0x37, 0x0a, 0x09, 0x6c, + 0x61, 0x73, 0x74, 0x5f, 0x73, 0x65, 0x65, 0x6e, 0x18, 0x08, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, + 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, + 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x52, 0x08, 0x6c, 0x61, 0x73, 0x74, + 0x53, 0x65, 0x65, 0x6e, 0x12, 0x50, 0x0a, 0x16, 0x6c, 0x61, 0x73, 0x74, 0x5f, 0x73, 0x75, 0x63, + 0x63, 0x65, 0x73, 0x73, 0x66, 0x75, 0x6c, 0x5f, 0x75, 0x70, 0x64, 0x61, 0x74, 0x65, 0x18, 0x09, + 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, + 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, + 0x52, 0x14, 0x6c, 0x61, 0x73, 0x74, 0x53, 0x75, 0x63, 0x63, 0x65, 0x73, 0x73, 0x66, 0x75, 0x6c, + 0x55, 0x70, 0x64, 0x61, 0x74, 0x65, 0x12, 0x32, 0x0a, 0x06, 0x65, 0x78, 0x70, 0x69, 0x72, 0x79, + 0x18, 0x0a, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, + 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, + 0x6d, 0x70, 0x52, 0x06, 0x65, 0x78, 0x70, 0x69, 0x72, 0x79, 0x12, 0x3a, 0x0a, 0x0c, 0x70, 0x72, + 0x65, 0x5f, 0x61, 0x75, 0x74, 0x68, 0x5f, 0x6b, 0x65, 0x79, 0x18, 0x0b, 0x20, 0x01, 0x28, 0x0b, + 0x32, 0x18, 0x2e, 0x68, 0x65, 0x61, 0x64, 0x73, 0x63, 0x61, 0x6c, 0x65, 0x2e, 0x76, 0x31, 0x2e, + 0x50, 0x72, 0x65, 0x41, 0x75, 0x74, 0x68, 0x4b, 0x65, 0x79, 0x52, 0x0a, 0x70, 0x72, 0x65, 0x41, + 0x75, 0x74, 0x68, 0x4b, 0x65, 0x79, 0x12, 0x39, 0x0a, 0x0a, 0x63, 0x72, 0x65, 0x61, 0x74, 0x65, + 0x64, 0x5f, 0x61, 0x74, 0x18, 0x0c, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, + 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, 0x6d, + 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x52, 0x09, 0x63, 0x72, 0x65, 0x61, 0x74, 0x65, 0x64, 0x41, + 0x74, 0x12, 0x45, 0x0a, 0x0f, 0x72, 0x65, 0x67, 0x69, 0x73, 0x74, 0x65, 0x72, 0x5f, 0x6d, 0x65, + 0x74, 0x68, 0x6f, 0x64, 0x18, 0x0d, 0x20, 0x01, 0x28, 0x0e, 0x32, 0x1c, 0x2e, 0x68, 0x65, 0x61, + 0x64, 0x73, 0x63, 0x61, 0x6c, 0x65, 0x2e, 0x76, 0x31, 0x2e, 0x52, 0x65, 0x67, 0x69, 0x73, 0x74, + 0x65, 0x72, 0x4d, 0x65, 0x74, 0x68, 0x6f, 0x64, 0x52, 0x0e, 0x72, 0x65, 0x67, 0x69, 0x73, 0x74, + 0x65, 0x72, 0x4d, 0x65, 0x74, 0x68, 0x6f, 0x64, 0x22, 0x48, 0x0a, 0x16, 0x52, 0x65, 0x67, 0x69, 0x73, 0x74, 0x65, 0x72, 0x4d, 0x61, 0x63, 0x68, 0x69, 0x6e, 0x65, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x1c, 0x0a, 0x09, 0x6e, 0x61, 0x6d, 0x65, 0x73, 0x70, 0x61, 0x63, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x09, 0x6e, 0x61, 0x6d, 0x65, 0x73, 0x70, 0x61, 0x63, 0x65, @@ -962,12 +952,12 @@ var file_headscale_v1_machine_proto_goTypes = []interface{}{ } var file_headscale_v1_machine_proto_depIdxs = []int32{ 14, // 0: headscale.v1.Machine.namespace:type_name -> headscale.v1.Namespace - 0, // 1: headscale.v1.Machine.register_method:type_name -> headscale.v1.RegisterMethod - 15, // 2: headscale.v1.Machine.last_seen:type_name -> google.protobuf.Timestamp - 15, // 3: headscale.v1.Machine.last_successful_update:type_name -> google.protobuf.Timestamp - 15, // 4: headscale.v1.Machine.expiry:type_name -> google.protobuf.Timestamp - 16, // 5: headscale.v1.Machine.pre_auth_key:type_name -> headscale.v1.PreAuthKey - 15, // 6: headscale.v1.Machine.created_at:type_name -> google.protobuf.Timestamp + 15, // 1: headscale.v1.Machine.last_seen:type_name -> google.protobuf.Timestamp + 15, // 2: headscale.v1.Machine.last_successful_update:type_name -> google.protobuf.Timestamp + 15, // 3: headscale.v1.Machine.expiry:type_name -> google.protobuf.Timestamp + 16, // 4: headscale.v1.Machine.pre_auth_key:type_name -> headscale.v1.PreAuthKey + 15, // 5: headscale.v1.Machine.created_at:type_name -> google.protobuf.Timestamp + 0, // 6: headscale.v1.Machine.register_method:type_name -> headscale.v1.RegisterMethod 1, // 7: headscale.v1.RegisterMachineResponse.machine:type_name -> headscale.v1.Machine 1, // 8: headscale.v1.GetMachineResponse.machine:type_name -> headscale.v1.Machine 1, // 9: headscale.v1.ExpireMachineResponse.machine:type_name -> headscale.v1.Machine diff --git a/gen/openapiv2/headscale/v1/headscale.swagger.json b/gen/openapiv2/headscale/v1/headscale.swagger.json index db348f41..8d808f98 100644 --- a/gen/openapiv2/headscale/v1/headscale.swagger.json +++ b/gen/openapiv2/headscale/v1/headscale.swagger.json @@ -885,12 +885,6 @@ "namespace": { "$ref": "#/definitions/v1Namespace" }, - "registered": { - "type": "boolean" - }, - "registerMethod": { - "$ref": "#/definitions/v1RegisterMethod" - }, "lastSeen": { "type": "string", "format": "date-time" @@ -909,6 +903,9 @@ "createdAt": { "type": "string", "format": "date-time" + }, + "registerMethod": { + "$ref": "#/definitions/v1RegisterMethod" } } }, From 5e92ddad4363815c0b29bd5933ec274db55bbb29 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 22:42:30 +0000 Subject: [PATCH 20/32] Remove redundant caches This commit removes the two extra caches (oidc, requested time) and uses the new central registration cache instead. The requested time is unified into the main machine object and the oidc key is just added to the same cache, as a string with the state as a key instead of machine key. --- api.go | 25 ++++++++++--------------- app.go | 33 ++++++++++++--------------------- app_test.go | 5 ----- grpcv1.go | 17 +---------------- machine.go | 2 -- oidc.go | 29 ++++------------------------- 6 files changed, 27 insertions(+), 84 deletions(-) diff --git a/api.go b/api.go index f831cacf..9b8fafd2 100644 --- a/api.go +++ b/api.go @@ -140,17 +140,25 @@ func (h *Headscale) RegistrationHandler(ctx *gin.Context) { // We create the machine and then keep it around until a callback // happens newMachine := Machine{ - Expiry: &time.Time{}, MachineKey: machineKeyStr, Name: req.Hostinfo.Hostname, NodeKey: NodePublicKeyStripPrefix(req.NodeKey), LastSeen: &now, } + if !req.Expiry.IsZero() { + log.Trace(). + Caller(). + Str("machine", req.Hostinfo.Hostname). + Time("expiry", req.Expiry). + Msg("Non-zero expiry time requested") + newMachine.Expiry = &req.Expiry + } + h.registrationCache.Set( machineKeyStr, newMachine, - requestedExpiryCacheExpiration, + registerCacheExpiration, ) h.handleMachineRegistrationNew(ctx, machineKey, req) @@ -490,19 +498,6 @@ func (h *Headscale) handleMachineRegistrationNew( strings.TrimSuffix(h.cfg.ServerURL, "/"), MachinePublicKeyStripPrefix(machineKey)) } - if !registerRequest.Expiry.IsZero() { - log.Trace(). - Caller(). - Str("machine", registerRequest.Hostinfo.Hostname). - Time("expiry", registerRequest.Expiry). - Msg("Non-zero expiry time requested, adding to cache") - h.requestedExpiryCache.Set( - machineKey.String(), - registerRequest.Expiry, - requestedExpiryCacheExpiration, - ) - } - respBody, err := encode(resp, &machineKey, h.privateKey) if err != nil { log.Error(). diff --git a/app.go b/app.go index 63ca10e4..f7a6e636 100644 --- a/app.go +++ b/app.go @@ -55,8 +55,8 @@ const ( HTTPReadTimeout = 30 * time.Second privateKeyFileMode = 0o600 - requestedExpiryCacheExpiration = time.Minute * 5 - requestedExpiryCacheCleanupInterval = time.Minute * 10 + registerCacheExpiration = time.Minute * 15 + registerCacheCleanup = time.Minute * 20 errUnsupportedDatabase = Error("unsupported DB") errUnsupportedLetsEncryptChallengeType = Error( @@ -148,11 +148,8 @@ type Headscale struct { lastStateChange sync.Map - oidcProvider *oidc.Provider - oauth2Config *oauth2.Config - oidcStateCache *cache.Cache - - requestedExpiryCache *cache.Cache + oidcProvider *oidc.Provider + oauth2Config *oauth2.Config registrationCache *cache.Cache @@ -204,25 +201,19 @@ func NewHeadscale(cfg Config) (*Headscale, error) { return nil, errUnsupportedDatabase } - requestedExpiryCache := cache.New( - requestedExpiryCacheExpiration, - requestedExpiryCacheCleanupInterval, - ) - registrationCache := cache.New( // TODO(kradalby): Add unified cache expiry config options - requestedExpiryCacheExpiration, - requestedExpiryCacheCleanupInterval, + registerCacheExpiration, + registerCacheCleanup, ) app := Headscale{ - cfg: cfg, - dbType: cfg.DBtype, - dbString: dbString, - privateKey: privKey, - aclRules: tailcfg.FilterAllowAll, // default allowall - requestedExpiryCache: requestedExpiryCache, - registrationCache: registrationCache, + cfg: cfg, + dbType: cfg.DBtype, + dbString: dbString, + privateKey: privKey, + aclRules: tailcfg.FilterAllowAll, // default allowall + registrationCache: registrationCache, } err = app.initDB() diff --git a/app_test.go b/app_test.go index 53c703a6..96036a1d 100644 --- a/app_test.go +++ b/app_test.go @@ -5,7 +5,6 @@ import ( "os" "testing" - "github.com/patrickmn/go-cache" "gopkg.in/check.v1" "inet.af/netaddr" ) @@ -50,10 +49,6 @@ func (s *Suite) ResetDB(c *check.C) { cfg: cfg, dbType: "sqlite3", dbString: tmpDir + "/headscale_test.db", - requestedExpiryCache: cache.New( - requestedExpiryCacheExpiration, - requestedExpiryCacheCleanupInterval, - ), } err = app.initDB() if err != nil { diff --git a/grpcv1.go b/grpcv1.go index 75732b1d..1c022905 100644 --- a/grpcv1.go +++ b/grpcv1.go @@ -160,25 +160,10 @@ func (api headscaleV1APIServer) RegisterMachine( Str("machine_key", request.GetKey()). Msg("Registering machine") - // TODO(kradalby): Currently, if it fails to find a requested expiry, non will be set - // This means that if a user is to slow with register a machine, it will possibly not - // have the correct expiry. - requestedTime := time.Time{} - if requestedTimeIf, found := api.h.requestedExpiryCache.Get(request.GetKey()); found { - log.Trace(). - Caller(). - Str("machine", request.Key). - Msg("Expiry time found in cache, assigning to node") - if reqTime, ok := requestedTimeIf.(time.Time); ok { - requestedTime = reqTime - } - } - machine, err := api.h.RegisterMachineFromAuthCallback( request.GetKey(), request.GetNamespace(), RegisterMethodCLI, - &requestedTime, ) if err != nil { return nil, err @@ -418,7 +403,7 @@ func (api headscaleV1APIServer) DebugCreateMachine( api.h.registrationCache.Set( request.GetKey(), newMachine, - requestedExpiryCacheExpiration, + registerCacheExpiration, ) return &v1.DebugCreateMachineResponse{Machine: newMachine.toProto()}, nil diff --git a/machine.go b/machine.go index ac1a1c84..03f727a2 100644 --- a/machine.go +++ b/machine.go @@ -683,7 +683,6 @@ func (h *Headscale) RegisterMachineFromAuthCallback( machineKeyStr string, namespaceName string, registrationMethod string, - expiry *time.Time, ) (*Machine, error) { if machineInterface, ok := h.registrationCache.Get(machineKeyStr); ok { if registrationMachine, ok := machineInterface.(Machine); ok { @@ -697,7 +696,6 @@ func (h *Headscale) RegisterMachineFromAuthCallback( registrationMachine.NamespaceID = namespace.ID registrationMachine.RegisterMethod = registrationMethod - registrationMachine.Expiry = expiry machine, err := h.RegisterMachine( registrationMachine, diff --git a/oidc.go b/oidc.go index e745236b..fe69a76f 100644 --- a/oidc.go +++ b/oidc.go @@ -10,20 +10,16 @@ import ( "html/template" "net/http" "strings" - "time" "github.com/coreos/go-oidc/v3/oidc" "github.com/gin-gonic/gin" - "github.com/patrickmn/go-cache" "github.com/rs/zerolog/log" "golang.org/x/oauth2" "tailscale.com/types/key" ) const ( - oidcStateCacheExpiration = time.Minute * 5 - oidcStateCacheCleanupInterval = time.Minute * 10 - randomByteSize = 16 + randomByteSize = 16 ) type IDTokenClaims struct { @@ -60,14 +56,6 @@ func (h *Headscale) initOIDC() error { } } - // init the state cache if it hasn't been already - if h.oidcStateCache == nil { - h.oidcStateCache = cache.New( - oidcStateCacheExpiration, - oidcStateCacheCleanupInterval, - ) - } - return nil } @@ -100,7 +88,7 @@ func (h *Headscale) RegisterOIDC(ctx *gin.Context) { stateStr := hex.EncodeToString(randomBlob)[:32] // place the machine key into the state cache, so it can be retrieved later - h.oidcStateCache.Set(stateStr, machineKeyStr, oidcStateCacheExpiration) + h.registrationCache.Set(stateStr, machineKeyStr, registerCacheExpiration) authURL := h.oauth2Config.AuthCodeURL(stateStr) log.Debug().Msgf("Redirecting to %s for authentication", authURL) @@ -196,7 +184,7 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { } // retrieve machinekey from state cache - machineKeyIf, machineKeyFound := h.oidcStateCache.Get(state) + machineKeyIf, machineKeyFound := h.registrationCache.Get(state) if !machineKeyFound { log.Error(). @@ -228,14 +216,6 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { return } - // TODO(kradalby): Currently, if it fails to find a requested expiry, non will be set - requestedTime := time.Time{} - if requestedTimeIf, found := h.requestedExpiryCache.Get(machineKey.String()); found { - if reqTime, ok := requestedTimeIf.(time.Time); ok { - requestedTime = reqTime - } - } - // retrieve machine information machine, err := h.GetMachineByMachineKey(machineKey) if err != nil { @@ -254,7 +234,7 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { Str("machine", machine.Name). Msg("machine already registered, reauthenticating") - h.RefreshMachine(machine, requestedTime) + h.RefreshMachine(machine, *machine.Expiry) var content bytes.Buffer if err := oidcCallbackTemplate.Execute(&content, oidcCallbackTemplateConfig{ @@ -329,7 +309,6 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { machineKeyStr, namespace.Name, RegisterMethodOIDC, - &requestedTime, ) if err != nil { log.Error(). From 8bef04d8df774d3d816e6f872436e4ec416bf8cc Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 22:45:42 +0000 Subject: [PATCH 21/32] Remove sorted todo --- oidc.go | 1 - 1 file changed, 1 deletion(-) diff --git a/oidc.go b/oidc.go index fe69a76f..93fc1be6 100644 --- a/oidc.go +++ b/oidc.go @@ -112,7 +112,6 @@ var oidcCallbackTemplate = template.Must( `), ) -// TODO: Why is the entire machine registration logic duplicated here? // OIDCCallback handles the callback from the OIDC endpoint // Retrieves the mkey from the state cache and adds the machine to the users email namespace // TODO: A confirmation page for new machines should be added to avoid phishing vulnerabilities From 379017602cec416135e8f34b4b407c4099d722c7 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 22:50:35 +0000 Subject: [PATCH 22/32] Reformat and add db backup note --- CHANGELOG.md | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20c23c99..1a2bda31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,29 +1,31 @@ # CHANGELOG -**0.15.0 (2022-xx-xx):** +## 0.15.0 (2022-xx-xx) -**BREAKING**: +**Note:** Take a backup of your database before upgrading. + +### BREAKING - Boundaries between Namespaces has been removed and all nodes can communicate by default [#357](https://github.com/juanfont/headscale/pull/357) - To limit access between nodes, use [ACLs](./docs/acls.md). -**Changes**: +### Changes - Fix a bug were the same IP could be assigned to multiple hosts if joined in quick succession [#346](https://github.com/juanfont/headscale/pull/346) - Simplify the code behind registration of machines [#366](https://github.com/juanfont/headscale/pull/366) - Nodes are now only written to database if they are registrated successfully -**0.14.0 (2022-02-24):** +## 0.14.0 (2022-02-24) -**UPCOMING BREAKING**: -From the **next** version (`0.15.0`), all machines will be able to communicate regardless of +**UPCOMING ### BREAKING +From the **next\*\* version (`0.15.0`), all machines will be able to communicate regardless of if they are in the same namespace. This means that the behaviour currently limited to ACLs will become default. From version `0.15.0`, all limitation of communications must be done with ACLs. This is a part of aligning `headscale`'s behaviour with Tailscale's upstream behaviour. -**BREAKING**: +### BREAKING - ACLs have been rewritten to align with the bevaviour Tailscale Control Panel provides. **NOTE:** This is only active if you use ACLs - Namespaces are now treated as Users @@ -31,17 +33,17 @@ This is a part of aligning `headscale`'s behaviour with Tailscale's upstream beh - Tags should now work correctly and adding a host to Headscale should now reload the rules. - The documentation have a [fictional example](docs/acls.md) that should cover some use cases of the ACLs features -**Features**: +### Features - Add support for configurable mTLS [docs](docs/tls.md#configuring-mutual-tls-authentication-mtls) [#297](https://github.com/juanfont/headscale/pull/297) -**Changes**: +### Changes - Remove dependency on CGO (switch from CGO SQLite to pure Go) [#346](https://github.com/juanfont/headscale/pull/346) **0.13.0 (2022-02-18):** -**Features**: +### Features - Add IPv6 support to the prefix assigned to namespaces - Add API Key support @@ -52,7 +54,7 @@ This is a part of aligning `headscale`'s behaviour with Tailscale's upstream beh - `oidc.domain_map` option has been removed - `strip_email_domain` option has been added (see [config-example.yaml](./config_example.yaml)) -**Changes**: +### Changes - `ip_prefix` is now superseded by `ip_prefixes` in the configuration [#208](https://github.com/juanfont/headscale/pull/208) - Upgrade `tailscale` (1.20.4) and other dependencies to latest [#314](https://github.com/juanfont/headscale/pull/314) @@ -61,35 +63,35 @@ This is a part of aligning `headscale`'s behaviour with Tailscale's upstream beh **0.12.4 (2022-01-29):** -**Changes**: +### Changes - Make gRPC Unix Socket permissions configurable [#292](https://github.com/juanfont/headscale/pull/292) - Trim whitespace before reading Private Key from file [#289](https://github.com/juanfont/headscale/pull/289) - Add new command to generate a private key for `headscale` [#290](https://github.com/juanfont/headscale/pull/290) - Fixed issue where hosts deleted from control server may be written back to the database, as long as they are connected to the control server [#278](https://github.com/juanfont/headscale/pull/278) -**0.12.3 (2022-01-13):** +## 0.12.3 (2022-01-13) -**Changes**: +### Changes - Added Alpine container [#270](https://github.com/juanfont/headscale/pull/270) - Minor updates in dependencies [#271](https://github.com/juanfont/headscale/pull/271) -**0.12.2 (2022-01-11):** +## 0.12.2 (2022-01-11) Happy New Year! -**Changes**: +### Changes - Fix Docker release [#258](https://github.com/juanfont/headscale/pull/258) - Rewrite main docs [#262](https://github.com/juanfont/headscale/pull/262) - Improve Docker docs [#263](https://github.com/juanfont/headscale/pull/263) -**0.12.1 (2021-12-24):** +## 0.12.1 (2021-12-24) (We are skipping 0.12.0 to correct a mishap done weeks ago with the version tagging) -**BREAKING**: +### BREAKING - Upgrade to Tailscale 1.18 [#229](https://github.com/juanfont/headscale/pull/229) - This change requires a new format for private key, private keys are now generated automatically: @@ -97,19 +99,19 @@ Happy New Year! 2. Restart `headscale`, a new key will be generated. 3. Restart all Tailscale clients to fetch the new key -**Changes**: +### Changes - Unify configuration example [#197](https://github.com/juanfont/headscale/pull/197) - Add stricter linting and formatting [#223](https://github.com/juanfont/headscale/pull/223) -**Features**: +### Features - Add gRPC and HTTP API (HTTP API is currently disabled) [#204](https://github.com/juanfont/headscale/pull/204) - Use gRPC between the CLI and the server [#206](https://github.com/juanfont/headscale/pull/206), [#212](https://github.com/juanfont/headscale/pull/212) - Beta OpenID Connect support [#126](https://github.com/juanfont/headscale/pull/126), [#227](https://github.com/juanfont/headscale/pull/227) -**0.11.0 (2021-10-25):** +## 0.11.0 (2021-10-25) -**BREAKING**: +### BREAKING - Make headscale fetch DERP map from URL and file [#196](https://github.com/juanfont/headscale/pull/196) From 82cb6b9ddce6d0f2e7b3539313378d301b54e671 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 23:00:41 +0000 Subject: [PATCH 23/32] Cleanup some unreachable code --- machine.go | 23 ----------------------- oidc.go | 2 +- 2 files changed, 1 insertion(+), 24 deletions(-) diff --git a/machine.go b/machine.go index 03f727a2..1560858b 100644 --- a/machine.go +++ b/machine.go @@ -718,25 +718,6 @@ func (h *Headscale) RegisterMachine(machine Machine, Str("machine_key", machine.MachineKey). Msg("Registering machine") - // If the machine is already in the database, it is seeking - // reauthentication, and by reaching this step, has been authenticated - // and need to have an updated expiry. - var machineKey key.MachinePublic - _ = machineKey.UnmarshalText( - []byte(MachinePublicKeyEnsurePrefix(machine.MachineKey)), - ) - machineFromDatabase, _ := h.GetMachineByMachineKey(machineKey) - if machineFromDatabase != nil { - log.Trace(). - Caller(). - Str("machine", machine.Name). - Msg("machine already registered, reauthenticating") - - h.RefreshMachine(machineFromDatabase, *machine.Expiry) - - return machineFromDatabase, nil - } - log.Trace(). Caller(). Str("machine", machine.Name). @@ -758,10 +739,6 @@ func (h *Headscale) RegisterMachine(machine Machine, machine.IPAddresses = ips - // TODO(kradalby): This field is uneccessary metadata, - // move it to tags instead of having a column. - // machine.RegisterMethod = registrationMethod - h.db.Save(&machine) log.Trace(). diff --git a/oidc.go b/oidc.go index 93fc1be6..8e89e27f 100644 --- a/oidc.go +++ b/oidc.go @@ -215,7 +215,7 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { return } - // retrieve machine information + // retrieve machine information if it exist machine, err := h.GetMachineByMachineKey(machineKey) if err != nil { log.Error().Msg("machine key not found in database") From 7c63412df53f2db419ddfcb66a6add530109a3c6 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 28 Feb 2022 23:02:41 +0000 Subject: [PATCH 24/32] Remove todo --- app.go | 1 - 1 file changed, 1 deletion(-) diff --git a/app.go b/app.go index f7a6e636..58185093 100644 --- a/app.go +++ b/app.go @@ -202,7 +202,6 @@ func NewHeadscale(cfg Config) (*Headscale, error) { } registrationCache := cache.New( - // TODO(kradalby): Add unified cache expiry config options registerCacheExpiration, registerCacheCleanup, ) From a455a874add8b559eaa287c4d380ecda0196b357 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Tue, 1 Mar 2022 21:01:46 +0100 Subject: [PATCH 25/32] feat(acls): normalize the group name --- acls.go | 46 ++++++++++++++++----- acls_test.go | 115 +++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 124 insertions(+), 37 deletions(-) diff --git a/acls.go b/acls.go index 1a597c24..7b93ad76 100644 --- a/acls.go +++ b/acls.go @@ -160,7 +160,7 @@ func (h *Headscale) generateACLPolicySrcIP( aclPolicy ACLPolicy, u string, ) ([]string, error) { - return expandAlias(machines, aclPolicy, u) + return expandAlias(machines, aclPolicy, u, h.cfg.OIDC.StripEmaildomain) } func (h *Headscale) generateACLPolicyDestPorts( @@ -186,7 +186,12 @@ func (h *Headscale) generateACLPolicyDestPorts( alias = fmt.Sprintf("%s:%s", tokens[0], tokens[1]) } - expanded, err := expandAlias(machines, aclPolicy, alias) + expanded, err := expandAlias( + machines, + aclPolicy, + alias, + h.cfg.OIDC.StripEmaildomain, + ) if err != nil { return nil, err } @@ -218,6 +223,7 @@ func expandAlias( machines []Machine, aclPolicy ACLPolicy, alias string, + stripEmailDomain bool, ) ([]string, error) { ips := []string{} if alias == "*" { @@ -225,7 +231,7 @@ func expandAlias( } if strings.HasPrefix(alias, "group:") { - namespaces, err := expandGroup(aclPolicy, alias) + namespaces, err := expandGroup(aclPolicy, alias, stripEmailDomain) if err != nil { return ips, err } @@ -240,7 +246,7 @@ func expandAlias( } if strings.HasPrefix(alias, "tag:") { - owners, err := expandTagOwners(aclPolicy, alias) + owners, err := expandTagOwners(aclPolicy, alias, stripEmailDomain) if err != nil { return ips, err } @@ -396,7 +402,11 @@ func filterMachinesByNamespace(machines []Machine, namespace string) []Machine { // expandTagOwners will return a list of namespace. An owner can be either a namespace or a group // a group cannot be composed of groups. -func expandTagOwners(aclPolicy ACLPolicy, tag string) ([]string, error) { +func expandTagOwners( + aclPolicy ACLPolicy, + tag string, + stripEmailDomain bool, +) ([]string, error) { var owners []string ows, ok := aclPolicy.TagOwners[tag] if !ok { @@ -408,7 +418,7 @@ func expandTagOwners(aclPolicy ACLPolicy, tag string) ([]string, error) { } for _, owner := range ows { if strings.HasPrefix(owner, "group:") { - gs, err := expandGroup(aclPolicy, owner) + gs, err := expandGroup(aclPolicy, owner, stripEmailDomain) if err != nil { return []string{}, err } @@ -423,8 +433,13 @@ func expandTagOwners(aclPolicy ACLPolicy, tag string) ([]string, error) { // expandGroup will return the list of namespace inside the group // after some validation. -func expandGroup(aclPolicy ACLPolicy, group string) ([]string, error) { - groups, ok := aclPolicy.Groups[group] +func expandGroup( + aclPolicy ACLPolicy, + group string, + stripEmailDomain bool, +) ([]string, error) { + outGroups := []string{} + aclGroups, ok := aclPolicy.Groups[group] if !ok { return []string{}, fmt.Errorf( "group %v isn't registered. %w", @@ -432,14 +447,23 @@ func expandGroup(aclPolicy ACLPolicy, group string) ([]string, error) { errInvalidGroup, ) } - for _, g := range groups { - if strings.HasPrefix(g, "group:") { + for _, group := range aclGroups { + if strings.HasPrefix(group, "group:") { return []string{}, fmt.Errorf( "%w. A group cannot be composed of groups. https://tailscale.com/kb/1018/acls/#groups", errInvalidGroup, ) } + grp, err := NormalizeNamespaceName(group, stripEmailDomain) + if err != nil { + return []string{}, fmt.Errorf( + "failed to normalize group %q, err: %w", + group, + errInvalidGroup, + ) + } + outGroups = append(outGroups, grp) } - return groups, nil + return outGroups, nil } diff --git a/acls_test.go b/acls_test.go index 9f0432a7..6cee02c9 100644 --- a/acls_test.go +++ b/acls_test.go @@ -430,8 +430,9 @@ func (s *Suite) TestPortGroup(c *check.C) { func Test_expandGroup(t *testing.T) { type args struct { - aclPolicy ACLPolicy - group string + aclPolicy ACLPolicy + group string + stripEmailDomain bool } tests := []struct { name string @@ -448,7 +449,8 @@ func Test_expandGroup(t *testing.T) { "group:foo": []string{"user2", "user3"}, }, }, - group: "group:test", + group: "group:test", + stripEmailDomain: true, }, want: []string{"user1", "user2", "user3"}, wantErr: false, @@ -462,15 +464,54 @@ func Test_expandGroup(t *testing.T) { "group:foo": []string{"user2", "user3"}, }, }, - group: "group:undefined", + group: "group:undefined", + stripEmailDomain: true, }, want: []string{}, wantErr: true, }, + { + name: "Expand emails in group", + args: args{ + aclPolicy: ACLPolicy{ + Groups: Groups{ + "group:admin": []string{ + "joe.bar@gmail.com", + "john.doe@yahoo.fr", + }, + }, + }, + group: "group:admin", + stripEmailDomain: true, + }, + want: []string{"joe.bar", "john.doe"}, + wantErr: false, + }, + { + name: "Expand emails in group", + args: args{ + aclPolicy: ACLPolicy{ + Groups: Groups{ + "group:admin": []string{ + "joe.bar@gmail.com", + "john.doe@yahoo.fr", + }, + }, + }, + group: "group:admin", + stripEmailDomain: false, + }, + want: []string{"joe.bar.gmail.com", "john.doe.yahoo.fr"}, + wantErr: false, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, err := expandGroup(test.args.aclPolicy, test.args.group) + got, err := expandGroup( + test.args.aclPolicy, + test.args.group, + test.args.stripEmailDomain, + ) if (err != nil) != test.wantErr { t.Errorf("expandGroup() error = %v, wantErr %v", err, test.wantErr) @@ -485,8 +526,9 @@ func Test_expandGroup(t *testing.T) { func Test_expandTagOwners(t *testing.T) { type args struct { - aclPolicy ACLPolicy - tag string + aclPolicy ACLPolicy + tag string + stripEmailDomain bool } tests := []struct { name string @@ -500,7 +542,8 @@ func Test_expandTagOwners(t *testing.T) { aclPolicy: ACLPolicy{ TagOwners: TagOwners{"tag:test": []string{"user1"}}, }, - tag: "tag:test", + tag: "tag:test", + stripEmailDomain: true, }, want: []string{"user1"}, wantErr: false, @@ -512,7 +555,8 @@ func Test_expandTagOwners(t *testing.T) { Groups: Groups{"group:foo": []string{"user1", "user2"}}, TagOwners: TagOwners{"tag:test": []string{"group:foo"}}, }, - tag: "tag:test", + tag: "tag:test", + stripEmailDomain: true, }, want: []string{"user1", "user2"}, wantErr: false, @@ -524,7 +568,8 @@ func Test_expandTagOwners(t *testing.T) { Groups: Groups{"group:foo": []string{"user1", "user2"}}, TagOwners: TagOwners{"tag:test": []string{"group:foo", "user3"}}, }, - tag: "tag:test", + tag: "tag:test", + stripEmailDomain: true, }, want: []string{"user1", "user2", "user3"}, wantErr: false, @@ -535,7 +580,8 @@ func Test_expandTagOwners(t *testing.T) { aclPolicy: ACLPolicy{ TagOwners: TagOwners{"tag:foo": []string{"group:foo", "user1"}}, }, - tag: "tag:test", + tag: "tag:test", + stripEmailDomain: true, }, want: []string{}, wantErr: true, @@ -547,7 +593,8 @@ func Test_expandTagOwners(t *testing.T) { Groups: Groups{"group:bar": []string{"user1", "user2"}}, TagOwners: TagOwners{"tag:test": []string{"group:foo", "user2"}}, }, - tag: "tag:test", + tag: "tag:test", + stripEmailDomain: true, }, want: []string{}, wantErr: true, @@ -555,7 +602,11 @@ func Test_expandTagOwners(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, err := expandTagOwners(test.args.aclPolicy, test.args.tag) + got, err := expandTagOwners( + test.args.aclPolicy, + test.args.tag, + test.args.stripEmailDomain, + ) if (err != nil) != test.wantErr { t.Errorf("expandTagOwners() error = %v, wantErr %v", err, test.wantErr) @@ -717,9 +768,10 @@ func Test_listMachinesInNamespace(t *testing.T) { // nolint func Test_expandAlias(t *testing.T) { type args struct { - machines []Machine - aclPolicy ACLPolicy - alias string + machines []Machine + aclPolicy ACLPolicy + alias string + stripEmailDomain bool } tests := []struct { name string @@ -739,7 +791,8 @@ func Test_expandAlias(t *testing.T) { }, }, }, - aclPolicy: ACLPolicy{}, + aclPolicy: ACLPolicy{}, + stripEmailDomain: true, }, want: []string{"*"}, wantErr: false, @@ -777,6 +830,7 @@ func Test_expandAlias(t *testing.T) { aclPolicy: ACLPolicy{ Groups: Groups{"group:accountant": []string{"joe", "marc"}}, }, + stripEmailDomain: true, }, want: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, wantErr: false, @@ -814,6 +868,7 @@ func Test_expandAlias(t *testing.T) { aclPolicy: ACLPolicy{ Groups: Groups{"group:accountant": []string{"joe", "marc"}}, }, + stripEmailDomain: true, }, want: []string{}, wantErr: true, @@ -821,9 +876,10 @@ func Test_expandAlias(t *testing.T) { { name: "simple ipaddress", args: args{ - alias: "10.0.0.3", - machines: []Machine{}, - aclPolicy: ACLPolicy{}, + alias: "10.0.0.3", + machines: []Machine{}, + aclPolicy: ACLPolicy{}, + stripEmailDomain: true, }, want: []string{"10.0.0.3"}, wantErr: false, @@ -838,6 +894,7 @@ func Test_expandAlias(t *testing.T) { "homeNetwork": netaddr.MustParseIPPrefix("192.168.1.0/24"), }, }, + stripEmailDomain: true, }, want: []string{"192.168.1.0/24"}, wantErr: false, @@ -845,9 +902,10 @@ func Test_expandAlias(t *testing.T) { { name: "simple host", args: args{ - alias: "10.0.0.1", - machines: []Machine{}, - aclPolicy: ACLPolicy{}, + alias: "10.0.0.1", + machines: []Machine{}, + aclPolicy: ACLPolicy{}, + stripEmailDomain: true, }, want: []string{"10.0.0.1"}, wantErr: false, @@ -855,9 +913,10 @@ func Test_expandAlias(t *testing.T) { { name: "simple CIDR", args: args{ - alias: "10.0.0.0/16", - machines: []Machine{}, - aclPolicy: ACLPolicy{}, + alias: "10.0.0.0/16", + machines: []Machine{}, + aclPolicy: ACLPolicy{}, + stripEmailDomain: true, }, want: []string{"10.0.0.0/16"}, wantErr: false, @@ -901,6 +960,7 @@ func Test_expandAlias(t *testing.T) { aclPolicy: ACLPolicy{ TagOwners: TagOwners{"tag:hr-webserver": []string{"joe"}}, }, + stripEmailDomain: true, }, want: []string{"100.64.0.1", "100.64.0.2"}, wantErr: false, @@ -941,6 +1001,7 @@ func Test_expandAlias(t *testing.T) { "tag:accountant-webserver": []string{"group:accountant"}, }, }, + stripEmailDomain: true, }, want: []string{}, wantErr: true, @@ -984,6 +1045,7 @@ func Test_expandAlias(t *testing.T) { aclPolicy: ACLPolicy{ TagOwners: TagOwners{"tag:accountant-webserver": []string{"joe"}}, }, + stripEmailDomain: true, }, want: []string{"100.64.0.4"}, wantErr: false, @@ -995,6 +1057,7 @@ func Test_expandAlias(t *testing.T) { test.args.machines, test.args.aclPolicy, test.args.alias, + test.args.stripEmailDomain, ) if (err != nil) != test.wantErr { t.Errorf("expandAlias() error = %v, wantErr %v", err, test.wantErr) From b2dca80e7a2f0364d476972dc0d6de2e653939e5 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Tue, 1 Mar 2022 21:16:33 +0100 Subject: [PATCH 26/32] docs: update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2b93ef6..c1b3ee8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ **Features**: - Add support for writing ACL files with YAML [#359](https://github.com/juanfont/headscale/pull/359) +- Users can now use emails in ACL's groups [#372](https://github.com/juanfont/headscale/issues/372) **Changes**: From 361b4f7f4f48fc399da149ffc2bf4b8d5e76c074 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Tue, 1 Mar 2022 22:43:25 +0100 Subject: [PATCH 27/32] fix(machine): allow to use * in ACL sources --- machine.go | 20 ++++- machine_test.go | 205 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 223 insertions(+), 2 deletions(-) diff --git a/machine.go b/machine.go index 3106d0b2..479380a0 100644 --- a/machine.go +++ b/machine.go @@ -173,6 +173,12 @@ func getFilteredByACLPeers( machine.IPAddresses.ToStringSlice(), peer.IPAddresses.ToStringSlice(), ) || // match source and destination + matchSourceAndDestinationWithRule( + rule.SrcIPs, + dst, + peer.IPAddresses.ToStringSlice(), + machine.IPAddresses.ToStringSlice(), + ) || // match return path matchSourceAndDestinationWithRule( rule.SrcIPs, dst, @@ -182,9 +188,21 @@ func getFilteredByACLPeers( matchSourceAndDestinationWithRule( rule.SrcIPs, dst, + []string{"*"}, + []string{"*"}, + ) || // match source and all destination + matchSourceAndDestinationWithRule( + rule.SrcIPs, + dst, + []string{"*"}, peer.IPAddresses.ToStringSlice(), + ) || // match source and all destination + matchSourceAndDestinationWithRule( + rule.SrcIPs, + dst, + []string{"*"}, machine.IPAddresses.ToStringSlice(), - ) { // match return path + ) { // match all sources and source peers[peer.ID] = peer } } diff --git a/machine_test.go b/machine_test.go index e9c91f8b..ed95aaf7 100644 --- a/machine_test.go +++ b/machine_test.go @@ -296,6 +296,7 @@ func (s *Suite) TestSerdeAddressStrignSlice(c *check.C) { } } +// nolint func Test_getFilteredByACLPeers(t *testing.T) { type args struct { machines []Machine @@ -443,7 +444,7 @@ func Test_getFilteredByACLPeers(t *testing.T) { }, }, machine: &Machine{ // current machine - ID: 1, + ID: 2, IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, Namespace: Namespace{Name: "marc"}, }, @@ -456,6 +457,208 @@ func Test_getFilteredByACLPeers(t *testing.T) { }, }, }, + { + name: "rules allows all hosts to reach one destination", + args: args{ + machines: []Machine{ // list of all machines in the database + { + ID: 1, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, + }, + { + ID: 2, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "marc"}, + }, + { + ID: 3, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "mickael"}, + }, + }, + rules: []tailcfg.FilterRule{ // list of all ACLRules registered + { + SrcIPs: []string{"*"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "100.64.0.2"}, + }, + }, + }, + machine: &Machine{ // current machine + ID: 1, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, + }, + }, + want: Machines{ + { + ID: 2, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "marc"}, + }, + }, + }, + { + name: "rules allows all hosts to reach one destination, destination can reach all hosts", + args: args{ + machines: []Machine{ // list of all machines in the database + { + ID: 1, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, + }, + { + ID: 2, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "marc"}, + }, + { + ID: 3, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "mickael"}, + }, + }, + rules: []tailcfg.FilterRule{ // list of all ACLRules registered + { + SrcIPs: []string{"*"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "100.64.0.2"}, + }, + }, + }, + machine: &Machine{ // current machine + ID: 2, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "marc"}, + }, + }, + want: Machines{ + { + ID: 1, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, + }, + { + ID: 3, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "mickael"}, + }, + }, + }, + { + name: "rule allows all hosts to reach all destinations", + args: args{ + machines: []Machine{ // list of all machines in the database + { + ID: 1, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, + }, + { + ID: 2, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "marc"}, + }, + { + ID: 3, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "mickael"}, + }, + }, + rules: []tailcfg.FilterRule{ // list of all ACLRules registered + { + SrcIPs: []string{"*"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "*"}, + }, + }, + }, + machine: &Machine{ // current machine + ID: 2, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, + Namespace: Namespace{Name: "marc"}, + }, + }, + want: Machines{ + { + ID: 1, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, + }, + { + ID: 3, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.3")}, + Namespace: Namespace{Name: "mickael"}, + }, + }, + }, + { + name: "without rule all communications are forbidden", + args: args{ + machines: []Machine{ // list of all machines in the database + { + ID: 1, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.1"), + }, + Namespace: Namespace{Name: "joe"}, + }, + { + ID: 2, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.2"), + }, + Namespace: Namespace{Name: "marc"}, + }, + { + ID: 3, + IPAddresses: MachineAddresses{ + netaddr.MustParseIP("100.64.0.3"), + }, + Namespace: Namespace{Name: "mickael"}, + }, + }, + rules: []tailcfg.FilterRule{ // list of all ACLRules registered + }, + machine: &Machine{ // current machine + ID: 2, + IPAddresses: MachineAddresses{netaddr.MustParseIP("100.64.0.2")}, + Namespace: Namespace{Name: "marc"}, + }, + }, + want: Machines{}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 77fe0b01f7d42f51e67ea0307cf52c2c975844ab Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Tue, 1 Mar 2022 22:50:22 +0100 Subject: [PATCH 28/32] docs: update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1b3ee8f..d75388b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ **Changes**: - Fix a bug were the same IP could be assigned to multiple hosts if joined in quick succession [#346](https://github.com/juanfont/headscale/pull/346) +- Fix a limitation in the ACLs that prevented users to write rules with `*` as source [#374](https://github.com/juanfont/headscale/issues/374) **0.14.0 (2022-02-24):** From 86ade72c193626560ea80b327510af91f1a12890 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 1 Mar 2022 18:51:56 +0000 Subject: [PATCH 29/32] Remove err check --- oidc.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/oidc.go b/oidc.go index 8e89e27f..f7cb2492 100644 --- a/oidc.go +++ b/oidc.go @@ -216,16 +216,7 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { } // retrieve machine information if it exist - machine, err := h.GetMachineByMachineKey(machineKey) - if err != nil { - log.Error().Msg("machine key not found in database") - ctx.String( - http.StatusInternalServerError, - "could not get machine info from database", - ) - - return - } + machine, _ := h.GetMachineByMachineKey(machineKey) if machine != nil { log.Trace(). From ec4dc68524b5396443e3a6b25d546d3855ad2c2f Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 2 Mar 2022 06:55:21 +0000 Subject: [PATCH 30/32] Use correct machinekey format for oidc reg --- oidc.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/oidc.go b/oidc.go index f7cb2492..c200f5e1 100644 --- a/oidc.go +++ b/oidc.go @@ -193,10 +193,12 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { return } - machineKeyStr, machineKeyOK := machineKeyIf.(string) + machineKeyFromCache, machineKeyOK := machineKeyIf.(string) var machineKey key.MachinePublic - err = machineKey.UnmarshalText([]byte(MachinePublicKeyEnsurePrefix(machineKeyStr))) + err = machineKey.UnmarshalText( + []byte(MachinePublicKeyEnsurePrefix(machineKeyFromCache)), + ) if err != nil { log.Error(). Msg("could not parse machine public key") @@ -295,6 +297,8 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { return } + machineKeyStr := MachinePublicKeyStripPrefix(machineKey) + _, err = h.RegisterMachineFromAuthCallback( machineKeyStr, namespace.Name, From ef422e69882f4c1ba0e04bc8019350e41fdabd59 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 2 Mar 2022 07:15:20 +0000 Subject: [PATCH 31/32] Protect against expiry nil --- api.go | 1 + machine.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/api.go b/api.go index 9b8fafd2..3b3b6757 100644 --- a/api.go +++ b/api.go @@ -144,6 +144,7 @@ func (h *Headscale) RegistrationHandler(ctx *gin.Context) { Name: req.Hostinfo.Hostname, NodeKey: NodePublicKeyStripPrefix(req.NodeKey), LastSeen: &now, + Expiry: &time.Time{}, } if !req.Expiry.IsZero() { diff --git a/machine.go b/machine.go index bd842ae3..c9370c27 100644 --- a/machine.go +++ b/machine.go @@ -117,7 +117,7 @@ func (machine Machine) isExpired() bool { // If Expiry is not set, the client has not indicated that // it wants an expiry time, it is therefor considered // to mean "not expired" - if machine.Expiry.IsZero() { + if machine.Expiry == nil || machine.Expiry.IsZero() { return false } From 1f8c7f427b82f6ff1259c4b35eb9969e90880d1d Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 2 Mar 2022 07:29:40 +0000 Subject: [PATCH 32/32] Add comment --- oidc.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/oidc.go b/oidc.go index c200f5e1..987bc8b2 100644 --- a/oidc.go +++ b/oidc.go @@ -218,6 +218,9 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { } // retrieve machine information if it exist + // The error is not important, because if it does not + // exist, then this is a new machine and we will move + // on to registration. machine, _ := h.GetMachineByMachineKey(machineKey) if machine != nil {