From 869e508d081ff870e218991f53d363e10963622a Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Wed, 4 Sep 2019 18:31:09 -0700
Subject: [PATCH 01/21] wip

---
 authority/authorize.go               |   2 +-
 authority/provisioner/collection.go  |   1 +
 authority/provisioner/provisioner.go |   6 +
 authority/provisioner/x5c.go         | 165 +++++++++++++++++++++++++++
 4 files changed, 173 insertions(+), 1 deletion(-)
 create mode 100644 authority/provisioner/x5c.go

diff --git a/authority/authorize.go b/authority/authorize.go
index b8f7cb6ff..7f9d16a6c 100644
--- a/authority/authorize.go
+++ b/authority/authorize.go
@@ -103,7 +103,7 @@ func (a *Authority) authorizeSign(ctx context.Context, ott string) ([]provisione
 	}
 	opts, err := p.AuthorizeSign(ctx, ott)
 	if err != nil {
-		return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext}
+		return nil, &apiError{errors.Wrap(err, "auth.authorizeSign"), http.StatusUnauthorized, errContext}
 	}
 	return opts, nil
 }
diff --git a/authority/provisioner/collection.go b/authority/provisioner/collection.go
index 906c5260e..990764410 100644
--- a/authority/provisioner/collection.go
+++ b/authority/provisioner/collection.go
@@ -83,6 +83,7 @@ func (c *Collection) LoadByToken(token *jose.JSONWebToken, claims *jose.Claims)
 		}
 		// If matches with stored audiences it will be a JWT token (default), and
 		// the id would be <issuer>:<kid>.
+		fmt.Printf("token.Headers[0].ExtraHeaders = %+v\n", token.Headers[0].ExtraHeaders)
 		return c.Load(claims.Issuer + ":" + token.Headers[0].KeyID)
 	}
 
diff --git a/authority/provisioner/provisioner.go b/authority/provisioner/provisioner.go
index 2a63161ca..155e34dec 100644
--- a/authority/provisioner/provisioner.go
+++ b/authority/provisioner/provisioner.go
@@ -86,6 +86,8 @@ const (
 	TypeAzure Type = 5
 	// TypeACME is used to indicate the ACME provisioners.
 	TypeACME Type = 6
+	// TypeX5C is used to indicate the X5C provisioners.
+	TypeX5C Type = 7
 
 	// RevokeAudienceKey is the key for the 'revoke' audiences in the audiences map.
 	RevokeAudienceKey = "revoke"
@@ -108,6 +110,8 @@ func (t Type) String() string {
 		return "Azure"
 	case TypeACME:
 		return "ACME"
+	case TypeX5C:
+		return "X5C"
 	default:
 		return ""
 	}
@@ -157,6 +161,8 @@ func (l *List) UnmarshalJSON(data []byte) error {
 			p = &Azure{}
 		case "acme":
 			p = &ACME{}
+		case "x5c":
+			p = &X5C{}
 		default:
 			// Skip unsupported provisioners. A client using this method may be
 			// compiled with a version of smallstep/certificates that does not
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
new file mode 100644
index 000000000..23437e0e1
--- /dev/null
+++ b/authority/provisioner/x5c.go
@@ -0,0 +1,165 @@
+package provisioner
+
+import (
+	"crypto/x509"
+	"time"
+
+	"github.com/pkg/errors"
+	"github.com/smallstep/cli/crypto/x509util"
+	"github.com/smallstep/cli/jose"
+)
+
+// X5C is the default provisioner, an entity that can sign tokens necessary for
+// signature requests.
+type X5C struct {
+	Type      string `json:"type"`
+	Name      string `json:"name"`
+	Roots     string `json:"roots"`
+	rootPool  *x509.CertPool
+	Claims    *Claims `json:"claims,omitempty"`
+	claimer   *Claimer
+	audiences Audiences
+}
+
+// GetID returns the provisioner unique identifier. The name and credential id
+// should uniquely identify any X5C provisioner.
+func (p *X5C) GetID() string {
+	return p.Name
+}
+
+// GetTokenID returns the identifier of the token.
+func (p *X5C) GetTokenID(ott string) (string, error) {
+	// Validate payload
+	token, err := jose.ParseSigned(ott)
+	if err != nil {
+		return "", errors.Wrap(err, "error parsing token")
+	}
+
+	// Get claims w/out verification. We need to look up the provisioner
+	// key in order to verify the claims and we need the issuer from the claims
+	// before we can look up the provisioner.
+	var claims jose.Claims
+	if err = token.UnsafeClaimsWithoutVerification(&claims); err != nil {
+		return "", errors.Wrap(err, "error verifying claims")
+	}
+	return claims.ID, nil
+}
+
+// GetName returns the name of the provisioner.
+func (p *X5C) GetName() string {
+	return p.Name
+}
+
+// GetType returns the type of provisioner.
+func (p *X5C) GetType() Type {
+	return TypeX5C
+}
+
+// GetEncryptedKey returns the base provisioner encrypted key if it's defined.
+func (p *X5C) GetEncryptedKey() (string, string, bool) {
+	return "", "", false
+}
+
+// Init initializes and validates the fields of a X5C type.
+func (p *X5C) Init(config Config) (err error) {
+	switch {
+	case p.Type == "":
+		return errors.New("provisioner type cannot be empty")
+	case p.Name == "":
+		return errors.New("provisioner name cannot be empty")
+	case p.Roots == "":
+		return errors.New("provisioner root(s) cannot be empty")
+	}
+
+	p.rootPool = x509.NewCertPool()
+	if len(p.Roots) > 0 && !p.rootPool.AppendCertsFromPEM([]byte(p.Roots)) {
+		return errors.Errorf("error parsing root certificate(s) for provisioner '%s'", p.Name)
+	}
+
+	// Update claims with global ones
+	if p.claimer, err = NewClaimer(p.Claims, config.Claims); err != nil {
+		return err
+	}
+
+	p.audiences = config.Audiences
+	return err
+}
+
+// authorizeToken performs common jwt authorization actions and returns the
+// claims for case specific downstream parsing.
+// e.g. a Sign request will auth/validate different fields than a Revoke request.
+func (p *X5C) authorizeToken(token string, audiences []string) (*jwtPayload, error) {
+	jwt, err := jose.ParseSigned(token)
+	if err != nil {
+		return nil, errors.Wrapf(err, "error parsing token")
+	}
+
+	verifiedChains, err := jwt.Headers[0].Certificates(x509.VerifyOptions{
+		Roots: p.rootPool,
+	})
+	if err != nil {
+		return nil, errors.Wrap(err, "error verifying x5c certificate chain")
+	}
+	leaf := verifiedChains[0][0]
+
+	var claims jwtPayload
+	if err = jwt.Claims(leaf.PublicKey, &claims); err != nil {
+		return nil, errors.Wrap(err, "error parsing claims")
+	}
+
+	// According to "rfc7519 JSON Web Token" acceptable skew should be no
+	// more than a few minutes.
+	if err = claims.ValidateWithLeeway(jose.Expected{
+		Issuer: p.Name,
+		Time:   time.Now().UTC(),
+	}, time.Minute); err != nil {
+		return nil, errors.Wrapf(err, "invalid token")
+	}
+
+	if claims.Subject == "" {
+		return nil, errors.New("token subject cannot be empty")
+	}
+
+	return &claims, nil
+}
+
+// AuthorizeRevoke returns an error if the provisioner does not have rights to
+// revoke the certificate with serial number in the `sub` property.
+func (p *X5C) AuthorizeRevoke(token string) error {
+	_, err := p.authorizeToken(token, p.audiences.Revoke)
+	return err
+}
+
+// AuthorizeSign validates the given token.
+func (p *X5C) AuthorizeSign(token string) ([]SignOption, error) {
+	claims, err := p.authorizeToken(token, p.audiences.Sign)
+	if err != nil {
+		return nil, err
+	}
+	// NOTE: This is for backwards compatibility with older versions of cli
+	// and certificates. Older versions added the token subject as the only SAN
+	// in a CSR by default.
+	if len(claims.SANs) == 0 {
+		claims.SANs = []string{claims.Subject}
+	}
+
+	dnsNames, ips, emails := x509util.SplitSANs(claims.SANs)
+	return []SignOption{
+		defaultPublicKeyValidator{},
+		commonNameValidator(claims.Subject),
+		dnsNamesValidator(dnsNames),
+		ipAddressesValidator(ips),
+		emailAddressesValidator(emails),
+		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
+		newProvisionerExtensionOption(TypeX5C, p.Name, ""),
+		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+	}, nil
+}
+
+// AuthorizeRenewal returns an error if the renewal is disabled.
+func (p *X5C) AuthorizeRenewal(cert *x509.Certificate) error {
+	if p.claimer.IsDisableRenewal() {
+		return errors.Errorf("renew is disabled for provisioner %s", p.GetID())
+	}
+	return nil
+}

From bcaf552e378770b86fc436b16090dacca9f630ba Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Wed, 18 Sep 2019 16:47:46 -0700
Subject: [PATCH 02/21] wip

---
 authority/provisioner/x5c.go | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index 23437e0e1..982ca0af5 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -1,6 +1,7 @@
 package provisioner
 
 import (
+	"context"
 	"crypto/x509"
 	"time"
 
@@ -102,6 +103,10 @@ func (p *X5C) authorizeToken(token string, audiences []string) (*jwtPayload, err
 	}
 	leaf := verifiedChains[0][0]
 
+	if leaf.KeyUsage&x509.KeyUsageDigitalSignature == 0 {
+		return nil, errors.New("certificate used to sign x5c token cannot be used for digital signature")
+	}
+
 	var claims jwtPayload
 	if err = jwt.Claims(leaf.PublicKey, &claims); err != nil {
 		return nil, errors.Wrap(err, "error parsing claims")
@@ -131,7 +136,7 @@ func (p *X5C) AuthorizeRevoke(token string) error {
 }
 
 // AuthorizeSign validates the given token.
-func (p *X5C) AuthorizeSign(token string) ([]SignOption, error) {
+func (p *X5C) AuthorizeSign(ctx context.Context, token string) ([]SignOption, error) {
 	claims, err := p.authorizeToken(token, p.audiences.Sign)
 	if err != nil {
 		return nil, err

From 2456a978e18e2bbb92e612fe9dd51c33b0c2e4e2 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Wed, 18 Sep 2019 19:23:03 -0700
Subject: [PATCH 03/21] wip

---
 Gopkg.toml                          | 2 +-
 authority/provisioner/collection.go | 3 ++-
 authority/provisioner/x5c.go        | 7 ++++++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/Gopkg.toml b/Gopkg.toml
index 8937374bf..6034dcd12 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -32,7 +32,7 @@
   name = "github.com/go-chi/chi"
 
 [[override]]
-  branch = "master"
+  branch = "x5c"
   name = "github.com/smallstep/cli"
 
 [[constraint]]
diff --git a/authority/provisioner/collection.go b/authority/provisioner/collection.go
index 990764410..0d14a65b1 100644
--- a/authority/provisioner/collection.go
+++ b/authority/provisioner/collection.go
@@ -83,7 +83,6 @@ func (c *Collection) LoadByToken(token *jose.JSONWebToken, claims *jose.Claims)
 		}
 		// If matches with stored audiences it will be a JWT token (default), and
 		// the id would be <issuer>:<kid>.
-		fmt.Printf("token.Headers[0].ExtraHeaders = %+v\n", token.Headers[0].ExtraHeaders)
 		return c.Load(claims.Issuer + ":" + token.Headers[0].KeyID)
 	}
 
@@ -130,6 +129,8 @@ func (c *Collection) LoadByCertificate(cert *x509.Certificate) (Interface, bool)
 				return c.Load("gcp/" + string(provisioner.Name))
 			case TypeACME:
 				return c.Load("acme/" + string(provisioner.Name))
+			case TypeX5C:
+				return c.Load("x5c/" + string(provisioner.Name))
 			default:
 				return c.Load(string(provisioner.CredentialID))
 			}
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index 982ca0af5..b387587d1 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -25,7 +25,7 @@ type X5C struct {
 // GetID returns the provisioner unique identifier. The name and credential id
 // should uniquely identify any X5C provisioner.
 func (p *X5C) GetID() string {
-	return p.Name
+	return "x5c/" + p.Name
 }
 
 // GetTokenID returns the identifier of the token.
@@ -107,6 +107,11 @@ func (p *X5C) authorizeToken(token string, audiences []string) (*jwtPayload, err
 		return nil, errors.New("certificate used to sign x5c token cannot be used for digital signature")
 	}
 
+	// Using the leaf certificates key to validate the claims accomplishes two
+	// things:
+	//   1. Asserts that the private key used to sign the token corresponds
+	//      to the public certificate in the `x5c` header of the token.
+	//   2. Asserts that the claims are valid - have not been tampered with.
 	var claims jwtPayload
 	if err = jwt.Claims(leaf.PublicKey, &claims); err != nil {
 		return nil, errors.Wrap(err, "error parsing claims")

From 2096ff875bb55ab7bfbb2f31107c3c97a9bc60fa Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Tue, 24 Sep 2019 12:49:32 -0700
Subject: [PATCH 04/21] wip

---
 authority/provisioner/aws.go              |  2 +-
 authority/provisioner/azure.go            |  2 +-
 authority/provisioner/gcp.go              |  2 +-
 authority/provisioner/jwk.go              |  2 +-
 authority/provisioner/oidc.go             |  2 +-
 authority/provisioner/sign_options.go     |  6 +-
 authority/provisioner/sign_ssh_options.go | 35 +++++++-
 authority/provisioner/x5c.go              | 97 ++++++++++++++++++++++-
 8 files changed, 137 insertions(+), 11 deletions(-)

diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go
index c6360dbd8..b3a5013e2 100644
--- a/authority/provisioner/aws.go
+++ b/authority/provisioner/aws.go
@@ -469,7 +469,7 @@ func (p *AWS) authorizeSSHSign(claims *awsPayload) ([]SignOption, error) {
 		// set the default extensions
 		&sshDefaultExtensionModifier{},
 		// checks the validity bounds, and set the validity if has not been set
-		&sshCertificateValidityModifier{p.claimer},
+		&sshCertificateValidityModifier{p.claimer, 0},
 		// validate public key
 		&sshDefaultPublicKeyValidator{},
 		// require all the fields in the SSH certificate
diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go
index e5e858dde..d4116c49a 100644
--- a/authority/provisioner/azure.go
+++ b/authority/provisioner/azure.go
@@ -326,7 +326,7 @@ func (p *Azure) authorizeSSHSign(claims azurePayload, name string) ([]SignOption
 		// set the default extensions
 		&sshDefaultExtensionModifier{},
 		// checks the validity bounds, and set the validity if has not been set
-		&sshCertificateValidityModifier{p.claimer},
+		&sshCertificateValidityModifier{p.claimer, 0},
 		// validate public key
 		&sshDefaultPublicKeyValidator{},
 		// require all the fields in the SSH certificate
diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go
index 63492fd4b..466cfc587 100644
--- a/authority/provisioner/gcp.go
+++ b/authority/provisioner/gcp.go
@@ -381,7 +381,7 @@ func (p *GCP) authorizeSSHSign(claims *gcpPayload) ([]SignOption, error) {
 		// set the default extensions
 		&sshDefaultExtensionModifier{},
 		// checks the validity bounds, and set the validity if has not been set
-		&sshCertificateValidityModifier{p.claimer},
+		&sshCertificateValidityModifier{p.claimer, 0},
 		// validate public key
 		&sshDefaultPublicKeyValidator{},
 		// require all the fields in the SSH certificate
diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go
index cdc90ff63..a4569d0f3 100644
--- a/authority/provisioner/jwk.go
+++ b/authority/provisioner/jwk.go
@@ -209,7 +209,7 @@ func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) {
 		// set the default extensions
 		&sshDefaultExtensionModifier{},
 		// checks the validity bounds, and set the validity if has not been set
-		&sshCertificateValidityModifier{p.claimer},
+		&sshCertificateValidityModifier{p.claimer, 0},
 		// validate public key
 		&sshDefaultPublicKeyValidator{},
 		// require all the fields in the SSH certificate
diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go
index d49374706..242b027ff 100644
--- a/authority/provisioner/oidc.go
+++ b/authority/provisioner/oidc.go
@@ -353,7 +353,7 @@ func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) {
 		// set the default extensions
 		&sshDefaultExtensionModifier{},
 		// checks the validity bounds, and set the validity if has not been set
-		&sshCertificateValidityModifier{o.claimer},
+		&sshCertificateValidityModifier{o.claimer, 0},
 		// validate public key
 		&sshDefaultPublicKeyValidator{},
 		// require all the fields in the SSH certificate
diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go
index d535dfb65..a15aca254 100644
--- a/authority/provisioner/sign_options.go
+++ b/authority/provisioner/sign_options.go
@@ -53,8 +53,12 @@ func (v profileWithOption) Option(Options) x509util.WithOption {
 	return x509util.WithOption(v)
 }
 
-// profileDefaultDuration is a wrapper against x509util.WithOption to conform the
+// profileDefaultDuration sets the NotBefore and NotAfter attributes on an
+// X509 certificate (profile) from values parsed along with the request.
+// This method is a wrapper against x509util.WithOption to conform the
 // interface.
+// NOTE: This method sets values, but does not validate them;
+// validation is done in the ValidityValidator.
 type profileDefaultDuration time.Duration
 
 func (v profileDefaultDuration) Option(so Options) x509util.WithOption {
diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go
index 94a77c50c..3048a1432 100644
--- a/authority/provisioner/sign_ssh_options.go
+++ b/authority/provisioner/sign_ssh_options.go
@@ -196,10 +196,20 @@ func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error {
 // CertType has not been set or is not valid.
 type sshCertificateValidityModifier struct {
 	*Claimer
+	// RemainingProvisioningCredentialDuraion is the remaining duration on the
+	// provisioning credential.
+	// E.g. x5c provisioners use a certificate as a provisioning credential.
+	// That certificate should not be able to provision new certificates with
+	// a duration longer than the remaining duration on the provisioning
+	// certificate.
+	RemainingProvisioningCredentialDuraion time.Duration
 }
 
 func (m *sshCertificateValidityModifier) Modify(cert *ssh.Certificate) error {
-	var d, min, max time.Duration
+	var (
+		d, min, max time.Duration
+		rem         = m.RemainingProvisioningCredentialDuraion
+	)
 	switch cert.CertType {
 	case ssh.UserCert:
 		d = m.DefaultUserSSHCertDuration()
@@ -215,6 +225,29 @@ func (m *sshCertificateValidityModifier) Modify(cert *ssh.Certificate) error {
 		return errors.Errorf("unknown ssh certificate type %d", cert.CertType)
 	}
 
+	// Use the remaining duration from the provisioning duration to set bounds
+	// and values if it is supplied.
+	if rem > 0 {
+		// If the remaining duration is less than the min duration for the requested
+		// type of SSH certificate then return an error.
+		if rem < min {
+			return errors.New("remaining duration on X5C certificate in the token " +
+				"is less than the minimum SSH duration on the X5C provisioner")
+		}
+		// If the remaining duration from the provisioning credential is less than
+		// the max duration for the requested type of SSH certificate then we
+		// reset our max bound.
+		if rem < max {
+			max = rem
+		}
+		// If the remaining duration from the provisioning credential is less than
+		// the default duration for the requested type of SSH certificate then we
+		// reset our default duration.
+		if rem < d {
+			d = rem
+		}
+	}
+
 	if cert.ValidAfter == 0 {
 		cert.ValidAfter = uint64(now().Truncate(time.Second).Unix())
 	}
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index b387587d1..a27aca3b4 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -10,6 +10,14 @@ import (
 	"github.com/smallstep/cli/jose"
 )
 
+// x5cPayload extends jwt.Claims with step attributes.
+type x5cPayload struct {
+	jose.Claims
+	SANs   []string     `json:"sans,omitempty"`
+	Step   *stepPayload `json:"step,omitempty"`
+	chains [][]*x509.Certificate
+}
+
 // X5C is the default provisioner, an entity that can sign tokens necessary for
 // signature requests.
 type X5C struct {
@@ -89,7 +97,7 @@ func (p *X5C) Init(config Config) (err error) {
 // authorizeToken performs common jwt authorization actions and returns the
 // claims for case specific downstream parsing.
 // e.g. a Sign request will auth/validate different fields than a Revoke request.
-func (p *X5C) authorizeToken(token string, audiences []string) (*jwtPayload, error) {
+func (p *X5C) authorizeToken(token string, audiences []string) (*x5cPayload, error) {
 	jwt, err := jose.ParseSigned(token)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error parsing token")
@@ -112,7 +120,7 @@ func (p *X5C) authorizeToken(token string, audiences []string) (*jwtPayload, err
 	//   1. Asserts that the private key used to sign the token corresponds
 	//      to the public certificate in the `x5c` header of the token.
 	//   2. Asserts that the claims are valid - have not been tampered with.
-	var claims jwtPayload
+	var claims x5cPayload
 	if err = jwt.Claims(leaf.PublicKey, &claims); err != nil {
 		return nil, errors.Wrap(err, "error parsing claims")
 	}
@@ -130,6 +138,8 @@ func (p *X5C) authorizeToken(token string, audiences []string) (*jwtPayload, err
 		return nil, errors.New("token subject cannot be empty")
 	}
 
+	// Save the verified chains on the x5c payload object.
+	claims.chains = verifiedChains
 	return &claims, nil
 }
 
@@ -146,6 +156,15 @@ func (p *X5C) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	if err != nil {
 		return nil, err
 	}
+
+	// Check for SSH token
+	if claims.Step != nil && claims.Step.SSH != nil {
+		if p.claimer.IsSSHCAEnabled() == false {
+			return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID())
+		}
+		return p.authorizeSSHSign(claims)
+	}
+
 	// NOTE: This is for backwards compatibility with older versions of cli
 	// and certificates. Older versions added the token subject as the only SAN
 	// in a CSR by default.
@@ -154,15 +173,43 @@ func (p *X5C) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	}
 
 	dnsNames, ips, emails := x509util.SplitSANs(claims.SANs)
+
+	// Get the remaining duration from the provisioning cert. The duration
+	// on the new certificate cannot be greater than the remaining duration of
+	// the provisioning certificate.
+	rem := claims.chains[0][0].NotAfter.Sub(time.Now())
+	minD, maxD, defaultD := p.claimer.MinTLSCertDuration(),
+		p.claimer.MaxTLSCertDuration(), p.claimer.DefaultTLSCertDuration()
+	if rem > 0 {
+		// If the remaining duration from the provisioning x5c Certificate is less
+		// than the provisioner's minimum default duration, then return an error.
+		if rem < minD {
+			return nil, errors.New("remaining duration on x5c certificate in the token " +
+				"is less than the minimum duration on the x5c provisioner")
+		}
+		// If the remaining duration from the provisioning credential is less than
+		// the max duration for the requested type of x509 certificate then we
+		// reset our max bound.
+		if rem < maxD {
+			maxD = rem
+		}
+		// If the remaining duration from the provisioning credential is less than
+		// the default duration for the requested type of x509 certificate then we
+		// reset our default duration.
+		if rem < defaultD {
+			defaultD = rem
+		}
+	}
+
 	return []SignOption{
 		defaultPublicKeyValidator{},
 		commonNameValidator(claims.Subject),
 		dnsNamesValidator(dnsNames),
 		ipAddressesValidator(ips),
 		emailAddressesValidator(emails),
-		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
+		profileDefaultDuration(defaultD),
 		newProvisionerExtensionOption(TypeX5C, p.Name, ""),
-		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		newValidityValidator(minD, maxD),
 	}, nil
 }
 
@@ -173,3 +220,45 @@ func (p *X5C) AuthorizeRenewal(cert *x509.Certificate) error {
 	}
 	return nil
 }
+
+// authorizeSSHSign returns the list of SignOption for a SignSSH request.
+func (p *X5C) authorizeSSHSign(claims *x5cPayload) ([]SignOption, error) {
+	t := now()
+	opts := claims.Step.SSH
+	signOptions := []SignOption{
+		// validates user's SSHOptions with the ones in the token
+		sshCertificateOptionsValidator(*opts),
+		// set the key id to the token subject
+		sshCertificateKeyIDModifier(claims.Subject),
+	}
+
+	// Add modifiers from custom claims
+	if opts.CertType != "" {
+		signOptions = append(signOptions, sshCertificateCertTypeModifier(opts.CertType))
+	}
+	if len(opts.Principals) > 0 {
+		signOptions = append(signOptions, sshCertificatePrincipalsModifier(opts.Principals))
+	}
+	if !opts.ValidAfter.IsZero() {
+		signOptions = append(signOptions, sshCertificateValidAfterModifier(opts.ValidAfter.RelativeTime(t).Unix()))
+	}
+	if !opts.ValidBefore.IsZero() {
+		signOptions = append(signOptions, sshCertificateValidBeforeModifier(opts.ValidBefore.RelativeTime(t).Unix()))
+	}
+
+	// Default to a user certificate with no principals if not set
+	signOptions = append(signOptions, sshCertificateDefaultsModifier{CertType: SSHUserCert})
+
+	rem := claims.chains[0][0].NotAfter.Sub(time.Now())
+
+	return append(signOptions,
+		// set the default extensions
+		&sshDefaultExtensionModifier{},
+		// checks the validity bounds, and set the validity if has not been set
+		&sshCertificateValidityModifier{p.claimer, rem},
+		// validate public key
+		&sshDefaultPublicKeyValidator{},
+		// require all the fields in the SSH certificate
+		&sshCertificateDefaultValidator{},
+	), nil
+}

From eced4a6c7ab01e3b859608ba23989d78358587cc Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Thu, 26 Sep 2019 16:00:07 -0700
Subject: [PATCH 05/21] wip

---
 .golangci.yml                             |   1 +
 Gopkg.toml                                |   3 -
 authority/authorize.go                    |   2 +-
 authority/provisioner/acme.go             |   7 +-
 authority/provisioner/aws.go              |  20 ++--
 authority/provisioner/azure.go            |  24 +++--
 authority/provisioner/gcp.go              |  24 +++--
 authority/provisioner/jwk.go              |  31 +++---
 authority/provisioner/oidc.go             |  31 +++---
 authority/provisioner/sign_options.go     | 109 ++++++++++++++++------
 authority/provisioner/sign_ssh_options.go |  73 +++++++++++----
 authority/provisioner/x5c.go              |  74 +++++++--------
 12 files changed, 249 insertions(+), 150 deletions(-)

diff --git a/.golangci.yml b/.golangci.yml
index 4defd73f1..2028654ad 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -51,6 +51,7 @@ linters:
     - deadcode
     - staticcheck
     - unused
+    - gosimple
 
 run:
   skip-dirs:
diff --git a/Gopkg.toml b/Gopkg.toml
index 6034dcd12..8e12c5e1b 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -23,9 +23,6 @@
 #   non-go = false
 #   go-tests = true
 #   unused-packages = true
-[[override]]
-  name = "gopkg.in/alecthomas/kingpin.v3-unstable"
-  revision = "63abe20a23e29e80bbef8089bd3dee3ac25e5306"
 
 [[constraint]]
   branch = "master"
diff --git a/authority/authorize.go b/authority/authorize.go
index 7f9d16a6c..b8f7cb6ff 100644
--- a/authority/authorize.go
+++ b/authority/authorize.go
@@ -103,7 +103,7 @@ func (a *Authority) authorizeSign(ctx context.Context, ott string) ([]provisione
 	}
 	opts, err := p.AuthorizeSign(ctx, ott)
 	if err != nil {
-		return nil, &apiError{errors.Wrap(err, "auth.authorizeSign"), http.StatusUnauthorized, errContext}
+		return nil, &apiError{errors.Wrap(err, "authorizeSign"), http.StatusUnauthorized, errContext}
 	}
 	return opts, nil
 }
diff --git a/authority/provisioner/acme.go b/authority/provisioner/acme.go
index 2d78e5a22..1b1c40113 100644
--- a/authority/provisioner/acme.go
+++ b/authority/provisioner/acme.go
@@ -69,10 +69,13 @@ func (p *ACME) AuthorizeSign(ctx context.Context, _ string) ([]SignOption, error
 		return nil, errors.Errorf("unexpected method type %d in context", m)
 	}
 	return []SignOption{
-		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
+		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeACME, p.Name, ""),
-		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		x509ProfileValidityModifier{p.claimer, 0},
+		// validators
 		defaultPublicKeyValidator{},
+		validityValidator{},
+		x509CertificateDurationValidator{p.claimer, 0},
 	}, nil
 }
 
diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go
index b3a5013e2..31bf2108a 100644
--- a/authority/provisioner/aws.go
+++ b/authority/provisioner/aws.go
@@ -296,11 +296,14 @@ func (p *AWS) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	}
 
 	return append(so,
+		// modifiers / withOptions
+		newProvisionerExtensionOption(TypeAWS, p.Name, doc.AccountID, "InstanceID", doc.InstanceID),
+		x509ProfileValidityModifier{p.claimer, 0},
+		// validators
 		defaultPublicKeyValidator{},
 		commonNameValidator(payload.Claims.Subject),
-		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
-		newProvisionerExtensionOption(TypeAWS, p.Name, doc.AccountID, "InstanceID", doc.InstanceID),
-		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		validityValidator{},
+		x509CertificateDurationValidator{p.claimer, 0},
 	), nil
 }
 
@@ -466,13 +469,16 @@ func (p *AWS) authorizeSSHSign(claims *awsPayload) ([]SignOption, error) {
 	signOptions = append(signOptions, sshCertificateDefaultsModifier(defaults))
 
 	return append(signOptions,
-		// set the default extensions
+		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
-		// checks the validity bounds, and set the validity if has not been set
+		// Checks the validity bounds, and set the validity if has not been set.
 		&sshCertificateValidityModifier{p.claimer, 0},
-		// validate public key
+		// Check the validity bounds against default provisioner and
+		// provisioning credential bounds.
+		&sshCertificateDurationValidator{p.claimer, 0},
+		// Validate public key.
 		&sshDefaultPublicKeyValidator{},
-		// require all the fields in the SSH certificate
+		// Require all the fields in the SSH certificate.
 		&sshCertificateDefaultValidator{},
 	), nil
 }
diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go
index d4116c49a..86a4d6d87 100644
--- a/authority/provisioner/azure.go
+++ b/authority/provisioner/azure.go
@@ -266,8 +266,8 @@ func (p *Azure) AuthorizeSign(ctx context.Context, token string) ([]SignOption,
 	}
 
 	// Check for the sign ssh method, default to sign X.509
-	if m := MethodFromContext(ctx); m == SignSSHMethod {
-		if p.claimer.IsSSHCAEnabled() == false {
+	if MethodFromContext(ctx) == SignSSHMethod {
+		if !p.claimer.IsSSHCAEnabled() {
 			return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID())
 		}
 		return p.authorizeSSHSign(claims, name)
@@ -284,10 +284,13 @@ func (p *Azure) AuthorizeSign(ctx context.Context, token string) ([]SignOption,
 	}
 
 	return append(so,
-		defaultPublicKeyValidator{},
-		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
+		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeAzure, p.Name, p.TenantID),
-		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		x509ProfileValidityModifier{p.claimer, 0},
+		// validators
+		defaultPublicKeyValidator{},
+		validityValidator{},
+		x509CertificateDurationValidator{p.claimer, 0},
 	), nil
 }
 
@@ -323,13 +326,16 @@ func (p *Azure) authorizeSSHSign(claims azurePayload, name string) ([]SignOption
 	signOptions = append(signOptions, sshCertificateDefaultsModifier(defaults))
 
 	return append(signOptions,
-		// set the default extensions
+		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
-		// checks the validity bounds, and set the validity if has not been set
+		// Checks the validity bounds, and set the validity if has not been set
 		&sshCertificateValidityModifier{p.claimer, 0},
-		// validate public key
+		// Check the validity bounds against default provisioner and
+		// provisioning credential bounds.
+		&sshCertificateDurationValidator{p.claimer, 0},
+		// Validate public key.
 		&sshDefaultPublicKeyValidator{},
-		// require all the fields in the SSH certificate
+		// Require all the fields in the SSH certificate.
 		&sshCertificateDefaultValidator{},
 	), nil
 }
diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go
index 466cfc587..a6115864f 100644
--- a/authority/provisioner/gcp.go
+++ b/authority/provisioner/gcp.go
@@ -213,8 +213,8 @@ func (p *GCP) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	}
 
 	// Check for the sign ssh method, default to sign X.509
-	if m := MethodFromContext(ctx); m == SignSSHMethod {
-		if p.claimer.IsSSHCAEnabled() == false {
+	if MethodFromContext(ctx) == SignSSHMethod {
+		if !p.claimer.IsSSHCAEnabled() {
 			return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID())
 		}
 		return p.authorizeSSHSign(claims)
@@ -237,10 +237,13 @@ func (p *GCP) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	}
 
 	return append(so,
-		defaultPublicKeyValidator{},
-		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
+		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeGCP, p.Name, claims.Subject, "InstanceID", ce.InstanceID, "InstanceName", ce.InstanceName),
-		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		x509ProfileValidityModifier{p.claimer, 0},
+		// validators
+		defaultPublicKeyValidator{},
+		validityValidator{},
+		x509CertificateDurationValidator{p.claimer, 0},
 	), nil
 }
 
@@ -378,13 +381,16 @@ func (p *GCP) authorizeSSHSign(claims *gcpPayload) ([]SignOption, error) {
 	signOptions = append(signOptions, sshCertificateDefaultsModifier(defaults))
 
 	return append(signOptions,
-		// set the default extensions
+		// Set the default extensions
 		&sshDefaultExtensionModifier{},
-		// checks the validity bounds, and set the validity if has not been set
+		// Checks the validity bounds, and set the validity if has not been set
 		&sshCertificateValidityModifier{p.claimer, 0},
-		// validate public key
+		// Check the validity bounds against default provisioner and
+		// provisioning credential bounds.
+		&sshCertificateDurationValidator{p.claimer, 0},
+		// Validate public key
 		&sshDefaultPublicKeyValidator{},
-		// require all the fields in the SSH certificate
+		// Require all the fields in the SSH certificate
 		&sshCertificateDefaultValidator{},
 	), nil
 }
diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go
index a4569d0f3..e2a7a80f2 100644
--- a/authority/provisioner/jwk.go
+++ b/authority/provisioner/jwk.go
@@ -141,8 +141,8 @@ func (p *JWK) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 		return nil, err
 	}
 
-	// Check for SSH token
-	if claims.Step != nil && claims.Step.SSH != nil {
+	// Check for SSH sign-ing request.
+	if MethodFromContext(ctx) == SignSSHMethod {
 		if p.claimer.IsSSHCAEnabled() == false {
 			return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID())
 		}
@@ -158,14 +158,17 @@ func (p *JWK) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 
 	dnsNames, ips, emails := x509util.SplitSANs(claims.SANs)
 	return []SignOption{
-		defaultPublicKeyValidator{},
+		// modifiers / withOptions
+		newProvisionerExtensionOption(TypeJWK, p.Name, p.Key.KeyID),
+		x509ProfileValidityModifier{p.claimer, 0},
+		// validators
 		commonNameValidator(claims.Subject),
+		defaultPublicKeyValidator{},
 		dnsNamesValidator(dnsNames),
-		ipAddressesValidator(ips),
 		emailAddressesValidator(emails),
-		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
-		newProvisionerExtensionOption(TypeJWK, p.Name, p.Key.KeyID),
-		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		ipAddressesValidator(ips),
+		validityValidator{},
+		x509CertificateDurationValidator{p.claimer, 0},
 	}, nil
 }
 
@@ -180,6 +183,9 @@ func (p *JWK) AuthorizeRenewal(cert *x509.Certificate) error {
 // authorizeSSHSign returns the list of SignOption for a SignSSH request.
 func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) {
 	t := now()
+	if claims.Step == nil || claims.Step.SSH == nil {
+		return nil, errors.New("authorization token must be an SSH provisioning token")
+	}
 	opts := claims.Step.SSH
 	signOptions := []SignOption{
 		// validates user's SSHOptions with the ones in the token
@@ -206,13 +212,16 @@ func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) {
 	signOptions = append(signOptions, sshCertificateDefaultsModifier{CertType: SSHUserCert})
 
 	return append(signOptions,
-		// set the default extensions
+		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
-		// checks the validity bounds, and set the validity if has not been set
+		// Set the validity bounds if not set.
 		&sshCertificateValidityModifier{p.claimer, 0},
-		// validate public key
+		// Check the validity bounds against default provisioner and
+		// provisioning credential bounds.
+		&sshCertificateDurationValidator{p.claimer, 0},
+		// Validate public key
 		&sshDefaultPublicKeyValidator{},
-		// require all the fields in the SSH certificate
+		// Require and validate all the default fields in the SSH certificate.
 		&sshCertificateDefaultValidator{},
 	), nil
 }
diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go
index 242b027ff..ff00f865f 100644
--- a/authority/provisioner/oidc.go
+++ b/authority/provisioner/oidc.go
@@ -285,27 +285,21 @@ func (o *OIDC) AuthorizeSign(ctx context.Context, token string) ([]SignOption, e
 	}
 
 	// Check for the sign ssh method, default to sign X.509
-	if m := MethodFromContext(ctx); m == SignSSHMethod {
-		if o.claimer.IsSSHCAEnabled() == false {
+	if MethodFromContext(ctx) == SignSSHMethod {
+		if !o.claimer.IsSSHCAEnabled() {
 			return nil, errors.Errorf("ssh ca is disabled for provisioner %s", o.GetID())
 		}
 		return o.authorizeSSHSign(claims)
 	}
 
-	// Admins should be able to authorize any SAN
-	if o.IsAdmin(claims.Email) {
-		return []SignOption{
-			profileDefaultDuration(o.claimer.DefaultTLSCertDuration()),
-			newProvisionerExtensionOption(TypeOIDC, o.Name, o.ClientID),
-			newValidityValidator(o.claimer.MinTLSCertDuration(), o.claimer.MaxTLSCertDuration()),
-		}, nil
-	}
-
 	so := []SignOption{
+		// modifiers / withOptions
 		defaultPublicKeyValidator{},
-		profileDefaultDuration(o.claimer.DefaultTLSCertDuration()),
+		x509ProfileValidityModifier{o.claimer, 0},
+		// validators
 		newProvisionerExtensionOption(TypeOIDC, o.Name, o.ClientID),
-		newValidityValidator(o.claimer.MinTLSCertDuration(), o.claimer.MaxTLSCertDuration()),
+		validityValidator{},
+		x509CertificateDurationValidator{o.claimer, 0},
 	}
 	// Admins should be able to authorize any SAN
 	if o.IsAdmin(claims.Email) {
@@ -350,13 +344,16 @@ func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) {
 	signOptions = append(signOptions, sshCertificateDefaultsModifier(defaults))
 
 	return append(signOptions,
-		// set the default extensions
+		// Set the default extensions
 		&sshDefaultExtensionModifier{},
-		// checks the validity bounds, and set the validity if has not been set
+		// Checks the validity bounds, and set the validity if has not been set
 		&sshCertificateValidityModifier{o.claimer, 0},
-		// validate public key
+		// Check the validity bounds against default provisioner and
+		// provisioning credential bounds.
+		&sshCertificateDurationValidator{o.claimer, 0},
+		// Validate public key
 		&sshDefaultPublicKeyValidator{},
-		// require all the fields in the SSH certificate
+		// Require all the fields in the SSH certificate
 		&sshCertificateDefaultValidator{},
 	), nil
 }
diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go
index a15aca254..f23ad8342 100644
--- a/authority/provisioner/sign_options.go
+++ b/authority/provisioner/sign_options.go
@@ -59,15 +59,38 @@ func (v profileWithOption) Option(Options) x509util.WithOption {
 // interface.
 // NOTE: This method sets values, but does not validate them;
 // validation is done in the ValidityValidator.
-type profileDefaultDuration time.Duration
+type x509ProfileValidityModifier struct {
+	*Claimer
+	// RemainingProvisioningCredentialDuraion is the remaining duration on the
+	// provisioning credential.
+	// E.g. x5c provisioners use a certificate as a provisioning credential.
+	// That certificate should not be able to provision new certificates with
+	// a duration longer than the remaining duration on the provisioning
+	// certificate.
+	RemainingProvisioningCredentialDuration time.Duration
+}
+
+func (m x509ProfileValidityModifier) Option(so Options) x509util.WithOption {
+	var (
+		d   = m.DefaultTLSCertDuration()
+		rem = m.RemainingProvisioningCredentialDuration
+	)
+
+	if rem > 0 {
+		// If the remaining duration from the provisioning credential is less than
+		// the default duration for the requested type of SSH certificate then we
+		// reset our default duration.
+		if rem < d {
+			d = rem
+		}
+	}
 
-func (v profileDefaultDuration) Option(so Options) x509util.WithOption {
-	notBefore := so.NotBefore.Time()
-	if notBefore.IsZero() {
-		notBefore = time.Now()
+	nbf := so.NotBefore.Time()
+	if nbf.IsZero() {
+		nbf = now()
 	}
-	notAfter := so.NotAfter.RelativeTime(notBefore)
-	return x509util.WithNotBeforeAfterDuration(notBefore, notAfter, time.Duration(v))
+	naf := so.NotAfter.RelativeTime(nbf)
+	return x509util.WithNotBeforeAfterDuration(nbf, naf, d)
 }
 
 // emailOnlyIdentity is a CertificateRequestValidator that checks that the only
@@ -201,40 +224,66 @@ func (v emailAddressesValidator) Valid(req *x509.CertificateRequest) error {
 	return nil
 }
 
-// validityValidator validates the certificate temporal validity settings.
-type validityValidator struct {
-	min time.Duration
-	max time.Duration
+// x509CertificateDurationValidator implements x509CertificateOptionsValidator
+// and checks the validity bounds. It will fail if a CertType has not been set
+// or is not valid.
+type x509CertificateDurationValidator struct {
+	*Claimer
+	// RemainingProvisioningCredentialDuraion is the remaining duration on the
+	// provisioning credential.
+	// E.g. x5c provisioners use a certificate as a provisioning credential.
+	// That certificate should not be able to provision new certificates with
+	// a duration longer than the remaining duration on the provisioning
+	// certificate.
+	RemainingProvisioningCredentialDuration time.Duration
 }
 
-// newValidityValidator return a new validity validator.
-func newValidityValidator(min, max time.Duration) *validityValidator {
-	return &validityValidator{min: min, max: max}
+func (m *x509CertificateDurationValidator) Valid(cert *x509.Certificate) error {
+	if cert.NotBefore.IsZero() || cert.NotAfter.IsZero() {
+		return errors.Errorf("notBefore or notAfter cannot be zero; nbf = %s, naf = %s",
+			cert.NotBefore, cert.NotAfter)
+	}
+
+	min, max := m.MinTLSCertDuration(), m.MaxTLSCertDuration()
+	rem := m.RemainingProvisioningCredentialDuration
+
+	if rem > 0 {
+		if rem < min {
+			return errors.Errorf("remaining duration on provisioning credential is "+
+				"less than provsioners default minimum duration; rem = %d, min = %d", rem, min)
+		}
+		// If the remaining duration from the provisioning credential is less than
+		// the max duration for the requested type of SSH certificate then we
+		// reset our max bound.
+		if rem < max {
+			max = rem
+		}
+	}
+
+	diff := cert.NotAfter.Sub(cert.NotBefore)
+	switch {
+	case diff < min:
+		return errors.Errorf("x509 certificate duration cannot be lower than %s", min)
+	case diff > max:
+		return errors.Errorf("x509 certificate duration cannot be greater than %s", max)
+	default:
+		return nil
+	}
 }
 
+// validityValidator validates the certificate temporal validity settings.
+type validityValidator struct{}
+
 // Validate validates the certificate temporal validity settings.
-func (v *validityValidator) Valid(crt *x509.Certificate) error {
-	var (
-		na  = crt.NotAfter
-		nb  = crt.NotBefore
-		d   = na.Sub(nb)
-		now = time.Now()
-	)
+func (v validityValidator) Valid(cert *x509.Certificate) error {
+	na, nb := cert.NotAfter, cert.NotBefore
 
-	if na.Before(now) {
+	if na.Before(now()) {
 		return errors.Errorf("NotAfter: %v cannot be in the past", na)
 	}
 	if na.Before(nb) {
 		return errors.Errorf("NotAfter: %v cannot be before NotBefore: %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)
-	}
-	if d > v.max {
-		return errors.Errorf("requested duration of %v is more than the authorized maximum certificate duration of %v",
-			d, v.max)
-	}
 	return nil
 }
 
diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go
index 3048a1432..c9495675e 100644
--- a/authority/provisioner/sign_ssh_options.go
+++ b/authority/provisioner/sign_ssh_options.go
@@ -207,39 +207,21 @@ type sshCertificateValidityModifier struct {
 
 func (m *sshCertificateValidityModifier) Modify(cert *ssh.Certificate) error {
 	var (
-		d, min, max time.Duration
-		rem         = m.RemainingProvisioningCredentialDuraion
+		d   time.Duration
+		rem = m.RemainingProvisioningCredentialDuraion
 	)
 	switch cert.CertType {
 	case ssh.UserCert:
 		d = m.DefaultUserSSHCertDuration()
-		min = m.MinUserSSHCertDuration()
-		max = m.MaxUserSSHCertDuration()
 	case ssh.HostCert:
 		d = m.DefaultHostSSHCertDuration()
-		min = m.MinHostSSHCertDuration()
-		max = m.MaxHostSSHCertDuration()
 	case 0:
 		return errors.New("ssh certificate type has not been set")
 	default:
 		return errors.Errorf("unknown ssh certificate type %d", cert.CertType)
 	}
 
-	// Use the remaining duration from the provisioning duration to set bounds
-	// and values if it is supplied.
 	if rem > 0 {
-		// If the remaining duration is less than the min duration for the requested
-		// type of SSH certificate then return an error.
-		if rem < min {
-			return errors.New("remaining duration on X5C certificate in the token " +
-				"is less than the minimum SSH duration on the X5C provisioner")
-		}
-		// If the remaining duration from the provisioning credential is less than
-		// the max duration for the requested type of SSH certificate then we
-		// reset our max bound.
-		if rem < max {
-			max = rem
-		}
 		// If the remaining duration from the provisioning credential is less than
 		// the default duration for the requested type of SSH certificate then we
 		// reset our default duration.
@@ -255,6 +237,57 @@ func (m *sshCertificateValidityModifier) Modify(cert *ssh.Certificate) error {
 		t := time.Unix(int64(cert.ValidAfter), 0)
 		cert.ValidBefore = uint64(t.Add(d).Unix())
 	}
+	return nil
+}
+
+// sshCertificateDurationValidator implements SSHCertificateOptionsValidator
+// and checks the validity bounds. It will fail if a CertType has not been set
+// or is not valid.
+type sshCertificateDurationValidator struct {
+	*Claimer
+	// RemainingProvisioningCredentialDuraion is the remaining duration on the
+	// provisioning credential.
+	// E.g. x5c provisioners use a certificate as a provisioning credential.
+	// That certificate should not be able to provision new certificates with
+	// a duration longer than the remaining duration on the provisioning
+	// certificate.
+	RemainingProvisioningCredentialDuraion time.Duration
+}
+
+func (m *sshCertificateDurationValidator) Valid(cert *ssh.Certificate) error {
+	if cert.ValidAfter == 0 || cert.ValidBefore == 0 {
+		return errors.Errorf("validAfter or validBefore cannot be zero; va = %d, vb = %d",
+			cert.ValidAfter, cert.ValidBefore)
+	}
+	var (
+		min, max time.Duration
+		rem      = m.RemainingProvisioningCredentialDuraion
+	)
+	switch cert.CertType {
+	case ssh.UserCert:
+		min = m.MinUserSSHCertDuration()
+		max = m.MaxUserSSHCertDuration()
+	case ssh.HostCert:
+		min = m.MinHostSSHCertDuration()
+		max = m.MaxHostSSHCertDuration()
+	case 0:
+		return errors.New("ssh certificate type has not been set")
+	default:
+		return errors.Errorf("unknown ssh certificate type %d", cert.CertType)
+	}
+
+	if rem > 0 {
+		if rem < min {
+			return errors.Errorf("remaining duration on provisioning credential is "+
+				"less than provsioners default minimum duration; rem = %d, min = %d", rem, min)
+		}
+		// If the remaining duration from the provisioning credential is less than
+		// the max duration for the requested type of SSH certificate then we
+		// reset our max bound.
+		if rem < max {
+			max = rem
+		}
+	}
 
 	diff := time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second
 	switch {
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index a27aca3b4..ef9015f6c 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -21,13 +21,13 @@ type x5cPayload struct {
 // X5C is the default provisioner, an entity that can sign tokens necessary for
 // signature requests.
 type X5C struct {
-	Type      string `json:"type"`
-	Name      string `json:"name"`
-	Roots     string `json:"roots"`
-	rootPool  *x509.CertPool
+	Type      string  `json:"type"`
+	Name      string  `json:"name"`
+	Roots     string  `json:"roots"`
 	Claims    *Claims `json:"claims,omitempty"`
 	claimer   *Claimer
 	audiences Audiences
+	rootPool  *x509.CertPool
 }
 
 // GetID returns the provisioner unique identifier. The name and credential id
@@ -90,7 +90,7 @@ func (p *X5C) Init(config Config) (err error) {
 		return err
 	}
 
-	p.audiences = config.Audiences
+	p.audiences = config.Audiences.WithFragment(p.GetID())
 	return err
 }
 
@@ -134,6 +134,11 @@ func (p *X5C) authorizeToken(token string, audiences []string) (*x5cPayload, err
 		return nil, errors.Wrapf(err, "invalid token")
 	}
 
+	// validate audiences with the defaults
+	if !matchesAudience(claims.Audience, audiences) {
+		return nil, errors.New("invalid token: invalid audience claim (aud)")
+	}
+
 	if claims.Subject == "" {
 		return nil, errors.New("token subject cannot be empty")
 	}
@@ -157,9 +162,9 @@ func (p *X5C) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 		return nil, err
 	}
 
-	// Check for SSH token
-	if claims.Step != nil && claims.Step.SSH != nil {
-		if p.claimer.IsSSHCAEnabled() == false {
+	// Check for SSH sign-ing request.
+	if MethodFromContext(ctx) == SignSSHMethod {
+		if !p.claimer.IsSSHCAEnabled() {
 			return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID())
 		}
 		return p.authorizeSSHSign(claims)
@@ -177,39 +182,20 @@ func (p *X5C) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	// Get the remaining duration from the provisioning cert. The duration
 	// on the new certificate cannot be greater than the remaining duration of
 	// the provisioning certificate.
-	rem := claims.chains[0][0].NotAfter.Sub(time.Now())
-	minD, maxD, defaultD := p.claimer.MinTLSCertDuration(),
-		p.claimer.MaxTLSCertDuration(), p.claimer.DefaultTLSCertDuration()
-	if rem > 0 {
-		// If the remaining duration from the provisioning x5c Certificate is less
-		// than the provisioner's minimum default duration, then return an error.
-		if rem < minD {
-			return nil, errors.New("remaining duration on x5c certificate in the token " +
-				"is less than the minimum duration on the x5c provisioner")
-		}
-		// If the remaining duration from the provisioning credential is less than
-		// the max duration for the requested type of x509 certificate then we
-		// reset our max bound.
-		if rem < maxD {
-			maxD = rem
-		}
-		// If the remaining duration from the provisioning credential is less than
-		// the default duration for the requested type of x509 certificate then we
-		// reset our default duration.
-		if rem < defaultD {
-			defaultD = rem
-		}
-	}
+	rem := claims.chains[0][0].NotAfter.Sub(now())
 
 	return []SignOption{
-		defaultPublicKeyValidator{},
+		// modifiers / withOptions
+		newProvisionerExtensionOption(TypeX5C, p.Name, ""),
+		x509ProfileValidityModifier{p.claimer, rem},
+		// validators
 		commonNameValidator(claims.Subject),
+		defaultPublicKeyValidator{},
 		dnsNamesValidator(dnsNames),
-		ipAddressesValidator(ips),
 		emailAddressesValidator(emails),
-		profileDefaultDuration(defaultD),
-		newProvisionerExtensionOption(TypeX5C, p.Name, ""),
-		newValidityValidator(minD, maxD),
+		ipAddressesValidator(ips),
+		validityValidator{},
+		x509CertificateDurationValidator{p.claimer, rem},
 	}, nil
 }
 
@@ -224,6 +210,9 @@ func (p *X5C) AuthorizeRenewal(cert *x509.Certificate) error {
 // authorizeSSHSign returns the list of SignOption for a SignSSH request.
 func (p *X5C) authorizeSSHSign(claims *x5cPayload) ([]SignOption, error) {
 	t := now()
+	if claims.Step == nil || claims.Step.SSH == nil {
+		return nil, errors.New("authorization token must be an SSH provisioning token")
+	}
 	opts := claims.Step.SSH
 	signOptions := []SignOption{
 		// validates user's SSHOptions with the ones in the token
@@ -249,16 +238,19 @@ func (p *X5C) authorizeSSHSign(claims *x5cPayload) ([]SignOption, error) {
 	// Default to a user certificate with no principals if not set
 	signOptions = append(signOptions, sshCertificateDefaultsModifier{CertType: SSHUserCert})
 
-	rem := claims.chains[0][0].NotAfter.Sub(time.Now())
+	rem := claims.chains[0][0].NotAfter.Sub(now())
 
 	return append(signOptions,
-		// set the default extensions
+		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
-		// checks the validity bounds, and set the validity if has not been set
+		// Checks the validity bounds, and set the validity if has not been set.
 		&sshCertificateValidityModifier{p.claimer, rem},
-		// validate public key
+		// Check the validity bounds against default provisioner and
+		// provisioning credential bounds.
+		&sshCertificateDurationValidator{p.claimer, rem},
+		// Validate public key.
 		&sshDefaultPublicKeyValidator{},
-		// require all the fields in the SSH certificate
+		// Require all the fields in the SSH certificate
 		&sshCertificateDefaultValidator{},
 	), nil
 }

From d2dc264eb78a869b2a618e8ad777c2b5390de60a Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Sun, 29 Sep 2019 23:12:04 -0700
Subject: [PATCH 06/21] wip

---
 authority/provisioner/acme.go             |   5 +-
 authority/provisioner/aws.go              |  18 ++--
 authority/provisioner/azure.go            |  18 ++--
 authority/provisioner/gcp.go              |  14 +--
 authority/provisioner/jwk.go              |  12 +--
 authority/provisioner/oidc.go             |  14 +--
 authority/provisioner/sign_options.go     |  90 ++++++++--------
 authority/provisioner/sign_ssh_options.go | 126 +++++++++-------------
 authority/provisioner/x5c.go              |  12 +--
 9 files changed, 133 insertions(+), 176 deletions(-)

diff --git a/authority/provisioner/acme.go b/authority/provisioner/acme.go
index 1b1c40113..e26b5165e 100644
--- a/authority/provisioner/acme.go
+++ b/authority/provisioner/acme.go
@@ -71,11 +71,10 @@ func (p *ACME) AuthorizeSign(ctx context.Context, _ string) ([]SignOption, error
 	return []SignOption{
 		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeACME, p.Name, ""),
-		x509ProfileValidityModifier{p.claimer, 0},
+		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
 		// validators
 		defaultPublicKeyValidator{},
-		validityValidator{},
-		x509CertificateDurationValidator{p.claimer, 0},
+		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	}, nil
 }
 
diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go
index 31bf2108a..30286dd6d 100644
--- a/authority/provisioner/aws.go
+++ b/authority/provisioner/aws.go
@@ -298,12 +298,11 @@ func (p *AWS) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	return append(so,
 		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeAWS, p.Name, doc.AccountID, "InstanceID", doc.InstanceID),
-		x509ProfileValidityModifier{p.claimer, 0},
+		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
 		// validators
 		defaultPublicKeyValidator{},
 		commonNameValidator(payload.Claims.Subject),
-		validityValidator{},
-		x509CertificateDurationValidator{p.claimer, 0},
+		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	), nil
 }
 
@@ -471,14 +470,11 @@ func (p *AWS) authorizeSSHSign(claims *awsPayload) ([]SignOption, error) {
 	return append(signOptions,
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
-		// Checks the validity bounds, and set the validity if has not been set.
-		&sshCertificateValidityModifier{p.claimer, 0},
-		// Check the validity bounds against default provisioner and
-		// provisioning credential bounds.
-		&sshCertificateDurationValidator{p.claimer, 0},
-		// Validate public key.
+		// Set the validity bounds if not set.
+		&sshDefaultTemporalModifier{p.claimer},
+		// Validate public key
 		&sshDefaultPublicKeyValidator{},
-		// Require all the fields in the SSH certificate.
-		&sshCertificateDefaultValidator{},
+		// Require all the fields in the SSH certificate
+		&sshCertificateDefaultValidator{p.claimer},
 	), nil
 }
diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go
index 86a4d6d87..7b2b9906f 100644
--- a/authority/provisioner/azure.go
+++ b/authority/provisioner/azure.go
@@ -286,11 +286,10 @@ func (p *Azure) AuthorizeSign(ctx context.Context, token string) ([]SignOption,
 	return append(so,
 		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeAzure, p.Name, p.TenantID),
-		x509ProfileValidityModifier{p.claimer, 0},
+		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
 		// validators
 		defaultPublicKeyValidator{},
-		validityValidator{},
-		x509CertificateDurationValidator{p.claimer, 0},
+		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	), nil
 }
 
@@ -328,15 +327,12 @@ func (p *Azure) authorizeSSHSign(claims azurePayload, name string) ([]SignOption
 	return append(signOptions,
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
-		// Checks the validity bounds, and set the validity if has not been set
-		&sshCertificateValidityModifier{p.claimer, 0},
-		// Check the validity bounds against default provisioner and
-		// provisioning credential bounds.
-		&sshCertificateDurationValidator{p.claimer, 0},
-		// Validate public key.
+		// Set the validity bounds if not set.
+		&sshDefaultTemporalModifier{p.claimer},
+		// Validate public key
 		&sshDefaultPublicKeyValidator{},
-		// Require all the fields in the SSH certificate.
-		&sshCertificateDefaultValidator{},
+		// Require all the fields in the SSH certificate
+		&sshCertificateDefaultValidator{p.claimer},
 	), nil
 }
 
diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go
index a6115864f..c7d9cd2bc 100644
--- a/authority/provisioner/gcp.go
+++ b/authority/provisioner/gcp.go
@@ -239,11 +239,10 @@ func (p *GCP) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	return append(so,
 		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeGCP, p.Name, claims.Subject, "InstanceID", ce.InstanceID, "InstanceName", ce.InstanceName),
-		x509ProfileValidityModifier{p.claimer, 0},
+		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
 		// validators
 		defaultPublicKeyValidator{},
-		validityValidator{},
-		x509CertificateDurationValidator{p.claimer, 0},
+		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	), nil
 }
 
@@ -383,14 +382,11 @@ func (p *GCP) authorizeSSHSign(claims *gcpPayload) ([]SignOption, error) {
 	return append(signOptions,
 		// Set the default extensions
 		&sshDefaultExtensionModifier{},
-		// Checks the validity bounds, and set the validity if has not been set
-		&sshCertificateValidityModifier{p.claimer, 0},
-		// Check the validity bounds against default provisioner and
-		// provisioning credential bounds.
-		&sshCertificateDurationValidator{p.claimer, 0},
+		// Set the validity bounds if not set.
+		&sshDefaultTemporalModifier{p.claimer},
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
 		// Require all the fields in the SSH certificate
-		&sshCertificateDefaultValidator{},
+		&sshCertificateDefaultValidator{p.claimer},
 	), nil
 }
diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go
index e2a7a80f2..8b3d6d23c 100644
--- a/authority/provisioner/jwk.go
+++ b/authority/provisioner/jwk.go
@@ -160,15 +160,14 @@ func (p *JWK) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	return []SignOption{
 		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeJWK, p.Name, p.Key.KeyID),
-		x509ProfileValidityModifier{p.claimer, 0},
+		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
 		// validators
 		commonNameValidator(claims.Subject),
 		defaultPublicKeyValidator{},
 		dnsNamesValidator(dnsNames),
 		emailAddressesValidator(emails),
 		ipAddressesValidator(ips),
-		validityValidator{},
-		x509CertificateDurationValidator{p.claimer, 0},
+		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	}, nil
 }
 
@@ -215,13 +214,10 @@ func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) {
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshCertificateValidityModifier{p.claimer, 0},
-		// Check the validity bounds against default provisioner and
-		// provisioning credential bounds.
-		&sshCertificateDurationValidator{p.claimer, 0},
+		&sshDefaultTemporalModifier{p.claimer},
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
 		// Require and validate all the default fields in the SSH certificate.
-		&sshCertificateDefaultValidator{},
+		&sshCertificateDefaultValidator{p.claimer},
 	), nil
 }
diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go
index ff00f865f..156f41228 100644
--- a/authority/provisioner/oidc.go
+++ b/authority/provisioner/oidc.go
@@ -295,11 +295,10 @@ func (o *OIDC) AuthorizeSign(ctx context.Context, token string) ([]SignOption, e
 	so := []SignOption{
 		// modifiers / withOptions
 		defaultPublicKeyValidator{},
-		x509ProfileValidityModifier{o.claimer, 0},
+		profileDefaultDuration(o.claimer.DefaultTLSCertDuration()),
 		// validators
 		newProvisionerExtensionOption(TypeOIDC, o.Name, o.ClientID),
-		validityValidator{},
-		x509CertificateDurationValidator{o.claimer, 0},
+		newTemporalValidator(o.claimer.MinTLSCertDuration(), o.claimer.MaxTLSCertDuration()),
 	}
 	// Admins should be able to authorize any SAN
 	if o.IsAdmin(claims.Email) {
@@ -346,15 +345,12 @@ func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) {
 	return append(signOptions,
 		// Set the default extensions
 		&sshDefaultExtensionModifier{},
-		// Checks the validity bounds, and set the validity if has not been set
-		&sshCertificateValidityModifier{o.claimer, 0},
-		// Check the validity bounds against default provisioner and
-		// provisioning credential bounds.
-		&sshCertificateDurationValidator{o.claimer, 0},
+		// Set the validity bounds if not set.
+		&sshDefaultTemporalModifier{o.claimer},
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
 		// Require all the fields in the SSH certificate
-		&sshCertificateDefaultValidator{},
+		&sshCertificateDefaultValidator{o.claimer},
 	), nil
 }
 
diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go
index f23ad8342..c960d53b5 100644
--- a/authority/provisioner/sign_options.go
+++ b/authority/provisioner/sign_options.go
@@ -224,66 +224,68 @@ func (v emailAddressesValidator) Valid(req *x509.CertificateRequest) error {
 	return nil
 }
 
-// x509CertificateDurationValidator implements x509CertificateOptionsValidator
-// and checks the validity bounds. It will fail if a CertType has not been set
-// or is not valid.
-type x509CertificateDurationValidator struct {
-	*Claimer
-	// RemainingProvisioningCredentialDuraion is the remaining duration on the
-	// provisioning credential.
-	// E.g. x5c provisioners use a certificate as a provisioning credential.
-	// That certificate should not be able to provision new certificates with
-	// a duration longer than the remaining duration on the provisioning
-	// certificate.
-	RemainingProvisioningCredentialDuration time.Duration
+// profileProvCredDuration adjusts the duration to the
+// min(default, remaining provisioning credential duration.
+type profileProvCredDuration struct {
+	def time.Duration
+	rem time.Duration
 }
 
-func (m *x509CertificateDurationValidator) Valid(cert *x509.Certificate) error {
-	if cert.NotBefore.IsZero() || cert.NotAfter.IsZero() {
-		return errors.Errorf("notBefore or notAfter cannot be zero; nbf = %s, naf = %s",
-			cert.NotBefore, cert.NotAfter)
+func (v profileProvCredDuration) Option(so Options) x509util.WithOption {
+	if v.rem < v.def {
+		v.def = v.rem
 	}
+	return profileDefaultDuration(v.def).Option(so)
+}
 
-	min, max := m.MinTLSCertDuration(), m.MaxTLSCertDuration()
-	rem := m.RemainingProvisioningCredentialDuration
+// profileDefaultDuration is a wrapper against x509util.WithOption to conform
+// the SignOption interface.
+type profileDefaultDuration time.Duration
 
-	if rem > 0 {
-		if rem < min {
-			return errors.Errorf("remaining duration on provisioning credential is "+
-				"less than provsioners default minimum duration; rem = %d, min = %d", rem, min)
-		}
-		// If the remaining duration from the provisioning credential is less than
-		// the max duration for the requested type of SSH certificate then we
-		// reset our max bound.
-		if rem < max {
-			max = rem
-		}
+func (v profileDefaultDuration) Option(so Options) x509util.WithOption {
+	notBefore := so.NotBefore.Time()
+	if notBefore.IsZero() {
+		notBefore = time.Now()
 	}
+	notAfter := so.NotAfter.RelativeTime(notBefore)
+	return x509util.WithNotBeforeAfterDuration(notBefore, notAfter, time.Duration(v))
+}
 
-	diff := cert.NotAfter.Sub(cert.NotBefore)
-	switch {
-	case diff < min:
-		return errors.Errorf("x509 certificate duration cannot be lower than %s", min)
-	case diff > max:
-		return errors.Errorf("x509 certificate duration cannot be greater than %s", max)
-	default:
-		return nil
-	}
+// temporalValidator validates the certificate temporal validity settings.
+type temporalValidator struct {
+	min time.Duration
+	max time.Duration
 }
 
-// validityValidator validates the certificate temporal validity settings.
-type validityValidator struct{}
+// newTemporalValidator return a new validity validator.
+func newTemporalValidator(min, max time.Duration) *temporalValidator {
+	return &temporalValidator{min: min, max: max}
+}
 
-// Validate validates the certificate temporal validity settings.
-func (v validityValidator) Valid(cert *x509.Certificate) error {
-	na, nb := cert.NotAfter, cert.NotBefore
+// Validate validates the certificate temporal settings (notBefore/notAfter)
+// and total duration.
+func (v *temporalValidator) Valid(crt *x509.Certificate) error {
+	var (
+		na  = crt.NotAfter
+		nb  = crt.NotBefore
+		d   = na.Sub(nb)
+		now = time.Now()
+	)
 
-	if na.Before(now()) {
+	if na.Before(now) {
 		return errors.Errorf("NotAfter: %v cannot be in the past", na)
 	}
 	if na.Before(nb) {
 		return errors.Errorf("NotAfter: %v cannot be before NotBefore: %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)
+	}
+	if d > v.max {
+		return errors.Errorf("requested duration of %v is more than the authorized maximum certificate duration of %v",
+			d, v.max)
+	}
 	return nil
 }
 
diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go
index c9495675e..ee6bc76ec 100644
--- a/authority/provisioner/sign_ssh_options.go
+++ b/authority/provisioner/sign_ssh_options.go
@@ -191,25 +191,16 @@ func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error {
 	}
 }
 
-// sshCertificateValidityModifier is a SSHCertificateModifier checks the
+// defaultSSHCertTemporalModifier is an SSHCertificateModifier that checks the
 // validity bounds, setting them if they are not provided. It will fail if a
 // CertType has not been set or is not valid.
-type sshCertificateValidityModifier struct {
+type sshDefaultTemporalModifier struct {
 	*Claimer
-	// RemainingProvisioningCredentialDuraion is the remaining duration on the
-	// provisioning credential.
-	// E.g. x5c provisioners use a certificate as a provisioning credential.
-	// That certificate should not be able to provision new certificates with
-	// a duration longer than the remaining duration on the provisioning
-	// certificate.
-	RemainingProvisioningCredentialDuraion time.Duration
 }
 
-func (m *sshCertificateValidityModifier) Modify(cert *ssh.Certificate) error {
-	var (
-		d   time.Duration
-		rem = m.RemainingProvisioningCredentialDuraion
-	)
+func (m *sshDefaultTemporalModifier) Modify(cert *ssh.Certificate) error {
+	var d time.Duration
+
 	switch cert.CertType {
 	case ssh.UserCert:
 		d = m.DefaultUserSSHCertDuration()
@@ -221,15 +212,6 @@ func (m *sshCertificateValidityModifier) Modify(cert *ssh.Certificate) error {
 		return errors.Errorf("unknown ssh certificate type %d", cert.CertType)
 	}
 
-	if rem > 0 {
-		// If the remaining duration from the provisioning credential is less than
-		// the default duration for the requested type of SSH certificate then we
-		// reset our default duration.
-		if rem < d {
-			d = rem
-		}
-	}
-
 	if cert.ValidAfter == 0 {
 		cert.ValidAfter = uint64(now().Truncate(time.Second).Unix())
 	}
@@ -237,67 +219,43 @@ func (m *sshCertificateValidityModifier) Modify(cert *ssh.Certificate) error {
 		t := time.Unix(int64(cert.ValidAfter), 0)
 		cert.ValidBefore = uint64(t.Add(d).Unix())
 	}
+
 	return nil
 }
 
-// sshCertificateDurationValidator implements SSHCertificateOptionsValidator
-// and checks the validity bounds. It will fail if a CertType has not been set
-// or is not valid.
-type sshCertificateDurationValidator struct {
+type sshProvisioningCredTemporalModifier struct {
 	*Claimer
-	// RemainingProvisioningCredentialDuraion is the remaining duration on the
-	// provisioning credential.
+	// Remaining duration on the provisioning credential.
 	// E.g. x5c provisioners use a certificate as a provisioning credential.
 	// That certificate should not be able to provision new certificates with
 	// a duration longer than the remaining duration on the provisioning
 	// certificate.
-	RemainingProvisioningCredentialDuraion time.Duration
+	rem time.Duration
 }
 
-func (m *sshCertificateDurationValidator) Valid(cert *ssh.Certificate) error {
-	if cert.ValidAfter == 0 || cert.ValidBefore == 0 {
-		return errors.Errorf("validAfter or validBefore cannot be zero; va = %d, vb = %d",
-			cert.ValidAfter, cert.ValidBefore)
-	}
-	var (
-		min, max time.Duration
-		rem      = m.RemainingProvisioningCredentialDuraion
-	)
-	switch cert.CertType {
-	case ssh.UserCert:
-		min = m.MinUserSSHCertDuration()
-		max = m.MaxUserSSHCertDuration()
-	case ssh.HostCert:
-		min = m.MinHostSSHCertDuration()
-		max = m.MaxHostSSHCertDuration()
-	case 0:
-		return errors.New("ssh certificate type has not been set")
-	default:
-		return errors.Errorf("unknown ssh certificate type %d", cert.CertType)
+// Modify has the following procedure:
+//  1) Use the existing ValidBefore & ValidAfter values if they are set; else
+//  2.a) Use the default duration to set the validity bounds and
+//  2.b) If 'rem' is non-zero and less than the default duration then
+//       reduce the duration to 'rem'.
+//
+// Step 2) only applies if ValidBefore is not set in the request.
+func (m *sshProvisioningCredTemporalModifier) Modify(cert *ssh.Certificate) error {
+	defMod := &sshDefaultTemporalModifier{m.Claimer}
+	if err := defMod.Modify(cert); err != nil {
+		return err
+	}
+	if m.rem == 0 {
+		return nil
 	}
 
-	if rem > 0 {
-		if rem < min {
-			return errors.Errorf("remaining duration on provisioning credential is "+
-				"less than provsioners default minimum duration; rem = %d, min = %d", rem, min)
-		}
-		// If the remaining duration from the provisioning credential is less than
-		// the max duration for the requested type of SSH certificate then we
-		// reset our max bound.
-		if rem < max {
-			max = rem
-		}
+	diff := cert.ValidBefore - cert.ValidAfter
+	if uint64(m.rem) < diff {
+		t := time.Unix(int64(cert.ValidAfter), 0)
+		cert.ValidBefore = uint64(t.Add(m.rem).Unix())
 	}
 
-	diff := time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second
-	switch {
-	case diff < min:
-		return errors.Errorf("ssh certificate duration cannot be lower than %s", min)
-	case diff > max:
-		return errors.Errorf("ssh certificate duration cannot be greater than %s", max)
-	default:
-		return nil
-	}
+	return nil
 }
 
 // sshCertificateOptionsValidator validates the user SSHOptions with the ones
@@ -313,10 +271,28 @@ func (v sshCertificateOptionsValidator) Valid(got SSHOptions) error {
 
 // sshCertificateDefaultValidator implements a simple validator for all the
 // fields in the SSH certificate.
-type sshCertificateDefaultValidator struct{}
+type sshCertificateDefaultValidator struct {
+	*Claimer
+}
 
 // Valid returns an error if the given certificate does not contain the necessary fields.
 func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error {
+	var min, max time.Duration
+	switch cert.CertType {
+	case ssh.UserCert:
+		min = v.MinUserSSHCertDuration()
+		max = v.MaxUserSSHCertDuration()
+	case ssh.HostCert:
+		min = v.MinHostSSHCertDuration()
+		max = v.MaxHostSSHCertDuration()
+	case 0:
+		return errors.New("ssh certificate type has not been set")
+	default:
+		return errors.Errorf("unknown ssh certificate type %d", cert.CertType)
+	}
+
+	duration := time.Duration(cert.ValidBefore - cert.ValidAfter)
+
 	switch {
 	case len(cert.Nonce) == 0:
 		return errors.New("ssh certificate nonce cannot be empty")
@@ -324,8 +300,6 @@ func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error {
 		return errors.New("ssh certificate key cannot be nil")
 	case cert.Serial == 0:
 		return errors.New("ssh certificate serial cannot be 0")
-	case cert.CertType != ssh.UserCert && cert.CertType != ssh.HostCert:
-		return errors.Errorf("ssh certificate has an unknown type: %d", cert.CertType)
 	case cert.KeyId == "":
 		return errors.New("ssh certificate key id cannot be empty")
 	case len(cert.ValidPrincipals) == 0:
@@ -336,6 +310,12 @@ func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error {
 		return errors.New("ssh certificate validBefore cannot be in the past")
 	case cert.ValidBefore < cert.ValidAfter:
 		return errors.New("ssh certificate validBefore cannot be before validAfter")
+	case duration < min:
+		return errors.Errorf("requested duration of %s is less than minimum "+
+			"accepted duration for selected provisioner of %s", duration, min)
+	case duration > max:
+		return errors.Errorf("requested duration of %s is greater than maximum "+
+			"accepted duration for selected provisioner of %s", duration, max)
 	case cert.CertType == ssh.UserCert && len(cert.Extensions) == 0:
 		return errors.New("ssh certificate extensions cannot be empty")
 	case cert.SignatureKey == nil:
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index ef9015f6c..be5d42298 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -187,15 +187,14 @@ func (p *X5C) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	return []SignOption{
 		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeX5C, p.Name, ""),
-		x509ProfileValidityModifier{p.claimer, rem},
+		profileProvCredDuration{p.claimer.DefaultTLSCertDuration(), rem},
 		// validators
 		commonNameValidator(claims.Subject),
 		defaultPublicKeyValidator{},
 		dnsNamesValidator(dnsNames),
 		emailAddressesValidator(emails),
 		ipAddressesValidator(ips),
-		validityValidator{},
-		x509CertificateDurationValidator{p.claimer, rem},
+		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	}, nil
 }
 
@@ -244,13 +243,10 @@ func (p *X5C) authorizeSSHSign(claims *x5cPayload) ([]SignOption, error) {
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Checks the validity bounds, and set the validity if has not been set.
-		&sshCertificateValidityModifier{p.claimer, rem},
-		// Check the validity bounds against default provisioner and
-		// provisioning credential bounds.
-		&sshCertificateDurationValidator{p.claimer, rem},
+		&sshProvisioningCredTemporalModifier{p.claimer, rem},
 		// Validate public key.
 		&sshDefaultPublicKeyValidator{},
 		// Require all the fields in the SSH certificate
-		&sshCertificateDefaultValidator{},
+		&sshCertificateDefaultValidator{p.claimer},
 	), nil
 }

From 775c883927e2321632957bbd31d51ff3a56212b3 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Sun, 29 Sep 2019 23:13:02 -0700
Subject: [PATCH 07/21] wip

---
 authority/provisioner/sign_options.go | 40 ---------------------------
 1 file changed, 40 deletions(-)

diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go
index c960d53b5..a7d222083 100644
--- a/authority/provisioner/sign_options.go
+++ b/authority/provisioner/sign_options.go
@@ -53,46 +53,6 @@ func (v profileWithOption) Option(Options) x509util.WithOption {
 	return x509util.WithOption(v)
 }
 
-// profileDefaultDuration sets the NotBefore and NotAfter attributes on an
-// X509 certificate (profile) from values parsed along with the request.
-// This method is a wrapper against x509util.WithOption to conform the
-// interface.
-// NOTE: This method sets values, but does not validate them;
-// validation is done in the ValidityValidator.
-type x509ProfileValidityModifier struct {
-	*Claimer
-	// RemainingProvisioningCredentialDuraion is the remaining duration on the
-	// provisioning credential.
-	// E.g. x5c provisioners use a certificate as a provisioning credential.
-	// That certificate should not be able to provision new certificates with
-	// a duration longer than the remaining duration on the provisioning
-	// certificate.
-	RemainingProvisioningCredentialDuration time.Duration
-}
-
-func (m x509ProfileValidityModifier) Option(so Options) x509util.WithOption {
-	var (
-		d   = m.DefaultTLSCertDuration()
-		rem = m.RemainingProvisioningCredentialDuration
-	)
-
-	if rem > 0 {
-		// If the remaining duration from the provisioning credential is less than
-		// the default duration for the requested type of SSH certificate then we
-		// reset our default duration.
-		if rem < d {
-			d = rem
-		}
-	}
-
-	nbf := so.NotBefore.Time()
-	if nbf.IsZero() {
-		nbf = now()
-	}
-	naf := so.NotAfter.RelativeTime(nbf)
-	return x509util.WithNotBeforeAfterDuration(nbf, naf, d)
-}
-
 // emailOnlyIdentity is a CertificateRequestValidator that checks that the only
 // SAN provided is the given email address.
 type emailOnlyIdentity string

From 8cf1005e418ce591fb84cd173dfe98b1bc590115 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Mon, 30 Sep 2019 11:28:57 -0700
Subject: [PATCH 08/21] wip

---
 authority/provisioner/aws.go              |  6 ++-
 authority/provisioner/azure.go            |  6 ++-
 authority/provisioner/gcp.go              |  6 ++-
 authority/provisioner/jwk.go              |  6 ++-
 authority/provisioner/oidc.go             |  6 ++-
 authority/provisioner/sign_ssh_options.go | 49 +++++++++++++++--------
 authority/provisioner/x5c.go              |  6 ++-
 7 files changed, 56 insertions(+), 29 deletions(-)

diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go
index 30286dd6d..a54013cb0 100644
--- a/authority/provisioner/aws.go
+++ b/authority/provisioner/aws.go
@@ -471,10 +471,12 @@ func (p *AWS) authorizeSSHSign(claims *awsPayload) ([]SignOption, error) {
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshDefaultTemporalModifier{p.claimer},
+		&sshValidityModifier{p.claimer},
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
+		// Validate the validity period.
+		&sshCertificateValidityValidator{p.claimer},
 		// Require all the fields in the SSH certificate
-		&sshCertificateDefaultValidator{p.claimer},
+		&sshCertificateDefaultValidator{},
 	), nil
 }
diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go
index 7b2b9906f..abfe900f9 100644
--- a/authority/provisioner/azure.go
+++ b/authority/provisioner/azure.go
@@ -328,11 +328,13 @@ func (p *Azure) authorizeSSHSign(claims azurePayload, name string) ([]SignOption
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshDefaultTemporalModifier{p.claimer},
+		&sshValidityModifier{p.claimer},
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
+		// Validate the validity period.
+		&sshCertificateValidityValidator{p.claimer},
 		// Require all the fields in the SSH certificate
-		&sshCertificateDefaultValidator{p.claimer},
+		&sshCertificateDefaultValidator{},
 	), nil
 }
 
diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go
index c7d9cd2bc..939030934 100644
--- a/authority/provisioner/gcp.go
+++ b/authority/provisioner/gcp.go
@@ -383,10 +383,12 @@ func (p *GCP) authorizeSSHSign(claims *gcpPayload) ([]SignOption, error) {
 		// Set the default extensions
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshDefaultTemporalModifier{p.claimer},
+		&sshValidityModifier{p.claimer},
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
+		// Validate the validity period.
+		&sshCertificateValidityValidator{p.claimer},
 		// Require all the fields in the SSH certificate
-		&sshCertificateDefaultValidator{p.claimer},
+		&sshCertificateDefaultValidator{},
 	), nil
 }
diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go
index 8b3d6d23c..ed54d3cfa 100644
--- a/authority/provisioner/jwk.go
+++ b/authority/provisioner/jwk.go
@@ -214,10 +214,12 @@ func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) {
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshDefaultTemporalModifier{p.claimer},
+		&sshValidityModifier{p.claimer},
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
+		// Validate the validity period.
+		&sshCertificateValidityValidator{p.claimer},
 		// Require and validate all the default fields in the SSH certificate.
-		&sshCertificateDefaultValidator{p.claimer},
+		&sshCertificateDefaultValidator{},
 	), nil
 }
diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go
index 156f41228..58be12011 100644
--- a/authority/provisioner/oidc.go
+++ b/authority/provisioner/oidc.go
@@ -346,11 +346,13 @@ func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) {
 		// Set the default extensions
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshDefaultTemporalModifier{o.claimer},
+		&sshValidityModifier{o.claimer},
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
+		// Validate the validity period.
+		&sshCertificateValidityValidator{o.claimer},
 		// Require all the fields in the SSH certificate
-		&sshCertificateDefaultValidator{o.claimer},
+		&sshCertificateDefaultValidator{},
 	), nil
 }
 
diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go
index ee6bc76ec..ac8731b42 100644
--- a/authority/provisioner/sign_ssh_options.go
+++ b/authority/provisioner/sign_ssh_options.go
@@ -191,14 +191,14 @@ func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error {
 	}
 }
 
-// defaultSSHCertTemporalModifier is an SSHCertificateModifier that checks the
+// sshValidityModifier is an SSHCertificateModifier that checks the
 // validity bounds, setting them if they are not provided. It will fail if a
 // CertType has not been set or is not valid.
-type sshDefaultTemporalModifier struct {
+type sshValidityModifier struct {
 	*Claimer
 }
 
-func (m *sshDefaultTemporalModifier) Modify(cert *ssh.Certificate) error {
+func (m *sshValidityModifier) Modify(cert *ssh.Certificate) error {
 	var d time.Duration
 
 	switch cert.CertType {
@@ -223,7 +223,7 @@ func (m *sshDefaultTemporalModifier) Modify(cert *ssh.Certificate) error {
 	return nil
 }
 
-type sshProvisioningCredTemporalModifier struct {
+type sshProvCredValidityModifier struct {
 	*Claimer
 	// Remaining duration on the provisioning credential.
 	// E.g. x5c provisioners use a certificate as a provisioning credential.
@@ -240,8 +240,8 @@ type sshProvisioningCredTemporalModifier struct {
 //       reduce the duration to 'rem'.
 //
 // Step 2) only applies if ValidBefore is not set in the request.
-func (m *sshProvisioningCredTemporalModifier) Modify(cert *ssh.Certificate) error {
-	defMod := &sshDefaultTemporalModifier{m.Claimer}
+func (m *sshProvCredValidityModifier) Modify(cert *ssh.Certificate) error {
+	defMod := &sshValidityModifier{m.Claimer}
 	if err := defMod.Modify(cert); err != nil {
 		return err
 	}
@@ -269,14 +269,11 @@ func (v sshCertificateOptionsValidator) Valid(got SSHOptions) error {
 	return want.match(got)
 }
 
-// sshCertificateDefaultValidator implements a simple validator for all the
-// fields in the SSH certificate.
-type sshCertificateDefaultValidator struct {
+type sshCertificateValidityValidator struct {
 	*Claimer
 }
 
-// Valid returns an error if the given certificate does not contain the necessary fields.
-func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error {
+func (v *sshCertificateValidityValidator) Valid(cert *ssh.Certificate) error {
 	var min, max time.Duration
 	switch cert.CertType {
 	case ssh.UserCert:
@@ -293,6 +290,30 @@ func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error {
 
 	duration := time.Duration(cert.ValidBefore - cert.ValidAfter)
 
+	switch {
+	case cert.ValidAfter == 0:
+		return errors.New("ssh certificate validAfter cannot be 0")
+	case cert.ValidBefore < uint64(now().Unix()):
+		return errors.New("ssh certificate validBefore cannot be in the past")
+	case cert.ValidBefore < cert.ValidAfter:
+		return errors.New("ssh certificate validBefore cannot be before validAfter")
+	case duration < min:
+		return errors.Errorf("requested duration of %s is less than minimum "+
+			"accepted duration for selected provisioner of %s", duration, min)
+	case duration > max:
+		return errors.Errorf("requested duration of %s is greater than maximum "+
+			"accepted duration for selected provisioner of %s", duration, max)
+	default:
+		return nil
+	}
+}
+
+// sshCertificateDefaultValidator implements a simple validator for all the
+// fields in the SSH certificate.
+type sshCertificateDefaultValidator struct{}
+
+// Valid returns an error if the given certificate does not contain the necessary fields.
+func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error {
 	switch {
 	case len(cert.Nonce) == 0:
 		return errors.New("ssh certificate nonce cannot be empty")
@@ -310,12 +331,6 @@ func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error {
 		return errors.New("ssh certificate validBefore cannot be in the past")
 	case cert.ValidBefore < cert.ValidAfter:
 		return errors.New("ssh certificate validBefore cannot be before validAfter")
-	case duration < min:
-		return errors.Errorf("requested duration of %s is less than minimum "+
-			"accepted duration for selected provisioner of %s", duration, min)
-	case duration > max:
-		return errors.Errorf("requested duration of %s is greater than maximum "+
-			"accepted duration for selected provisioner of %s", duration, max)
 	case cert.CertType == ssh.UserCert && len(cert.Extensions) == 0:
 		return errors.New("ssh certificate extensions cannot be empty")
 	case cert.SignatureKey == nil:
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index be5d42298..544b5625a 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -243,10 +243,12 @@ func (p *X5C) authorizeSSHSign(claims *x5cPayload) ([]SignOption, error) {
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Checks the validity bounds, and set the validity if has not been set.
-		&sshProvisioningCredTemporalModifier{p.claimer, rem},
+		&sshProvCredValidityModifier{p.claimer, rem},
 		// Validate public key.
 		&sshDefaultPublicKeyValidator{},
+		// Validate the validity period.
+		&sshCertificateValidityValidator{p.claimer},
 		// Require all the fields in the SSH certificate
-		&sshCertificateDefaultValidator{p.claimer},
+		&sshCertificateDefaultValidator{},
 	), nil
 }

From b02f41463ca9f28c0b20aa907e4310a1275f2f9b Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Tue, 1 Oct 2019 14:15:30 -0700
Subject: [PATCH 09/21] wip

---
 authority/provisioner/utils_test.go | 33 +++++++++++++++++++++++++++++
 authority/provisioner/x5c.go        | 27 +++++++++++++++++------
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go
index 2760a16b8..e685cece8 100644
--- a/authority/provisioner/utils_test.go
+++ b/authority/provisioner/utils_test.go
@@ -162,6 +162,39 @@ func generateJWK() (*JWK, error) {
 	}, nil
 }
 
+func generateX5C() (*JWK, error) {
+	name, err := randutil.Alphanumeric(10)
+	if err != nil {
+		return nil, err
+	}
+	jwk, err := generateJSONWebKey()
+	if err != nil {
+		return nil, err
+	}
+	jwe, err := encryptJSONWebKey(jwk)
+	if err != nil {
+		return nil, err
+	}
+	public := jwk.Public()
+	encrypted, err := jwe.CompactSerialize()
+	if err != nil {
+		return nil, err
+	}
+	claimer, err := NewClaimer(nil, globalProvisionerClaims)
+	if err != nil {
+		return nil, err
+	}
+	return &JWK{
+		Name:         name,
+		Type:         "JWK",
+		Key:          &public,
+		EncryptedKey: encrypted,
+		Claims:       &globalProvisionerClaims,
+		audiences:    testAudiences,
+		claimer:      claimer,
+	}, nil
+}
+
 func generateOIDC() (*OIDC, error) {
 	name, err := randutil.Alphanumeric(10)
 	if err != nil {
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index 544b5625a..b5e79ec8a 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -3,6 +3,7 @@ package provisioner
 import (
 	"context"
 	"crypto/x509"
+	"encoding/pem"
 	"time"
 
 	"github.com/pkg/errors"
@@ -23,7 +24,7 @@ type x5cPayload struct {
 type X5C struct {
 	Type      string  `json:"type"`
 	Name      string  `json:"name"`
-	Roots     string  `json:"roots"`
+	Roots     []byte  `json:"roots"`
 	Claims    *Claims `json:"claims,omitempty"`
 	claimer   *Claimer
 	audiences Audiences
@@ -70,28 +71,42 @@ func (p *X5C) GetEncryptedKey() (string, string, bool) {
 }
 
 // Init initializes and validates the fields of a X5C type.
-func (p *X5C) Init(config Config) (err error) {
+func (p *X5C) Init(config Config) error {
 	switch {
 	case p.Type == "":
 		return errors.New("provisioner type cannot be empty")
 	case p.Name == "":
 		return errors.New("provisioner name cannot be empty")
-	case p.Roots == "":
+	case p.Roots == nil || len(p.Roots) == 0:
 		return errors.New("provisioner root(s) cannot be empty")
 	}
 
 	p.rootPool = x509.NewCertPool()
-	if len(p.Roots) > 0 && !p.rootPool.AppendCertsFromPEM([]byte(p.Roots)) {
-		return errors.Errorf("error parsing root certificate(s) for provisioner '%s'", p.Name)
+
+	var (
+		block *pem.Block
+		rest  = p.Roots
+	)
+	for rest != nil {
+		block, rest = pem.Decode(rest)
+		if block == nil {
+			return errors.New("error parsing provisioner x5c roots")
+		}
+		cert, err := x509.ParseCertificate(block.Bytes)
+		if err != nil {
+			return errors.Wrap(err, "error parsing x509 certificate from PEM block")
+		}
+		p.rootPool.AddCert(cert)
 	}
 
 	// Update claims with global ones
+	var err error
 	if p.claimer, err = NewClaimer(p.Claims, config.Claims); err != nil {
 		return err
 	}
 
 	p.audiences = config.Audiences.WithFragment(p.GetID())
-	return err
+	return nil
 }
 
 // authorizeToken performs common jwt authorization actions and returns the

From 40481f0b713893122d03b8140a89d39abc816925 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Tue, 1 Oct 2019 21:22:37 -0700
Subject: [PATCH 10/21] wip

---
 authority/provisioner/acme.go               |   2 +-
 authority/provisioner/aws.go                |   2 +-
 authority/provisioner/azure.go              |   2 +-
 authority/provisioner/gcp.go                |   2 +-
 authority/provisioner/jwk.go                |   2 +-
 authority/provisioner/oidc.go               |   2 +-
 authority/provisioner/sign_options.go       |  12 +-
 authority/provisioner/testdata/x5c-leaf.crt |  24 +
 authority/provisioner/testdata/x5c-leaf.key |   5 +
 authority/provisioner/utils_test.go         | 114 +++--
 authority/provisioner/x5c.go                |   9 +-
 authority/provisioner/x5c_test.go           | 481 ++++++++++++++++++++
 12 files changed, 616 insertions(+), 41 deletions(-)
 create mode 100644 authority/provisioner/testdata/x5c-leaf.crt
 create mode 100644 authority/provisioner/testdata/x5c-leaf.key
 create mode 100644 authority/provisioner/x5c_test.go

diff --git a/authority/provisioner/acme.go b/authority/provisioner/acme.go
index e26b5165e..d1933d475 100644
--- a/authority/provisioner/acme.go
+++ b/authority/provisioner/acme.go
@@ -74,7 +74,7 @@ func (p *ACME) AuthorizeSign(ctx context.Context, _ string) ([]SignOption, error
 		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
 		// validators
 		defaultPublicKeyValidator{},
-		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	}, nil
 }
 
diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go
index a54013cb0..9352de7d8 100644
--- a/authority/provisioner/aws.go
+++ b/authority/provisioner/aws.go
@@ -302,7 +302,7 @@ func (p *AWS) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 		// validators
 		defaultPublicKeyValidator{},
 		commonNameValidator(payload.Claims.Subject),
-		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	), nil
 }
 
diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go
index abfe900f9..c1b48b99d 100644
--- a/authority/provisioner/azure.go
+++ b/authority/provisioner/azure.go
@@ -289,7 +289,7 @@ func (p *Azure) AuthorizeSign(ctx context.Context, token string) ([]SignOption,
 		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
 		// validators
 		defaultPublicKeyValidator{},
-		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	), nil
 }
 
diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go
index 939030934..6f0627bfa 100644
--- a/authority/provisioner/gcp.go
+++ b/authority/provisioner/gcp.go
@@ -242,7 +242,7 @@ func (p *GCP) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
 		// validators
 		defaultPublicKeyValidator{},
-		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	), nil
 }
 
diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go
index ed54d3cfa..f3d8d95e4 100644
--- a/authority/provisioner/jwk.go
+++ b/authority/provisioner/jwk.go
@@ -167,7 +167,7 @@ func (p *JWK) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 		dnsNamesValidator(dnsNames),
 		emailAddressesValidator(emails),
 		ipAddressesValidator(ips),
-		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	}, nil
 }
 
diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go
index 58be12011..822ce083a 100644
--- a/authority/provisioner/oidc.go
+++ b/authority/provisioner/oidc.go
@@ -298,7 +298,7 @@ func (o *OIDC) AuthorizeSign(ctx context.Context, token string) ([]SignOption, e
 		profileDefaultDuration(o.claimer.DefaultTLSCertDuration()),
 		// validators
 		newProvisionerExtensionOption(TypeOIDC, o.Name, o.ClientID),
-		newTemporalValidator(o.claimer.MinTLSCertDuration(), o.claimer.MaxTLSCertDuration()),
+		newValidityValidator(o.claimer.MinTLSCertDuration(), o.claimer.MaxTLSCertDuration()),
 	}
 	// Admins should be able to authorize any SAN
 	if o.IsAdmin(claims.Email) {
diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go
index a7d222083..4d20bad1f 100644
--- a/authority/provisioner/sign_options.go
+++ b/authority/provisioner/sign_options.go
@@ -211,20 +211,20 @@ func (v profileDefaultDuration) Option(so Options) x509util.WithOption {
 	return x509util.WithNotBeforeAfterDuration(notBefore, notAfter, time.Duration(v))
 }
 
-// temporalValidator validates the certificate temporal validity settings.
-type temporalValidator struct {
+// validityValidator validates the certificate temporal validity settings.
+type validityValidator struct {
 	min time.Duration
 	max time.Duration
 }
 
-// newTemporalValidator return a new validity validator.
-func newTemporalValidator(min, max time.Duration) *temporalValidator {
-	return &temporalValidator{min: min, max: max}
+// newValidityValidator return a new validity validator.
+func newValidityValidator(min, max time.Duration) *validityValidator {
+	return &validityValidator{min: min, max: max}
 }
 
 // Validate validates the certificate temporal settings (notBefore/notAfter)
 // and total duration.
-func (v *temporalValidator) Valid(crt *x509.Certificate) error {
+func (v *validityValidator) Valid(crt *x509.Certificate) error {
 	var (
 		na  = crt.NotAfter
 		nb  = crt.NotBefore
diff --git a/authority/provisioner/testdata/x5c-leaf.crt b/authority/provisioner/testdata/x5c-leaf.crt
new file mode 100644
index 000000000..2d674c117
--- /dev/null
+++ b/authority/provisioner/testdata/x5c-leaf.crt
@@ -0,0 +1,24 @@
+-----BEGIN CERTIFICATE-----
+MIIBuDCCAV+gAwIBAgIQFdu723gqgGaTaqjf6ny88zAKBggqhkjOPQQDAjAcMRow
+GAYDVQQDExFpbnRlcm1lZGlhdGUtdGVzdDAgFw0xOTEwMDIwMzE4NTNaGA8yMTE5
+MDkwODAzMTg1MVowFDESMBAGA1UEAxMJbGVhZi10ZXN0MFkwEwYHKoZIzj0CAQYI
+KoZIzj0DAQcDQgAEaV6807GhWEtMxA39zjuMVHAiN2/Ri5B1R1s+Y/8mlrKIvuvr
+VpgSPXYruNRFduPWX564Abz/TDmb276JbKGeQqOBiDCBhTAOBgNVHQ8BAf8EBAMC
+BaAwHQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMB0GA1UdDgQWBBReMkPW
+f4MNWdg7KN4xI4ZLJd0IJDAfBgNVHSMEGDAWgBSckDGJlzLaJsdy698XH32gPDMp
+czAUBgNVHREEDTALgglsZWFmLXRlc3QwCgYIKoZIzj0EAwIDRwAwRAIgKYLKXpTN
+wtvZZaIvDzq1p8MO/SZ8yI42Ot69dNk/QtkCIBSvg5PozYcfbvwkgX5SwsjfYu0Z
+AvUgkUQ2G25NBRmX
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIBtjCCAVygAwIBAgIQNr+f4IkABY2n4wx4sLOMrTAKBggqhkjOPQQDAjAUMRIw
+EAYDVQQDEwlyb290LXRlc3QwIBcNMTkxMDAyMDI0MDM0WhgPMjExOTA5MDgwMjQw
+MzJaMBwxGjAYBgNVBAMTEWludGVybWVkaWF0ZS10ZXN0MFkwEwYHKoZIzj0CAQYI
+KoZIzj0DAQcDQgAEflfRhPjgJXv4zsPWahXjM2UU61aRFErN0iw88ZPyxea22fxl
+qN9ezntTXxzsS+mZiWapl8B40ACJgvP+WLQBHKOBhTCBgjAOBgNVHQ8BAf8EBAMC
+AQYwEgYDVR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQUnJAxiZcy2ibHcuvfFx99
+oDwzKXMwHwYDVR0jBBgwFoAUpHS7FfaQ5bCrTxUeu6R2ZC3VGOowHAYDVR0RBBUw
+E4IRaW50ZXJtZWRpYXRlLXRlc3QwCgYIKoZIzj0EAwIDSAAwRQIgII8XpQ8ezDO1
+2xdq3hShf155C5X/5jO8qr0VyEJgzlkCIQCTqph1Gwu/dmuf6dYLCfQqJyb371LC
+lgsqsR63is+0YQ==
+-----END CERTIFICATE-----
diff --git a/authority/provisioner/testdata/x5c-leaf.key b/authority/provisioner/testdata/x5c-leaf.key
new file mode 100644
index 000000000..f77d015e8
--- /dev/null
+++ b/authority/provisioner/testdata/x5c-leaf.key
@@ -0,0 +1,5 @@
+-----BEGIN EC PRIVATE KEY-----
+MHcCAQEEIALytC4LyTTAagMLMv+rzq2vtfhFkhuyBz4kqsnRs6zioAoGCCqGSM49
+AwEHoUQDQgAEaV6807GhWEtMxA39zjuMVHAiN2/Ri5B1R1s+Y/8mlrKIvuvrVpgS
+PXYruNRFduPWX564Abz/TDmb276JbKGeQg==
+-----END EC PRIVATE KEY-----
diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go
index e685cece8..38bd08e8d 100644
--- a/authority/provisioner/utils_test.go
+++ b/authority/provisioner/utils_test.go
@@ -162,21 +162,22 @@ func generateJWK() (*JWK, error) {
 	}, nil
 }
 
-func generateX5C() (*JWK, error) {
-	name, err := randutil.Alphanumeric(10)
-	if err != nil {
-		return nil, err
-	}
-	jwk, err := generateJSONWebKey()
-	if err != nil {
-		return nil, err
+func generateX5C(root []byte) (*X5C, error) {
+	if root == nil {
+		root = []byte(`-----BEGIN CERTIFICATE-----
+MIIBhTCCASqgAwIBAgIRAMalM7pKi0GCdKjO6u88OyowCgYIKoZIzj0EAwIwFDES
+MBAGA1UEAxMJcm9vdC10ZXN0MCAXDTE5MTAwMjAyMzk0OFoYDzIxMTkwOTA4MDIz
+OTQ4WjAUMRIwEAYDVQQDEwlyb290LXRlc3QwWTATBgcqhkjOPQIBBggqhkjOPQMB
+BwNCAAS29QTCXUu7cx9sa9wZPpRSFq/zXaw8Ai3EIygayrBsKnX42U2atBUjcBZO
+BWL6A+PpLzU9ja867U5SYNHERS+Oo1swWTAOBgNVHQ8BAf8EBAMCAQYwEgYDVR0T
+AQH/BAgwBgEB/wIBATAdBgNVHQ4EFgQUpHS7FfaQ5bCrTxUeu6R2ZC3VGOowFAYD
+VR0RBA0wC4IJcm9vdC10ZXN0MAoGCCqGSM49BAMCA0kAMEYCIQC2vgqwla0u8LHH
+1MHob14qvS5o76HautbIBW7fcHzz5gIhAIx5A2+wkJYX4026kqaZCk/1sAwTxSGY
+M46l92gdOozT
+-----END CERTIFICATE-----`)
 	}
-	jwe, err := encryptJSONWebKey(jwk)
-	if err != nil {
-		return nil, err
-	}
-	public := jwk.Public()
-	encrypted, err := jwe.CompactSerialize()
+
+	name, err := randutil.Alphanumeric(10)
 	if err != nil {
 		return nil, err
 	}
@@ -184,14 +185,32 @@ func generateX5C() (*JWK, error) {
 	if err != nil {
 		return nil, err
 	}
-	return &JWK{
-		Name:         name,
-		Type:         "JWK",
-		Key:          &public,
-		EncryptedKey: encrypted,
-		Claims:       &globalProvisionerClaims,
-		audiences:    testAudiences,
-		claimer:      claimer,
+
+	rootPool := x509.NewCertPool()
+
+	var (
+		block *pem.Block
+		rest  = root
+	)
+	for rest != nil {
+		block, rest = pem.Decode(rest)
+		if block == nil {
+			break
+		}
+		cert, err := x509.ParseCertificate(block.Bytes)
+		if err != nil {
+			return nil, errors.Wrap(err, "error parsing x509 certificate from PEM block")
+		}
+		rootPool.AddCert(cert)
+	}
+	return &X5C{
+		Name:      name,
+		Type:      "X5C",
+		Roots:     root,
+		Claims:    &globalProvisionerClaims,
+		audiences: testAudiences,
+		claimer:   claimer,
+		rootPool:  rootPool,
 	}, nil
 }
 
@@ -479,11 +498,31 @@ func generateSimpleToken(iss, aud string, jwk *jose.JSONWebKey) (string, error)
 	return generateToken("subject", iss, aud, "name@smallstep.com", []string{"test.smallstep.com"}, time.Now(), jwk)
 }
 
-func generateToken(sub, iss, aud string, email string, sans []string, iat time.Time, jwk *jose.JSONWebKey) (string, error) {
-	sig, err := jose.NewSigner(
-		jose.SigningKey{Algorithm: jose.ES256, Key: jwk.Key},
-		new(jose.SignerOptions).WithType("JWT").WithHeader("kid", jwk.KeyID),
-	)
+type tokOption func(*jose.SignerOptions) error
+
+func withX5CHdr(certs []*x509.Certificate) tokOption {
+	return func(so *jose.SignerOptions) error {
+		strs := make([]string, len(certs))
+		for i, cert := range certs {
+			strs[i] = base64.StdEncoding.EncodeToString(cert.Raw)
+		}
+		so.WithHeader("x5c", strs)
+		return nil
+	}
+}
+
+func generateToken(sub, iss, aud string, email string, sans []string, iat time.Time, jwk *jose.JSONWebKey, tokOpts ...tokOption) (string, error) {
+	so := new(jose.SignerOptions)
+	so.WithType("JWT")
+	so.WithHeader("kid", jwk.KeyID)
+
+	for _, o := range tokOpts {
+		if err := o(so); err != nil {
+			return "", err
+		}
+	}
+
+	sig, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.ES256, Key: jwk.Key}, so)
 	if err != nil {
 		return "", err
 	}
@@ -775,3 +814,24 @@ func generateACME() (*ACME, error) {
 	}
 	return p, nil
 }
+
+func parseCerts(b []byte) ([]*x509.Certificate, error) {
+	var (
+		block *pem.Block
+		rest  = b
+		certs = []*x509.Certificate{}
+	)
+	for rest != nil {
+		block, rest = pem.Decode(rest)
+		if block == nil {
+			break
+		}
+		cert, err := x509.ParseCertificate(block.Bytes)
+		if err != nil {
+			return nil, errors.Wrap(err, "error parsing x509 certificate from PEM block")
+		}
+
+		certs = append(certs, cert)
+	}
+	return certs, nil
+}
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index b5e79ec8a..6804a1972 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -90,7 +90,7 @@ func (p *X5C) Init(config Config) error {
 	for rest != nil {
 		block, rest = pem.Decode(rest)
 		if block == nil {
-			return errors.New("error parsing provisioner x5c roots")
+			break
 		}
 		cert, err := x509.ParseCertificate(block.Bytes)
 		if err != nil {
@@ -99,6 +99,11 @@ func (p *X5C) Init(config Config) error {
 		p.rootPool.AddCert(cert)
 	}
 
+	// Verify that at least one root was found.
+	if len(p.rootPool.Subjects()) == 0 {
+		return errors.Errorf("no x509 certificates found in roots attribute for provisioner %s", p.GetName())
+	}
+
 	// Update claims with global ones
 	var err error
 	if p.claimer, err = NewClaimer(p.Claims, config.Claims); err != nil {
@@ -209,7 +214,7 @@ func (p *X5C) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 		dnsNamesValidator(dnsNames),
 		emailAddressesValidator(emails),
 		ipAddressesValidator(ips),
-		newTemporalValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
+		newValidityValidator(p.claimer.MinTLSCertDuration(), p.claimer.MaxTLSCertDuration()),
 	}, nil
 }
 
diff --git a/authority/provisioner/x5c_test.go b/authority/provisioner/x5c_test.go
new file mode 100644
index 000000000..0d7315f6d
--- /dev/null
+++ b/authority/provisioner/x5c_test.go
@@ -0,0 +1,481 @@
+package provisioner
+
+import (
+	"crypto/x509"
+	"testing"
+	"time"
+
+	"github.com/pkg/errors"
+	"github.com/smallstep/assert"
+	"github.com/smallstep/cli/crypto/pemutil"
+	"github.com/smallstep/cli/jose"
+)
+
+func TestX5C_Getters(t *testing.T) {
+	p, err := generateX5C(nil)
+	assert.FatalError(t, err)
+	id := "x5c/" + p.Name
+	if got := p.GetID(); got != id {
+		t.Errorf("X5C.GetID() = %v, want %v:%v", got, p.Name, id)
+	}
+	if got := p.GetName(); got != p.Name {
+		t.Errorf("X5C.GetName() = %v, want %v", got, p.Name)
+	}
+	if got := p.GetType(); got != TypeX5C {
+		t.Errorf("X5C.GetType() = %v, want %v", got, TypeX5C)
+	}
+	kid, key, ok := p.GetEncryptedKey()
+	if kid != "" || key != "" || ok == true {
+		t.Errorf("X5C.GetEncryptedKey() = (%v, %v, %v), want (%v, %v, %v)",
+			kid, key, ok, "", "", false)
+	}
+}
+
+func TestX5C_Init(t *testing.T) {
+	type ProvisionerValidateTest struct {
+		p          *X5C
+		err        error
+		extraValid func(*X5C) error
+	}
+	tests := map[string]func(*testing.T) ProvisionerValidateTest{
+		"fail/empty": func(t *testing.T) ProvisionerValidateTest {
+			return ProvisionerValidateTest{
+				p:   &X5C{},
+				err: errors.New("provisioner type cannot be empty"),
+			}
+		},
+		"fail/empty-name": func(t *testing.T) ProvisionerValidateTest {
+			return ProvisionerValidateTest{
+				p: &X5C{
+					Type: "X5C",
+				},
+				err: errors.New("provisioner name cannot be empty"),
+			}
+		},
+		"fail/empty-type": func(t *testing.T) ProvisionerValidateTest {
+			return ProvisionerValidateTest{
+				p:   &X5C{Name: "foo"},
+				err: errors.New("provisioner type cannot be empty"),
+			}
+		},
+		"fail/empty-key": func(t *testing.T) ProvisionerValidateTest {
+			return ProvisionerValidateTest{
+				p:   &X5C{Name: "foo", Type: "bar"},
+				err: errors.New("provisioner root(s) cannot be empty"),
+			}
+		},
+		"fail/no-valid-root-certs": func(t *testing.T) ProvisionerValidateTest {
+			return ProvisionerValidateTest{
+				p:   &X5C{Name: "foo", Type: "bar", Roots: []byte("foo"), audiences: testAudiences},
+				err: errors.Errorf("no x509 certificates found in roots attribute for provisioner foo"),
+			}
+		},
+		"fail/invalid-duration": func(t *testing.T) ProvisionerValidateTest {
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			p.Claims = &Claims{DefaultTLSDur: &Duration{0}}
+			return ProvisionerValidateTest{
+				p:   p,
+				err: errors.New("claims: DefaultTLSCertDuration must be greater than 0"),
+			}
+		},
+		"ok": func(t *testing.T) ProvisionerValidateTest {
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			return ProvisionerValidateTest{
+				p: p,
+			}
+		},
+		"ok/root-chain": func(t *testing.T) ProvisionerValidateTest {
+			p, err := generateX5C([]byte(`-----BEGIN CERTIFICATE-----
+MIIBtjCCAVygAwIBAgIQNr+f4IkABY2n4wx4sLOMrTAKBggqhkjOPQQDAjAUMRIw
+EAYDVQQDEwlyb290LXRlc3QwIBcNMTkxMDAyMDI0MDM0WhgPMjExOTA5MDgwMjQw
+MzJaMBwxGjAYBgNVBAMTEWludGVybWVkaWF0ZS10ZXN0MFkwEwYHKoZIzj0CAQYI
+KoZIzj0DAQcDQgAEflfRhPjgJXv4zsPWahXjM2UU61aRFErN0iw88ZPyxea22fxl
+qN9ezntTXxzsS+mZiWapl8B40ACJgvP+WLQBHKOBhTCBgjAOBgNVHQ8BAf8EBAMC
+AQYwEgYDVR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQUnJAxiZcy2ibHcuvfFx99
+oDwzKXMwHwYDVR0jBBgwFoAUpHS7FfaQ5bCrTxUeu6R2ZC3VGOowHAYDVR0RBBUw
+E4IRaW50ZXJtZWRpYXRlLXRlc3QwCgYIKoZIzj0EAwIDSAAwRQIgII8XpQ8ezDO1
+2xdq3hShf155C5X/5jO8qr0VyEJgzlkCIQCTqph1Gwu/dmuf6dYLCfQqJyb371LC
+lgsqsR63is+0YQ==
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIBhTCCASqgAwIBAgIRAMalM7pKi0GCdKjO6u88OyowCgYIKoZIzj0EAwIwFDES
+MBAGA1UEAxMJcm9vdC10ZXN0MCAXDTE5MTAwMjAyMzk0OFoYDzIxMTkwOTA4MDIz
+OTQ4WjAUMRIwEAYDVQQDEwlyb290LXRlc3QwWTATBgcqhkjOPQIBBggqhkjOPQMB
+BwNCAAS29QTCXUu7cx9sa9wZPpRSFq/zXaw8Ai3EIygayrBsKnX42U2atBUjcBZO
+BWL6A+PpLzU9ja867U5SYNHERS+Oo1swWTAOBgNVHQ8BAf8EBAMCAQYwEgYDVR0T
+AQH/BAgwBgEB/wIBATAdBgNVHQ4EFgQUpHS7FfaQ5bCrTxUeu6R2ZC3VGOowFAYD
+VR0RBA0wC4IJcm9vdC10ZXN0MAoGCCqGSM49BAMCA0kAMEYCIQC2vgqwla0u8LHH
+1MHob14qvS5o76HautbIBW7fcHzz5gIhAIx5A2+wkJYX4026kqaZCk/1sAwTxSGY
+M46l92gdOozT
+-----END CERTIFICATE-----`))
+			assert.FatalError(t, err)
+			return ProvisionerValidateTest{
+				p: p,
+				extraValid: func(p *X5C) error {
+					numCerts := len(p.rootPool.Subjects())
+					if numCerts != 2 {
+						return errors.Errorf("unexpected number of certs: want 2, but got %d", numCerts)
+					}
+					return nil
+				},
+			}
+		},
+	}
+
+	config := Config{
+		Claims:    globalProvisionerClaims,
+		Audiences: testAudiences,
+	}
+	for name, get := range tests {
+		t.Run(name, func(t *testing.T) {
+			tc := get(t)
+			err := tc.p.Init(config)
+			if err != nil {
+				if assert.NotNil(t, tc.err) {
+					assert.Equals(t, tc.err.Error(), err.Error())
+				}
+			} else {
+				if assert.Nil(t, tc.err) {
+					assert.Equals(t, tc.p.audiences, config.Audiences.WithFragment(tc.p.GetID()))
+					if tc.extraValid != nil {
+						assert.Nil(t, tc.extraValid(tc.p))
+					}
+				}
+			}
+		})
+	}
+}
+
+func TestX5C_authorizeToken(t *testing.T) {
+	type test struct {
+		p     *X5C
+		token string
+		err   error
+	}
+	tests := map[string]func(*testing.T) test{
+		"fail/bad-token": func(t *testing.T) test {
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: "foo",
+				err:   errors.New("error parsing token"),
+			}
+		},
+		"fail/invalid-cert-chain": func(t *testing.T) test {
+			certs, err := parseCerts([]byte(`-----BEGIN CERTIFICATE-----
+MIIBpTCCAUugAwIBAgIRAOn2LHXjYyTXQ7PNjDTSKiIwCgYIKoZIzj0EAwIwHDEa
+MBgGA1UEAxMRU21hbGxzdGVwIFJvb3QgQ0EwHhcNMTkwOTE0MDk1NTM2WhcNMjkw
+OTExMDk1NTM2WjAkMSIwIAYDVQQDExlTbWFsbHN0ZXAgSW50ZXJtZWRpYXRlIENB
+MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE2Cs0TY0dLM4b2s+z8+cc3JJp/W5H
+zQRvICX/1aJ4MuObNLcvoSguJwJEkYpGB5fhb0KvoL+ebHfEOywGNwrWkaNmMGQw
+DgYDVR0PAQH/BAQDAgEGMBIGA1UdEwEB/wQIMAYBAf8CAQAwHQYDVR0OBBYEFNLJ
+4ZXoX9cI6YkGPxgs2US3ssVzMB8GA1UdIwQYMBaAFGIwpqz85wL29aF47Vj9XSVM
+P9K7MAoGCCqGSM49BAMCA0gAMEUCIQC5c1ldDcesDb31GlO5cEJvOcRrIrNtkk8m
+a5wpg+9s6QIgHIW6L60F8klQX+EO3o0SBqLeNcaskA4oSZsKjEdpSGo=
+-----END CERTIFICATE-----`))
+			assert.FatalError(t, err)
+			jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
+			assert.FatalError(t, err)
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("", p.Name, testAudiences.Sign[0], "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: tok,
+				err:   errors.New("error verifying x5c certificate chain: x509: certificate signed by unknown authority"),
+			}
+		},
+		"fail/doubled-up-self-signed-cert": func(t *testing.T) test {
+			certs, err := parseCerts([]byte(`-----BEGIN CERTIFICATE-----
+MIIBgjCCASigAwIBAgIQIZiE9wpmSj6SMMDfHD17qjAKBggqhkjOPQQDAjAQMQ4w
+DAYDVQQDEwVsZWFmMjAgFw0xOTEwMDIwMzEzNTlaGA8yMTE5MDkwODAzMTM1OVow
+EDEOMAwGA1UEAxMFbGVhZjIwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATuajJI
+3YgDaj+jorioJzGJc2+V1hUM7XzN9tIHoUeItgny9GW08TrTc23h1cCZteNZvayG
+M0wGpGeXOnE4IlH9o2IwYDAOBgNVHQ8BAf8EBAMCBSAwHQYDVR0lBBYwFAYIKwYB
+BQUHAwEGCCsGAQUFBwMCMB0GA1UdDgQWBBT99+JChTh3LWOHaqlSwNiwND18/zAQ
+BgNVHREECTAHggVsZWFmMjAKBggqhkjOPQQDAgNIADBFAiB7gMRy3t81HpcnoRAS
+ELZmDFaEnoLCsVfbmanFykazQQIhAI0sZjoE9t6gvzQp7XQp6CoxzCc3Jv3FwZ8G
+EXAHTA9L
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIBgjCCASigAwIBAgIQIZiE9wpmSj6SMMDfHD17qjAKBggqhkjOPQQDAjAQMQ4w
+DAYDVQQDEwVsZWFmMjAgFw0xOTEwMDIwMzEzNTlaGA8yMTE5MDkwODAzMTM1OVow
+EDEOMAwGA1UEAxMFbGVhZjIwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATuajJI
+3YgDaj+jorioJzGJc2+V1hUM7XzN9tIHoUeItgny9GW08TrTc23h1cCZteNZvayG
+M0wGpGeXOnE4IlH9o2IwYDAOBgNVHQ8BAf8EBAMCBSAwHQYDVR0lBBYwFAYIKwYB
+BQUHAwEGCCsGAQUFBwMCMB0GA1UdDgQWBBT99+JChTh3LWOHaqlSwNiwND18/zAQ
+BgNVHREECTAHggVsZWFmMjAKBggqhkjOPQQDAgNIADBFAiB7gMRy3t81HpcnoRAS
+ELZmDFaEnoLCsVfbmanFykazQQIhAI0sZjoE9t6gvzQp7XQp6CoxzCc3Jv3FwZ8G
+EXAHTA9L
+-----END CERTIFICATE-----`))
+			assert.FatalError(t, err)
+			jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
+			assert.FatalError(t, err)
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("", p.Name, testAudiences.Sign[0], "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: tok,
+				err:   errors.New("error verifying x5c certificate chain: x509: certificate signed by unknown authority"),
+			}
+		},
+		"fail/digital-signature-ext-required": func(t *testing.T) test {
+			certs, err := parseCerts([]byte(`-----BEGIN CERTIFICATE-----
+MIIBuTCCAV+gAwIBAgIQeRJLdDMIdn/T2ORKxYABezAKBggqhkjOPQQDAjAcMRow
+GAYDVQQDExFpbnRlcm1lZGlhdGUtdGVzdDAgFw0xOTEwMDIwMjQxMTRaGA8yMTE5
+MDkwODAyNDExMlowFDESMBAGA1UEAxMJbGVhZi10ZXN0MFkwEwYHKoZIzj0CAQYI
+KoZIzj0DAQcDQgAEDA1nGTOujobkcBWklyvymhWE5gQlvNLarVzhhhvPDw+MK2LX
+yqkXrYZM10GrwQZuQ7ykHnjz00U/KXpPRQ7+0qOBiDCBhTAOBgNVHQ8BAf8EBAMC
+BSAwHQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMB0GA1UdDgQWBBQYv0AK
+3GUOvC+m8ZTfyhn7tKQOazAfBgNVHSMEGDAWgBSckDGJlzLaJsdy698XH32gPDMp
+czAUBgNVHREEDTALgglsZWFmLXRlc3QwCgYIKoZIzj0EAwIDSAAwRQIhAPmertx0
+lchRU3kAu647exvlhEr1xosPOu6P8kVYbtTEAiAA51w9EYIT/Zb26M3eQV817T2g
+Dnhl0ElPQsA92pkqbA==
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIBtjCCAVygAwIBAgIQNr+f4IkABY2n4wx4sLOMrTAKBggqhkjOPQQDAjAUMRIw
+EAYDVQQDEwlyb290LXRlc3QwIBcNMTkxMDAyMDI0MDM0WhgPMjExOTA5MDgwMjQw
+MzJaMBwxGjAYBgNVBAMTEWludGVybWVkaWF0ZS10ZXN0MFkwEwYHKoZIzj0CAQYI
+KoZIzj0DAQcDQgAEflfRhPjgJXv4zsPWahXjM2UU61aRFErN0iw88ZPyxea22fxl
+qN9ezntTXxzsS+mZiWapl8B40ACJgvP+WLQBHKOBhTCBgjAOBgNVHQ8BAf8EBAMC
+AQYwEgYDVR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQUnJAxiZcy2ibHcuvfFx99
+oDwzKXMwHwYDVR0jBBgwFoAUpHS7FfaQ5bCrTxUeu6R2ZC3VGOowHAYDVR0RBBUw
+E4IRaW50ZXJtZWRpYXRlLXRlc3QwCgYIKoZIzj0EAwIDSAAwRQIgII8XpQ8ezDO1
+2xdq3hShf155C5X/5jO8qr0VyEJgzlkCIQCTqph1Gwu/dmuf6dYLCfQqJyb371LC
+lgsqsR63is+0YQ==
+-----END CERTIFICATE-----`))
+			assert.FatalError(t, err)
+			jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
+			assert.FatalError(t, err)
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+
+			tok, err := generateToken("", p.Name, testAudiences.Sign[0], "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: tok,
+				err:   errors.New("certificate used to sign x5c token cannot be used for digital signature"),
+			}
+		},
+		"fail/signature-does-not-match-x5c-pub-key": func(t *testing.T) test {
+			certs, err := parseCerts([]byte(`-----BEGIN CERTIFICATE-----
+MIIBuDCCAV+gAwIBAgIQFdu723gqgGaTaqjf6ny88zAKBggqhkjOPQQDAjAcMRow
+GAYDVQQDExFpbnRlcm1lZGlhdGUtdGVzdDAgFw0xOTEwMDIwMzE4NTNaGA8yMTE5
+MDkwODAzMTg1MVowFDESMBAGA1UEAxMJbGVhZi10ZXN0MFkwEwYHKoZIzj0CAQYI
+KoZIzj0DAQcDQgAEaV6807GhWEtMxA39zjuMVHAiN2/Ri5B1R1s+Y/8mlrKIvuvr
+VpgSPXYruNRFduPWX564Abz/TDmb276JbKGeQqOBiDCBhTAOBgNVHQ8BAf8EBAMC
+BaAwHQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMB0GA1UdDgQWBBReMkPW
+f4MNWdg7KN4xI4ZLJd0IJDAfBgNVHSMEGDAWgBSckDGJlzLaJsdy698XH32gPDMp
+czAUBgNVHREEDTALgglsZWFmLXRlc3QwCgYIKoZIzj0EAwIDRwAwRAIgKYLKXpTN
+wtvZZaIvDzq1p8MO/SZ8yI42Ot69dNk/QtkCIBSvg5PozYcfbvwkgX5SwsjfYu0Z
+AvUgkUQ2G25NBRmX
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIBtjCCAVygAwIBAgIQNr+f4IkABY2n4wx4sLOMrTAKBggqhkjOPQQDAjAUMRIw
+EAYDVQQDEwlyb290LXRlc3QwIBcNMTkxMDAyMDI0MDM0WhgPMjExOTA5MDgwMjQw
+MzJaMBwxGjAYBgNVBAMTEWludGVybWVkaWF0ZS10ZXN0MFkwEwYHKoZIzj0CAQYI
+KoZIzj0DAQcDQgAEflfRhPjgJXv4zsPWahXjM2UU61aRFErN0iw88ZPyxea22fxl
+qN9ezntTXxzsS+mZiWapl8B40ACJgvP+WLQBHKOBhTCBgjAOBgNVHQ8BAf8EBAMC
+AQYwEgYDVR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQUnJAxiZcy2ibHcuvfFx99
+oDwzKXMwHwYDVR0jBBgwFoAUpHS7FfaQ5bCrTxUeu6R2ZC3VGOowHAYDVR0RBBUw
+E4IRaW50ZXJtZWRpYXRlLXRlc3QwCgYIKoZIzj0EAwIDSAAwRQIgII8XpQ8ezDO1
+2xdq3hShf155C5X/5jO8qr0VyEJgzlkCIQCTqph1Gwu/dmuf6dYLCfQqJyb371LC
+lgsqsR63is+0YQ==
+-----END CERTIFICATE-----`))
+			assert.FatalError(t, err)
+			jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
+			assert.FatalError(t, err)
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("", "foobar", testAudiences.Sign[0], "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: tok,
+				err:   errors.New("error parsing claims: square/go-jose: error in cryptographic primitive"),
+			}
+		},
+		"fail/invalid-issuer": func(t *testing.T) test {
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			jwk, err := jose.ParseKey("./testdata/x5c-leaf.key")
+			assert.FatalError(t, err)
+
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("", "foobar", testAudiences.Sign[0], "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: tok,
+				err:   errors.New("invalid token: square/go-jose/jwt: validation failed, invalid issuer claim (iss)"),
+			}
+		},
+		"fail/invalid-audience": func(t *testing.T) test {
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			jwk, err := jose.ParseKey("./testdata/x5c-leaf.key")
+			assert.FatalError(t, err)
+
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("", p.GetName(), "foobar", "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: tok,
+				err:   errors.New("invalid token: invalid audience claim (aud)"),
+			}
+		},
+		"fail/empty-subject": func(t *testing.T) test {
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			jwk, err := jose.ParseKey("./testdata/x5c-leaf.key")
+			assert.FatalError(t, err)
+
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("", p.GetName(), testAudiences.Sign[0], "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: tok,
+				err:   errors.New("token subject cannot be empty"),
+			}
+		},
+		"ok": func(t *testing.T) test {
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			jwk, err := jose.ParseKey("./testdata/x5c-leaf.key")
+			assert.FatalError(t, err)
+
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("foo", p.GetName(), testAudiences.Sign[0], "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: tok,
+			}
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			tc := tt(t)
+			if claims, err := tc.p.authorizeToken(tc.token, testAudiences.Sign); err != nil {
+				if assert.NotNil(t, tc.err) {
+					assert.HasPrefix(t, err.Error(), tc.err.Error())
+				}
+			} else {
+				if assert.Nil(t, tc.err) {
+					assert.NotNil(t, claims)
+					assert.NotNil(t, claims.chains)
+				}
+			}
+		})
+	}
+}
+
+func TestX5C_AuthorizeReovke(t *testing.T) {
+	type test struct {
+		p     *X5C
+		token string
+		err   error
+	}
+	tests := map[string]func(*testing.T) test{
+		"fail/invalid-token": func(t *testing.T) test {
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: "foo",
+				err:   errors.New("error parsing token"),
+			}
+		},
+		"ok": func(t *testing.T) test {
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			jwk, err := jose.ParseKey("./testdata/x5c-leaf.key")
+			assert.FatalError(t, err)
+
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("foo", p.GetName(), testAudiences.Revoke[0], "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: tok,
+			}
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			tc := tt(t)
+			if err := tc.p.AuthorizeRevoke(tc.token); err != nil {
+				if assert.NotNil(t, tc.err) {
+					assert.HasPrefix(t, err.Error(), tc.err.Error())
+				}
+			} else {
+				assert.Nil(t, tc.err)
+			}
+		})
+	}
+}
+
+func TestX5C_AuthorizeRenewal(t *testing.T) {
+	p1, err := generateX5C(nil)
+	assert.FatalError(t, err)
+	p2, err := generateX5C(nil)
+	assert.FatalError(t, err)
+
+	// disable renewal
+	disable := true
+	p2.Claims = &Claims{DisableRenewal: &disable}
+	p2.claimer, err = NewClaimer(p2.Claims, globalProvisionerClaims)
+	assert.FatalError(t, err)
+
+	type args struct {
+		cert *x509.Certificate
+	}
+	tests := []struct {
+		name    string
+		prov    *X5C
+		args    args
+		wantErr bool
+	}{
+		{"ok", p1, args{nil}, false},
+		{"fail", p2, args{nil}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := tt.prov.AuthorizeRenewal(tt.args.cert); (err != nil) != tt.wantErr {
+				t.Errorf("X5C.AuthorizeRenewal() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}

From 8629ef267709325fdb132d4e821b31ac57285457 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Thu, 3 Oct 2019 10:29:11 -0700
Subject: [PATCH 11/21] wip

---
 authority/provisioner/jwk_test.go   |  18 ---
 authority/provisioner/utils_test.go |  25 ++-
 authority/provisioner/x5c.go        |   2 +-
 authority/provisioner/x5c_test.go   | 241 +++++++++++++++++++++++++++-
 4 files changed, 262 insertions(+), 24 deletions(-)

diff --git a/authority/provisioner/jwk_test.go b/authority/provisioner/jwk_test.go
index 0e7ed57a2..6c368e10f 100644
--- a/authority/provisioner/jwk_test.go
+++ b/authority/provisioner/jwk_test.go
@@ -15,24 +15,6 @@ import (
 	"github.com/smallstep/cli/jose"
 )
 
-var (
-	defaultDisableRenewal   = false
-	defaultEnableSSHCA      = true
-	globalProvisionerClaims = Claims{
-		MinTLSDur:         &Duration{5 * time.Minute},
-		MaxTLSDur:         &Duration{24 * time.Hour},
-		DefaultTLSDur:     &Duration{24 * time.Hour},
-		DisableRenewal:    &defaultDisableRenewal,
-		MinUserSSHDur:     &Duration{Duration: 5 * time.Minute}, // User SSH certs
-		MaxUserSSHDur:     &Duration{Duration: 24 * time.Hour},
-		DefaultUserSSHDur: &Duration{Duration: 4 * time.Hour},
-		MinHostSSHDur:     &Duration{Duration: 5 * time.Minute}, // Host SSH certs
-		MaxHostSSHDur:     &Duration{Duration: 30 * 24 * time.Hour},
-		DefaultHostSSHDur: &Duration{Duration: 30 * 24 * time.Hour},
-		EnableSSHCA:       &defaultEnableSSHCA,
-	}
-)
-
 func TestJWK_Getters(t *testing.T) {
 	p, err := generateJWK()
 	assert.FatalError(t, err)
diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go
index 38bd08e8d..bb0b6853f 100644
--- a/authority/provisioner/utils_test.go
+++ b/authority/provisioner/utils_test.go
@@ -19,10 +19,27 @@ import (
 	"github.com/smallstep/cli/jose"
 )
 
-var testAudiences = Audiences{
-	Sign:   []string{"https://ca.smallstep.com/sign", "https://ca.smallstep.com/1.0/sign"},
-	Revoke: []string{"https://ca.smallstep.com/revoke", "https://ca.smallstep.com/1.0/revoke"},
-}
+var (
+	defaultDisableRenewal   = false
+	defaultEnableSSHCA      = true
+	globalProvisionerClaims = Claims{
+		MinTLSDur:         &Duration{5 * time.Minute},
+		MaxTLSDur:         &Duration{24 * time.Hour},
+		DefaultTLSDur:     &Duration{24 * time.Hour},
+		DisableRenewal:    &defaultDisableRenewal,
+		MinUserSSHDur:     &Duration{Duration: 5 * time.Minute}, // User SSH certs
+		MaxUserSSHDur:     &Duration{Duration: 24 * time.Hour},
+		DefaultUserSSHDur: &Duration{Duration: 4 * time.Hour},
+		MinHostSSHDur:     &Duration{Duration: 5 * time.Minute}, // Host SSH certs
+		MaxHostSSHDur:     &Duration{Duration: 30 * 24 * time.Hour},
+		DefaultHostSSHDur: &Duration{Duration: 30 * 24 * time.Hour},
+		EnableSSHCA:       &defaultEnableSSHCA,
+	}
+	testAudiences = Audiences{
+		Sign:   []string{"https://ca.smallstep.com/sign", "https://ca.smallstep.com/1.0/sign"},
+		Revoke: []string{"https://ca.smallstep.com/revoke", "https://ca.smallstep.com/1.0/revoke"},
+	}
+)
 
 const awsTestCertificate = `-----BEGIN CERTIFICATE-----
 MIICFTCCAX6gAwIBAgIRAKmbVVYAl/1XEqRfF3eJ97MwDQYJKoZIhvcNAQELBQAw
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index 6804a1972..24f0c0272 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -228,7 +228,6 @@ func (p *X5C) AuthorizeRenewal(cert *x509.Certificate) error {
 
 // authorizeSSHSign returns the list of SignOption for a SignSSH request.
 func (p *X5C) authorizeSSHSign(claims *x5cPayload) ([]SignOption, error) {
-	t := now()
 	if claims.Step == nil || claims.Step.SSH == nil {
 		return nil, errors.New("authorization token must be an SSH provisioning token")
 	}
@@ -247,6 +246,7 @@ func (p *X5C) authorizeSSHSign(claims *x5cPayload) ([]SignOption, error) {
 	if len(opts.Principals) > 0 {
 		signOptions = append(signOptions, sshCertificatePrincipalsModifier(opts.Principals))
 	}
+	t := now()
 	if !opts.ValidAfter.IsZero() {
 		signOptions = append(signOptions, sshCertificateValidAfterModifier(opts.ValidAfter.RelativeTime(t).Unix()))
 	}
diff --git a/authority/provisioner/x5c_test.go b/authority/provisioner/x5c_test.go
index 0d7315f6d..163144aeb 100644
--- a/authority/provisioner/x5c_test.go
+++ b/authority/provisioner/x5c_test.go
@@ -1,7 +1,9 @@
 package provisioner
 
 import (
+	"context"
 	"crypto/x509"
+	"net"
 	"testing"
 	"time"
 
@@ -399,7 +401,244 @@ lgsqsR63is+0YQ==
 	}
 }
 
-func TestX5C_AuthorizeReovke(t *testing.T) {
+func TestX5C_AuthorizeSign(t *testing.T) {
+	type test struct {
+		p      *X5C
+		token  string
+		ctx    context.Context
+		err    error
+		dns    []string
+		emails []string
+		ips    []net.IP
+	}
+	tests := map[string]func(*testing.T) test{
+		"fail/invalid-token": func(t *testing.T) test {
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: "foo",
+				ctx:   NewContextWithMethod(context.Background(), SignMethod),
+				err:   errors.New("error parsing token"),
+			}
+		},
+		"fail/ssh/disabled": func(t *testing.T) test {
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			jwk, err := jose.ParseKey("./testdata/x5c-leaf.key")
+			assert.FatalError(t, err)
+
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			*p.Claims.EnableSSHCA = false
+			tok, err := generateToken("foo", p.GetName(), testAudiences.Sign[0], "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				ctx:   NewContextWithMethod(context.Background(), SignSSHMethod),
+				token: tok,
+				err:   errors.Errorf("ssh ca is disabled for provisioner x5c/%s", p.GetName()),
+			}
+		},
+		"ok/empty-sans": func(t *testing.T) test {
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			jwk, err := jose.ParseKey("./testdata/x5c-leaf.key")
+			assert.FatalError(t, err)
+
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("foo", p.GetName(), testAudiences.Sign[0], "",
+				[]string{}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:      p,
+				ctx:    NewContextWithMethod(context.Background(), SignMethod),
+				token:  tok,
+				dns:    []string{"foo"},
+				emails: []string{},
+				ips:    []net.IP{},
+			}
+		},
+		"ok/multi-sans": func(t *testing.T) test {
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			jwk, err := jose.ParseKey("./testdata/x5c-leaf.key")
+			assert.FatalError(t, err)
+
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("foo", p.GetName(), testAudiences.Sign[0], "",
+				[]string{"127.0.0.1", "foo", "max@smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:      p,
+				ctx:    NewContextWithMethod(context.Background(), SignMethod),
+				token:  tok,
+				dns:    []string{"foo"},
+				emails: []string{"max@smallstep.com"},
+				ips:    []net.IP{net.ParseIP("127.0.0.1")},
+			}
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			tc := tt(t)
+			if opts, err := tc.p.AuthorizeSign(tc.ctx, tc.token); err != nil {
+				if assert.NotNil(t, tc.err) {
+					assert.HasPrefix(t, err.Error(), tc.err.Error())
+				}
+			} else {
+				if assert.Nil(t, tc.err) {
+					if assert.NotNil(t, opts) {
+						tot := 0
+						for _, o := range opts {
+							switch v := o.(type) {
+							case *provisionerExtensionOption:
+								assert.Equals(t, v.Type, int(TypeX5C))
+								assert.Equals(t, v.Name, tc.p.GetName())
+								assert.Equals(t, v.CredentialID, "")
+								assert.Len(t, 0, v.KeyValuePairs)
+							case profileProvCredDuration:
+								assert.Equals(t, v.def, tc.p.claimer.DefaultTLSCertDuration())
+
+								claims, err := tc.p.authorizeToken(tc.token, tc.p.audiences.Sign)
+								assert.FatalError(t, err)
+								wantRem := time.Until(claims.chains[0][0].NotAfter)
+								assert.True(t, wantRem < v.rem)
+								assert.True(t, wantRem+time.Minute > v.rem)
+							case commonNameValidator:
+								assert.Equals(t, string(v), "foo")
+							case defaultPublicKeyValidator:
+							case dnsNamesValidator:
+								assert.Equals(t, []string(v), tc.dns)
+							case emailAddressesValidator:
+								assert.Equals(t, []string(v), tc.emails)
+							case ipAddressesValidator:
+								assert.Equals(t, []net.IP(v), tc.ips)
+							case *validityValidator:
+								assert.Equals(t, v.min, tc.p.claimer.MinTLSCertDuration())
+								assert.Equals(t, v.max, tc.p.claimer.MaxTLSCertDuration())
+							default:
+								assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v))
+							}
+							tot++
+						}
+						assert.Equals(t, tot, 8)
+					}
+				}
+			}
+		})
+	}
+}
+
+func TestX5C_authorizeSSHSign(t *testing.T) {
+	_, fn := mockNow()
+	defer fn()
+	type test struct {
+		p      *X5C
+		claims *x5cPayload
+		err    error
+	}
+	tests := map[string]func(*testing.T) test{
+		"fail/no-Step-claim": func(t *testing.T) test {
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			return test{
+				p:      p,
+				claims: new(x5cPayload),
+				err:    errors.New("authorization token must be an SSH provisioning token"),
+			}
+		},
+		"fail/no-SSH-subattribute-in-claims": func(t *testing.T) test {
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			return test{
+				p:      p,
+				claims: &x5cPayload{Step: new(stepPayload)},
+				err:    errors.New("authorization token must be an SSH provisioning token"),
+			}
+		},
+		"ok/with-claims": func(t *testing.T) test {
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			return test{
+				p: p,
+				claims: &x5cPayload{
+					Step: &stepPayload{SSH: &SSHOptions{
+						CertType:    SSHHostCert,
+						Principals:  []string{"max", "mariano", "alan"},
+						ValidAfter:  TimeDuration{d: 5 * time.Minute},
+						ValidBefore: TimeDuration{d: 10 * time.Minute},
+					}},
+					Claims: jose.Claims{Subject: "foo"},
+					chains: [][]*x509.Certificate{certs},
+				},
+			}
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			tc := tt(t)
+			if opts, err := tc.p.authorizeSSHSign(tc.claims); err != nil {
+				if assert.NotNil(t, tc.err) {
+					assert.HasPrefix(t, err.Error(), tc.err.Error())
+				}
+			} else {
+				if assert.Nil(t, tc.err) {
+					if assert.NotNil(t, opts) {
+						tot := 0
+						nw := now()
+						for _, o := range opts {
+							switch v := o.(type) {
+							case sshCertificateOptionsValidator:
+								assert.Equals(t, SSHOptions(v), *tc.claims.Step.SSH)
+							case sshCertificateKeyIDModifier:
+								assert.Equals(t, string(v), "foo")
+							case sshCertificateCertTypeModifier:
+								assert.Equals(t, string(v), tc.claims.Step.SSH.CertType)
+							case sshCertificatePrincipalsModifier:
+								assert.Equals(t, []string(v), tc.claims.Step.SSH.Principals)
+							case sshCertificateValidAfterModifier:
+								assert.Equals(t, int64(v), tc.claims.Step.SSH.ValidAfter.RelativeTime(nw).Unix())
+							case sshCertificateValidBeforeModifier:
+								assert.Equals(t, int64(v), tc.claims.Step.SSH.ValidBefore.RelativeTime(nw).Unix())
+							case sshCertificateDefaultsModifier:
+								assert.Equals(t, SSHOptions(v), SSHOptions{CertType: SSHUserCert})
+							case *sshProvCredValidityModifier:
+								assert.Equals(t, v.Claimer, tc.p.claimer)
+
+								wantRem := time.Until(tc.claims.chains[0][0].NotAfter)
+								assert.True(t, wantRem < v.rem)
+								assert.True(t, wantRem+time.Minute > v.rem)
+							case *sshCertificateValidityValidator:
+								assert.Equals(t, v.Claimer, tc.p.claimer)
+							case *sshDefaultExtensionModifier, *sshDefaultPublicKeyValidator,
+								*sshCertificateDefaultValidator:
+							default:
+								assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v))
+							}
+							tot++
+						}
+						if len(tc.claims.Step.SSH.CertType) > 0 {
+							assert.Equals(t, tot, 12)
+						} else {
+							assert.Equals(t, tot, 8)
+						}
+					}
+				}
+			}
+		})
+	}
+}
+
+func TestX5C_AuthorizeRevoke(t *testing.T) {
 	type test struct {
 		p     *X5C
 		token string

From 7b6ac1e4e103a7f9d456c50b56e55ad8cd59209a Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Thu, 3 Oct 2019 15:12:29 -0700
Subject: [PATCH 12/21] wip

---
 authority/provisioner/acme_test.go        | 39 +++++++--------
 authority/provisioner/aws_test.go         |  2 +-
 authority/provisioner/jwk_test.go         | 59 ++++++++++++++---------
 authority/provisioner/oidc.go             |  4 +-
 authority/provisioner/oidc_test.go        | 31 ++++++++++--
 authority/provisioner/sign_ssh_options.go |  9 +++-
 authority/provisioner/utils_test.go       | 18 +++++++
 authority/provisioner/x5c_test.go         | 43 +++++++++++++++--
 authority/tls_test.go                     | 17 ++++---
 9 files changed, 157 insertions(+), 65 deletions(-)

diff --git a/authority/provisioner/acme_test.go b/authority/provisioner/acme_test.go
index c5e6b13a2..51231ba34 100644
--- a/authority/provisioner/acme_test.go
+++ b/authority/provisioner/acme_test.go
@@ -155,28 +155,23 @@ func TestACME_AuthorizeSign(t *testing.T) {
 				if assert.NotNil(t, got) {
 					assert.Len(t, 4, got)
 
-					_pdd := got[0]
-					pdd, ok := _pdd.(profileDefaultDuration)
-					assert.True(t, ok)
-					assert.Equals(t, pdd, profileDefaultDuration(86400000000000))
-
-					_peo := got[1]
-					peo, ok := _peo.(*provisionerExtensionOption)
-					assert.True(t, ok)
-					assert.Equals(t, peo.Type, 6)
-					assert.Equals(t, peo.Name, "test@acme-provisioner.com")
-					assert.Equals(t, peo.CredentialID, "")
-					assert.Equals(t, peo.KeyValuePairs, nil)
-
-					_vv := got[2]
-					vv, ok := _vv.(*validityValidator)
-					assert.True(t, ok)
-					assert.Equals(t, vv.min, time.Duration(300000000000))
-					assert.Equals(t, vv.max, time.Duration(86400000000000))
-
-					_dpkv := got[3]
-					_, ok = _dpkv.(defaultPublicKeyValidator)
-					assert.True(t, ok)
+					for _, o := range got {
+						switch v := o.(type) {
+						case *provisionerExtensionOption:
+							assert.Equals(t, v.Type, int(TypeACME))
+							assert.Equals(t, v.Name, tt.prov.GetName())
+							assert.Equals(t, v.CredentialID, "")
+							assert.Len(t, 0, v.KeyValuePairs)
+						case profileDefaultDuration:
+							assert.Equals(t, time.Duration(v), tt.prov.claimer.DefaultTLSCertDuration())
+						case defaultPublicKeyValidator:
+						case *validityValidator:
+							assert.Equals(t, v.min, tt.prov.claimer.MinTLSCertDuration())
+							assert.Equals(t, v.max, tt.prov.claimer.MaxTLSCertDuration())
+						default:
+							assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v))
+						}
+					}
 				}
 			}
 		})
diff --git a/authority/provisioner/aws_test.go b/authority/provisioner/aws_test.go
index dd82ec9ba..1b1a63ffb 100644
--- a/authority/provisioner/aws_test.go
+++ b/authority/provisioner/aws_test.go
@@ -410,7 +410,7 @@ func TestAWS_AuthorizeSign_SSH(t *testing.T) {
 		wantErr     bool
 		wantSignErr bool
 	}{
-		{"ok", p1, args{t1, SSHOptions{}, pub}, expectedHostOptions, false, false},
+		{"okey-dokey", p1, args{t1, SSHOptions{}, pub}, expectedHostOptions, false, false},
 		{"ok-rsa2048", p1, args{t1, SSHOptions{}, rsa2048.Public()}, expectedHostOptions, false, false},
 		{"ok-type", p1, args{t1, SSHOptions{CertType: "host"}, pub}, expectedHostOptions, false, false},
 		{"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal"}}, pub}, expectedHostOptions, false, false},
diff --git a/authority/provisioner/jwk_test.go b/authority/provisioner/jwk_test.go
index 6c368e10f..185f1596c 100644
--- a/authority/provisioner/jwk_test.go
+++ b/authority/provisioner/jwk_test.go
@@ -6,11 +6,12 @@ import (
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/x509"
-	"errors"
+	"net"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/pkg/errors"
 	"github.com/smallstep/assert"
 	"github.com/smallstep/cli/jose"
 )
@@ -229,7 +230,7 @@ func TestJWK_AuthorizeSign(t *testing.T) {
 	key1, err := decryptJSONWebKey(p1.EncryptedKey)
 	assert.FatalError(t, err)
 
-	t1, err := generateSimpleToken(p1.Name, testAudiences.Sign[0], key1)
+	t1, err := generateToken("subject", p1.Name, testAudiences.Sign[0], "name@smallstep.com", []string{"127.0.0.1", "max@smallstep.com", "foo"}, time.Now(), key1)
 	assert.FatalError(t, err)
 
 	t2, err := generateToken("subject", p1.Name, testAudiences.Sign[0], "name@smallstep.com", []string{}, time.Now(), key1)
@@ -242,14 +243,17 @@ func TestJWK_AuthorizeSign(t *testing.T) {
 		token string
 	}
 	tests := []struct {
-		name string
-		prov *JWK
-		args args
-		err  error
+		name   string
+		prov   *JWK
+		args   args
+		err    error
+		dns    []string
+		emails []string
+		ips    []net.IP
 	}{
-		{"fail-signature", p1, args{failSig}, errors.New("error parsing claims: square/go-jose: error in cryptographic primitive")},
-		{"ok-sans", p1, args{t1}, nil},
-		{"ok-no-sans", p1, args{t2}, nil},
+		{name: "fail-signature", prov: p1, args: args{failSig}, err: errors.New("error parsing claims: square/go-jose: error in cryptographic primitive")},
+		{"ok-sans", p1, args{t1}, nil, []string{"foo"}, []string{"max@smallstep.com"}, []net.IP{net.ParseIP("127.0.0.1")}},
+		{"ok-no-sans", p1, args{t2}, nil, []string{"subject"}, []string{}, []net.IP{}},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -261,19 +265,30 @@ func TestJWK_AuthorizeSign(t *testing.T) {
 			} else {
 				if assert.NotNil(t, got) {
 					assert.Len(t, 8, got)
-
-					_cnv := got[1]
-					cnv, ok := _cnv.(commonNameValidator)
-					assert.True(t, ok)
-					assert.Equals(t, string(cnv), "subject")
-
-					_dnv := got[2]
-					dnv, ok := _dnv.(dnsNamesValidator)
-					assert.True(t, ok)
-					if tt.name == "ok-sans" {
-						assert.Equals(t, []string(dnv), []string{"test.smallstep.com"})
-					} else {
-						assert.Equals(t, []string(dnv), []string{"subject"})
+					for _, o := range got {
+						switch v := o.(type) {
+						case *provisionerExtensionOption:
+							assert.Equals(t, v.Type, int(TypeJWK))
+							assert.Equals(t, v.Name, tt.prov.GetName())
+							assert.Equals(t, v.CredentialID, tt.prov.Key.KeyID)
+							assert.Len(t, 0, v.KeyValuePairs)
+						case profileDefaultDuration:
+							assert.Equals(t, time.Duration(v), tt.prov.claimer.DefaultTLSCertDuration())
+						case commonNameValidator:
+							assert.Equals(t, string(v), "subject")
+						case defaultPublicKeyValidator:
+						case dnsNamesValidator:
+							assert.Equals(t, []string(v), tt.dns)
+						case emailAddressesValidator:
+							assert.Equals(t, []string(v), tt.emails)
+						case ipAddressesValidator:
+							assert.Equals(t, []net.IP(v), tt.ips)
+						case *validityValidator:
+							assert.Equals(t, v.min, tt.prov.claimer.MinTLSCertDuration())
+							assert.Equals(t, v.max, tt.prov.claimer.MaxTLSCertDuration())
+						default:
+							assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v))
+						}
 					}
 				}
 			}
diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go
index 822ce083a..73663d664 100644
--- a/authority/provisioner/oidc.go
+++ b/authority/provisioner/oidc.go
@@ -294,10 +294,10 @@ func (o *OIDC) AuthorizeSign(ctx context.Context, token string) ([]SignOption, e
 
 	so := []SignOption{
 		// modifiers / withOptions
-		defaultPublicKeyValidator{},
+		newProvisionerExtensionOption(TypeOIDC, o.Name, o.ClientID),
 		profileDefaultDuration(o.claimer.DefaultTLSCertDuration()),
 		// validators
-		newProvisionerExtensionOption(TypeOIDC, o.Name, o.ClientID),
+		defaultPublicKeyValidator{},
 		newValidityValidator(o.claimer.MinTLSCertDuration(), o.claimer.MaxTLSCertDuration()),
 	}
 	// Admins should be able to authorize any SAN
diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go
index 6e99a0fbd..516e0f0ea 100644
--- a/authority/provisioner/oidc_test.go
+++ b/authority/provisioner/oidc_test.go
@@ -11,6 +11,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/pkg/errors"
 	"github.com/smallstep/assert"
 	"github.com/smallstep/cli/jose"
 )
@@ -298,11 +299,31 @@ func TestOIDC_AuthorizeSign(t *testing.T) {
 			if err != nil {
 				assert.Nil(t, got)
 			} else {
-				assert.NotNil(t, got)
-				if tt.name == "admin" {
-					assert.Len(t, 3, got)
-				} else {
-					assert.Len(t, 5, got)
+				if assert.NotNil(t, got) {
+					if tt.name == "admin" {
+						assert.Len(t, 4, got)
+					} else {
+						assert.Len(t, 5, got)
+					}
+					for _, o := range got {
+						switch v := o.(type) {
+						case *provisionerExtensionOption:
+							assert.Equals(t, v.Type, int(TypeOIDC))
+							assert.Equals(t, v.Name, tt.prov.GetName())
+							assert.Equals(t, v.CredentialID, tt.prov.ClientID)
+							assert.Len(t, 0, v.KeyValuePairs)
+						case profileDefaultDuration:
+							assert.Equals(t, time.Duration(v), tt.prov.claimer.DefaultTLSCertDuration())
+						case defaultPublicKeyValidator:
+						case *validityValidator:
+							assert.Equals(t, v.min, tt.prov.claimer.MinTLSCertDuration())
+							assert.Equals(t, v.max, tt.prov.claimer.MaxTLSCertDuration())
+						case emailOnlyIdentity:
+							assert.Equals(t, string(v), "name@smallstep.com")
+						default:
+							assert.FatalError(t, errors.Errorf("unexpected sign option of type %T", v))
+						}
+					}
 				}
 			}
 		})
diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go
index ac8731b42..cb9eec0a7 100644
--- a/authority/provisioner/sign_ssh_options.go
+++ b/authority/provisioner/sign_ssh_options.go
@@ -3,6 +3,7 @@ package provisioner
 import (
 	"crypto/rsa"
 	"encoding/binary"
+	"fmt"
 	"math/big"
 	"time"
 
@@ -288,7 +289,11 @@ func (v *sshCertificateValidityValidator) Valid(cert *ssh.Certificate) error {
 		return errors.Errorf("unknown ssh certificate type %d", cert.CertType)
 	}
 
-	duration := time.Duration(cert.ValidBefore - cert.ValidAfter)
+	// seconds
+	duration, err := time.ParseDuration(fmt.Sprintf("%ds", cert.ValidBefore-cert.ValidAfter))
+	if err != nil {
+		return errors.Wrap(err, "error converting ssh certificate duration")
+	}
 
 	switch {
 	case cert.ValidAfter == 0:
@@ -321,6 +326,8 @@ func (v *sshCertificateDefaultValidator) Valid(cert *ssh.Certificate) error {
 		return errors.New("ssh certificate key cannot be nil")
 	case cert.Serial == 0:
 		return errors.New("ssh certificate serial cannot be 0")
+	case cert.CertType != ssh.UserCert && cert.CertType != ssh.HostCert:
+		return errors.Errorf("ssh certificate has an unknown type: %d", cert.CertType)
 	case cert.KeyId == "":
 		return errors.New("ssh certificate key id cannot be empty")
 	case len(cert.ValidPrincipals) == 0:
diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go
index bb0b6853f..8f4fbaad6 100644
--- a/authority/provisioner/utils_test.go
+++ b/authority/provisioner/utils_test.go
@@ -41,6 +41,24 @@ var (
 	}
 )
 
+func provisionerClaims() *Claims {
+	ddr := false
+	des := true
+	return &Claims{
+		MinTLSDur:         &Duration{5 * time.Minute},
+		MaxTLSDur:         &Duration{24 * time.Hour},
+		DefaultTLSDur:     &Duration{24 * time.Hour},
+		DisableRenewal:    &ddr,
+		MinUserSSHDur:     &Duration{Duration: 5 * time.Minute}, // User SSH certs
+		MaxUserSSHDur:     &Duration{Duration: 24 * time.Hour},
+		DefaultUserSSHDur: &Duration{Duration: 4 * time.Hour},
+		MinHostSSHDur:     &Duration{Duration: 5 * time.Minute}, // Host SSH certs
+		MaxHostSSHDur:     &Duration{Duration: 30 * 24 * time.Hour},
+		DefaultHostSSHDur: &Duration{Duration: 30 * 24 * time.Hour},
+		EnableSSHCA:       &des,
+	}
+}
+
 const awsTestCertificate = `-----BEGIN CERTIFICATE-----
 MIICFTCCAX6gAwIBAgIRAKmbVVYAl/1XEqRfF3eJ97MwDQYJKoZIhvcNAQELBQAw
 GDEWMBQGA1UEAxMNQVdTIFRlc3QgQ2VydDAeFw0xOTA0MjQyMjU3MzlaFw0yOTA0
diff --git a/authority/provisioner/x5c_test.go b/authority/provisioner/x5c_test.go
index 163144aeb..84744a58c 100644
--- a/authority/provisioner/x5c_test.go
+++ b/authority/provisioner/x5c_test.go
@@ -430,7 +430,8 @@ func TestX5C_AuthorizeSign(t *testing.T) {
 
 			p, err := generateX5C(nil)
 			assert.FatalError(t, err)
-			*p.Claims.EnableSSHCA = false
+			p.claimer.claims = provisionerClaims()
+			*p.claimer.claims.EnableSSHCA = false
 			tok, err := generateToken("foo", p.GetName(), testAudiences.Sign[0], "",
 				[]string{"test.smallstep.com"}, time.Now(), jwk,
 				withX5CHdr(certs))
@@ -442,6 +443,25 @@ func TestX5C_AuthorizeSign(t *testing.T) {
 				err:   errors.Errorf("ssh ca is disabled for provisioner x5c/%s", p.GetName()),
 			}
 		},
+		"fail/ssh/invalid-token": func(t *testing.T) test {
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			jwk, err := jose.ParseKey("./testdata/x5c-leaf.key")
+			assert.FatalError(t, err)
+
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			tok, err := generateToken("foo", p.GetName(), testAudiences.Sign[0], "",
+				[]string{"test.smallstep.com"}, time.Now(), jwk,
+				withX5CHdr(certs))
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				ctx:   NewContextWithMethod(context.Background(), SignSSHMethod),
+				token: tok,
+				err:   errors.New("authorization token must be an SSH provisioning token"),
+			}
+		},
 		"ok/empty-sans": func(t *testing.T) test {
 			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
 			assert.FatalError(t, err)
@@ -582,6 +602,20 @@ func TestX5C_authorizeSSHSign(t *testing.T) {
 				},
 			}
 		},
+		"ok/without-claims": func(t *testing.T) test {
+			p, err := generateX5C(nil)
+			assert.FatalError(t, err)
+			certs, err := pemutil.ReadCertificateBundle("./testdata/x5c-leaf.crt")
+			assert.FatalError(t, err)
+			return test{
+				p: p,
+				claims: &x5cPayload{
+					Step:   &stepPayload{SSH: &SSHOptions{}},
+					Claims: jose.Claims{Subject: "foo"},
+					chains: [][]*x509.Certificate{certs},
+				},
+			}
+		},
 	}
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
@@ -598,6 +632,8 @@ func TestX5C_authorizeSSHSign(t *testing.T) {
 						for _, o := range opts {
 							switch v := o.(type) {
 							case sshCertificateOptionsValidator:
+								tc.claims.Step.SSH.ValidAfter.t = time.Time{}
+								tc.claims.Step.SSH.ValidBefore.t = time.Time{}
 								assert.Equals(t, SSHOptions(v), *tc.claims.Step.SSH)
 							case sshCertificateKeyIDModifier:
 								assert.Equals(t, string(v), "foo")
@@ -614,9 +650,8 @@ func TestX5C_authorizeSSHSign(t *testing.T) {
 							case *sshProvCredValidityModifier:
 								assert.Equals(t, v.Claimer, tc.p.claimer)
 
-								wantRem := time.Until(tc.claims.chains[0][0].NotAfter)
-								assert.True(t, wantRem < v.rem)
-								assert.True(t, wantRem+time.Minute > v.rem)
+								wantRem := tc.claims.chains[0][0].NotAfter.Sub(nw)
+								assert.Equals(t, wantRem, v.rem)
 							case *sshCertificateValidityValidator:
 								assert.Equals(t, v.Claimer, tc.p.claimer)
 							case *sshDefaultExtensionModifier, *sshDefaultPublicKeyValidator,
diff --git a/authority/tls_test.go b/authority/tls_test.go
index 8d443fd45..f719d6157 100644
--- a/authority/tls_test.go
+++ b/authority/tls_test.go
@@ -208,14 +208,15 @@ func TestSign(t *testing.T) {
 		},
 		"fail rsa key too short": func(t *testing.T) *signTest {
 			shortRSAKeyPEM := `-----BEGIN CERTIFICATE REQUEST-----
-MIIBdDCB2wIBADAOMQwwCgYDVQQDEwNmb28wgaIwDQYJKoZIhvcNAQEBBQADgZAA
-MIGMAoGEAK8dks7oV6kcIFEaWna7CDGYPAE8IL7rNi+ruQ1dIYz+JtxT7OPjbCn/
-t5iqni96+35iS/8CvMtEuquOMTMSWOWwlurrbTbLqCazuz/g233o8udxSxhny3cY
-wHogp4cXCX6cFll6DeUnoCEuTTSIu8IBHbK48VfNw4V4gGz6cp/H93HrAgMBAAGg
-ITAfBgkqhkiG9w0BCQ4xEjAQMA4GA1UdEQQHMAWCA2ZvbzANBgkqhkiG9w0BAQsF
-AAOBhABCZsYM+Kgje68Z9Fjl2+cBwtQHvZDarh+cz6W1SchinZ1T0aNQvSj/otOe
-ttnEF4Rq8zqzr4fbv+AF451Mx36AkfgZr9XWGzxidrH+fBCNWXWNR+ymhrL6UFTG
-2FbarLt9jN2aJLAYQPwtSeGTAZ74tLOPRPnTP6aMfFNg4XCR0uveHA==
+MIIBhDCB7gIBADAZMRcwFQYDVQQDEw5zbWFsbHN0ZXAgdGVzdDCBnzANBgkqhkiG
+9w0BAQEFAAOBjQAwgYkCgYEA5JlgH99HvHHsCD6XTqqYj3bXU2oIlnYGoLVs7IJ4
+k205rv5/YWky2gjdpIv0Tnaf3o57IJ891lB7GiyO5iHIEUv5N9dVzrdUboyzk2uZ
+7JMMNB43CSLB2oNuwJjLeAM/yBzlhRnvpKjrNSfSV+cH54FXdnbFbcTFMStnjqKG
+MeECAwEAAaAsMCoGCSqGSIb3DQEJDjEdMBswGQYDVR0RBBIwEIIOc21hbGxzdGVw
+IHRlc3QwDQYJKoZIhvcNAQELBQADgYEAKwsbr8Zfcq05DgOoJ//cXMFK1SP8ktRU
+N2++E8Ww0Tet9oyNRArqxxS/UyVio63D3wynzRAB25PFGpYG1cN4b81Gv/foFUT6
+W5kR63lNVHBHgQmv5mA8YFsfrJHstaz5k727v2LMHEYIf5/3i16d5zhuxUoaPTYr
+ZYtQ9Ot36qc=
 -----END CERTIFICATE REQUEST-----`
 			block, _ := pem.Decode([]byte(shortRSAKeyPEM))
 			assert.FatalError(t, err)

From 1a17758fcfb19b460f442f808ce44fab61fe4a1a Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Thu, 3 Oct 2019 16:24:31 -0700
Subject: [PATCH 13/21] wip

---
 authority/provisioner/sign_ssh_options.go     |  29 +++-
 .../provisioner/sign_ssh_options_test.go      | 157 ++++++++++++++++++
 2 files changed, 177 insertions(+), 9 deletions(-)

diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go
index cb9eec0a7..9d031b27c 100644
--- a/authority/provisioner/sign_ssh_options.go
+++ b/authority/provisioner/sign_ssh_options.go
@@ -250,8 +250,15 @@ func (m *sshProvCredValidityModifier) Modify(cert *ssh.Certificate) error {
 		return nil
 	}
 
-	diff := cert.ValidBefore - cert.ValidAfter
-	if uint64(m.rem) < diff {
+	if cert.ValidBefore < cert.ValidAfter {
+		return errors.New("ssh certificate validBefore cannot be before validAfter")
+	}
+	dur, err := time.ParseDuration(fmt.Sprintf("%ds", cert.ValidBefore-cert.ValidAfter))
+	if err != nil {
+		return errors.Wrapf(err, "error converting ssh certificate duration; "+
+			"vaf: %d, vbf: %d, d: %d", cert.ValidAfter, cert.ValidBefore, cert.ValidBefore-cert.ValidAfter)
+	}
+	if m.rem < dur {
 		t := time.Unix(int64(cert.ValidAfter), 0)
 		cert.ValidBefore = uint64(t.Add(m.rem).Unix())
 	}
@@ -275,6 +282,15 @@ type sshCertificateValidityValidator struct {
 }
 
 func (v *sshCertificateValidityValidator) Valid(cert *ssh.Certificate) error {
+	switch {
+	case cert.ValidAfter == 0:
+		return errors.New("ssh certificate validAfter cannot be 0")
+	case cert.ValidBefore < uint64(now().Unix()):
+		return errors.New("ssh certificate validBefore cannot be in the past")
+	case cert.ValidBefore < cert.ValidAfter:
+		return errors.New("ssh certificate validBefore cannot be before validAfter")
+	}
+
 	var min, max time.Duration
 	switch cert.CertType {
 	case ssh.UserCert:
@@ -292,16 +308,11 @@ func (v *sshCertificateValidityValidator) Valid(cert *ssh.Certificate) error {
 	// seconds
 	duration, err := time.ParseDuration(fmt.Sprintf("%ds", cert.ValidBefore-cert.ValidAfter))
 	if err != nil {
-		return errors.Wrap(err, "error converting ssh certificate duration")
+		return errors.Wrapf(err, "error converting ssh certificate duration; "+
+			"vaf: %d, vbf: %d, d: %d", cert.ValidAfter, cert.ValidBefore, cert.ValidBefore-cert.ValidAfter)
 	}
 
 	switch {
-	case cert.ValidAfter == 0:
-		return errors.New("ssh certificate validAfter cannot be 0")
-	case cert.ValidBefore < uint64(now().Unix()):
-		return errors.New("ssh certificate validBefore cannot be in the past")
-	case cert.ValidBefore < cert.ValidAfter:
-		return errors.New("ssh certificate validBefore cannot be before validAfter")
 	case duration < min:
 		return errors.Errorf("requested duration of %s is less than minimum "+
 			"accepted duration for selected provisioner of %s", duration, min)
diff --git a/authority/provisioner/sign_ssh_options_test.go b/authority/provisioner/sign_ssh_options_test.go
index 95c6913c2..356a6f74a 100644
--- a/authority/provisioner/sign_ssh_options_test.go
+++ b/authority/provisioner/sign_ssh_options_test.go
@@ -190,3 +190,160 @@ func Test_sshCertificateDefaultValidator_Valid(t *testing.T) {
 		})
 	}
 }
+
+func Test_sshCertificateValidityValidator(t *testing.T) {
+	p, err := generateX5C(nil)
+	assert.FatalError(t, err)
+	v := sshCertificateValidityValidator{p.claimer}
+	n := now()
+	tests := []struct {
+		name string
+		cert *ssh.Certificate
+		err  error
+	}{
+		{
+			"fail/validAfter-0",
+			&ssh.Certificate{CertType: ssh.UserCert},
+			errors.New("ssh certificate validAfter cannot be 0"),
+		},
+		{
+			"fail/validBefore-in-past",
+			&ssh.Certificate{CertType: ssh.UserCert, ValidAfter: uint64(now().Unix()), ValidBefore: uint64(now().Add(-time.Minute).Unix())},
+			errors.New("ssh certificate validBefore cannot be in the past"),
+		},
+		{
+			"fail/validBefore-before-validAfter",
+			&ssh.Certificate{CertType: ssh.UserCert, ValidAfter: uint64(now().Add(5 * time.Minute).Unix()), ValidBefore: uint64(now().Add(3 * time.Minute).Unix())},
+			errors.New("ssh certificate validBefore cannot be before validAfter"),
+		},
+		{
+			"fail/cert-type-not-set",
+			&ssh.Certificate{ValidAfter: uint64(now().Unix()), ValidBefore: uint64(now().Add(10 * time.Minute).Unix())},
+			errors.New("ssh certificate type has not been set"),
+		},
+		{
+			"fail/unexpected-cert-type",
+			&ssh.Certificate{
+				CertType:    3,
+				ValidAfter:  uint64(now().Unix()),
+				ValidBefore: uint64(now().Add(10 * time.Minute).Unix()),
+			},
+			errors.New("unknown ssh certificate type 3"),
+		},
+		{
+			"fail/duration<min",
+			&ssh.Certificate{
+				CertType:    1,
+				ValidAfter:  uint64(n.Unix()),
+				ValidBefore: uint64(n.Add(4 * time.Minute).Unix()),
+			},
+			errors.New("requested duration of 4m0s is less than minimum accepted duration for selected provisioner of 5m0s"),
+		},
+		{
+			"fail/duration>max",
+			&ssh.Certificate{
+				CertType:    1,
+				ValidAfter:  uint64(n.Unix()),
+				ValidBefore: uint64(n.Add(48 * time.Hour).Unix()),
+			},
+			errors.New("requested duration of 48h0m0s is greater than maximum accepted duration for selected provisioner of 24h0m0s"),
+		},
+		{
+			"ok",
+			&ssh.Certificate{
+				CertType:    1,
+				ValidAfter:  uint64(now().Unix()),
+				ValidBefore: uint64(now().Add(8 * time.Hour).Unix()),
+			},
+			nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := v.Valid(tt.cert); err != nil {
+				if assert.NotNil(t, tt.err) {
+					assert.HasPrefix(t, err.Error(), tt.err.Error())
+				}
+			} else {
+				assert.Nil(t, tt.err)
+			}
+		})
+	}
+}
+
+func Test_sshProvCredValidityModifier(t *testing.T) {
+	p, err := generateX5C(nil)
+	assert.FatalError(t, err)
+	n := now()
+	tests := []struct {
+		name string
+		rem  time.Duration
+		cert *ssh.Certificate
+		err  error
+	}{
+		{
+			name: "fail/defaultValidityModifier-error",
+			rem:  0,
+			cert: &ssh.Certificate{
+				ValidAfter:  uint64(n.Unix()),
+				ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
+			},
+			err: errors.New("ssh certificate type has not been set"),
+		},
+		{
+			name: "fail/validBefore-before-validAfter",
+			rem:  4 * time.Hour,
+			cert: &ssh.Certificate{
+				CertType:    ssh.UserCert,
+				ValidAfter:  uint64(n.Unix()),
+				ValidBefore: uint64(n.Add(-8 * time.Hour).Unix()),
+			},
+			err: errors.New("ssh certificate validBefore cannot be before validAfter"),
+		},
+		{
+			name: "ok/rem-0/noop",
+			rem:  0,
+			cert: &ssh.Certificate{
+				CertType:    ssh.UserCert,
+				ValidAfter:  uint64(n.Unix()),
+				ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
+			},
+		},
+		{
+			name: "ok/rem>dur-noop",
+			rem:  16 * time.Hour,
+			cert: &ssh.Certificate{
+				CertType:    ssh.UserCert,
+				ValidAfter:  uint64(n.Unix()),
+				ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
+			},
+		},
+		{
+			name: "ok/rem<dur",
+			rem:  4 * time.Hour,
+			cert: &ssh.Certificate{
+				CertType:    ssh.UserCert,
+				ValidAfter:  uint64(n.Unix()),
+				ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			v := sshProvCredValidityModifier{p.claimer, tt.rem}
+			if err := v.Modify(tt.cert); err != nil {
+				if assert.NotNil(t, tt.err) {
+					assert.HasPrefix(t, err.Error(), tt.err.Error())
+				}
+			} else {
+				if tt.name == "ok/rem<dur" {
+					assert.Equals(t, tt.cert.ValidAfter, uint64(n.Unix()))
+					assert.Equals(t, tt.cert.ValidBefore, uint64(n.Add(tt.rem).Unix()))
+				} else {
+					assert.Equals(t, tt.cert.ValidAfter, uint64(n.Unix()))
+					assert.Equals(t, tt.cert.ValidBefore, uint64(n.Add(8*time.Hour).Unix()))
+				}
+			}
+		})
+	}
+}

From a1e4bbd4c4d1b324cb368cc470a97c4ee1856fc9 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Thu, 3 Oct 2019 21:08:47 -0700
Subject: [PATCH 14/21] wipw

---
 authority/provisioner/aws_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/authority/provisioner/aws_test.go b/authority/provisioner/aws_test.go
index 1b1a63ffb..dd82ec9ba 100644
--- a/authority/provisioner/aws_test.go
+++ b/authority/provisioner/aws_test.go
@@ -410,7 +410,7 @@ func TestAWS_AuthorizeSign_SSH(t *testing.T) {
 		wantErr     bool
 		wantSignErr bool
 	}{
-		{"okey-dokey", p1, args{t1, SSHOptions{}, pub}, expectedHostOptions, false, false},
+		{"ok", p1, args{t1, SSHOptions{}, pub}, expectedHostOptions, false, false},
 		{"ok-rsa2048", p1, args{t1, SSHOptions{}, rsa2048.Public()}, expectedHostOptions, false, false},
 		{"ok-type", p1, args{t1, SSHOptions{CertType: "host"}, pub}, expectedHostOptions, false, false},
 		{"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"127.0.0.1", "ip-127-0-0-1.us-west-1.compute.internal"}}, pub}, expectedHostOptions, false, false},

From 338d53c3474b873c3291ddc294dcab0ec417eed5 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Thu, 3 Oct 2019 21:17:59 -0700
Subject: [PATCH 15/21] wip

---
 authority/provisioner/sign_options.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go
index 4d20bad1f..100cbd946 100644
--- a/authority/provisioner/sign_options.go
+++ b/authority/provisioner/sign_options.go
@@ -211,7 +211,7 @@ func (v profileDefaultDuration) Option(so Options) x509util.WithOption {
 	return x509util.WithNotBeforeAfterDuration(notBefore, notAfter, time.Duration(v))
 }
 
-// validityValidator validates the certificate temporal validity settings.
+// validityValidator validates the certificate validity settings.
 type validityValidator struct {
 	min time.Duration
 	max time.Duration
@@ -222,7 +222,7 @@ func newValidityValidator(min, max time.Duration) *validityValidator {
 	return &validityValidator{min: min, max: max}
 }
 
-// Validate validates the certificate temporal settings (notBefore/notAfter)
+// Valid validates the certificate validity settings (notBefore/notAfter) and
 // and total duration.
 func (v *validityValidator) Valid(crt *x509.Certificate) error {
 	var (

From 5a96947b983a0f6683a26e802483266fba8ecdf9 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Wed, 9 Oct 2019 12:27:16 -0700
Subject: [PATCH 16/21] wip

---
 authority/provisioner/sign_ssh_options.go | 22 ++++++----------------
 authority/provisioner/x5c.go              |  2 +-
 2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go
index 9d031b27c..af73b9fb3 100644
--- a/authority/provisioner/sign_ssh_options.go
+++ b/authority/provisioner/sign_ssh_options.go
@@ -3,7 +3,6 @@ package provisioner
 import (
 	"crypto/rsa"
 	"encoding/binary"
-	"fmt"
 	"math/big"
 	"time"
 
@@ -253,11 +252,7 @@ func (m *sshProvCredValidityModifier) Modify(cert *ssh.Certificate) error {
 	if cert.ValidBefore < cert.ValidAfter {
 		return errors.New("ssh certificate validBefore cannot be before validAfter")
 	}
-	dur, err := time.ParseDuration(fmt.Sprintf("%ds", cert.ValidBefore-cert.ValidAfter))
-	if err != nil {
-		return errors.Wrapf(err, "error converting ssh certificate duration; "+
-			"vaf: %d, vbf: %d, d: %d", cert.ValidAfter, cert.ValidBefore, cert.ValidBefore-cert.ValidAfter)
-	}
+	dur := time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second
 	if m.rem < dur {
 		t := time.Unix(int64(cert.ValidAfter), 0)
 		cert.ValidBefore = uint64(t.Add(m.rem).Unix())
@@ -306,19 +301,14 @@ func (v *sshCertificateValidityValidator) Valid(cert *ssh.Certificate) error {
 	}
 
 	// seconds
-	duration, err := time.ParseDuration(fmt.Sprintf("%ds", cert.ValidBefore-cert.ValidAfter))
-	if err != nil {
-		return errors.Wrapf(err, "error converting ssh certificate duration; "+
-			"vaf: %d, vbf: %d, d: %d", cert.ValidAfter, cert.ValidBefore, cert.ValidBefore-cert.ValidAfter)
-	}
-
+	dur := time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second
 	switch {
-	case duration < min:
+	case dur < min:
 		return errors.Errorf("requested duration of %s is less than minimum "+
-			"accepted duration for selected provisioner of %s", duration, min)
-	case duration > max:
+			"accepted duration for selected provisioner of %s", dur, min)
+	case dur > max:
 		return errors.Errorf("requested duration of %s is greater than maximum "+
-			"accepted duration for selected provisioner of %s", duration, max)
+			"accepted duration for selected provisioner of %s", dur, max)
 	default:
 		return nil
 	}
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index 24f0c0272..26584fc6c 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -77,7 +77,7 @@ func (p *X5C) Init(config Config) error {
 		return errors.New("provisioner type cannot be empty")
 	case p.Name == "":
 		return errors.New("provisioner name cannot be empty")
-	case p.Roots == nil || len(p.Roots) == 0:
+	case len(p.Roots) == 0:
 		return errors.New("provisioner root(s) cannot be empty")
 	}
 

From 180882e3ff042e6955dccefe5e395718167da841 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Thu, 10 Oct 2019 18:42:29 -0700
Subject: [PATCH 17/21] wip

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 644b2d9df..6878eef4e 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
-PKG?=github.com/smallstep/certificates/cmd/step-ca
+KG?=github.com/smallstep/certificates/cmd/step-ca
 BINNAME?=step-ca
 
 # Set V to 1 for verbose output from the Makefile

From a5f9db5d19318385d9118b7fb46a86495f48a067 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Thu, 10 Oct 2019 18:42:43 -0700
Subject: [PATCH 18/21] wip

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 6878eef4e..644b2d9df 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
-KG?=github.com/smallstep/certificates/cmd/step-ca
+PKG?=github.com/smallstep/certificates/cmd/step-ca
 BINNAME?=step-ca
 
 # Set V to 1 for verbose output from the Makefile

From 35e77c6af3426e3582b3049021e4839a3f7b34e6 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Fri, 11 Oct 2019 13:25:52 -0700
Subject: [PATCH 19/21] wip

---
 authority/provisioner/aws.go              |  2 +-
 authority/provisioner/azure.go            |  2 +-
 authority/provisioner/gcp.go              |  2 +-
 authority/provisioner/jwk.go              |  2 +-
 authority/provisioner/oidc.go             |  2 +-
 authority/provisioner/sign_options.go     | 48 +++++++++++++---
 authority/provisioner/sign_ssh_options.go | 67 ++++++++++-------------
 authority/provisioner/x5c.go              |  4 +-
 8 files changed, 78 insertions(+), 51 deletions(-)

diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go
index 9352de7d8..3611986b6 100644
--- a/authority/provisioner/aws.go
+++ b/authority/provisioner/aws.go
@@ -471,7 +471,7 @@ func (p *AWS) authorizeSSHSign(claims *awsPayload) ([]SignOption, error) {
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshValidityModifier{p.claimer},
+		sshDefaultValidityModifier(p.claimer),
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
 		// Validate the validity period.
diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go
index c1b48b99d..d8252799d 100644
--- a/authority/provisioner/azure.go
+++ b/authority/provisioner/azure.go
@@ -328,7 +328,7 @@ func (p *Azure) authorizeSSHSign(claims azurePayload, name string) ([]SignOption
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshValidityModifier{p.claimer},
+		sshDefaultValidityModifier(p.claimer),
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
 		// Validate the validity period.
diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go
index 6f0627bfa..b2ec509a6 100644
--- a/authority/provisioner/gcp.go
+++ b/authority/provisioner/gcp.go
@@ -383,7 +383,7 @@ func (p *GCP) authorizeSSHSign(claims *gcpPayload) ([]SignOption, error) {
 		// Set the default extensions
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshValidityModifier{p.claimer},
+		sshDefaultValidityModifier(p.claimer),
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
 		// Validate the validity period.
diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go
index f3d8d95e4..b8d3be705 100644
--- a/authority/provisioner/jwk.go
+++ b/authority/provisioner/jwk.go
@@ -214,7 +214,7 @@ func (p *JWK) authorizeSSHSign(claims *jwtPayload) ([]SignOption, error) {
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshValidityModifier{p.claimer},
+		sshDefaultValidityModifier(p.claimer),
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
 		// Validate the validity period.
diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go
index 73663d664..b65d9b6f0 100644
--- a/authority/provisioner/oidc.go
+++ b/authority/provisioner/oidc.go
@@ -346,7 +346,7 @@ func (o *OIDC) authorizeSSHSign(claims *openIDPayload) ([]SignOption, error) {
 		// Set the default extensions
 		&sshDefaultExtensionModifier{},
 		// Set the validity bounds if not set.
-		&sshValidityModifier{o.claimer},
+		sshDefaultValidityModifier(o.claimer),
 		// Validate public key
 		&sshDefaultPublicKeyValidator{},
 		// Validate the validity period.
diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go
index 100cbd946..fc03a6504 100644
--- a/authority/provisioner/sign_options.go
+++ b/authority/provisioner/sign_options.go
@@ -184,18 +184,52 @@ func (v emailAddressesValidator) Valid(req *x509.CertificateRequest) error {
 	return nil
 }
 
-// profileProvCredDuration adjusts the duration to the
-// min(default, remaining provisioning credential duration.
-type profileProvCredDuration struct {
+// remainderDuration adjusts the duration to
+// min(default, remaining provisioning credential duration).
+// E.g. if the default is 12hrs but the remaining validity of the provisioning
+// credential is only 4hrs, this option will set the value to 4hrs (the min of the two values).
+type remainderDuration struct {
 	def time.Duration
 	rem time.Duration
 }
 
-func (v profileProvCredDuration) Option(so Options) x509util.WithOption {
-	if v.rem < v.def {
-		v.def = v.rem
+func (v remainderDuration) Option(so Options) x509util.WithOption {
+	return func(p x509util.Profile) error {
+		n := now()
+		notBefore := so.NotBefore.Time()
+		if notBefore.IsZero() {
+			notBefore = n
+		}
+		notAfter := so.NotAfter.RelativeTime(notBefore)
+
+		if notAfter.IsZero() {
+			// If NotAfter is zero, then we haven't specified a duration in the request.
+			// So we just need to verify that the relative remaining duration is not less than
+			// zero (throw error), and if it is less than the default duration then
+			// modify the default duration.
+			remtd := &TimeDuration{d: v.rem}
+			relativeRemainder := remtd.Time().Sub(notBefore)
+			if relativeRemainder < 0 {
+				return errors.Errorf("remaining provisioning credential duration (%s) expires "+
+					"before requested certificate notBefore (%s)", v.rem, notBefore)
+			} else if relativeRemainder < v.def {
+				v.def = relativeRemainder
+			}
+		} else {
+			// If NotAfter is defined then verify that remaining duration
+			// on the provisioning credential is after NotAfter.
+			if n.Add(v.rem).Before(notAfter) {
+				return errors.Errorf("remaining provisioning credential duration (%s) expires "+
+					"before requested certificate notAfter (%s)", v.rem, notAfter)
+			}
+			// NOTE: re-setting the def below is unecessary (since notAfter is set).
+			// But for posterity ...
+			if v.rem < v.def {
+				v.def = v.rem
+			}
+		}
+		return x509util.WithNotBeforeAfterDuration(notBefore, notAfter, v.def)(p)
 	}
-	return profileDefaultDuration(v.def).Option(so)
 }
 
 // profileDefaultDuration is a wrapper against x509util.WithOption to conform
diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go
index af73b9fb3..f803d7818 100644
--- a/authority/provisioner/sign_ssh_options.go
+++ b/authority/provisioner/sign_ssh_options.go
@@ -196,6 +196,7 @@ func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error {
 // CertType has not been set or is not valid.
 type sshValidityModifier struct {
 	*Claimer
+	rem time.Duration
 }
 
 func (m *sshValidityModifier) Modify(cert *ssh.Certificate) error {
@@ -212,53 +213,45 @@ func (m *sshValidityModifier) Modify(cert *ssh.Certificate) error {
 		return errors.Errorf("unknown ssh certificate type %d", cert.CertType)
 	}
 
+	n := now()
 	if cert.ValidAfter == 0 {
-		cert.ValidAfter = uint64(now().Truncate(time.Second).Unix())
+		cert.ValidAfter = uint64(n.Truncate(time.Second).Unix())
 	}
+
 	if cert.ValidBefore == 0 {
-		t := time.Unix(int64(cert.ValidAfter), 0)
-		cert.ValidBefore = uint64(t.Add(d).Unix())
+		va := time.Unix(int64(cert.ValidAfter), 0)
+		if m.rem > 0 {
+			remtd := &TimeDuration{d: m.rem}
+			relativeRemainder := remtd.Time().Sub(va)
+			if relativeRemainder < 0 {
+				return errors.Errorf("remaining provisioning credential duration (%s) expires "+
+					"before requested certificate validAfter (%s)", m.rem, va)
+			} else if relativeRemainder < d {
+				d = relativeRemainder
+			}
+		}
+		cert.ValidBefore = uint64(va.Add(d).Unix())
+	} else if m.rem > 0 {
+		vb := time.Unix(int64(cert.ValidBefore), 0)
+		if n.Add(m.rem).Before(vb) {
+			return errors.Errorf("remaining provisioning credential duration (%s) expires "+
+				"before requested certificate validBefore (%s)", m.rem, vb)
+		}
 	}
 
 	return nil
 }
 
-type sshProvCredValidityModifier struct {
-	*Claimer
-	// Remaining duration on the provisioning credential.
-	// E.g. x5c provisioners use a certificate as a provisioning credential.
-	// That certificate should not be able to provision new certificates with
-	// a duration longer than the remaining duration on the provisioning
-	// certificate.
-	rem time.Duration
+func sshDefaultValidityModifier(c *Claimer) SSHCertificateModifier {
+	return &sshValidityModifier{c, 0}
 }
 
-// Modify has the following procedure:
-//  1) Use the existing ValidBefore & ValidAfter values if they are set; else
-//  2.a) Use the default duration to set the validity bounds and
-//  2.b) If 'rem' is non-zero and less than the default duration then
-//       reduce the duration to 'rem'.
-//
-// Step 2) only applies if ValidBefore is not set in the request.
-func (m *sshProvCredValidityModifier) Modify(cert *ssh.Certificate) error {
-	defMod := &sshValidityModifier{m.Claimer}
-	if err := defMod.Modify(cert); err != nil {
-		return err
-	}
-	if m.rem == 0 {
-		return nil
-	}
-
-	if cert.ValidBefore < cert.ValidAfter {
-		return errors.New("ssh certificate validBefore cannot be before validAfter")
-	}
-	dur := time.Duration(cert.ValidBefore-cert.ValidAfter) * time.Second
-	if m.rem < dur {
-		t := time.Unix(int64(cert.ValidAfter), 0)
-		cert.ValidBefore = uint64(t.Add(m.rem).Unix())
-	}
-
-	return nil
+// sshRemainderValidityModifier adjusts the duration to
+// min(default, remaining provisioning credential duration).
+// E.g. if the default is 12hrs but the remaining validity of the provisioning
+// credential is only 4hrs, this option will set the value to 4hrs (the min of the two values).
+func sshRemainderValidityModifier(c *Claimer, rem time.Duration) SSHCertificateModifier {
+	return &sshValidityModifier{c, rem}
 }
 
 // sshCertificateOptionsValidator validates the user SSHOptions with the ones
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index 26584fc6c..dda404c4b 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -207,7 +207,7 @@ func (p *X5C) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	return []SignOption{
 		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeX5C, p.Name, ""),
-		profileProvCredDuration{p.claimer.DefaultTLSCertDuration(), rem},
+		remainderDuration{p.claimer.DefaultTLSCertDuration(), rem},
 		// validators
 		commonNameValidator(claims.Subject),
 		defaultPublicKeyValidator{},
@@ -263,7 +263,7 @@ func (p *X5C) authorizeSSHSign(claims *x5cPayload) ([]SignOption, error) {
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Checks the validity bounds, and set the validity if has not been set.
-		&sshProvCredValidityModifier{p.claimer, rem},
+		sshRemainderValidityModifier(p.claimer, rem),
 		// Validate public key.
 		&sshDefaultPublicKeyValidator{},
 		// Validate the validity period.

From 79eef91909d0c9e113454aaeab31f2d6a3de7cd2 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Fri, 11 Oct 2019 20:53:44 -0700
Subject: [PATCH 20/21] wip

---
 authority/provisioner/sign_options.go         |  85 +++++-----
 authority/provisioner/sign_options_test.go    |  84 ++++++++++
 authority/provisioner/sign_ssh_options.go     |  42 ++---
 .../provisioner/sign_ssh_options_test.go      | 155 +++++++++++-------
 authority/provisioner/x5c.go                  |  11 +-
 authority/provisioner/x5c_test.go             |  12 +-
 6 files changed, 246 insertions(+), 143 deletions(-)

diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go
index fc03a6504..53921a3c2 100644
--- a/authority/provisioner/sign_options.go
+++ b/authority/provisioner/sign_options.go
@@ -184,65 +184,58 @@ func (v emailAddressesValidator) Valid(req *x509.CertificateRequest) error {
 	return nil
 }
 
-// remainderDuration adjusts the duration to
-// min(default, remaining provisioning credential duration).
-// E.g. if the default is 12hrs but the remaining validity of the provisioning
-// credential is only 4hrs, this option will set the value to 4hrs (the min of the two values).
-type remainderDuration struct {
-	def time.Duration
-	rem time.Duration
+// profileDefaultDuration is a wrapper against x509util.WithOption to conform
+// the SignOption interface.
+type profileDefaultDuration time.Duration
+
+func (v profileDefaultDuration) Option(so Options) x509util.WithOption {
+	notBefore := so.NotBefore.Time()
+	if notBefore.IsZero() {
+		notBefore = time.Now()
+	}
+	notAfter := so.NotAfter.RelativeTime(notBefore)
+	return x509util.WithNotBeforeAfterDuration(notBefore, notAfter, time.Duration(v))
+}
+
+// profileLimitDuration is an x509 profile option that modifies an x509 validity
+// period according to an imposed expiration time.
+type profileLimitDuration struct {
+	def      time.Duration
+	notAfter time.Time
 }
 
-func (v remainderDuration) Option(so Options) x509util.WithOption {
+// Option returns an x509util option that limits the validity period of a
+// certificate to one that is superficially imposed.
+func (v profileLimitDuration) Option(so Options) x509util.WithOption {
 	return func(p x509util.Profile) error {
 		n := now()
 		notBefore := so.NotBefore.Time()
 		if notBefore.IsZero() {
 			notBefore = n
 		}
-		notAfter := so.NotAfter.RelativeTime(notBefore)
+		if notBefore.After(v.notAfter) {
+			return errors.Errorf("provisioning credential expiration (%s) is before "+
+				"requested certificate notBefore (%s)", v.notAfter, notBefore)
+		}
 
+		notAfter := so.NotAfter.RelativeTime(notBefore)
+		if notAfter.After(v.notAfter) {
+			return errors.Errorf("provisioning credential expiration (%s) is before "+
+				"requested certificate notAfter (%s)", v.notAfter, notBefore)
+		}
 		if notAfter.IsZero() {
-			// If NotAfter is zero, then we haven't specified a duration in the request.
-			// So we just need to verify that the relative remaining duration is not less than
-			// zero (throw error), and if it is less than the default duration then
-			// modify the default duration.
-			remtd := &TimeDuration{d: v.rem}
-			relativeRemainder := remtd.Time().Sub(notBefore)
-			if relativeRemainder < 0 {
-				return errors.Errorf("remaining provisioning credential duration (%s) expires "+
-					"before requested certificate notBefore (%s)", v.rem, notBefore)
-			} else if relativeRemainder < v.def {
-				v.def = relativeRemainder
-			}
-		} else {
-			// If NotAfter is defined then verify that remaining duration
-			// on the provisioning credential is after NotAfter.
-			if n.Add(v.rem).Before(notAfter) {
-				return errors.Errorf("remaining provisioning credential duration (%s) expires "+
-					"before requested certificate notAfter (%s)", v.rem, notAfter)
-			}
-			// NOTE: re-setting the def below is unecessary (since notAfter is set).
-			// But for posterity ...
-			if v.rem < v.def {
-				v.def = v.rem
+			t := notBefore.Add(v.def)
+			if t.After(v.notAfter) {
+				notAfter = v.notAfter
+			} else {
+				notAfter = t
 			}
 		}
-		return x509util.WithNotBeforeAfterDuration(notBefore, notAfter, v.def)(p)
-	}
-}
-
-// profileDefaultDuration is a wrapper against x509util.WithOption to conform
-// the SignOption interface.
-type profileDefaultDuration time.Duration
-
-func (v profileDefaultDuration) Option(so Options) x509util.WithOption {
-	notBefore := so.NotBefore.Time()
-	if notBefore.IsZero() {
-		notBefore = time.Now()
+		crt := p.Subject()
+		crt.NotBefore = notBefore
+		crt.NotAfter = notAfter
+		return nil
 	}
-	notAfter := so.NotAfter.RelativeTime(notBefore)
-	return x509util.WithNotBeforeAfterDuration(notBefore, notAfter, time.Duration(v))
 }
 
 // validityValidator validates the certificate validity settings.
diff --git a/authority/provisioner/sign_options_test.go b/authority/provisioner/sign_options_test.go
index 38f1d5e66..8a452dabd 100644
--- a/authority/provisioner/sign_options_test.go
+++ b/authority/provisioner/sign_options_test.go
@@ -273,3 +273,87 @@ func Test_validityValidator_Valid(t *testing.T) {
 		})
 	}
 }
+
+func Test_profileLimitDuration_Option(t *testing.T) {
+	n := now()
+	type test struct {
+		pld   profileLimitDuration
+		so    Options
+		cert  *x509.Certificate
+		valid func(*x509.Certificate)
+		err   error
+	}
+	tests := map[string]func() test{
+		"fail/notBefore-after-limit": func() test {
+			d, err := ParseTimeDuration("8h")
+			assert.FatalError(t, err)
+			return test{
+				pld:  profileLimitDuration{def: 4 * time.Hour, notAfter: n.Add(6 * time.Hour)},
+				so:   Options{NotBefore: d},
+				cert: new(x509.Certificate),
+				err:  errors.New("provisioning credential expiration ("),
+			}
+		},
+		"fail/requested-notAfter-after-limit": func() test {
+			d, err := ParseTimeDuration("4h")
+			assert.FatalError(t, err)
+			return test{
+				pld:  profileLimitDuration{def: 4 * time.Hour, notAfter: n.Add(6 * time.Hour)},
+				so:   Options{NotBefore: NewTimeDuration(n.Add(3 * time.Hour)), NotAfter: d},
+				cert: new(x509.Certificate),
+				err:  errors.New("provisioning credential expiration ("),
+			}
+		},
+		"ok/valid-notAfter-requested": func() test {
+			d, err := ParseTimeDuration("2h")
+			assert.FatalError(t, err)
+			return test{
+				pld:  profileLimitDuration{def: 4 * time.Hour, notAfter: n.Add(6 * time.Hour)},
+				so:   Options{NotBefore: NewTimeDuration(n.Add(3 * time.Hour)), NotAfter: d},
+				cert: new(x509.Certificate),
+				valid: func(cert *x509.Certificate) {
+					assert.Equals(t, cert.NotBefore, n.Add(3*time.Hour))
+					assert.Equals(t, cert.NotAfter, n.Add(5*time.Hour))
+				},
+			}
+		},
+		"ok/valid-notAfter-nil-limit-over-default": func() test {
+			return test{
+				pld:  profileLimitDuration{def: 1 * time.Hour, notAfter: n.Add(6 * time.Hour)},
+				so:   Options{NotBefore: NewTimeDuration(n.Add(3 * time.Hour))},
+				cert: new(x509.Certificate),
+				valid: func(cert *x509.Certificate) {
+					assert.Equals(t, cert.NotBefore, n.Add(3*time.Hour))
+					assert.Equals(t, cert.NotAfter, n.Add(4*time.Hour))
+				},
+			}
+		},
+		"ok/valid-notAfter-nil-limit-under-default": func() test {
+			return test{
+				pld:  profileLimitDuration{def: 4 * time.Hour, notAfter: n.Add(6 * time.Hour)},
+				so:   Options{NotBefore: NewTimeDuration(n.Add(3 * time.Hour))},
+				cert: new(x509.Certificate),
+				valid: func(cert *x509.Certificate) {
+					assert.Equals(t, cert.NotBefore, n.Add(3*time.Hour))
+					assert.Equals(t, cert.NotAfter, n.Add(6*time.Hour))
+				},
+			}
+		},
+	}
+	for name, run := range tests {
+		t.Run(name, func(t *testing.T) {
+			tt := run()
+			prof := &x509util.Leaf{}
+			prof.SetSubject(tt.cert)
+			if err := tt.pld.Option(tt.so)(prof); err != nil {
+				if assert.NotNil(t, tt.err) {
+					assert.HasPrefix(t, err.Error(), tt.err.Error())
+				}
+			} else {
+				if assert.Nil(t, tt.err) {
+					tt.valid(prof.Subject())
+				}
+			}
+		})
+	}
+}
diff --git a/authority/provisioner/sign_ssh_options.go b/authority/provisioner/sign_ssh_options.go
index f803d7818..a8f63cd51 100644
--- a/authority/provisioner/sign_ssh_options.go
+++ b/authority/provisioner/sign_ssh_options.go
@@ -196,7 +196,7 @@ func (m *sshDefaultExtensionModifier) Modify(cert *ssh.Certificate) error {
 // CertType has not been set or is not valid.
 type sshValidityModifier struct {
 	*Claimer
-	rem time.Duration
+	validBefore time.Time
 }
 
 func (m *sshValidityModifier) Modify(cert *ssh.Certificate) error {
@@ -213,29 +213,29 @@ func (m *sshValidityModifier) Modify(cert *ssh.Certificate) error {
 		return errors.Errorf("unknown ssh certificate type %d", cert.CertType)
 	}
 
+	hasLimit := !m.validBefore.IsZero()
+
 	n := now()
 	if cert.ValidAfter == 0 {
 		cert.ValidAfter = uint64(n.Truncate(time.Second).Unix())
 	}
+	certValidAfter := time.Unix(int64(cert.ValidAfter), 0)
+	if hasLimit && certValidAfter.After(m.validBefore) {
+		return errors.Errorf("provisioning credential expiration (%s) is before "+
+			"requested certificate validAfter (%s)", m.validBefore, certValidAfter)
+	}
 
 	if cert.ValidBefore == 0 {
-		va := time.Unix(int64(cert.ValidAfter), 0)
-		if m.rem > 0 {
-			remtd := &TimeDuration{d: m.rem}
-			relativeRemainder := remtd.Time().Sub(va)
-			if relativeRemainder < 0 {
-				return errors.Errorf("remaining provisioning credential duration (%s) expires "+
-					"before requested certificate validAfter (%s)", m.rem, va)
-			} else if relativeRemainder < d {
-				d = relativeRemainder
-			}
+		certValidBefore := certValidAfter.Add(d)
+		if hasLimit && m.validBefore.Before(certValidBefore) {
+			certValidBefore = m.validBefore
 		}
-		cert.ValidBefore = uint64(va.Add(d).Unix())
-	} else if m.rem > 0 {
-		vb := time.Unix(int64(cert.ValidBefore), 0)
-		if n.Add(m.rem).Before(vb) {
-			return errors.Errorf("remaining provisioning credential duration (%s) expires "+
-				"before requested certificate validBefore (%s)", m.rem, vb)
+		cert.ValidBefore = uint64(certValidBefore.Unix())
+	} else if hasLimit {
+		certValidBefore := time.Unix(int64(cert.ValidBefore), 0)
+		if m.validBefore.Before(certValidBefore) {
+			return errors.Errorf("provisioning credential expiration (%s) is before "+
+				"requested certificate validBefore (%s)", m.validBefore, certValidBefore)
 		}
 	}
 
@@ -243,15 +243,15 @@ func (m *sshValidityModifier) Modify(cert *ssh.Certificate) error {
 }
 
 func sshDefaultValidityModifier(c *Claimer) SSHCertificateModifier {
-	return &sshValidityModifier{c, 0}
+	return &sshValidityModifier{c, time.Time{}}
 }
 
-// sshRemainderValidityModifier adjusts the duration to
+// sshLimitValidityModifier adjusts the duration to
 // min(default, remaining provisioning credential duration).
 // E.g. if the default is 12hrs but the remaining validity of the provisioning
 // credential is only 4hrs, this option will set the value to 4hrs (the min of the two values).
-func sshRemainderValidityModifier(c *Claimer, rem time.Duration) SSHCertificateModifier {
-	return &sshValidityModifier{c, rem}
+func sshLimitValidityModifier(c *Claimer, validBefore time.Time) SSHCertificateModifier {
+	return &sshValidityModifier{c, validBefore}
 }
 
 // sshCertificateOptionsValidator validates the user SSHOptions with the ones
diff --git a/authority/provisioner/sign_ssh_options_test.go b/authority/provisioner/sign_ssh_options_test.go
index 356a6f74a..25a441212 100644
--- a/authority/provisioner/sign_ssh_options_test.go
+++ b/authority/provisioner/sign_ssh_options_test.go
@@ -271,77 +271,114 @@ func Test_sshCertificateValidityValidator(t *testing.T) {
 	}
 }
 
-func Test_sshProvCredValidityModifier(t *testing.T) {
+func Test_sshValidityModifier(t *testing.T) {
+	n := now()
 	p, err := generateX5C(nil)
 	assert.FatalError(t, err)
-	n := now()
-	tests := []struct {
-		name string
-		rem  time.Duration
-		cert *ssh.Certificate
-		err  error
-	}{
-		{
-			name: "fail/defaultValidityModifier-error",
-			rem:  0,
-			cert: &ssh.Certificate{
-				ValidAfter:  uint64(n.Unix()),
-				ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
-			},
-			err: errors.New("ssh certificate type has not been set"),
+	type test struct {
+		svm   *sshValidityModifier
+		cert  *ssh.Certificate
+		valid func(*ssh.Certificate)
+		err   error
+	}
+	tests := map[string]func() test{
+		"fail/type-not-set": func() test {
+			return test{
+				svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(6 * time.Hour)},
+				cert: &ssh.Certificate{
+					ValidAfter:  uint64(n.Unix()),
+					ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
+				},
+				err: errors.New("ssh certificate type has not been set"),
+			}
 		},
-		{
-			name: "fail/validBefore-before-validAfter",
-			rem:  4 * time.Hour,
-			cert: &ssh.Certificate{
-				CertType:    ssh.UserCert,
-				ValidAfter:  uint64(n.Unix()),
-				ValidBefore: uint64(n.Add(-8 * time.Hour).Unix()),
-			},
-			err: errors.New("ssh certificate validBefore cannot be before validAfter"),
+		"fail/type-not-recognized": func() test {
+			return test{
+				svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(6 * time.Hour)},
+				cert: &ssh.Certificate{
+					CertType:    4,
+					ValidAfter:  uint64(n.Unix()),
+					ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
+				},
+				err: errors.New("unknown ssh certificate type 4"),
+			}
 		},
-		{
-			name: "ok/rem-0/noop",
-			rem:  0,
-			cert: &ssh.Certificate{
-				CertType:    ssh.UserCert,
-				ValidAfter:  uint64(n.Unix()),
-				ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
-			},
+		"fail/requested-validAfter-after-limit": func() test {
+			return test{
+				svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(1 * time.Hour)},
+				cert: &ssh.Certificate{
+					CertType:    1,
+					ValidAfter:  uint64(n.Add(2 * time.Hour).Unix()),
+					ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
+				},
+				err: errors.Errorf("provisioning credential expiration ("),
+			}
 		},
-		{
-			name: "ok/rem>dur-noop",
-			rem:  16 * time.Hour,
-			cert: &ssh.Certificate{
-				CertType:    ssh.UserCert,
-				ValidAfter:  uint64(n.Unix()),
-				ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
-			},
+		"fail/requested-validBefore-after-limit": func() test {
+			return test{
+				svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(1 * time.Hour)},
+				cert: &ssh.Certificate{
+					CertType:    1,
+					ValidAfter:  uint64(n.Unix()),
+					ValidBefore: uint64(n.Add(2 * time.Hour).Unix()),
+				},
+				err: errors.New("provisioning credential expiration ("),
+			}
 		},
-		{
-			name: "ok/rem<dur",
-			rem:  4 * time.Hour,
-			cert: &ssh.Certificate{
-				CertType:    ssh.UserCert,
-				ValidAfter:  uint64(n.Unix()),
-				ValidBefore: uint64(n.Add(8 * time.Hour).Unix()),
-			},
+		"ok/valid-requested-validBefore": func() test {
+			va, vb := uint64(n.Unix()), uint64(n.Add(2*time.Hour).Unix())
+			return test{
+				svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(3 * time.Hour)},
+				cert: &ssh.Certificate{
+					CertType:    1,
+					ValidAfter:  va,
+					ValidBefore: vb,
+				},
+				valid: func(cert *ssh.Certificate) {
+					assert.Equals(t, cert.ValidAfter, va)
+					assert.Equals(t, cert.ValidBefore, vb)
+				},
+			}
+		},
+		"ok/empty-requested-validBefore-limit-after-default": func() test {
+			va := uint64(n.Unix())
+			return test{
+				svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(5 * time.Hour)},
+				cert: &ssh.Certificate{
+					CertType:   1,
+					ValidAfter: va,
+				},
+				valid: func(cert *ssh.Certificate) {
+					assert.Equals(t, cert.ValidAfter, va)
+					assert.Equals(t, cert.ValidBefore, uint64(n.Add(4*time.Hour).Unix()))
+				},
+			}
+		},
+		"ok/empty-requested-validBefore-limit-before-default": func() test {
+			va := uint64(n.Unix())
+			return test{
+				svm: &sshValidityModifier{Claimer: p.claimer, validBefore: n.Add(3 * time.Hour)},
+				cert: &ssh.Certificate{
+					CertType:   1,
+					ValidAfter: va,
+				},
+				valid: func(cert *ssh.Certificate) {
+					assert.Equals(t, cert.ValidAfter, va)
+					assert.Equals(t, cert.ValidBefore, uint64(n.Add(3*time.Hour).Unix()))
+				},
+			}
 		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			v := sshProvCredValidityModifier{p.claimer, tt.rem}
-			if err := v.Modify(tt.cert); err != nil {
+	for name, run := range tests {
+		t.Run(name, func(t *testing.T) {
+			tt := run()
+			if err := tt.svm.Modify(tt.cert); err != nil {
 				if assert.NotNil(t, tt.err) {
 					assert.HasPrefix(t, err.Error(), tt.err.Error())
 				}
 			} else {
-				if tt.name == "ok/rem<dur" {
-					assert.Equals(t, tt.cert.ValidAfter, uint64(n.Unix()))
-					assert.Equals(t, tt.cert.ValidBefore, uint64(n.Add(tt.rem).Unix()))
-				} else {
-					assert.Equals(t, tt.cert.ValidAfter, uint64(n.Unix()))
-					assert.Equals(t, tt.cert.ValidBefore, uint64(n.Add(8*time.Hour).Unix()))
+				if assert.Nil(t, tt.err) {
+					tt.valid(tt.cert)
 				}
 			}
 		})
diff --git a/authority/provisioner/x5c.go b/authority/provisioner/x5c.go
index dda404c4b..557259827 100644
--- a/authority/provisioner/x5c.go
+++ b/authority/provisioner/x5c.go
@@ -199,15 +199,10 @@ func (p *X5C) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 
 	dnsNames, ips, emails := x509util.SplitSANs(claims.SANs)
 
-	// Get the remaining duration from the provisioning cert. The duration
-	// on the new certificate cannot be greater than the remaining duration of
-	// the provisioning certificate.
-	rem := claims.chains[0][0].NotAfter.Sub(now())
-
 	return []SignOption{
 		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeX5C, p.Name, ""),
-		remainderDuration{p.claimer.DefaultTLSCertDuration(), rem},
+		profileLimitDuration{p.claimer.DefaultTLSCertDuration(), claims.chains[0][0].NotAfter},
 		// validators
 		commonNameValidator(claims.Subject),
 		defaultPublicKeyValidator{},
@@ -257,13 +252,11 @@ func (p *X5C) authorizeSSHSign(claims *x5cPayload) ([]SignOption, error) {
 	// Default to a user certificate with no principals if not set
 	signOptions = append(signOptions, sshCertificateDefaultsModifier{CertType: SSHUserCert})
 
-	rem := claims.chains[0][0].NotAfter.Sub(now())
-
 	return append(signOptions,
 		// Set the default extensions.
 		&sshDefaultExtensionModifier{},
 		// Checks the validity bounds, and set the validity if has not been set.
-		sshRemainderValidityModifier(p.claimer, rem),
+		sshLimitValidityModifier(p.claimer, claims.chains[0][0].NotAfter),
 		// Validate public key.
 		&sshDefaultPublicKeyValidator{},
 		// Validate the validity period.
diff --git a/authority/provisioner/x5c_test.go b/authority/provisioner/x5c_test.go
index 84744a58c..477e32674 100644
--- a/authority/provisioner/x5c_test.go
+++ b/authority/provisioner/x5c_test.go
@@ -523,14 +523,12 @@ func TestX5C_AuthorizeSign(t *testing.T) {
 								assert.Equals(t, v.Name, tc.p.GetName())
 								assert.Equals(t, v.CredentialID, "")
 								assert.Len(t, 0, v.KeyValuePairs)
-							case profileProvCredDuration:
+							case profileLimitDuration:
 								assert.Equals(t, v.def, tc.p.claimer.DefaultTLSCertDuration())
 
 								claims, err := tc.p.authorizeToken(tc.token, tc.p.audiences.Sign)
 								assert.FatalError(t, err)
-								wantRem := time.Until(claims.chains[0][0].NotAfter)
-								assert.True(t, wantRem < v.rem)
-								assert.True(t, wantRem+time.Minute > v.rem)
+								assert.Equals(t, v.notAfter, claims.chains[0][0].NotAfter)
 							case commonNameValidator:
 								assert.Equals(t, string(v), "foo")
 							case defaultPublicKeyValidator:
@@ -647,11 +645,9 @@ func TestX5C_authorizeSSHSign(t *testing.T) {
 								assert.Equals(t, int64(v), tc.claims.Step.SSH.ValidBefore.RelativeTime(nw).Unix())
 							case sshCertificateDefaultsModifier:
 								assert.Equals(t, SSHOptions(v), SSHOptions{CertType: SSHUserCert})
-							case *sshProvCredValidityModifier:
+							case *sshValidityModifier:
 								assert.Equals(t, v.Claimer, tc.p.claimer)
-
-								wantRem := tc.claims.chains[0][0].NotAfter.Sub(nw)
-								assert.Equals(t, wantRem, v.rem)
+								assert.Equals(t, v.validBefore, tc.claims.chains[0][0].NotAfter)
 							case *sshCertificateValidityValidator:
 								assert.Equals(t, v.Claimer, tc.p.claimer)
 							case *sshDefaultExtensionModifier, *sshDefaultPublicKeyValidator,

From e107b507b0ac48ab03ad33cacb05cf762aa2cdb9 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Mon, 14 Oct 2019 14:07:12 -0700
Subject: [PATCH 21/21] wip

---
 Gopkg.toml                   |  2 +-
 acme/api/account.go          |  3 --
 acme/api/handler.go          |  5 ----
 acme/api/middleware.go       |  9 ------
 acme/api/middleware_test.go  |  8 ------
 acme/api/order.go            |  3 --
 authority/provisioner/aws.go |  4 +--
 authority/provisioner/jwk.go |  2 +-
 ca/renew.go                  |  2 +-
 ca/signal.go                 | 56 ++++++++++++++++--------------------
 ca/tls_options_test.go       |  4 +--
 logging/handler.go           |  2 +-
 12 files changed, 33 insertions(+), 67 deletions(-)

diff --git a/Gopkg.toml b/Gopkg.toml
index 8e12c5e1b..97fb234bb 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -29,7 +29,7 @@
   name = "github.com/go-chi/chi"
 
 [[override]]
-  branch = "x5c"
+  branch = "master"
   name = "github.com/smallstep/cli"
 
 [[constraint]]
diff --git a/acme/api/account.go b/acme/api/account.go
index 05d6a0841..fb43d4f94 100644
--- a/acme/api/account.go
+++ b/acme/api/account.go
@@ -128,7 +128,6 @@ func (h *Handler) NewAccount(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Location", h.Auth.GetLink(acme.AccountLink,
 		acme.URLSafeProvisionerName(prov), true, acc.GetID()))
 	api.JSONStatus(w, acc, httpStatus)
-	return
 }
 
 // GetUpdateAccount is the api for updating an ACME account.
@@ -172,7 +171,6 @@ func (h *Handler) GetUpdateAccount(w http.ResponseWriter, r *http.Request) {
 	}
 	w.Header().Set("Location", h.Auth.GetLink(acme.AccountLink, acme.URLSafeProvisionerName(prov), true, acc.GetID()))
 	api.JSON(w, acc)
-	return
 }
 
 func logOrdersByAccount(w http.ResponseWriter, oids []string) {
@@ -209,5 +207,4 @@ func (h *Handler) GetOrdersByAccount(w http.ResponseWriter, r *http.Request) {
 	}
 	api.JSON(w, orders)
 	logOrdersByAccount(w, orders)
-	return
 }
diff --git a/acme/api/handler.go b/acme/api/handler.go
index 423c08ea9..11cd74f22 100644
--- a/acme/api/handler.go
+++ b/acme/api/handler.go
@@ -113,7 +113,6 @@ func (h *Handler) GetNonce(w http.ResponseWriter, r *http.Request) {
 	} else {
 		w.WriteHeader(http.StatusNoContent)
 	}
-	return
 }
 
 // GetDirectory is the ACME resource for returning a directory configuration
@@ -126,7 +125,6 @@ func (h *Handler) GetDirectory(w http.ResponseWriter, r *http.Request) {
 	}
 	dir := h.Auth.GetDirectory(prov)
 	api.JSON(w, dir)
-	return
 }
 
 // GetAuthz ACME api for retrieving an Authz.
@@ -149,7 +147,6 @@ func (h *Handler) GetAuthz(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set("Location", h.Auth.GetLink(acme.AuthzLink, acme.URLSafeProvisionerName(prov), true, authz.GetID()))
 	api.JSON(w, authz)
-	return
 }
 
 // GetChallenge ACME api for retrieving a Challenge.
@@ -191,7 +188,6 @@ func (h *Handler) GetChallenge(w http.ResponseWriter, r *http.Request) {
 	w.Header().Add("Link", link(getLink(acme.AuthzLink, acme.URLSafeProvisionerName(prov), true, ch.GetAuthzID()), "up"))
 	w.Header().Set("Location", getLink(acme.ChallengeLink, acme.URLSafeProvisionerName(prov), true, ch.GetID()))
 	api.JSON(w, ch)
-	return
 }
 
 // GetCertificate ACME api for retrieving a Certificate.
@@ -210,5 +206,4 @@ func (h *Handler) GetCertificate(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set("Content-Type", "application/pem-certificate-chain; charset=utf-8")
 	w.Write(certBytes)
-	return
 }
diff --git a/acme/api/middleware.go b/acme/api/middleware.go
index 3f4c99a53..af2618bff 100644
--- a/acme/api/middleware.go
+++ b/acme/api/middleware.go
@@ -42,7 +42,6 @@ func (h *Handler) addNonce(next nextHTTP) nextHTTP {
 		w.Header().Set("Cache-Control", "no-store")
 		logNonce(w, nonce)
 		next(w, r)
-		return
 	}
 }
 
@@ -57,7 +56,6 @@ func (h *Handler) addDirLink(next nextHTTP) nextHTTP {
 		}
 		w.Header().Add("Link", link(h.Auth.GetLink(acme.DirectoryLink, acme.URLSafeProvisionerName(prov), true), "index"))
 		next(w, r)
-		return
 	}
 }
 
@@ -87,7 +85,6 @@ func (h *Handler) verifyContentType(next nextHTTP) nextHTTP {
 		}
 		api.WriteError(w, acme.MalformedErr(errors.Errorf(
 			"expected content-type to be in %s, but got %s", expected, ct)))
-		return
 	}
 }
 
@@ -106,7 +103,6 @@ func (h *Handler) parseJWS(next nextHTTP) nextHTTP {
 		}
 		ctx := context.WithValue(r.Context(), jwsContextKey, jws)
 		next(w, r.WithContext(ctx))
-		return
 	}
 }
 
@@ -202,7 +198,6 @@ func (h *Handler) validateJWS(next nextHTTP) nextHTTP {
 			return
 		}
 		next(w, r)
-		return
 	}
 }
 
@@ -248,7 +243,6 @@ func (h *Handler) extractJWK(next nextHTTP) nextHTTP {
 			ctx = context.WithValue(ctx, accContextKey, acc)
 		}
 		next(w, r.WithContext(ctx))
-		return
 	}
 }
 
@@ -275,7 +269,6 @@ func (h *Handler) lookupProvisioner(next nextHTTP) nextHTTP {
 		}
 		ctx = context.WithValue(ctx, provisionerContextKey, p)
 		next(w, r.WithContext(ctx))
-		return
 	}
 }
 
@@ -355,7 +348,6 @@ func (h *Handler) verifyAndExtractJWSPayload(next nextHTTP) nextHTTP {
 			isEmptyJSON: string(payload) == "{}",
 		})
 		next(w, r.WithContext(ctx))
-		return
 	}
 }
 
@@ -372,6 +364,5 @@ func (h *Handler) isPostAsGet(next nextHTTP) nextHTTP {
 			return
 		}
 		next(w, r)
-		return
 	}
 }
diff --git a/acme/api/middleware_test.go b/acme/api/middleware_test.go
index 18fafd8d9..f8aa322c4 100644
--- a/acme/api/middleware_test.go
+++ b/acme/api/middleware_test.go
@@ -26,7 +26,6 @@ var testBody = []byte("foo")
 
 func testNext(w http.ResponseWriter, r *http.Request) {
 	w.Write(testBody)
-	return
 }
 
 func TestHandlerAddNonce(t *testing.T) {
@@ -471,7 +470,6 @@ func TestHandlerParseJWS(t *testing.T) {
 					assert.FatalError(t, err)
 					assert.Equals(t, gotRaw, expRaw)
 					w.Write(testBody)
-					return
 				},
 				statusCode: 200,
 			}
@@ -923,7 +921,6 @@ func TestHandlerLookupJWK(t *testing.T) {
 					assert.FatalError(t, err)
 					assert.Equals(t, _jwk, jwk)
 					w.Write(testBody)
-					return
 				},
 				statusCode: 200,
 			}
@@ -1114,7 +1111,6 @@ func TestHandlerExtractJWK(t *testing.T) {
 					assert.FatalError(t, err)
 					assert.Equals(t, _jwk.KeyID, pub.KeyID)
 					w.Write(testBody)
-					return
 				},
 				statusCode: 200,
 			}
@@ -1139,7 +1135,6 @@ func TestHandlerExtractJWK(t *testing.T) {
 					assert.FatalError(t, err)
 					assert.Equals(t, _jwk.KeyID, pub.KeyID)
 					w.Write(testBody)
-					return
 				},
 				statusCode: 200,
 			}
@@ -1448,7 +1443,6 @@ func TestHandlerValidateJWS(t *testing.T) {
 				ctx: context.WithValue(context.Background(), jwsContextKey, jws),
 				next: func(w http.ResponseWriter, r *http.Request) {
 					w.Write(testBody)
-					return
 				},
 				statusCode: 200,
 			}
@@ -1479,7 +1473,6 @@ func TestHandlerValidateJWS(t *testing.T) {
 				ctx: context.WithValue(context.Background(), jwsContextKey, jws),
 				next: func(w http.ResponseWriter, r *http.Request) {
 					w.Write(testBody)
-					return
 				},
 				statusCode: 200,
 			}
@@ -1510,7 +1503,6 @@ func TestHandlerValidateJWS(t *testing.T) {
 				ctx: context.WithValue(context.Background(), jwsContextKey, jws),
 				next: func(w http.ResponseWriter, r *http.Request) {
 					w.Write(testBody)
-					return
 				},
 				statusCode: 200,
 			}
diff --git a/acme/api/order.go b/acme/api/order.go
index 83d1e26e5..1d4911022 100644
--- a/acme/api/order.go
+++ b/acme/api/order.go
@@ -97,7 +97,6 @@ func (h *Handler) NewOrder(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set("Location", h.Auth.GetLink(acme.OrderLink, acme.URLSafeProvisionerName(prov), true, o.GetID()))
 	api.JSONStatus(w, o, http.StatusCreated)
-	return
 }
 
 // GetOrder ACME api for retrieving an order.
@@ -121,7 +120,6 @@ func (h *Handler) GetOrder(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set("Location", h.Auth.GetLink(acme.OrderLink, acme.URLSafeProvisionerName(prov), true, o.GetID()))
 	api.JSON(w, o)
-	return
 }
 
 // FinalizeOrder attemptst to finalize an order and create a certificate.
@@ -160,5 +158,4 @@ func (h *Handler) FinalizeOrder(w http.ResponseWriter, r *http.Request) {
 
 	w.Header().Set("Location", h.Auth.GetLink(acme.OrderLink, acme.URLSafeProvisionerName(prov), true, o.ID))
 	api.JSON(w, o)
-	return
 }
diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go
index 3611986b6..e1b2ef9d5 100644
--- a/authority/provisioner/aws.go
+++ b/authority/provisioner/aws.go
@@ -274,8 +274,8 @@ func (p *AWS) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	}
 
 	// Check for the sign ssh method, default to sign X.509
-	if m := MethodFromContext(ctx); m == SignSSHMethod {
-		if p.claimer.IsSSHCAEnabled() == false {
+	if MethodFromContext(ctx) == SignSSHMethod {
+		if !p.claimer.IsSSHCAEnabled() {
 			return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID())
 		}
 		return p.authorizeSSHSign(payload)
diff --git a/authority/provisioner/jwk.go b/authority/provisioner/jwk.go
index b8d3be705..f9178bb7d 100644
--- a/authority/provisioner/jwk.go
+++ b/authority/provisioner/jwk.go
@@ -143,7 +143,7 @@ func (p *JWK) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 
 	// Check for SSH sign-ing request.
 	if MethodFromContext(ctx) == SignSSHMethod {
-		if p.claimer.IsSSHCAEnabled() == false {
+		if !p.claimer.IsSSHCAEnabled() {
 			return nil, errors.Errorf("ssh ca is disabled for provisioner %s", p.GetID())
 		}
 		return p.authorizeSSHSign(claims)
diff --git a/ca/renew.go b/ca/renew.go
index 442347818..6a4fd22b9 100644
--- a/ca/renew.go
+++ b/ca/renew.go
@@ -178,7 +178,7 @@ func (r *TLSRenewer) renewCertificate() {
 }
 
 func (r *TLSRenewer) nextRenewDuration(notAfter time.Time) time.Duration {
-	d := notAfter.Sub(time.Now()) - r.renewBefore
+	d := time.Until(notAfter) - r.renewBefore
 	n := rand.Int63n(int64(r.renewJitter))
 	d -= time.Duration(n)
 	if d < 0 {
diff --git a/ca/signal.go b/ca/signal.go
index 0d950435d..598cc6f60 100644
--- a/ca/signal.go
+++ b/ca/signal.go
@@ -28,20 +28,17 @@ func StopHandler(servers ...Stopper) {
 	signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM)
 	defer signal.Stop(signals)
 
-	for {
-		select {
-		case sig := <-signals:
-			switch sig {
-			case syscall.SIGINT, syscall.SIGTERM:
-				log.Println("shutting down ...")
-				for _, server := range servers {
-					err := server.Stop()
-					if err != nil {
-						log.Printf("error stopping server: %s", err.Error())
-					}
+	for sig := range signals {
+		switch sig {
+		case syscall.SIGINT, syscall.SIGTERM:
+			log.Println("shutting down ...")
+			for _, server := range servers {
+				err := server.Stop()
+				if err != nil {
+					log.Printf("error stopping server: %s", err.Error())
 				}
-				return
 			}
+			return
 		}
 	}
 }
@@ -54,28 +51,25 @@ func StopReloaderHandler(servers ...StopReloader) {
 	signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP)
 	defer signal.Stop(signals)
 
-	for {
-		select {
-		case sig := <-signals:
-			switch sig {
-			case syscall.SIGHUP:
-				log.Println("reloading ...")
-				for _, server := range servers {
-					err := server.Reload()
-					if err != nil {
-						log.Printf("error reloading server: %+v", err)
-					}
+	for sig := range signals {
+		switch sig {
+		case syscall.SIGHUP:
+			log.Println("reloading ...")
+			for _, server := range servers {
+				err := server.Reload()
+				if err != nil {
+					log.Printf("error reloading server: %+v", err)
 				}
-			case syscall.SIGINT, syscall.SIGTERM:
-				log.Println("shutting down ...")
-				for _, server := range servers {
-					err := server.Stop()
-					if err != nil {
-						log.Printf("error stopping server: %s", err.Error())
-					}
+			}
+		case syscall.SIGINT, syscall.SIGTERM:
+			log.Println("shutting down ...")
+			for _, server := range servers {
+				err := server.Stop()
+				if err != nil {
+					log.Printf("error stopping server: %s", err.Error())
 				}
-				return
 			}
+			return
 		}
 	}
 }
diff --git a/ca/tls_options_test.go b/ca/tls_options_test.go
index a422799e8..e2ed4234b 100644
--- a/ca/tls_options_test.go
+++ b/ca/tls_options_test.go
@@ -553,7 +553,7 @@ func equalPools(a, b *x509.CertPool) bool {
 	for i := range subjects {
 		sB[i] = string(subjects[i])
 	}
-	sort.Sort(sort.StringSlice(sA))
-	sort.Sort(sort.StringSlice(sB))
+	sort.Strings(sA)
+	sort.Strings(sB)
 	return reflect.DeepEqual(sA, sB)
 }
diff --git a/logging/handler.go b/logging/handler.go
index 7a8ae0bbb..c59736d92 100644
--- a/logging/handler.go
+++ b/logging/handler.go
@@ -32,7 +32,7 @@ func (l *LoggerHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	t := time.Now()
 	rw := NewResponseLogger(w)
 	l.next.ServeHTTP(rw, r)
-	d := time.Now().Sub(t)
+	d := time.Since(t)
 	l.writeEntry(rw, r, t, d)
 }