diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go index 764916b6..95f7fc39 100644 --- a/authority/provisioner/sign_options.go +++ b/authority/provisioner/sign_options.go @@ -8,12 +8,15 @@ import ( "crypto/x509/pkix" "encoding/asn1" "encoding/json" + "fmt" "net" + "net/http" "net/url" "reflect" "time" "github.com/pkg/errors" + "github.com/smallstep/certificates/errs" "go.step.sm/crypto/x509util" ) @@ -369,8 +372,19 @@ func newValidityValidator(min, max time.Duration) *validityValidator { return &validityValidator{min: min, max: max} } +// TODO(mariano): refactor errs package to allow sending real errors to the +// user. +func badRequest(format string, args ...interface{}) error { + msg := fmt.Sprintf(format, args...) + return &errs.Error{ + Status: http.StatusBadRequest, + Msg: msg, + Err: errors.New(msg), + } +} + // Valid validates the certificate validity settings (notBefore/notAfter) and -// and total duration. +// total duration. func (v *validityValidator) Valid(cert *x509.Certificate, o SignOptions) error { var ( na = cert.NotAfter.Truncate(time.Second) @@ -381,22 +395,20 @@ func (v *validityValidator) Valid(cert *x509.Certificate, o SignOptions) error { d := na.Sub(nb) if na.Before(now) { - return errors.Errorf("notAfter cannot be in the past; na=%v", na) + return badRequest("notAfter cannot be in the past; na=%v", na) } if na.Before(nb) { - return errors.Errorf("notAfter cannot be before notBefore; na=%v, nb=%v", na, nb) + return badRequest("notAfter cannot be before notBefore; na=%v, nb=%v", na, nb) } if d < v.min { - return errors.Errorf("requested duration of %v is less than the authorized minimum certificate duration of %v", - d, v.min) + return badRequest("requested duration of %v is less than the authorized minimum certificate duration of %v", d, v.min) } // NOTE: this check is not "technically correct". We're allowing the max // duration of a cert to be "max + backdate" and not all certificates will // be backdated (e.g. if a user passes the NotBefore value then we do not // apply a backdate). This is good enough. if d > v.max+o.Backdate { - return errors.Errorf("requested duration of %v is more than the authorized maximum certificate duration of %v", - d, v.max+o.Backdate) + return badRequest("requested duration of %v is more than the authorized maximum certificate duration of %v", d, v.max+o.Backdate) } return nil } diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index 158470d1..878d3d02 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -335,11 +335,11 @@ type sshCertValidityValidator struct { func (v *sshCertValidityValidator) Valid(cert *ssh.Certificate, opts SignSSHOptions) error { switch { case cert.ValidAfter == 0: - return errors.New("ssh certificate validAfter cannot be 0") + return badRequest("ssh certificate validAfter cannot be 0") case cert.ValidBefore < uint64(now().Unix()): - return errors.New("ssh certificate validBefore cannot be in the past") + return badRequest("ssh certificate validBefore cannot be in the past") case cert.ValidBefore < cert.ValidAfter: - return errors.New("ssh certificate validBefore cannot be before validAfter") + return badRequest("ssh certificate validBefore cannot be before validAfter") } var min, max time.Duration @@ -351,9 +351,9 @@ func (v *sshCertValidityValidator) Valid(cert *ssh.Certificate, opts SignSSHOpti min = v.MinHostSSHCertDuration() max = v.MaxHostSSHCertDuration() case 0: - return errors.New("ssh certificate type has not been set") + return badRequest("ssh certificate type has not been set") default: - return errors.Errorf("unknown ssh certificate type %d", cert.CertType) + return badRequest("unknown ssh certificate type %d", cert.CertType) } // To not take into account the backdate, time.Now() will be used to @@ -362,11 +362,9 @@ func (v *sshCertValidityValidator) Valid(cert *ssh.Certificate, opts SignSSHOpti switch { case dur < min: - return errors.Errorf("requested duration of %s is less than minimum "+ - "accepted duration for selected provisioner of %s", dur, min) + return badRequest("requested duration of %s is less than minimum accepted duration for selected provisioner of %s", dur, min) case dur > max+opts.Backdate: - return errors.Errorf("requested duration of %s is greater than maximum "+ - "accepted duration for selected provisioner of %s", dur, max+opts.Backdate) + return badRequest("requested duration of %s is greater than maximum accepted duration for selected provisioner of %s", dur, max+opts.Backdate) default: return nil } diff --git a/authority/tls_test.go b/authority/tls_test.go index 158e6f4f..ba05b9fc 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -310,7 +310,7 @@ func TestAuthority_Sign(t *testing.T) { extraOpts: extraOpts, signOpts: _signOpts, err: errors.New("authority.Sign: requested duration of 25h0m0s is more than the authorized maximum certificate duration of 24h1m0s"), - code: http.StatusUnauthorized, + code: http.StatusBadRequest, } }, "fail validate sans when adding common name not in claims": func(t *testing.T) *signTest {