From 7a03c43fe2ec87a2928198b9fe17b17b19934d8a Mon Sep 17 00:00:00 2001 From: Raal Goff Date: Mon, 5 Sep 2022 12:43:32 +0800 Subject: [PATCH 1/2] allow missing Email claim in OIDC tokens, use subject when its missing --- authority/provisioner/oidc.go | 61 ++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index c1bcc741..bb3745b7 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -376,31 +376,46 @@ func (o *OIDC) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, "oidc.AuthorizeSSHSign") } - // Enforce an email claim + + if claims.Subject == "" { + return nil, errs.Unauthorized("oidc.AuthorizeSSHSign: failed to validate oidc token payload: subject not found") + } + + var data sshutil.TemplateData + var principals []string + if claims.Email == "" { - return nil, errs.Unauthorized("oidc.AuthorizeSSHSign: failed to validate oidc token payload: email not found") - } + // If email is empty, use the Subject claim instead to create minimal data for the template to use + data = sshutil.CreateTemplateData(sshutil.UserCert, claims.Subject, nil) + if v, err := unsafeParseSigned(token); err == nil { + data.SetToken(v) + } - // Get the identity using either the default identityFunc or one injected - // externally. Note that the PreferredUsername might be empty. - // TBD: Would preferred_username present a safety issue here? - iden, err := o.ctl.GetIdentity(ctx, claims.Email) - if err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, "oidc.AuthorizeSSHSign") - } + principals = nil + } else { + // Get the identity using either the default identityFunc or one injected + // externally. Note that the PreferredUsername might be empty. + // TBD: Would preferred_username present a safety issue here? + iden, err := o.ctl.GetIdentity(ctx, claims.Email) + if err != nil { + return nil, errs.Wrap(http.StatusInternalServerError, err, "oidc.AuthorizeSSHSign") + } - // Certificate templates. - data := sshutil.CreateTemplateData(sshutil.UserCert, claims.Email, iden.Usernames) - if v, err := unsafeParseSigned(token); err == nil { - data.SetToken(v) - } - // Add custom extensions added in the identity function. - for k, v := range iden.Permissions.Extensions { - data.AddExtension(k, v) - } - // Add custom critical options added in the identity function. - for k, v := range iden.Permissions.CriticalOptions { - data.AddCriticalOption(k, v) + // Certificate templates. + data = sshutil.CreateTemplateData(sshutil.UserCert, claims.Email, iden.Usernames) + if v, err := unsafeParseSigned(token); err == nil { + data.SetToken(v) + } + // Add custom extensions added in the identity function. + for k, v := range iden.Permissions.Extensions { + data.AddExtension(k, v) + } + // Add custom critical options added in the identity function. + for k, v := range iden.Permissions.CriticalOptions { + data.AddCriticalOption(k, v) + } + + principals = iden.Usernames } // Use the default template unless no-templates are configured and email is @@ -429,7 +444,7 @@ func (o *OIDC) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption } else { signOptions = append(signOptions, sshCertOptionsValidator(SignSSHOptions{ CertType: SSHUserCert, - Principals: iden.Usernames, + Principals: principals, })) } From b89f2104693dff7321dee04a4cc7106bf2885abe Mon Sep 17 00:00:00 2001 From: Raal Goff Date: Wed, 7 Sep 2022 07:45:27 +0800 Subject: [PATCH 2/2] remove fail-email test and add ok-empty-email test --- authority/provisioner/oidc_test.go | 9 +++++++-- authority/provisioner/ssh_test.go | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index 62ea3f24..7f80315f 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -523,7 +523,12 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) { okAdmin, err := generateOIDCToken("subject", "the-issuer", p3.ClientID, "root@example.com", "", time.Now(), &keys.Keys[0]) assert.FatalError(t, err) // Empty email - failEmail, err := generateToken("subject", "the-issuer", p3.ClientID, "", []string{}, time.Now(), &keys.Keys[0]) + emptyEmail, err := generateToken("subject", "the-issuer", p1.ClientID, "", []string{}, time.Now(), &keys.Keys[0]) + expectemptyEmailOptions := &SignSSHOptions{ + CertType: "user", + Principals: []string{}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(p1.ctl.Claimer.DefaultUserSSHCertDuration())), + } assert.FatalError(t, err) key, err := generateJSONWebKey() @@ -571,6 +576,7 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) { {"ok", p1, args{t1, SignSSHOptions{}, pub}, expectedUserOptions, http.StatusOK, false, false}, {"ok-rsa2048", p1, args{t1, SignSSHOptions{}, rsa2048.Public()}, expectedUserOptions, http.StatusOK, false, false}, {"ok-user", p1, args{t1, SignSSHOptions{CertType: "user"}, pub}, expectedUserOptions, http.StatusOK, false, false}, + {"ok-empty-email", p1, args{emptyEmail, SignSSHOptions{CertType: "user"}, pub}, expectemptyEmailOptions, http.StatusOK, false, false}, {"ok-principals", p1, args{t1, SignSSHOptions{Principals: []string{"name"}}, pub}, &SignSSHOptions{CertType: "user", Principals: []string{"name", "name@smallstep.com"}, ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, http.StatusOK, false, false}, @@ -593,7 +599,6 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) { {"fail-rsa1024", p1, args{t1, SignSSHOptions{}, rsa1024.Public()}, expectedUserOptions, http.StatusOK, false, true}, {"fail-user-host", p1, args{t1, SignSSHOptions{CertType: "host"}, pub}, nil, http.StatusOK, false, true}, {"fail-user-principals", p1, args{t1, SignSSHOptions{Principals: []string{"root"}}, pub}, nil, http.StatusOK, false, true}, - {"fail-email", p3, args{failEmail, SignSSHOptions{}, pub}, nil, http.StatusUnauthorized, true, false}, {"fail-getIdentity", p5, args{failGetIdentityToken, SignSSHOptions{}, pub}, nil, http.StatusInternalServerError, true, false}, {"fail-sshCA-disabled", p6, args{"foo", SignSSHOptions{}, pub}, nil, http.StatusUnauthorized, true, false}, // Missing parametrs diff --git a/authority/provisioner/ssh_test.go b/authority/provisioner/ssh_test.go index 90271443..b86945a3 100644 --- a/authority/provisioner/ssh_test.go +++ b/authority/provisioner/ssh_test.go @@ -20,7 +20,7 @@ func validateSSHCertificate(cert *ssh.Certificate, opts *SignSSHOptions) error { return fmt.Errorf("certificate signature is nil") case cert.SignatureKey == nil: return fmt.Errorf("certificate signature is nil") - case !reflect.DeepEqual(cert.ValidPrincipals, opts.Principals): + case !reflect.DeepEqual(cert.ValidPrincipals, opts.Principals) && (len(opts.Principals) > 0 || len(cert.ValidPrincipals) > 0): return fmt.Errorf("certificate principals are not equal, want %v, got %v", opts.Principals, cert.ValidPrincipals) case cert.CertType != ssh.UserCert && cert.CertType != ssh.HostCert: return fmt.Errorf("certificate type %v is not valid", cert.CertType)