From 1c8f610ca9c4033991d395a430f630786774396a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 23 Jul 2019 18:46:43 -0700 Subject: [PATCH 01/40] Add initial implementation of an SSH CA using the JWK provisioner. Fixes smallstep/ca-component#187 --- api/api.go | 3 + api/ssh.go | 150 +++++++++++ authority/authority.go | 6 + authority/config.go | 14 +- authority/provisioner/claims.go | 82 +++++- authority/provisioner/jwk.go | 47 +++- authority/provisioner/sign_ssh_options.go | 290 ++++++++++++++++++++++ authority/provisioner/timeduration.go | 11 + authority/ssh.go | 114 +++++++++ ca/client.go | 22 ++ 10 files changed, 730 insertions(+), 9 deletions(-) create mode 100644 api/ssh.go create mode 100644 authority/provisioner/sign_ssh_options.go create mode 100644 authority/ssh.go diff --git a/api/api.go b/api/api.go index f1013c0a..a001dac6 100644 --- a/api/api.go +++ b/api/api.go @@ -26,6 +26,7 @@ import ( // Authority is the interface implemented by a CA authority. type Authority interface { + SSHAuthority // NOTE: Authorize will be deprecated in future releases. Please use the // context specific Authoirize[Sign|Revoke|etc.] methods. Authorize(ott string) ([]provisioner.SignOption, error) @@ -249,6 +250,8 @@ func (h *caHandler) Route(r Router) { r.MethodFunc("GET", "/federation", h.Federation) // For compatibility with old code: r.MethodFunc("POST", "/re-sign", h.Renew) + // SSH CA + r.MethodFunc("POST", "/sign-ssh", h.SignSSH) } // Health is an HTTP handler that returns the status of the server. diff --git a/api/ssh.go b/api/ssh.go new file mode 100644 index 00000000..0e3e4791 --- /dev/null +++ b/api/ssh.go @@ -0,0 +1,150 @@ +package api + +import ( + "bytes" + "encoding/base64" + "encoding/json" + "net/http" + + "github.com/pkg/errors" + "github.com/smallstep/certificates/authority/provisioner" + "golang.org/x/crypto/ssh" +) + +// SSHAuthority is the interface implemented by a SSH CA authority. +type SSHAuthority interface { + SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) +} + +// SignSSHRequest is the request body of an SSH certificate request. +type SignSSHRequest struct { + PublicKey []byte `json:"publicKey"` //base64 encoded + OTT string `json:"ott"` + CertType string `json:"certType"` + Principals []string `json:"principals"` + ValidAfter TimeDuration `json:"validAfter"` + ValidBefore TimeDuration `json:"validBefore"` +} + +// SignSSHResponse is the response object that returns the SSH certificate. +type SignSSHResponse struct { + Certificate SSHCertificate `json:"crt"` +} + +// SSHCertificate represents the response SSH certificate. +type SSHCertificate struct { + *ssh.Certificate +} + +// MarshalJSON implements the json.Marshaler interface. The certificate is +// quoted string using the PEM encoding. +func (c SSHCertificate) MarshalJSON() ([]byte, error) { + if c.Certificate == nil { + return []byte("null"), nil + } + s := base64.StdEncoding.EncodeToString(c.Certificate.Marshal()) + return []byte(`"` + s + `"`), nil +} + +// UnmarshalJSON implements the json.Unmarshaler interface. The certificate is +// expected to be a quoted string using the PEM encoding. +func (c *SSHCertificate) UnmarshalJSON(data []byte) error { + var s string + if err := json.Unmarshal(data, &s); err != nil { + return errors.Wrap(err, "error decoding certificate") + } + certData, err := base64.StdEncoding.DecodeString(s) + if err != nil { + return errors.Wrap(err, "error decoding certificate") + } + pub, err := ssh.ParsePublicKey(certData) + if err != nil { + return errors.Wrap(err, "error decoding certificate") + } + cert, ok := pub.(*ssh.Certificate) + if !ok { + return errors.Errorf("error decoding certificate: %T is not an *ssh.Certificate", pub) + } + c.Certificate = cert + return nil +} + +// Validate validates the SignSSHRequest. +func (s *SignSSHRequest) Validate() error { + switch { + case s.CertType != "" && s.CertType != provisioner.SSHUserCert && s.CertType != provisioner.SSHHostCert: + return errors.Errorf("unknown certType %s", s.CertType) + case len(s.PublicKey) == 0: + return errors.New("missing or empty publicKey") + case len(s.OTT) == 0: + return errors.New("missing or empty ott") + default: + return nil + } +} + +// ParsePublicKey returns the ssh.PublicKey from the request. +func (s *SignSSHRequest) ParsePublicKey() (ssh.PublicKey, error) { + // Validate pub key. + data := make([]byte, base64.StdEncoding.DecodedLen(len(s.PublicKey))) + if _, err := base64.StdEncoding.Decode(data, s.PublicKey); err != nil { + return nil, errors.Wrap(err, "error decoding publicKey") + } + + // Trim padding from end of key. + data = bytes.TrimRight(data, "\x00") + publicKey, err := ssh.ParsePublicKey(data) + if err != nil { + return nil, errors.Wrap(err, "error parsing publicKey") + } + + return publicKey, nil +} + +// SignSSH is an HTTP handler that reads an SignSSHRequest with a one-time-token +// (ott) from the body and creates a new SSH certificate with the information in +// the request. +func (h *caHandler) SignSSH(w http.ResponseWriter, r *http.Request) { + var body SignSSHRequest + if err := ReadJSON(r.Body, &body); err != nil { + WriteError(w, BadRequest(errors.Wrap(err, "error reading request body"))) + return + } + + logOtt(w, body.OTT) + if err := body.Validate(); err != nil { + WriteError(w, err) + return + } + + publicKey, err := body.ParsePublicKey() + if err != nil { + WriteError(w, BadRequest(err)) + return + } + + opts := provisioner.SSHOptions{ + CertType: body.CertType, + Principals: body.Principals, + ValidBefore: body.ValidBefore, + ValidAfter: body.ValidAfter, + } + + signOpts, err := h.Authority.AuthorizeSign(body.OTT) + if err != nil { + WriteError(w, Unauthorized(err)) + return + } + + cert, err := h.Authority.SignSSH(publicKey, opts, signOpts...) + if err != nil { + WriteError(w, Forbidden(err)) + return + } + + w.WriteHeader(http.StatusCreated) + // logCertificate(w, cert) + JSON(w, &SignSSHResponse{ + Certificate: SSHCertificate{cert}, + }) +} diff --git a/authority/authority.go b/authority/authority.go index 33340029..c4d9d1cd 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -1,6 +1,7 @@ package authority import ( + "crypto" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -20,6 +21,8 @@ type Authority struct { config *Config rootX509Certs []*x509.Certificate intermediateIdentity *x509util.Identity + sshCAUserCertSignKey crypto.Signer + sshCAHostCertSignKey crypto.Signer validateOnce bool certificates *sync.Map startTime time.Time @@ -117,6 +120,9 @@ func (a *Authority) init() error { } } + a.sshCAHostCertSignKey = a.intermediateIdentity.Key.(crypto.Signer) + a.sshCAUserCertSignKey = a.intermediateIdentity.Key.(crypto.Signer) + // Store all the provisioners for _, p := range a.config.AuthorityConfig.Provisioners { if err := a.provisioners.Store(p); err != nil { diff --git a/authority/config.go b/authority/config.go index 77854812..7cfdf744 100644 --- a/authority/config.go +++ b/authority/config.go @@ -29,10 +29,16 @@ var ( } defaultDisableRenewal = false globalProvisionerClaims = provisioner.Claims{ - MinTLSDur: &provisioner.Duration{Duration: 5 * time.Minute}, - MaxTLSDur: &provisioner.Duration{Duration: 24 * time.Hour}, - DefaultTLSDur: &provisioner.Duration{Duration: 24 * time.Hour}, - DisableRenewal: &defaultDisableRenewal, + MinTLSDur: &provisioner.Duration{Duration: 5 * time.Minute}, // TLS certs + MaxTLSDur: &provisioner.Duration{Duration: 24 * time.Hour}, + DefaultTLSDur: &provisioner.Duration{Duration: 24 * time.Hour}, + DisableRenewal: &defaultDisableRenewal, + MinUserSSHDur: &provisioner.Duration{Duration: 5 * time.Minute}, // User SSH certs + MaxUserSSHDur: &provisioner.Duration{Duration: 24 * time.Hour}, + DefaultUserSSHDur: &provisioner.Duration{Duration: 4 * time.Hour}, + MinHostSSHDur: &provisioner.Duration{Duration: 5 * time.Minute}, // Host SSH certs + MaxHostSSHDur: &provisioner.Duration{Duration: 30 * 24 * time.Hour}, + DefaultHostSSHDur: &provisioner.Duration{Duration: 30 * 24 * time.Hour}, } ) diff --git a/authority/provisioner/claims.go b/authority/provisioner/claims.go index 1109e0c7..23c85002 100644 --- a/authority/provisioner/claims.go +++ b/authority/provisioner/claims.go @@ -8,10 +8,18 @@ import ( // Claims so that individual provisioners can override global claims. type Claims struct { + // TLS CA properties MinTLSDur *Duration `json:"minTLSCertDuration,omitempty"` MaxTLSDur *Duration `json:"maxTLSCertDuration,omitempty"` DefaultTLSDur *Duration `json:"defaultTLSCertDuration,omitempty"` DisableRenewal *bool `json:"disableRenewal,omitempty"` + // SSH CA properties + MinUserSSHDur *Duration `json:"minUserSSHCertDuration,omitempty"` + MaxUserSSHDur *Duration `json:"maxUserSSHCertDuration,omitempty"` + DefaultUserSSHDur *Duration `json:"defaultUserSSHCertDuration,omitempty"` + MinHostSSHDur *Duration `json:"minHostSSHCertDuration,omitempty"` + MaxHostSSHDur *Duration `json:"maxHostSSHCertDuration,omitempty"` + DefaultHostSSHDur *Duration `json:"defaultHostSSHCertDuration,omitempty"` } // Claimer is the type that controls claims. It provides an interface around the @@ -31,10 +39,16 @@ func NewClaimer(claims *Claims, global Claims) (*Claimer, error) { func (c *Claimer) Claims() Claims { disableRenewal := c.IsDisableRenewal() return Claims{ - MinTLSDur: &Duration{c.MinTLSCertDuration()}, - MaxTLSDur: &Duration{c.MaxTLSCertDuration()}, - DefaultTLSDur: &Duration{c.DefaultTLSCertDuration()}, - DisableRenewal: &disableRenewal, + MinTLSDur: &Duration{c.MinTLSCertDuration()}, + MaxTLSDur: &Duration{c.MaxTLSCertDuration()}, + DefaultTLSDur: &Duration{c.DefaultTLSCertDuration()}, + DisableRenewal: &disableRenewal, + MinUserSSHDur: &Duration{c.MinUserSSHCertDuration()}, + MaxUserSSHDur: &Duration{c.MaxUserSSHCertDuration()}, + DefaultUserSSHDur: &Duration{c.DefaultUserSSHCertDuration()}, + MinHostSSHDur: &Duration{c.MinHostSSHCertDuration()}, + MaxHostSSHDur: &Duration{c.MaxHostSSHCertDuration()}, + DefaultHostSSHDur: &Duration{c.DefaultHostSSHCertDuration()}, } } @@ -78,6 +92,66 @@ func (c *Claimer) IsDisableRenewal() bool { return *c.claims.DisableRenewal } +// DefaultUserSSHCertDuration returns the default SSH user cert duration for the +// provisioner. If the default is not set within the provisioner, then the +// global default from the authority configuration will be used. +func (c *Claimer) DefaultUserSSHCertDuration() time.Duration { + if c.claims == nil || c.claims.DefaultUserSSHDur == nil { + return c.global.DefaultUserSSHDur.Duration + } + return c.claims.DefaultUserSSHDur.Duration +} + +// MinUserSSHCertDuration returns the minimum SSH user cert duration for the +// provisioner. If the minimum is not set within the provisioner, then the +// global minimum from the authority configuration will be used. +func (c *Claimer) MinUserSSHCertDuration() time.Duration { + if c.claims == nil || c.claims.MinUserSSHDur == nil { + return c.global.MinUserSSHDur.Duration + } + return c.claims.MinUserSSHDur.Duration +} + +// MaxUserSSHCertDuration returns the maximum SSH user cert duration for the +// provisioner. If the maximum is not set within the provisioner, then the +// global maximum from the authority configuration will be used. +func (c *Claimer) MaxUserSSHCertDuration() time.Duration { + if c.claims == nil || c.claims.MaxUserSSHDur == nil { + return c.global.MaxUserSSHDur.Duration + } + return c.claims.MaxUserSSHDur.Duration +} + +// DefaultHostSSHCertDuration returns the default SSH host cert duration for the +// provisioner. If the default is not set within the provisioner, then the +// global default from the authority configuration will be used. +func (c *Claimer) DefaultHostSSHCertDuration() time.Duration { + if c.claims == nil || c.claims.DefaultHostSSHDur == nil { + return c.global.DefaultHostSSHDur.Duration + } + return c.claims.DefaultHostSSHDur.Duration +} + +// MinHostSSHCertDuration returns the minimum SSH host cert duration for the +// provisioner. If the minimum is not set within the provisioner, then the +// global minimum from the authority configuration will be used. +func (c *Claimer) MinHostSSHCertDuration() time.Duration { + if c.claims == nil || c.claims.MinHostSSHDur == nil { + return c.global.MinHostSSHDur.Duration + } + return c.claims.MinHostSSHDur.Duration +} + +// MaxHostSSHCertDuration returns the maximum SSH Host cert duration for the +// provisioner. If the maximum is not set within the provisioner, then the +// global maximum from the authority configuration will be used. +func (c *Claimer) MaxHostSSHCertDuration() time.Duration { + if c.claims == nil || c.claims.MaxHostSSHDur == nil { + return c.global.MaxHostSSHDur.Duration + } + return c.claims.MaxHostSSHDur.Duration +} + // Validate validates and modifies the Claims with default values. func (c *Claimer) Validate() error { var ( diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index dca5dce9..a21815b9 100644 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -12,7 +12,12 @@ import ( // jwtPayload extends jwt.Claims with step attributes. type jwtPayload struct { jose.Claims - SANs []string `json:"sans,omitempty"` + SANs []string `json:"sans,omitempty"` + Step *stepPayload `json:"step,omitempty"` +} + +type stepPayload struct { + SSH *SSHOptions `json:"ssh,omitempty"` } // JWK is the default provisioner, an entity that can sign tokens necessary for @@ -134,6 +139,12 @@ func (p *JWK) AuthorizeSign(token string) ([]SignOption, error) { if err != nil { return nil, err } + + // Check for SSH token + if claims.Step != nil && claims.Step.SSH != nil { + return p.authorizeSSHSign(claims) + } + // NOTE: This is for backwards compatibility with older versions of cli // and certificates. Older versions added the token subject as the only SAN // in a CSR by default. @@ -159,3 +170,37 @@ func (p *JWK) AuthorizeRenewal(cert *x509.Certificate) error { } return nil } + +func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) { + t := now() + opts := claims.Step.SSH + signOptions := []SignOption{ + // validates user's SSHOptions with the ones in the token + &sshCertificateOptionsValidator{opts}, + // set the default extensions + &sshDefaultExtensionModifier{}, + // set the key id to the token subject + sshCertificateKeyIDModifier(claims.Subject), + } + + // Add modifiers from custom claims + if opts.CertType != "" { + signOptions = append(signOptions, sshCertificateCertTypeModifier(opts.CertType)) + } + if len(opts.Principals) > 0 { + signOptions = append(signOptions, sshCertificatePrincipalsModifier(opts.Principals)) + } + if !opts.ValidAfter.IsZero() { + signOptions = append(signOptions, sshCertificateValidAfterModifier(opts.ValidAfter.RelativeTime(t).Unix())) + } + if !opts.ValidBefore.IsZero() { + signOptions = append(signOptions, sshCertificateValidBeforeModifier(opts.ValidBefore.RelativeTime(t).Unix())) + } + + return append(signOptions, + // checks the validity bounds, and set the validity if has not been set + &sshCertificateValidityModifier{p.claimer}, + // require all the fields in the SSH certificate + &sshCertificateDefaultValidator{}, + ), nil +} diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go new file mode 100644 index 00000000..3f4412ad --- /dev/null +++ b/authority/provisioner/sign_ssh_options.go @@ -0,0 +1,290 @@ +package provisioner + +import ( + "fmt" + "time" + + "github.com/pkg/errors" + "golang.org/x/crypto/ssh" +) + +const ( + // SSHUserCert is the string used to represent ssh.UserCert. + SSHUserCert = "user" + + // SSHHostCert is the string used to represent ssh.HostCert. + SSHHostCert = "host" +) + +// SSHCertificateModifier is the interface used to change properties in an SSH +// certificate. +type SSHCertificateModifier interface { + SignOption + Modify(cert *ssh.Certificate) error +} + +// SSHCertificateOptionModifier is the interface used to add custom options used +// to modify the SSH certificate. +type SSHCertificateOptionModifier interface { + SignOption + Option(o SSHOptions) SSHCertificateModifier +} + +// SSHCertificateValidator is the interface used to validate an SSH certificate. +type SSHCertificateValidator interface { + SignOption + Valid(crt *ssh.Certificate) error +} + +// SSHCertificateOptionsValidator is the interface used to validate the custom +// options used to modify the SSH certificate. +type SSHCertificateOptionsValidator interface { + SignOption + Valid(got SSHOptions) error +} + +// SSHOptions contains the options that can be passed to the SignSSH method. +type SSHOptions struct { + CertType string `json:"certType"` + Principals []string `json:"principals"` + ValidAfter TimeDuration `json:"validAfter,omitempty"` + ValidBefore TimeDuration `json:"validBefore,omitempty"` +} + +// Type returns the uint32 representation of the CertType. +func (o SSHOptions) Type() uint32 { + return sshCertTypeUInt32(o.CertType) +} + +// Modify implements SSHCertificateModifier and sets the SSHOption in the ssh.Certificate. +func (o SSHOptions) Modify(cert *ssh.Certificate) error { + switch o.CertType { + case "": // ignore + case SSHUserCert: + cert.CertType = ssh.UserCert + case SSHHostCert: + cert.CertType = ssh.HostCert + default: + return errors.Errorf("ssh certificate has an unknown type: %s", o.CertType) + } + cert.ValidPrincipals = o.Principals + if !o.ValidAfter.IsZero() { + cert.ValidAfter = uint64(o.ValidAfter.Time().Unix()) + } + if !o.ValidBefore.IsZero() { + cert.ValidBefore = uint64(o.ValidBefore.Time().Unix()) + } + if cert.ValidAfter > 0 && cert.ValidBefore > 0 && cert.ValidAfter > cert.ValidBefore { + return errors.New("ssh certificate valid after cannot be greater than valid before") + } + return nil +} + +// match compares two SSHOptions and return an error if they don't match. It +// ignores zero values. +func (o SSHOptions) match(got SSHOptions) error { + if o.CertType != "" && got.CertType != "" && o.CertType != got.CertType { + return errors.Errorf("ssh certificate type does not match - got %v, want %v", got.CertType, o.CertType) + } + if len(o.Principals) > 0 && len(got.Principals) > 0 && !equalStringSlice(o.Principals, got.Principals) { + return errors.Errorf("ssh certificate principals does not match - got %v, want %v", got.Principals, o.Principals) + } + if !o.ValidAfter.IsZero() && !got.ValidAfter.IsZero() && !o.ValidAfter.Equal(&got.ValidAfter) { + return errors.Errorf("ssh certificate valid after does not match - got %v, want %v", got.ValidAfter, o.ValidAfter) + } + if !o.ValidBefore.IsZero() && !got.ValidBefore.IsZero() && !o.ValidBefore.Equal(&got.ValidBefore) { + return errors.Errorf("ssh certificate valid before does not match - got %v, want %v", got.ValidBefore, o.ValidBefore) + } + return nil +} + +// sshCertificateKeyIDModifier is an SSHCertificateModifier that sets the given +// Key ID in the SSH certificate. +type sshCertificateKeyIDModifier string + +func (m sshCertificateKeyIDModifier) Modify(cert *ssh.Certificate) error { + cert.KeyId = string(m) + return nil +} + +// sshCertificateCertTypeModifier is an SSHCertificateModifier that sets the +// certificate type to the SSH certificate. +type sshCertificateCertTypeModifier string + +func (m sshCertificateCertTypeModifier) Modify(cert *ssh.Certificate) error { + cert.CertType = sshCertTypeUInt32(string(m)) + return nil +} + +// sshCertificatePrincipalsModifier is an SSHCertificateModifier that sets the +// principals to the SSH certificate. +type sshCertificatePrincipalsModifier []string + +func (m sshCertificatePrincipalsModifier) Modify(cert *ssh.Certificate) error { + cert.ValidPrincipals = []string(m) + return nil +} + +// sshCertificateValidAfterModifier is an SSHCertificateModifier that sets the +// ValidAfter in the SSH certificate. +type sshCertificateValidAfterModifier uint64 + +func (m sshCertificateValidAfterModifier) Modify(cert *ssh.Certificate) error { + cert.ValidAfter = uint64(m) + return nil +} + +// sshCertificateValidBeforeModifier is an SSHCertificateModifier that sets the +// ValidBefore in the SSH certificate. +type sshCertificateValidBeforeModifier uint64 + +func (m sshCertificateValidBeforeModifier) Modify(cert *ssh.Certificate) error { + cert.ValidBefore = uint64(m) + return nil +} + +// sshDefaultExtensionModifier implements an SSHCertificateModifier that sets +// the default extensions in an SSH certificate. +type sshDefaultExtensionModifier struct{} + +func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error { + if cert.Extensions == nil { + cert.Extensions = make(map[string]string) + } + cert.Extensions["permit-X11-forwarding"] = "" + cert.Extensions["permit-agent-forwarding"] = "" + cert.Extensions["permit-port-forwarding"] = "" + cert.Extensions["permit-pty"] = "" + cert.Extensions["permit-user-rc"] = "" + return nil +} + +// sshCertificateValidityModifier is a SSHCertificateModifier checks the +// validity bounds, setting them if they are not provided. It will fail if a +// CertType has not been set or is not valid. +type sshCertificateValidityModifier struct { + *Claimer +} + +func (m *sshCertificateValidityModifier) Modify(cert *ssh.Certificate) error { + var d, min, max time.Duration + switch cert.CertType { + case ssh.UserCert: + d = m.DefaultUserSSHCertDuration() + min = m.MinUserSSHCertDuration() + max = m.MaxUserSSHCertDuration() + case ssh.HostCert: + d = m.DefaultHostSSHCertDuration() + min = m.MinHostSSHCertDuration() + max = m.MaxHostSSHCertDuration() + case 0: + return errors.New("ssh certificate type has not been set") + default: + return errors.Errorf("unknown ssh certificate type %d", cert.CertType) + } + + if cert.ValidAfter == 0 { + cert.ValidAfter = uint64(now().Unix()) + } + if cert.ValidBefore == 0 { + t := time.Unix(int64(cert.ValidAfter), 0) + cert.ValidBefore = uint64(t.Add(d).Unix()) + } + + diff := time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second + switch { + case diff < max: + return errors.Errorf("ssh certificate duration cannot be lower than %s", min) + case diff > max: + return errors.Errorf("ssh certificate duration cannot be greater than %s", max) + default: + return nil + } +} + +// sshCertificateOptionsValidator validates the user SSHOptions with the ones +// usually present in the token. +type sshCertificateOptionsValidator struct { + *SSHOptions +} + +func (want *sshCertificateOptionsValidator) Valid(got SSHOptions) error { + return want.match(got) +} + +// sshCertificateDefaultValidator implements a simple validator for all the +// fields in the SSH certificate. +type sshCertificateDefaultValidator struct{} + +// Valid returns an error if the given certificate does not contain the necessary fields. +func (v *sshCertificateDefaultValidator) Valid(crt *ssh.Certificate) error { + switch { + case len(crt.Nonce) == 0: + return errors.New("ssh certificate nonce cannot be empty") + case crt.Key == nil: + return errors.New("ssh certificate key cannot be nil") + case crt.Serial == 0: + return errors.New("ssh certificate serial cannot be 0") + case crt.CertType != ssh.UserCert && crt.CertType != ssh.HostCert: + return errors.Errorf("ssh certificate has an unknown type: %d", crt.CertType) + case crt.KeyId == "": + return errors.New("ssh certificate key id cannot be empty") + case len(crt.ValidPrincipals) == 0: + return errors.New("ssh certificate valid principals cannot be empty") + case crt.ValidAfter == 0: + return errors.New("ssh certificate valid after cannot be 0") + case crt.ValidBefore == 0: + return errors.New("ssh certificate valid before cannot be 0") + case len(crt.Extensions) == 0: + return errors.New("ssh certificate extensions cannot be empty") + case crt.SignatureKey == nil: + return errors.New("ssh certificate signature key cannot be nil") + case crt.Signature == nil: + return errors.New("ssh certificate signature cannot be nil") + default: + return nil + } +} + +// sshCertTypeName returns the string representation of the given ssh.CertType. +func sshCertTypeString(ct uint32) string { + switch ct { + case 0: + return "" + case ssh.UserCert: + return SSHUserCert + case ssh.HostCert: + return SSHHostCert + default: + return fmt.Sprintf("unknown (%d)", ct) + } +} + +// sshCertTypeUInt32 +func sshCertTypeUInt32(ct string) uint32 { + switch ct { + case SSHUserCert: + return ssh.UserCert + case SSHHostCert: + return ssh.HostCert + default: + return 0 + } +} + +func equalStringSlice(a, b []string) bool { + var l int + if l = len(a); l != len(b) { + return false + } + visit := make(map[string]struct{}, l) + for i := 0; i < l; i++ { + visit[a[i]] = struct{}{} + } + for i := 0; i < l; i++ { + if _, ok := visit[b[i]]; !ok { + return false + } + } + return true +} diff --git a/authority/provisioner/timeduration.go b/authority/provisioner/timeduration.go index fea967d5..33104df3 100644 --- a/authority/provisioner/timeduration.go +++ b/authority/provisioner/timeduration.go @@ -57,6 +57,17 @@ func (t *TimeDuration) SetTime(tt time.Time) { t.t, t.d = tt, 0 } +// IsZero returns true the TimeDuration represents the zero value, false +// otherwise. +func (t *TimeDuration) IsZero() bool { + return t.t.IsZero() && t.d == 0 +} + +// Equal returns if t and other are equal. +func (t *TimeDuration) Equal(other *TimeDuration) bool { + return t.t.Equal(other.t) && t.d == other.d +} + // MarshalJSON implements the json.Marshaler interface. If the time is set it // will return the time in RFC 3339 format if not it will return the duration // string. diff --git a/authority/ssh.go b/authority/ssh.go new file mode 100644 index 00000000..952811ec --- /dev/null +++ b/authority/ssh.go @@ -0,0 +1,114 @@ +package authority + +import ( + "crypto/rand" + "crypto/sha256" + "encoding/binary" + "encoding/hex" + "net/http" + "strings" + + "github.com/pkg/errors" + "github.com/smallstep/certificates/authority/provisioner" + "github.com/smallstep/cli/crypto/randutil" + "golang.org/x/crypto/ssh" +) + +func generateSSHPublicKeyID(key ssh.PublicKey) string { + sum := sha256.Sum256(key.Marshal()) + return strings.ToLower(hex.EncodeToString(sum[:])) +} + +// SignSSH creates a signed SSH certificate with the given public key and options. +func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) { + var mods []provisioner.SSHCertificateModifier + var validators []provisioner.SSHCertificateValidator + + for _, op := range signOpts { + switch o := op.(type) { + // modify the ssh.Certificate + case provisioner.SSHCertificateModifier: + mods = append(mods, o) + // modify the ssh.Certificate given the SSHOptions + case provisioner.SSHCertificateOptionModifier: + mods = append(mods, o.Option(opts)) + // validate the ssh.Certificate + case provisioner.SSHCertificateValidator: + validators = append(validators, o) + // validate the given SSHOptions + case provisioner.SSHCertificateOptionsValidator: + if err := o.Valid(opts); err != nil { + return nil, &apiError{err: err, code: http.StatusUnauthorized} + } + default: + return nil, &apiError{ + err: errors.Errorf("signSSH: invalid extra option type %T", o), + code: http.StatusInternalServerError, + } + } + } + + nonce, err := randutil.ASCII(32) + if err != nil { + return nil, err + } + + var serial uint64 + if err := binary.Read(rand.Reader, binary.BigEndian, &serial); err != nil { + return nil, errors.Wrap(err, "error reading random number") + } + + // Build base certificate with the key and some random values + cert := &ssh.Certificate{ + Nonce: []byte(nonce), + Key: key, + Serial: serial, + } + + // Use opts to modify the certificate + if err := opts.Modify(cert); err != nil { + return nil, err + } + + // Use provisioner modifiers + for _, m := range mods { + if err := m.Modify(cert); err != nil { + return nil, &apiError{err: err, code: http.StatusInternalServerError} + } + } + + // Get signer from authority keys + var signer ssh.Signer + switch cert.CertType { + case ssh.UserCert: + signer, err = ssh.NewSignerFromSigner(a.sshCAUserCertSignKey) + case ssh.HostCert: + signer, err = ssh.NewSignerFromSigner(a.sshCAHostCertSignKey) + default: + return nil, &apiError{ + err: errors.Errorf("unexpected ssh certificate type: %d", cert.CertType), + code: http.StatusInternalServerError, + } + } + cert.SignatureKey = signer.PublicKey() + + // Get bytes for signing trailing the signature length. + data := cert.Marshal() + data = data[:len(data)-4] + + // Sign the certificate + sig, err := signer.Sign(rand.Reader, data) + if err != nil { + return nil, err + } + cert.Signature = sig + + // User provisioners validators + for _, v := range validators { + if err := v.Valid(cert); err != nil { + return nil, &apiError{err: err, code: http.StatusUnauthorized} + } + } + + return cert, nil +} diff --git a/ca/client.go b/ca/client.go index c9766293..30a43924 100644 --- a/ca/client.go +++ b/ca/client.go @@ -373,6 +373,28 @@ func (c *Client) Sign(req *api.SignRequest) (*api.SignResponse, error) { return &sign, nil } +// SignSSH performs the SSH certificate sign request to the CA and returns the +// api.SignSSHResponse struct. +func (c *Client) SignSSH(req *api.SignSSHRequest) (*api.SignSSHResponse, error) { + body, err := json.Marshal(req) + if err != nil { + return nil, errors.Wrap(err, "error marshaling request") + } + u := c.endpoint.ResolveReference(&url.URL{Path: "/sign-ssh"}) + resp, err := c.client.Post(u.String(), "application/json", bytes.NewReader(body)) + if err != nil { + return nil, errors.Wrapf(err, "client POST %s failed", u) + } + if resp.StatusCode >= 400 { + return nil, readError(resp.Body) + } + var sign api.SignSSHResponse + if err := readJSON(resp.Body, &sign); err != nil { + return nil, errors.Wrapf(err, "error reading %s", u) + } + return &sign, nil +} + // Renew performs the renew request to the CA and returns the api.SignResponse // struct. func (c *Client) Renew(tr http.RoundTripper) (*api.SignResponse, error) { From 3ff410c695f55b9cf7f5f9be9fc0c417ec64b9e4 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 25 Jul 2019 18:41:32 -0700 Subject: [PATCH 02/40] fix ssh validity modifier --- authority/provisioner/sign_ssh_options.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 3f4412ad..1b981504 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -193,7 +193,7 @@ func (m *sshCertificateValidityModifier) Modify(cert *ssh.Certificate) error { diff := time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second switch { - case diff < max: + case diff < min: return errors.Errorf("ssh certificate duration cannot be lower than %s", min) case diff > max: return errors.Errorf("ssh certificate duration cannot be greater than %s", max) From d7221e15acd3a79c4e402e6dc249e2a648829d74 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 25 Jul 2019 18:41:46 -0700 Subject: [PATCH 03/40] Always marshal timeduration as a string --- authority/provisioner/timeduration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authority/provisioner/timeduration.go b/authority/provisioner/timeduration.go index 33104df3..ca80ba10 100644 --- a/authority/provisioner/timeduration.go +++ b/authority/provisioner/timeduration.go @@ -75,7 +75,7 @@ func (t TimeDuration) MarshalJSON() ([]byte, error) { switch { case t.t.IsZero(): if t.d == 0 { - return []byte("null"), nil + return []byte(`""`), nil } return json.Marshal(t.d.String()) default: From d008d2d4d109da38513a7e30431c0b5e651dfc0b Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 25 Jul 2019 18:42:32 -0700 Subject: [PATCH 04/40] Use default base64 encoding for public key --- api/ssh.go | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/api/ssh.go b/api/ssh.go index 0e3e4791..de91b559 100644 --- a/api/ssh.go +++ b/api/ssh.go @@ -1,7 +1,6 @@ package api import ( - "bytes" "encoding/base64" "encoding/json" "net/http" @@ -83,24 +82,6 @@ func (s *SignSSHRequest) Validate() error { } } -// ParsePublicKey returns the ssh.PublicKey from the request. -func (s *SignSSHRequest) ParsePublicKey() (ssh.PublicKey, error) { - // Validate pub key. - data := make([]byte, base64.StdEncoding.DecodedLen(len(s.PublicKey))) - if _, err := base64.StdEncoding.Decode(data, s.PublicKey); err != nil { - return nil, errors.Wrap(err, "error decoding publicKey") - } - - // Trim padding from end of key. - data = bytes.TrimRight(data, "\x00") - publicKey, err := ssh.ParsePublicKey(data) - if err != nil { - return nil, errors.Wrap(err, "error parsing publicKey") - } - - return publicKey, nil -} - // SignSSH is an HTTP handler that reads an SignSSHRequest with a one-time-token // (ott) from the body and creates a new SSH certificate with the information in // the request. @@ -117,9 +98,9 @@ func (h *caHandler) SignSSH(w http.ResponseWriter, r *http.Request) { return } - publicKey, err := body.ParsePublicKey() + publicKey, err := ssh.ParsePublicKey(body.PublicKey) if err != nil { - WriteError(w, BadRequest(err)) + WriteError(w, BadRequest(errors.Wrap(err, "error parsing publicKey"))) return } From 2127d09ef31143435598fbc8c8a804210a1fbe96 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 11:56:14 -0700 Subject: [PATCH 05/40] Rename context type to apiCtx. It will conflict with the context package. --- authority/authorize.go | 2 +- authority/error.go | 4 ++-- authority/provisioners.go | 4 ++-- authority/root.go | 6 +++--- authority/tls.go | 12 ++++++------ 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/authority/authorize.go b/authority/authorize.go index 1a7e45d3..72bfff8b 100644 --- a/authority/authorize.go +++ b/authority/authorize.go @@ -82,7 +82,7 @@ func (a *Authority) Authorize(ott string) ([]provisioner.SignOption, error) { // AuthorizeSign authorizes a signature request by validating and authenticating // a OTT that must be sent w/ the request. func (a *Authority) AuthorizeSign(ott string) ([]provisioner.SignOption, error) { - var errContext = context{"ott": ott} + var errContext = apiCtx{"ott": ott} p, err := a.authorizeToken(ott) if err != nil { diff --git a/authority/error.go b/authority/error.go index 056d3147..85293f20 100644 --- a/authority/error.go +++ b/authority/error.go @@ -4,13 +4,13 @@ import ( "net/http" ) -type context map[string]interface{} +type apiCtx map[string]interface{} // Error implements the api.Error interface and adds context to error messages. type apiError struct { err error code int - context context + context apiCtx } // Cause implements the errors.Causer interface and returns the original error. diff --git a/authority/provisioners.go b/authority/provisioners.go index 289b52a4..5328eb4d 100644 --- a/authority/provisioners.go +++ b/authority/provisioners.go @@ -13,7 +13,7 @@ func (a *Authority) GetEncryptedKey(kid string) (string, error) { key, ok := a.provisioners.LoadEncryptedKey(kid) if !ok { return "", &apiError{errors.Errorf("encrypted key with kid %s was not found", kid), - http.StatusNotFound, context{}} + http.StatusNotFound, apiCtx{}} } return key, nil } @@ -31,7 +31,7 @@ func (a *Authority) LoadProvisionerByCertificate(crt *x509.Certificate) (provisi p, ok := a.provisioners.LoadByCertificate(crt) if !ok { return nil, &apiError{errors.Errorf("provisioner not found"), - http.StatusNotFound, context{}} + http.StatusNotFound, apiCtx{}} } return p, nil } diff --git a/authority/root.go b/authority/root.go index 51ed6ac5..3794a6c8 100644 --- a/authority/root.go +++ b/authority/root.go @@ -12,13 +12,13 @@ func (a *Authority) Root(sum string) (*x509.Certificate, error) { val, ok := a.certificates.Load(sum) if !ok { return nil, &apiError{errors.Errorf("certificate with fingerprint %s was not found", sum), - http.StatusNotFound, context{}} + http.StatusNotFound, apiCtx{}} } crt, ok := val.(*x509.Certificate) if !ok { return nil, &apiError{errors.Errorf("stored value is not a *x509.Certificate"), - http.StatusInternalServerError, context{}} + http.StatusInternalServerError, apiCtx{}} } return crt, nil } @@ -53,7 +53,7 @@ func (a *Authority) GetFederation() (federation []*x509.Certificate, err error) if !ok { federation = nil err = &apiError{errors.Errorf("stored value is not a *x509.Certificate"), - http.StatusInternalServerError, context{}} + http.StatusInternalServerError, apiCtx{}} return false } federation = append(federation, crt) diff --git a/authority/tls.go b/authority/tls.go index fdaba130..d54e4373 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -58,7 +58,7 @@ func withDefaultASN1DN(def *x509util.ASN1DN) x509util.WithOption { // Sign creates a signed certificate from a certificate signing request. func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Options, extraOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error) { var ( - errContext = context{"csr": csr, "signOptions": signOpts} + errContext = apiCtx{"csr": csr, "signOptions": signOpts} mods = []x509util.WithOption{withDefaultASN1DN(a.config.AuthorityConfig.Template)} certValidators = []provisioner.CertificateValidator{} issIdentity = a.intermediateIdentity @@ -181,23 +181,23 @@ func (a *Authority) Renew(oldCert *x509.Certificate) (*x509.Certificate, *x509.C leaf, err := x509util.NewLeafProfileWithTemplate(newCert, issIdentity.Crt, issIdentity.Key) if err != nil { - return nil, nil, &apiError{err, http.StatusInternalServerError, context{}} + return nil, nil, &apiError{err, http.StatusInternalServerError, apiCtx{}} } crtBytes, err := leaf.CreateCertificate() if err != nil { return nil, nil, &apiError{errors.Wrap(err, "error renewing certificate from existing server certificate"), - http.StatusInternalServerError, context{}} + http.StatusInternalServerError, apiCtx{}} } serverCert, err := x509.ParseCertificate(crtBytes) if err != nil { return nil, nil, &apiError{errors.Wrap(err, "error parsing new server certificate"), - http.StatusInternalServerError, context{}} + http.StatusInternalServerError, apiCtx{}} } caCert, err := x509.ParseCertificate(issIdentity.Crt.Raw) if err != nil { return nil, nil, &apiError{errors.Wrap(err, "error parsing intermediate certificate"), - http.StatusInternalServerError, context{}} + http.StatusInternalServerError, apiCtx{}} } return serverCert, caCert, nil @@ -222,7 +222,7 @@ type RevokeOptions struct { // // TODO: Add OCSP and CRL support. func (a *Authority) Revoke(opts *RevokeOptions) error { - errContext := context{ + errContext := apiCtx{ "serialNumber": opts.Serial, "reasonCode": opts.ReasonCode, "reason": opts.Reason, From e1cd5ee8c3656fb72572db877241e252182a7430 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 12:34:27 -0700 Subject: [PATCH 06/40] Add context to the Authorize method. Fix tests. --- authority/authorize.go | 47 +++++++++++++++++++--------------- authority/authorize_test.go | 35 +++++++++++++------------ authority/provisioners_test.go | 2 +- authority/root_test.go | 4 +-- authority/tls_test.go | 24 +++++++++-------- 5 files changed, 62 insertions(+), 50 deletions(-) diff --git a/authority/authorize.go b/authority/authorize.go index 72bfff8b..b354fe84 100644 --- a/authority/authorize.go +++ b/authority/authorize.go @@ -1,6 +1,7 @@ package authority import ( + "context" "crypto/x509" "net/http" "strings" @@ -72,31 +73,37 @@ func (a *Authority) authorizeToken(ott string) (provisioner.Interface, error) { return p, nil } -// Authorize is a passthrough to AuthorizeSign. -// NOTE: Authorize will be deprecated in a future release. Please use the -// context specific Authorize[Sign|Revoke|etc.] going forwards. -func (a *Authority) Authorize(ott string) ([]provisioner.SignOption, error) { - return a.AuthorizeSign(ott) -} - -// AuthorizeSign authorizes a signature request by validating and authenticating -// a OTT that must be sent w/ the request. -func (a *Authority) AuthorizeSign(ott string) ([]provisioner.SignOption, error) { +// Authorize grabs the method from the context and authorizes a signature +// request by validating the one-time-token. +func (a *Authority) Authorize(ctx context.Context, ott string) ([]provisioner.SignOption, error) { var errContext = apiCtx{"ott": ott} + switch m := provisioner.MethodFromContext(ctx); m { + case provisioner.SignMethod, provisioner.SignSSHMethod: + p, err := a.authorizeToken(ott) + if err != nil { + return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext} + } - p, err := a.authorizeToken(ott) - if err != nil { - return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext} - } + // Call the provisioner AuthorizeSign method to apply provisioner specific + // auth claims and get the signing options. + opts, err := p.AuthorizeSign(context.Background(), ott) + if err != nil { + return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext} + } - // Call the provisioner AuthorizeSign method to apply provisioner specific - // auth claims and get the signing options. - opts, err := p.AuthorizeSign(ott) - if err != nil { - return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext} + return opts, nil + case provisioner.RevokeMethod: + return nil, &apiError{errors.New("authorize: revoke method is not supported"), http.StatusInternalServerError, errContext} + default: + return nil, &apiError{errors.Errorf("authorize: method %d is not supported", m), http.StatusInternalServerError, errContext} } +} - return opts, nil +// AuthorizeSign authorizes a signature request by validating and authenticating +// a OTT that must be sent w/ the request. +func (a *Authority) AuthorizeSign(ott string) ([]provisioner.SignOption, error) { + ctx := provisioner.NewContextWithMethod(context.Background(), provisioner.SignMethod) + return a.Authorize(ctx, ott) } // authorizeRevoke authorizes a revocation request by validating and authenticating diff --git a/authority/authorize_test.go b/authority/authorize_test.go index 368e9807..82be0689 100644 --- a/authority/authorize_test.go +++ b/authority/authorize_test.go @@ -1,11 +1,14 @@ package authority import ( + "context" "crypto/x509" "net/http" "testing" "time" + "github.com/smallstep/certificates/authority/provisioner" + "github.com/pkg/errors" "github.com/smallstep/assert" "github.com/smallstep/cli/crypto/pemutil" @@ -73,7 +76,7 @@ func TestAuthority_authorizeToken(t *testing.T) { auth: a, ott: "foo", err: &apiError{errors.New("authorizeToken: error parsing token"), - http.StatusUnauthorized, context{"ott": "foo"}}, + http.StatusUnauthorized, apiCtx{"ott": "foo"}}, } }, "fail/prehistoric-token": func(t *testing.T) *authorizeTest { @@ -92,7 +95,7 @@ func TestAuthority_authorizeToken(t *testing.T) { auth: a, ott: raw, err: &apiError{errors.New("authorizeToken: token issued before the bootstrap of certificate authority"), - http.StatusUnauthorized, context{"ott": raw}}, + http.StatusUnauthorized, apiCtx{"ott": raw}}, } }, "fail/provisioner-not-found": func(t *testing.T) *authorizeTest { @@ -114,7 +117,7 @@ func TestAuthority_authorizeToken(t *testing.T) { auth: a, ott: raw, err: &apiError{errors.New("authorizeToken: provisioner not found or invalid audience (https://test.ca.smallstep.com/revoke)"), - http.StatusUnauthorized, context{"ott": raw}}, + http.StatusUnauthorized, apiCtx{"ott": raw}}, } }, "ok/simpledb": func(t *testing.T) *authorizeTest { @@ -151,7 +154,7 @@ func TestAuthority_authorizeToken(t *testing.T) { auth: _a, ott: raw, err: &apiError{errors.New("authorizeToken: token already used"), - http.StatusUnauthorized, context{"ott": raw}}, + http.StatusUnauthorized, apiCtx{"ott": raw}}, } }, "ok/mockNoSQLDB": func(t *testing.T) *authorizeTest { @@ -199,7 +202,7 @@ func TestAuthority_authorizeToken(t *testing.T) { auth: _a, ott: raw, err: &apiError{errors.New("authorizeToken: failed when checking if token already used: force"), - http.StatusInternalServerError, context{"ott": raw}}, + http.StatusInternalServerError, apiCtx{"ott": raw}}, } }, "fail/mockNoSQLDB/token-already-used": func(t *testing.T) *authorizeTest { @@ -224,7 +227,7 @@ func TestAuthority_authorizeToken(t *testing.T) { auth: _a, ott: raw, err: &apiError{errors.New("authorizeToken: token already used"), - http.StatusUnauthorized, context{"ott": raw}}, + http.StatusUnauthorized, apiCtx{"ott": raw}}, } }, } @@ -391,7 +394,7 @@ func TestAuthority_AuthorizeSign(t *testing.T) { auth: a, ott: "foo", err: &apiError{errors.New("authorizeSign: authorizeToken: error parsing token"), - http.StatusUnauthorized, context{"ott": "foo"}}, + http.StatusUnauthorized, apiCtx{"ott": "foo"}}, } }, "fail/invalid-subject": func(t *testing.T) *authorizeTest { @@ -409,7 +412,7 @@ func TestAuthority_AuthorizeSign(t *testing.T) { auth: a, ott: raw, err: &apiError{errors.New("authorizeSign: token subject cannot be empty"), - http.StatusUnauthorized, context{"ott": raw}}, + http.StatusUnauthorized, apiCtx{"ott": raw}}, } }, "ok": func(t *testing.T) *authorizeTest { @@ -484,7 +487,7 @@ func TestAuthority_Authorize(t *testing.T) { auth: a, ott: "foo", err: &apiError{errors.New("authorizeSign: authorizeToken: error parsing token"), - http.StatusUnauthorized, context{"ott": "foo"}}, + http.StatusUnauthorized, apiCtx{"ott": "foo"}}, } }, "fail/invalid-subject": func(t *testing.T) *authorizeTest { @@ -502,7 +505,7 @@ func TestAuthority_Authorize(t *testing.T) { auth: a, ott: raw, err: &apiError{errors.New("authorizeSign: token subject cannot be empty"), - http.StatusUnauthorized, context{"ott": raw}}, + http.StatusUnauthorized, apiCtx{"ott": raw}}, } }, "ok": func(t *testing.T) *authorizeTest { @@ -526,8 +529,8 @@ func TestAuthority_Authorize(t *testing.T) { for name, genTestCase := range tests { t.Run(name, func(t *testing.T) { tc := genTestCase(t) - - got, err := tc.auth.Authorize(tc.ott) + ctx := provisioner.NewContextWithMethod(context.Background(), provisioner.SignMethod) + got, err := tc.auth.Authorize(ctx, tc.ott) if err != nil { if assert.NotNil(t, tc.err) { assert.Nil(t, got) @@ -577,7 +580,7 @@ func TestAuthority_authorizeRenewal(t *testing.T) { auth: a, crt: fooCrt, err: &apiError{errors.New("renew: force"), - http.StatusInternalServerError, context{"serialNumber": "102012593071130646873265215610956555026"}}, + http.StatusInternalServerError, apiCtx{"serialNumber": "102012593071130646873265215610956555026"}}, } }, "fail/revoked": func(t *testing.T) *authorizeTest { @@ -591,7 +594,7 @@ func TestAuthority_authorizeRenewal(t *testing.T) { auth: a, crt: fooCrt, err: &apiError{errors.New("renew: certificate has been revoked"), - http.StatusUnauthorized, context{"serialNumber": "102012593071130646873265215610956555026"}}, + http.StatusUnauthorized, apiCtx{"serialNumber": "102012593071130646873265215610956555026"}}, } }, "fail/load-provisioner": func(t *testing.T) *authorizeTest { @@ -605,7 +608,7 @@ func TestAuthority_authorizeRenewal(t *testing.T) { auth: a, crt: otherCrt, err: &apiError{errors.New("renew: provisioner not found"), - http.StatusUnauthorized, context{"serialNumber": "41633491264736369593451462439668497527"}}, + http.StatusUnauthorized, apiCtx{"serialNumber": "41633491264736369593451462439668497527"}}, } }, "fail/provisioner-authorize-renewal-fail": func(t *testing.T) *authorizeTest { @@ -620,7 +623,7 @@ func TestAuthority_authorizeRenewal(t *testing.T) { auth: a, crt: renewDisabledCrt, err: &apiError{errors.New("renew: renew is disabled for provisioner renew_disabled:IMi94WBNI6gP5cNHXlZYNUzvMjGdHyBRmFoo-lCEaqk"), - http.StatusUnauthorized, context{"serialNumber": "119772236532068856521070735128919532568"}}, + http.StatusUnauthorized, apiCtx{"serialNumber": "119772236532068856521070735128919532568"}}, } }, "ok": func(t *testing.T) *authorizeTest { diff --git a/authority/provisioners_test.go b/authority/provisioners_test.go index 303c4e8a..fb84a31d 100644 --- a/authority/provisioners_test.go +++ b/authority/provisioners_test.go @@ -35,7 +35,7 @@ func TestGetEncryptedKey(t *testing.T) { a: a, kid: "foo", err: &apiError{errors.Errorf("encrypted key with kid foo was not found"), - http.StatusNotFound, context{}}, + http.StatusNotFound, apiCtx{}}, } }, } diff --git a/authority/root_test.go b/authority/root_test.go index d4caf71a..4b648d78 100644 --- a/authority/root_test.go +++ b/authority/root_test.go @@ -19,8 +19,8 @@ func TestRoot(t *testing.T) { sum string err *apiError }{ - "not-found": {"foo", &apiError{errors.New("certificate with fingerprint foo was not found"), http.StatusNotFound, context{}}}, - "invalid-stored-certificate": {"invaliddata", &apiError{errors.New("stored value is not a *x509.Certificate"), http.StatusInternalServerError, context{}}}, + "not-found": {"foo", &apiError{errors.New("certificate with fingerprint foo was not found"), http.StatusNotFound, apiCtx{}}}, + "invalid-stored-certificate": {"invaliddata", &apiError{errors.New("stored value is not a *x509.Certificate"), http.StatusInternalServerError, apiCtx{}}}, "success": {"189f573cfa159251e445530847ef80b1b62a3a380ee670dcb49e33ed34da0616", nil}, } diff --git a/authority/tls_test.go b/authority/tls_test.go index 142eedde..7aaa453a 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -1,6 +1,7 @@ package authority import ( + "context" "crypto/rand" "crypto/sha1" "crypto/x509" @@ -102,7 +103,8 @@ func TestSign(t *testing.T) { assert.FatalError(t, err) token, err := generateToken("smallstep test", "step-cli", "https://test.ca.smallstep.com/sign", []string{"test.smallstep.com"}, time.Now(), key) assert.FatalError(t, err) - extraOpts, err := a.Authorize(token) + ctx := provisioner.NewContextWithMethod(context.Background(), provisioner.SignMethod) + extraOpts, err := a.Authorize(ctx, token) assert.FatalError(t, err) type signTest struct { @@ -123,7 +125,7 @@ func TestSign(t *testing.T) { signOpts: signOpts, err: &apiError{errors.New("sign: invalid certificate request"), http.StatusBadRequest, - context{"csr": csr, "signOptions": signOpts}, + apiCtx{"csr": csr, "signOptions": signOpts}, }, } }, @@ -137,7 +139,7 @@ func TestSign(t *testing.T) { signOpts: signOpts, err: &apiError{errors.New("sign: invalid extra option type string"), http.StatusInternalServerError, - context{"csr": csr, "signOptions": signOpts}, + apiCtx{"csr": csr, "signOptions": signOpts}, }, } }, @@ -152,7 +154,7 @@ func TestSign(t *testing.T) { signOpts: signOpts, err: &apiError{errors.New("sign: default ASN1DN template cannot be nil"), http.StatusInternalServerError, - context{"csr": csr, "signOptions": signOpts}, + apiCtx{"csr": csr, "signOptions": signOpts}, }, } }, @@ -167,7 +169,7 @@ func TestSign(t *testing.T) { signOpts: signOpts, err: &apiError{errors.New("sign: error creating new leaf certificate"), http.StatusInternalServerError, - context{"csr": csr, "signOptions": signOpts}, + apiCtx{"csr": csr, "signOptions": signOpts}, }, } }, @@ -184,7 +186,7 @@ func TestSign(t *testing.T) { signOpts: _signOpts, err: &apiError{errors.New("sign: requested duration of 25h0m0s is more than the authorized maximum certificate duration of 24h0m0s"), http.StatusUnauthorized, - context{"csr": csr, "signOptions": _signOpts}, + apiCtx{"csr": csr, "signOptions": _signOpts}, }, } }, @@ -199,7 +201,7 @@ func TestSign(t *testing.T) { signOpts: signOpts, err: &apiError{errors.New("sign: certificate request does not contain the valid DNS names - got [test.smallstep.com smallstep test], want [test.smallstep.com]"), http.StatusUnauthorized, - context{"csr": csr, "signOptions": signOpts}, + apiCtx{"csr": csr, "signOptions": signOpts}, }, } }, @@ -210,7 +212,7 @@ func TestSign(t *testing.T) { storeCertificate: func(crt *x509.Certificate) error { return &apiError{errors.New("force"), http.StatusInternalServerError, - context{"csr": csr, "signOptions": signOpts}} + apiCtx{"csr": csr, "signOptions": signOpts}} }, } return &signTest{ @@ -220,7 +222,7 @@ func TestSign(t *testing.T) { signOpts: signOpts, err: &apiError{errors.New("sign: error storing certificate in db: force"), http.StatusInternalServerError, - context{"csr": csr, "signOptions": signOpts}, + apiCtx{"csr": csr, "signOptions": signOpts}, }, } }, @@ -373,7 +375,7 @@ func TestRenew(t *testing.T) { auth: _a, crt: crt, err: &apiError{errors.New("error renewing certificate from existing server certificate"), - http.StatusInternalServerError, context{}}, + http.StatusInternalServerError, apiCtx{}}, }, nil }, "fail-unauthorized": func() (*renewTest, error) { @@ -568,7 +570,7 @@ func TestRevoke(t *testing.T) { validAudience := []string{"https://test.ca.smallstep.com/revoke"} now := time.Now().UTC() getCtx := func() map[string]interface{} { - return context{ + return apiCtx{ "serialNumber": "sn", "reasonCode": reasonCode, "reason": reason, From ba2ba5492840c63e95ac47e6a3e3b8bc79a3420c Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 12:52:13 -0700 Subject: [PATCH 07/40] Adapt api package to new interfaces. --- api/api.go | 3 ++- api/api_test.go | 18 ++++++++++++++---- api/ssh.go | 4 +++- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/api/api.go b/api/api.go index a001dac6..4639e20c 100644 --- a/api/api.go +++ b/api/api.go @@ -1,6 +1,7 @@ package api import ( + "context" "crypto/dsa" "crypto/ecdsa" "crypto/rsa" @@ -29,7 +30,7 @@ type Authority interface { SSHAuthority // NOTE: Authorize will be deprecated in future releases. Please use the // context specific Authoirize[Sign|Revoke|etc.] methods. - Authorize(ott string) ([]provisioner.SignOption, error) + Authorize(ctx context.Context, ott string) ([]provisioner.SignOption, error) AuthorizeSign(ott string) ([]provisioner.SignOption, error) GetTLSOptions() *tlsutil.TLSOptions Root(shasum string) (*x509.Certificate, error) diff --git a/api/api_test.go b/api/api_test.go index 3f99dd68..7a3c843d 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -23,6 +23,8 @@ import ( "testing" "time" + "golang.org/x/crypto/ssh" + "github.com/go-chi/chi" "github.com/smallstep/certificates/authority" "github.com/smallstep/certificates/authority/provisioner" @@ -418,7 +420,7 @@ type mockProvisioner struct { getEncryptedKey func() (string, string, bool) init func(provisioner.Config) error authorizeRevoke func(ott string) error - authorizeSign func(ott string) ([]provisioner.SignOption, error) + authorizeSign func(ctx context.Context, ott string) ([]provisioner.SignOption, error) authorizeRenewal func(*x509.Certificate) error } @@ -474,9 +476,9 @@ func (m *mockProvisioner) AuthorizeRevoke(ott string) error { return m.err } -func (m *mockProvisioner) AuthorizeSign(ott string) ([]provisioner.SignOption, error) { +func (m *mockProvisioner) AuthorizeSign(ctx context.Context, ott string) ([]provisioner.SignOption, error) { if m.authorizeSign != nil { - return m.authorizeSign(ott) + return m.authorizeSign(ctx, ott) } return m.ret1.([]provisioner.SignOption), m.err } @@ -495,6 +497,7 @@ type mockAuthority struct { getTLSOptions func() *tlsutil.TLSOptions root func(shasum string) (*x509.Certificate, error) sign func(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error) + singSSH func(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) renew func(cert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error) loadProvisionerByCertificate func(cert *x509.Certificate) (provisioner.Interface, error) getProvisioners func(nextCursor string, limit int) (provisioner.List, string, error) @@ -505,7 +508,7 @@ type mockAuthority struct { } // TODO: remove once Authorize is deprecated. -func (m *mockAuthority) Authorize(ott string) ([]provisioner.SignOption, error) { +func (m *mockAuthority) Authorize(ctx context.Context, ott string) ([]provisioner.SignOption, error) { return m.AuthorizeSign(ott) } @@ -537,6 +540,13 @@ func (m *mockAuthority) Sign(cr *x509.CertificateRequest, opts provisioner.Optio return m.ret1.(*x509.Certificate), m.ret2.(*x509.Certificate), m.err } +func (m *mockAuthority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) { + if m.singSSH != nil { + return m.singSSH(key, opts, signOpts...) + } + return m.ret1.(*ssh.Certificate), m.err +} + func (m *mockAuthority) Renew(cert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error) { if m.renew != nil { return m.renew(cert) diff --git a/api/ssh.go b/api/ssh.go index de91b559..7e730bc3 100644 --- a/api/ssh.go +++ b/api/ssh.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/base64" "encoding/json" "net/http" @@ -111,7 +112,8 @@ func (h *caHandler) SignSSH(w http.ResponseWriter, r *http.Request) { ValidAfter: body.ValidAfter, } - signOpts, err := h.Authority.AuthorizeSign(body.OTT) + ctx := provisioner.NewContextWithMethod(context.Background(), provisioner.SignSSHMethod) + signOpts, err := h.Authority.Authorize(ctx, body.OTT) if err != nil { WriteError(w, Unauthorized(err)) return From 7a64a84761cbe9ec55cbe9bb550ec52dbee4469f Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 15:53:09 -0700 Subject: [PATCH 08/40] Pass the given context. --- authority/authorize.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authority/authorize.go b/authority/authorize.go index b354fe84..6ce2d5bd 100644 --- a/authority/authorize.go +++ b/authority/authorize.go @@ -86,7 +86,7 @@ func (a *Authority) Authorize(ctx context.Context, ott string) ([]provisioner.Si // Call the provisioner AuthorizeSign method to apply provisioner specific // auth claims and get the signing options. - opts, err := p.AuthorizeSign(context.Background(), ott) + opts, err := p.AuthorizeSign(ctx, ott) if err != nil { return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext} } From a44b0a1d52dbc7f875458c41bf20c4d733fb52a1 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 15:53:43 -0700 Subject: [PATCH 09/40] Fix typo --- api/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/api.go b/api/api.go index 4639e20c..fd091c86 100644 --- a/api/api.go +++ b/api/api.go @@ -29,7 +29,7 @@ import ( type Authority interface { SSHAuthority // NOTE: Authorize will be deprecated in future releases. Please use the - // context specific Authoirize[Sign|Revoke|etc.] methods. + // context specific Authorize[Sign|Revoke|etc.] methods. Authorize(ctx context.Context, ott string) ([]provisioner.SignOption, error) AuthorizeSign(ott string) ([]provisioner.SignOption, error) GetTLSOptions() *tlsutil.TLSOptions From f01286bb48d1629e975d0a70c37cc63ca244458c Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 15:54:07 -0700 Subject: [PATCH 10/40] Add support for SSH certificates to OIDC. Update the interface for all the provisioners. --- authority/provisioner/aws.go | 15 +++++- authority/provisioner/azure.go | 13 ++++- authority/provisioner/gcp.go | 15 +++++- authority/provisioner/jwk.go | 4 +- authority/provisioner/method.go | 34 +++++++++++++ authority/provisioner/noop.go | 7 ++- authority/provisioner/oidc.go | 58 ++++++++++++++++++++++- authority/provisioner/provisioner.go | 6 ++- authority/provisioner/sign_ssh_options.go | 8 ++-- 9 files changed, 147 insertions(+), 13 deletions(-) create mode 100644 authority/provisioner/method.go diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go index 8b288e3d..738a2d33 100644 --- a/authority/provisioner/aws.go +++ b/authority/provisioner/aws.go @@ -1,6 +1,7 @@ package provisioner import ( + "context" "crypto/sha256" "crypto/x509" "encoding/base64" @@ -266,13 +267,18 @@ func (p *AWS) Init(config Config) (err error) { // AuthorizeSign validates the given token and returns the sign options that // will be used on certificate creation. -func (p *AWS) AuthorizeSign(token string) ([]SignOption, error) { +func (p *AWS) AuthorizeSign(ctx context.Context, token string) ([]SignOption, error) { payload, err := p.authorizeToken(token) if err != nil { return nil, err } - doc := payload.document + // Check for the sign ssh method, default to sign X.509 + if m := MethodFromContext(ctx); m == SignSSHMethod { + return p.authorizeSSHSign(payload) + } + + doc := payload.document // Enforce known CN and default DNS and IP if configured. // By default we'll accept the CN and SANs in the CSR. // There's no way to trust them other than TOFU. @@ -432,3 +438,8 @@ func (p *AWS) authorizeToken(token string) (*awsPayload, error) { payload.document = doc return &payload, nil } + +// authorizeSSHSign returns the list of SignOption for a SignSSH request. +func (p *AWS) authorizeSSHSign(claims *awsPayload) ([]SignOption, error) { + return nil, nil +} diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go index ee7fb744..9e6a41a7 100644 --- a/authority/provisioner/azure.go +++ b/authority/provisioner/azure.go @@ -1,6 +1,7 @@ package provisioner import ( + "context" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -209,7 +210,7 @@ func (p *Azure) Init(config Config) (err error) { // AuthorizeSign validates the given token and returns the sign options that // will be used on certificate creation. -func (p *Azure) AuthorizeSign(token string) ([]SignOption, error) { +func (p *Azure) AuthorizeSign(ctx context.Context, token string) ([]SignOption, error) { jwt, err := jose.ParseSigned(token) if err != nil { return nil, errors.Wrapf(err, "error parsing token") @@ -264,6 +265,11 @@ func (p *Azure) AuthorizeSign(token string) ([]SignOption, error) { } } + // Check for the sign ssh method, default to sign X.509 + if m := MethodFromContext(ctx); m == SignSSHMethod { + return p.authorizeSSHSign(claims) + } + // Enforce known common name and default DNS if configured. // By default we'll accept the CN and SANs in the CSR. // There's no way to trust them other than TOFU. @@ -295,6 +301,11 @@ func (p *Azure) AuthorizeRevoke(token string) error { return errors.New("revoke is not supported on a Azure provisioner") } +// authorizeSSHSign returns the list of SignOption for a SignSSH request. +func (p *Azure) authorizeSSHSign(claims azurePayload) ([]SignOption, error) { + return nil, nil +} + // assertConfig initializes the config if it has not been initialized func (p *Azure) assertConfig() { if p.config == nil { diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go index 234b00fe..b50078c1 100644 --- a/authority/provisioner/gcp.go +++ b/authority/provisioner/gcp.go @@ -2,6 +2,7 @@ package provisioner import ( "bytes" + "context" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -205,13 +206,18 @@ func (p *GCP) Init(config Config) error { // AuthorizeSign validates the given token and returns the sign options that // will be used on certificate creation. -func (p *GCP) AuthorizeSign(token string) ([]SignOption, error) { +func (p *GCP) AuthorizeSign(ctx context.Context, token string) ([]SignOption, error) { claims, err := p.authorizeToken(token) if err != nil { return nil, err } - ce := claims.Google.ComputeEngine + // Check for the sign ssh method, default to sign X.509 + if m := MethodFromContext(ctx); m == SignSSHMethod { + return p.authorizeSSHSign(claims) + } + + ce := claims.Google.ComputeEngine // Enforce known common name and default DNS if configured. // By default we we'll accept the CN and SANs in the CSR. // There's no way to trust them other than TOFU. @@ -344,3 +350,8 @@ func (p *GCP) authorizeToken(token string) (*gcpPayload, error) { return &claims, nil } + +// authorizeSSHSign returns the list of SignOption for a SignSSH request. +func (p *GCP) authorizeSSHSign(claims *gcpPayload) ([]SignOption, error) { + return nil, nil +} diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index a21815b9..7c983b74 100644 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -1,6 +1,7 @@ package provisioner import ( + "context" "crypto/x509" "time" @@ -134,7 +135,7 @@ func (p *JWK) AuthorizeRevoke(token string) error { } // AuthorizeSign validates the given token. -func (p *JWK) AuthorizeSign(token string) ([]SignOption, error) { +func (p *JWK) AuthorizeSign(ctx context.Context, token string) ([]SignOption, error) { claims, err := p.authorizeToken(token, p.audiences.Sign) if err != nil { return nil, err @@ -171,6 +172,7 @@ func (p *JWK) AuthorizeRenewal(cert *x509.Certificate) error { return nil } +// authorizeSSHSign returns the list of SignOption for a SignSSH request. func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) { t := now() opts := claims.Step.SSH diff --git a/authority/provisioner/method.go b/authority/provisioner/method.go new file mode 100644 index 00000000..c8f96885 --- /dev/null +++ b/authority/provisioner/method.go @@ -0,0 +1,34 @@ +package provisioner + +import ( + "context" +) + +// Method indicates the action to action that we will perform, it's used as part +// of the context in the call to authorize. It defaults to Sing. +type Method int + +// The key to save the Method in the context. +type methodKey struct{} + +const ( + // SignMethod is the method used to sign X.509 certificates. + SignMethod Method = iota + // SignSSHMethod is the method used to sign SSH certificate. + SignSSHMethod + // RevokeMethod is the method used to revoke X.509 certificates. + RevokeMethod +) + +// NewContextWithMethod creates a new context from ctx and attaches method to +// it. +func NewContextWithMethod(ctx context.Context, method Method) context.Context { + return context.WithValue(ctx, methodKey{}, method) +} + +// MethodFromContext returns the Method saved in ctx. Returns Sign if the given +// context has no Method associated with it. +func MethodFromContext(ctx context.Context) Method { + m, _ := ctx.Value(methodKey{}).(Method) + return m +} diff --git a/authority/provisioner/noop.go b/authority/provisioner/noop.go index 44fd4600..5bdc0677 100644 --- a/authority/provisioner/noop.go +++ b/authority/provisioner/noop.go @@ -1,6 +1,9 @@ package provisioner -import "crypto/x509" +import ( + "context" + "crypto/x509" +) // noop provisioners is a provisioner that accepts anything. type noop struct{} @@ -28,7 +31,7 @@ func (p *noop) Init(config Config) error { return nil } -func (p *noop) AuthorizeSign(token string) ([]SignOption, error) { +func (p *noop) AuthorizeSign(ctx context.Context, token string) ([]SignOption, error) { return []SignOption{}, nil } diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index 0b2e2700..dcddc7ff 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -1,6 +1,7 @@ package provisioner import ( + "context" "crypto/x509" "encoding/json" "net/http" @@ -259,11 +260,17 @@ func (o *OIDC) AuthorizeRevoke(token string) error { } // AuthorizeSign validates the given token. -func (o *OIDC) AuthorizeSign(token string) ([]SignOption, error) { +func (o *OIDC) AuthorizeSign(ctx context.Context, token string) ([]SignOption, error) { claims, err := o.authorizeToken(token) if err != nil { return nil, err } + + // Check for the sign ssh method, default to sign X.509 + if m := MethodFromContext(ctx); m == SignSSHMethod { + return o.authorizeSSHSign(claims) + } + // Admins should be able to authorize any SAN if o.IsAdmin(claims.Email) { return []SignOption{ @@ -289,6 +296,37 @@ func (o *OIDC) AuthorizeRenewal(cert *x509.Certificate) error { return nil } +// authorizeSSHSign returns the list of SignOption for a SignSSH request. +func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) { + signOptions := []SignOption{ + // set the default extensions + &sshDefaultExtensionModifier{}, + // set the key id to the token subject + sshCertificateKeyIDModifier(claims.Email), + } + + // Non-admins are only able to sign user certificates + if o.IsAdmin(claims.Email) { + signOptions = append(signOptions, &sshCertificateOptionsValidator{}) + } else { + name := principalFromEmail(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}, + }}) + } + + return append(signOptions, + // checks the validity bounds, and set the validity if has not been set + &sshCertificateValidityModifier{o.claimer}, + // require all the fields in the SSH certificate + &sshCertificateDefaultValidator{}, + ), nil +} + func getAndDecode(uri string, v interface{}) error { resp, err := http.Get(uri) if err != nil { @@ -300,3 +338,21 @@ func getAndDecode(uri string, v interface{}) error { } return nil } + +func principalFromEmail(email string) string { + if i := strings.LastIndex(email, "@"); i >= 0 { + email = email[:i] + } + return strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z': + return r + case r >= '0' && r <= '9': + return r + case r == '.': // drop dots + return -1 + default: + return '_' + } + }, strings.ToLower(email)) +} diff --git a/authority/provisioner/provisioner.go b/authority/provisioner/provisioner.go index 711b0439..0fd3f440 100644 --- a/authority/provisioner/provisioner.go +++ b/authority/provisioner/provisioner.go @@ -1,14 +1,18 @@ package provisioner import ( + "context" "crypto/x509" "encoding/json" "net/url" + "regexp" "strings" "github.com/pkg/errors" ) +var sshUserRegex = regexp.MustCompile("^[a-z][-a-z0-9_]*$") + // Interface is the interface that all provisioner types must implement. type Interface interface { GetID() string @@ -17,7 +21,7 @@ type Interface interface { GetType() Type GetEncryptedKey() (kid string, key string, ok bool) Init(config Config) error - AuthorizeSign(token string) ([]SignOption, error) + AuthorizeSign(ctx context.Context, token string) ([]SignOption, error) AuthorizeRenewal(cert *x509.Certificate) error AuthorizeRevoke(token string) error } diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 1b981504..6825131d 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -205,11 +205,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 { - *SSHOptions + Want *SSHOptions } -func (want *sshCertificateOptionsValidator) Valid(got SSHOptions) error { - return want.match(got) +// Valid implements SSHCertificateOptionsValidator and returns nil if both +// SSHOptions match. +func (v *sshCertificateOptionsValidator) Valid(got SSHOptions) error { + return v.Want.match(got) } // sshCertificateDefaultValidator implements a simple validator for all the From 48c98dea2a9482a68270d74588910192e0f32ea1 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 16:21:22 -0700 Subject: [PATCH 11/40] Make SanitizeSSHPrincipal a public function. --- authority/provisioner/oidc.go | 20 +--------------- authority/provisioner/provisioner.go | 28 +++++++++++++++++++++-- authority/provisioner/sign_ssh_options.go | 3 +++ 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index dcddc7ff..a454a84f 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -309,7 +309,7 @@ func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) { if o.IsAdmin(claims.Email) { signOptions = append(signOptions, &sshCertificateOptionsValidator{}) } else { - name := principalFromEmail(claims.Email) + name := SanitizeSSHPrincipal(claims.Email) if !sshUserRegex.MatchString(name) { return nil, errors.Errorf("invalid principal '%s' from email address '%s'", name, claims.Email) } @@ -338,21 +338,3 @@ func getAndDecode(uri string, v interface{}) error { } return nil } - -func principalFromEmail(email string) string { - if i := strings.LastIndex(email, "@"); i >= 0 { - email = email[:i] - } - return strings.Map(func(r rune) rune { - switch { - case r >= 'a' && r <= 'z': - return r - case r >= '0' && r <= '9': - return r - case r == '.': // drop dots - return -1 - default: - return '_' - } - }, strings.ToLower(email)) -} diff --git a/authority/provisioner/provisioner.go b/authority/provisioner/provisioner.go index 0fd3f440..bb59bbb9 100644 --- a/authority/provisioner/provisioner.go +++ b/authority/provisioner/provisioner.go @@ -11,8 +11,6 @@ import ( "github.com/pkg/errors" ) -var sshUserRegex = regexp.MustCompile("^[a-z][-a-z0-9_]*$") - // Interface is the interface that all provisioner types must implement. type Interface interface { GetID() string @@ -164,3 +162,29 @@ func (l *List) UnmarshalJSON(data []byte) error { return nil } + +var sshUserRegex = regexp.MustCompile("^[a-z][-a-z0-9_]*$") + +// SanitizeSSHPrincipal grabs an email or a string with the format local@domain +// and returns a sanitized version of the local, valid to be used as a user +// name. If the email starts with a letter between a and z, the resulting string +// will match the regular expression `^[a-z][-a-z0-9_]*$`. +func SanitizeSSHPrincipal(email string) string { + if i := strings.LastIndex(email, "@"); i >= 0 { + email = email[:i] + } + return strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z': + return r + case r >= '0' && r <= '9': + return r + case r == '-': + return '-' + case r == '.': // drop dots + return -1 + default: + return '_' + } + }, strings.ToLower(email)) +} diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 6825131d..8039aabb 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -211,6 +211,9 @@ type sshCertificateOptionsValidator struct { // 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) } From 53f62f871cc324a1bf4675c140f993837df43980 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 16:36:46 -0700 Subject: [PATCH 12/40] Set not extensions to host certificates. --- authority/provisioner/jwk.go | 4 ++-- authority/provisioner/oidc.go | 4 ++-- authority/provisioner/sign_ssh_options.go | 26 +++++++++++++++-------- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index 7c983b74..ff53cce8 100644 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -179,8 +179,6 @@ func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) { signOptions := []SignOption{ // validates user's SSHOptions with the ones in the token &sshCertificateOptionsValidator{opts}, - // set the default extensions - &sshDefaultExtensionModifier{}, // set the key id to the token subject sshCertificateKeyIDModifier(claims.Subject), } @@ -200,6 +198,8 @@ func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) { } return append(signOptions, + // set the default extensions + &sshDefaultExtensionModifier{}, // checks the validity bounds, and set the validity if has not been set &sshCertificateValidityModifier{p.claimer}, // require all the fields in the SSH certificate diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index a454a84f..bc108c01 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -299,8 +299,6 @@ func (o *OIDC) AuthorizeRenewal(cert *x509.Certificate) error { // authorizeSSHSign returns the list of SignOption for a SignSSH request. func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) { signOptions := []SignOption{ - // set the default extensions - &sshDefaultExtensionModifier{}, // set the key id to the token subject sshCertificateKeyIDModifier(claims.Email), } @@ -320,6 +318,8 @@ func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) { } return append(signOptions, + // set the default extensions + &sshDefaultExtensionModifier{}, // checks the validity bounds, and set the validity if has not been set &sshCertificateValidityModifier{o.claimer}, // require all the fields in the SSH certificate diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 8039aabb..b5d79ffa 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -148,15 +148,23 @@ func (m sshCertificateValidBeforeModifier) Modify(cert *ssh.Certificate) error { type sshDefaultExtensionModifier struct{} func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error { - if cert.Extensions == nil { - cert.Extensions = make(map[string]string) + switch cert.CertType { + // Default to no extensions to HostCert + case ssh.HostCert: + return nil + case ssh.UserCert: + if cert.Extensions == nil { + cert.Extensions = make(map[string]string) + } + cert.Extensions["permit-X11-forwarding"] = "" + cert.Extensions["permit-agent-forwarding"] = "" + cert.Extensions["permit-port-forwarding"] = "" + cert.Extensions["permit-pty"] = "" + cert.Extensions["permit-user-rc"] = "" + return nil + default: + return errors.New("ssh certificate type has not been set or is invalid") } - cert.Extensions["permit-X11-forwarding"] = "" - cert.Extensions["permit-agent-forwarding"] = "" - cert.Extensions["permit-port-forwarding"] = "" - cert.Extensions["permit-pty"] = "" - cert.Extensions["permit-user-rc"] = "" - return nil } // sshCertificateValidityModifier is a SSHCertificateModifier checks the @@ -240,7 +248,7 @@ func (v *sshCertificateDefaultValidator) Valid(crt *ssh.Certificate) error { return errors.New("ssh certificate valid after cannot be 0") case crt.ValidBefore == 0: return errors.New("ssh certificate valid before cannot be 0") - case len(crt.Extensions) == 0: + case crt.CertType == ssh.UserCert && len(crt.Extensions) == 0: return errors.New("ssh certificate extensions cannot be empty") case crt.SignatureKey == nil: return errors.New("ssh certificate signature key cannot be nil") From 41b97372e62775c72eec40a264ffef268375838f Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 16:38:57 -0700 Subject: [PATCH 13/40] Rename function to SanitizeSSHUserPrincipal --- authority/provisioner/oidc.go | 2 +- authority/provisioner/provisioner.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index bc108c01..8c89ea37 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -307,7 +307,7 @@ func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) { if o.IsAdmin(claims.Email) { signOptions = append(signOptions, &sshCertificateOptionsValidator{}) } else { - name := SanitizeSSHPrincipal(claims.Email) + name := SanitizeSSHUserPrincipal(claims.Email) if !sshUserRegex.MatchString(name) { return nil, errors.Errorf("invalid principal '%s' from email address '%s'", name, claims.Email) } diff --git a/authority/provisioner/provisioner.go b/authority/provisioner/provisioner.go index bb59bbb9..96bec692 100644 --- a/authority/provisioner/provisioner.go +++ b/authority/provisioner/provisioner.go @@ -165,11 +165,11 @@ func (l *List) UnmarshalJSON(data []byte) error { var sshUserRegex = regexp.MustCompile("^[a-z][-a-z0-9_]*$") -// SanitizeSSHPrincipal grabs an email or a string with the format local@domain -// and returns a sanitized version of the local, valid to be used as a user -// name. If the email starts with a letter between a and z, the resulting string -// will match the regular expression `^[a-z][-a-z0-9_]*$`. -func SanitizeSSHPrincipal(email string) string { +// SanitizeSSHUserPrincipal grabs an email or a string with the format +// local@domain and returns a sanitized version of the local, valid to be used +// as a user name. If the email starts with a letter between a and z, the +// resulting string will match the regular expression `^[a-z][-a-z0-9_]*$`. +func SanitizeSSHUserPrincipal(email string) string { if i := strings.LastIndex(email, "@"); i >= 0 { email = email[:i] } From 7583f1c73927efc2e1b8ec5047b05f009081de18 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 17:54:13 -0700 Subject: [PATCH 14/40] Do not require all principals, allow subgroups. --- authority/provisioner/sign_ssh_options.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index b5d79ffa..ead5b9a9 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -86,7 +86,7 @@ func (o SSHOptions) match(got SSHOptions) error { if o.CertType != "" && got.CertType != "" && o.CertType != got.CertType { return errors.Errorf("ssh certificate type does not match - got %v, want %v", got.CertType, o.CertType) } - if len(o.Principals) > 0 && len(got.Principals) > 0 && !equalStringSlice(o.Principals, got.Principals) { + if len(o.Principals) > 0 && len(got.Principals) > 0 && !containsAllMembers(o.Principals, got.Principals) { return errors.Errorf("ssh certificate principals does not match - got %v, want %v", got.Principals, o.Principals) } if !o.ValidAfter.IsZero() && !got.ValidAfter.IsZero() && !o.ValidAfter.Equal(&got.ValidAfter) { @@ -285,17 +285,18 @@ func sshCertTypeUInt32(ct string) uint32 { } } -func equalStringSlice(a, b []string) bool { - var l int - if l = len(a); l != len(b) { +// containsAllMembers reports whether all members of subgroup are within group. +func containsAllMembers(group, subgroup []string) bool { + lg, lsg := len(group), len(subgroup) + if lsg > lg { return false } - visit := make(map[string]struct{}, l) - for i := 0; i < l; i++ { - visit[a[i]] = struct{}{} + visit := make(map[string]struct{}, lg) + for i := 0; i < lg; i++ { + visit[group[i]] = struct{}{} } - for i := 0; i < l; i++ { - if _, ok := visit[b[i]]; !ok { + for i := 0; i < lsg; i++ { + if _, ok := visit[group[i]]; !ok { return false } } From 7d670b20eaad043c81e48b3805849f4dea36118c Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 17:54:38 -0700 Subject: [PATCH 15/40] Add support of ssh host certinficates in AWS provisioner. --- authority/provisioner/aws.go | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go index 738a2d33..03a0747e 100644 --- a/authority/provisioner/aws.go +++ b/authority/provisioner/aws.go @@ -441,5 +441,27 @@ func (p *AWS) authorizeToken(token string) (*awsPayload, error) { // authorizeSSHSign returns the list of SignOption for a SignSSH request. func (p *AWS) authorizeSSHSign(claims *awsPayload) ([]SignOption, error) { - return nil, nil + doc := claims.document + + signOptions := []SignOption{ + // set the key id to the token subject + sshCertificateKeyIDModifier(claims.Subject), + } + + signOptions = append(signOptions, &sshCertificateOptionsValidator{&SSHOptions{ + CertType: SSHHostCert, + Principals: []string{ + doc.PrivateIP, + fmt.Sprintf("ip-%s.%s.compute.internal", strings.Replace(doc.PrivateIP, ".", "-", -1), doc.Region), + }, + }}) + + return append(signOptions, + // set the default extensions + &sshDefaultExtensionModifier{}, + // checks the validity bounds, and set the validity if has not been set + &sshCertificateValidityModifier{p.claimer}, + // require all the fields in the SSH certificate + &sshCertificateDefaultValidator{}, + ), nil } From aef52e433457a3a79acefef083363a3224bbe911 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 18:01:20 -0700 Subject: [PATCH 16/40] Add support for SSH host certificates in azure. --- authority/provisioner/azure.go | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go index 9e6a41a7..611aa58a 100644 --- a/authority/provisioner/azure.go +++ b/authority/provisioner/azure.go @@ -267,7 +267,7 @@ func (p *Azure) AuthorizeSign(ctx context.Context, token string) ([]SignOption, // Check for the sign ssh method, default to sign X.509 if m := MethodFromContext(ctx); m == SignSSHMethod { - return p.authorizeSSHSign(claims) + return p.authorizeSSHSign(claims, name) } // Enforce known common name and default DNS if configured. @@ -302,8 +302,25 @@ func (p *Azure) AuthorizeRevoke(token string) error { } // authorizeSSHSign returns the list of SignOption for a SignSSH request. -func (p *Azure) authorizeSSHSign(claims azurePayload) ([]SignOption, error) { - return nil, nil +func (p *Azure) authorizeSSHSign(claims azurePayload, name string) ([]SignOption, error) { + signOptions := []SignOption{ + // set the key id to the token subject + sshCertificateKeyIDModifier(claims.Subject), + } + + signOptions = append(signOptions, &sshCertificateOptionsValidator{&SSHOptions{ + CertType: SSHHostCert, + Principals: []string{name}, + }}) + + return append(signOptions, + // set the default extensions + &sshDefaultExtensionModifier{}, + // checks the validity bounds, and set the validity if has not been set + &sshCertificateValidityModifier{p.claimer}, + // require all the fields in the SSH certificate + &sshCertificateDefaultValidator{}, + ), nil } // assertConfig initializes the config if it has not been initialized From 18a285e84702cad2c05efba4458169788c72994a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 18:04:01 -0700 Subject: [PATCH 17/40] Change azure ssh key id. --- authority/provisioner/azure.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go index 611aa58a..f5d04adb 100644 --- a/authority/provisioner/azure.go +++ b/authority/provisioner/azure.go @@ -305,7 +305,7 @@ func (p *Azure) AuthorizeRevoke(token string) error { func (p *Azure) authorizeSSHSign(claims azurePayload, name string) ([]SignOption, error) { signOptions := []SignOption{ // set the key id to the token subject - sshCertificateKeyIDModifier(claims.Subject), + sshCertificateKeyIDModifier(name), } signOptions = append(signOptions, &sshCertificateOptionsValidator{&SSHOptions{ From 221d323b6874ab5266b54d2ca856d0a07e9548ee Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 18:16:52 -0700 Subject: [PATCH 18/40] Fix containsAllMembers --- authority/provisioner/sign_ssh_options.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index ead5b9a9..2ceab061 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -95,6 +95,7 @@ func (o SSHOptions) match(got SSHOptions) error { if !o.ValidBefore.IsZero() && !got.ValidBefore.IsZero() && !o.ValidBefore.Equal(&got.ValidBefore) { return errors.Errorf("ssh certificate valid before does not match - got %v, want %v", got.ValidBefore, o.ValidBefore) } + fmt.Printf("want %+v\ngot %+v\n", o, got) return nil } @@ -288,7 +289,7 @@ func sshCertTypeUInt32(ct string) uint32 { // containsAllMembers reports whether all members of subgroup are within group. func containsAllMembers(group, subgroup []string) bool { lg, lsg := len(group), len(subgroup) - if lsg > lg { + if lsg > lg || (lg > 0 && lsg == 0) { return false } visit := make(map[string]struct{}, lg) @@ -296,7 +297,7 @@ func containsAllMembers(group, subgroup []string) bool { visit[group[i]] = struct{}{} } for i := 0; i < lsg; i++ { - if _, ok := visit[group[i]]; !ok { + if _, ok := visit[subgroup[i]]; !ok { return false } } From b827a59e966837add338391d149c6329231778f6 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 18:17:20 -0700 Subject: [PATCH 19/40] Add SSH host certificate support for GCP provisioner. --- authority/provisioner/gcp.go | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go index b50078c1..4415772d 100644 --- a/authority/provisioner/gcp.go +++ b/authority/provisioner/gcp.go @@ -353,5 +353,27 @@ func (p *GCP) authorizeToken(token string) (*gcpPayload, error) { // authorizeSSHSign returns the list of SignOption for a SignSSH request. func (p *GCP) authorizeSSHSign(claims *gcpPayload) ([]SignOption, error) { - return nil, nil + ce := claims.Google.ComputeEngine + + signOptions := []SignOption{ + // set the key id to the token subject + sshCertificateKeyIDModifier(ce.InstanceName), + } + + signOptions = append(signOptions, &sshCertificateOptionsValidator{&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), + }, + }}) + + return append(signOptions, + // set the default extensions + &sshDefaultExtensionModifier{}, + // checks the validity bounds, and set the validity if has not been set + &sshCertificateValidityModifier{p.claimer}, + // require all the fields in the SSH certificate + &sshCertificateDefaultValidator{}, + ), nil } From f8cacc11b158a00b4e4df5756d3810fda46f86e1 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 29 Jul 2019 18:24:34 -0700 Subject: [PATCH 20/40] Fix tests. --- authority/provisioner/aws_test.go | 4 +++- authority/provisioner/azure_test.go | 4 +++- authority/provisioner/gcp_test.go | 4 +++- authority/provisioner/jwk_test.go | 4 +++- authority/provisioner/noop_test.go | 4 +++- authority/provisioner/oidc_test.go | 4 +++- authority/provisioner/timeduration_test.go | 4 ++-- 7 files changed, 20 insertions(+), 8 deletions(-) diff --git a/authority/provisioner/aws_test.go b/authority/provisioner/aws_test.go index 41793848..af101dcf 100644 --- a/authority/provisioner/aws_test.go +++ b/authority/provisioner/aws_test.go @@ -1,6 +1,7 @@ package provisioner import ( + "context" "crypto/rand" "crypto/rsa" "crypto/sha256" @@ -347,7 +348,8 @@ func TestAWS_AuthorizeSign(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := tt.aws.AuthorizeSign(tt.args.token) + ctx := NewContextWithMethod(context.Background(), SignMethod) + got, err := tt.aws.AuthorizeSign(ctx, tt.args.token) if (err != nil) != tt.wantErr { t.Errorf("AWS.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/authority/provisioner/azure_test.go b/authority/provisioner/azure_test.go index c370682a..dc8e514c 100644 --- a/authority/provisioner/azure_test.go +++ b/authority/provisioner/azure_test.go @@ -1,6 +1,7 @@ package provisioner import ( + "context" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -295,7 +296,8 @@ func TestAzure_AuthorizeSign(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := tt.azure.AuthorizeSign(tt.args.token) + ctx := NewContextWithMethod(context.Background(), SignMethod) + got, err := tt.azure.AuthorizeSign(ctx, tt.args.token) if (err != nil) != tt.wantErr { t.Errorf("Azure.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/authority/provisioner/gcp_test.go b/authority/provisioner/gcp_test.go index ce935526..12e60af0 100644 --- a/authority/provisioner/gcp_test.go +++ b/authority/provisioner/gcp_test.go @@ -1,6 +1,7 @@ package provisioner import ( + "context" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -330,7 +331,8 @@ func TestGCP_AuthorizeSign(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := tt.gcp.AuthorizeSign(tt.args.token) + ctx := NewContextWithMethod(context.Background(), SignMethod) + got, err := tt.gcp.AuthorizeSign(ctx, tt.args.token) if (err != nil) != tt.wantErr { t.Errorf("GCP.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/authority/provisioner/jwk_test.go b/authority/provisioner/jwk_test.go index 21789ca8..09317374 100644 --- a/authority/provisioner/jwk_test.go +++ b/authority/provisioner/jwk_test.go @@ -1,6 +1,7 @@ package provisioner import ( + "context" "crypto/x509" "errors" "strings" @@ -259,7 +260,8 @@ func TestJWK_AuthorizeSign(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got, err := tt.prov.AuthorizeSign(tt.args.token); err != nil { + ctx := NewContextWithMethod(context.Background(), SignMethod) + if got, err := tt.prov.AuthorizeSign(ctx, tt.args.token); err != nil { if assert.NotNil(t, tt.err) { assert.HasPrefix(t, err.Error(), tt.err.Error()) } diff --git a/authority/provisioner/noop_test.go b/authority/provisioner/noop_test.go index 8f25eb8c..a389b6b6 100644 --- a/authority/provisioner/noop_test.go +++ b/authority/provisioner/noop_test.go @@ -1,6 +1,7 @@ package provisioner import ( + "context" "crypto/x509" "testing" @@ -21,7 +22,8 @@ func Test_noop(t *testing.T) { assert.Equals(t, "", key) assert.Equals(t, false, ok) - sigOptions, err := p.AuthorizeSign("foo") + ctx := NewContextWithMethod(context.Background(), SignMethod) + sigOptions, err := p.AuthorizeSign(ctx, "foo") assert.Equals(t, []SignOption{}, sigOptions) assert.Equals(t, nil, err) } diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index 80920f41..8eb6a7c2 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -1,6 +1,7 @@ package provisioner import ( + "context" "crypto/x509" "fmt" "strings" @@ -276,7 +277,8 @@ func TestOIDC_AuthorizeSign(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := tt.prov.AuthorizeSign(tt.args.token) + ctx := NewContextWithMethod(context.Background(), SignMethod) + got, err := tt.prov.AuthorizeSign(ctx, tt.args.token) if (err != nil) != tt.wantErr { t.Errorf("OIDC.Authorize() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/authority/provisioner/timeduration_test.go b/authority/provisioner/timeduration_test.go index 97dd4ce5..642a79ed 100644 --- a/authority/provisioner/timeduration_test.go +++ b/authority/provisioner/timeduration_test.go @@ -137,7 +137,7 @@ func TestTimeDuration_MarshalJSON(t *testing.T) { want []byte wantErr bool }{ - {"null", TimeDuration{}, []byte("null"), false}, + {"empty", TimeDuration{}, []byte(`""`), false}, {"timestamp", TimeDuration{t: tm}, []byte(`"2020-03-14T15:09:26.535897Z"`), false}, {"duration", TimeDuration{d: 1 * time.Hour}, []byte(`"1h0m0s"`), false}, {"fail", TimeDuration{t: time.Date(-1, 0, 0, 0, 0, 0, 0, time.UTC)}, nil, true}, @@ -166,7 +166,7 @@ func TestTimeDuration_UnmarshalJSON(t *testing.T) { want *TimeDuration wantErr bool }{ - {"null", args{[]byte("null")}, &TimeDuration{}, false}, + {"empty", args{[]byte(`""`)}, &TimeDuration{}, false}, {"timestamp", args{[]byte(`"2020-03-14T15:09:26.535897Z"`)}, &TimeDuration{t: time.Unix(1584198566, 535897000).UTC()}, false}, {"duration", args{[]byte(`"1h"`)}, &TimeDuration{d: time.Hour}, false}, {"fail", args{[]byte("123")}, &TimeDuration{}, true}, From ad91842d062e993647b5750aa90c509f562ee442 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 30 Jul 2019 15:28:04 -0700 Subject: [PATCH 21/40] Add test for SanitizeSSHUserPrincipal --- authority/provisioner/provisioner_test.go | 30 ++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/authority/provisioner/provisioner_test.go b/authority/provisioner/provisioner_test.go index 11615e1a..d79c2b69 100644 --- a/authority/provisioner/provisioner_test.go +++ b/authority/provisioner/provisioner_test.go @@ -1,6 +1,8 @@ package provisioner -import "testing" +import ( + "testing" +) func TestType_String(t *testing.T) { tests := []struct { @@ -24,3 +26,29 @@ func TestType_String(t *testing.T) { }) } } + +func TestSanitizeSSHUserPrincipal(t *testing.T) { + type args struct { + email string + } + tests := []struct { + name string + args args + want string + }{ + {"simple", args{"foobar"}, "foobar"}, + {"camelcase", args{"FooBar"}, "foobar"}, + {"email", args{"foo@example.com"}, "foo"}, + {"email with dots", args{"foo.bar.zar@example.com"}, "foobarzar"}, + {"email with dashes", args{"foo-bar-zar@example.com"}, "foo-bar-zar"}, + {"email with underscores", args{"foo_bar_zar@example.com"}, "foo_bar_zar"}, + {"email with symbols", args{"Foo.Bar0123456789!#$%&'*+-/=?^_`{|}~;@example.com"}, "foobar0123456789________-___________"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := SanitizeSSHUserPrincipal(tt.args.email); got != tt.want { + t.Errorf("SanitizeSSHUserPrincipal() = %v, want %v", got, tt.want) + } + }) + } +} From 780eeb54876f693fc11642d72567220722b2fbad Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 30 Jul 2019 16:56:30 -0700 Subject: [PATCH 22/40] Remove debug print. --- authority/provisioner/sign_ssh_options.go | 1 - 1 file changed, 1 deletion(-) diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 2ceab061..4748dc09 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -95,7 +95,6 @@ func (o SSHOptions) match(got SSHOptions) error { if !o.ValidBefore.IsZero() && !got.ValidBefore.IsZero() && !o.ValidBefore.Equal(&got.ValidBefore) { return errors.Errorf("ssh certificate valid before does not match - got %v, want %v", got.ValidBefore, o.ValidBefore) } - fmt.Printf("want %+v\ngot %+v\n", o, got) return nil } From b0240772da880e9346f2f0e9f4ea0f737af1b54a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 30 Jul 2019 18:23:54 -0700 Subject: [PATCH 23/40] Add tests for SSH certs with JWK provisioners. --- authority/provisioner/jwk_test.go | 193 +++++++++++++++++++++++++++- authority/provisioner/utils_test.go | 132 +++++++++++++++++++ 2 files changed, 321 insertions(+), 4 deletions(-) diff --git a/authority/provisioner/jwk_test.go b/authority/provisioner/jwk_test.go index 09317374..96e8da3b 100644 --- a/authority/provisioner/jwk_test.go +++ b/authority/provisioner/jwk_test.go @@ -2,23 +2,32 @@ package provisioner import ( "context" + "crypto" "crypto/x509" "errors" + "math" "strings" "testing" "time" "github.com/smallstep/assert" "github.com/smallstep/cli/jose" + "golang.org/x/crypto/ssh" ) var ( defaultDisableRenewal = false globalProvisionerClaims = Claims{ - MinTLSDur: &Duration{5 * time.Minute}, - MaxTLSDur: &Duration{24 * time.Hour}, - DefaultTLSDur: &Duration{24 * time.Hour}, - DisableRenewal: &defaultDisableRenewal, + MinTLSDur: &Duration{5 * time.Minute}, + MaxTLSDur: &Duration{24 * time.Hour}, + DefaultTLSDur: &Duration{24 * time.Hour}, + DisableRenewal: &defaultDisableRenewal, + MinUserSSHDur: &Duration{Duration: 5 * time.Minute}, // User SSH certs + MaxUserSSHDur: &Duration{Duration: 24 * time.Hour}, + DefaultUserSSHDur: &Duration{Duration: 4 * time.Hour}, + MinHostSSHDur: &Duration{Duration: 5 * time.Minute}, // Host SSH certs + MaxHostSSHDur: &Duration{Duration: 30 * 24 * time.Hour}, + DefaultHostSSHDur: &Duration{Duration: 30 * 24 * time.Hour}, } ) @@ -320,3 +329,179 @@ func TestJWK_AuthorizeRenewal(t *testing.T) { }) } } + +func TestJWK_AuthorizeSign_SSH(t *testing.T) { + p1, err := generateJWK() + assert.FatalError(t, err) + jwk, err := decryptJSONWebKey(p1.EncryptedKey) + assert.FatalError(t, err) + + iss, aud := p1.Name, testAudiences.Sign[0] + + t1, err := generateSimpleSSHUserToken(iss, aud, jwk) + assert.FatalError(t, err) + + t2, err := generateSimpleSSHHostToken(iss, aud, jwk) + assert.FatalError(t, err) + + // invalid signature + failSig := t1[0 : len(t1)-2] + + key, err := generateJSONWebKey() + assert.FatalError(t, err) + + signer, err := generateJSONWebKey() + assert.FatalError(t, err) + + type args struct { + token string + } + type expected struct { + certType uint32 + principals []string + } + tests := []struct { + name string + prov *JWK + args args + expected expected + err error + }{ + {"ok-user", p1, args{t1}, expected{ssh.UserCert, []string{"name"}}, nil}, + {"ok-host", p1, args{t2}, expected{ssh.HostCert, []string{"smallstep.com"}}, nil}, + {"fail-signature", p1, args{failSig}, expected{}, errors.New("error parsing claims: square/go-jose: error in cryptographic primitive")}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := NewContextWithMethod(context.Background(), SignSSHMethod) + if got, err := tt.prov.AuthorizeSign(ctx, tt.args.token); err != nil { + if assert.NotNil(t, tt.err) { + assert.HasPrefix(t, err.Error(), tt.err.Error()) + } + } else if assert.NotNil(t, got) { + cert, err := signSSHCertificate(key.Public().Key, SSHOptions{}, got, signer.Key.(crypto.Signer)) + assert.FatalError(t, err) + assert.NotNil(t, cert.Signature) + assert.Equals(t, tt.expected.certType, cert.CertType) + assert.Equals(t, tt.expected.principals, cert.ValidPrincipals) + if cert.CertType == ssh.UserCert { + assert.Len(t, 5, cert.Extensions) + } else { + assert.Nil(t, cert.Extensions) + } + } + }) + } +} + +func TestJWK_AuthorizeSign_SSHOptions(t *testing.T) { + p1, err := generateJWK() + assert.FatalError(t, err) + jwk, err := decryptJSONWebKey(p1.EncryptedKey) + assert.FatalError(t, err) + + userDuration := p1.claimer.DefaultUserSSHCertDuration() + hostDuration := p1.claimer.DefaultHostSSHCertDuration() + + now := time.Now() + sub, iss, aud := "subject@smallstep.com", p1.Name, testAudiences.Sign[0] + iat := now + + key, err := generateJSONWebKey() + assert.FatalError(t, err) + + signer, err := generateJSONWebKey() + assert.FatalError(t, err) + + type args struct { + sub, iss, aud string + iat time.Time + tokSSHOpts *SSHOptions + userSSHOpts *SSHOptions + jwk *jose.JSONWebKey + } + type expected struct { + certType uint32 + principals []string + validAfter time.Time + validBefore time.Time + } + tests := []struct { + name string + prov *JWK + args args + expected expected + wantErr bool + wantSignErr bool + }{ + {"ok-user", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{ssh.UserCert, []string{"name"}, now, now.Add(userDuration)}, false, false}, + {"ok-host", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, &SSHOptions{}, jwk}, expected{ssh.HostCert, []string{"smallstep.com"}, now, now.Add(hostDuration)}, false, false}, + {"ok-user-opts", p1, args{sub, iss, aud, iat, &SSHOptions{}, &SSHOptions{CertType: "user", Principals: []string{"name"}}, jwk}, expected{ssh.UserCert, []string{"name"}, now, now.Add(userDuration)}, false, false}, + {"ok-host-opts", p1, args{sub, iss, aud, iat, &SSHOptions{}, &SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, jwk}, expected{ssh.HostCert, []string{"smallstep.com"}, now, now.Add(hostDuration)}, false, false}, + {"ok-user-mixed", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user"}, &SSHOptions{Principals: []string{"name"}}, jwk}, expected{ssh.UserCert, []string{"name"}, now, now.Add(userDuration)}, false, false}, + {"ok-host-mixed", p1, args{sub, iss, aud, iat, &SSHOptions{Principals: []string{"smallstep.com"}}, &SSHOptions{CertType: "host"}, jwk}, expected{ssh.HostCert, []string{"smallstep.com"}, now, now.Add(hostDuration)}, false, false}, + {"ok-user-validAfter", p1, args{sub, iss, aud, iat, &SSHOptions{ + CertType: "user", Principals: []string{"name"}, + }, &SSHOptions{ + ValidAfter: NewTimeDuration(now.Add(-time.Hour)), + }, jwk}, expected{ssh.UserCert, []string{"name"}, now.Add(-time.Hour), now.Add(userDuration - time.Hour)}, false, false}, + {"ok-user-validBefore", p1, args{sub, iss, aud, iat, &SSHOptions{ + CertType: "user", Principals: []string{"name"}, + }, &SSHOptions{ + ValidBefore: NewTimeDuration(now.Add(time.Hour)), + }, jwk}, expected{ssh.UserCert, []string{"name"}, now, now.Add(time.Hour)}, false, false}, + {"ok-user-validAfter-validBefore", p1, args{sub, iss, aud, iat, &SSHOptions{ + CertType: "user", Principals: []string{"name"}, + }, &SSHOptions{ + ValidAfter: NewTimeDuration(now.Add(10 * time.Minute)), ValidBefore: NewTimeDuration(now.Add(time.Hour)), + }, jwk}, expected{ssh.UserCert, []string{"name"}, now.Add(10 * time.Minute), now.Add(time.Hour)}, false, false}, + {"ok-user-match", p1, args{sub, iss, aud, iat, &SSHOptions{ + CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(now), ValidBefore: NewTimeDuration(now.Add(1 * time.Hour)), + }, &SSHOptions{ + CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(now), ValidBefore: NewTimeDuration(now.Add(1 * time.Hour)), + }, jwk}, expected{ssh.UserCert, []string{"name"}, now, now.Add(1 * time.Hour)}, false, false}, + {"fail-certType", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{CertType: "host"}, jwk}, expected{}, false, true}, + {"fail-principals", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{Principals: []string{"root"}}, jwk}, expected{}, false, true}, + {"fail-validAfter", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(now)}, &SSHOptions{ValidAfter: NewTimeDuration(now.Add(time.Hour))}, jwk}, expected{}, false, true}, + {"fail-validBefore", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}, ValidBefore: NewTimeDuration(now.Add(time.Hour))}, &SSHOptions{ValidBefore: NewTimeDuration(now.Add(10 * time.Hour))}, jwk}, expected{}, false, true}, + {"fail-subject", p1, args{"", iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{}, true, false}, + {"fail-issuer", p1, args{sub, "invalid", aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{}, true, false}, + {"fail-audience", p1, args{sub, iss, "invalid", iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{}, true, false}, + {"fail-expired", p1, args{sub, iss, aud, iat.Add(-6 * time.Minute), &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{}, true, false}, + {"fail-notBefore", p1, args{sub, iss, aud, iat.Add(5 * time.Minute), &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{}, true, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := NewContextWithMethod(context.Background(), SignSSHMethod) + token, err := generateSSHToken(tt.args.sub, tt.args.iss, tt.args.aud, tt.args.iat, tt.args.tokSSHOpts, tt.args.jwk) + assert.FatalError(t, err) + if got, err := tt.prov.AuthorizeSign(ctx, token); (err != nil) != tt.wantErr { + t.Errorf("JWK.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) + } else if !tt.wantErr && assert.NotNil(t, got) { + var opts SSHOptions + if tt.args.userSSHOpts != nil { + opts = *tt.args.userSSHOpts + } + if cert, err := signSSHCertificate(key.Public().Key, opts, got, signer.Key.(crypto.Signer)); (err != nil) != tt.wantSignErr { + t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) + } else if !tt.wantSignErr && assert.NotNil(t, cert) { + assert.NotNil(t, cert.Signature) + assert.NotNil(t, cert.SignatureKey) + assert.Equals(t, tt.expected.certType, cert.CertType) + assert.Equals(t, tt.expected.principals, cert.ValidPrincipals) + assert.True(t, equalsUint64Delta(uint64(tt.expected.validAfter.Unix()), cert.ValidAfter, 60)) + assert.True(t, equalsUint64Delta(uint64(tt.expected.validBefore.Unix()), cert.ValidBefore, 60)) + if cert.CertType == ssh.UserCert { + assert.Len(t, 5, cert.Extensions) + } else { + assert.Nil(t, cert.Extensions) + } + } + } + }) + } +} + +func equalsUint64Delta(a, b uint64, delta uint64) bool { + return math.Abs(float64(a)-float64(b)) <= float64(delta) +} diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go index 7871b75d..14b0a3cd 100644 --- a/authority/provisioner/utils_test.go +++ b/authority/provisioner/utils_test.go @@ -14,6 +14,8 @@ import ( "net/http/httptest" "time" + "golang.org/x/crypto/ssh" + "github.com/pkg/errors" "github.com/smallstep/cli/crypto/randutil" "github.com/smallstep/cli/jose" @@ -480,6 +482,54 @@ func generateToken(sub, iss, aud string, email string, sans []string, iat time.T return jose.Signed(sig).Claims(claims).CompactSerialize() } +func generateSimpleSSHUserToken(iss, aud string, jwk *jose.JSONWebKey) (string, error) { + return generateSSHToken("subject@localhost", iss, aud, time.Now(), &SSHOptions{ + CertType: "user", + Principals: []string{"name"}, + }, jwk) +} + +func generateSimpleSSHHostToken(iss, aud string, jwk *jose.JSONWebKey) (string, error) { + return generateSSHToken("subject@localhost", iss, aud, time.Now(), &SSHOptions{ + CertType: "host", + Principals: []string{"smallstep.com"}, + }, jwk) +} + +func generateSSHToken(sub, iss, aud string, iat time.Time, sshOpts *SSHOptions, 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), + ) + if err != nil { + return "", err + } + + id, err := randutil.ASCII(64) + if err != nil { + return "", err + } + + claims := struct { + jose.Claims + Step *stepPayload `json:"step,omitempty"` + }{ + Claims: jose.Claims{ + ID: id, + Subject: sub, + Issuer: iss, + IssuedAt: jose.NewNumericDate(iat), + NotBefore: jose.NewNumericDate(iat), + Expiry: jose.NewNumericDate(iat.Add(5 * time.Minute)), + Audience: []string{aud}, + }, + Step: &stepPayload{ + SSH: sshOpts, + }, + } + return jose.Signed(sig).Claims(claims).CompactSerialize() +} + func generateGCPToken(sub, iss, aud, instanceID, instanceName, projectID, zone string, iat time.Time, jwk *jose.JSONWebKey) (string, error) { sig, err := jose.NewSigner( jose.SigningKey{Algorithm: jose.ES256, Key: jwk.Key}, @@ -682,3 +732,85 @@ func generateJWKServer(n int) *httptest.Server { srv.Start() return srv } + +func signSSHCertificate(key crypto.PublicKey, opts SSHOptions, signOpts []SignOption, signKey crypto.Signer) (*ssh.Certificate, error) { + pub, err := ssh.NewPublicKey(key) + if err != nil { + return nil, err + } + + var mods []SSHCertificateModifier + var validators []SSHCertificateValidator + + for _, op := range signOpts { + switch o := op.(type) { + // modify the ssh.Certificate + case SSHCertificateModifier: + mods = append(mods, o) + // modify the ssh.Certificate given the SSHOptions + case SSHCertificateOptionModifier: + mods = append(mods, o.Option(opts)) + // validate the ssh.Certificate + case SSHCertificateValidator: + validators = append(validators, o) + // validate the given SSHOptions + case SSHCertificateOptionsValidator: + if err := o.Valid(opts); err != nil { + return nil, err + } + default: + return nil, errors.Errorf("signSSH: invalid extra option type %T", o) + } + } + + // Build base certificate with the key and some random values + cert := &ssh.Certificate{ + Nonce: []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 0}, + Key: pub, + Serial: 1234567890, + } + + // Use opts to modify the certificate + if err := opts.Modify(cert); err != nil { + return nil, err + } + + // Use provisioner modifiers + for _, m := range mods { + if err := m.Modify(cert); err != nil { + return nil, err + } + } + + // Get signer from authority keys + var signer ssh.Signer + switch cert.CertType { + case ssh.UserCert: + signer, err = ssh.NewSignerFromSigner(signKey) + case ssh.HostCert: + signer, err = ssh.NewSignerFromSigner(signKey) + default: + return nil, errors.Errorf("unexpected ssh certificate type: %d", cert.CertType) + } + cert.SignatureKey = signer.PublicKey() + + // Get bytes for signing trailing the signature length. + data := cert.Marshal() + data = data[:len(data)-4] + + // Sign the certificate + sig, err := signer.Sign(rand.Reader, data) + if err != nil { + return nil, err + } + cert.Signature = sig + + // User provisioners validators + for _, v := range validators { + if err := v.Valid(cert); err != nil { + return nil, err + } + } + + return cert, nil +} From 4c1a11c1bc128d4234e8660245aeb1de1565838a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 31 Jul 2019 12:36:31 -0700 Subject: [PATCH 24/40] Add Unix method to TimeDuration. --- authority/provisioner/timeduration.go | 7 ++++- authority/provisioner/timeduration_test.go | 32 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/authority/provisioner/timeduration.go b/authority/provisioner/timeduration.go index ca80ba10..7d197217 100644 --- a/authority/provisioner/timeduration.go +++ b/authority/provisioner/timeduration.go @@ -113,11 +113,16 @@ func (t *TimeDuration) UnmarshalJSON(data []byte) error { return errors.Errorf("failed to parse %s", data) } -// Time calculates the embedded time.Time, sets it if necessary, and returns it. +// Time calculates the time if needed and returns it. func (t *TimeDuration) Time() time.Time { return t.RelativeTime(now()) } +// Unix calculates the time if needed it and returns the Unix time in seconds. +func (t *TimeDuration) Unix() int64 { + return t.RelativeTime(now()).Unix() +} + // RelativeTime returns the embedded time.Time or the base time plus the // duration if this is not zero. func (t *TimeDuration) RelativeTime(base time.Time) time.Time { diff --git a/authority/provisioner/timeduration_test.go b/authority/provisioner/timeduration_test.go index 642a79ed..1def427c 100644 --- a/authority/provisioner/timeduration_test.go +++ b/authority/provisioner/timeduration_test.go @@ -217,6 +217,38 @@ func TestTimeDuration_Time(t *testing.T) { } } +func TestTimeDuration_Unix(t *testing.T) { + nowFn := now + defer func() { + now = nowFn + now() + }() + tm := time.Unix(1584198566, 535897000).UTC() + now = func() time.Time { + return tm + } + tests := []struct { + name string + timeDuration *TimeDuration + want int64 + }{ + {"zero", nil, -62135596800}, + {"zero", &TimeDuration{}, -62135596800}, + {"timestamp", &TimeDuration{t: tm}, 1584198566}, + {"local", &TimeDuration{t: tm.Local()}, 1584198566}, + {"duration", &TimeDuration{d: 1 * time.Hour}, 1584202166}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.timeDuration.Unix() + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("TimeDuration.Unix() = %v, want %v", got, tt.want) + + } + }) + } +} + func TestTimeDuration_String(t *testing.T) { nowFn := now defer func() { From c17375a10a9b475a3ddc84e015e3cecc85de33fa Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 31 Jul 2019 12:53:03 -0700 Subject: [PATCH 25/40] Create convenient method to mock the timeduration. --- authority/provisioner/timeduration_test.go | 45 ++++++++-------------- 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/authority/provisioner/timeduration_test.go b/authority/provisioner/timeduration_test.go index 1def427c..65bd6f96 100644 --- a/authority/provisioner/timeduration_test.go +++ b/authority/provisioner/timeduration_test.go @@ -6,6 +6,17 @@ import ( "time" ) +func mockNow() (time.Time, func()) { + tm := time.Unix(1584198566, 535897000).UTC() + nowFn := now + now = func() time.Time { + return tm + } + return tm, func() { + now = nowFn + } +} + func TestNewTimeDuration(t *testing.T) { tm := time.Unix(1584198566, 535897000).UTC() type args struct { @@ -186,15 +197,8 @@ func TestTimeDuration_UnmarshalJSON(t *testing.T) { } func TestTimeDuration_Time(t *testing.T) { - nowFn := now - defer func() { - now = nowFn - now() - }() - tm := time.Unix(1584198566, 535897000).UTC() - now = func() time.Time { - return tm - } + tm, fn := mockNow() + defer fn() tests := []struct { name string timeDuration *TimeDuration @@ -211,22 +215,14 @@ func TestTimeDuration_Time(t *testing.T) { got := tt.timeDuration.Time() if !reflect.DeepEqual(got, tt.want) { t.Errorf("TimeDuration.Time() = %v, want %v", got, tt.want) - } }) } } func TestTimeDuration_Unix(t *testing.T) { - nowFn := now - defer func() { - now = nowFn - now() - }() - tm := time.Unix(1584198566, 535897000).UTC() - now = func() time.Time { - return tm - } + tm, fn := mockNow() + defer fn() tests := []struct { name string timeDuration *TimeDuration @@ -250,15 +246,8 @@ func TestTimeDuration_Unix(t *testing.T) { } func TestTimeDuration_String(t *testing.T) { - nowFn := now - defer func() { - now = nowFn - now() - }() - tm := time.Unix(1584198566, 535897000).UTC() - now = func() time.Time { - return tm - } + tm, fn := mockNow() + defer fn() type fields struct { t time.Time d time.Duration From a8f4ad1b8e1283b6a135f2fdf620efedd01fb99b Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 31 Jul 2019 17:03:33 -0700 Subject: [PATCH 26/40] 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 From d231bfb7644bebe2b9203447b68d5a34c7f93fad Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 31 Jul 2019 17:04:17 -0700 Subject: [PATCH 27/40] Update jwk and oidc tests. --- authority/provisioner/jwk_test.go | 178 ++++++++++++++++------------ authority/provisioner/oidc_test.go | 112 +++++++++++++++++ authority/provisioner/utils_test.go | 84 ------------- 3 files changed, 211 insertions(+), 163 deletions(-) diff --git a/authority/provisioner/jwk_test.go b/authority/provisioner/jwk_test.go index 96e8da3b..9952f7ff 100644 --- a/authority/provisioner/jwk_test.go +++ b/authority/provisioner/jwk_test.go @@ -5,14 +5,12 @@ import ( "crypto" "crypto/x509" "errors" - "math" "strings" "testing" "time" "github.com/smallstep/assert" "github.com/smallstep/cli/jose" - "golang.org/x/crypto/ssh" ) var ( @@ -331,6 +329,9 @@ func TestJWK_AuthorizeRenewal(t *testing.T) { } func TestJWK_AuthorizeSign_SSH(t *testing.T) { + tm, fn := mockNow() + defer fn() + p1, err := generateJWK() assert.FatalError(t, err) jwk, err := decryptJSONWebKey(p1.EncryptedKey) @@ -353,41 +354,59 @@ func TestJWK_AuthorizeSign_SSH(t *testing.T) { signer, err := generateJSONWebKey() assert.FatalError(t, err) - type args struct { - token string + userDuration := p1.claimer.DefaultUserSSHCertDuration() + hostDuration := p1.claimer.DefaultHostSSHCertDuration() + expectedUserOptions := &SSHOptions{ + CertType: "user", Principals: []string{"name"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration)), } - type expected struct { - certType uint32 - principals []string + expectedHostOptions := &SSHOptions{ + CertType: "host", Principals: []string{"smallstep.com"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), + } + + type args struct { + token string + sshOpts SSHOptions } tests := []struct { - name string - prov *JWK - args args - expected expected - err error + name string + prov *JWK + args args + expected *SSHOptions + wantErr bool + wantSignErr bool }{ - {"ok-user", p1, args{t1}, expected{ssh.UserCert, []string{"name"}}, nil}, - {"ok-host", p1, args{t2}, expected{ssh.HostCert, []string{"smallstep.com"}}, nil}, - {"fail-signature", p1, args{failSig}, expected{}, errors.New("error parsing claims: square/go-jose: error in cryptographic primitive")}, + {"user", p1, args{t1, SSHOptions{}}, expectedUserOptions, false, false}, + {"user-type", p1, args{t1, SSHOptions{CertType: "user"}}, expectedUserOptions, false, false}, + {"user-principals", p1, args{t1, SSHOptions{Principals: []string{"name"}}}, expectedUserOptions, false, false}, + {"user-options", p1, args{t1, SSHOptions{CertType: "user", Principals: []string{"name"}}}, expectedUserOptions, false, false}, + {"host", p1, args{t2, SSHOptions{}}, expectedHostOptions, false, false}, + {"host-type", p1, args{t2, SSHOptions{CertType: "host"}}, expectedHostOptions, false, false}, + {"host-principals", p1, args{t2, SSHOptions{Principals: []string{"smallstep.com"}}}, expectedHostOptions, false, false}, + {"host-options", p1, args{t2, SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}}, expectedHostOptions, false, false}, + {"fail-signature", p1, args{failSig, SSHOptions{}}, nil, true, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := NewContextWithMethod(context.Background(), SignSSHMethod) - if got, err := tt.prov.AuthorizeSign(ctx, tt.args.token); err != nil { - if assert.NotNil(t, tt.err) { - assert.HasPrefix(t, err.Error(), tt.err.Error()) - } + got, err := tt.prov.AuthorizeSign(ctx, tt.args.token) + if (err != nil) != tt.wantErr { + t.Errorf("OIDC.Authorize() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err != nil { + assert.Nil(t, got) } else if assert.NotNil(t, got) { - cert, err := signSSHCertificate(key.Public().Key, SSHOptions{}, got, signer.Key.(crypto.Signer)) - assert.FatalError(t, err) - assert.NotNil(t, cert.Signature) - assert.Equals(t, tt.expected.certType, cert.CertType) - assert.Equals(t, tt.expected.principals, cert.ValidPrincipals) - if cert.CertType == ssh.UserCert { - assert.Len(t, 5, cert.Extensions) + cert, err := signSSHCertificate(key.Public().Key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) + if (err != nil) != tt.wantSignErr { + t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) } else { - assert.Nil(t, cert.Extensions) + if tt.wantSignErr { + assert.Nil(t, cert) + } else { + assert.NoError(t, validateSSHCertificate(cert, tt.expected)) + } } } }) @@ -395,17 +414,15 @@ func TestJWK_AuthorizeSign_SSH(t *testing.T) { } func TestJWK_AuthorizeSign_SSHOptions(t *testing.T) { + tm, fn := mockNow() + defer fn() + p1, err := generateJWK() assert.FatalError(t, err) jwk, err := decryptJSONWebKey(p1.EncryptedKey) assert.FatalError(t, err) - userDuration := p1.claimer.DefaultUserSSHCertDuration() - hostDuration := p1.claimer.DefaultHostSSHCertDuration() - - now := time.Now() - sub, iss, aud := "subject@smallstep.com", p1.Name, testAudiences.Sign[0] - iat := now + sub, iss, aud, iat := "subject@smallstep.com", p1.Name, testAudiences.Sign[0], time.Now() key, err := generateJSONWebKey() assert.FatalError(t, err) @@ -413,6 +430,16 @@ func TestJWK_AuthorizeSign_SSHOptions(t *testing.T) { signer, err := generateJSONWebKey() assert.FatalError(t, err) + userDuration := p1.claimer.DefaultUserSSHCertDuration() + hostDuration := p1.claimer.DefaultHostSSHCertDuration() + expectedUserOptions := &SSHOptions{ + CertType: "user", Principals: []string{"name"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration)), + } + expectedHostOptions := &SSHOptions{ + CertType: "host", Principals: []string{"smallstep.com"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), + } type args struct { sub, iss, aud string iat time.Time @@ -420,55 +447,57 @@ func TestJWK_AuthorizeSign_SSHOptions(t *testing.T) { userSSHOpts *SSHOptions jwk *jose.JSONWebKey } - type expected struct { - certType uint32 - principals []string - validAfter time.Time - validBefore time.Time - } tests := []struct { name string prov *JWK args args - expected expected + expected *SSHOptions wantErr bool wantSignErr bool }{ - {"ok-user", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{ssh.UserCert, []string{"name"}, now, now.Add(userDuration)}, false, false}, - {"ok-host", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, &SSHOptions{}, jwk}, expected{ssh.HostCert, []string{"smallstep.com"}, now, now.Add(hostDuration)}, false, false}, - {"ok-user-opts", p1, args{sub, iss, aud, iat, &SSHOptions{}, &SSHOptions{CertType: "user", Principals: []string{"name"}}, jwk}, expected{ssh.UserCert, []string{"name"}, now, now.Add(userDuration)}, false, false}, - {"ok-host-opts", p1, args{sub, iss, aud, iat, &SSHOptions{}, &SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, jwk}, expected{ssh.HostCert, []string{"smallstep.com"}, now, now.Add(hostDuration)}, false, false}, - {"ok-user-mixed", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user"}, &SSHOptions{Principals: []string{"name"}}, jwk}, expected{ssh.UserCert, []string{"name"}, now, now.Add(userDuration)}, false, false}, - {"ok-host-mixed", p1, args{sub, iss, aud, iat, &SSHOptions{Principals: []string{"smallstep.com"}}, &SSHOptions{CertType: "host"}, jwk}, expected{ssh.HostCert, []string{"smallstep.com"}, now, now.Add(hostDuration)}, false, false}, + {"ok-user", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expectedUserOptions, false, false}, + {"ok-host", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, &SSHOptions{}, jwk}, expectedHostOptions, false, false}, + {"ok-user-opts", p1, args{sub, iss, aud, iat, &SSHOptions{}, &SSHOptions{CertType: "user", Principals: []string{"name"}}, jwk}, expectedUserOptions, false, false}, + {"ok-host-opts", p1, args{sub, iss, aud, iat, &SSHOptions{}, &SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, jwk}, expectedHostOptions, false, false}, + {"ok-user-mixed", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user"}, &SSHOptions{Principals: []string{"name"}}, jwk}, expectedUserOptions, false, false}, + {"ok-host-mixed", p1, args{sub, iss, aud, iat, &SSHOptions{Principals: []string{"smallstep.com"}}, &SSHOptions{CertType: "host"}, jwk}, expectedHostOptions, false, false}, {"ok-user-validAfter", p1, args{sub, iss, aud, iat, &SSHOptions{ CertType: "user", Principals: []string{"name"}, }, &SSHOptions{ - ValidAfter: NewTimeDuration(now.Add(-time.Hour)), - }, jwk}, expected{ssh.UserCert, []string{"name"}, now.Add(-time.Hour), now.Add(userDuration - time.Hour)}, false, false}, + ValidAfter: NewTimeDuration(tm.Add(-time.Hour)), + }, jwk}, &SSHOptions{ + CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(tm.Add(-time.Hour)), ValidBefore: NewTimeDuration(tm.Add(userDuration - time.Hour)), + }, false, false}, {"ok-user-validBefore", p1, args{sub, iss, aud, iat, &SSHOptions{ CertType: "user", Principals: []string{"name"}, }, &SSHOptions{ - ValidBefore: NewTimeDuration(now.Add(time.Hour)), - }, jwk}, expected{ssh.UserCert, []string{"name"}, now, now.Add(time.Hour)}, false, false}, + ValidBefore: NewTimeDuration(tm.Add(time.Hour)), + }, jwk}, &SSHOptions{ + CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(time.Hour)), + }, false, false}, {"ok-user-validAfter-validBefore", p1, args{sub, iss, aud, iat, &SSHOptions{ CertType: "user", Principals: []string{"name"}, }, &SSHOptions{ - ValidAfter: NewTimeDuration(now.Add(10 * time.Minute)), ValidBefore: NewTimeDuration(now.Add(time.Hour)), - }, jwk}, expected{ssh.UserCert, []string{"name"}, now.Add(10 * time.Minute), now.Add(time.Hour)}, false, false}, + ValidAfter: NewTimeDuration(tm.Add(10 * time.Minute)), ValidBefore: NewTimeDuration(tm.Add(time.Hour)), + }, jwk}, &SSHOptions{ + CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(tm.Add(10 * time.Minute)), ValidBefore: NewTimeDuration(tm.Add(time.Hour)), + }, false, false}, {"ok-user-match", p1, args{sub, iss, aud, iat, &SSHOptions{ - CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(now), ValidBefore: NewTimeDuration(now.Add(1 * time.Hour)), + CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(1 * time.Hour)), }, &SSHOptions{ - CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(now), ValidBefore: NewTimeDuration(now.Add(1 * time.Hour)), - }, jwk}, expected{ssh.UserCert, []string{"name"}, now, now.Add(1 * time.Hour)}, false, false}, - {"fail-certType", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{CertType: "host"}, jwk}, expected{}, false, true}, - {"fail-principals", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{Principals: []string{"root"}}, jwk}, expected{}, false, true}, - {"fail-validAfter", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(now)}, &SSHOptions{ValidAfter: NewTimeDuration(now.Add(time.Hour))}, jwk}, expected{}, false, true}, - {"fail-validBefore", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}, ValidBefore: NewTimeDuration(now.Add(time.Hour))}, &SSHOptions{ValidBefore: NewTimeDuration(now.Add(10 * time.Hour))}, jwk}, expected{}, false, true}, - {"fail-subject", p1, args{"", iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{}, true, false}, - {"fail-issuer", p1, args{sub, "invalid", aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{}, true, false}, - {"fail-audience", p1, args{sub, iss, "invalid", iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{}, true, false}, - {"fail-expired", p1, args{sub, iss, aud, iat.Add(-6 * time.Minute), &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{}, true, false}, - {"fail-notBefore", p1, args{sub, iss, aud, iat.Add(5 * time.Minute), &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, expected{}, true, false}, + CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(1 * time.Hour)), + }, jwk}, &SSHOptions{ + CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(time.Hour)), + }, false, false}, + {"fail-certType", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{CertType: "host"}, jwk}, nil, false, true}, + {"fail-principals", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{Principals: []string{"root"}}, jwk}, nil, false, true}, + {"fail-validAfter", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}, ValidAfter: NewTimeDuration(tm)}, &SSHOptions{ValidAfter: NewTimeDuration(tm.Add(time.Hour))}, jwk}, nil, false, true}, + {"fail-validBefore", p1, args{sub, iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}, ValidBefore: NewTimeDuration(tm.Add(time.Hour))}, &SSHOptions{ValidBefore: NewTimeDuration(tm.Add(10 * time.Hour))}, jwk}, nil, false, true}, + {"fail-subject", p1, args{"", iss, aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, nil, true, false}, + {"fail-issuer", p1, args{sub, "invalid", aud, iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, nil, true, false}, + {"fail-audience", p1, args{sub, iss, "invalid", iat, &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, nil, true, false}, + {"fail-expired", p1, args{sub, iss, aud, iat.Add(-6 * time.Minute), &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, nil, true, false}, + {"fail-notBefore", p1, args{sub, iss, aud, iat.Add(5 * time.Minute), &SSHOptions{CertType: "user", Principals: []string{"name"}}, &SSHOptions{}, jwk}, nil, true, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -482,26 +511,17 @@ func TestJWK_AuthorizeSign_SSHOptions(t *testing.T) { if tt.args.userSSHOpts != nil { opts = *tt.args.userSSHOpts } - if cert, err := signSSHCertificate(key.Public().Key, opts, got, signer.Key.(crypto.Signer)); (err != nil) != tt.wantSignErr { + cert, err := signSSHCertificate(key.Public().Key, opts, got, signer.Key.(crypto.Signer)) + if (err != nil) != tt.wantSignErr { t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) - } else if !tt.wantSignErr && assert.NotNil(t, cert) { - assert.NotNil(t, cert.Signature) - assert.NotNil(t, cert.SignatureKey) - assert.Equals(t, tt.expected.certType, cert.CertType) - assert.Equals(t, tt.expected.principals, cert.ValidPrincipals) - assert.True(t, equalsUint64Delta(uint64(tt.expected.validAfter.Unix()), cert.ValidAfter, 60)) - assert.True(t, equalsUint64Delta(uint64(tt.expected.validBefore.Unix()), cert.ValidBefore, 60)) - if cert.CertType == ssh.UserCert { - assert.Len(t, 5, cert.Extensions) + } else { + if tt.wantSignErr { + assert.Nil(t, cert) } else { - assert.Nil(t, cert.Extensions) + assert.NoError(t, validateSSHCertificate(cert, tt.expected)) } } } }) } } - -func equalsUint64Delta(a, b uint64, delta uint64) bool { - return math.Abs(float64(a)-float64(b)) <= float64(delta) -} diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index 8eb6a7c2..787d596d 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -2,6 +2,7 @@ package provisioner import ( "context" + "crypto" "crypto/x509" "fmt" "strings" @@ -297,6 +298,117 @@ func TestOIDC_AuthorizeSign(t *testing.T) { } } +func TestOIDC_AuthorizeSign_SSH(t *testing.T) { + tm, fn := mockNow() + defer fn() + + srv := generateJWKServer(2) + defer srv.Close() + + var keys jose.JSONWebKeySet + assert.FatalError(t, getAndDecode(srv.URL+"/private", &keys)) + + // Create test provisioners + p1, err := generateOIDC() + assert.FatalError(t, err) + p2, err := generateOIDC() + assert.FatalError(t, err) + p3, err := generateOIDC() + assert.FatalError(t, err) + // 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} + p1.ConfigurationEndpoint = srv.URL + "/.well-known/openid-configuration" + p2.ConfigurationEndpoint = srv.URL + "/.well-known/openid-configuration" + p3.ConfigurationEndpoint = srv.URL + "/.well-known/openid-configuration" + assert.FatalError(t, p1.Init(config)) + assert.FatalError(t, p2.Init(config)) + assert.FatalError(t, p3.Init(config)) + + t1, err := generateSimpleToken("the-issuer", p1.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{}, 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) + + key, err := generateJSONWebKey() + assert.FatalError(t, err) + + signer, err := generateJSONWebKey() + assert.FatalError(t, err) + + userDuration := p1.claimer.DefaultUserSSHCertDuration() + hostDuration := p1.claimer.DefaultHostSSHCertDuration() + expectedUserOptions := &SSHOptions{ + CertType: "user", Principals: []string{"name"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration)), + } + expectedAdminOptions := &SSHOptions{ + CertType: "user", Principals: []string{"root"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration)), + } + expectedHostOptions := &SSHOptions{ + CertType: "host", Principals: []string{"smallstep.com"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), + } + + type args struct { + token string + sshOpts SSHOptions + } + tests := []struct { + name string + prov *OIDC + args args + expected *SSHOptions + wantErr bool + wantSignErr bool + }{ + {"ok", p1, args{t1, SSHOptions{}}, expectedUserOptions, false, false}, + {"ok-user", p1, args{t1, SSHOptions{CertType: "user"}}, expectedUserOptions, false, false}, + {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"name"}}}, expectedUserOptions, false, false}, + {"ok-options", p1, args{t1, SSHOptions{CertType: "user", Principals: []string{"name"}}}, expectedUserOptions, false, false}, + {"admin", p3, args{okAdmin, SSHOptions{}}, expectedAdminOptions, false, false}, + {"admin-user", p3, args{okAdmin, SSHOptions{CertType: "user"}}, expectedAdminOptions, false, false}, + {"admin-principals", p3, args{okAdmin, SSHOptions{Principals: []string{"root"}}}, expectedAdminOptions, false, false}, + {"admin-options", p3, args{okAdmin, SSHOptions{CertType: "user", Principals: []string{"name"}}}, expectedUserOptions, false, false}, + {"admin-host", p3, args{okAdmin, SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}}, expectedHostOptions, false, false}, + {"fail-user-host", p1, args{t1, SSHOptions{CertType: "host"}}, nil, false, true}, + {"fail-user-principals", p1, args{t1, SSHOptions{Principals: []string{"root"}}}, nil, false, true}, + {"fail-email", p3, args{failEmail, SSHOptions{}}, nil, true, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := NewContextWithMethod(context.Background(), SignSSHMethod) + got, err := tt.prov.AuthorizeSign(ctx, tt.args.token) + if (err != nil) != tt.wantErr { + t.Errorf("OIDC.Authorize() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err != nil { + assert.Nil(t, got) + } else if assert.NotNil(t, got) { + cert, err := signSSHCertificate(key.Public().Key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) + if (err != nil) != tt.wantSignErr { + t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) + } else { + if tt.wantSignErr { + assert.Nil(t, cert) + } else { + assert.NoError(t, validateSSHCertificate(cert, tt.expected)) + } + } + } + }) + } +} + func TestOIDC_AuthorizeRevoke(t *testing.T) { srv := generateJWKServer(2) defer srv.Close() diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go index 14b0a3cd..c6e820ed 100644 --- a/authority/provisioner/utils_test.go +++ b/authority/provisioner/utils_test.go @@ -14,8 +14,6 @@ import ( "net/http/httptest" "time" - "golang.org/x/crypto/ssh" - "github.com/pkg/errors" "github.com/smallstep/cli/crypto/randutil" "github.com/smallstep/cli/jose" @@ -732,85 +730,3 @@ func generateJWKServer(n int) *httptest.Server { srv.Start() return srv } - -func signSSHCertificate(key crypto.PublicKey, opts SSHOptions, signOpts []SignOption, signKey crypto.Signer) (*ssh.Certificate, error) { - pub, err := ssh.NewPublicKey(key) - if err != nil { - return nil, err - } - - var mods []SSHCertificateModifier - var validators []SSHCertificateValidator - - for _, op := range signOpts { - switch o := op.(type) { - // modify the ssh.Certificate - case SSHCertificateModifier: - mods = append(mods, o) - // modify the ssh.Certificate given the SSHOptions - case SSHCertificateOptionModifier: - mods = append(mods, o.Option(opts)) - // validate the ssh.Certificate - case SSHCertificateValidator: - validators = append(validators, o) - // validate the given SSHOptions - case SSHCertificateOptionsValidator: - if err := o.Valid(opts); err != nil { - return nil, err - } - default: - return nil, errors.Errorf("signSSH: invalid extra option type %T", o) - } - } - - // Build base certificate with the key and some random values - cert := &ssh.Certificate{ - Nonce: []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 0}, - Key: pub, - Serial: 1234567890, - } - - // Use opts to modify the certificate - if err := opts.Modify(cert); err != nil { - return nil, err - } - - // Use provisioner modifiers - for _, m := range mods { - if err := m.Modify(cert); err != nil { - return nil, err - } - } - - // Get signer from authority keys - var signer ssh.Signer - switch cert.CertType { - case ssh.UserCert: - signer, err = ssh.NewSignerFromSigner(signKey) - case ssh.HostCert: - signer, err = ssh.NewSignerFromSigner(signKey) - default: - return nil, errors.Errorf("unexpected ssh certificate type: %d", cert.CertType) - } - cert.SignatureKey = signer.PublicKey() - - // Get bytes for signing trailing the signature length. - data := cert.Marshal() - data = data[:len(data)-4] - - // Sign the certificate - sig, err := signer.Sign(rand.Reader, data) - if err != nil { - return nil, err - } - cert.Signature = sig - - // User provisioners validators - for _, v := range validators { - if err := v.Valid(cert); err != nil { - return nil, err - } - } - - return cert, nil -} From f8a71899fd5aafc69cd468ff977786cbd63759b3 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 31 Jul 2019 17:46:28 -0700 Subject: [PATCH 28/40] Add missing file. --- authority/provisioner/ssh_test.go | 122 ++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 authority/provisioner/ssh_test.go diff --git a/authority/provisioner/ssh_test.go b/authority/provisioner/ssh_test.go new file mode 100644 index 00000000..003f4e9a --- /dev/null +++ b/authority/provisioner/ssh_test.go @@ -0,0 +1,122 @@ +package provisioner + +import ( + "crypto" + "crypto/rand" + "fmt" + "reflect" + "time" + + "golang.org/x/crypto/ssh" +) + +func validateSSHCertificate(cert *ssh.Certificate, opts *SSHOptions) error { + switch { + case cert == nil: + return fmt.Errorf("certificate is nil") + case cert.Signature == nil: + 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): + 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) + case opts.CertType == "user" && cert.CertType != ssh.UserCert: + return fmt.Errorf("certificate type is not valid, want %v, got %v", ssh.UserCert, cert.CertType) + case opts.CertType == "host" && cert.CertType != ssh.HostCert: + return fmt.Errorf("certificate type is not valid, want %v, got %v", ssh.HostCert, cert.CertType) + case cert.ValidAfter != uint64(opts.ValidAfter.Unix()): + return fmt.Errorf("certificate valid after is not valid, want %v, got %v", opts.ValidAfter.Unix(), time.Unix(int64(cert.ValidAfter), 0)) + case cert.ValidBefore != uint64(opts.ValidBefore.Unix()): + return fmt.Errorf("certificate valid after is not valid, want %v, got %v", opts.ValidAfter.Unix(), time.Unix(int64(cert.ValidAfter), 0)) + case opts.CertType == "user" && len(cert.Extensions) != 5: + return fmt.Errorf("certificate extensions number is invalid, want 5, got %d", len(cert.Extensions)) + case opts.CertType == "host" && len(cert.Extensions) != 0: + return fmt.Errorf("certificate extensions number is invalid, want 0, got %d", len(cert.Extensions)) + default: + return nil + } +} + +func signSSHCertificate(key crypto.PublicKey, opts SSHOptions, signOpts []SignOption, signKey crypto.Signer) (*ssh.Certificate, error) { + pub, err := ssh.NewPublicKey(key) + if err != nil { + return nil, err + } + + var mods []SSHCertificateModifier + var validators []SSHCertificateValidator + + for _, op := range signOpts { + switch o := op.(type) { + // modify the ssh.Certificate + case SSHCertificateModifier: + mods = append(mods, o) + // modify the ssh.Certificate given the SSHOptions + case SSHCertificateOptionModifier: + mods = append(mods, o.Option(opts)) + // validate the ssh.Certificate + case SSHCertificateValidator: + validators = append(validators, o) + // validate the given SSHOptions + case SSHCertificateOptionsValidator: + if err := o.Valid(opts); err != nil { + return nil, err + } + default: + return nil, fmt.Errorf("signSSH: invalid extra option type %T", o) + } + } + + // Build base certificate with the key and some random values + cert := &ssh.Certificate{ + Nonce: []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 0}, + Key: pub, + Serial: 1234567890, + } + + // Use opts to modify the certificate + if err := opts.Modify(cert); err != nil { + return nil, err + } + + // Use provisioner modifiers + for _, m := range mods { + if err := m.Modify(cert); err != nil { + return nil, err + } + } + + // Get signer from authority keys + var signer ssh.Signer + switch cert.CertType { + case ssh.UserCert: + signer, err = ssh.NewSignerFromSigner(signKey) + case ssh.HostCert: + signer, err = ssh.NewSignerFromSigner(signKey) + default: + return nil, fmt.Errorf("unexpected ssh certificate type: %d", cert.CertType) + } + cert.SignatureKey = signer.PublicKey() + + // Get bytes for signing trailing the signature length. + data := cert.Marshal() + data = data[:len(data)-4] + + // Sign the certificate + sig, err := signer.Sign(rand.Reader, data) + if err != nil { + return nil, err + } + cert.Signature = sig + + // User provisioners validators + for _, v := range validators { + if err := v.Valid(cert); err != nil { + return nil, err + } + } + + return cert, nil +} From 2cac85a8c863ece0b5237da485280b3bd1e477d0 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 31 Jul 2019 18:11:46 -0700 Subject: [PATCH 29/40] Add aws tests. --- authority/provisioner/aws_test.go | 79 +++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/authority/provisioner/aws_test.go b/authority/provisioner/aws_test.go index af101dcf..45f4dcfd 100644 --- a/authority/provisioner/aws_test.go +++ b/authority/provisioner/aws_test.go @@ -2,6 +2,7 @@ package provisioner import ( "context" + "crypto" "crypto/rand" "crypto/rsa" "crypto/sha256" @@ -359,6 +360,84 @@ func TestAWS_AuthorizeSign(t *testing.T) { } } +func TestAWS_AuthorizeSign_SSH(t *testing.T) { + tm, fn := mockNow() + defer fn() + + p1, srv, err := generateAWSWithServer() + assert.FatalError(t, err) + defer srv.Close() + + t1, err := p1.GetIdentityToken("foo.local", "https://ca.smallstep.com") + assert.FatalError(t, err) + + key, err := generateJSONWebKey() + assert.FatalError(t, err) + + signer, err := generateJSONWebKey() + assert.FatalError(t, err) + + hostDuration := p1.claimer.DefaultHostSSHCertDuration() + expectedHostOptions := &SSHOptions{ + CertType: "host", Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), + } + expectedHostOptionsIP := &SSHOptions{ + CertType: "host", Principals: []string{"127.0.0.1"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), + } + expectedHostOptionsHostname := &SSHOptions{ + CertType: "host", Principals: []string{"ip-127-0-0-1.us-west-1.compute.internal"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), + } + + type args struct { + token string + sshOpts SSHOptions + } + tests := []struct { + name string + aws *AWS + args args + expected *SSHOptions + wantErr bool + wantSignErr bool + }{ + {"ok", p1, args{t1, SSHOptions{}}, expectedHostOptions, false, false}, + {"ok-type", p1, args{t1, SSHOptions{CertType: "host"}}, expectedHostOptions, false, false}, + {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal"}}}, expectedHostOptions, false, false}, + {"ok-principal-ip", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1"}}}, expectedHostOptionsIP, false, false}, + {"ok-principal-hostname", p1, args{t1, SSHOptions{Principals: []string{"ip-127-0-0-1.us-west-1.compute.internal"}}}, expectedHostOptionsHostname, false, false}, + {"ok-options", p1, args{t1, SSHOptions{CertType: "host", Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal"}}}, expectedHostOptions, false, false}, + {"fail-type", p1, args{t1, SSHOptions{CertType: "user"}}, nil, false, true}, + {"fail-principal", p1, args{t1, SSHOptions{Principals: []string{"smallstep.com"}}}, nil, false, true}, + {"fail-extra-principal", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal", "smallstep.com"}}}, nil, false, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := NewContextWithMethod(context.Background(), SignSSHMethod) + got, err := tt.aws.AuthorizeSign(ctx, tt.args.token) + if (err != nil) != tt.wantErr { + t.Errorf("AWS.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err != nil { + assert.Nil(t, got) + } else if assert.NotNil(t, got) { + cert, err := signSSHCertificate(key.Public().Key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) + if (err != nil) != tt.wantSignErr { + t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) + } else { + if tt.wantSignErr { + assert.Nil(t, cert) + } else { + assert.NoError(t, validateSSHCertificate(cert, tt.expected)) + } + } + } + }) + } +} func TestAWS_AuthorizeRenewal(t *testing.T) { p1, err := generateAWS() assert.FatalError(t, err) From 7983aa866159235d7c6771972230179c94f794da Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 31 Jul 2019 18:16:17 -0700 Subject: [PATCH 30/40] Add azure ssh tests. --- authority/provisioner/azure_test.go | 70 +++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/authority/provisioner/azure_test.go b/authority/provisioner/azure_test.go index dc8e514c..f9594045 100644 --- a/authority/provisioner/azure_test.go +++ b/authority/provisioner/azure_test.go @@ -2,6 +2,7 @@ package provisioner import ( "context" + "crypto" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -307,6 +308,75 @@ func TestAzure_AuthorizeSign(t *testing.T) { } } +func TestAzure_AuthorizeSign_SSH(t *testing.T) { + tm, fn := mockNow() + defer fn() + + p1, srv, err := generateAzureWithServer() + assert.FatalError(t, err) + defer srv.Close() + + t1, err := p1.GetIdentityToken("subject", "caURL") + assert.FatalError(t, err) + + key, err := generateJSONWebKey() + assert.FatalError(t, err) + + signer, err := generateJSONWebKey() + assert.FatalError(t, err) + + hostDuration := p1.claimer.DefaultHostSSHCertDuration() + expectedHostOptions := &SSHOptions{ + CertType: "host", Principals: []string{"virtualMachine"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), + } + + type args struct { + token string + sshOpts SSHOptions + } + tests := []struct { + name string + azure *Azure + args args + expected *SSHOptions + wantErr bool + wantSignErr bool + }{ + {"ok", p1, args{t1, SSHOptions{}}, expectedHostOptions, false, false}, + {"ok-type", p1, args{t1, SSHOptions{CertType: "host"}}, expectedHostOptions, false, false}, + {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"virtualMachine"}}}, expectedHostOptions, false, false}, + {"ok-options", p1, args{t1, SSHOptions{CertType: "host", Principals: []string{"virtualMachine"}}}, expectedHostOptions, false, false}, + {"fail-type", p1, args{t1, SSHOptions{CertType: "user"}}, nil, false, true}, + {"fail-principal", p1, args{t1, SSHOptions{Principals: []string{"smallstep.com"}}}, nil, false, true}, + {"fail-extra-principal", p1, args{t1, SSHOptions{Principals: []string{"virtualMachine", "smallstep.com"}}}, nil, false, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := NewContextWithMethod(context.Background(), SignSSHMethod) + got, err := tt.azure.AuthorizeSign(ctx, tt.args.token) + if (err != nil) != tt.wantErr { + t.Errorf("Azure.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err != nil { + assert.Nil(t, got) + } else if assert.NotNil(t, got) { + cert, err := signSSHCertificate(key.Public().Key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) + if (err != nil) != tt.wantSignErr { + t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) + } else { + if tt.wantSignErr { + assert.Nil(t, cert) + } else { + assert.NoError(t, validateSSHCertificate(cert, tt.expected)) + } + } + } + }) + } +} + func TestAzure_AuthorizeRenewal(t *testing.T) { p1, err := generateAzure() assert.FatalError(t, err) From dc657565a7da924200697453ecb0d7e00190ab6d Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 31 Jul 2019 18:22:21 -0700 Subject: [PATCH 31/40] Add SSH test for GCP. --- authority/provisioner/gcp_test.go | 82 +++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/authority/provisioner/gcp_test.go b/authority/provisioner/gcp_test.go index 12e60af0..11f5a138 100644 --- a/authority/provisioner/gcp_test.go +++ b/authority/provisioner/gcp_test.go @@ -2,6 +2,7 @@ package provisioner import ( "context" + "crypto" "crypto/sha256" "crypto/x509" "encoding/hex" @@ -342,6 +343,87 @@ func TestGCP_AuthorizeSign(t *testing.T) { } } +func TestGCP_AuthorizeSign_SSH(t *testing.T) { + tm, fn := mockNow() + defer fn() + + p1, err := generateGCP() + assert.FatalError(t, err) + + t1, err := generateGCPToken(p1.ServiceAccounts[0], + "https://accounts.google.com", p1.GetID(), + "instance-id", "instance-name", "project-id", "zone", + time.Now(), &p1.keyStore.keySet.Keys[0]) + assert.FatalError(t, err) + + key, err := generateJSONWebKey() + assert.FatalError(t, err) + + signer, err := generateJSONWebKey() + assert.FatalError(t, err) + + hostDuration := p1.claimer.DefaultHostSSHCertDuration() + expectedHostOptions := &SSHOptions{ + CertType: "host", Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), + } + expectedHostOptionsPrincipal1 := &SSHOptions{ + CertType: "host", Principals: []string{"instance-name.c.project-id.internal"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), + } + expectedHostOptionsPrincipal2 := &SSHOptions{ + CertType: "host", Principals: []string{"instance-name.zone.c.project-id.internal"}, + ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(hostDuration)), + } + + type args struct { + token string + sshOpts SSHOptions + } + tests := []struct { + name string + gcp *GCP + args args + expected *SSHOptions + wantErr bool + wantSignErr bool + }{ + {"ok", p1, args{t1, SSHOptions{}}, expectedHostOptions, false, false}, + {"ok-type", p1, args{t1, SSHOptions{CertType: "host"}}, expectedHostOptions, false, false}, + {"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"}}}, expectedHostOptions, false, false}, + {"ok-principal1", p1, args{t1, SSHOptions{Principals: []string{"instance-name.c.project-id.internal"}}}, expectedHostOptionsPrincipal1, false, false}, + {"ok-principal2", p1, args{t1, SSHOptions{Principals: []string{"instance-name.zone.c.project-id.internal"}}}, expectedHostOptionsPrincipal2, false, false}, + {"ok-options", p1, args{t1, SSHOptions{CertType: "host", Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"}}}, expectedHostOptions, false, false}, + {"fail-type", p1, args{t1, SSHOptions{CertType: "user"}}, nil, false, true}, + {"fail-principal", p1, args{t1, SSHOptions{Principals: []string{"smallstep.com"}}}, nil, false, true}, + {"fail-extra-principal", p1, args{t1, SSHOptions{Principals: []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal", "smallstep.com"}}}, nil, false, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := NewContextWithMethod(context.Background(), SignSSHMethod) + got, err := tt.gcp.AuthorizeSign(ctx, tt.args.token) + if (err != nil) != tt.wantErr { + t.Errorf("GCP.AuthorizeSign() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err != nil { + assert.Nil(t, got) + } else if assert.NotNil(t, got) { + cert, err := signSSHCertificate(key.Public().Key, tt.args.sshOpts, got, signer.Key.(crypto.Signer)) + if (err != nil) != tt.wantSignErr { + t.Errorf("SignSSH error = %v, wantSignErr %v", err, tt.wantSignErr) + } else { + if tt.wantSignErr { + assert.Nil(t, cert) + } else { + assert.NoError(t, validateSSHCertificate(cert, tt.expected)) + } + } + } + }) + } +} + func TestGCP_AuthorizeRenewal(t *testing.T) { p1, err := generateGCP() assert.FatalError(t, err) From 00ebee870bbca81b7d3c6d7a4d8885a8f3b06835 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 1 Aug 2019 13:13:50 -0700 Subject: [PATCH 32/40] Do not show value on boolean flags help. --- cmd/step-ca/main.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/step-ca/main.go b/cmd/step-ca/main.go index 2513b552..3a63e2ef 100644 --- a/cmd/step-ca/main.go +++ b/cmd/step-ca/main.go @@ -226,7 +226,11 @@ func stringifyFlag(f cli.Flag) string { usage := fv.FieldByName("Usage").String() placeholder := placeholderString.FindString(usage) if placeholder == "" { - placeholder = "" + switch f.(type) { + case cli.BoolFlag, cli.BoolTFlag: + default: + placeholder = "" + } } return cli.FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder) + "\t" + usage } From 004ea12212588acdce971a182c785d1868719da0 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 1 Aug 2019 15:04:56 -0700 Subject: [PATCH 33/40] Allow to use custom SSH user/host key files. --- authority/authority.go | 34 ++++++++++++++++++++++++++++++++-- authority/authorize.go | 35 ++++++++++++++++++++++------------- authority/config.go | 7 +++++++ authority/ssh.go | 12 ++++++++++++ 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/authority/authority.go b/authority/authority.go index c4d9d1cd..848a4f63 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -8,6 +8,7 @@ import ( "sync" "time" + "github.com/pkg/errors" "github.com/smallstep/certificates/authority/provisioner" "github.com/smallstep/certificates/db" "github.com/smallstep/cli/crypto/pemutil" @@ -120,8 +121,21 @@ func (a *Authority) init() error { } } - a.sshCAHostCertSignKey = a.intermediateIdentity.Key.(crypto.Signer) - a.sshCAUserCertSignKey = a.intermediateIdentity.Key.(crypto.Signer) + // Decrypt and load SSH keys + if a.config.SSH != nil { + if a.config.SSH.HostKey != "" { + a.sshCAHostCertSignKey, err = parseCryptoSigner(a.config.SSH.HostKey, a.config.Password) + if err != nil { + return err + } + } + if a.config.SSH.UserKey != "" { + a.sshCAUserCertSignKey, err = parseCryptoSigner(a.config.SSH.UserKey, a.config.Password) + if err != nil { + return err + } + } + } // Store all the provisioners for _, p := range a.config.AuthorityConfig.Provisioners { @@ -149,3 +163,19 @@ func (a *Authority) GetDatabase() db.AuthDB { func (a *Authority) Shutdown() error { return a.db.Shutdown() } + +func parseCryptoSigner(filename, password string) (crypto.Signer, error) { + var opts []pemutil.Options + if password != "" { + opts = append(opts, pemutil.WithPassword([]byte(password))) + } + key, err := pemutil.Read(filename, opts...) + if err != nil { + return nil, err + } + signer, ok := key.(crypto.Signer) + if !ok { + return nil, errors.Errorf("key %s of type %T cannot be used for signing operations", filename, key) + } + return signer, nil +} diff --git a/authority/authorize.go b/authority/authorize.go index 6ce2d5bd..e514681a 100644 --- a/authority/authorize.go +++ b/authority/authorize.go @@ -78,20 +78,13 @@ func (a *Authority) authorizeToken(ott string) (provisioner.Interface, error) { func (a *Authority) Authorize(ctx context.Context, ott string) ([]provisioner.SignOption, error) { var errContext = apiCtx{"ott": ott} switch m := provisioner.MethodFromContext(ctx); m { - case provisioner.SignMethod, provisioner.SignSSHMethod: - p, err := a.authorizeToken(ott) - if err != nil { - return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext} - } - - // Call the provisioner AuthorizeSign method to apply provisioner specific - // auth claims and get the signing options. - opts, err := p.AuthorizeSign(ctx, ott) - if err != nil { - return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext} + case provisioner.SignMethod: + return a.authorizeSign(ctx, ott) + case provisioner.SignSSHMethod: + if a.sshCAHostCertSignKey == nil && a.sshCAUserCertSignKey == nil { + return nil, &apiError{errors.New("authorize: ssh signing is not enabled"), http.StatusNotImplemented, errContext} } - - return opts, nil + return a.authorizeSign(ctx, ott) case provisioner.RevokeMethod: return nil, &apiError{errors.New("authorize: revoke method is not supported"), http.StatusInternalServerError, errContext} default: @@ -99,6 +92,22 @@ func (a *Authority) Authorize(ctx context.Context, ott string) ([]provisioner.Si } } +// authorizeSign loads the provisioner from the token, checks that it has not +// been used again and calls the provisioner AuthorizeSign method. returns a +// list of methods to apply to the signing flow. +func (a *Authority) authorizeSign(ctx context.Context, ott string) ([]provisioner.SignOption, error) { + var errContext = apiCtx{"ott": ott} + p, err := a.authorizeToken(ott) + if err != nil { + return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext} + } + opts, err := p.AuthorizeSign(ctx, ott) + if err != nil { + return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext} + } + return opts, nil +} + // AuthorizeSign authorizes a signature request by validating and authenticating // a OTT that must be sent w/ the request. func (a *Authority) AuthorizeSign(ott string) ([]provisioner.SignOption, error) { diff --git a/authority/config.go b/authority/config.go index 7cfdf744..4117d279 100644 --- a/authority/config.go +++ b/authority/config.go @@ -50,6 +50,7 @@ type Config struct { IntermediateKey string `json:"key"` Address string `json:"address"` DNSNames []string `json:"dnsNames"` + SSH *SSHConfig `json:"ssh,omitempty"` Logger json.RawMessage `json:"logger,omitempty"` DB *db.Config `json:"db,omitempty"` Monitoring json.RawMessage `json:"monitoring,omitempty"` @@ -98,6 +99,12 @@ func (c *AuthConfig) Validate(audiences provisioner.Audiences) error { return nil } +// SSHConfig contains the user and host keys. +type SSHConfig struct { + HostKey string `json:"hostKey"` + UserKey string `json:"userKey"` +} + // LoadConfiguration parses the given filename in JSON format and returns the // configuration struct. func LoadConfiguration(filename string) (*Config, error) { diff --git a/authority/ssh.go b/authority/ssh.go index 952811ec..e3201683 100644 --- a/authority/ssh.go +++ b/authority/ssh.go @@ -81,8 +81,20 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign var signer ssh.Signer switch cert.CertType { case ssh.UserCert: + if a.sshCAUserCertSignKey == nil { + return nil, &apiError{ + err: errors.New("signSSH: user certificate signing is not enabled"), + code: http.StatusNotImplemented, + } + } signer, err = ssh.NewSignerFromSigner(a.sshCAUserCertSignKey) case ssh.HostCert: + if a.sshCAHostCertSignKey == nil { + return nil, &apiError{ + err: errors.New("signSSH: host certificate signing is not enabled"), + code: http.StatusNotImplemented, + } + } signer, err = ssh.NewSignerFromSigner(a.sshCAHostCertSignKey) default: return nil, &apiError{ From 390aecca0b7f705c4b05ef7235cd638a7f37e8cb Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 1 Aug 2019 18:15:04 -0700 Subject: [PATCH 34/40] Check for error creating signers. --- authority/ssh.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/authority/ssh.go b/authority/ssh.go index e3201683..d6f9dc4c 100644 --- a/authority/ssh.go +++ b/authority/ssh.go @@ -87,7 +87,12 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign code: http.StatusNotImplemented, } } - signer, err = ssh.NewSignerFromSigner(a.sshCAUserCertSignKey) + if signer, err = ssh.NewSignerFromSigner(a.sshCAUserCertSignKey); err != nil { + return nil, &apiError{ + err: errors.Wrap(err, "signSSH: error creating signer"), + code: http.StatusInternalServerError, + } + } case ssh.HostCert: if a.sshCAHostCertSignKey == nil { return nil, &apiError{ @@ -95,7 +100,12 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign code: http.StatusNotImplemented, } } - signer, err = ssh.NewSignerFromSigner(a.sshCAHostCertSignKey) + if signer, err = ssh.NewSignerFromSigner(a.sshCAHostCertSignKey); err != nil { + return nil, &apiError{ + err: errors.Wrap(err, "signSSH: error creating signer"), + code: http.StatusInternalServerError, + } + } default: return nil, &apiError{ err: errors.Errorf("unexpected ssh certificate type: %d", cert.CertType), From e71072d389b0002ef397c86dd2f0a1a9a89cd5e6 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 2 Aug 2019 17:48:34 -0700 Subject: [PATCH 35/40] Add experimental support for provisioning users. --- api/ssh.go | 42 +++++-- authority/config.go | 6 +- authority/provisioner/sign_ssh_options.go | 15 +++ authority/ssh.go | 131 +++++++++++++++++++--- 4 files changed, 170 insertions(+), 24 deletions(-) diff --git a/api/ssh.go b/api/ssh.go index 7e730bc3..92deff5e 100644 --- a/api/ssh.go +++ b/api/ssh.go @@ -14,21 +14,24 @@ import ( // SSHAuthority is the interface implemented by a SSH CA authority. type SSHAuthority interface { SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) + SignSSHAddUser(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) } // SignSSHRequest is the request body of an SSH certificate request. type SignSSHRequest struct { - PublicKey []byte `json:"publicKey"` //base64 encoded - OTT string `json:"ott"` - CertType string `json:"certType"` - Principals []string `json:"principals"` - ValidAfter TimeDuration `json:"validAfter"` - ValidBefore TimeDuration `json:"validBefore"` + PublicKey []byte `json:"publicKey"` //base64 encoded + OTT string `json:"ott"` + CertType string `json:"certType,omitempty"` + Principals []string `json:"principals,omitempty"` + ValidAfter TimeDuration `json:"validAfter,omitempty"` + ValidBefore TimeDuration `json:"validBefore,omitempty"` + AddUserPublicKey []byte `json:"addUserPublicKey,omitempty"` } // SignSSHResponse is the response object that returns the SSH certificate. type SignSSHResponse struct { - Certificate SSHCertificate `json:"crt"` + Certificate SSHCertificate `json:"crt"` + AddUserCertificate SSHCertificate `json:"addUserCrt"` } // SSHCertificate represents the response SSH certificate. @@ -53,6 +56,10 @@ func (c *SSHCertificate) UnmarshalJSON(data []byte) error { if err := json.Unmarshal(data, &s); err != nil { return errors.Wrap(err, "error decoding certificate") } + if s == "" { + c.Certificate = nil + return nil + } certData, err := base64.StdEncoding.DecodeString(s) if err != nil { return errors.Wrap(err, "error decoding certificate") @@ -105,6 +112,15 @@ func (h *caHandler) SignSSH(w http.ResponseWriter, r *http.Request) { return } + var addUserPublicKey ssh.PublicKey + if body.AddUserPublicKey != nil { + addUserPublicKey, err = ssh.ParsePublicKey(body.AddUserPublicKey) + if err != nil { + WriteError(w, BadRequest(errors.Wrap(err, "error parsing addUserPublicKey"))) + return + } + } + opts := provisioner.SSHOptions{ CertType: body.CertType, Principals: body.Principals, @@ -125,9 +141,19 @@ func (h *caHandler) SignSSH(w http.ResponseWriter, r *http.Request) { return } + var addUserCert *ssh.Certificate + if addUserPublicKey != nil && cert.CertType == ssh.UserCert && len(cert.ValidPrincipals) == 1 { + addUserCert, err = h.Authority.SignSSHAddUser(addUserPublicKey, cert) + if err != nil { + WriteError(w, Forbidden(err)) + return + } + } + w.WriteHeader(http.StatusCreated) // logCertificate(w, cert) JSON(w, &SignSSHResponse{ - Certificate: SSHCertificate{cert}, + Certificate: SSHCertificate{cert}, + AddUserCertificate: SSHCertificate{addUserCert}, }) } diff --git a/authority/config.go b/authority/config.go index 4117d279..412f54db 100644 --- a/authority/config.go +++ b/authority/config.go @@ -101,8 +101,10 @@ func (c *AuthConfig) Validate(audiences provisioner.Audiences) error { // SSHConfig contains the user and host keys. type SSHConfig struct { - HostKey string `json:"hostKey"` - UserKey string `json:"userKey"` + HostKey string `json:"hostKey"` + UserKey string `json:"userKey"` + AddUserPrincipal string `json:"addUserPrincipal"` + AddUserCommand string `json:"addUserCommand"` } // LoadConfiguration parses the given filename in JSON format and returns the diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index d1bf78bd..83f4ee15 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -14,6 +14,9 @@ const ( // SSHHostCert is the string used to represent ssh.HostCert. SSHHostCert = "host" + + // sshProvisionerCommand is the provisioner command + sshProvisionerCommand = "sudo adduser --quiet --disabled-password --gecos '' %s 2>/dev/null ; nc -q0 localhost 22" ) // SSHCertificateModifier is the interface used to change properties in an SSH @@ -188,6 +191,18 @@ func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error { } } +type sshProvisionerExtensionModifier string + +func (m sshProvisionerExtensionModifier) Modify(cert *ssh.Certificate) error { + if cert.CertType == ssh.UserCert { + if cert.CriticalOptions == nil { + cert.CriticalOptions = make(map[string]string) + } + cert.CriticalOptions["force-command"] = fmt.Sprintf(sshProvisionerCommand, m) + } + return nil +} + // sshCertificateValidityModifier is a SSHCertificateModifier checks the // validity bounds, setting them if they are not provided. It will fail if a // CertType has not been set or is not valid. diff --git a/authority/ssh.go b/authority/ssh.go index d6f9dc4c..6dd37a67 100644 --- a/authority/ssh.go +++ b/authority/ssh.go @@ -2,9 +2,7 @@ package authority import ( "crypto/rand" - "crypto/sha256" "encoding/binary" - "encoding/hex" "net/http" "strings" @@ -14,10 +12,17 @@ import ( "golang.org/x/crypto/ssh" ) -func generateSSHPublicKeyID(key ssh.PublicKey) string { - sum := sha256.Sum256(key.Marshal()) - return strings.ToLower(hex.EncodeToString(sum[:])) -} +const ( + // SSHAddUserPrincipal is the principal that will run the add user command. + // Defaults to "provisioner" but it can be changed in the configuration. + SSHAddUserPrincipal = "provisioner" + + // SSHAddUserCommand is the default command to run to add a new user. + // Defaults to "sudo useradd -m ; nc -q0 localhost 22" but it can be changed in the + // configuration. The string "" will be replace by the new + // principal to add. + SSHAddUserCommand = "sudo useradd -m ; nc -q0 localhost 22" +) // SignSSH creates a signed SSH certificate with the given public key and options. func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) { @@ -38,7 +43,7 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign // validate the given SSHOptions case provisioner.SSHCertificateOptionsValidator: if err := o.Valid(opts); err != nil { - return nil, &apiError{err: err, code: http.StatusUnauthorized} + return nil, &apiError{err: err, code: http.StatusForbidden} } default: return nil, &apiError{ @@ -50,12 +55,15 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign nonce, err := randutil.ASCII(32) if err != nil { - return nil, err + return nil, &apiError{err: err, code: http.StatusInternalServerError} } var serial uint64 if err := binary.Read(rand.Reader, binary.BigEndian, &serial); err != nil { - return nil, errors.Wrap(err, "error reading random number") + return nil, &apiError{ + err: errors.Wrap(err, "signSSH: error reading random number"), + code: http.StatusInternalServerError, + } } // Build base certificate with the key and some random values @@ -67,13 +75,13 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign // Use opts to modify the certificate if err := opts.Modify(cert); err != nil { - return nil, err + return nil, &apiError{err: err, code: http.StatusForbidden} } // Use provisioner modifiers for _, m := range mods { if err := m.Modify(cert); err != nil { - return nil, &apiError{err: err, code: http.StatusInternalServerError} + return nil, &apiError{err: err, code: http.StatusForbidden} } } @@ -108,7 +116,7 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign } default: return nil, &apiError{ - err: errors.Errorf("unexpected ssh certificate type: %d", cert.CertType), + err: errors.Errorf("signSSH: unexpected ssh certificate type: %d", cert.CertType), code: http.StatusInternalServerError, } } @@ -121,16 +129,111 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign // Sign the certificate sig, err := signer.Sign(rand.Reader, data) if err != nil { - return nil, err + return nil, &apiError{ + err: errors.Wrap(err, "signSSH: error signing certificate"), + code: http.StatusInternalServerError, + } } cert.Signature = sig // User provisioners validators for _, v := range validators { if err := v.Valid(cert); err != nil { - return nil, &apiError{err: err, code: http.StatusUnauthorized} + return nil, &apiError{err: err, code: http.StatusForbidden} + } + } + + return cert, nil +} + +// SignSSHAddUser signs a certificate that provisions a new user in a server. +func (a *Authority) SignSSHAddUser(key ssh.PublicKey, subject *ssh.Certificate) (*ssh.Certificate, error) { + if a.sshCAUserCertSignKey == nil { + return nil, &apiError{ + err: errors.New("signSSHProxy: user certificate signing is not enabled"), + code: http.StatusNotImplemented, + } + } + if subject.CertType != ssh.UserCert { + return nil, &apiError{ + err: errors.New("signSSHProxy: certificate is not a user certificate"), + code: http.StatusForbidden, + } + } + if len(subject.ValidPrincipals) != 1 { + return nil, &apiError{ + err: errors.New("signSSHProxy: certificate does not have only one principal"), + code: http.StatusForbidden, + } + } + + nonce, err := randutil.ASCII(32) + if err != nil { + return nil, &apiError{err: err, code: http.StatusInternalServerError} + } + + var serial uint64 + if err := binary.Read(rand.Reader, binary.BigEndian, &serial); err != nil { + return nil, &apiError{ + err: errors.Wrap(err, "signSSHProxy: error reading random number"), + code: http.StatusInternalServerError, + } + } + + signer, err := ssh.NewSignerFromSigner(a.sshCAUserCertSignKey) + if err != nil { + return nil, &apiError{ + err: errors.Wrap(err, "signSSHProxy: error creating signer"), + code: http.StatusInternalServerError, } } + principal := subject.ValidPrincipals[0] + addUserPrincipal := a.getAddUserPrincipal() + + cert := &ssh.Certificate{ + Nonce: []byte(nonce), + Key: key, + Serial: serial, + CertType: ssh.UserCert, + KeyId: principal + "-" + addUserPrincipal, + ValidPrincipals: []string{addUserPrincipal}, + ValidAfter: subject.ValidAfter, + ValidBefore: subject.ValidBefore, + Permissions: ssh.Permissions{ + CriticalOptions: map[string]string{ + "force-command": a.getAddUserCommand(principal), + }, + }, + SignatureKey: signer.PublicKey(), + } + + // Get bytes for signing trailing the signature length. + data := cert.Marshal() + data = data[:len(data)-4] + + // Sign the certificate + sig, err := signer.Sign(rand.Reader, data) + if err != nil { + return nil, err + } + cert.Signature = sig return cert, nil } + +func (a *Authority) getAddUserPrincipal() (cmd string) { + if a.config.SSH.AddUserPrincipal == "" { + return SSHAddUserPrincipal + } + return a.config.SSH.AddUserPrincipal +} + +func (a *Authority) getAddUserCommand(principal string) string { + var cmd string + if a.config.SSH.AddUserCommand == "" { + cmd = SSHAddUserCommand + } else { + cmd = a.config.SSH.AddUserCommand + } + return strings.Replace(cmd, "", principal, -1) +} From 57a529cc1a0cd6b4309abe11d20a8d0b3b12ac3d Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 5 Aug 2019 11:40:27 -0700 Subject: [PATCH 36/40] Allow to enable the SSH CA per provisioner --- authority/config.go | 2 ++ authority/provisioner/aws.go | 3 +++ authority/provisioner/azure.go | 3 +++ authority/provisioner/claims.go | 13 +++++++++++++ authority/provisioner/gcp.go | 3 +++ authority/provisioner/jwk.go | 3 +++ authority/provisioner/oidc.go | 3 +++ 7 files changed, 30 insertions(+) diff --git a/authority/config.go b/authority/config.go index 412f54db..99fdf457 100644 --- a/authority/config.go +++ b/authority/config.go @@ -28,6 +28,7 @@ var ( Renegotiation: false, } defaultDisableRenewal = false + defaultEnableSSHCA = false globalProvisionerClaims = provisioner.Claims{ MinTLSDur: &provisioner.Duration{Duration: 5 * time.Minute}, // TLS certs MaxTLSDur: &provisioner.Duration{Duration: 24 * time.Hour}, @@ -39,6 +40,7 @@ var ( MinHostSSHDur: &provisioner.Duration{Duration: 5 * time.Minute}, // Host SSH certs MaxHostSSHDur: &provisioner.Duration{Duration: 30 * 24 * time.Hour}, DefaultHostSSHDur: &provisioner.Duration{Duration: 30 * 24 * time.Hour}, + EnableSSHCA: &defaultEnableSSHCA, } ) diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go index 07285940..dbe33c5c 100644 --- a/authority/provisioner/aws.go +++ b/authority/provisioner/aws.go @@ -275,6 +275,9 @@ func (p *AWS) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er // Check for the sign ssh method, default to sign X.509 if m := MethodFromContext(ctx); m == SignSSHMethod { + if p.claimer.IsSSHCAEnabled() == false { + return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID()) + } return p.authorizeSSHSign(payload) } diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go index d84e9f91..6b298133 100644 --- a/authority/provisioner/azure.go +++ b/authority/provisioner/azure.go @@ -267,6 +267,9 @@ func (p *Azure) AuthorizeSign(ctx context.Context, token string) ([]SignOption, // Check for the sign ssh method, default to sign X.509 if m := MethodFromContext(ctx); m == SignSSHMethod { + if p.claimer.IsSSHCAEnabled() == false { + return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID()) + } return p.authorizeSSHSign(claims, name) } diff --git a/authority/provisioner/claims.go b/authority/provisioner/claims.go index 23c85002..4eba5ad7 100644 --- a/authority/provisioner/claims.go +++ b/authority/provisioner/claims.go @@ -20,6 +20,7 @@ type Claims struct { MinHostSSHDur *Duration `json:"minHostSSHCertDuration,omitempty"` MaxHostSSHDur *Duration `json:"maxHostSSHCertDuration,omitempty"` DefaultHostSSHDur *Duration `json:"defaultHostSSHCertDuration,omitempty"` + EnableSSHCA *bool `json:"enableSSHCA,omitempty"` } // Claimer is the type that controls claims. It provides an interface around the @@ -38,6 +39,7 @@ func NewClaimer(claims *Claims, global Claims) (*Claimer, error) { // Claims returns the merge of the inner and global claims. func (c *Claimer) Claims() Claims { disableRenewal := c.IsDisableRenewal() + enableSSHCA := c.IsSSHCAEnabled() return Claims{ MinTLSDur: &Duration{c.MinTLSCertDuration()}, MaxTLSDur: &Duration{c.MaxTLSCertDuration()}, @@ -49,6 +51,7 @@ func (c *Claimer) Claims() Claims { MinHostSSHDur: &Duration{c.MinHostSSHCertDuration()}, MaxHostSSHDur: &Duration{c.MaxHostSSHCertDuration()}, DefaultHostSSHDur: &Duration{c.DefaultHostSSHCertDuration()}, + EnableSSHCA: &enableSSHCA, } } @@ -152,6 +155,16 @@ func (c *Claimer) MaxHostSSHCertDuration() time.Duration { return c.claims.MaxHostSSHDur.Duration } +// IsSSHCAEnabled returns if the SSH CA is enabled for the provisioner. If the +// property is not set within the provisioner, then the global value from the +// authority configuration will be used. +func (c *Claimer) IsSSHCAEnabled() bool { + if c.claims == nil || c.claims.EnableSSHCA == nil { + return *c.global.EnableSSHCA + } + return *c.claims.EnableSSHCA +} + // Validate validates and modifies the Claims with default values. func (c *Claimer) Validate() error { var ( diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go index d5dde616..0604c572 100644 --- a/authority/provisioner/gcp.go +++ b/authority/provisioner/gcp.go @@ -214,6 +214,9 @@ func (p *GCP) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er // Check for the sign ssh method, default to sign X.509 if m := MethodFromContext(ctx); m == SignSSHMethod { + if p.claimer.IsSSHCAEnabled() == false { + return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID()) + } return p.authorizeSSHSign(claims) } diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go index 012e08a9..5ecf4d6e 100644 --- a/authority/provisioner/jwk.go +++ b/authority/provisioner/jwk.go @@ -143,6 +143,9 @@ func (p *JWK) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er // Check for SSH token if claims.Step != nil && claims.Step.SSH != nil { + if p.claimer.IsSSHCAEnabled() == false { + return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID()) + } return p.authorizeSSHSign(claims) } diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index 0c0f840e..a71d2c1b 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -268,6 +268,9 @@ func (o *OIDC) AuthorizeSign(ctx context.Context, token string) ([]SignOption, e // Check for the sign ssh method, default to sign X.509 if m := MethodFromContext(ctx); m == SignSSHMethod { + if o.claimer.IsSSHCAEnabled() == false { + return nil, errors.Errorf("ssh ca is disabled for provisioner %s", o.GetID()) + } return o.authorizeSSHSign(claims) } From ca74bb1de52dd993c06c39dc94a78ea7003ec213 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 5 Aug 2019 16:06:05 -0700 Subject: [PATCH 37/40] Add ssh api tests. --- api/api_test.go | 11 +- api/ssh.go | 16 +-- api/ssh_test.go | 327 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 344 insertions(+), 10 deletions(-) create mode 100644 api/ssh_test.go diff --git a/api/api_test.go b/api/api_test.go index 7a3c843d..4fb980e2 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -23,14 +23,13 @@ import ( "testing" "time" - "golang.org/x/crypto/ssh" - "github.com/go-chi/chi" "github.com/smallstep/certificates/authority" "github.com/smallstep/certificates/authority/provisioner" "github.com/smallstep/certificates/logging" "github.com/smallstep/cli/crypto/tlsutil" "github.com/smallstep/cli/jose" + "golang.org/x/crypto/ssh" ) const ( @@ -498,6 +497,7 @@ type mockAuthority struct { root func(shasum string) (*x509.Certificate, error) sign func(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error) singSSH func(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) + singSSHAddUser func(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) renew func(cert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error) loadProvisionerByCertificate func(cert *x509.Certificate) (provisioner.Interface, error) getProvisioners func(nextCursor string, limit int) (provisioner.List, string, error) @@ -547,6 +547,13 @@ func (m *mockAuthority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, return m.ret1.(*ssh.Certificate), m.err } +func (m *mockAuthority) SignSSHAddUser(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) { + if m.singSSHAddUser != nil { + return m.singSSHAddUser(key, cert) + } + return m.ret1.(*ssh.Certificate), m.err +} + func (m *mockAuthority) Renew(cert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error) { if m.renew != nil { return m.renew(cert) diff --git a/api/ssh.go b/api/ssh.go index 92deff5e..a847c9a0 100644 --- a/api/ssh.go +++ b/api/ssh.go @@ -30,13 +30,13 @@ type SignSSHRequest struct { // SignSSHResponse is the response object that returns the SSH certificate. type SignSSHResponse struct { - Certificate SSHCertificate `json:"crt"` - AddUserCertificate SSHCertificate `json:"addUserCrt"` + Certificate SSHCertificate `json:"crt"` + AddUserCertificate *SSHCertificate `json:"addUserCrt,omitempty"` } // SSHCertificate represents the response SSH certificate. type SSHCertificate struct { - *ssh.Certificate + *ssh.Certificate `json:"omitempty"` } // MarshalJSON implements the json.Marshaler interface. The certificate is @@ -102,7 +102,7 @@ func (h *caHandler) SignSSH(w http.ResponseWriter, r *http.Request) { logOtt(w, body.OTT) if err := body.Validate(); err != nil { - WriteError(w, err) + WriteError(w, BadRequest(err)) return } @@ -141,19 +141,19 @@ func (h *caHandler) SignSSH(w http.ResponseWriter, r *http.Request) { return } - var addUserCert *ssh.Certificate + var addUserCertificate *SSHCertificate if addUserPublicKey != nil && cert.CertType == ssh.UserCert && len(cert.ValidPrincipals) == 1 { - addUserCert, err = h.Authority.SignSSHAddUser(addUserPublicKey, cert) + addUserCert, err := h.Authority.SignSSHAddUser(addUserPublicKey, cert) if err != nil { WriteError(w, Forbidden(err)) return } + addUserCertificate = &SSHCertificate{addUserCert} } w.WriteHeader(http.StatusCreated) - // logCertificate(w, cert) JSON(w, &SignSSHResponse{ Certificate: SSHCertificate{cert}, - AddUserCertificate: SSHCertificate{addUserCert}, + AddUserCertificate: addUserCertificate, }) } diff --git a/api/ssh_test.go b/api/ssh_test.go new file mode 100644 index 00000000..df3edf31 --- /dev/null +++ b/api/ssh_test.go @@ -0,0 +1,327 @@ +package api + +import ( + "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "encoding/base64" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "reflect" + "testing" + "time" + + "github.com/smallstep/assert" + "github.com/smallstep/certificates/authority/provisioner" + "github.com/smallstep/certificates/logging" + "golang.org/x/crypto/ssh" +) + +var ( + sshSignerKey = mustKey() + sshUserKey = mustKey() + sshHostKey = mustKey() +) + +func mustKey() *ecdsa.PrivateKey { + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + panic(err) + } + return priv +} + +func signSSHCertificate(cert *ssh.Certificate) error { + signerKey, err := ssh.NewPublicKey(sshSignerKey.Public()) + if err != nil { + return err + } + signer, err := ssh.NewSignerFromSigner(sshSignerKey) + if err != nil { + return err + } + cert.SignatureKey = signerKey + data := cert.Marshal() + data = data[:len(data)-4] + sig, err := signer.Sign(rand.Reader, data) + if err != nil { + return err + } + cert.Signature = sig + return nil +} + +func getSignedUserCertificate() (*ssh.Certificate, error) { + key, err := ssh.NewPublicKey(sshUserKey.Public()) + if err != nil { + return nil, err + } + t := time.Now() + cert := &ssh.Certificate{ + Nonce: []byte("1234567890"), + Key: key, + Serial: 1234567890, + CertType: ssh.UserCert, + KeyId: "user@localhost", + ValidPrincipals: []string{"user"}, + ValidAfter: uint64(t.Unix()), + ValidBefore: uint64(t.Add(time.Hour).Unix()), + Permissions: ssh.Permissions{ + CriticalOptions: map[string]string{}, + Extensions: map[string]string{ + "permit-X11-forwarding": "", + "permit-agent-forwarding": "", + "permit-port-forwarding": "", + "permit-pty": "", + "permit-user-rc": "", + }, + }, + Reserved: []byte{}, + } + if err := signSSHCertificate(cert); err != nil { + return nil, err + } + return cert, nil +} + +func getSignedHostCertificate() (*ssh.Certificate, error) { + key, err := ssh.NewPublicKey(sshHostKey.Public()) + if err != nil { + return nil, err + } + t := time.Now() + cert := &ssh.Certificate{ + Nonce: []byte("1234567890"), + Key: key, + Serial: 1234567890, + CertType: ssh.UserCert, + KeyId: "internal.smallstep.com", + ValidPrincipals: []string{"internal.smallstep.com"}, + ValidAfter: uint64(t.Unix()), + ValidBefore: uint64(t.Add(time.Hour).Unix()), + Permissions: ssh.Permissions{ + CriticalOptions: map[string]string{}, + Extensions: map[string]string{}, + }, + Reserved: []byte{}, + } + if err := signSSHCertificate(cert); err != nil { + return nil, err + } + return cert, nil +} + +func TestSSHCertificate_MarshalJSON(t *testing.T) { + user, err := getSignedUserCertificate() + assert.FatalError(t, err) + host, err := getSignedHostCertificate() + assert.FatalError(t, err) + userB64 := base64.StdEncoding.EncodeToString(user.Marshal()) + hostB64 := base64.StdEncoding.EncodeToString(host.Marshal()) + + type fields struct { + Certificate *ssh.Certificate + } + tests := []struct { + name string + fields fields + want []byte + wantErr bool + }{ + {"nil", fields{Certificate: nil}, []byte("null"), false}, + {"user", fields{Certificate: user}, []byte(`"` + userB64 + `"`), false}, + {"user", fields{Certificate: host}, []byte(`"` + hostB64 + `"`), false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := SSHCertificate{ + Certificate: tt.fields.Certificate, + } + got, err := c.MarshalJSON() + if (err != nil) != tt.wantErr { + t.Errorf("SSHCertificate.MarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("SSHCertificate.MarshalJSON() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestSSHCertificate_UnmarshalJSON(t *testing.T) { + user, err := getSignedUserCertificate() + assert.FatalError(t, err) + host, err := getSignedHostCertificate() + assert.FatalError(t, err) + userB64 := base64.StdEncoding.EncodeToString(user.Marshal()) + hostB64 := base64.StdEncoding.EncodeToString(host.Marshal()) + keyB64 := base64.StdEncoding.EncodeToString(user.Key.Marshal()) + + type args struct { + data []byte + } + tests := []struct { + name string + args args + want *ssh.Certificate + wantErr bool + }{ + {"null", args{[]byte(`null`)}, nil, false}, + {"empty", args{[]byte(`""`)}, nil, false}, + {"user", args{[]byte(`"` + userB64 + `"`)}, user, false}, + {"host", args{[]byte(`"` + hostB64 + `"`)}, host, false}, + {"bad-string", args{[]byte(userB64)}, nil, true}, + {"bad-base64", args{[]byte(`"this-is-not-base64"`)}, nil, true}, + {"bad-key", args{[]byte(`"bm90LWEta2V5"`)}, nil, true}, + {"bat-cert", args{[]byte(`"` + keyB64 + `"`)}, nil, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &SSHCertificate{} + if err := c.UnmarshalJSON(tt.args.data); (err != nil) != tt.wantErr { + t.Errorf("SSHCertificate.UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(tt.want, c.Certificate) { + t.Errorf("SSHCertificate.UnmarshalJSON() got = %v, want %v\n", c.Certificate, tt.want) + } + }) + } +} + +func TestSignSSHRequest_Validate(t *testing.T) { + type fields struct { + PublicKey []byte + OTT string + CertType string + Principals []string + ValidAfter TimeDuration + ValidBefore TimeDuration + AddUserPublicKey []byte + } + tests := []struct { + name string + fields fields + wantErr bool + }{ + {"ok-empty", fields{[]byte("Zm9v"), "ott", "", []string{"user"}, TimeDuration{}, TimeDuration{}, nil}, false}, + {"ok-user", fields{[]byte("Zm9v"), "ott", "user", []string{"user"}, TimeDuration{}, TimeDuration{}, nil}, false}, + {"ok-host", fields{[]byte("Zm9v"), "ott", "host", []string{"user"}, TimeDuration{}, TimeDuration{}, nil}, false}, + {"key", fields{nil, "ott", "user", []string{"user"}, TimeDuration{}, TimeDuration{}, nil}, true}, + {"key", fields{[]byte(""), "ott", "user", []string{"user"}, TimeDuration{}, TimeDuration{}, nil}, true}, + {"type", fields{[]byte("Zm9v"), "ott", "foo", []string{"user"}, TimeDuration{}, TimeDuration{}, nil}, true}, + {"ott", fields{[]byte("Zm9v"), "", "user", []string{"user"}, TimeDuration{}, TimeDuration{}, nil}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &SignSSHRequest{ + PublicKey: tt.fields.PublicKey, + OTT: tt.fields.OTT, + CertType: tt.fields.CertType, + Principals: tt.fields.Principals, + ValidAfter: tt.fields.ValidAfter, + ValidBefore: tt.fields.ValidBefore, + AddUserPublicKey: tt.fields.AddUserPublicKey, + } + if err := s.Validate(); (err != nil) != tt.wantErr { + t.Errorf("SignSSHRequest.Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func Test_caHandler_SignSSH(t *testing.T) { + user, err := getSignedUserCertificate() + assert.FatalError(t, err) + host, err := getSignedHostCertificate() + assert.FatalError(t, err) + + userB64 := base64.StdEncoding.EncodeToString(user.Marshal()) + hostB64 := base64.StdEncoding.EncodeToString(host.Marshal()) + + userReq, err := json.Marshal(SignSSHRequest{ + PublicKey: user.Key.Marshal(), + OTT: "ott", + }) + assert.FatalError(t, err) + hostReq, err := json.Marshal(SignSSHRequest{ + PublicKey: host.Key.Marshal(), + OTT: "ott", + }) + assert.FatalError(t, err) + userAddReq, err := json.Marshal(SignSSHRequest{ + PublicKey: user.Key.Marshal(), + OTT: "ott", + AddUserPublicKey: user.Key.Marshal(), + }) + assert.FatalError(t, err) + + type fields struct { + Authority Authority + } + type args struct { + w http.ResponseWriter + r *http.Request + } + tests := []struct { + name string + req []byte + authErr error + signCert *ssh.Certificate + signErr error + addUserCert *ssh.Certificate + addUserErr error + body []byte + statusCode int + }{ + {"ok-user", userReq, nil, user, nil, nil, nil, []byte(fmt.Sprintf(`{"crt":"%s"}`, userB64)), http.StatusCreated}, + {"ok-host", hostReq, nil, host, nil, nil, nil, []byte(fmt.Sprintf(`{"crt":"%s"}`, hostB64)), http.StatusCreated}, + {"ok-user-add", userAddReq, nil, user, nil, user, nil, []byte(fmt.Sprintf(`{"crt":"%s","addUserCrt":"%s"}`, userB64, userB64)), http.StatusCreated}, + {"fail-body", []byte("bad-json"), nil, nil, nil, nil, nil, nil, http.StatusBadRequest}, + {"fail-validate", []byte("{}"), nil, nil, nil, nil, nil, nil, http.StatusBadRequest}, + {"fail-publicKey", []byte(`{"publicKey":"Zm9v","ott":"ott"}`), nil, nil, nil, nil, nil, nil, http.StatusBadRequest}, + {"fail-publicKey", []byte(fmt.Sprintf(`{"publicKey":"%s","ott":"ott","addUserPublicKey":"Zm9v"}`, base64.StdEncoding.EncodeToString(user.Key.Marshal()))), nil, nil, nil, nil, nil, nil, http.StatusBadRequest}, + {"fail-authorize", userReq, fmt.Errorf("an-error"), nil, nil, nil, nil, nil, http.StatusUnauthorized}, + {"fail-signSSH", userReq, nil, nil, fmt.Errorf("an-error"), nil, nil, nil, http.StatusForbidden}, + {"fail-SignSSHAddUser", userAddReq, nil, user, nil, nil, fmt.Errorf("an-error"), nil, http.StatusForbidden}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := New(&mockAuthority{ + authorizeSign: func(ott string) ([]provisioner.SignOption, error) { + return []provisioner.SignOption{}, tt.authErr + }, + singSSH: func(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) { + return tt.signCert, tt.signErr + }, + singSSHAddUser: func(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) { + return tt.addUserCert, tt.addUserErr + }, + }).(*caHandler) + + req := httptest.NewRequest("POST", "http://example.com/sign-ssh", bytes.NewReader(tt.req)) + w := httptest.NewRecorder() + h.SignSSH(logging.NewResponseLogger(w), req) + res := w.Result() + + if res.StatusCode != tt.statusCode { + t.Errorf("caHandler.Root StatusCode = %d, wants %d", res.StatusCode, tt.statusCode) + } + + body, err := ioutil.ReadAll(res.Body) + res.Body.Close() + if err != nil { + t.Errorf("caHandler.Root unexpected error = %v", err) + } + if tt.statusCode < http.StatusBadRequest { + if !bytes.Equal(bytes.TrimSpace(body), tt.body) { + t.Errorf("caHandler.Root Body = %s, wants %s", body, tt.body) + } + } + }) + } +} From 34e1e3380a06abc8263637e2fc2ddf08404dc966 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 5 Aug 2019 16:14:25 -0700 Subject: [PATCH 38/40] Fix lint errors. --- Gopkg.lock | 8 +++++- authority/provisioner/jwk_test.go | 2 ++ authority/provisioner/sign_ssh_options.go | 30 ----------------------- authority/provisioner/ssh_test.go | 3 +++ 4 files changed, 12 insertions(+), 31 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index b629b1f1..f27646a9 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -344,15 +344,20 @@ [[projects]] branch = "master" - digest = "1:5dd7da6df07f42194cb25d162b4b89664ed7b08d7d4334f6a288393d54b095ce" + digest = "1:afc49fe39c8c591fc2c8ddc73adc4c69e67125dde6c58e24c91b3b0cf78602be" name = "golang.org/x/crypto" packages = [ "cryptobyte", "cryptobyte/asn1", + "curve25519", "ed25519", "ed25519/internal/edwards25519", + "internal/chacha20", + "internal/subtle", "ocsp", "pbkdf2", + "poly1305", + "ssh", "ssh/terminal", ] pruneopts = "UT" @@ -494,6 +499,7 @@ "github.com/tsenart/deadcode", "github.com/urfave/cli", "golang.org/x/crypto/ocsp", + "golang.org/x/crypto/ssh", "golang.org/x/net/http2", "gopkg.in/square/go-jose.v2", "gopkg.in/square/go-jose.v2/jwt", diff --git a/authority/provisioner/jwk_test.go b/authority/provisioner/jwk_test.go index 9952f7ff..f8f0ff88 100644 --- a/authority/provisioner/jwk_test.go +++ b/authority/provisioner/jwk_test.go @@ -15,6 +15,7 @@ import ( var ( defaultDisableRenewal = false + defaultEnableSSHCA = true globalProvisionerClaims = Claims{ MinTLSDur: &Duration{5 * time.Minute}, MaxTLSDur: &Duration{24 * time.Hour}, @@ -26,6 +27,7 @@ var ( MinHostSSHDur: &Duration{Duration: 5 * time.Minute}, // Host SSH certs MaxHostSSHDur: &Duration{Duration: 30 * 24 * time.Hour}, DefaultHostSSHDur: &Duration{Duration: 30 * 24 * time.Hour}, + EnableSSHCA: &defaultEnableSSHCA, } ) diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 83f4ee15..bcf0c798 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -1,7 +1,6 @@ package provisioner import ( - "fmt" "time" "github.com/pkg/errors" @@ -14,9 +13,6 @@ const ( // SSHHostCert is the string used to represent ssh.HostCert. SSHHostCert = "host" - - // sshProvisionerCommand is the provisioner command - sshProvisionerCommand = "sudo adduser --quiet --disabled-password --gecos '' %s 2>/dev/null ; nc -q0 localhost 22" ) // SSHCertificateModifier is the interface used to change properties in an SSH @@ -191,18 +187,6 @@ func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error { } } -type sshProvisionerExtensionModifier string - -func (m sshProvisionerExtensionModifier) Modify(cert *ssh.Certificate) error { - if cert.CertType == ssh.UserCert { - if cert.CriticalOptions == nil { - cert.CriticalOptions = make(map[string]string) - } - cert.CriticalOptions["force-command"] = fmt.Sprintf(sshProvisionerCommand, m) - } - return nil -} - // sshCertificateValidityModifier is a SSHCertificateModifier checks the // validity bounds, setting them if they are not provided. It will fail if a // CertType has not been set or is not valid. @@ -291,20 +275,6 @@ func (v *sshCertificateDefaultValidator) Valid(crt *ssh.Certificate) error { } } -// sshCertTypeName returns the string representation of the given ssh.CertType. -func sshCertTypeString(ct uint32) string { - switch ct { - case 0: - return "" - case ssh.UserCert: - return SSHUserCert - case ssh.HostCert: - return SSHHostCert - default: - return fmt.Sprintf("unknown (%d)", ct) - } -} - // sshCertTypeUInt32 func sshCertTypeUInt32(ct string) uint32 { switch ct { diff --git a/authority/provisioner/ssh_test.go b/authority/provisioner/ssh_test.go index 003f4e9a..1b31f78b 100644 --- a/authority/provisioner/ssh_test.go +++ b/authority/provisioner/ssh_test.go @@ -98,6 +98,9 @@ func signSSHCertificate(key crypto.PublicKey, opts SSHOptions, signOpts []SignOp default: return nil, fmt.Errorf("unexpected ssh certificate type: %d", cert.CertType) } + if err != nil { + return nil, err + } cert.SignatureKey = signer.PublicKey() // Get bytes for signing trailing the signature length. From db4baa04871d29fa981c07785e3d8c013065fbd8 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 5 Aug 2019 18:35:00 -0700 Subject: [PATCH 39/40] Add tests for authority sign ssh methods. --- authority/ssh_test.go | 252 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 252 insertions(+) create mode 100644 authority/ssh_test.go diff --git a/authority/ssh_test.go b/authority/ssh_test.go new file mode 100644 index 00000000..37a9a8f7 --- /dev/null +++ b/authority/ssh_test.go @@ -0,0 +1,252 @@ +package authority + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "fmt" + "testing" + "time" + + "github.com/smallstep/assert" + "github.com/smallstep/certificates/authority/provisioner" + "golang.org/x/crypto/ssh" +) + +type sshTestModifier ssh.Certificate + +func (m sshTestModifier) Modify(cert *ssh.Certificate) error { + if m.CertType != 0 { + cert.CertType = m.CertType + } + if m.KeyId != "" { + cert.KeyId = m.KeyId + } + if m.ValidAfter != 0 { + cert.ValidAfter = m.ValidAfter + } + if m.ValidBefore != 0 { + cert.ValidBefore = m.ValidBefore + } + if len(m.ValidPrincipals) != 0 { + cert.ValidPrincipals = m.ValidPrincipals + } + if m.Permissions.CriticalOptions != nil { + cert.Permissions.CriticalOptions = m.Permissions.CriticalOptions + } + if m.Permissions.Extensions != nil { + cert.Permissions.Extensions = m.Permissions.Extensions + } + return nil +} + +type sshTestCertModifier string + +func (m sshTestCertModifier) Modify(cert *ssh.Certificate) error { + if m == "" { + return nil + } + return fmt.Errorf(string(m)) +} + +type sshTestCertValidator string + +func (v sshTestCertValidator) Valid(crt *ssh.Certificate) error { + if v == "" { + return nil + } + return fmt.Errorf(string(v)) +} + +type sshTestOptionsValidator string + +func (v sshTestOptionsValidator) Valid(opts provisioner.SSHOptions) error { + if v == "" { + return nil + } + return fmt.Errorf(string(v)) +} + +type sshTestOptionsModifier string + +func (m sshTestOptionsModifier) Option(opts provisioner.SSHOptions) provisioner.SSHCertificateModifier { + return sshTestCertModifier(string(m)) +} + +func TestAuthority_SignSSH(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + assert.FatalError(t, err) + pub, err := ssh.NewPublicKey(key.Public()) + assert.FatalError(t, err) + signKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + assert.FatalError(t, err) + + userOptions := sshTestModifier{ + CertType: ssh.UserCert, + } + hostOptions := sshTestModifier{ + CertType: ssh.HostCert, + } + + now := time.Now() + + type fields struct { + sshCAUserCertSignKey crypto.Signer + sshCAHostCertSignKey crypto.Signer + } + type args struct { + key ssh.PublicKey + opts provisioner.SSHOptions + signOpts []provisioner.SignOption + } + type want struct { + CertType uint32 + Principals []string + ValidAfter uint64 + ValidBefore uint64 + } + tests := []struct { + name string + fields fields + args args + want want + wantErr bool + }{ + {"ok-user", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{userOptions}}, want{CertType: ssh.UserCert}, false}, + {"ok-host", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{hostOptions}}, want{CertType: ssh.HostCert}, false}, + {"ok-opts-type-user", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{CertType: "user"}, []provisioner.SignOption{}}, want{CertType: ssh.UserCert}, false}, + {"ok-opts-type-host", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{CertType: "host"}, []provisioner.SignOption{}}, want{CertType: ssh.HostCert}, false}, + {"ok-opts-principals", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{}}, want{CertType: ssh.UserCert, Principals: []string{"user"}}, false}, + {"ok-opts-principals", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{CertType: "host", Principals: []string{"foo.test.com", "bar.test.com"}}, []provisioner.SignOption{}}, want{CertType: ssh.HostCert, Principals: []string{"foo.test.com", "bar.test.com"}}, false}, + {"ok-opts-valid-after", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{CertType: "user", ValidAfter: provisioner.NewTimeDuration(now)}, []provisioner.SignOption{}}, want{CertType: ssh.UserCert, ValidAfter: uint64(now.Unix())}, false}, + {"ok-opts-valid-before", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{CertType: "host", ValidBefore: provisioner.NewTimeDuration(now)}, []provisioner.SignOption{}}, want{CertType: ssh.HostCert, ValidBefore: uint64(now.Unix())}, false}, + {"ok-cert-validator", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{userOptions, sshTestCertValidator("")}}, want{CertType: ssh.UserCert}, false}, + {"ok-cert-modifier", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{userOptions, sshTestCertModifier("")}}, want{CertType: ssh.UserCert}, false}, + {"ok-opts-validator", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{userOptions, sshTestOptionsValidator("")}}, want{CertType: ssh.UserCert}, false}, + {"ok-opts-modifier", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{userOptions, sshTestOptionsModifier("")}}, want{CertType: ssh.UserCert}, false}, + {"fail-opts-type", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{CertType: "foo"}, []provisioner.SignOption{}}, want{}, true}, + {"fail-cert-validator", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{userOptions, sshTestCertValidator("an error")}}, want{}, true}, + {"fail-cert-modifier", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{userOptions, sshTestCertModifier("an error")}}, want{}, true}, + {"fail-opts-validator", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{userOptions, sshTestOptionsValidator("an error")}}, want{}, true}, + {"fail-opts-modifier", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{userOptions, sshTestOptionsModifier("an error")}}, want{}, true}, + {"fail-bad-sign-options", fields{signKey, signKey}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{userOptions, "wrong type"}}, want{}, true}, + {"fail-no-user-key", fields{nil, signKey}, args{pub, provisioner.SSHOptions{CertType: "user"}, []provisioner.SignOption{}}, want{}, true}, + {"fail-no-host-key", fields{signKey, nil}, args{pub, provisioner.SSHOptions{CertType: "host"}, []provisioner.SignOption{}}, want{}, true}, + {"fail-bad-type", fields{signKey, nil}, args{pub, provisioner.SSHOptions{}, []provisioner.SignOption{sshTestModifier{CertType: 0}}}, want{}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := testAuthority(t) + a.sshCAUserCertSignKey = tt.fields.sshCAUserCertSignKey + a.sshCAHostCertSignKey = tt.fields.sshCAHostCertSignKey + + got, err := a.SignSSH(tt.args.key, tt.args.opts, tt.args.signOpts...) + if (err != nil) != tt.wantErr { + t.Errorf("Authority.SignSSH() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err == nil && assert.NotNil(t, got) { + assert.Equals(t, tt.want.CertType, got.CertType) + assert.Equals(t, tt.want.Principals, got.ValidPrincipals) + assert.Equals(t, tt.want.ValidAfter, got.ValidAfter) + assert.Equals(t, tt.want.ValidBefore, got.ValidBefore) + assert.NotNil(t, got.Key) + assert.NotNil(t, got.Nonce) + assert.NotEquals(t, 0, got.Serial) + assert.NotNil(t, got.Signature) + assert.NotNil(t, got.SignatureKey) + } + }) + } +} + +func TestAuthority_SignSSHAddUser(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + assert.FatalError(t, err) + pub, err := ssh.NewPublicKey(key.Public()) + assert.FatalError(t, err) + signKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + assert.FatalError(t, err) + + type fields struct { + sshCAUserCertSignKey crypto.Signer + sshCAHostCertSignKey crypto.Signer + addUserPrincipal string + addUserCommand string + } + type args struct { + key ssh.PublicKey + subject *ssh.Certificate + } + type want struct { + CertType uint32 + Principals []string + ValidAfter uint64 + ValidBefore uint64 + ForceCommand string + } + + now := time.Now() + validCert := &ssh.Certificate{ + CertType: ssh.UserCert, + ValidPrincipals: []string{"user"}, + ValidAfter: uint64(now.Unix()), + ValidBefore: uint64(now.Add(time.Hour).Unix()), + } + validWant := want{ + CertType: ssh.UserCert, + Principals: []string{"provisioner"}, + ValidAfter: uint64(now.Unix()), + ValidBefore: uint64(now.Add(time.Hour).Unix()), + ForceCommand: "sudo useradd -m user; nc -q0 localhost 22", + } + + tests := []struct { + name string + fields fields + args args + want want + wantErr bool + }{ + {"ok", fields{signKey, signKey, "", ""}, args{pub, validCert}, validWant, false}, + {"ok-no-host-key", fields{signKey, nil, "", ""}, args{pub, validCert}, validWant, false}, + {"ok-custom-principal", fields{signKey, signKey, "my-principal", ""}, args{pub, &ssh.Certificate{CertType: ssh.UserCert, ValidPrincipals: []string{"user"}}}, want{CertType: ssh.UserCert, Principals: []string{"my-principal"}, ForceCommand: "sudo useradd -m user; nc -q0 localhost 22"}, false}, + {"ok-custom-command", fields{signKey, signKey, "", "foo "}, args{pub, &ssh.Certificate{CertType: ssh.UserCert, ValidPrincipals: []string{"user"}}}, want{CertType: ssh.UserCert, Principals: []string{"provisioner"}, ForceCommand: "foo user user"}, false}, + {"ok-custom-principal-and-command", fields{signKey, signKey, "my-principal", "foo "}, args{pub, &ssh.Certificate{CertType: ssh.UserCert, ValidPrincipals: []string{"user"}}}, want{CertType: ssh.UserCert, Principals: []string{"my-principal"}, ForceCommand: "foo user user"}, false}, + {"fail-no-user-key", fields{nil, signKey, "", ""}, args{pub, validCert}, want{}, true}, + {"fail-no-user-cert", fields{signKey, signKey, "", ""}, args{pub, &ssh.Certificate{CertType: ssh.HostCert, ValidPrincipals: []string{"foo"}}}, want{}, true}, + {"fail-no-principals", fields{signKey, signKey, "", ""}, args{pub, &ssh.Certificate{CertType: ssh.UserCert, ValidPrincipals: []string{}}}, want{}, true}, + {"fail-many-principals", fields{signKey, signKey, "", ""}, args{pub, &ssh.Certificate{CertType: ssh.UserCert, ValidPrincipals: []string{"foo", "bar"}}}, want{}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := testAuthority(t) + a.sshCAUserCertSignKey = tt.fields.sshCAUserCertSignKey + a.sshCAHostCertSignKey = tt.fields.sshCAHostCertSignKey + a.config.SSH = &SSHConfig{ + AddUserPrincipal: tt.fields.addUserPrincipal, + AddUserCommand: tt.fields.addUserCommand, + } + got, err := a.SignSSHAddUser(tt.args.key, tt.args.subject) + if (err != nil) != tt.wantErr { + t.Errorf("Authority.SignSSHAddUser() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err == nil && assert.NotNil(t, got) { + assert.Equals(t, tt.want.CertType, got.CertType) + assert.Equals(t, tt.want.Principals, got.ValidPrincipals) + assert.Equals(t, tt.args.subject.ValidPrincipals[0]+"-"+tt.want.Principals[0], got.KeyId) + assert.Equals(t, tt.want.ValidAfter, got.ValidAfter) + assert.Equals(t, tt.want.ValidBefore, got.ValidBefore) + assert.Equals(t, map[string]string{"force-command": tt.want.ForceCommand}, got.CriticalOptions) + assert.Equals(t, nil, got.Extensions) + assert.NotNil(t, got.Key) + assert.NotNil(t, got.Nonce) + assert.NotEquals(t, 0, got.Serial) + assert.NotNil(t, got.Signature) + assert.NotNil(t, got.SignatureKey) + } + }) + } +} From 61d52a8510bc69cd12e69a679123ae144536078d Mon Sep 17 00:00:00 2001 From: max furman Date: Sun, 8 Sep 2019 21:05:36 -0700 Subject: [PATCH 40/40] Small fixes associated with PR review * additions and grammar edits to documentation * clarification of error msgs --- api/api_test.go | 12 ++++----- api/ssh.go | 12 ++++----- api/ssh_test.go | 4 +-- authority/authorize.go | 5 +++- authority/provisioner/sign_ssh_options.go | 30 +++++++++++------------ authority/ssh.go | 2 +- 6 files changed, 34 insertions(+), 31 deletions(-) diff --git a/api/api_test.go b/api/api_test.go index 545aafc7..5ece5cc9 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -502,8 +502,8 @@ type mockAuthority struct { getTLSOptions func() *tlsutil.TLSOptions root func(shasum string) (*x509.Certificate, error) sign func(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) (*x509.Certificate, *x509.Certificate, error) - singSSH func(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) - singSSHAddUser func(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) + signSSH func(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) + signSSHAddUser func(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) renew func(cert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error) loadProvisionerByCertificate func(cert *x509.Certificate) (provisioner.Interface, error) getProvisioners func(nextCursor string, limit int) (provisioner.List, string, error) @@ -547,15 +547,15 @@ func (m *mockAuthority) Sign(cr *x509.CertificateRequest, opts provisioner.Optio } func (m *mockAuthority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) { - if m.singSSH != nil { - return m.singSSH(key, opts, signOpts...) + if m.signSSH != nil { + return m.signSSH(key, opts, signOpts...) } return m.ret1.(*ssh.Certificate), m.err } func (m *mockAuthority) SignSSHAddUser(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) { - if m.singSSHAddUser != nil { - return m.singSSHAddUser(key, cert) + if m.signSSHAddUser != nil { + return m.signSSHAddUser(key, cert) } return m.ret1.(*ssh.Certificate), m.err } diff --git a/api/ssh.go b/api/ssh.go index a847c9a0..7bcae7cf 100644 --- a/api/ssh.go +++ b/api/ssh.go @@ -39,8 +39,8 @@ type SSHCertificate struct { *ssh.Certificate `json:"omitempty"` } -// MarshalJSON implements the json.Marshaler interface. The certificate is -// quoted string using the PEM encoding. +// MarshalJSON implements the json.Marshaler interface. Returns a quoted, +// base64 encoded, openssh wire format version of the certificate. func (c SSHCertificate) MarshalJSON() ([]byte, error) { if c.Certificate == nil { return []byte("null"), nil @@ -50,7 +50,7 @@ func (c SSHCertificate) MarshalJSON() ([]byte, error) { } // UnmarshalJSON implements the json.Unmarshaler interface. The certificate is -// expected to be a quoted string using the PEM encoding. +// expected to be a quoted, base64 encoded, openssh wire formatted block of bytes. func (c *SSHCertificate) UnmarshalJSON(data []byte) error { var s string if err := json.Unmarshal(data, &s); err != nil { @@ -62,15 +62,15 @@ func (c *SSHCertificate) UnmarshalJSON(data []byte) error { } certData, err := base64.StdEncoding.DecodeString(s) if err != nil { - return errors.Wrap(err, "error decoding certificate") + return errors.Wrap(err, "error decoding ssh certificate") } pub, err := ssh.ParsePublicKey(certData) if err != nil { - return errors.Wrap(err, "error decoding certificate") + return errors.Wrap(err, "error parsing ssh certificate") } cert, ok := pub.(*ssh.Certificate) if !ok { - return errors.Errorf("error decoding certificate: %T is not an *ssh.Certificate", pub) + return errors.Errorf("error decoding ssh certificate: %T is not an *ssh.Certificate", pub) } c.Certificate = cert return nil diff --git a/api/ssh_test.go b/api/ssh_test.go index df3edf31..f37bcad8 100644 --- a/api/ssh_test.go +++ b/api/ssh_test.go @@ -295,10 +295,10 @@ func Test_caHandler_SignSSH(t *testing.T) { authorizeSign: func(ott string) ([]provisioner.SignOption, error) { return []provisioner.SignOption{}, tt.authErr }, - singSSH: func(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) { + signSSH: func(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) { return tt.signCert, tt.signErr }, - singSSHAddUser: func(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) { + signSSHAddUser: func(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) { return tt.addUserCert, tt.addUserErr }, }).(*caHandler) diff --git a/authority/authorize.go b/authority/authorize.go index e514681a..b8f7cb6f 100644 --- a/authority/authorize.go +++ b/authority/authorize.go @@ -93,7 +93,7 @@ func (a *Authority) Authorize(ctx context.Context, ott string) ([]provisioner.Si } // authorizeSign loads the provisioner from the token, checks that it has not -// been used again and calls the provisioner AuthorizeSign method. returns a +// been used again and calls the provisioner AuthorizeSign method. Returns a // list of methods to apply to the signing flow. func (a *Authority) authorizeSign(ctx context.Context, ott string) ([]provisioner.SignOption, error) { var errContext = apiCtx{"ott": ott} @@ -110,6 +110,9 @@ func (a *Authority) authorizeSign(ctx context.Context, ott string) ([]provisione // AuthorizeSign authorizes a signature request by validating and authenticating // a OTT that must be sent w/ the request. +// +// NOTE: This method is deprecated and should not be used. We make it available +// in the short term os as not to break existing clients. func (a *Authority) AuthorizeSign(ott string) ([]provisioner.SignOption, error) { ctx := provisioner.NewContextWithMethod(context.Background(), provisioner.SignMethod) return a.Authorize(ctx, ott) diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index bcf0c798..9ca2a95c 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -32,7 +32,7 @@ type SSHCertificateOptionModifier interface { // SSHCertificateValidator is the interface used to validate an SSH certificate. type SSHCertificateValidator interface { SignOption - Valid(crt *ssh.Certificate) error + Valid(cert *ssh.Certificate) error } // SSHCertificateOptionsValidator is the interface used to validate the custom @@ -169,7 +169,7 @@ type sshDefaultExtensionModifier struct{} func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error { switch cert.CertType { - // Default to no extensions to HostCert + // Default to no extensions for HostCert. case ssh.HostCert: return nil case ssh.UserCert: @@ -246,29 +246,29 @@ func (v sshCertificateOptionsValidator) Valid(got SSHOptions) error { type sshCertificateDefaultValidator struct{} // Valid returns an error if the given certificate does not contain the necessary fields. -func (v *sshCertificateDefaultValidator) Valid(crt *ssh.Certificate) error { +func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error { switch { - case len(crt.Nonce) == 0: + case len(cert.Nonce) == 0: return errors.New("ssh certificate nonce cannot be empty") - case crt.Key == nil: + case cert.Key == nil: return errors.New("ssh certificate key cannot be nil") - case crt.Serial == 0: + case cert.Serial == 0: return errors.New("ssh certificate serial cannot be 0") - case crt.CertType != ssh.UserCert && crt.CertType != ssh.HostCert: - return errors.Errorf("ssh certificate has an unknown type: %d", crt.CertType) - case crt.KeyId == "": + case cert.CertType != ssh.UserCert && cert.CertType != ssh.HostCert: + return errors.Errorf("ssh certificate has an unknown type: %d", cert.CertType) + case cert.KeyId == "": return errors.New("ssh certificate key id cannot be empty") - case len(crt.ValidPrincipals) == 0: + case len(cert.ValidPrincipals) == 0: return errors.New("ssh certificate valid principals cannot be empty") - case crt.ValidAfter == 0: + case cert.ValidAfter == 0: return errors.New("ssh certificate valid after cannot be 0") - case crt.ValidBefore == 0: + case cert.ValidBefore == 0: return errors.New("ssh certificate valid before cannot be 0") - case crt.CertType == ssh.UserCert && len(crt.Extensions) == 0: + case cert.CertType == ssh.UserCert && len(cert.Extensions) == 0: return errors.New("ssh certificate extensions cannot be empty") - case crt.SignatureKey == nil: + case cert.SignatureKey == nil: return errors.New("ssh certificate signature key cannot be nil") - case crt.Signature == nil: + case cert.Signature == nil: return errors.New("ssh certificate signature cannot be nil") default: return nil diff --git a/authority/ssh.go b/authority/ssh.go index 6dd37a67..2f69b3ca 100644 --- a/authority/ssh.go +++ b/authority/ssh.go @@ -150,7 +150,7 @@ func (a *Authority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, sign func (a *Authority) SignSSHAddUser(key ssh.PublicKey, subject *ssh.Certificate) (*ssh.Certificate, error) { if a.sshCAUserCertSignKey == nil { return nil, &apiError{ - err: errors.New("signSSHProxy: user certificate signing is not enabled"), + err: errors.New("signSSHAddUser: user certificate signing is not enabled"), code: http.StatusNotImplemented, } }