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.

Sign up for GitHub

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

Continue and warn when tidying in pki if an entry or value is nil #4214

Merged
merged 1 commit into from
Mar 29, 2018
Merged

Continue and warn when tidying in pki if an entry or value is nil #4214

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 34 additions & 8 deletions builtin/logical/pki/path_tidy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"time"

"github.com/hashicorp/errwrap"
"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/logical/framework"
)
Expand Down Expand Up @@ -54,6 +55,8 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr

bufferDuration := time.Duration(safetyBuffer) * time.Second

var resp *logical.Response

if tidyCertStore {
serials, err := req.Storage.List(ctx, "certs/")
if err != nil {
Expand All @@ -67,11 +70,23 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr
}

if certEntry == nil {
return nil, fmt.Errorf("certificate entry for serial %s is nil", serial)
if resp == nil {
resp = &logical.Response{}
}
resp.AddWarning(fmt.Sprintf("Certificate entry for serial %s is nil; tidying up since it is no longer useful for any server operations", serial))
if err := req.Storage.Delete(ctx, "certs/"+serial); err != nil {
return nil, errwrap.Wrapf(fmt.Sprintf("error deleting nil entry with serial %s: {{err}}", serial), err)
}
}

if certEntry.Value == nil || len(certEntry.Value) == 0 {
return nil, fmt.Errorf("found entry for serial %s but actual certificate is empty", serial)
if resp == nil {
resp = &logical.Response{}
}
resp.AddWarning(fmt.Sprintf("Certificate entry for serial %s is nil; tidying up since it is no longer useful for any server operations", serial))
if err := req.Storage.Delete(ctx, "certs/"+serial); err != nil {
return nil, errwrap.Wrapf(fmt.Sprintf("error deleting entry with nil value with serial %s: {{err}}", serial), err)
}
}

cert, err := x509.ParseCertificate(certEntry.Value)
Expand Down Expand Up @@ -104,14 +119,25 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr
if err != nil {
return nil, fmt.Errorf("unable to fetch revoked cert with serial %s: %s", serial, err)
}

if revokedEntry == nil {
return nil, fmt.Errorf("revoked certificate entry for serial %s is nil", serial)
if resp == nil {
resp = &logical.Response{}
}
resp.AddWarning(fmt.Sprintf("Revoked entry for serial %s is nil; tidying up since it is no longer useful for any server operations", serial))
if err := req.Storage.Delete(ctx, "revoked/"+serial); err != nil {
return nil, errwrap.Wrapf(fmt.Sprintf("error deleting nil revoked entry with serial %s: {{err}}", serial), err)
}
}

if revokedEntry.Value == nil || len(revokedEntry.Value) == 0 {
// TODO: In this case, remove it and continue? How likely is this to
// happen? Alternately, could skip it entirely, or could implement a
// delete function so that there is a way to remove these
return nil, fmt.Errorf("found revoked serial but actual certificate is empty")
if resp == nil {
resp = &logical.Response{}
}
resp.AddWarning(fmt.Sprintf("Revoked entry for serial %s has nil value; tidying up since it is no longer useful for any server operations", serial))
if err := req.Storage.Delete(ctx, "revoked/"+serial); err != nil {
return nil, errwrap.Wrapf(fmt.Sprintf("error deleting revoked entry with nil value with serial %s: {{err}}", serial), err)
}
}

err = revokedEntry.DecodeJSON(&revInfo)
Expand Down Expand Up @@ -139,7 +165,7 @@ func (b *backend) pathTidyWrite(ctx context.Context, req *logical.Request, d *fr
}
}

return nil, nil
return resp, nil
}

const pathTidyHelpSyn = `
Expand Down