Small fixes associated with PR review

* additions and grammar edits to documentation
* clarification of error msgs
This commit is contained in:
max furman 2019-09-08 21:05:36 -07:00
parent 10e7b81b9f
commit 61d52a8510
6 changed files with 34 additions and 31 deletions

View file

@ -502,8 +502,8 @@ type mockAuthority struct {
getTLSOptions func() *tlsutil.TLSOptions getTLSOptions func() *tlsutil.TLSOptions
root func(shasum string) (*x509.Certificate, error) root func(shasum string) (*x509.Certificate, error)
sign func(cr *x509.CertificateRequest, opts provisioner.Options, signOpts ...provisioner.SignOption) (*x509.Certificate, *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) signSSH func(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error)
singSSHAddUser func(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) signSSHAddUser func(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error)
renew func(cert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error) renew func(cert *x509.Certificate) (*x509.Certificate, *x509.Certificate, error)
loadProvisionerByCertificate func(cert *x509.Certificate) (provisioner.Interface, error) loadProvisionerByCertificate func(cert *x509.Certificate) (provisioner.Interface, error)
getProvisioners func(nextCursor string, limit int) (provisioner.List, string, 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) { func (m *mockAuthority) SignSSH(key ssh.PublicKey, opts provisioner.SSHOptions, signOpts ...provisioner.SignOption) (*ssh.Certificate, error) {
if m.singSSH != nil { if m.signSSH != nil {
return m.singSSH(key, opts, signOpts...) return m.signSSH(key, opts, signOpts...)
} }
return m.ret1.(*ssh.Certificate), m.err return m.ret1.(*ssh.Certificate), m.err
} }
func (m *mockAuthority) SignSSHAddUser(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) { func (m *mockAuthority) SignSSHAddUser(key ssh.PublicKey, cert *ssh.Certificate) (*ssh.Certificate, error) {
if m.singSSHAddUser != nil { if m.signSSHAddUser != nil {
return m.singSSHAddUser(key, cert) return m.signSSHAddUser(key, cert)
} }
return m.ret1.(*ssh.Certificate), m.err return m.ret1.(*ssh.Certificate), m.err
} }

View file

@ -39,8 +39,8 @@ type SSHCertificate struct {
*ssh.Certificate `json:"omitempty"` *ssh.Certificate `json:"omitempty"`
} }
// MarshalJSON implements the json.Marshaler interface. The certificate is // MarshalJSON implements the json.Marshaler interface. Returns a quoted,
// quoted string using the PEM encoding. // base64 encoded, openssh wire format version of the certificate.
func (c SSHCertificate) MarshalJSON() ([]byte, error) { func (c SSHCertificate) MarshalJSON() ([]byte, error) {
if c.Certificate == nil { if c.Certificate == nil {
return []byte("null"), nil return []byte("null"), nil
@ -50,7 +50,7 @@ func (c SSHCertificate) MarshalJSON() ([]byte, error) {
} }
// UnmarshalJSON implements the json.Unmarshaler interface. The certificate is // 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 { func (c *SSHCertificate) UnmarshalJSON(data []byte) error {
var s string var s string
if err := json.Unmarshal(data, &s); err != nil { 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) certData, err := base64.StdEncoding.DecodeString(s)
if err != nil { if err != nil {
return errors.Wrap(err, "error decoding certificate") return errors.Wrap(err, "error decoding ssh certificate")
} }
pub, err := ssh.ParsePublicKey(certData) pub, err := ssh.ParsePublicKey(certData)
if err != nil { if err != nil {
return errors.Wrap(err, "error decoding certificate") return errors.Wrap(err, "error parsing ssh certificate")
} }
cert, ok := pub.(*ssh.Certificate) cert, ok := pub.(*ssh.Certificate)
if !ok { 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 c.Certificate = cert
return nil return nil

View file

@ -295,10 +295,10 @@ func Test_caHandler_SignSSH(t *testing.T) {
authorizeSign: func(ott string) ([]provisioner.SignOption, error) { authorizeSign: func(ott string) ([]provisioner.SignOption, error) {
return []provisioner.SignOption{}, tt.authErr 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 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 return tt.addUserCert, tt.addUserErr
}, },
}).(*caHandler) }).(*caHandler)

View file

@ -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 // 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. // list of methods to apply to the signing flow.
func (a *Authority) authorizeSign(ctx context.Context, ott string) ([]provisioner.SignOption, error) { func (a *Authority) authorizeSign(ctx context.Context, ott string) ([]provisioner.SignOption, error) {
var errContext = apiCtx{"ott": ott} 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 // AuthorizeSign authorizes a signature request by validating and authenticating
// a OTT that must be sent w/ the request. // 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) { func (a *Authority) AuthorizeSign(ott string) ([]provisioner.SignOption, error) {
ctx := provisioner.NewContextWithMethod(context.Background(), provisioner.SignMethod) ctx := provisioner.NewContextWithMethod(context.Background(), provisioner.SignMethod)
return a.Authorize(ctx, ott) return a.Authorize(ctx, ott)

View file

@ -32,7 +32,7 @@ type SSHCertificateOptionModifier interface {
// SSHCertificateValidator is the interface used to validate an SSH certificate. // SSHCertificateValidator is the interface used to validate an SSH certificate.
type SSHCertificateValidator interface { type SSHCertificateValidator interface {
SignOption SignOption
Valid(crt *ssh.Certificate) error Valid(cert *ssh.Certificate) error
} }
// SSHCertificateOptionsValidator is the interface used to validate the custom // SSHCertificateOptionsValidator is the interface used to validate the custom
@ -169,7 +169,7 @@ type sshDefaultExtensionModifier struct{}
func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error { func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error {
switch cert.CertType { switch cert.CertType {
// Default to no extensions to HostCert // Default to no extensions for HostCert.
case ssh.HostCert: case ssh.HostCert:
return nil return nil
case ssh.UserCert: case ssh.UserCert:
@ -246,29 +246,29 @@ func (v sshCertificateOptionsValidator) Valid(got SSHOptions) error {
type sshCertificateDefaultValidator struct{} type sshCertificateDefaultValidator struct{}
// Valid returns an error if the given certificate does not contain the necessary fields. // 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 { switch {
case len(crt.Nonce) == 0: case len(cert.Nonce) == 0:
return errors.New("ssh certificate nonce cannot be empty") 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") 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") return errors.New("ssh certificate serial cannot be 0")
case crt.CertType != ssh.UserCert && crt.CertType != ssh.HostCert: case cert.CertType != ssh.UserCert && cert.CertType != ssh.HostCert:
return errors.Errorf("ssh certificate has an unknown type: %d", crt.CertType) return errors.Errorf("ssh certificate has an unknown type: %d", cert.CertType)
case crt.KeyId == "": case cert.KeyId == "":
return errors.New("ssh certificate key id cannot be empty") 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") 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") 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") 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") 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") 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") return errors.New("ssh certificate signature cannot be nil")
default: default:
return nil return nil

View file

@ -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) { func (a *Authority) SignSSHAddUser(key ssh.PublicKey, subject *ssh.Certificate) (*ssh.Certificate, error) {
if a.sshCAUserCertSignKey == nil { if a.sshCAUserCertSignKey == nil {
return nil, &apiError{ 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, code: http.StatusNotImplemented,
} }
} }