Skip to content

Commit

Permalink
Add checks for other error types within the PKI plugin (#14195)
Browse files Browse the repository at this point in the history
* Add checks for other error types within the PKI plugin

 - The PKI plugin assumes the code it is calling always returns an error
   of type errutil.UserError or errutil.InternalError. While I believe
   so far this is still true, it would be easy to add a code path that
   just returns a generic error and we would completely ignore it.
 - This was found within some managed key testing where I forgot to wrap
   an error within one of the expected types

* Add changelog
stevendpclark authored Feb 22, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 0b58510 commit 34f128f
Showing 6 changed files with 65 additions and 51 deletions.
12 changes: 7 additions & 5 deletions builtin/logical/pki/path_config_crl.go
Original file line number Diff line number Diff line change
@@ -112,11 +112,13 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
if oldDisable != config.Disable {
// It wasn't disabled but now it is, rotate
crlErr := buildCRL(ctx, b, req, true)
switch crlErr.(type) {
case errutil.UserError:
return logical.ErrorResponse(fmt.Sprintf("Error during CRL building: %s", crlErr)), nil
case errutil.InternalError:
return nil, fmt.Errorf("error encountered during CRL building: %w", crlErr)
if crlErr != nil {
switch crlErr.(type) {
case errutil.UserError:
return logical.ErrorResponse(fmt.Sprintf("Error during CRL building: %s", crlErr)), nil
default:
return nil, fmt.Errorf("error encountered during CRL building: %w", crlErr)
}
}
}

20 changes: 11 additions & 9 deletions builtin/logical/pki/path_fetch.go
Original file line number Diff line number Diff line change
@@ -192,13 +192,15 @@ func (b *backend) pathFetchRead(ctx context.Context, req *logical.Request, data

if serial == "ca_chain" {
caInfo, err := fetchCAInfo(ctx, b, req)
switch err.(type) {
case errutil.UserError:
response = logical.ErrorResponse(err.Error())
goto reply
case errutil.InternalError:
retErr = err
goto reply
if err != nil {
switch err.(type) {
case errutil.UserError:
response = logical.ErrorResponse(err.Error())
goto reply
default:
retErr = err
goto reply
}
}

caChain := caInfo.GetCAChain()
@@ -232,7 +234,7 @@ func (b *backend) pathFetchRead(ctx context.Context, req *logical.Request, data
case errutil.UserError:
response = logical.ErrorResponse(funcErr.Error())
goto reply
case errutil.InternalError:
default:
retErr = funcErr
goto reply
}
@@ -260,7 +262,7 @@ func (b *backend) pathFetchRead(ctx context.Context, req *logical.Request, data
case errutil.UserError:
response = logical.ErrorResponse(funcErr.Error())
goto reply
case errutil.InternalError:
default:
retErr = funcErr
goto reply
}
16 changes: 9 additions & 7 deletions builtin/logical/pki/path_issue_sign.go
Original file line number Diff line number Diff line change
@@ -173,13 +173,15 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d

var caErr error
signingBundle, caErr := fetchCAInfo(ctx, b, req)
switch caErr.(type) {
case errutil.UserError:
return nil, errutil.UserError{Err: fmt.Sprintf(
"could not fetch the CA certificate (was one set?): %s", caErr)}
case errutil.InternalError:
return nil, errutil.InternalError{Err: fmt.Sprintf(
"error fetching CA certificate: %s", caErr)}
if caErr != nil {
switch caErr.(type) {
case errutil.UserError:
return nil, errutil.UserError{Err: fmt.Sprintf(
"could not fetch the CA certificate (was one set?): %s", caErr)}
default:
return nil, errutil.InternalError{Err: fmt.Sprintf(
"error fetching CA certificate: %s", caErr)}
}
}

input := &inputBundle{
24 changes: 13 additions & 11 deletions builtin/logical/pki/path_revoke.go
Original file line number Diff line number Diff line change
@@ -69,18 +69,20 @@ func (b *backend) pathRotateCRLRead(ctx context.Context, req *logical.Request, d
defer b.revokeStorageLock.RUnlock()

crlErr := buildCRL(ctx, b, req, false)
switch crlErr.(type) {
case errutil.UserError:
return logical.ErrorResponse(fmt.Sprintf("Error during CRL building: %s", crlErr)), nil
case errutil.InternalError:
return nil, fmt.Errorf("error encountered during CRL building: %w", crlErr)
default:
return &logical.Response{
Data: map[string]interface{}{
"success": true,
},
}, nil
if crlErr != nil {
switch crlErr.(type) {
case errutil.UserError:
return logical.ErrorResponse(fmt.Sprintf("Error during CRL building: %s", crlErr)), nil
default:
return nil, fmt.Errorf("error encountered during CRL building: %w", crlErr)
}
}

return &logical.Response{
Data: map[string]interface{}{
"success": true,
},
}, nil
}

const pathRevokeHelpSyn = `
41 changes: 22 additions & 19 deletions builtin/logical/pki/path_root.go
Original file line number Diff line number Diff line change
@@ -137,7 +137,8 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
}
if entry != nil {
resp := &logical.Response{}
resp.AddWarning(fmt.Sprintf("Refusing to generate a root certificate over an existing root certificate. If you really want to destroy the original root certificate, please issue a delete against %sroot.", req.MountPoint))
resp.AddWarning(fmt.Sprintf("Refusing to generate a root certificate over an existing root certificate. "+
"If you really want to destroy the original root certificate, please issue a delete against %s root.", req.MountPoint))
return resp, nil
}

@@ -162,8 +163,6 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
switch err.(type) {
case errutil.UserError:
return logical.ErrorResponse(err.Error()), nil
case errutil.InternalError:
return nil, err
default:
return nil, err
}
@@ -296,13 +295,15 @@ func (b *backend) pathCASignIntermediate(ctx context.Context, req *logical.Reque

var caErr error
signingBundle, caErr := fetchCAInfo(ctx, b, req)
switch caErr.(type) {
case errutil.UserError:
return nil, errutil.UserError{Err: fmt.Sprintf(
"could not fetch the CA certificate (was one set?): %s", caErr)}
case errutil.InternalError:
return nil, errutil.InternalError{Err: fmt.Sprintf(
"error fetching CA certificate: %s", caErr)}
if caErr != nil {
switch caErr.(type) {
case errutil.UserError:
return nil, errutil.UserError{Err: fmt.Sprintf(
"could not fetch the CA certificate (was one set?): %s", caErr)}
default:
return nil, errutil.InternalError{Err: fmt.Sprintf(
"error fetching CA certificate: %s", caErr)}
}
}

useCSRValues := data.Get("use_csr_values").(bool)
@@ -323,8 +324,9 @@ func (b *backend) pathCASignIntermediate(ctx context.Context, req *logical.Reque
switch err.(type) {
case errutil.UserError:
return logical.ErrorResponse(err.Error()), nil
case errutil.InternalError:
return nil, err
default:
return nil, errutil.InternalError{Err: fmt.Sprintf(
"error signing cert: %s", err)}
}
}

@@ -422,13 +424,14 @@ func (b *backend) pathCASignSelfIssued(ctx context.Context, req *logical.Request

var caErr error
signingBundle, caErr := fetchCAInfo(ctx, b, req)
switch caErr.(type) {
case errutil.UserError:
return nil, errutil.UserError{Err: fmt.Sprintf(
"could not fetch the CA certificate (was one set?): %s", caErr)}
case errutil.InternalError:
return nil, errutil.InternalError{Err: fmt.Sprintf(
"error fetching CA certificate: %s", caErr)}
if caErr != nil {
switch caErr.(type) {
case errutil.UserError:
return nil, errutil.UserError{Err: fmt.Sprintf(
"could not fetch the CA certificate (was one set?): %s", caErr)}
default:
return nil, errutil.InternalError{Err: fmt.Sprintf("error fetching CA certificate: %s", caErr)}
}
}

signingCB, err := signingBundle.ToCertBundle()
3 changes: 3 additions & 0 deletions changelog/14195.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
secrets/pki: Add error handling for error types other than UserError or InternalError
```

0 comments on commit 34f128f

Please sign in to comment.