Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address some small nits in PKI rotation #15019

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 20 additions & 28 deletions builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3855,25 +3855,7 @@ func TestBackend_RevokePlusTidy_Intermediate(t *testing.T) {

// Get CRL and ensure the tidied cert is still in the list after the tidy
// operation since it's not past the NotAfter (ttl) value yet.
req := client.NewRequest("GET", "/v1/pki/crl")
resp, err := client.RawRequest(req)
if err != nil {
t.Fatal(err)
}
defer resp.Body.Close()

crlBytes, err := ioutil.ReadAll(resp.Body)
if err != nil {
t.Fatalf("err: %s", err)
}
if len(crlBytes) == 0 {
t.Fatalf("expected CRL in response body")
}

crl, err := x509.ParseDERCRL(crlBytes)
if err != nil {
t.Fatal(err)
}
crl := getParsedCrl(t, client, "pki")

revokedCerts := crl.TBSCertList.RevokedCertificates
if len(revokedCerts) == 0 {
Expand Down Expand Up @@ -3986,30 +3968,36 @@ func TestBackend_RevokePlusTidy_Intermediate(t *testing.T) {
}
}

req = client.NewRequest("GET", "/v1/pki/crl")
resp, err = client.RawRequest(req)
crl = getParsedCrl(t, client, "pki")

revokedCerts = crl.TBSCertList.RevokedCertificates
if len(revokedCerts) != 0 {
t.Fatal("expected CRL to be empty")
}
}

func getParsedCrl(t *testing.T, client *api.Client, mountPoint string) *pkix.CertificateList {
path := fmt.Sprintf("/v1/%s/crl", mountPoint)
req := client.NewRequest("GET", path)
resp, err := client.RawRequest(req)
if err != nil {
t.Fatal(err)
}
defer resp.Body.Close()

crlBytes, err = ioutil.ReadAll(resp.Body)
crlBytes, err := ioutil.ReadAll(resp.Body)
if err != nil {
t.Fatalf("err: %s", err)
}
if len(crlBytes) == 0 {
t.Fatalf("expected CRL in response body")
}

crl, err = x509.ParseDERCRL(crlBytes)
crl, err := x509.ParseDERCRL(crlBytes)
if err != nil {
t.Fatal(err)
}

revokedCerts = crl.TBSCertList.RevokedCertificates
if len(revokedCerts) != 0 {
t.Fatal("expected CRL to be empty")
}
return crl
}

func TestBackend_Root_FullCAChain(t *testing.T) {
Expand Down Expand Up @@ -4141,6 +4129,10 @@ func runFullCAChainTest(t *testing.T, keyType string) {
t.Fatal("expected intermediate chain information")
}

// Verify we have a proper CRL now
crl := getParsedCrl(t, client, "pki-intermediate")
require.Equal(t, 0, len(crl.TBSCertList.RevokedCertificates))

fullChain = resp.Data["ca_chain"].(string)
if !strings.Contains(fullChain, intermediateCert) {
t.Fatal("expected full chain to contain intermediate certificate")
Expand Down
7 changes: 7 additions & 0 deletions builtin/logical/pki/path_manage_issuers.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,13 @@ func (b *backend) pathImportIssuers(ctx context.Context, req *logical.Request, d
}
}

if len(createdIssuers) > 0 {
err := buildCRL(ctx, b, req, true)
if err != nil {
return nil, err
}
}

return &logical.Response{
Data: map[string]interface{}{
"mapping": issuerKeyMap,
Expand Down
10 changes: 0 additions & 10 deletions builtin/logical/pki/path_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,16 +190,6 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
return nil, fmt.Errorf("unable to store certificate locally: %w", err)
}

// For ease of later use, also store just the certificate at a known
// location
err = req.Storage.Put(ctx, &logical.StorageEntry{
Key: "ca",
Value: parsedBundle.CertificateBytes,
})
if err != nil {
return nil, err
}

// Build a fresh CRL
err = buildCRL(ctx, b, req, true)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion builtin/logical/pki/storage_migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func migrateStorage(ctx context.Context, req *logical.InitializationRequest, log

logger.Warn("performing PKI migration to new keys/issuers layout")

anIssuer, aKey, err := writeCaBundle(ctx, s, legacyBundle, "", "")
anIssuer, aKey, err := writeCaBundle(ctx, s, legacyBundle, "current", "current")
if err != nil {
return err
}
Expand Down
2 changes: 2 additions & 0 deletions builtin/logical/pki/storage_migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,11 @@ func Test_migrateStorageSimpleBundle(t *testing.T) {
keyId := keyIds[0]
issuer, err := fetchIssuerById(ctx, s, issuerId)
require.NoError(t, err)
require.Equal(t, "current", issuer.Name) // RFC says we should import with Name=current

key, err := fetchKeyById(ctx, s, keyId)
require.NoError(t, err)
require.Equal(t, "current", key.Name) // RFC says we should import with Name=current

require.Equal(t, issuerId, issuer.ID)
require.Equal(t, bundle.SerialNumber, issuer.SerialNumber)
Expand Down