From 61d52a8510bc69cd12e69a679123ae144536078d Mon Sep 17 00:00:00 2001 From: max furman Date: Sun, 8 Sep 2019 21:05:36 -0700 Subject: [PATCH] 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, } }