From 1aadd63cefc5d9281eb1644dcb505a96073cd2b6 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 17 Nov 2021 12:00:54 -0800 Subject: [PATCH] Use always badRequest on duration errors. --- authority/provisioner/sign_options.go | 15 ++------------- authority/provisioner/sign_ssh_options.go | 4 ++-- authority/tls_test.go | 2 +- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go index 7268b63b..6b013fc8 100644 --- a/authority/provisioner/sign_options.go +++ b/authority/provisioner/sign_options.go @@ -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 // and total duration. 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) } 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 // 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 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 } diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go index e594c952..878d3d02 100644 --- a/authority/provisioner/sign_ssh_options.go +++ b/authority/provisioner/sign_ssh_options.go @@ -362,9 +362,9 @@ func (v *sshCertValidityValidator) Valid(cert *ssh.Certificate, opts SignSSHOpti switch { 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: - 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: return nil } diff --git a/authority/tls_test.go b/authority/tls_test.go index f1d1748d..9fb47f00 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 {