diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index e0aba2b7f8..1666629049 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -617,8 +617,9 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s Operation: logical.UpdateOperation, Path: "root/generate/exported", Data: map[string]interface{}{ - "common_name": "Root Cert", - "ttl": "180h", + "common_name": "Root Cert", + "ttl": "180h", + "not_before_duration": "3h", }, Check: func(resp *logical.Response) error { if resp.Secret != nil && resp.Secret.LeaseID != "" { @@ -2247,8 +2248,9 @@ func runTestSignVerbatim(t *testing.T, keyType string) { // generate root rootData := map[string]interface{}{ - "common_name": "test.com", - "not_after": "9999-12-31T23:59:59Z", + "common_name": "test.com", + "not_after": "9999-12-31T23:59:59Z", + "not_before_duration": "3h", } resp, err := b.HandleRequest(context.Background(), &logical.Request{ @@ -7599,6 +7601,87 @@ func TestIssuance_AlwaysEnforceErr(t *testing.T) { }) } +// TestIssuance_NotBefore verifies that issuance fails with a NotAfter in the past. +func TestIssuance_NotBefore(t *testing.T) { + t.Parallel() + b, s := CreateBackendWithStorage(t) + + resp, err := CBWrite(b, s, "root/generate/internal", map[string]interface{}{ + "common_name": "root myvault.com", + "key_type": "ec", + "ttl": "10h", + "issuer_name": "root-ca", + "key_name": "root-key", + }) + requireSuccessNonNilResponse(t, resp, err, "expected root generation to succeed") + + resp, err = CBWrite(b, s, "roles/test-role", map[string]interface{}{ + "allow_any_name": true, + "key_type": "ec", + "allowed_serial_numbers": "*", + }) + + // Make sure sign-intermediate fails if the NotAfter is in the past + t.Run("ca-issuance", func(t *testing.T) { + resp, err = CBWrite(b, s, "intermediate/generate/internal", map[string]interface{}{ + "common_name": "myint.com", + }) + requireSuccessNonNilResponse(t, resp, err, "failed generating intermediary CSR") + requireFieldsSetInResp(t, resp, "csr") + csr := resp.Data["csr"] + + _, err = CBWrite(b, s, "issuer/root-ca/sign-intermediate", map[string]interface{}{ + "csr": csr, + "use_csr_values": true, + "not_before_duration": "1s", + "not_after": time.Now().Add(-2 * time.Hour).Format(time.RFC3339), + }) + require.ErrorContains(t, err, "notAfter before notBefore", "expected error from sign csr got: %v", resp) + + _, err = CBWrite(b, s, "issuer/root-ca/sign-intermediate", map[string]interface{}{ + "csr": csr, + "use_csr_values": true, + "not_before_duration": "1s", + "not_after": time.Now().Add(2 * time.Hour).Format(time.RFC3339), + }) + requireSuccessNonNilResponse(t, resp, err, "notAfter after notBefore") + time.Sleep(time.Second) + }) + + // Make sure sign of leaf cert fails if the NotAfter is in the past + t.Run("sign-leaf-csr", func(t *testing.T) { + _, csrPem := generateTestCsr(t, certutil.ECPrivateKey, 256) + + resp, err = CBWrite(b, s, "issuer/root-ca/sign/test-role", map[string]interface{}{ + "csr": csrPem, + "not_after": time.Now().Add(-2 * time.Hour).Format(time.RFC3339), + }) + require.ErrorContains(t, err, "notAfter before notBefore", "expected error from sign csr got: %v", resp) + + resp, err = CBWrite(b, s, "issuer/root-ca/sign/test-role", map[string]interface{}{ + "csr": csrPem, + "not_after": time.Now().Add(2 * time.Hour).Format(time.RFC3339), + }) + requireSuccessNonNilResponse(t, resp, err, "sign should have succeeded with a NotAfter in the future") + }) + + // Make sure issue of leaf cert fails if the NotAfter is in the past + t.Run("issue-leaf-csr", func(t *testing.T) { + resp, err = CBWrite(b, s, "issuer/root-ca/issue/test-role", map[string]interface{}{ + "common_name": "leaf.example.com", + "not_after": time.Now().Add(-2 * time.Hour).Format(time.RFC3339), + }) + require.ErrorContains(t, err, "notAfter before notBefore", "expected error from issue got: %v", resp) + + // Make sure it works if we are under + resp, err = CBWrite(b, s, "issuer/root-ca/issue/test-role", map[string]interface{}{ + "common_name": "leaf.example.com", + "not_after": time.Now().Add(2 * time.Hour).Format(time.RFC3339), + }) + requireSuccessNonNilResponse(t, resp, err, "issue should have worked with a NotAfter in the future") + }) +} + // TestIssuance_SignIntermediateKeyUsages tests the field "key_usage" both when directly generating a root certificate, // and when signing a CA CSR. In particular, this test verifies that: // - the key usage DigitalSignature is added if present @@ -7701,3 +7784,122 @@ var ( edCAKey string edCACert string ) + +func getCACert(notBefore, notAfter time.Time) (string, error) { + caCertTemplate := &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "root.localhost", + }, + DNSNames: []string{"root.localhost"}, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + SerialNumber: big.NewInt(mathrand.Int63()), + NotBefore: notBefore, + NotAfter: notAfter, + BasicConstraintsValid: true, + IsCA: true, + } + + rak, err := cryptoutil.GenerateRSAKey(rand.Reader, 2048) + if err != nil { + return "", err + } + + caBytes, err := x509.CreateCertificate(rand.Reader, caCertTemplate, caCertTemplate, rak.Public(), rak) + if err != nil { + panic(err) + } + pemCert := string(pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: caBytes, + })) + + pemKey := string(pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(rak), + })) + + return pemCert + "\n" + pemKey, nil +} + +func requireResponse(t *testing.T, expectErr bool, resp *logical.Response, err error, msgAndArgs ...interface{}) { + t.Helper() + + if expectErr { + if err == nil { + err = resp.Error() + } + require.Error(t, err, msgAndArgs) + } else { + requireSuccessNonNilResponse(t, resp, err, msgAndArgs) + } +} + +// TestIssuance_ValidityPeriodContainedByCA attempt to issue/sign certs with a 1h ttl, +// having first imported a CA with NotBefore/NotAfter set to test various conditions. +func TestIssuance_ValidityPeriodContainedByCA(t *testing.T) { + for _, tc := range []struct { + name string + caNotBefore time.Time + caNotAfter time.Time + expectErr bool + }{ + { + name: "valid", + caNotBefore: time.Now().Add(-time.Hour), + caNotAfter: time.Now().Add(5 * time.Hour), + expectErr: false, + }, + { + name: "caExpired", + caNotBefore: time.Now().Add(-time.Hour), + caNotAfter: time.Now().Add(-time.Minute), + expectErr: true, + }, + { + name: "caWillExpire", + caNotBefore: time.Now().Add(-time.Hour), + caNotAfter: time.Now().Add(30 * time.Minute), + expectErr: true, + }, + { + name: "caNotYet", + caNotBefore: time.Now().Add(time.Hour), + caNotAfter: time.Now().Add(2 * time.Hour), + expectErr: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + b, s := CreateBackendWithStorage(t) + + ca, err := getCACert(tc.caNotBefore, tc.caNotAfter) + require.NoError(t, err) + + resp, err := CBWrite(b, s, "config/ca", map[string]interface{}{"pem_bundle": ca}) + requireSuccessNonNilResponse(t, resp, err) + + resp, err = CBWrite(b, s, "roles/test-role", map[string]interface{}{ + "allow_any_name": true, + "key_type": "ec", + "allowed_serial_numbers": "*", + }) + + t.Run("sign-leaf-csr", func(t *testing.T) { + _, csrPem := generateTestCsr(t, certutil.ECPrivateKey, 256) + + resp, err = CBWrite(b, s, "sign/test-role", map[string]interface{}{ + "csr": csrPem, + "not_after": time.Now().Add(time.Hour).Format(time.RFC3339), + }) + requireResponse(t, tc.expectErr, resp, err) + }) + + t.Run("issue-leaf-csr", func(t *testing.T) { + resp, err = CBWrite(b, s, "issue/test-role", map[string]interface{}{ + "common_name": "leaf.example.com", + "not_after": time.Now().Add(time.Hour).Format(time.RFC3339), + }) + requireResponse(t, tc.expectErr, resp, err) + }) + }) + } +} diff --git a/builtin/logical/pki/issuing/issue_common.go b/builtin/logical/pki/issuing/issue_common.go index 057a69ce4f..1fd2ad1a1d 100644 --- a/builtin/logical/pki/issuing/issue_common.go +++ b/builtin/logical/pki/issuing/issue_common.go @@ -999,6 +999,15 @@ func GetCertificateNotAfter(b logical.SystemView, role *RoleEntry, input CertNot if err != nil { return time.Time{}, warnings, err } + notBefore := time.Now().Add(-role.NotBeforeDuration) + if notAfter.Before(notBefore) { + return time.Time{}, warnings, errutil.UserError{Err: "notAfter before notBefore"} + } + + if caSign != nil && notBefore.Before(caSign.Certificate.NotBefore) { + return time.Time{}, warnings, errutil.UserError{Err: "notBefore before signer's notBefore"} + } + return notAfter, warnings, nil } diff --git a/changelog/_10478.txt b/changelog/_10478.txt new file mode 100644 index 0000000000..47b526047c --- /dev/null +++ b/changelog/_10478.txt @@ -0,0 +1,3 @@ +```release-note:bug +secrets/pki: Return error when issuing/signing certs whose NotAfter is before NotBefore or whose validity period isn't contained by the CA's. +``` \ No newline at end of file