Merge pull request #750 from smallstep/duration-errors

Report duration errors directly to the cli.

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",
+		return badRequest("requested duration of %v is less than the authorized minimum certificate duration of %v", d, v.min)
-			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",
+		return badRequest("requested duration of %v is more than the authorized maximum certificate duration of %v", d, v.max+o.Backdate)
-			d, v.max+o.Backdate)
 	}
 	return nil
 }

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 "+
+		return badRequest("requested duration of %s is less than minimum accepted duration for selected provisioner of %s", dur, min)
-			"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 "+
+		return badRequest("requested duration of %s is greater than maximum accepted duration for selected provisioner of %s", dur, max+opts.Backdate)
-			"accepted duration for selected provisioner of %s", dur, max+opts.Backdate)
 	default:
 		return nil
 	}

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 {