From ca0be818331baac40b5fa1af185b846cd675385f Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:31:06 +0200 Subject: [PATCH 01/15] Target the latest version for golint --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 17a9dcbc..20f49af4 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -26,7 +26,7 @@ jobs: if: steps.changed-files.outputs.any_changed == 'true' uses: golangci/golangci-lint-action@v2 with: - version: v1.46.1 + version: v1.49.0 # Only block PRs on new problems. # If this is not enabled, we will end up having PRs From 68305df9b20ede9756bdd6f726ddb8847ca38a7f Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:32:29 +0200 Subject: [PATCH 02/15] Applied gofumpt --- config.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config.go b/config.go index 678b575d..14350b78 100644 --- a/config.go +++ b/config.go @@ -5,12 +5,11 @@ import ( "errors" "fmt" "io/fs" + "net/netip" "net/url" "strings" "time" - "net/netip" - "github.com/coreos/go-oidc/v3/oidc" "github.com/rs/zerolog" "github.com/rs/zerolog/log" From f4d197485ce0fcee0407e08fb1024d4c26ebeefb Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:33:00 +0200 Subject: [PATCH 03/15] Use library const for HTTP verbs --- derp.go | 2 +- derp_server.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/derp.go b/derp.go index 6a153d2a..c3d100b3 100644 --- a/derp.go +++ b/derp.go @@ -34,7 +34,7 @@ func loadDERPMapFromURL(addr url.URL) (*tailcfg.DERPMap, error) { ctx, cancel := context.WithTimeout(context.Background(), HTTPReadTimeout) defer cancel() - req, err := http.NewRequestWithContext(ctx, "GET", addr.String(), nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, addr.String(), nil) if err != nil { return nil, err } diff --git a/derp_server.go b/derp_server.go index dbdbc7a5..b9650154 100644 --- a/derp_server.go +++ b/derp_server.go @@ -174,7 +174,7 @@ func (h *Headscale) DERPProbeHandler( req *http.Request, ) { switch req.Method { - case "HEAD", "GET": + case http.MethodHead, http.MethodGet: writer.Header().Set("Access-Control-Allow-Origin", "*") writer.WriteHeader(http.StatusOK) default: From 582122851d572b46126639e04160049c507684c7 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:34:23 +0200 Subject: [PATCH 04/15] Go do not like underscores in packages --- app.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app.go b/app.go index 6e37fcd4..ac4ba51a 100644 --- a/app.go +++ b/app.go @@ -18,7 +18,7 @@ import ( "github.com/coreos/go-oidc/v3/oidc" "github.com/gorilla/mux" - grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" + grpcMiddleware "github.com/grpc-ecosystem/go-grpc-middleware" "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" v1 "github.com/juanfont/headscale/gen/go/headscale/v1" "github.com/patrickmn/go-cache" @@ -601,7 +601,7 @@ func (h *Headscale) Serve() error { grpcOptions := []grpc.ServerOption{ grpc.UnaryInterceptor( - grpc_middleware.ChainUnaryServer( + grpcMiddleware.ChainUnaryServer( h.grpcAuthenticationInterceptor, zerolog.NewUnaryServerInterceptor(), ), From 9a1438d2e3916a5fde656e145f32536118338d44 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:35:13 +0200 Subject: [PATCH 05/15] Use inherited context --- derp_server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/derp_server.go b/derp_server.go index b9650154..6fe897bb 100644 --- a/derp_server.go +++ b/derp_server.go @@ -154,7 +154,7 @@ func (h *Headscale) DERPHandler( if !fastStart { pubKey := h.privateKey.Public() - pubKeyStr := pubKey.UntypedHexString() // nolint + pubKeyStr := pubKey.UntypedHexString() //nolint fmt.Fprintf(conn, "HTTP/1.1 101 Switching Protocols\r\n"+ "Upgrade: DERP\r\n"+ "Connection: Upgrade\r\n"+ @@ -202,7 +202,7 @@ func (h *Headscale) DERPBootstrapDNSHandler( ) { dnsEntries := make(map[string][]net.IP) - resolvCtx, cancel := context.WithTimeout(context.Background(), time.Minute) + resolvCtx, cancel := context.WithTimeout(req.Context(), time.Minute) defer cancel() var resolver net.Resolver for _, region := range h.DERPMap.Regions { From c1c36036ae590f7e238da1418b21fdd0c91e9431 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:35:39 +0200 Subject: [PATCH 06/15] Add timeouts for the Noise server --- noise.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/noise.go b/noise.go index c8e6674d..45bff7b3 100644 --- a/noise.go +++ b/noise.go @@ -31,7 +31,9 @@ func (h *Headscale) NoiseUpgradeHandler( return } - server := http.Server{} + server := http.Server{ + ReadTimeout: HTTPReadTimeout, + } server.Handler = h2c.NewHandler(h.noiseMux, &http2.Server{}) err = server.Serve(netutil.NewOneConnListener(noiseConn, nil)) if err != nil { From f2fda4f90670e959ba784c4cdd56ad3a03a634fd Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:36:03 +0200 Subject: [PATCH 07/15] Return error on marshaling issues --- protocol_common_utils.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/protocol_common_utils.go b/protocol_common_utils.go index 3dc435f3..a189418c 100644 --- a/protocol_common_utils.go +++ b/protocol_common_utils.go @@ -75,6 +75,8 @@ func (h *Headscale) marshalResponse( Caller(). Err(err). Msg("Cannot marshal response") + + return nil, err } if machineKey.IsZero() { // if Noise From 0d074b1da6aabf0e67b702f3acce0b7838ead582 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:37:49 +0200 Subject: [PATCH 08/15] setLastStateChangeToNow was always receiving nil --- app.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/app.go b/app.go index ac4ba51a..af622c0b 100644 --- a/app.go +++ b/app.go @@ -860,19 +860,17 @@ func (h *Headscale) getTLSSettings() (*tls.Config, error) { } } -func (h *Headscale) setLastStateChangeToNow(namespaces ...string) { +func (h *Headscale) setLastStateChangeToNow() { var err error now := time.Now().UTC() - if len(namespaces) == 0 { - namespaces, err = h.ListNamespacesStr() - if err != nil { - log.Error(). - Caller(). - Err(err). - Msg("failed to fetch all namespaces, failing to update last changed state.") - } + namespaces, err := h.ListNamespacesStr() + if err != nil { + log.Error(). + Caller(). + Err(err). + Msg("failed to fetch all namespaces, failing to update last changed state.") } for _, namespace := range namespaces { From e0857f022661f6c8e97a3c9562485f9444feffdc Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:40:14 +0200 Subject: [PATCH 09/15] Removed unused parameters in protocol functions --- protocol_common.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/protocol_common.go b/protocol_common.go index 154c14c9..d0fe8a18 100644 --- a/protocol_common.go +++ b/protocol_common.go @@ -105,7 +105,7 @@ func (h *Headscale) handleRegisterCommon( if errors.Is(err, gorm.ErrRecordNotFound) { // If the machine has AuthKey set, handle registration via PreAuthKeys if registerRequest.Auth.AuthKey != "" { - h.handleAuthKeyCommon(writer, req, registerRequest, machineKey) + h.handleAuthKeyCommon(writer, registerRequest, machineKey) return } @@ -134,7 +134,7 @@ func (h *Headscale) handleRegisterCommon( case <-req.Context().Done(): return case <-ticker.C: - h.handleNewMachineCommon(writer, req, registerRequest, machineKey) + h.handleNewMachineCommon(writer, registerRequest, machineKey) return } @@ -190,7 +190,7 @@ func (h *Headscale) handleRegisterCommon( registerCacheExpiration, ) - h.handleNewMachineCommon(writer, req, registerRequest, machineKey) + h.handleNewMachineCommon(writer, registerRequest, machineKey) return } @@ -207,7 +207,7 @@ func (h *Headscale) handleRegisterCommon( // https://github.com/tailscale/tailscale/blob/main/tailcfg/tailcfg.go#L648 if !registerRequest.Expiry.IsZero() && registerRequest.Expiry.UTC().Before(now) { - h.handleMachineLogOutCommon(writer, req, *machine, machineKey) + h.handleMachineLogOutCommon(writer, *machine, machineKey) return } @@ -256,7 +256,6 @@ func (h *Headscale) handleRegisterCommon( // TODO: check if any locks are needed around IP allocation. func (h *Headscale) handleAuthKeyCommon( writer http.ResponseWriter, - req *http.Request, registerRequest tailcfg.RegisterRequest, machineKey key.MachinePublic, ) { @@ -455,7 +454,6 @@ func (h *Headscale) handleAuthKeyCommon( // for authorizing the machine. This url is then showed to the user by the local Tailscale client. func (h *Headscale) handleNewMachineCommon( writer http.ResponseWriter, - req *http.Request, registerRequest tailcfg.RegisterRequest, machineKey key.MachinePublic, ) { @@ -511,7 +509,6 @@ func (h *Headscale) handleNewMachineCommon( func (h *Headscale) handleMachineLogOutCommon( writer http.ResponseWriter, - req *http.Request, machine Machine, machineKey key.MachinePublic, ) { @@ -699,7 +696,7 @@ func (h *Headscale) handleMachineExpiredCommon( Msg("Machine registration has expired. Sending a authurl to register") if registerRequest.Auth.AuthKey != "" { - h.handleAuthKeyCommon(writer, req, registerRequest, machineKey) + h.handleAuthKeyCommon(writer, registerRequest, machineKey) return } From 4527801d48a64e8055c537503efefbffb7820713 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:41:31 +0200 Subject: [PATCH 10/15] More unused parameters removed in protocol functions --- protocol_common.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/protocol_common.go b/protocol_common.go index d0fe8a18..a1ff06ec 100644 --- a/protocol_common.go +++ b/protocol_common.go @@ -215,7 +215,7 @@ func (h *Headscale) handleRegisterCommon( // If machine is not expired, and is register, we have a already accepted this machine, // let it proceed with a valid registration if !machine.isExpired() { - h.handleMachineValidRegistrationCommon(writer, req, *machine, machineKey) + h.handleMachineValidRegistrationCommon(writer, *machine, machineKey) return } @@ -226,7 +226,6 @@ func (h *Headscale) handleRegisterCommon( !machine.isExpired() { h.handleMachineRefreshKeyCommon( writer, - req, registerRequest, *machine, machineKey, @@ -236,7 +235,7 @@ func (h *Headscale) handleRegisterCommon( } // The machine has expired - h.handleMachineExpiredCommon(writer, req, registerRequest, *machine, machineKey) + h.handleMachineExpiredCommon(writer, registerRequest, *machine, machineKey) machine.Expiry = &time.Time{} h.registrationCache.Set( @@ -567,7 +566,6 @@ func (h *Headscale) handleMachineLogOutCommon( func (h *Headscale) handleMachineValidRegistrationCommon( writer http.ResponseWriter, - req *http.Request, machine Machine, machineKey key.MachinePublic, ) { @@ -621,7 +619,6 @@ func (h *Headscale) handleMachineValidRegistrationCommon( func (h *Headscale) handleMachineRefreshKeyCommon( writer http.ResponseWriter, - req *http.Request, registerRequest tailcfg.RegisterRequest, machine Machine, machineKey key.MachinePublic, @@ -681,7 +678,6 @@ func (h *Headscale) handleMachineRefreshKeyCommon( func (h *Headscale) handleMachineExpiredCommon( writer http.ResponseWriter, - req *http.Request, registerRequest tailcfg.RegisterRequest, machine Machine, machineKey key.MachinePublic, From f23e9dc2354d8b9b8086fa1c5bfd1ad08fd27f0f Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:43:09 +0200 Subject: [PATCH 11/15] Pass the req context when pinging the DB --- api.go | 2 +- db.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api.go b/api.go index 18ac72fe..f5de5038 100644 --- a/api.go +++ b/api.go @@ -52,7 +52,7 @@ func (h *Headscale) HealthHandler( } } - if err := h.pingDB(); err != nil { + if err := h.pingDB(req.Context()); err != nil { respond(err) return diff --git a/db.go b/db.go index 17df384d..a1a4ef33 100644 --- a/db.go +++ b/db.go @@ -221,8 +221,8 @@ func (h *Headscale) setValue(key string, value string) error { return nil } -func (h *Headscale) pingDB() error { - ctx, cancel := context.WithTimeout(context.Background(), time.Second) +func (h *Headscale) pingDB(ctx context.Context) error { + ctx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() db, err := h.db.DB() if err != nil { From 7a78314d9dee8f04e5033914813607c94f0ad922 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:44:24 +0200 Subject: [PATCH 12/15] Remove nolint directives --- acls_test.go | 1 - machine_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/acls_test.go b/acls_test.go index db04ee3d..fc0f84e2 100644 --- a/acls_test.go +++ b/acls_test.go @@ -825,7 +825,6 @@ func Test_listMachinesInNamespace(t *testing.T) { } } -// nolint func Test_expandAlias(t *testing.T) { type args struct { machines []Machine diff --git a/machine_test.go b/machine_test.go index cadd0df8..e5ef19b2 100644 --- a/machine_test.go +++ b/machine_test.go @@ -540,7 +540,6 @@ func Test_getTags(t *testing.T) { } } -// nolint func Test_getFilteredByACLPeers(t *testing.T) { type args struct { machines []Machine From 434747e0070c35d99aa89a03645d25179d950835 Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 11:47:05 +0200 Subject: [PATCH 13/15] Use timeout in lets encrypt http server --- app.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app.go b/app.go index af622c0b..59101be3 100644 --- a/app.go +++ b/app.go @@ -820,10 +820,19 @@ func (h *Headscale) getTLSSettings() (*tls.Config, error) { // Configuration via autocert with HTTP-01. This requires listening on // port 80 for the certificate validation in addition to the headscale // service, which can be configured to run on any other port. + + server := &http.Server{ + Addr: h.cfg.TLS.LetsEncrypt.Listen, + Handler: certManager.HTTPHandler(http.HandlerFunc(h.redirect)), + ReadTimeout: HTTPReadTimeout, + } + + err := server.ListenAndServe() + go func() { log.Fatal(). Caller(). - Err(http.ListenAndServe(h.cfg.TLS.LetsEncrypt.Listen, certManager.HTTPHandler(http.HandlerFunc(h.redirect)))). + Err(err). Msg("failed to set up a HTTP server") }() From 52073ce7c984ab8973b6800a08486490b398bb6d Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 15:02:18 +0200 Subject: [PATCH 14/15] Pass context in OIDC helpers --- oidc.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/oidc.go b/oidc.go index 60d531e5..f0af600e 100644 --- a/oidc.go +++ b/oidc.go @@ -148,12 +148,12 @@ func (h *Headscale) OIDCCallback( return } - rawIDToken, err := h.getIDTokenForOIDCCallback(writer, code, state) + rawIDToken, err := h.getIDTokenForOIDCCallback(req.Context(), writer, code, state) if err != nil { return } - idToken, err := h.verifyIDTokenForOIDCCallback(writer, rawIDToken) + idToken, err := h.verifyIDTokenForOIDCCallback(req.Context(), writer, rawIDToken) if err != nil { return } @@ -240,10 +240,11 @@ func validateOIDCCallbackParams( } func (h *Headscale) getIDTokenForOIDCCallback( + ctx context.Context, writer http.ResponseWriter, code, state string, ) (string, error) { - oauth2Token, err := h.oauth2Config.Exchange(context.Background(), code) + oauth2Token, err := h.oauth2Config.Exchange(ctx, code) if err != nil { log.Error(). Err(err). @@ -287,11 +288,12 @@ func (h *Headscale) getIDTokenForOIDCCallback( } func (h *Headscale) verifyIDTokenForOIDCCallback( + ctx context.Context, writer http.ResponseWriter, rawIDToken string, ) (*oidc.IDToken, error) { verifier := h.oidcProvider.Verifier(&oidc.Config{ClientID: h.cfg.OIDC.ClientID}) - idToken, err := verifier.Verify(context.Background(), rawIDToken) + idToken, err := verifier.Verify(ctx, rawIDToken) if err != nil { log.Error(). Err(err). From 204dedaa4901f43991561d05650c8ff6ab6b427c Mon Sep 17 00:00:00 2001 From: Juan Font Alonso Date: Sun, 4 Sep 2022 15:14:12 +0200 Subject: [PATCH 15/15] Only pass the context in pollmap, no req needed --- protocol_common_poll.go | 8 ++++---- protocol_legacy_poll.go | 2 +- protocol_noise_poll.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/protocol_common_poll.go b/protocol_common_poll.go index 65dcb556..6dedfd0b 100644 --- a/protocol_common_poll.go +++ b/protocol_common_poll.go @@ -22,7 +22,7 @@ const machineNameContextKey = contextKey("machineName") // managed the poll loop. func (h *Headscale) handlePollCommon( writer http.ResponseWriter, - req *http.Request, + ctx context.Context, machine *Machine, mapRequest tailcfg.MapRequest, isNoise bool, @@ -201,7 +201,7 @@ func (h *Headscale) handlePollCommon( h.pollNetMapStream( writer, - req, + ctx, machine, mapRequest, pollDataChan, @@ -221,7 +221,7 @@ func (h *Headscale) handlePollCommon( // ensuring we communicate updates and data to the connected clients. func (h *Headscale) pollNetMapStream( writer http.ResponseWriter, - req *http.Request, + ctxReq context.Context, machine *Machine, mapRequest tailcfg.MapRequest, pollDataChan chan []byte, @@ -232,7 +232,7 @@ func (h *Headscale) pollNetMapStream( h.pollNetMapStreamWG.Add(1) defer h.pollNetMapStreamWG.Done() - ctx := context.WithValue(req.Context(), machineNameContextKey, machine.Hostname) + ctx := context.WithValue(ctxReq, machineNameContextKey, machine.Hostname) ctx, cancel := context.WithCancel(ctx) defer cancel() diff --git a/protocol_legacy_poll.go b/protocol_legacy_poll.go index f7ef6548..f27ee4e3 100644 --- a/protocol_legacy_poll.go +++ b/protocol_legacy_poll.go @@ -90,5 +90,5 @@ func (h *Headscale) PollNetMapHandler( Str("machine", machine.Hostname). Msg("A machine is entering polling via the legacy protocol") - h.handlePollCommon(writer, req, machine, mapRequest, false) + h.handlePollCommon(writer, req.Context(), machine, mapRequest, false) } diff --git a/protocol_noise_poll.go b/protocol_noise_poll.go index 8498dcfb..b15183c8 100644 --- a/protocol_noise_poll.go +++ b/protocol_noise_poll.go @@ -63,5 +63,5 @@ func (h *Headscale) NoisePollNetMapHandler( Str("machine", machine.Hostname). Msg("A machine is entering polling via the Noise protocol") - h.handlePollCommon(writer, req, machine, mapRequest, true) + h.handlePollCommon(writer, req.Context(), machine, mapRequest, true) }