From 41bad2b9fdac7171e68babce0b6b045316d298f3 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 5 Jan 2025 07:35:18 +0000 Subject: [PATCH 1/3] flake.lock: Update (#2324) --- flake.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flake.lock b/flake.lock index 5e3aad8b..e8ec0b76 100644 --- a/flake.lock +++ b/flake.lock @@ -20,11 +20,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1735268880, - "narHash": "sha256-7QEFnKkzD13SPxs+UFR5bUFN2fRw+GlL0am72ZjNre4=", + "lastModified": 1735915915, + "narHash": "sha256-Q4HuFAvoKAIiTRZTUxJ0ZXeTC7lLfC9/dggGHNXNlCw=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "7cc0bff31a3a705d3ac4fdceb030a17239412210", + "rev": "a27871180d30ebee8aa6b11bf7fef8a52f024733", "type": "github" }, "original": { From fa641e38b8a62ad665e15370a2b29a48c6486060 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 8 Jan 2025 16:29:37 +0100 Subject: [PATCH 2/3] Set CSRF cookies for OIDC (#2328) * set state and nounce in oidc to prevent csrf Fixes #2276 * try to fix new postgres issue Signed-off-by: Kristoffer Dalby --------- Signed-off-by: Kristoffer Dalby --- .github/workflows/test.yml | 6 ++++ hscontrol/oidc.go | 61 +++++++++++++++++++++++++++++++---- integration/auth_oidc_test.go | 54 +++++++++++++++++++++++-------- 3 files changed, 100 insertions(+), 21 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f4659332..610c60f6 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -34,4 +34,10 @@ jobs: - name: Run tests if: steps.changed-files.outputs.files == 'true' + env: + # As of 2025-01-06, these env vars was not automatically + # set anymore which breaks the initdb for postgres on + # some of the database migration tests. + LC_ALL: "en_US.UTF-8" + LC_CTYPE: "en_US.UTF-8" run: nix develop --command -- gotestsum diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go index 35e3c778..8f3003cb 100644 --- a/hscontrol/oidc.go +++ b/hscontrol/oidc.go @@ -3,9 +3,7 @@ package hscontrol import ( "bytes" "context" - "crypto/rand" _ "embed" - "encoding/hex" "errors" "fmt" "html/template" @@ -157,13 +155,19 @@ func (a *AuthProviderOIDC) RegisterHandler( return } - randomBlob := make([]byte, randomByteSize) - if _, err := rand.Read(randomBlob); err != nil { + // Set the state and nonce cookies to protect against CSRF attacks + state, err := setCSRFCookie(writer, req, "state") + if err != nil { http.Error(writer, "Internal server error", http.StatusInternalServerError) return } - stateStr := hex.EncodeToString(randomBlob)[:32] + // Set the state and nonce cookies to protect against CSRF attacks + nonce, err := setCSRFCookie(writer, req, "nonce") + if err != nil { + http.Error(writer, "Internal server error", http.StatusInternalServerError) + return + } // Initialize registration info with machine key registrationInfo := RegistrationInfo{ @@ -191,11 +195,12 @@ func (a *AuthProviderOIDC) RegisterHandler( for k, v := range a.cfg.ExtraParams { extras = append(extras, oauth2.SetAuthURLParam(k, v)) } + extras = append(extras, oidc.Nonce(nonce)) // Cache the registration info - a.registrationCache.Set(stateStr, registrationInfo) + a.registrationCache.Set(state, registrationInfo) - authURL := a.oauth2Config.AuthCodeURL(stateStr, extras...) + authURL := a.oauth2Config.AuthCodeURL(state, extras...) log.Debug().Msgf("Redirecting to %s for authentication", authURL) http.Redirect(writer, req, authURL, http.StatusFound) @@ -228,11 +233,34 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler( return } + log.Debug().Interface("cookies", req.Cookies()).Msg("Received oidc callback") + cookieState, err := req.Cookie("state") + if err != nil { + http.Error(writer, "state not found", http.StatusBadRequest) + return + } + + if state != cookieState.Value { + http.Error(writer, "state did not match", http.StatusBadRequest) + return + } + idToken, err := a.extractIDToken(req.Context(), code, state) if err != nil { http.Error(writer, err.Error(), http.StatusBadRequest) return } + + nonce, err := req.Cookie("nonce") + if err != nil { + http.Error(writer, "nonce not found", http.StatusBadRequest) + return + } + if idToken.Nonce != nonce.Value { + http.Error(writer, "nonce did not match", http.StatusBadRequest) + return + } + nodeExpiry := a.determineNodeExpiry(idToken.Expiry) var claims types.OIDCClaims @@ -592,3 +620,22 @@ func getUserName( return userName, nil } + +func setCSRFCookie(w http.ResponseWriter, r *http.Request, name string) (string, error) { + val, err := util.GenerateRandomStringURLSafe(64) + if err != nil { + return val, err + } + + c := &http.Cookie{ + Path: "/oidc/callback", + Name: name, + Value: val, + MaxAge: int(time.Hour.Seconds()), + Secure: r.TLS != nil, + HttpOnly: true, + } + http.SetCookie(w, c) + + return val, nil +} diff --git a/integration/auth_oidc_test.go b/integration/auth_oidc_test.go index e8b49991..e74eae56 100644 --- a/integration/auth_oidc_test.go +++ b/integration/auth_oidc_test.go @@ -10,6 +10,8 @@ import ( "log" "net" "net/http" + "net/http/cookiejar" + "net/http/httptest" "net/netip" "sort" "strconv" @@ -747,6 +749,24 @@ func (s *AuthOIDCScenario) runMockOIDC(accessTTL time.Duration, users []mockoidc }, nil } +type LoggingRoundTripper struct{} + +func (t LoggingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + noTls := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // nolint + } + resp, err := noTls.RoundTrip(req) + if err != nil { + return nil, err + } + + log.Printf("---") + log.Printf("method: %s | url: %s", resp.Request.Method, resp.Request.URL.String()) + log.Printf("status: %d | cookies: %+v", resp.StatusCode, resp.Cookies()) + + return resp, nil +} + func (s *AuthOIDCScenario) runTailscaleUp( userStr, loginServer string, ) error { @@ -758,35 +778,39 @@ func (s *AuthOIDCScenario) runTailscaleUp( log.Printf("running tailscale up for user %s", userStr) if user, ok := s.users[userStr]; ok { for _, client := range user.Clients { - c := client + tsc := client user.joinWaitGroup.Go(func() error { - loginURL, err := c.LoginWithURL(loginServer) + loginURL, err := tsc.LoginWithURL(loginServer) if err != nil { - log.Printf("%s failed to run tailscale up: %s", c.Hostname(), err) + log.Printf("%s failed to run tailscale up: %s", tsc.Hostname(), err) } - loginURL.Host = fmt.Sprintf("%s:8080", headscale.GetIP()) + loginURL.Host = fmt.Sprintf("%s:8080", headscale.GetHostname()) loginURL.Scheme = "http" if len(headscale.GetCert()) > 0 { loginURL.Scheme = "https" } - insecureTransport := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // nolint + httptest.NewRecorder() + hc := &http.Client{ + Transport: LoggingRoundTripper{}, + } + hc.Jar, err = cookiejar.New(nil) + if err != nil { + log.Printf("failed to create cookie jar: %s", err) } - log.Printf("%s login url: %s\n", c.Hostname(), loginURL.String()) + log.Printf("%s login url: %s\n", tsc.Hostname(), loginURL.String()) - log.Printf("%s logging in with url", c.Hostname()) - httpClient := &http.Client{Transport: insecureTransport} + log.Printf("%s logging in with url", tsc.Hostname()) ctx := context.Background() req, _ := http.NewRequestWithContext(ctx, http.MethodGet, loginURL.String(), nil) - resp, err := httpClient.Do(req) + resp, err := hc.Do(req) if err != nil { log.Printf( "%s failed to login using url %s: %s", - c.Hostname(), + tsc.Hostname(), loginURL, err, ) @@ -794,8 +818,10 @@ func (s *AuthOIDCScenario) runTailscaleUp( return err } + log.Printf("cookies: %+v", hc.Jar.Cookies(loginURL)) + if resp.StatusCode != http.StatusOK { - log.Printf("%s response code of oidc login request was %s", c.Hostname(), resp.Status) + log.Printf("%s response code of oidc login request was %s", tsc.Hostname(), resp.Status) body, _ := io.ReadAll(resp.Body) log.Printf("body: %s", body) @@ -806,12 +832,12 @@ func (s *AuthOIDCScenario) runTailscaleUp( _, err = io.ReadAll(resp.Body) if err != nil { - log.Printf("%s failed to read response body: %s", c.Hostname(), err) + log.Printf("%s failed to read response body: %s", tsc.Hostname(), err) return err } - log.Printf("Finished request for %s to join tailnet", c.Hostname()) + log.Printf("Finished request for %s to join tailnet", tsc.Hostname()) return nil }) From ede4f97a16b0b2d357d3584431e9feb34d43fc89 Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Wed, 8 Jan 2025 11:11:48 +0100 Subject: [PATCH 3/3] Fix typos --- CHANGELOG.md | 4 ++-- docs/ref/dns.md | 4 ++-- docs/ref/integration/web-ui.md | 2 +- docs/setup/install/source.md | 4 ++-- flake.nix | 2 +- hscontrol/db/node.go | 6 +++--- hscontrol/db/routes.go | 6 +++--- hscontrol/db/routes_test.go | 4 ++-- hscontrol/notifier/notifier.go | 2 +- hscontrol/policy/acls.go | 2 +- hscontrol/poll.go | 4 ++-- hscontrol/types/config.go | 4 ++-- 12 files changed, 22 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce3e10e7..476c40d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,7 +153,7 @@ This will also affect the way you ### Changes -- Improved compatibilty of built-in DERP server with clients connecting over +- Improved compatibility of built-in DERP server with clients connecting over WebSocket [#2132](https://github.com/juanfont/headscale/pull/2132) - Allow nodes to use SSH agent forwarding [#2145](https://github.com/juanfont/headscale/pull/2145) @@ -262,7 +262,7 @@ part of adopting [#1460](https://github.com/juanfont/headscale/pull/1460). - `prefixes.allocation` can be set to assign IPs at `sequential` or `random`. [#1869](https://github.com/juanfont/headscale/pull/1869) - MagicDNS domains no longer contain usernames []() - - This is in preperation to fix Headscales implementation of tags which + - This is in preparation to fix Headscales implementation of tags which currently does not correctly remove the link between a tagged device and a user. As tagged devices will not have a user, this will require a change to the DNS generation, removing the username, see diff --git a/docs/ref/dns.md b/docs/ref/dns.md index 9eaa5245..3777661a 100644 --- a/docs/ref/dns.md +++ b/docs/ref/dns.md @@ -1,13 +1,13 @@ # DNS -Headscale supports [most DNS features](../about/features.md) from Tailscale. DNS releated settings can be configured +Headscale supports [most DNS features](../about/features.md) from Tailscale. DNS related settings can be configured within `dns` section of the [configuration file](./configuration.md). ## Setting extra DNS records Headscale allows to set extra DNS records which are made available via [MagicDNS](https://tailscale.com/kb/1081/magicdns). Extra DNS records can be configured either via static entries in the -[configuration file](./configuration.md) or from a JSON file that Headscale continously watches for changes: +[configuration file](./configuration.md) or from a JSON file that Headscale continuously watches for changes: * Use the `dns.extra_records` option in the [configuration file](./configuration.md) for entries that are static and don't change while Headscale is running. Those entries are processed when Headscale is starting up and changes to the diff --git a/docs/ref/integration/web-ui.md b/docs/ref/integration/web-ui.md index de86e5d7..4bcb7495 100644 --- a/docs/ref/integration/web-ui.md +++ b/docs/ref/integration/web-ui.md @@ -11,7 +11,7 @@ Headscale doesn't provide a built-in web interface but users may pick one from t | --------------- | ------------------------------------------------------- | ----------------------------------------------------------------------------------- | | headscale-webui | [Github](https://github.com/ifargle/headscale-webui) | A simple headscale web UI for small-scale deployments. | | headscale-ui | [Github](https://github.com/gurucomputing/headscale-ui) | A web frontend for the headscale Tailscale-compatible coordination server | -| HeadscaleUi | [GitHub](https://github.com/simcu/headscale-ui) | A static headscale admin ui, no backend enviroment required | +| HeadscaleUi | [GitHub](https://github.com/simcu/headscale-ui) | A static headscale admin ui, no backend environment required | | Headplane | [GitHub](https://github.com/tale/headplane) | An advanced Tailscale inspired frontend for headscale | | headscale-admin | [Github](https://github.com/GoodiesHQ/headscale-admin) | Headscale-Admin is meant to be a simple, modern web interface for headscale | | ouroboros | [Github](https://github.com/yellowsink/ouroboros) | Ouroboros is designed for users to manage their own devices, rather than for admins | diff --git a/docs/setup/install/source.md b/docs/setup/install/source.md index 327430b4..27074855 100644 --- a/docs/setup/install/source.md +++ b/docs/setup/install/source.md @@ -16,7 +16,7 @@ README](https://github.com/juanfont/headscale#contributing) for more information ### Install from source ```shell -# Install prerequistes +# Install prerequisites pkg_add go git clone https://github.com/juanfont/headscale.git @@ -42,7 +42,7 @@ cp headscale /usr/local/sbin ### Install from source via cross compile ```shell -# Install prerequistes +# Install prerequisites # 1. go v1.20+: headscale newer than 0.21 needs go 1.20+ to compile # 2. gmake: Makefile in the headscale repo is written in GNU make syntax diff --git a/flake.nix b/flake.nix index 8afb67ea..507f82d7 100644 --- a/flake.nix +++ b/flake.nix @@ -31,7 +31,7 @@ checkFlags = ["-short"]; # When updating go.mod or go.sum, a new sha will need to be calculated, - # update this if you have a mismatch after doing a change to thos files. + # update this if you have a mismatch after doing a change to those files. vendorHash = "sha256-SBfeixT8DQOrK2SWmHHSOBtzRdSZs+pwomHpw6Jd+qc="; subPackages = ["cmd/headscale"]; diff --git a/hscontrol/db/node.go b/hscontrol/db/node.go index 1c2a165c..ce9c90e9 100644 --- a/hscontrol/db/node.go +++ b/hscontrol/db/node.go @@ -245,7 +245,7 @@ func RenameNode(tx *gorm.DB, return fmt.Errorf("renaming node: %w", err) } - uniq, err := isUnqiueName(tx, newName) + uniq, err := isUniqueName(tx, newName) if err != nil { return fmt.Errorf("checking if name is unique: %w", err) } @@ -630,7 +630,7 @@ func generateGivenName(suppliedName string, randomSuffix bool) (string, error) { return suppliedName, nil } -func isUnqiueName(tx *gorm.DB, name string) (bool, error) { +func isUniqueName(tx *gorm.DB, name string) (bool, error) { nodes := types.Nodes{} if err := tx. Where("given_name = ?", name).Find(&nodes).Error; err != nil { @@ -649,7 +649,7 @@ func ensureUniqueGivenName( return "", err } - unique, err := isUnqiueName(tx, givenName) + unique, err := isUniqueName(tx, givenName) if err != nil { return "", err } diff --git a/hscontrol/db/routes.go b/hscontrol/db/routes.go index 6325dacc..8d86145a 100644 --- a/hscontrol/db/routes.go +++ b/hscontrol/db/routes.go @@ -417,10 +417,10 @@ func SaveNodeRoutes(tx *gorm.DB, node *types.Node) (bool, error) { return sendUpdate, nil } -// FailoverNodeRoutesIfNeccessary takes a node and checks if the node's route +// FailoverNodeRoutesIfNecessary takes a node and checks if the node's route // need to be failed over to another host. // If needed, the failover will be attempted. -func FailoverNodeRoutesIfNeccessary( +func FailoverNodeRoutesIfNecessary( tx *gorm.DB, isLikelyConnected *xsync.MapOf[types.NodeID, bool], node *types.Node, @@ -473,7 +473,7 @@ nodeRouteLoop: return &types.StateUpdate{ Type: types.StatePeerChanged, ChangeNodes: chng, - Message: "called from db.FailoverNodeRoutesIfNeccessary", + Message: "called from db.FailoverNodeRoutesIfNecessary", }, nil } diff --git a/hscontrol/db/routes_test.go b/hscontrol/db/routes_test.go index 909024fc..4547339a 100644 --- a/hscontrol/db/routes_test.go +++ b/hscontrol/db/routes_test.go @@ -342,7 +342,7 @@ func dbForTest(t *testing.T, testName string) *HSDatabase { return db } -func TestFailoverNodeRoutesIfNeccessary(t *testing.T) { +func TestFailoverNodeRoutesIfNecessary(t *testing.T) { su := func(nids ...types.NodeID) *types.StateUpdate { return &types.StateUpdate{ ChangeNodes: nids, @@ -648,7 +648,7 @@ func TestFailoverNodeRoutesIfNeccessary(t *testing.T) { want := tt.want[step] got, err := Write(db.DB, func(tx *gorm.DB) (*types.StateUpdate, error) { - return FailoverNodeRoutesIfNeccessary(tx, smap(isConnected), node) + return FailoverNodeRoutesIfNecessary(tx, smap(isConnected), node) }) if (err != nil) != tt.wantErr { diff --git a/hscontrol/notifier/notifier.go b/hscontrol/notifier/notifier.go index ceede6ba..eb1df73a 100644 --- a/hscontrol/notifier/notifier.go +++ b/hscontrol/notifier/notifier.go @@ -243,7 +243,7 @@ func (n *Notifier) sendAll(update types.StateUpdate) { // has shut down the channel and is waiting for the lock held here in RemoveNode. // This means that there is potential for a deadlock which would stop all updates // going out to clients. This timeout prevents that from happening by moving on to the - // next node if the context is cancelled. Afther sendAll releases the lock, the add/remove + // next node if the context is cancelled. After sendAll releases the lock, the add/remove // call will succeed and the update will go to the correct nodes on the next call. ctx, cancel := context.WithTimeout(context.Background(), n.cfg.Tuning.NotifierSendTimeout) defer cancel() diff --git a/hscontrol/policy/acls.go b/hscontrol/policy/acls.go index 3d7a6f4a..9ac9b2f4 100644 --- a/hscontrol/policy/acls.go +++ b/hscontrol/policy/acls.go @@ -62,7 +62,7 @@ func theInternet() *netipx.IPSet { internetBuilder.RemovePrefix(tsaddr.CGNATRange()) // Delete "cant find DHCP networks" - internetBuilder.RemovePrefix(netip.MustParsePrefix("fe80::/10")) // link-loca + internetBuilder.RemovePrefix(netip.MustParsePrefix("fe80::/10")) // link-local internetBuilder.RemovePrefix(netip.MustParsePrefix("169.254.0.0/16")) theInternetSet, _ := internetBuilder.IPSet() diff --git a/hscontrol/poll.go b/hscontrol/poll.go index e6047d45..1eaa4803 100644 --- a/hscontrol/poll.go +++ b/hscontrol/poll.go @@ -387,7 +387,7 @@ func (m *mapSession) serveLongPoll() { func (m *mapSession) pollFailoverRoutes(where string, node *types.Node) { update, err := db.Write(m.h.db.DB, func(tx *gorm.DB) (*types.StateUpdate, error) { - return db.FailoverNodeRoutesIfNeccessary(tx, m.h.nodeNotifier.LikelyConnectedMap(), node) + return db.FailoverNodeRoutesIfNecessary(tx, m.h.nodeNotifier.LikelyConnectedMap(), node) }) if err != nil { m.errf(err, fmt.Sprintf("failed to ensure failover routes, %s", where)) @@ -453,7 +453,7 @@ func (m *mapSession) handleEndpointUpdate() { // If there is no NetInfo, keep the previous one. // From 1.66 the client only sends it if changed: // https://github.com/tailscale/tailscale/commit/e1011f138737286ecf5123ff887a7a5800d129a2 - // TODO(kradalby): evaulate if we need better comparing of hostinfo + // TODO(kradalby): evaluate if we need better comparing of hostinfo // before we take the changes. if m.req.Hostinfo.NetInfo == nil && m.node.Hostinfo != nil { m.req.Hostinfo.NetInfo = m.node.Hostinfo.NetInfo diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go index b462b8e9..815c7f69 100644 --- a/hscontrol/types/config.go +++ b/hscontrol/types/config.go @@ -617,7 +617,7 @@ func dns() (DNSConfig, error) { // UnmarshalKey is compatible with Environment Variables. // err := viper.UnmarshalKey("dns", &dns) // if err != nil { - // return DNSConfig{}, fmt.Errorf("unmarshaling dns config: %w", err) + // return DNSConfig{}, fmt.Errorf("unmarshalling dns config: %w", err) // } dns.MagicDNS = viper.GetBool("dns.magic_dns") @@ -632,7 +632,7 @@ func dns() (DNSConfig, error) { err := viper.UnmarshalKey("dns.extra_records", &extraRecords) if err != nil { - return DNSConfig{}, fmt.Errorf("unmarshaling dns extra records: %w", err) + return DNSConfig{}, fmt.Errorf("unmarshalling dns extra records: %w", err) } dns.ExtraRecords = extraRecords }