Use always badRequest on duration errors.

This commit is contained in:
Mariano Cano 2021-11-17 12:00:54 -08:00
parent 41fec1577d
commit 1aadd63cef
3 changed files with 5 additions and 16 deletions

View file

@ -383,17 +383,6 @@ func badRequest(format string, args ...interface{}) error {
} }
} }
// TODO(mariano): refactor errs package to allow sending real errors to the
// user.
func unauthorized(format string, args ...interface{}) error {
msg := fmt.Sprintf(format, args...)
return &errs.Error{
Status: http.StatusUnauthorized,
Msg: msg,
Err: errors.New(msg),
}
}
// Valid validates the certificate validity settings (notBefore/notAfter) and // Valid validates the certificate validity settings (notBefore/notAfter) and
// and total duration. // and total duration.
func (v *validityValidator) Valid(cert *x509.Certificate, o SignOptions) error { func (v *validityValidator) Valid(cert *x509.Certificate, o SignOptions) error {
@ -412,14 +401,14 @@ func (v *validityValidator) Valid(cert *x509.Certificate, o SignOptions) error {
return badRequest("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 { if d < v.min {
return unauthorized("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 // 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 // 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 // be backdated (e.g. if a user passes the NotBefore value then we do not
// apply a backdate). This is good enough. // apply a backdate). This is good enough.
if d > v.max+o.Backdate { if d > v.max+o.Backdate {
return unauthorized("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 return nil
} }

View file

@ -362,9 +362,9 @@ func (v *sshCertValidityValidator) Valid(cert *ssh.Certificate, opts SignSSHOpti
switch { switch {
case dur < min: case dur < min:
return unauthorized("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: case dur > max+opts.Backdate:
return unauthorized("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: default:
return nil return nil
} }

View file

@ -310,7 +310,7 @@ func TestAuthority_Sign(t *testing.T) {
extraOpts: extraOpts, extraOpts: extraOpts,
signOpts: _signOpts, signOpts: _signOpts,
err: errors.New("authority.Sign: requested duration of 25h0m0s is more than the authorized maximum certificate duration of 24h1m0s"), 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 { "fail validate sans when adding common name not in claims": func(t *testing.T) *signTest {