From 60880d1f0ab04e2cc75b3be02ce71ab96a877e3f Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 15 Mar 2019 13:49:50 -0700 Subject: [PATCH] Add domains and check emails properly. --- authority/provisioner/jwk_test.go | 8 ++-- authority/provisioner/oidc.go | 37 ++++++++++++++++- authority/provisioner/oidc_test.go | 61 +++++++++++++++++++++++------ authority/provisioner/utils_test.go | 6 +-- 4 files changed, 91 insertions(+), 21 deletions(-) diff --git a/authority/provisioner/jwk_test.go b/authority/provisioner/jwk_test.go index c714ded0..53260313 100644 --- a/authority/provisioner/jwk_test.go +++ b/authority/provisioner/jwk_test.go @@ -119,7 +119,7 @@ func TestJWK_Authorize(t *testing.T) { assert.FatalError(t, err) t2, err := generateSimpleToken(p2.Name, testAudiences[1], key2) assert.FatalError(t, err) - t3, err := generateToken("test.smallstep.com", p1.Name, testAudiences[0], []string{}, time.Now(), key1) + t3, err := generateToken("test.smallstep.com", p1.Name, testAudiences[0], "", []string{}, time.Now(), key1) assert.FatalError(t, err) // Invalid tokens @@ -142,13 +142,13 @@ func TestJWK_Authorize(t *testing.T) { // invalid signature failSig := t1[0 : len(t1)-2] // no subject - failSub, err := generateToken("", p1.Name, testAudiences[0], []string{"test.smallstep.com"}, time.Now(), key1) + failSub, err := generateToken("", p1.Name, testAudiences[0], "", []string{"test.smallstep.com"}, time.Now(), key1) assert.FatalError(t, err) // expired - failExp, err := generateToken("subject", p1.Name, testAudiences[0], []string{"test.smallstep.com"}, time.Now().Add(-360*time.Second), key1) + failExp, err := generateToken("subject", p1.Name, testAudiences[0], "", []string{"test.smallstep.com"}, time.Now().Add(-360*time.Second), key1) assert.FatalError(t, err) // not before - failNbf, err := generateToken("subject", p1.Name, testAudiences[0], []string{"test.smallstep.com"}, time.Now().Add(360*time.Second), key1) + failNbf, err := generateToken("subject", p1.Name, testAudiences[0], "", []string{"test.smallstep.com"}, time.Now().Add(360*time.Second), key1) assert.FatalError(t, err) // Remove encrypted key for p2 diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index e58f0294..99d0240d 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -4,6 +4,7 @@ import ( "crypto/x509" "encoding/json" "net/http" + "strings" "time" "github.com/pkg/errors" @@ -49,8 +50,9 @@ type OIDC struct { ClientID string `json:"clientID"` ClientSecret string `json:"clientSecret"` ConfigurationEndpoint string `json:"configurationEndpoint"` + Admins []string `json:"admins"` + Domains []string `json:"domains"` Claims *Claims `json:"claims,omitempty"` - Admins []string `json:"admins,omitempty"` configuration openIDConfiguration keyStore *keyStore } @@ -58,14 +60,22 @@ type OIDC struct { // IsAdmin returns true if the given email is in the Admins whitelist, false // otherwise. func (o *OIDC) IsAdmin(email string) bool { + email = sanitizeEmail(email) for _, e := range o.Admins { - if e == email { + if email == sanitizeEmail(e) { return true } } return false } +func sanitizeEmail(email string) string { + if i := strings.LastIndex(email, "@"); i >= 0 { + email = email[:i] + strings.ToLower(email[i:]) + } + return email +} + // GetID returns the provisioner unique identifier, the OIDC provisioner the // uses the clientID for this. func (o *OIDC) GetID() string { @@ -130,9 +140,32 @@ func (o *OIDC) ValidatePayload(p openIDPayload) error { }, time.Minute); err != nil { return errors.Wrap(err, "failed to validate payload") } + + // Validate azp if present if p.AuthorizedParty != "" && p.AuthorizedParty != o.ClientID { return errors.New("failed to validate payload: invalid azp") } + + // Enforce an email claim + if p.Email == "" { + return errors.New("failed to validate payload: email not found") + } + + // Validate domains (case-insensitive) + if !o.IsAdmin(p.Email) && len(o.Domains) > 0 { + email := sanitizeEmail(p.Email) + var found bool + for _, d := range o.Domains { + if strings.HasSuffix(email, "@"+strings.ToLower(d)) { + found = true + break + } + } + if !found { + return errors.New("failed to validate payload: email is not allowed") + } + } + return nil } diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index 2af64e15..eddf27fa 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -2,6 +2,7 @@ package provisioner import ( "crypto/x509" + "fmt" "strings" "testing" "time" @@ -72,6 +73,7 @@ func TestOIDC_Init(t *testing.T) { ConfigurationEndpoint string Claims *Claims Admins []string + Domains []string } type args struct { config Config @@ -82,14 +84,15 @@ func TestOIDC_Init(t *testing.T) { args args wantErr bool }{ - {"ok", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil}, args{config}, false}, - {"ok-admins", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, []string{"foo@smallstep.com"}}, args{config}, false}, - {"ok-no-secret", fields{"oidc", "name", "client-id", "", srv.URL + "/openid-configuration", nil, nil}, args{config}, false}, - {"no-name", fields{"oidc", "", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil}, args{config}, true}, - {"no-type", fields{"", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil}, args{config}, true}, - {"no-client-id", fields{"oidc", "name", "", "client-secret", srv.URL + "/openid-configuration", nil, nil}, args{config}, true}, - {"no-configuration", fields{"oidc", "name", "client-id", "client-secret", "", nil, nil}, args{config}, true}, - {"bad-configuration", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil}, args{config}, true}, + {"ok", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil, nil}, args{config}, false}, + {"ok-admins", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, []string{"foo@smallstep.com"}, nil}, args{config}, false}, + {"ok-domains", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil, []string{"smallstep.com"}}, args{config}, false}, + {"ok-no-secret", fields{"oidc", "name", "client-id", "", srv.URL + "/openid-configuration", nil, nil, nil}, args{config}, false}, + {"no-name", fields{"oidc", "", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil, nil}, args{config}, true}, + {"no-type", fields{"", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil, nil}, args{config}, true}, + {"no-client-id", fields{"oidc", "name", "", "client-secret", srv.URL + "/openid-configuration", nil, nil, nil}, args{config}, true}, + {"no-configuration", fields{"oidc", "name", "client-id", "client-secret", "", nil, nil, nil}, args{config}, true}, + {"bad-configuration", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, nil}, args{config}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -129,8 +132,9 @@ func TestOIDC_Authorize(t *testing.T) { assert.FatalError(t, err) p3, err := generateOIDC() assert.FatalError(t, err) - // Admin - p3.Admins = []string{"name@smallstep.com"} + // Admin + Domains + p3.Admins = []string{"name@smallstep.com", "root@example.com"} + p3.Domains = []string{"smallstep.com"} // Update configuration endpoints and initialize config := Config{Claims: globalProvisionerClaims} @@ -148,6 +152,15 @@ func TestOIDC_Authorize(t *testing.T) { t3, err := generateSimpleToken("the-issuer", p3.ClientID, &keys.Keys[0]) assert.FatalError(t, err) + // Admin email not in domains + okAdmin, err := generateToken("subject", "the-issuer", p3.ClientID, "root@example.com", []string{"test.smallstep.com"}, time.Now(), &keys.Keys[0]) + assert.FatalError(t, err) + // Invalid email + failEmail, err := generateToken("subject", "the-issuer", p3.ClientID, "", []string{}, time.Now(), &keys.Keys[0]) + assert.FatalError(t, err) + failDomain, err := generateToken("subject", "the-issuer", p3.ClientID, "name@example.com", []string{}, time.Now(), &keys.Keys[0]) + assert.FatalError(t, err) + // Invalid tokens parts := strings.Split(t1, ".") key, err := generateJSONWebKey() @@ -168,10 +181,10 @@ func TestOIDC_Authorize(t *testing.T) { // invalid signature failSig := t1[0 : len(t1)-2] // expired - failExp, err := generateToken("subject", "the-issuer", p1.ClientID, []string{}, time.Now().Add(-360*time.Second), &keys.Keys[0]) + failExp, err := generateToken("subject", "the-issuer", p1.ClientID, "name@smallstep.com", []string{}, time.Now().Add(-360*time.Second), &keys.Keys[0]) assert.FatalError(t, err) // not before - failNbf, err := generateToken("subject", "the-issuer", p1.ClientID, []string{}, time.Now().Add(360*time.Second), &keys.Keys[0]) + failNbf, err := generateToken("subject", "the-issuer", p1.ClientID, "name@smallstep.com", []string{}, time.Now().Add(360*time.Second), &keys.Keys[0]) assert.FatalError(t, err) type args struct { @@ -186,6 +199,9 @@ func TestOIDC_Authorize(t *testing.T) { {"ok1", p1, args{t1}, false}, {"ok2", p2, args{t2}, false}, {"admin", p3, args{t3}, false}, + {"admin", p3, args{okAdmin}, false}, + {"fail-email", p3, args{failEmail}, true}, + {"fail-domain", p3, args{failDomain}, true}, {"fail-key", p1, args{failKey}, true}, {"fail-token", p1, args{failTok}, true}, {"fail-claims", p1, args{failClaims}, true}, @@ -199,6 +215,7 @@ func TestOIDC_Authorize(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := tt.prov.Authorize(tt.args.token) if (err != nil) != tt.wantErr { + fmt.Println(tt) t.Errorf("OIDC.Authorize() error = %v, wantErr %v", err, tt.wantErr) return } @@ -288,3 +305,23 @@ func TestOIDC_AuthorizeRevoke(t *testing.T) { }) } } + +func Test_sanitizeEmail(t *testing.T) { + tests := []struct { + name string + email string + want string + }{ + {"equal", "name@smallstep.com", "name@smallstep.com"}, + {"domain-insensitive", "name@SMALLSTEP.COM", "name@smallstep.com"}, + {"local-sensitive", "NaMe@smallSTEP.CoM", "NaMe@smallstep.com"}, + {"multiple-@", "NaMe@NaMe@smallSTEP.CoM", "NaMe@NaMe@smallstep.com"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := sanitizeEmail(tt.email); got != tt.want { + t.Errorf("sanitizeEmail() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go index e90be240..e53b42d3 100644 --- a/authority/provisioner/utils_test.go +++ b/authority/provisioner/utils_test.go @@ -173,10 +173,10 @@ func generateCollection(nJWK, nOIDC int) (*Collection, error) { } func generateSimpleToken(iss, aud string, jwk *jose.JSONWebKey) (string, error) { - return generateToken("subject", iss, aud, []string{"test.smallstep.com"}, time.Now(), jwk) + return generateToken("subject", iss, aud, "name@smallstep.com", []string{"test.smallstep.com"}, time.Now(), jwk) } -func generateToken(sub, iss, aud string, sans []string, iat time.Time, jwk *jose.JSONWebKey) (string, error) { +func generateToken(sub, iss, aud string, email string, sans []string, iat time.Time, jwk *jose.JSONWebKey) (string, error) { sig, err := jose.NewSigner( jose.SigningKey{Algorithm: jose.ES256, Key: jwk.Key}, new(jose.SignerOptions).WithType("JWT").WithHeader("kid", jwk.KeyID), @@ -204,8 +204,8 @@ func generateToken(sub, iss, aud string, sans []string, iat time.Time, jwk *jose Expiry: jose.NewNumericDate(iat.Add(5 * time.Minute)), Audience: []string{aud}, }, + Email: email, SANS: sans, - Email: "name@smallstep.com", } return jose.Signed(sig).Claims(claims).CompactSerialize() }