From a8f4ad1b8e1283b6a135f2fdf620efedd01fb99b Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 31 Jul 2019 17:03:33 -0700 Subject: [PATCH] Set default SSH options if no user options are given. --- authority/provisioner/aws.go | 9 +++++-- authority/provisioner/azure.go | 9 +++++-- authority/provisioner/gcp.go | 9 +++++-- authority/provisioner/jwk.go | 5 +++- authority/provisioner/oidc.go | 29 +++++++++++--------- authority/provisioner/sign_ssh_options.go | 33 +++++++++++++++++------ 6 files changed, 67 insertions(+), 27 deletions(-) diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go index 03a0747e..07285940 100644 --- a/authority/provisioner/aws.go +++ b/authority/provisioner/aws.go @@ -448,13 +448,18 @@ func (p *AWS) authorizeSSHSign(claims *awsPayload) ([]SignOption, error) { sshCertificateKeyIDModifier(claims.Subject), } - signOptions = append(signOptions, &sshCertificateOptionsValidator{&SSHOptions{ + // Default to host + known IPs/hostnames + defaults := SSHOptions{ CertType: SSHHostCert, Principals: []string{ doc.PrivateIP, fmt.Sprintf("ip-%s.%s.compute.internal", strings.Replace(doc.PrivateIP, ".", "-", -1), doc.Region), }, - }}) + } + // Validate user options + signOptions = append(signOptions, sshCertificateOptionsValidator(defaults)) + // Set defaults if not given as user options + signOptions = append(signOptions, sshCertificateDefaultsModifier(defaults)) return append(signOptions, // set the default extensions diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go index f5d04adb..d84e9f91 100644 --- a/authority/provisioner/azure.go +++ b/authority/provisioner/azure.go @@ -308,10 +308,15 @@ func (p *Azure) authorizeSSHSign(claims azurePayload, name string) ([]SignOption sshCertificateKeyIDModifier(name), } - signOptions = append(signOptions, &sshCertificateOptionsValidator{&SSHOptions{ + // Default to host + known hostnames + defaults := SSHOptions{ CertType: SSHHostCert, Principals: []string{name}, - }}) + } + // Validate user options + signOptions = append(signOptions, sshCertificateOptionsValidator(defaults)) + // Set defaults if not given as user options + signOptions = append(signOptions, sshCertificateDefaultsModifier(defaults)) return append(signOptions, // set the default extensions diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go index 4415772d..d5dde616 100644 --- a/authority/provisioner/gcp.go +++ b/authority/provisioner/gcp.go @@ -360,13 +360,18 @@ func (p *GCP) authorizeSSHSign(claims *gcpPayload) ([]SignOption, error) { sshCertificateKeyIDModifier(ce.InstanceName), } - signOptions = append(signOptions, &sshCertificateOptionsValidator{&SSHOptions{ + // Default to host + known hostnames + defaults := SSHOptions{ CertType: SSHHostCert, 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), }, - }}) + } + // Validate user options + signOptions = append(signOptions, sshCertificateOptionsValidator(defaults)) + // Set defaults if not given as user options + signOptions = append(signOptions, sshCertificateDefaultsModifier(defaults)) return append(signOptions, // set the default extensions diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index ff53cce8..012e08a9 100644 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -178,7 +178,7 @@ func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) { opts := claims.Step.SSH signOptions := []SignOption{ // validates user's SSHOptions with the ones in the token - &sshCertificateOptionsValidator{opts}, + sshCertificateOptionsValidator(*opts), // set the key id to the token subject sshCertificateKeyIDModifier(claims.Subject), } @@ -197,6 +197,9 @@ func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) { signOptions = append(signOptions, sshCertificateValidBeforeModifier(opts.ValidBefore.RelativeTime(t).Unix())) } + // Default to a user certificate with no principals if not set + signOptions = append(signOptions, sshCertificateDefaultsModifier{CertType: SSHUserCert}) + return append(signOptions, // set the default extensions &sshDefaultExtensionModifier{}, diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index 8c89ea37..0c0f840e 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -303,20 +303,25 @@ func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) { sshCertificateKeyIDModifier(claims.Email), } - // Non-admins are only able to sign user certificates - if o.IsAdmin(claims.Email) { - signOptions = append(signOptions, &sshCertificateOptionsValidator{}) - } else { - name := SanitizeSSHUserPrincipal(claims.Email) - if !sshUserRegex.MatchString(name) { - return nil, errors.Errorf("invalid principal '%s' from email address '%s'", name, claims.Email) - } - signOptions = append(signOptions, &sshCertificateOptionsValidator{&SSHOptions{ - CertType: SSHUserCert, - Principals: []string{name}, - }}) + name := SanitizeSSHUserPrincipal(claims.Email) + if !sshUserRegex.MatchString(name) { + return nil, errors.Errorf("invalid principal '%s' from email address '%s'", name, claims.Email) } + // Admin users will default to user + name but they can be changed by the + // user options. Non-admins are only able to sign user certificates. + defaults := SSHOptions{ + CertType: SSHUserCert, + Principals: []string{name}, + } + + if !o.IsAdmin(claims.Email) { + signOptions = append(signOptions, sshCertificateOptionsValidator(defaults)) + } + + // Default to a user with name as principal if not set + signOptions = append(signOptions, sshCertificateDefaultsModifier(defaults)) + return append(signOptions, // set the default extensions &sshDefaultExtensionModifier{}, diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 4748dc09..d1bf78bd 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -143,6 +143,27 @@ func (m sshCertificateValidBeforeModifier) Modify(cert *ssh.Certificate) error { return nil } +// sshCertificateDefaultModifier implements a SSHCertificateModifier that +// modifies the certificate with the given options if they are not set. +type sshCertificateDefaultsModifier SSHOptions + +// Modify implements the SSHCertificateModifier interface. +func (m sshCertificateDefaultsModifier) Modify(cert *ssh.Certificate) error { + if cert.CertType == 0 { + cert.CertType = sshCertTypeUInt32(m.CertType) + } + if len(cert.ValidPrincipals) == 0 { + cert.ValidPrincipals = m.Principals + } + if cert.ValidAfter == 0 && !m.ValidAfter.IsZero() { + cert.ValidAfter = uint64(m.ValidAfter.Unix()) + } + if cert.ValidBefore == 0 && !m.ValidBefore.IsZero() { + cert.ValidBefore = uint64(m.ValidBefore.Unix()) + } + return nil +} + // sshDefaultExtensionModifier implements an SSHCertificateModifier that sets // the default extensions in an SSH certificate. type sshDefaultExtensionModifier struct{} @@ -212,17 +233,13 @@ func (m *sshCertificateValidityModifier) Modify(cert *ssh.Certificate) error { // sshCertificateOptionsValidator validates the user SSHOptions with the ones // usually present in the token. -type sshCertificateOptionsValidator struct { - Want *SSHOptions -} +type sshCertificateOptionsValidator SSHOptions // Valid implements SSHCertificateOptionsValidator and returns nil if both // SSHOptions match. -func (v *sshCertificateOptionsValidator) Valid(got SSHOptions) error { - if v.Want == nil { - return nil - } - return v.Want.match(got) +func (v sshCertificateOptionsValidator) Valid(got SSHOptions) error { + want := SSHOptions(v) + return want.match(got) } // sshCertificateDefaultValidator implements a simple validator for all the