Merge pull request #220 from smallstep/identity-cert-duration

Enforce a duration for identity certificates

api/ssh.go

@@ -308,14 +308,6 @@ func (h *caHandler) SSHSign(w http.ResponseWriter, r *http.Request) {
 	// Sign identity certificate if available.
 	var identityCertificate []Certificate
 	if cr := body.IdentityCSR.CertificateRequest; cr != nil {
-		var opts provisioner.Options
-		// Use same duration as ssh certificate for user certificates
-		if cert.CertType == ssh.UserCert {
-			opts = provisioner.Options{
-				NotBefore: provisioner.NewTimeDuration(time.Unix(int64(cert.ValidAfter), 0)),
-				NotAfter:  provisioner.NewTimeDuration(time.Unix(int64(cert.ValidBefore), 0)),
-			}
-		}
 		ctx := authority.NewContextWithSkipTokenReuse(r.Context())
 		ctx = provisioner.NewContextWithMethod(ctx, provisioner.SignMethod)
 		signOpts, err := h.Authority.Authorize(ctx, body.OTT)
@@ -323,7 +315,14 @@ func (h *caHandler) SSHSign(w http.ResponseWriter, r *http.Request) {
 			WriteError(w, errs.UnauthorizedErr(err))
 			return
 		}
-		certChain, err := h.Authority.Sign(cr, opts, signOpts...)
+
+		// Enforce the same duration as ssh certificate.
+		signOpts = append(signOpts, &identityModifier{
+			NotBefore: time.Unix(int64(cert.ValidAfter), 0),
+			NotAfter:  time.Unix(int64(cert.ValidBefore), 0),
+		})
+
+		certChain, err := h.Authority.Sign(cr, provisioner.Options{}, signOpts...)
 		if err != nil {
 			WriteError(w, errs.ForbiddenErr(err))
 			return
@@ -483,3 +482,15 @@ func (h *caHandler) SSHBastion(w http.ResponseWriter, r *http.Request) {
 		Bastion:  bastion,
 	})
 }
+
+// identityModifier is a custom modifier used to force a fixed duration.
+type identityModifier struct {
+	NotBefore time.Time
+	NotAfter  time.Time
+}
+
+func (m *identityModifier) Enforce(cert *x509.Certificate) error {
+	cert.NotBefore = m.NotBefore
+	cert.NotAfter = m.NotAfter
+	return nil
+}

authority/provisioner/sign_options.go

@@ -47,6 +47,13 @@ type ProfileModifier interface {
 	Option(o Options) x509util.WithOption
 }
 
+// CertificateEnforcer is the interface used to modify a certificate after
+// validation.
+type CertificateEnforcer interface {
+	SignOption
+	Enforce(cert *x509.Certificate) error
+}
+
 // profileWithOption is a wrapper against x509util.WithOption to conform the
 // interface.
 type profileWithOption x509util.WithOption

authority/tls.go

@@ -61,9 +61,10 @@ func withDefaultASN1DN(def *x509util.ASN1DN) x509util.WithOption {
 // Sign creates a signed certificate from a certificate signing request.
 func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Options, extraOpts ...provisioner.SignOption) ([]*x509.Certificate, error) {
 	var (
-		opts           = []interface{}{errs.WithKeyVal("csr", csr), errs.WithKeyVal("signOptions", signOpts)}
+		opts            = []interface{}{errs.WithKeyVal("csr", csr), errs.WithKeyVal("signOptions", signOpts)}
-		mods           = []x509util.WithOption{withDefaultASN1DN(a.config.AuthorityConfig.Template)}
+		mods            = []x509util.WithOption{withDefaultASN1DN(a.config.AuthorityConfig.Template)}
-		certValidators = []provisioner.CertificateValidator{}
+		certValidators  = []provisioner.CertificateValidator{}
+		forcedModifiers = []provisioner.CertificateEnforcer{}
 	)
 
 	// Set backdate with the configured value
@@ -79,6 +80,8 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti
 			}
 		case provisioner.ProfileModifier:
 			mods = append(mods, k.Option(signOpts))
+		case provisioner.CertificateEnforcer:
+			forcedModifiers = append(forcedModifiers, k)
 		default:
 			return nil, errs.InternalServer("authority.Sign; invalid extra option type %T", append([]interface{}{k}, opts...)...)
 		}
@@ -93,12 +96,20 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti
 		return nil, errs.Wrap(http.StatusInternalServerError, err, "authority.Sign", opts...)
 	}
 
+	// Certificate validation
 	for _, v := range certValidators {
 		if err := v.Valid(leaf.Subject(), signOpts); err != nil {
 			return nil, errs.Wrap(http.StatusUnauthorized, err, "authority.Sign", opts...)
 		}
 	}
 
+	// Certificate modifier after validation
+	for _, m := range forcedModifiers {
+		if err := m.Enforce(leaf.Subject()); err != nil {
+			return nil, errs.Wrap(http.StatusUnauthorized, err, "authority.Sign", opts...)
+		}
+	}
+
 	crtBytes, err := leaf.CreateCertificate()
 	if err != nil {
 		return nil, errs.Wrap(http.StatusInternalServerError, err,

authority/tls_test.go

@@ -41,6 +41,17 @@ type stepProvisionerASN1 struct {
 	CredentialID []byte
 }
 
+type certificateDurationEnforcer struct {
+	NotBefore time.Time
+	NotAfter  time.Time
+}
+
+func (m *certificateDurationEnforcer) Enforce(cert *x509.Certificate) error {
+	cert.NotBefore = m.NotBefore
+	cert.NotAfter = m.NotAfter
+	return nil
+}
+
 func withProvisionerOID(name, kid string) x509util.WithOption {
 	return func(p x509util.Profile) error {
 		crt := p.Subject()
@@ -114,6 +125,8 @@ func TestAuthority_Sign(t *testing.T) {
 		csr       *x509.CertificateRequest
 		signOpts  provisioner.Options
 		extraOpts []provisioner.SignOption
+		notBefore time.Time
+		notAfter  time.Time
 		err       error
 		code      int
 	}
@@ -253,6 +266,31 @@ ZYtQ9Ot36qc=
 				csr:       csr,
 				extraOpts: extraOpts,
 				signOpts:  signOpts,
+				notBefore: signOpts.NotBefore.Time().Truncate(time.Second),
+				notAfter:  signOpts.NotAfter.Time().Truncate(time.Second),
+			}
+		},
+		"ok with enforced modifier": func(t *testing.T) *signTest {
+			csr := getCSR(t, priv)
+			now := time.Now().UTC()
+			enforcedExtraOptions := append(extraOpts, &certificateDurationEnforcer{
+				NotBefore: now,
+				NotAfter:  now.Add(365 * 24 * time.Hour),
+			})
+			_a := testAuthority(t)
+			_a.db = &db.MockAuthDB{
+				MStoreCertificate: func(crt *x509.Certificate) error {
+					assert.Equals(t, crt.Subject.CommonName, "smallstep test")
+					return nil
+				},
+			}
+			return &signTest{
+				auth:      a,
+				csr:       csr,
+				extraOpts: enforcedExtraOptions,
+				signOpts:  signOpts,
+				notBefore: now.Truncate(time.Second),
+				notAfter:  now.Add(365 * 24 * time.Hour).Truncate(time.Second),
 			}
 		},
 	}
@@ -279,8 +317,8 @@ ZYtQ9Ot36qc=
 				leaf := certChain[0]
 				intermediate := certChain[1]
 				if assert.Nil(t, tc.err) {
-					assert.Equals(t, leaf.NotBefore, signOpts.NotBefore.Time().Truncate(time.Second))
+					assert.Equals(t, leaf.NotBefore, tc.notBefore)
-					assert.Equals(t, leaf.NotAfter, signOpts.NotAfter.Time().Truncate(time.Second))
+					assert.Equals(t, leaf.NotAfter, tc.notAfter)
 					tmplt := a.config.AuthorityConfig.Template
 					assert.Equals(t, fmt.Sprintf("%v", leaf.Subject),
 						fmt.Sprintf("%v", &pkix.Name{