Trap errors related to vault pki list-intermediate issuer reading (#19165)
* Rename files to match test suite and existing pattern * Factor out issuer loading into a dedicated function - Add a little more checks/validation when loading the a PKI issuer - Factor out the issuer loading into a dedicated function - Leverage existing health check code to parse issuer certificates * Read parent issuer once instead of reloading it for every child - Read in our parent issuer once instead of running it for every child we want to compare against - Provides clearer error message that we have failed reading from which path to the end user * PR Feedback - Rename a variable for clarity - Use readIssuer in the validation of the parent issuer within pkiIssuer - Add some missing return 1 statements in error handlers that had been missed
This commit is contained in:
parent
42369ac698
commit
7d9dd37f88
@ -47,7 +47,7 @@ func parsePEM(contents string) ([]byte, error) {
|
||||
return pemBlock.Bytes, nil
|
||||
}
|
||||
|
||||
func parsePEMCert(contents string) (*x509.Certificate, error) {
|
||||
func ParsePEMCert(contents string) (*x509.Certificate, error) {
|
||||
parsed, err := parsePEM(contents)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@ -89,7 +89,7 @@ func pkiFetchIssuer(e *Executor, issuer string, versionError func()) (bool, *Pat
|
||||
}
|
||||
|
||||
if len(issuerRet.ParsedCache) == 0 {
|
||||
cert, err := parsePEMCert(issuerRet.Secret.Data["certificate"].(string))
|
||||
cert, err := ParsePEMCert(issuerRet.Secret.Data["certificate"].(string))
|
||||
if err != nil {
|
||||
return true, issuerRet, nil, fmt.Errorf("unable to parse issuer %v's certificate: %w", issuer, err)
|
||||
}
|
||||
@ -114,7 +114,7 @@ func pkiFetchIssuerEntry(e *Executor, issuer string, versionError func()) (bool,
|
||||
}
|
||||
|
||||
if len(issuerRet.ParsedCache) == 0 {
|
||||
cert, err := parsePEMCert(issuerRet.Secret.Data["certificate"].(string))
|
||||
cert, err := ParsePEMCert(issuerRet.Secret.Data["certificate"].(string))
|
||||
if err != nil {
|
||||
return true, issuerRet, nil, fmt.Errorf("unable to parse issuer %v's certificate: %w", issuer, err)
|
||||
}
|
||||
@ -222,7 +222,7 @@ func pkiFetchLeaf(e *Executor, serial string, versionError func()) (bool, *PathF
|
||||
}
|
||||
|
||||
if len(leafRet.ParsedCache) == 0 {
|
||||
cert, err := parsePEMCert(leafRet.Secret.Data["certificate"].(string))
|
||||
cert, err := ParsePEMCert(leafRet.Secret.Data["certificate"].(string))
|
||||
if err != nil {
|
||||
return true, leafRet, nil, fmt.Errorf("unable to parse leaf %v's certificate: %w", serial, err)
|
||||
}
|
||||
|
@ -64,12 +64,12 @@ func (h *AllowIfModifiedSince) Evaluate(e *Executor) (results []*Result, err err
|
||||
return []*Result{&ret}, nil
|
||||
}
|
||||
|
||||
req, err := stringList(h.TuneData["passthrough_request_headers"])
|
||||
req, err := StringList(h.TuneData["passthrough_request_headers"])
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("unable to parse value from server for passthrough_request_headers: %w", err)
|
||||
}
|
||||
|
||||
resp, err := stringList(h.TuneData["allowed_response_headers"])
|
||||
resp, err := StringList(h.TuneData["allowed_response_headers"])
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("unable to parse value from server for allowed_response_headers: %w", err)
|
||||
}
|
||||
|
@ -83,7 +83,7 @@ func (h *AuditVisibility) DefaultConfig() map[string]interface{} {
|
||||
func (h *AuditVisibility) LoadConfig(config map[string]interface{}) error {
|
||||
var err error
|
||||
|
||||
coerced, err := stringList(config["ignored_parameters"])
|
||||
coerced, err := StringList(config["ignored_parameters"])
|
||||
if err != nil {
|
||||
return fmt.Errorf("error parsing %v.ignored_parameters: %v", h.Name(), err)
|
||||
}
|
||||
@ -128,7 +128,7 @@ func (h *AuditVisibility) Evaluate(e *Executor) (results []*Result, err error) {
|
||||
"audit_non_hmac_response_keys": VisibleRespParams,
|
||||
}
|
||||
for source, visibleList := range sourceMap {
|
||||
actual, err := stringList(h.TuneData[source])
|
||||
actual, err := StringList(h.TuneData[source])
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error parsing %v from server: %v", source, err)
|
||||
}
|
||||
@ -158,7 +158,7 @@ func (h *AuditVisibility) Evaluate(e *Executor) (results []*Result, err error) {
|
||||
"audit_non_hmac_response_keys": HiddenRespParams,
|
||||
}
|
||||
for source, hiddenList := range sourceMap {
|
||||
actual, err := stringList(h.TuneData[source])
|
||||
actual, err := StringList(h.TuneData[source])
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error parsing %v from server: %v", source, err)
|
||||
}
|
||||
|
@ -6,7 +6,7 @@ import (
|
||||
"github.com/hashicorp/vault/sdk/logical"
|
||||
)
|
||||
|
||||
func stringList(source interface{}) ([]string, error) {
|
||||
func StringList(source interface{}) ([]string, error) {
|
||||
if source == nil {
|
||||
return nil, nil
|
||||
}
|
||||
|
@ -107,10 +107,12 @@ func pkiIssue(c *BaseCommand, parentMountIssuer string, intermediateMount string
|
||||
// Sanity Check the Parent Issuer
|
||||
if !strings.Contains(parentMountIssuer, "/issuer/") {
|
||||
c.UI.Error(fmt.Sprintf("Parent Issuer %v is Not a PKI Issuer Path of the format /mount/issuer/issuer-ref", parentMountIssuer))
|
||||
return 1
|
||||
}
|
||||
_, err = client.Logical().Read(parentMountIssuer + "/json")
|
||||
_, err = readIssuer(client, parentMountIssuer)
|
||||
if err != nil {
|
||||
c.UI.Error(fmt.Sprintf("Unable to access parent issuer %v: %v", parentMountIssuer, err))
|
||||
return 1
|
||||
}
|
||||
|
||||
// Set-up Failure State (Immediately Before First Write Call)
|
||||
@ -231,6 +233,7 @@ func readAndOutputNewCertificate(client *api.Client, intermediateMount string, i
|
||||
if err != nil || resp == nil {
|
||||
c.UI.Error(fmt.Sprintf("Error Reading Fully Imported Certificate from %v : %v",
|
||||
intermediateMount+"/issuer/"+issuerId, err))
|
||||
return
|
||||
}
|
||||
|
||||
OutputSecret(c.UI, resp)
|
||||
|
@ -186,10 +186,16 @@ func (c *PKIListIntermediateCommand) Run(args []string) int {
|
||||
"signature_match": c.flagSignatureMatch,
|
||||
}
|
||||
|
||||
issuerResp, err := readIssuer(client, issuer)
|
||||
if err != nil {
|
||||
c.UI.Error(fmt.Sprintf("Failed to read parent issuer on path %s: %s", issuer, err.Error()))
|
||||
return 1
|
||||
}
|
||||
|
||||
for _, child := range issued {
|
||||
path := sanitizePath(child)
|
||||
if path != "" {
|
||||
err, verifyResults := verifySignBetween(client, issuer, path)
|
||||
verifyResults, err := verifySignBetween(client, issuerResp, path)
|
||||
if err != nil {
|
||||
c.UI.Error(fmt.Sprintf("Failed to run verification on path %v: %v", path, err))
|
||||
return 1
|
@ -6,7 +6,6 @@ import (
|
||||
"crypto/rsa"
|
||||
"crypto/x509"
|
||||
"encoding/hex"
|
||||
"encoding/pem"
|
||||
"fmt"
|
||||
"io"
|
||||
"net"
|
||||
@ -93,31 +92,25 @@ func (c *PKIReIssueCACommand) Run(args []string) int {
|
||||
}
|
||||
|
||||
parentIssuer := sanitizePath(args[0]) // /pki/issuer/default
|
||||
templateIssuer := sanitizePath(args[1])
|
||||
intermediateMount := sanitizePath(args[2])
|
||||
|
||||
templateCertificateResp, err := client.Logical().Read(sanitizePath(args[1]))
|
||||
templateIssuerBundle, err := readIssuer(client, templateIssuer)
|
||||
if err != nil {
|
||||
c.UI.Error(fmt.Sprintf("Error fetching template certificate %v : %v", sanitizePath(args[1]), err))
|
||||
return 1
|
||||
}
|
||||
templateCertificateRaw, ok := templateCertificateResp.Data["certificate"]
|
||||
if !ok {
|
||||
c.UI.Error(fmt.Sprintf("No Certificate Field Found at %v instead found : %v", sanitizePath(args[1]), templateCertificateResp))
|
||||
return 1
|
||||
}
|
||||
certificatePemString := templateCertificateRaw.(string)
|
||||
certificatePem := []byte(certificatePemString)
|
||||
certificateBlock, _ := pem.Decode(certificatePem)
|
||||
certificate, err := x509.ParseCertificate(certificateBlock.Bytes)
|
||||
if err != nil {
|
||||
c.UI.Error(fmt.Sprintf("Error parsing template certificate at %v : %v", sanitizePath(args[1]), err))
|
||||
c.UI.Error(fmt.Sprintf("Error fetching template certificate %v : %v", templateIssuer, err))
|
||||
return 1
|
||||
}
|
||||
certificate := templateIssuerBundle.certificate
|
||||
|
||||
useExistingKey := c.flagKeyStorageSource == "existing"
|
||||
keyRef := ""
|
||||
if useExistingKey { // TODO: Better Error information
|
||||
keyRef = templateCertificateResp.Data["key_id"].(string)
|
||||
if useExistingKey {
|
||||
keyRef = templateIssuerBundle.keyId
|
||||
|
||||
if keyRef == "" {
|
||||
c.UI.Error(fmt.Sprintf("Template issuer %s did not have a key id field set in response which is required", templateIssuer))
|
||||
return 1
|
||||
}
|
||||
}
|
||||
|
||||
templateData, err := parseTemplateCertificate(*certificate, useExistingKey, keyRef)
|
||||
|
@ -8,9 +8,10 @@ import (
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/hashicorp/vault/command/healthcheck"
|
||||
|
||||
"github.com/ghodss/yaml"
|
||||
"github.com/hashicorp/vault/api"
|
||||
"github.com/hashicorp/vault/sdk/helper/certutil"
|
||||
"github.com/ryanuber/columnize"
|
||||
)
|
||||
|
||||
@ -93,7 +94,13 @@ func (c *PKIVerifySignCommand) Run(args []string) int {
|
||||
return 1
|
||||
}
|
||||
|
||||
err, results := verifySignBetween(client, issuer, issued)
|
||||
issuerResp, err := readIssuer(client, issuer)
|
||||
if err != nil {
|
||||
c.UI.Error(fmt.Sprintf("Failed to read issuer: %s: %s", issuer, err.Error()))
|
||||
return 1
|
||||
}
|
||||
|
||||
results, err := verifySignBetween(client, issuerResp, issued)
|
||||
if err != nil {
|
||||
c.UI.Error(fmt.Sprintf("Failed to run verification: %v", err))
|
||||
return pkiRetUsage
|
||||
@ -104,60 +111,34 @@ func (c *PKIVerifySignCommand) Run(args []string) int {
|
||||
return 0
|
||||
}
|
||||
|
||||
func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) (error, map[string]bool) {
|
||||
func verifySignBetween(client *api.Client, issuerResp *issuerResponse, issuedPath string) (map[string]bool, error) {
|
||||
// Note that this eats warnings
|
||||
|
||||
// Fetch and Parse the Potential Issuer:
|
||||
issuerResp, err := client.Logical().Read(issuerPath)
|
||||
if err != nil {
|
||||
return fmt.Errorf("error: unable to fetch issuer %v: %w", issuerPath, err), nil
|
||||
}
|
||||
issuerCertPem := issuerResp.Data["certificate"].(string)
|
||||
issuerCertBundle, err := certutil.ParsePEMBundle(issuerCertPem)
|
||||
if err != nil {
|
||||
return err, nil
|
||||
}
|
||||
issuerKeyId := issuerCertBundle.Certificate.SubjectKeyId
|
||||
issuerCert := issuerResp.certificate
|
||||
issuerKeyId := issuerCert.SubjectKeyId
|
||||
|
||||
// Fetch and Parse the Potential Issued Cert
|
||||
issuedCertResp, err := client.Logical().Read(issuedPath)
|
||||
issuedCertBundle, err := readIssuer(client, issuedPath)
|
||||
if err != nil {
|
||||
return fmt.Errorf("error: unable to fetch issuer %v: %w", issuerPath, err), nil
|
||||
return nil, fmt.Errorf("error: unable to fetch issuer %v: %w", issuedPath, err)
|
||||
}
|
||||
if len(issuedPath) <= 2 {
|
||||
return fmt.Errorf("%v", issuedPath), nil
|
||||
}
|
||||
caChainRaw := issuedCertResp.Data["ca_chain"]
|
||||
if caChainRaw == nil {
|
||||
return fmt.Errorf("no ca_chain information on %v", issuedPath), nil
|
||||
}
|
||||
caChainCast := caChainRaw.([]interface{})
|
||||
caChain := make([]string, len(caChainCast))
|
||||
for i, cert := range caChainCast {
|
||||
caChain[i] = cert.(string)
|
||||
}
|
||||
issuedCertPem := issuedCertResp.Data["certificate"].(string)
|
||||
issuedCertBundle, err := certutil.ParsePEMBundle(issuedCertPem)
|
||||
if err != nil {
|
||||
return err, nil
|
||||
}
|
||||
parentKeyId := issuedCertBundle.Certificate.AuthorityKeyId
|
||||
parentKeyId := issuedCertBundle.certificate.AuthorityKeyId
|
||||
|
||||
// Check the Chain-Match
|
||||
rootCertPool := x509.NewCertPool()
|
||||
rootCertPool.AddCert(issuerCertBundle.Certificate)
|
||||
rootCertPool.AddCert(issuerCert)
|
||||
checkTrustPathOptions := x509.VerifyOptions{
|
||||
Roots: rootCertPool,
|
||||
}
|
||||
trust := false
|
||||
trusts, err := issuedCertBundle.Certificate.Verify(checkTrustPathOptions)
|
||||
trusts, err := issuedCertBundle.certificate.Verify(checkTrustPathOptions)
|
||||
if err != nil && !strings.Contains(err.Error(), "certificate signed by unknown authority") {
|
||||
return err, nil
|
||||
return nil, err
|
||||
} else if err == nil {
|
||||
for _, chain := range trusts {
|
||||
// Output of this Should Only Have One Trust with Chain of Length Two (Child followed by Parent)
|
||||
for _, cert := range chain {
|
||||
if issuedCertBundle.Certificate.Equal(cert) {
|
||||
if issuedCertBundle.certificate.Equal(cert) {
|
||||
trust = true
|
||||
break
|
||||
}
|
||||
@ -166,29 +147,113 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string)
|
||||
}
|
||||
|
||||
pathMatch := false
|
||||
for _, cert := range caChain {
|
||||
if strings.TrimSpace(cert) == strings.TrimSpace(issuerCertPem) { // TODO: Decode into ASN1 to Check
|
||||
for _, cert := range issuedCertBundle.caChain {
|
||||
if bytes.Equal(cert.Raw, issuerCert.Raw) {
|
||||
pathMatch = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
signatureMatch := false
|
||||
err = issuedCertBundle.Certificate.CheckSignatureFrom(issuerCertBundle.Certificate)
|
||||
err = issuedCertBundle.certificate.CheckSignatureFrom(issuerCert)
|
||||
if err == nil {
|
||||
signatureMatch = true
|
||||
}
|
||||
|
||||
result := map[string]bool{
|
||||
// This comparison isn't strictly correct, despite a standard ordering these are sets
|
||||
"subject_match": bytes.Equal(issuerCertBundle.Certificate.RawSubject, issuedCertBundle.Certificate.RawIssuer),
|
||||
"subject_match": bytes.Equal(issuerCert.RawSubject, issuedCertBundle.certificate.RawIssuer),
|
||||
"path_match": pathMatch,
|
||||
"trust_match": trust, // TODO: Refactor into a reasonable function
|
||||
"key_id_match": bytes.Equal(parentKeyId, issuerKeyId),
|
||||
"signature_match": signatureMatch,
|
||||
}
|
||||
|
||||
return nil, result
|
||||
return result, nil
|
||||
}
|
||||
|
||||
type issuerResponse struct {
|
||||
keyId string
|
||||
certificate *x509.Certificate
|
||||
caChain []*x509.Certificate
|
||||
}
|
||||
|
||||
func readIssuer(client *api.Client, issuerPath string) (*issuerResponse, error) {
|
||||
issuerResp, err := client.Logical().Read(issuerPath)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
issuerCertPem, err := requireStrRespField(issuerResp, "certificate")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
issuerCert, err := healthcheck.ParsePEMCert(issuerCertPem)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("unable to parse issuer %v's certificate: %w", issuerPath, err)
|
||||
}
|
||||
|
||||
caChainPem, err := requireStrListRespField(issuerResp, "ca_chain")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("unable to parse issuer %v's CA chain: %w", issuerPath, err)
|
||||
}
|
||||
|
||||
var caChain []*x509.Certificate
|
||||
for _, pem := range caChainPem {
|
||||
trimmedPem := strings.TrimSpace(pem)
|
||||
if trimmedPem == "" {
|
||||
continue
|
||||
}
|
||||
cert, err := healthcheck.ParsePEMCert(trimmedPem)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
caChain = append(caChain, cert)
|
||||
}
|
||||
|
||||
keyId := optStrRespField(issuerResp, "key_id")
|
||||
|
||||
return &issuerResponse{
|
||||
keyId: keyId,
|
||||
certificate: issuerCert,
|
||||
caChain: caChain,
|
||||
}, nil
|
||||
}
|
||||
|
||||
func optStrRespField(resp *api.Secret, reqField string) string {
|
||||
if resp == nil || resp.Data == nil {
|
||||
return ""
|
||||
}
|
||||
if val, present := resp.Data[reqField]; !present {
|
||||
return ""
|
||||
} else if strVal, castOk := val.(string); !castOk || strVal == "" {
|
||||
return ""
|
||||
} else {
|
||||
return strVal
|
||||
}
|
||||
}
|
||||
|
||||
func requireStrRespField(resp *api.Secret, reqField string) (string, error) {
|
||||
if resp == nil || resp.Data == nil {
|
||||
return "", fmt.Errorf("nil response received, %s field unavailable", reqField)
|
||||
}
|
||||
if val, present := resp.Data[reqField]; !present {
|
||||
return "", fmt.Errorf("response did not contain field: %s", reqField)
|
||||
} else if strVal, castOk := val.(string); !castOk || strVal == "" {
|
||||
return "", fmt.Errorf("field %s value was blank or not a string: %v", reqField, val)
|
||||
} else {
|
||||
return strVal, nil
|
||||
}
|
||||
}
|
||||
|
||||
func requireStrListRespField(resp *api.Secret, reqField string) ([]string, error) {
|
||||
if resp == nil || resp.Data == nil {
|
||||
return nil, fmt.Errorf("nil response received, %s field unavailable", reqField)
|
||||
}
|
||||
if val, present := resp.Data[reqField]; !present {
|
||||
return nil, fmt.Errorf("response did not contain field: %s", reqField)
|
||||
} else {
|
||||
return healthcheck.StringList(val)
|
||||
}
|
||||
}
|
||||
|
||||
func (c *PKIVerifySignCommand) outputResults(results map[string]bool, potentialParent, potentialChild string) error {
|
Loading…
Reference in New Issue
Block a user