From e8af03cd36c4de3bab4bbe06a45e6a3fc68b3026 Mon Sep 17 00:00:00 2001 From: adantop Date: Thu, 21 Sep 2023 11:55:38 -0600 Subject: [PATCH 1/4] Allow User Certs for Service Accounts in the GCP provisioner adding tests linting refactor to generate just the sign options fix linting and adding toggle for user and host certs resolving linting error --- api/ssh.go | 1 + authority/provisioner/gcp.go | 88 ++++++++++++++++++++++++------- authority/provisioner/gcp_test.go | 22 ++++++-- authority/provisioner/method.go | 14 +++++ 4 files changed, 103 insertions(+), 22 deletions(-) diff --git a/api/ssh.go b/api/ssh.go index 08294c71c..41dd6673b 100644 --- a/api/ssh.go +++ b/api/ssh.go @@ -289,6 +289,7 @@ func SSHSign(w http.ResponseWriter, r *http.Request) { ctx := provisioner.NewContextWithMethod(r.Context(), provisioner.SSHSignMethod) ctx = provisioner.NewContextWithToken(ctx, body.OTT) + ctx = provisioner.NewContextWithCertType(ctx, opts.CertType) a := mustAuthority(ctx) signOpts, err := a.Authorize(ctx, body.OTT) diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go index 2296b1b06..0bb9d3fa9 100644 --- a/authority/provisioner/gcp.go +++ b/authority/provisioner/gcp.go @@ -89,6 +89,8 @@ type GCP struct { ProjectIDs []string `json:"projectIDs"` DisableCustomSANs bool `json:"disableCustomSANs"` DisableTrustOnFirstUse bool `json:"disableTrustOnFirstUse"` + EnableSSHCAUser bool `json:"enableSSHCAUser"` + DisableSSHCAHost bool `json:"disableSSHCAHost"` InstanceAge Duration `json:"instanceAge,omitempty"` Claims *Claims `json:"claims,omitempty"` Options *Options `json:"options,omitempty"` @@ -387,31 +389,41 @@ func (p *GCP) authorizeToken(token string) (*gcpPayload, error) { } // AuthorizeSSHSign returns the list of SignOption for a SignSSH request. -func (p *GCP) AuthorizeSSHSign(_ context.Context, token string) ([]SignOption, error) { - if !p.ctl.Claimer.IsSSHCAEnabled() { - return nil, errs.Unauthorized("gcp.AuthorizeSSHSign; sshCA is disabled for gcp provisioner '%s'", p.GetName()) +func (p *GCP) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption, error) { + certType, hasCertType := CertTypeFromContext(ctx) + if !hasCertType { + certType = SSHHostCert } + + err := p.isUnauthorizedToIssueSSHCert(certType) + if err != nil { + return nil, err + } + claims, err := p.authorizeToken(token) if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, "gcp.AuthorizeSSHSign") } - ce := claims.Google.ComputeEngine - signOptions := []SignOption{} + var principals []string + var keyID string + var defaults SignSSHOptions + var ct sshutil.CertType + var template string - // Enforce host certificate. - defaults := SignSSHOptions{ - CertType: SSHHostCert, + switch certType { + case SSHHostCert: + defaults, keyID, principals, ct, template = p.genHostOptions(ctx, claims) + case SSHUserCert: + defaults, keyID, principals, ct, template = p.genUserOptions(ctx, claims) + default: + return nil, errs.Unauthorized("gcp.AuthorizeSSHSign; invalid requested certType") } - // Validated principals. - principals := []string{ - fmt.Sprintf("%s.c.%s.internal", ce.InstanceName, ce.ProjectID), - fmt.Sprintf("%s.%s.c.%s.internal", ce.InstanceName, ce.Zone, ce.ProjectID), - } + signOptions := []SignOption{} - // Only enforce known principals if disable custom sans is true. - if p.DisableCustomSANs { + // Only enforce known principals if disable custom sans is true, or it is a user cert request + if p.DisableCustomSANs || certType == SSHUserCert { defaults.Principals = principals } else { // Check that at least one principal is sent in the request. @@ -421,12 +433,12 @@ func (p *GCP) AuthorizeSSHSign(_ context.Context, token string) ([]SignOption, e } // Certificate templates. - data := sshutil.CreateTemplateData(sshutil.HostCert, ce.InstanceName, principals) + data := sshutil.CreateTemplateData(ct, keyID, principals) if v, err := unsafeParseSigned(token); err == nil { data.SetToken(v) } - templateOptions, err := CustomSSHTemplateOptions(p.Options, data, sshutil.DefaultIIDTemplate) + templateOptions, err := CustomSSHTemplateOptions(p.Options, data, template) if err != nil { return nil, errs.Wrap(http.StatusInternalServerError, err, "gcp.AuthorizeSSHSign") } @@ -445,12 +457,50 @@ func (p *GCP) AuthorizeSSHSign(_ context.Context, token string) ([]SignOption, e // Require all the fields in the SSH certificate &sshCertDefaultValidator{}, // Ensure that all principal names are allowed - newSSHNamePolicyValidator(p.ctl.getPolicy().getSSHHost(), nil), + newSSHNamePolicyValidator(p.ctl.getPolicy().getSSHHost(), p.ctl.getPolicy().getSSHUser()), // Call webhooks p.ctl.newWebhookController( data, linkedca.Webhook_SSH, - webhook.WithAuthorizationPrincipal(ce.InstanceID), + webhook.WithAuthorizationPrincipal(keyID), ), ), nil } + +func (p *GCP) genHostOptions(_ context.Context, claims *gcpPayload) (SignSSHOptions, string, []string, sshutil.CertType, string) { + ce := claims.Google.ComputeEngine + keyID := ce.InstanceName + + principals := []string{ + fmt.Sprintf("%s.c.%s.internal", ce.InstanceName, ce.ProjectID), + fmt.Sprintf("%s.%s.c.%s.internal", ce.InstanceName, ce.Zone, ce.ProjectID), + } + + return SignSSHOptions{CertType: SSHHostCert}, keyID, principals, sshutil.HostCert, sshutil.DefaultIIDTemplate +} + +func (p *GCP) genUserOptions(_ context.Context, claims *gcpPayload) (SignSSHOptions, string, []string, sshutil.CertType, string) { + keyID := claims.Email + principals := []string{ + SanitizeSSHUserPrincipal(claims.Email), + claims.Email, + } + + return SignSSHOptions{CertType: SSHUserCert}, keyID, principals, sshutil.UserCert, sshutil.DefaultTemplate +} + +func (p *GCP) isUnauthorizedToIssueSSHCert(certType string) error { + if !p.ctl.Claimer.IsSSHCAEnabled() { + return errs.Unauthorized("gcp.AuthorizeSSHSign; sshCA is disabled for gcp provisioner '%s'", p.GetName()) + } + + if certType == SSHHostCert && p.DisableSSHCAHost { + return errs.Unauthorized("gcp.AuthorizeSSHSign; sshCA for Hosts is disabled for gcp provisioner '%s'", p.GetName()) + } + + if certType == SSHUserCert && !p.EnableSSHCAUser { + return errs.Unauthorized("gcp.AuthorizeSSHSign; sshCA for Users is disabled for gcp provisioner '%s'", p.GetName()) + } + + return nil +} diff --git a/authority/provisioner/gcp_test.go b/authority/provisioner/gcp_test.go index ef791614e..98f53b66d 100644 --- a/authority/provisioner/gcp_test.go +++ b/authority/provisioner/gcp_test.go @@ -592,6 +592,7 @@ func TestGCP_AuthorizeSSHSign(t *testing.T) { p1, err := generateGCP() assert.FatalError(t, err) p1.DisableCustomSANs = true + p1.EnableSSHCAUser = true p2, err := generateGCP() assert.FatalError(t, err) @@ -605,6 +606,10 @@ func TestGCP_AuthorizeSSHSign(t *testing.T) { p3.ctl.Claimer, err = NewClaimer(p3.Claims, globalProvisionerClaims) assert.FatalError(t, err) + p4, err := generateGCP() + assert.FatalError(t, err) + p4.DisableSSHCAHost = true + t1, err := generateGCPToken(p1.ServiceAccounts[0], "https://accounts.google.com", p1.GetID(), "instance-id", "instance-name", "project-id", "zone", @@ -647,6 +652,10 @@ func TestGCP_AuthorizeSSHSign(t *testing.T) { CertType: "host", Principals: []string{"foo.bar", "bar.foo"}, ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), } + expectedUserOptions := &SignSSHOptions{ + CertType: "user", Principals: []string{"foo", "foo@developer.gserviceaccount.com"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(p1.ctl.Claimer.DefaultUserSSHCertDuration())), + } type args struct { token string @@ -664,22 +673,29 @@ func TestGCP_AuthorizeSSHSign(t *testing.T) { }{ {"ok", p1, args{t1, SignSSHOptions{}, pub}, expectedHostOptions, http.StatusOK, false, false}, {"ok-rsa2048", p1, args{t1, SignSSHOptions{}, rsa2048.Public()}, expectedHostOptions, http.StatusOK, false, false}, - {"ok-type", p1, args{t1, SignSSHOptions{CertType: "host"}, pub}, expectedHostOptions, http.StatusOK, false, false}, + {"ok-type-host", p1, args{t1, SignSSHOptions{CertType: "host"}, pub}, expectedHostOptions, http.StatusOK, false, false}, + {"ok-type-user", p1, args{t1, SignSSHOptions{CertType: "user"}, pub}, expectedUserOptions, http.StatusOK, false, false}, {"ok-principals", p1, args{t1, SignSSHOptions{Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"}}, pub}, expectedHostOptions, http.StatusOK, false, false}, {"ok-principal1", p1, args{t1, SignSSHOptions{Principals: []string{"instance-name.c.project-id.internal"}}, pub}, expectedHostOptionsPrincipal1, http.StatusOK, false, false}, {"ok-principal2", p1, args{t1, SignSSHOptions{Principals: []string{"instance-name.zone.c.project-id.internal"}}, pub}, expectedHostOptionsPrincipal2, http.StatusOK, false, false}, {"ok-options", p1, args{t1, SignSSHOptions{CertType: "host", Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"}}, pub}, expectedHostOptions, http.StatusOK, false, false}, {"ok-custom", p2, args{t2, SignSSHOptions{Principals: []string{"foo.bar", "bar.foo"}}, pub}, expectedCustomOptions, http.StatusOK, false, false}, {"fail-rsa1024", p1, args{t1, SignSSHOptions{}, rsa1024.Public()}, expectedHostOptions, http.StatusOK, false, true}, - {"fail-type", p1, args{t1, SignSSHOptions{CertType: "user"}, pub}, nil, http.StatusOK, false, true}, {"fail-principal", p1, args{t1, SignSSHOptions{Principals: []string{"smallstep.com"}}, pub}, nil, http.StatusOK, false, true}, {"fail-extra-principal", p1, args{t1, SignSSHOptions{Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal", "smallstep.com"}}, pub}, nil, http.StatusOK, false, true}, {"fail-sshCA-disabled", p3, args{"foo", SignSSHOptions{}, pub}, expectedHostOptions, http.StatusUnauthorized, true, false}, + {"fail-type-host", p4, args{"foo", SignSSHOptions{CertType: "host"}, pub}, nil, http.StatusUnauthorized, true, false}, + {"fail-type-user", p4, args{"foo", SignSSHOptions{CertType: "host"}, pub}, nil, http.StatusUnauthorized, true, false}, {"fail-invalid-token", p1, args{"foo", SignSSHOptions{}, pub}, expectedHostOptions, http.StatusUnauthorized, true, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := tt.gcp.AuthorizeSSHSign(context.Background(), tt.args.token) + ctx := context.Background() + if tt.args.sshOpts.CertType == SSHUserCert { + ctx = NewContextWithCertType(ctx, SSHUserCert) + } + + got, err := tt.gcp.AuthorizeSSHSign(ctx, tt.args.token) if (err != nil) != tt.wantErr { t.Errorf("GCP.AuthorizeSSHSign() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/authority/provisioner/method.go b/authority/provisioner/method.go index 19aa6224f..d01ce12c5 100644 --- a/authority/provisioner/method.go +++ b/authority/provisioner/method.go @@ -78,3 +78,17 @@ func TokenFromContext(ctx context.Context) (string, bool) { token, ok := ctx.Value(tokenKey{}).(string) return token, ok } + +// The key to save the certTypeKey in the context. +type certTypeKey struct{} + +// NewContextWithCertType creates a new context with the given CertType. +func NewContextWithCertType(ctx context.Context, certType string) context.Context { + return context.WithValue(ctx, certTypeKey{}, certType) +} + +// CertTypeFromContext returns the certType stored in the given context. +func CertTypeFromContext(ctx context.Context) (string, bool) { + certType, ok := ctx.Value(certTypeKey{}).(string) + return certType, ok +} From 0a43b55d5309b26208ee433c8c2b4ac849a1a8f7 Mon Sep 17 00:00:00 2001 From: adantop Date: Wed, 8 May 2024 09:43:11 -0600 Subject: [PATCH 2/4] Adding test for new attribute defaults --- authority/provisioner/gcp_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/authority/provisioner/gcp_test.go b/authority/provisioner/gcp_test.go index 98f53b66d..d391e45db 100644 --- a/authority/provisioner/gcp_test.go +++ b/authority/provisioner/gcp_test.go @@ -186,6 +186,7 @@ func TestGCP_Init(t *testing.T) { args args wantErr bool }{ + {"ok", fields{"GCP", "name", nil, zero, nil}, args{config, srv.URL}, false}, {"ok", fields{"GCP", "name", nil, zero, nil}, args{config, srv.URL}, false}, {"ok", fields{"GCP", "name", []string{"service-account"}, zero, nil}, args{config, srv.URL}, false}, {"ok", fields{"GCP", "name", []string{"service-account"}, Duration{Duration: 1 * time.Minute}, nil}, args{config, srv.URL}, false}, @@ -211,6 +212,14 @@ func TestGCP_Init(t *testing.T) { if err := p.Init(tt.args.config); (err != nil) != tt.wantErr { t.Errorf("GCP.Init() error = %v, wantErr %v", err, tt.wantErr) } + + if p.EnableSSHCAUser { + t.Errorf("By default EnableSSHCAUser should be false") + } + + if p.DisableSSHCAHost { + t.Errorf("By default DisableSSHCAHost should be false") + } }) } } From 3ab951f963018576b1c530d16983a0ea3e7eb2d7 Mon Sep 17 00:00:00 2001 From: adantop Date: Thu, 9 May 2024 15:00:14 -0600 Subject: [PATCH 3/4] Replace sanitized username with google pattern service account usernames --- authority/provisioner/gcp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go index 0bb9d3fa9..b7b0af751 100644 --- a/authority/provisioner/gcp.go +++ b/authority/provisioner/gcp.go @@ -482,7 +482,7 @@ func (p *GCP) genHostOptions(_ context.Context, claims *gcpPayload) (SignSSHOpti func (p *GCP) genUserOptions(_ context.Context, claims *gcpPayload) (SignSSHOptions, string, []string, sshutil.CertType, string) { keyID := claims.Email principals := []string{ - SanitizeSSHUserPrincipal(claims.Email), + fmt.Sprintf("sa_%v", claims.Subject), claims.Email, } From bedb040659414defc5bdf7f6bfadfa78d3d37927 Mon Sep 17 00:00:00 2001 From: adantop Date: Thu, 30 May 2024 13:54:03 -0600 Subject: [PATCH 4/4] Updating options to be Disable* and of type *bool --- authority/provisioner/gcp.go | 28 +++++++++++++++++++++++----- authority/provisioner/gcp_test.go | 16 ++++++++++------ authority/provisioner/utils_test.go | 12 +++++++----- 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go index b7b0af751..8f6211d3a 100644 --- a/authority/provisioner/gcp.go +++ b/authority/provisioner/gcp.go @@ -30,6 +30,12 @@ const gcpCertsURL = "https://www.googleapis.com/oauth2/v3/certs" // gcpIdentityURL is the base url for the identity document in GCP. const gcpIdentityURL = "http://metadata/computeMetadata/v1/instance/service-accounts/default/identity" +// DefaultDisableSSHCAHost is the default value for SSH Host CA used when DisableSSHCAHost is not set +var DefaultDisableSSHCAHost = false + +// DefaultDisableSSHCAUser is the default value for SSH User CA used when DisableSSHCAUser is not set +var DefaultDisableSSHCAUser = true + // gcpPayload extends jwt.Claims with custom GCP attributes. type gcpPayload struct { jose.Claims @@ -89,8 +95,8 @@ type GCP struct { ProjectIDs []string `json:"projectIDs"` DisableCustomSANs bool `json:"disableCustomSANs"` DisableTrustOnFirstUse bool `json:"disableTrustOnFirstUse"` - EnableSSHCAUser bool `json:"enableSSHCAUser"` - DisableSSHCAHost bool `json:"disableSSHCAHost"` + DisableSSHCAUser *bool `json:"disableSSHCAUser,omitempty"` + DisableSSHCAHost *bool `json:"disableSSHCAHost,omitempty"` InstanceAge Duration `json:"instanceAge,omitempty"` Claims *Claims `json:"claims,omitempty"` Options *Options `json:"options,omitempty"` @@ -201,6 +207,14 @@ func (p *GCP) GetIdentityToken(subject, caURL string) (string, error) { // Init validates and initializes the GCP provisioner. func (p *GCP) Init(config Config) (err error) { + if p.DisableSSHCAHost == nil { + p.DisableSSHCAHost = &DefaultDisableSSHCAHost + } + + if p.DisableSSHCAUser == nil { + p.DisableSSHCAUser = &DefaultDisableSSHCAUser + } + switch { case p.Type == "": return errors.New("provisioner type cannot be empty") @@ -479,10 +493,14 @@ func (p *GCP) genHostOptions(_ context.Context, claims *gcpPayload) (SignSSHOpti return SignSSHOptions{CertType: SSHHostCert}, keyID, principals, sshutil.HostCert, sshutil.DefaultIIDTemplate } +func FormatServiceAccountUsername(serviceAccountId string) string { + return fmt.Sprintf("sa_%v", serviceAccountId) +} + func (p *GCP) genUserOptions(_ context.Context, claims *gcpPayload) (SignSSHOptions, string, []string, sshutil.CertType, string) { keyID := claims.Email principals := []string{ - fmt.Sprintf("sa_%v", claims.Subject), + FormatServiceAccountUsername(claims.Subject), claims.Email, } @@ -494,11 +512,11 @@ func (p *GCP) isUnauthorizedToIssueSSHCert(certType string) error { return errs.Unauthorized("gcp.AuthorizeSSHSign; sshCA is disabled for gcp provisioner '%s'", p.GetName()) } - if certType == SSHHostCert && p.DisableSSHCAHost { + if certType == SSHHostCert && *p.DisableSSHCAHost { return errs.Unauthorized("gcp.AuthorizeSSHSign; sshCA for Hosts is disabled for gcp provisioner '%s'", p.GetName()) } - if certType == SSHUserCert && !p.EnableSSHCAUser { + if certType == SSHUserCert && *p.DisableSSHCAUser { return errs.Unauthorized("gcp.AuthorizeSSHSign; sshCA for Users is disabled for gcp provisioner '%s'", p.GetName()) } diff --git a/authority/provisioner/gcp_test.go b/authority/provisioner/gcp_test.go index d391e45db..5f5587a44 100644 --- a/authority/provisioner/gcp_test.go +++ b/authority/provisioner/gcp_test.go @@ -213,11 +213,11 @@ func TestGCP_Init(t *testing.T) { t.Errorf("GCP.Init() error = %v, wantErr %v", err, tt.wantErr) } - if p.EnableSSHCAUser { - t.Errorf("By default EnableSSHCAUser should be false") + if *p.DisableSSHCAUser != true { + t.Errorf("By default DisableSSHCAUser should be true") } - if p.DisableSSHCAHost { + if *p.DisableSSHCAHost != false { t.Errorf("By default DisableSSHCAHost should be false") } }) @@ -601,7 +601,9 @@ func TestGCP_AuthorizeSSHSign(t *testing.T) { p1, err := generateGCP() assert.FatalError(t, err) p1.DisableCustomSANs = true - p1.EnableSSHCAUser = true + // enable ssh user CA + disableSSCAUser := false + p1.DisableSSHCAUser = &disableSSCAUser p2, err := generateGCP() assert.FatalError(t, err) @@ -617,7 +619,9 @@ func TestGCP_AuthorizeSSHSign(t *testing.T) { p4, err := generateGCP() assert.FatalError(t, err) - p4.DisableSSHCAHost = true + // disable ssh host CA + disableSSCAHost := true + p4.DisableSSHCAHost = &disableSSCAHost t1, err := generateGCPToken(p1.ServiceAccounts[0], "https://accounts.google.com", p1.GetID(), @@ -662,7 +666,7 @@ func TestGCP_AuthorizeSSHSign(t *testing.T) { ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), } expectedUserOptions := &SignSSHOptions{ - CertType: "user", Principals: []string{"foo", "foo@developer.gserviceaccount.com"}, + CertType: "user", Principals: []string{FormatServiceAccountUsername(p1.ServiceAccounts[0]), "foo@developer.gserviceaccount.com"}, ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(p1.ctl.Claimer.DefaultUserSSHCertDuration())), } diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go index a599a8351..975192b20 100644 --- a/authority/provisioner/utils_test.go +++ b/authority/provisioner/utils_test.go @@ -363,11 +363,13 @@ func generateGCP() (*GCP, error) { return nil, err } p := &GCP{ - Type: "GCP", - Name: name, - ServiceAccounts: []string{serviceAccount}, - Claims: &globalProvisionerClaims, - config: newGCPConfig(), + Type: "GCP", + Name: name, + ServiceAccounts: []string{serviceAccount}, + Claims: &globalProvisionerClaims, + DisableSSHCAHost: &DefaultDisableSSHCAHost, + DisableSSHCAUser: &DefaultDisableSSHCAUser, + config: newGCPConfig(), keyStore: &keyStore{ keySet: jose.JSONWebKeySet{Keys: []jose.JSONWebKey{*jwk}}, expiry: time.Now().Add(24 * time.Hour),