Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allowing GCP provisioner to issue SSH User Certificates - Option 2 #1558

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Allowing GCP provisioner to issue SSH User Certificates - Option 2 #1558

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e8af03c
Allow User Certs for Service Accounts in the GCP provisioner
adantop Sep 21, 2023
0a43b55
Adding test for new attribute defaults
adantop May 8, 2024
3ab951f
Replace sanitized username with google pattern service account usernames
adantop May 9, 2024
bedb040
Updating options to be Disable* and of type *bool
adantop May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions api/ssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
88 changes: 69 additions & 19 deletions authority/provisioner/gcp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe these should both be Disable-prefixed, or both be Enable-prefixed? or the smallstep team might prefer not having the host CA be an option.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dopey How do y'all feel about this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer Disable* but at the same time as it is backward compatible. We can also achieve backward compatibility with *bool. assuming nil true in some cases and false in others, if we use *bool we might also want the omitempty. But this last option is ugly.

InstanceAge Duration `json:"instanceAge,omitempty"`
Claims *Claims `json:"claims,omitempty"`
Options *Options `json:"options,omitempty"`
Expand Down Expand Up @@ -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.
Expand All @@ -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")
}
Expand All @@ -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{
fmt.Sprintf("sa_%v", claims.Subject),
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
}
31 changes: 28 additions & 3 deletions authority/provisioner/gcp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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")
}
})
}
}
Expand Down Expand Up @@ -592,6 +601,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)
Expand All @@ -605,6 +615,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",
Expand Down Expand Up @@ -647,6 +661,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
Expand All @@ -664,22 +682,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
Expand Down
14 changes: 14 additions & 0 deletions authority/provisioner/method.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}