From 92ffac625ec7b32d2a82f89146497df67def1a47 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Tue, 22 Feb 2022 12:45:50 +0100 Subject: [PATCH 01/16] feat(namespace): add normalization function for namespace --- dns.go | 6 +- machine.go | 6 +- namespaces.go | 57 ++++++++++++++ namespaces_test.go | 61 +++++++++++++++ oidc_test.go | 180 --------------------------------------------- 5 files changed, 120 insertions(+), 190 deletions(-) delete mode 100644 oidc_test.go diff --git a/dns.go b/dns.go index 45e0faeb..9a91cff0 100644 --- a/dns.go +++ b/dns.go @@ -165,11 +165,7 @@ func getMapResponseDNSConfig( dnsConfig.Domains, fmt.Sprintf( "%s.%s", - strings.ReplaceAll( - machine.Namespace.Name, - "@", - ".", - ), // Replace @ with . for valid domain for machine + machine.Namespace.Name, baseDomain, ), ) diff --git a/machine.go b/machine.go index ee483422..25edd1d1 100644 --- a/machine.go +++ b/machine.go @@ -724,11 +724,7 @@ func (machine Machine) toNode( hostname = fmt.Sprintf( "%s.%s.%s", machine.Name, - strings.ReplaceAll( - machine.Namespace.Name, - "@", - ".", - ), // Replace @ with . for valid domain for machine + machine.Namespace.Name, baseDomain, ) } else { diff --git a/namespaces.go b/namespaces.go index bdd440cf..8160b031 100644 --- a/namespaces.go +++ b/namespaces.go @@ -2,7 +2,10 @@ package headscale import ( "errors" + "fmt" + "regexp" "strconv" + "strings" "time" v1 "github.com/juanfont/headscale/gen/go/headscale/v1" @@ -16,8 +19,11 @@ const ( errNamespaceExists = Error("Namespace already exists") errNamespaceNotFound = Error("Namespace not found") errNamespaceNotEmptyOfNodes = Error("Namespace not empty: node(s) found") + errInvalidNamespaceName = Error("Invalid namespace name") ) +var normalizeNamespaceRegex = regexp.MustCompile("[^a-z0-9-.]+") + // Namespace is the way Headscale implements the concept of users in Tailscale // // At the end of the day, users in Tailscale are some kind of 'bubbles' or namespaces @@ -30,7 +36,12 @@ type Namespace struct { // CreateNamespace creates a new Namespace. Returns error if could not be created // or another namespace already exists. func (h *Headscale) CreateNamespace(name string) (*Namespace, error) { + var err error namespace := Namespace{} + name, err = NormalizeNamespaceName(name) + if err != nil { + return nil, err + } if err := h.db.Where("name = ?", name).First(&namespace).Error; err == nil { return nil, errNamespaceExists } @@ -50,6 +61,10 @@ func (h *Headscale) CreateNamespace(name string) (*Namespace, error) { // DestroyNamespace destroys a Namespace. Returns error if the Namespace does // not exist or if there are machines associated with it. func (h *Headscale) DestroyNamespace(name string) error { + name, err := NormalizeNamespaceName(name) + if err != nil { + return err + } namespace, err := h.GetNamespace(name) if err != nil { return errNamespaceNotFound @@ -84,10 +99,15 @@ func (h *Headscale) DestroyNamespace(name string) error { // RenameNamespace renames a Namespace. Returns error if the Namespace does // not exist or if another Namespace exists with the new name. func (h *Headscale) RenameNamespace(oldName, newName string) error { + var err error oldNamespace, err := h.GetNamespace(oldName) if err != nil { return err } + newName, err = NormalizeNamespaceName(newName) + if err != nil { + return err + } _, err = h.GetNamespace(newName) if err == nil { return errNamespaceExists @@ -108,6 +128,10 @@ func (h *Headscale) RenameNamespace(oldName, newName string) error { // GetNamespace fetches a namespace by name. func (h *Headscale) GetNamespace(name string) (*Namespace, error) { namespace := Namespace{} + name, err := NormalizeNamespaceName(name) + if err != nil { + return nil, err + } if result := h.db.First(&namespace, "name = ?", name); errors.Is( result.Error, gorm.ErrRecordNotFound, @@ -130,6 +154,10 @@ func (h *Headscale) ListNamespaces() ([]Namespace, error) { // ListMachinesInNamespace gets all the nodes in a given namespace. func (h *Headscale) ListMachinesInNamespace(name string) ([]Machine, error) { + name, err := NormalizeNamespaceName(name) + if err != nil { + return nil, err + } namespace, err := h.GetNamespace(name) if err != nil { return nil, err @@ -145,6 +173,10 @@ func (h *Headscale) ListMachinesInNamespace(name string) ([]Machine, error) { // ListSharedMachinesInNamespace returns all the machines that are shared to the specified namespace. func (h *Headscale) ListSharedMachinesInNamespace(name string) ([]Machine, error) { + name, err := NormalizeNamespaceName(name) + if err != nil { + return nil, err + } namespace, err := h.GetNamespace(name) if err != nil { return nil, err @@ -170,6 +202,10 @@ func (h *Headscale) ListSharedMachinesInNamespace(name string) ([]Machine, error // SetMachineNamespace assigns a Machine to a namespace. func (h *Headscale) SetMachineNamespace(machine *Machine, namespaceName string) error { + namespaceName, err := NormalizeNamespaceName(namespaceName) + if err != nil { + return err + } namespace, err := h.GetNamespace(namespaceName) if err != nil { return err @@ -233,3 +269,24 @@ func (n *Namespace) toProto() *v1.Namespace { CreatedAt: timestamppb.New(n.CreatedAt), } } + +// NormalizeNamespaceName will replace forbidden chars in namespace +// it can also return an error if the namespace doesn't respect RFC 952 and 1123 +func NormalizeNamespaceName(name string) (string, error) { + name = strings.ToLower(name) + name = strings.ReplaceAll(name, "@", ".") + name = strings.ReplaceAll(name, "'", "") + name = normalizeNamespaceRegex.ReplaceAllString(name, "-") + + for _, elt := range strings.Split(name, ".") { + if len(elt) > 63 { + return "", fmt.Errorf( + "label %v is more than 63 chars: %w", + elt, + errInvalidNamespaceName, + ) + } + } + + return name, nil +} diff --git a/namespaces_test.go b/namespaces_test.go index d07deb96..cf2b323e 100644 --- a/namespaces_test.go +++ b/namespaces_test.go @@ -1,6 +1,8 @@ package headscale import ( + "testing" + "github.com/rs/zerolog/log" "gopkg.in/check.v1" "gorm.io/gorm" @@ -239,3 +241,62 @@ func (s *Suite) TestGetMapResponseUserProfiles(c *check.C) { } c.Assert(found, check.Equals, true) } + +func TestNormalizeNamespaceName(t *testing.T) { + type args struct { + name string + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "normalize simple name", + args: args{name: "normalize-simple.name"}, + want: "normalize-simple.name", + wantErr: false, + }, + { + name: "normalize an email", + args: args{name: "foo.bar@example.com"}, + want: "foo.bar.example.com", + wantErr: false, + }, + { + name: "normalize complex email", + args: args{name: "foo.bar+complex-email@example.com"}, + want: "foo.bar-complex-email.example.com", + wantErr: false, + }, + { + name: "namespace name with space", + args: args{name: "name space"}, + want: "name-space", + wantErr: false, + }, + { + name: "namespace with quote", + args: args{name: "Jamie's iPhone 5"}, + want: "jamies-iphone-5", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := NormalizeNamespaceName(tt.args.name) + if (err != nil) != tt.wantErr { + t.Errorf( + "NormalizeNamespaceName() error = %v, wantErr %v", + err, + tt.wantErr, + ) + return + } + if got != tt.want { + t.Errorf("NormalizeNamespaceName() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/oidc_test.go b/oidc_test.go deleted file mode 100644 index d50027a9..00000000 --- a/oidc_test.go +++ /dev/null @@ -1,180 +0,0 @@ -package headscale - -import ( - "sync" - "testing" - - "github.com/coreos/go-oidc/v3/oidc" - "github.com/patrickmn/go-cache" - "golang.org/x/oauth2" - "gorm.io/gorm" - "tailscale.com/tailcfg" - "tailscale.com/types/key" -) - -func TestHeadscale_getNamespaceFromEmail(t *testing.T) { - type fields struct { - cfg Config - db *gorm.DB - dbString string - dbType string - dbDebug bool - privateKey *key.MachinePrivate - aclPolicy *ACLPolicy - aclRules []tailcfg.FilterRule - lastStateChange sync.Map - oidcProvider *oidc.Provider - oauth2Config *oauth2.Config - oidcStateCache *cache.Cache - } - type args struct { - email string - } - tests := []struct { - name string - fields fields - args args - want string - want1 bool - }{ - { - name: "match all", - fields: fields{ - cfg: Config{ - OIDC: OIDCConfig{ - MatchMap: map[string]string{ - ".*": "space", - }, - }, - }, - }, - args: args{ - email: "test@example.no", - }, - want: "space", - want1: true, - }, - { - name: "match user", - fields: fields{ - cfg: Config{ - OIDC: OIDCConfig{ - MatchMap: map[string]string{ - "specific@user\\.no": "user-namespace", - }, - }, - }, - }, - args: args{ - email: "specific@user.no", - }, - want: "user-namespace", - want1: true, - }, - { - name: "match domain", - fields: fields{ - cfg: Config{ - OIDC: OIDCConfig{ - MatchMap: map[string]string{ - ".*@example\\.no": "example", - }, - }, - }, - }, - args: args{ - email: "test@example.no", - }, - want: "example", - want1: true, - }, - { - name: "multi match domain", - fields: fields{ - cfg: Config{ - OIDC: OIDCConfig{ - MatchMap: map[string]string{ - ".*@example\\.no": "exammple", - ".*@gmail\\.com": "gmail", - }, - }, - }, - }, - args: args{ - email: "someuser@gmail.com", - }, - want: "gmail", - want1: true, - }, - { - name: "no match domain", - fields: fields{ - cfg: Config{ - OIDC: OIDCConfig{ - MatchMap: map[string]string{ - ".*@dontknow.no": "never", - }, - }, - }, - }, - args: args{ - email: "test@wedontknow.no", - }, - want: "", - want1: false, - }, - { - name: "multi no match domain", - fields: fields{ - cfg: Config{ - OIDC: OIDCConfig{ - MatchMap: map[string]string{ - ".*@dontknow.no": "never", - ".*@wedontknow.no": "other", - ".*\\.no": "stuffy", - }, - }, - }, - }, - args: args{ - email: "tasy@nonofthem.com", - }, - want: "", - want1: false, - }, - } - //nolint - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - app := &Headscale{ - cfg: test.fields.cfg, - db: test.fields.db, - dbString: test.fields.dbString, - dbType: test.fields.dbType, - dbDebug: test.fields.dbDebug, - privateKey: test.fields.privateKey, - aclPolicy: test.fields.aclPolicy, - aclRules: test.fields.aclRules, - lastStateChange: test.fields.lastStateChange, - oidcProvider: test.fields.oidcProvider, - oauth2Config: test.fields.oauth2Config, - oidcStateCache: test.fields.oidcStateCache, - } - got, got1 := app.getNamespaceFromEmail(test.args.email) - if got != test.want { - t.Errorf( - "Headscale.getNamespaceFromEmail() got = %v, want %v", - got, - test.want, - ) - } - if got1 != test.want1 { - t.Errorf( - "Headscale.getNamespaceFromEmail() got1 = %v, want %v", - got1, - test.want1, - ) - } - }) - } -} From 0191ea93ffd83e367530ca8c3b4be78dc6f05dbc Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Tue, 22 Feb 2022 12:46:45 +0100 Subject: [PATCH 02/16] feat(oidc): bind email to namespace --- oidc.go | 178 +++++++++++++++++++++++++------------------------------- 1 file changed, 79 insertions(+), 99 deletions(-) diff --git a/oidc.go b/oidc.go index a47863ff..495832a5 100644 --- a/oidc.go +++ b/oidc.go @@ -9,7 +9,6 @@ import ( "fmt" "html/template" "net/http" - "regexp" "strings" "time" @@ -282,109 +281,90 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { now := time.Now().UTC() - if namespaceName, ok := h.getNamespaceFromEmail(claims.Email); ok { - // register the machine if it's new - if !machine.Registered { - log.Debug().Msg("Registering new machine after successful callback") - - namespace, err := h.GetNamespace(namespaceName) - if errors.Is(err, gorm.ErrRecordNotFound) { - 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 - } - - ips, err := h.getAvailableIPs() - if err != nil { - log.Error(). - Caller(). - Err(err). - Msg("could not get an IP from the pool") - ctx.String( - http.StatusInternalServerError, - "could not get an IP from the pool", - ) - - 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 - if err := oidcCallbackTemplate.Execute(&content, oidcCallbackTemplateConfig{ - User: claims.Email, - Verb: "Authenticated", - }); err != nil { - log.Error(). - Str("func", "OIDCCallback"). - Str("type", "authenticate"). - Err(err). - Msg("Could not render OIDC callback template") - ctx.Data( - http.StatusInternalServerError, - "text/html; charset=utf-8", - []byte("Could not render OIDC callback template"), - ) - } - - ctx.Data(http.StatusOK, "text/html; charset=utf-8", content.Bytes()) - + namespaceName, err := NormalizeNamespaceName(claims.Email) + if err != nil { + log.Error().Err(err).Caller().Msgf("couldn't normalize email") + ctx.String( + http.StatusInternalServerError, + "couldn't normalize email", + ) return } + // register the machine if it's new + if !machine.Registered { + log.Debug().Msg("Registering new machine after successful callback") - log.Error(). - Caller(). - Str("email", claims.Email). - Str("username", claims.Username). - Str("machine", machine.Name). - Msg("Email could not be mapped to a namespace") - ctx.String( - http.StatusBadRequest, - "email from claim could not be mapped to a namespace", - ) -} + namespace, err := h.GetNamespace(namespaceName) + if errors.Is(err, gorm.ErrRecordNotFound) { + namespace, err = h.CreateNamespace(namespaceName) -// getNamespaceFromEmail passes the users email through a list of "matchers" -// and iterates through them until it matches and returns a namespace. -// If no match is found, an empty string will be returned. -// TODO(kradalby): golang Maps key order is not stable, so this list is _not_ deterministic. Find a way to make the list of keys stable, preferably in the order presented in a users configuration. -func (h *Headscale) getNamespaceFromEmail(email string) (string, bool) { - for match, namespace := range h.cfg.OIDC.MatchMap { - regex := regexp.MustCompile(match) - if regex.MatchString(email) { - return namespace, true + 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 } + + ips, err := h.getAvailableIPs() + if err != nil { + log.Error(). + Caller(). + Err(err). + Msg("could not get an IP from the pool") + ctx.String( + http.StatusInternalServerError, + "could not get an IP from the pool", + ) + + return + } + + machine.IPAddresses = ips + machine.NamespaceID = namespace.ID + machine.Registered = true + machine.RegisterMethod = RegisterMethodOIDC + machine.LastSuccessfulUpdate = &now + machine.Expiry = &requestedTime + h.db.Save(&machine) } - return "", false + var content bytes.Buffer + if err := oidcCallbackTemplate.Execute(&content, oidcCallbackTemplateConfig{ + User: claims.Email, + Verb: "Authenticated", + }); err != nil { + log.Error(). + Str("func", "OIDCCallback"). + Str("type", "authenticate"). + Err(err). + Msg("Could not render OIDC callback template") + ctx.Data( + http.StatusInternalServerError, + "text/html; charset=utf-8", + []byte("Could not render OIDC callback template"), + ) + } + + ctx.Data(http.StatusOK, "text/html; charset=utf-8", content.Bytes()) + + return } From 717250adb3fc8cf6a37550292caf70e5f17979a7 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Tue, 22 Feb 2022 20:58:08 +0100 Subject: [PATCH 03/16] feat: removing matchmap from headscale --- app.go | 1 - cmd/headscale/cli/utils.go | 15 --------------- config-example.yaml | 4 ---- 3 files changed, 20 deletions(-) diff --git a/app.go b/app.go index ac350ecf..020f6fc2 100644 --- a/app.go +++ b/app.go @@ -110,7 +110,6 @@ type OIDCConfig struct { Issuer string ClientID string ClientSecret string - MatchMap map[string]string } type DERPConfig struct { diff --git a/cmd/headscale/cli/utils.go b/cmd/headscale/cli/utils.go index 85dcae71..f738ad37 100644 --- a/cmd/headscale/cli/utils.go +++ b/cmd/headscale/cli/utils.go @@ -10,7 +10,6 @@ import ( "net/url" "os" "path/filepath" - "regexp" "strconv" "strings" "time" @@ -356,8 +355,6 @@ func getHeadscaleApp() (*headscale.Headscale, error) { cfg := getHeadscaleConfig() - cfg.OIDC.MatchMap = loadOIDCMatchMap() - app, err := headscale.NewHeadscale(cfg) if err != nil { return nil, err @@ -514,18 +511,6 @@ func (tokenAuth) RequireTransportSecurity() bool { return true } -// loadOIDCMatchMap is a wrapper around viper to verifies that the keys in -// the match map is valid regex strings. -func loadOIDCMatchMap() map[string]string { - strMap := viper.GetStringMapString("oidc.domain_map") - - for oidcMatcher := range strMap { - _ = regexp.MustCompile(oidcMatcher) - } - - return strMap -} - func GetFileMode(key string) fs.FileMode { modeStr := viper.GetString(key) diff --git a/config-example.yaml b/config-example.yaml index ba0c653f..17f556b8 100644 --- a/config-example.yaml +++ b/config-example.yaml @@ -180,7 +180,3 @@ unix_socket_permission: "0770" # client_id: "your-oidc-client-id" # client_secret: "your-oidc-client-secret" # -# # Domain map is used to map incomming users (by their email) to -# # a namespace. The key can be a string, or regex. -# domain_map: -# ".*": default-namespace From afd4a3706ebe1d93f4ee9c4714df8de9bc7f4715 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Tue, 22 Feb 2022 21:05:39 +0100 Subject: [PATCH 04/16] chore: update formating --- namespaces.go | 9 +++++++-- namespaces_test.go | 1 + oidc.go | 3 +-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/namespaces.go b/namespaces.go index 8160b031..ec674b91 100644 --- a/namespaces.go +++ b/namespaces.go @@ -22,6 +22,11 @@ const ( errInvalidNamespaceName = Error("Invalid namespace name") ) +const ( + // value related to RFC 1123 and 952. + labelHostnameLength = 63 +) + var normalizeNamespaceRegex = regexp.MustCompile("[^a-z0-9-.]+") // Namespace is the way Headscale implements the concept of users in Tailscale @@ -271,7 +276,7 @@ func (n *Namespace) toProto() *v1.Namespace { } // NormalizeNamespaceName will replace forbidden chars in namespace -// it can also return an error if the namespace doesn't respect RFC 952 and 1123 +// it can also return an error if the namespace doesn't respect RFC 952 and 1123. func NormalizeNamespaceName(name string) (string, error) { name = strings.ToLower(name) name = strings.ReplaceAll(name, "@", ".") @@ -279,7 +284,7 @@ func NormalizeNamespaceName(name string) (string, error) { name = normalizeNamespaceRegex.ReplaceAllString(name, "-") for _, elt := range strings.Split(name, ".") { - if len(elt) > 63 { + if len(elt) > labelHostnameLength { return "", fmt.Errorf( "label %v is more than 63 chars: %w", elt, diff --git a/namespaces_test.go b/namespaces_test.go index cf2b323e..d3519f9e 100644 --- a/namespaces_test.go +++ b/namespaces_test.go @@ -292,6 +292,7 @@ func TestNormalizeNamespaceName(t *testing.T) { err, tt.wantErr, ) + return } if got != tt.want { diff --git a/oidc.go b/oidc.go index 495832a5..78caa641 100644 --- a/oidc.go +++ b/oidc.go @@ -288,6 +288,7 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { http.StatusInternalServerError, "couldn't normalize email", ) + return } // register the machine if it's new @@ -365,6 +366,4 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { } ctx.Data(http.StatusOK, "text/html; charset=utf-8", content.Bytes()) - - return } From fe0b43eaaff43fd2692aa3c3da5c1ea7b92d2311 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Tue, 22 Feb 2022 21:20:59 +0100 Subject: [PATCH 05/16] chore: update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c04a170..459d6c11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ This is a part of aligning `headscale`'s behaviour with Tailscale's upstream beh - Add API Key support - Enable remote control of `headscale` via CLI [docs](docs/remote-cli.md) - Enable HTTP API (beta, subject to change) +- OIDC login will map user to an associated namespace. **Changes**: @@ -35,6 +36,7 @@ This is a part of aligning `headscale`'s behaviour with Tailscale's upstream beh - Upgrade `tailscale` (1.20.4) and other dependencies to latest [#314](https://github.com/juanfont/headscale/pull/314) - fix swapped machine<->namespace labels in `/metrics` [#312](https://github.com/juanfont/headscale/pull/312) - remove key-value based update mechanism for namespace changes [#316](https://github.com/juanfont/headscale/pull/316) +- removal of the `oidc.domain_map` configuration parameter **0.12.4 (2022-01-29):** From 45727dbb214f934b4a71bd694fa64570a6a4cf2e Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Wed, 23 Feb 2022 11:07:24 +0100 Subject: [PATCH 06/16] feat(namespace): add check function for namespace --- namespaces.go | 30 ++++++++++++++++++++++++-- namespaces_test.go | 52 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/namespaces.go b/namespaces.go index ec674b91..5062cc5e 100644 --- a/namespaces.go +++ b/namespaces.go @@ -27,7 +27,7 @@ const ( labelHostnameLength = 63 ) -var normalizeNamespaceRegex = regexp.MustCompile("[^a-z0-9-.]+") +var invalidCharsInNamespaceRegex = regexp.MustCompile("[^a-z0-9-.]+") // Namespace is the way Headscale implements the concept of users in Tailscale // @@ -281,7 +281,7 @@ func NormalizeNamespaceName(name string) (string, error) { name = strings.ToLower(name) name = strings.ReplaceAll(name, "@", ".") name = strings.ReplaceAll(name, "'", "") - name = normalizeNamespaceRegex.ReplaceAllString(name, "-") + name = invalidCharsInNamespaceRegex.ReplaceAllString(name, "-") for _, elt := range strings.Split(name, ".") { if len(elt) > labelHostnameLength { @@ -295,3 +295,29 @@ func NormalizeNamespaceName(name string) (string, error) { return name, nil } + +func CheckNamespaceName(name string) error { + if len(name) > labelHostnameLength { + return fmt.Errorf( + "Namespace must not be over 63 chars. %v doesn't comply with this rule: %w", + name, + errInvalidNamespaceName, + ) + } + if strings.ToLower(name) != name { + return fmt.Errorf( + "Namespace name should be lowercase. %v doesn't comply with this rule: %w", + name, + errInvalidNamespaceName, + ) + } + if invalidCharsInNamespaceRegex.MatchString(name) { + return fmt.Errorf( + "Namespace name should only be composed of lowercase ASCII letters numbers, hyphen and dots. %v doesn't comply with theses rules: %w", + name, + errInvalidNamespaceName, + ) + } + + return nil +} diff --git a/namespaces_test.go b/namespaces_test.go index d3519f9e..df4c5b00 100644 --- a/namespaces_test.go +++ b/namespaces_test.go @@ -74,13 +74,13 @@ func (s *Suite) TestRenameNamespace(c *check.C) { c.Assert(err, check.IsNil) c.Assert(len(namespaces), check.Equals, 1) - err = app.RenameNamespace("test", "test_renamed") + err = app.RenameNamespace("test", "test-renamed") c.Assert(err, check.IsNil) _, err = app.GetNamespace("test") c.Assert(err, check.Equals, errNamespaceNotFound) - _, err = app.GetNamespace("test_renamed") + _, err = app.GetNamespace("test-renamed") c.Assert(err, check.IsNil) err = app.RenameNamespace("test_does_not_exit", "test") @@ -90,7 +90,7 @@ func (s *Suite) TestRenameNamespace(c *check.C) { c.Assert(err, check.IsNil) c.Assert(namespaceTest2.Name, check.Equals, "test2") - err = app.RenameNamespace("test2", "test_renamed") + err = app.RenameNamespace("test2", "test-renamed") c.Assert(err, check.Equals, errNamespaceExists) } @@ -301,3 +301,49 @@ func TestNormalizeNamespaceName(t *testing.T) { }) } } + +func TestCheckNamespaceName(t *testing.T) { + type args struct { + name string + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "valid: namespace", + args: args{name: "valid-namespace"}, + wantErr: false, + }, + { + name: "invalid: capitalized namespace", + args: args{name: "Invalid-CapItaLIzed-namespace"}, + wantErr: true, + }, + { + name: "invalid: email as namespace", + args: args{name: "foo.bar@example.com"}, + wantErr: true, + }, + { + name: "invalid: chars in namespace name", + args: args{name: "super-namespace+name"}, + wantErr: true, + }, + { + name: "invalid: too long name for namespace", + args: args{ + name: "super-long-namespace-name-that-should-be-a-little-more-than-63-chars", + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := CheckNamespaceName(tt.args.name); (err != nil) != tt.wantErr { + t.Errorf("CheckNamespaceName() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} From 995731a29c067890a7ddb7b1c3a2979f2e21272c Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Wed, 23 Feb 2022 11:13:37 +0100 Subject: [PATCH 07/16] fix(namespace): checknamespace name before actions I keep the check server side because it's better from a security point of view. --- namespaces.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/namespaces.go b/namespaces.go index 5062cc5e..f2765620 100644 --- a/namespaces.go +++ b/namespaces.go @@ -41,12 +41,11 @@ type Namespace struct { // CreateNamespace creates a new Namespace. Returns error if could not be created // or another namespace already exists. func (h *Headscale) CreateNamespace(name string) (*Namespace, error) { - var err error - namespace := Namespace{} - name, err = NormalizeNamespaceName(name) + err := CheckNamespaceName(name) if err != nil { return nil, err } + namespace := Namespace{} if err := h.db.Where("name = ?", name).First(&namespace).Error; err == nil { return nil, errNamespaceExists } @@ -66,7 +65,7 @@ func (h *Headscale) CreateNamespace(name string) (*Namespace, error) { // DestroyNamespace destroys a Namespace. Returns error if the Namespace does // not exist or if there are machines associated with it. func (h *Headscale) DestroyNamespace(name string) error { - name, err := NormalizeNamespaceName(name) + err := CheckNamespaceName(name) if err != nil { return err } @@ -109,7 +108,7 @@ func (h *Headscale) RenameNamespace(oldName, newName string) error { if err != nil { return err } - newName, err = NormalizeNamespaceName(newName) + err = CheckNamespaceName(newName) if err != nil { return err } @@ -132,11 +131,11 @@ func (h *Headscale) RenameNamespace(oldName, newName string) error { // GetNamespace fetches a namespace by name. func (h *Headscale) GetNamespace(name string) (*Namespace, error) { - namespace := Namespace{} - name, err := NormalizeNamespaceName(name) + err := CheckNamespaceName(name) if err != nil { return nil, err } + namespace := Namespace{} if result := h.db.First(&namespace, "name = ?", name); errors.Is( result.Error, gorm.ErrRecordNotFound, @@ -159,7 +158,7 @@ func (h *Headscale) ListNamespaces() ([]Namespace, error) { // ListMachinesInNamespace gets all the nodes in a given namespace. func (h *Headscale) ListMachinesInNamespace(name string) ([]Machine, error) { - name, err := NormalizeNamespaceName(name) + err := CheckNamespaceName(name) if err != nil { return nil, err } @@ -178,7 +177,7 @@ func (h *Headscale) ListMachinesInNamespace(name string) ([]Machine, error) { // ListSharedMachinesInNamespace returns all the machines that are shared to the specified namespace. func (h *Headscale) ListSharedMachinesInNamespace(name string) ([]Machine, error) { - name, err := NormalizeNamespaceName(name) + err := CheckNamespaceName(name) if err != nil { return nil, err } @@ -207,7 +206,7 @@ func (h *Headscale) ListSharedMachinesInNamespace(name string) ([]Machine, error // SetMachineNamespace assigns a Machine to a namespace. func (h *Headscale) SetMachineNamespace(machine *Machine, namespaceName string) error { - namespaceName, err := NormalizeNamespaceName(namespaceName) + err := CheckNamespaceName(namespaceName) if err != nil { return err } From fcdbe7c5109b83065c150338b143c13e611b4cc6 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Wed, 23 Feb 2022 11:38:20 +0100 Subject: [PATCH 08/16] fix(utils_test): fix namespace name --- utils_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils_test.go b/utils_test.go index feb44d5a..f1931608 100644 --- a/utils_test.go +++ b/utils_test.go @@ -20,7 +20,7 @@ func (s *Suite) TestGetUsedIps(c *check.C) { ips, err := app.getAvailableIPs() c.Assert(err, check.IsNil) - namespace, err := app.CreateNamespace("test_ip") + namespace, err := app.CreateNamespace("test-ip") c.Assert(err, check.IsNil) pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) @@ -142,7 +142,7 @@ func (s *Suite) TestGetAvailableIpMachineWithoutIP(c *check.C) { c.Assert(len(ips), check.Equals, 1) c.Assert(ips[0].String(), check.Equals, expected.String()) - namespace, err := app.CreateNamespace("test_ip") + namespace, err := app.CreateNamespace("test-ip") c.Assert(err, check.IsNil) pak, err := app.CreatePreAuthKey(namespace.Name, false, false, nil) From cef0a2b0b3d6bae1a0a58d74a08363a82e9c1060 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Wed, 23 Feb 2022 11:40:48 +0100 Subject: [PATCH 09/16] fix(namespaces_test): fix missing namespace name --- namespaces_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/namespaces_test.go b/namespaces_test.go index df4c5b00..6b8df2bb 100644 --- a/namespaces_test.go +++ b/namespaces_test.go @@ -83,7 +83,7 @@ func (s *Suite) TestRenameNamespace(c *check.C) { _, err = app.GetNamespace("test-renamed") c.Assert(err, check.IsNil) - err = app.RenameNamespace("test_does_not_exit", "test") + err = app.RenameNamespace("test-does-not-exit", "test") c.Assert(err, check.Equals, errNamespaceNotFound) namespaceTest2, err := app.CreateNamespace("test2") From 7e4709c13f79574620133d639498bff945c8423b Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Wed, 23 Feb 2022 13:35:57 +0100 Subject: [PATCH 10/16] fix(namespace): remove name validation for destroy and get --- namespaces.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/namespaces.go b/namespaces.go index f2765620..cf1efe56 100644 --- a/namespaces.go +++ b/namespaces.go @@ -65,10 +65,6 @@ func (h *Headscale) CreateNamespace(name string) (*Namespace, error) { // DestroyNamespace destroys a Namespace. Returns error if the Namespace does // not exist or if there are machines associated with it. func (h *Headscale) DestroyNamespace(name string) error { - err := CheckNamespaceName(name) - if err != nil { - return err - } namespace, err := h.GetNamespace(name) if err != nil { return errNamespaceNotFound @@ -131,10 +127,6 @@ func (h *Headscale) RenameNamespace(oldName, newName string) error { // GetNamespace fetches a namespace by name. func (h *Headscale) GetNamespace(name string) (*Namespace, error) { - err := CheckNamespaceName(name) - if err != nil { - return nil, err - } namespace := Namespace{} if result := h.db.First(&namespace, "name = ?", name); errors.Is( result.Error, From 4f1f235a2e1ce251ba11d86bc96a6c1ca72f087b Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Wed, 23 Feb 2022 14:03:07 +0100 Subject: [PATCH 11/16] feat: add strip_email_domain to normalization of namespace --- app.go | 7 +++--- cmd/headscale/cli/utils.go | 9 ++++--- config-example.yaml | 6 +++++ namespaces.go | 9 +++++-- namespaces_test.go | 49 ++++++++++++++++++++++++++++---------- oidc.go | 2 +- 6 files changed, 61 insertions(+), 21 deletions(-) diff --git a/app.go b/app.go index 020f6fc2..fa1f5a08 100644 --- a/app.go +++ b/app.go @@ -107,9 +107,10 @@ type Config struct { } type OIDCConfig struct { - Issuer string - ClientID string - ClientSecret string + Issuer string + ClientID string + ClientSecret string + StripEmaildomain bool } type DERPConfig struct { diff --git a/cmd/headscale/cli/utils.go b/cmd/headscale/cli/utils.go index f738ad37..d38bb260 100644 --- a/cmd/headscale/cli/utils.go +++ b/cmd/headscale/cli/utils.go @@ -63,6 +63,8 @@ func LoadConfig(path string) error { viper.SetDefault("cli.timeout", "5s") viper.SetDefault("cli.insecure", false) + viper.SetDefault("oidc.strip_email_domain", true) + if err := viper.ReadInConfig(); err != nil { return fmt.Errorf("fatal error reading config file: %w", err) } @@ -323,9 +325,10 @@ func getHeadscaleConfig() headscale.Config { UnixSocketPermission: GetFileMode("unix_socket_permission"), OIDC: headscale.OIDCConfig{ - Issuer: viper.GetString("oidc.issuer"), - ClientID: viper.GetString("oidc.client_id"), - ClientSecret: viper.GetString("oidc.client_secret"), + Issuer: viper.GetString("oidc.issuer"), + ClientID: viper.GetString("oidc.client_id"), + ClientSecret: viper.GetString("oidc.client_secret"), + StripEmaildomain: viper.GetBool("oidc.strip_email_domain"), }, CLI: headscale.CLIConfig{ diff --git a/config-example.yaml b/config-example.yaml index 17f556b8..71fdfaaf 100644 --- a/config-example.yaml +++ b/config-example.yaml @@ -180,3 +180,9 @@ unix_socket_permission: "0770" # client_id: "your-oidc-client-id" # client_secret: "your-oidc-client-secret" # +# If `strip_email_domain` is set to `true`, the domain part of the username email address will be removed. +# This will transform `first-name.last-name@example.com` to the namespace `first-name.last-name` +# If `strip_email_domain` is set to `false` the domain part will NOT be removed resulting to the following +# namespace: `first-name.last-name.example.com` +# +# strip_email_domain: true diff --git a/namespaces.go b/namespaces.go index cf1efe56..b02f7a75 100644 --- a/namespaces.go +++ b/namespaces.go @@ -268,10 +268,15 @@ func (n *Namespace) toProto() *v1.Namespace { // NormalizeNamespaceName will replace forbidden chars in namespace // it can also return an error if the namespace doesn't respect RFC 952 and 1123. -func NormalizeNamespaceName(name string) (string, error) { +func NormalizeNamespaceName(name string, stripEmailDomain bool) (string, error) { name = strings.ToLower(name) - name = strings.ReplaceAll(name, "@", ".") name = strings.ReplaceAll(name, "'", "") + if stripEmailDomain { + idx := strings.Index(name, "@") + name = name[:idx] + } else { + name = strings.ReplaceAll(name, "@", ".") + } name = invalidCharsInNamespaceRegex.ReplaceAllString(name, "-") for _, elt := range strings.Split(name, ".") { diff --git a/namespaces_test.go b/namespaces_test.go index 6b8df2bb..6fb572cc 100644 --- a/namespaces_test.go +++ b/namespaces_test.go @@ -244,7 +244,8 @@ func (s *Suite) TestGetMapResponseUserProfiles(c *check.C) { func TestNormalizeNamespaceName(t *testing.T) { type args struct { - name string + name string + stripEmailDomain bool } tests := []struct { name string @@ -253,39 +254,63 @@ func TestNormalizeNamespaceName(t *testing.T) { wantErr bool }{ { - name: "normalize simple name", - args: args{name: "normalize-simple.name"}, + name: "normalize simple name", + args: args{ + name: "normalize-simple.name", + stripEmailDomain: false, + }, want: "normalize-simple.name", wantErr: false, }, { - name: "normalize an email", - args: args{name: "foo.bar@example.com"}, + name: "normalize an email", + args: args{ + name: "foo.bar@example.com", + stripEmailDomain: false, + }, want: "foo.bar.example.com", wantErr: false, }, { - name: "normalize complex email", - args: args{name: "foo.bar+complex-email@example.com"}, + name: "normalize an email domain should be removed", + args: args{ + name: "foo.bar@example.com", + stripEmailDomain: true, + }, + want: "foo.bar", + wantErr: false, + }, + { + name: "normalize complex email", + args: args{ + name: "foo.bar+complex-email@example.com", + stripEmailDomain: false, + }, want: "foo.bar-complex-email.example.com", wantErr: false, }, { - name: "namespace name with space", - args: args{name: "name space"}, + name: "namespace name with space", + args: args{ + name: "name space", + stripEmailDomain: false, + }, want: "name-space", wantErr: false, }, { - name: "namespace with quote", - args: args{name: "Jamie's iPhone 5"}, + name: "namespace with quote", + args: args{ + name: "Jamie's iPhone 5", + stripEmailDomain: false, + }, want: "jamies-iphone-5", wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := NormalizeNamespaceName(tt.args.name) + got, err := NormalizeNamespaceName(tt.args.name, tt.args.stripEmailDomain) if (err != nil) != tt.wantErr { t.Errorf( "NormalizeNamespaceName() error = %v, wantErr %v", diff --git a/oidc.go b/oidc.go index 78caa641..2036c4dc 100644 --- a/oidc.go +++ b/oidc.go @@ -281,7 +281,7 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { now := time.Now().UTC() - namespaceName, err := NormalizeNamespaceName(claims.Email) + namespaceName, err := NormalizeNamespaceName(claims.Email, h.cfg.OIDC.StripEmaildomain) if err != nil { log.Error().Err(err).Caller().Msgf("couldn't normalize email") ctx.String( From 972bef1194c44091dfe694c1abbf77fc39b70c26 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Wed, 23 Feb 2022 14:21:46 +0100 Subject: [PATCH 12/16] feat: add length error if hostname too long --- machine.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/machine.go b/machine.go index 25edd1d1..4536ef3f 100644 --- a/machine.go +++ b/machine.go @@ -25,6 +25,11 @@ const ( 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") +) + +const ( + maxHostnameLength = 255 ) // Machine is a Headscale client. @@ -727,6 +732,13 @@ func (machine Machine) toNode( machine.Namespace.Name, baseDomain, ) + if len(hostname) > maxHostnameLength { + return nil, fmt.Errorf( + "hostname %q is too long it cannot except 255 ASCII chars: %w", + hostname, + errHostnameTooLong, + ) + } } else { hostname = machine.Name } From 046116656b325922a5c39d07398b343f3bfbe33f Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Wed, 23 Feb 2022 14:22:21 +0100 Subject: [PATCH 13/16] chore: update formatting --- oidc.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/oidc.go b/oidc.go index 2036c4dc..edea32a8 100644 --- a/oidc.go +++ b/oidc.go @@ -281,7 +281,10 @@ func (h *Headscale) OIDCCallback(ctx *gin.Context) { now := time.Now().UTC() - namespaceName, err := NormalizeNamespaceName(claims.Email, h.cfg.OIDC.StripEmaildomain) + namespaceName, err := NormalizeNamespaceName( + claims.Email, + h.cfg.OIDC.StripEmaildomain, + ) if err != nil { log.Error().Err(err).Caller().Msgf("couldn't normalize email") ctx.String( From ae6a20e4d93a7bd53404fd32cbd46e9728705669 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Wed, 23 Feb 2022 14:28:20 +0100 Subject: [PATCH 14/16] fix: add valid test identified by linter --- namespaces.go | 6 +++--- namespaces_test.go | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/namespaces.go b/namespaces.go index b02f7a75..e5c6448e 100644 --- a/namespaces.go +++ b/namespaces.go @@ -271,9 +271,9 @@ func (n *Namespace) toProto() *v1.Namespace { func NormalizeNamespaceName(name string, stripEmailDomain bool) (string, error) { name = strings.ToLower(name) name = strings.ReplaceAll(name, "'", "") - if stripEmailDomain { - idx := strings.Index(name, "@") - name = name[:idx] + atIdx := strings.Index(name, "@") + if stripEmailDomain && atIdx > 0 { + name = name[:atIdx] } else { name = strings.ReplaceAll(name, "@", ".") } diff --git a/namespaces_test.go b/namespaces_test.go index 6fb572cc..6cef5f5b 100644 --- a/namespaces_test.go +++ b/namespaces_test.go @@ -280,6 +280,15 @@ func TestNormalizeNamespaceName(t *testing.T) { want: "foo.bar", wantErr: false, }, + { + name: "strip enabled no email passed as argument", + args: args{ + name: "not-email-and-strip-enabled", + stripEmailDomain: true, + }, + want: "not-email-and-strip-enabled", + wantErr: false, + }, { name: "normalize complex email", args: args{ From f9ce32fe1a856ee423711c603566086413973586 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Thu, 24 Feb 2022 13:34:36 +0100 Subject: [PATCH 15/16] Update CHANGELOG.md Co-authored-by: Kristoffer Dalby --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a489421e..b66477c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,10 @@ This is a part of aligning `headscale`'s behaviour with Tailscale's upstream beh - Add API Key support - Enable remote control of `headscale` via CLI [docs](docs/remote-cli.md) - Enable HTTP API (beta, subject to change) -- OIDC login will map user to an associated namespace. +- OpenID Connect users will be mapped per namespaces + - Each user will get its own namespace, created if it does not exist + - `oidc.domain_map` option has been removed + - `strip_email_domain` option has been added (see [config-example.yaml](./config_example.yaml)) **Changes**: From 47e8442d915072366a080fc9c4fdd45b9c380be9 Mon Sep 17 00:00:00 2001 From: Adrien Raffin-Caboisse Date: Thu, 24 Feb 2022 13:34:48 +0100 Subject: [PATCH 16/16] Update CHANGELOG.md Co-authored-by: Kristoffer Dalby --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b66477c6..39e07773 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,6 @@ This is a part of aligning `headscale`'s behaviour with Tailscale's upstream beh - Upgrade `tailscale` (1.20.4) and other dependencies to latest [#314](https://github.com/juanfont/headscale/pull/314) - fix swapped machine<->namespace labels in `/metrics` [#312](https://github.com/juanfont/headscale/pull/312) - remove key-value based update mechanism for namespace changes [#316](https://github.com/juanfont/headscale/pull/316) -- removal of the `oidc.domain_map` configuration parameter **0.12.4 (2022-01-29):**