From 6ccd5b1def9788a5b71e6151a544adcb4b066510 Mon Sep 17 00:00:00 2001 From: kitography Date: Fri, 16 Dec 2022 09:36:15 -0500 Subject: [PATCH 01/21] The verify-sign command in it's cleanest existing form. --- command/commands.go | 5 + command/pki_verify_sign_command.go | 202 +++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 command/pki_verify_sign_command.go diff --git a/command/commands.go b/command/commands.go index 35465ef81136..903bf5fad637 100644 --- a/command/commands.go +++ b/command/commands.go @@ -526,6 +526,11 @@ func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) { BaseCommand: getBaseCommand(), }, nil }, + "pki verify-sign": func() (cli.Command, error) { + return &PKIVerifySignCommand{ + BaseCommand: getBaseCommand(), + }, nil + }, "plugin": func() (cli.Command, error) { return &PluginCommand{ BaseCommand: getBaseCommand(), diff --git a/command/pki_verify_sign_command.go b/command/pki_verify_sign_command.go new file mode 100644 index 000000000000..8170865a9cc9 --- /dev/null +++ b/command/pki_verify_sign_command.go @@ -0,0 +1,202 @@ +package command + +import ( + "crypto/x509" + "encoding/json" + "fmt" + "github.com/ghodss/yaml" + "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/sdk/helper/certutil" + "github.com/ryanuber/columnize" + "strconv" + "strings" +) + +type PKIVerifySignCommand struct { + *BaseCommand + + flagConfig string + flagReturnIndicator string + flagDefaultDisabled bool + flagList bool +} + +func (c *PKIVerifySignCommand) Synopsis() string { + return "Check Whether One Certificate Validates Another Specified Certificate" +} + +func (c *PKIVerifySignCommand) Help() string { + helpText := ` +Usage: vault pki verify-sign POSSIBLE-ISSUER POSSIBLE-ISSUED +Returns four fields of information: +- signature_match: was the key of the issuer used to sign the issued +- path_match: the possible issuer appears in the valid certificate chain of the issued +- key_id_match: does the key-id of the issuer match the key_id of the subject +- subject_match: does the subject name of the issuer match the issuer subject of the issued +` + return strings.TrimSpace(helpText) +} + +func (c *PKIVerifySignCommand) Flags() *FlagSets { + set := c.flagSet(FlagSetHTTP | FlagSetOutputFormat) + return set +} + +func (c *PKIVerifySignCommand) Run(args []string) int { + f := c.Flags() + if err := f.Parse(args); err != nil { + c.UI.Error(err.Error()) + return 1 + } + + args = f.Args() + + if len(args) < 2 { + if len(args) == 0 { + c.UI.Error("Not enough arguments (expected potential issuer and issued, got nothing)") + } else { + c.UI.Error("Not enough arguments (expected both potential issuer and issued, got only one)") + } + return 1 + } else if len(args) > 2 { + c.UI.Error(fmt.Sprintf("Too many arguments (expected only potential issuer and issued, got %d arguments)", len(args))) + for _, arg := range args { + if strings.HasPrefix(arg, "-") { + c.UI.Warn(fmt.Sprintf("Options (%v) must be specified before positional arguments (%v)", arg, args[0])) + break + } + } + return 1 + } + + issuer := sanitizePath(args[0]) + issued := sanitizePath(args[1]) + + err, results := verifySignBetween(c.client, issuer, issued) + if err != nil { + c.UI.Error(fmt.Sprintf("Failed to run verification: %v", err)) + return pkiRetUsage + } + + c.outputResults(results) + + return 0 +} + +func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) (error, map[string]bool) { + // TODO: Stop Eating Warnings Here + + // 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 + } + issuerName := issuerCertBundle.Certificate.Subject + issuerKeyId := issuerCertBundle.Certificate.SubjectKeyId + + // Fetch and Parse the Potential Issued Cert + issuedCertResp, err := client.Logical().Read(issuedPath) + if err != nil { + return fmt.Errorf("error: unable to fetch issuer %v: %w", issuerPath, err), nil + } + caChain := issuedCertResp.Data["ca_chain"].(string) + issuedCertPem := issuedCertResp.Data["certificate"].(string) + issuedCertBundle, err := certutil.ParsePEMBundle(issuedCertPem) + if err != nil { + return err, nil + } + parentIssuerName := issuerCertBundle.Certificate.Issuer + parentKeyId := issuerCertBundle.Certificate.AuthorityKeyId + + // Check the Chain-Match + rootCertPool := x509.NewCertPool() // TODO: Check if it matters to use Root Only Here (There's also an Intermediate Cert Pool) + rootCertPool.AddCert(issuerCertBundle.Certificate) + checkTrustPathOptions := x509.VerifyOptions{ + Roots: rootCertPool, + } + trusts, err := issuedCertBundle.Certificate.Verify(checkTrustPathOptions) + trust := false + for _, chain := range trusts { + for _, cert := range chain { + if issuedCertBundle.Certificate.Equal(cert) { + trust = true + } + } + } + + result := map[string]bool{ + "subject_match": parentIssuerName.String() == issuerName.String(), // TODO: No Equals Defined on Name, Check This + "path_match": strings.Contains(caChain, issuerCertPem), // TODO: Check Trimming + "trust_match": trust, // TODO: Refactor into a reasonable function + "key_id_match": isKeyIDEqual(parentKeyId, issuerKeyId), + "signature_match": false, // TODO: Checking the Signature Has to Be Done Per-Algorithm + } + + return nil, result +} + +func isKeyIDEqual(first, second []byte) bool { + // TODO: Check if Trimming Makes Sense Here - Eg. Could this Be Padded? + // TODO: There has to be a library that does this + if len(first) != len(second) { + return false + } + for i, byteOfFirst := range first { + if byteOfFirst != second[i] { + return false + } + } + return true +} + +func (c *PKIVerifySignCommand) outputResults(results map[string]bool) error { + switch Format(c.UI) { + case "", "table": + return c.outputResultsTable(results) + case "json": + return c.outputResultsJSON(results) + case "yaml": + return c.outputResultsYAML(results) + default: + return fmt.Errorf("unknown output format: %v", Format(c.UI)) + } +} + +func (c *PKIVerifySignCommand) outputResultsTable(results map[string]bool) error { + data := []string{"field" + hopeDelim + "value"} + for field, finding := range results { + row := field + hopeDelim + strconv.FormatBool(finding) + data = append(data, row) + } + c.UI.Output(tableOutput(data, &columnize.Config{ + Delim: hopeDelim, + })) + c.UI.Output("\n") + + return nil +} + +func (c *PKIVerifySignCommand) outputResultsJSON(results map[string]bool) error { + bytes, err := json.MarshalIndent(results, "", " ") + if err != nil { + return err + } + + c.UI.Output(string(bytes)) + return nil +} + +func (c *PKIVerifySignCommand) outputResultsYAML(results map[string]bool) error { + bytes, err := yaml.Marshal(results) + if err != nil { + return err + } + + c.UI.Output(string(bytes)) + return nil +} From 7c793c3663c7841f9868e726abde010e7ea24449 Mon Sep 17 00:00:00 2001 From: kitography Date: Fri, 16 Dec 2022 10:10:06 -0500 Subject: [PATCH 02/21] Working state --- command/pki_verify_sign_command.go | 34 +++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/command/pki_verify_sign_command.go b/command/pki_verify_sign_command.go index 8170865a9cc9..a905d29a5ec0 100644 --- a/command/pki_verify_sign_command.go +++ b/command/pki_verify_sign_command.go @@ -72,7 +72,13 @@ func (c *PKIVerifySignCommand) Run(args []string) int { issuer := sanitizePath(args[0]) issued := sanitizePath(args[1]) - err, results := verifySignBetween(c.client, issuer, issued) + client, err := c.Client() + if err != nil { + c.UI.Error(fmt.Sprintf("Failed to obtain client: %w", err)) + return 1 + } + + err, results := verifySignBetween(client, issuer, issued) if err != nil { c.UI.Error(fmt.Sprintf("Failed to run verification: %v", err)) return pkiRetUsage @@ -104,7 +110,11 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) if err != nil { return fmt.Errorf("error: unable to fetch issuer %v: %w", issuerPath, err), nil } - caChain := issuedCertResp.Data["ca_chain"].(string) + caChainRaw := issuedCertResp.Data["ca_chain"].([]interface{}) + caChain := make([]string, len(caChainRaw)) + for i, cert := range caChainRaw { + caChain[i] = cert.(string) + } issuedCertPem := issuedCertResp.Data["certificate"].(string) issuedCertBundle, err := certutil.ParsePEMBundle(issuedCertPem) if err != nil { @@ -129,12 +139,26 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) } } + path_match := false + for _, cert := range caChain { + if cert == issuerCertPem { // TODO: Check Trimming + path_match = true + break + } + } + + signatureMatch := false + err = issuedCertBundle.Certificate.CheckSignatureFrom(issuerCertBundle.Certificate) + if err == nil { + signatureMatch = true + } + result := map[string]bool{ "subject_match": parentIssuerName.String() == issuerName.String(), // TODO: No Equals Defined on Name, Check This - "path_match": strings.Contains(caChain, issuerCertPem), // TODO: Check Trimming - "trust_match": trust, // TODO: Refactor into a reasonable function + "path_match": path_match, + "trust_match": trust, // TODO: Refactor into a reasonable function "key_id_match": isKeyIDEqual(parentKeyId, issuerKeyId), - "signature_match": false, // TODO: Checking the Signature Has to Be Done Per-Algorithm + "signature_match": signatureMatch, // TODO: Checking the Signature Has to Be Done Per-Algorithm } return nil, result From 4f83d60c61b277714b7a6bded9294bb942a544cc Mon Sep 17 00:00:00 2001 From: kitography Date: Fri, 16 Dec 2022 10:31:01 -0500 Subject: [PATCH 03/21] Updates to proper verification syntax Co-authored-by: 'Alex Scheel' --- command/pki_verify_sign_command.go | 52 +++++++++++++----------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/command/pki_verify_sign_command.go b/command/pki_verify_sign_command.go index a905d29a5ec0..dd3613299eae 100644 --- a/command/pki_verify_sign_command.go +++ b/command/pki_verify_sign_command.go @@ -1,6 +1,7 @@ package command import ( + "bytes" "crypto/x509" "encoding/json" "fmt" @@ -102,7 +103,6 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) if err != nil { return err, nil } - issuerName := issuerCertBundle.Certificate.Subject issuerKeyId := issuerCertBundle.Certificate.SubjectKeyId // Fetch and Parse the Potential Issued Cert @@ -120,29 +120,34 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) if err != nil { return err, nil } - parentIssuerName := issuerCertBundle.Certificate.Issuer parentKeyId := issuerCertBundle.Certificate.AuthorityKeyId // Check the Chain-Match - rootCertPool := x509.NewCertPool() // TODO: Check if it matters to use Root Only Here (There's also an Intermediate Cert Pool) + rootCertPool := x509.NewCertPool() rootCertPool.AddCert(issuerCertBundle.Certificate) checkTrustPathOptions := x509.VerifyOptions{ Roots: rootCertPool, } - trusts, err := issuedCertBundle.Certificate.Verify(checkTrustPathOptions) trust := false - for _, chain := range trusts { - for _, cert := range chain { - if issuedCertBundle.Certificate.Equal(cert) { - trust = true + trusts, err := issuedCertBundle.Certificate.Verify(checkTrustPathOptions) + if err != nil && !strings.Contains(err.Error(), "certificate signed by unknown authority") { + return err, nil + } 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) { + trust = true + break + } } } } - path_match := false + pathMatch := false for _, cert := range caChain { - if cert == issuerCertPem { // TODO: Check Trimming - path_match = true + if strings.TrimSpace(cert) == strings.TrimSpace(issuerCertPem) { // TODO: Decode into ASN1 to Check + pathMatch = true break } } @@ -152,32 +157,19 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) if err == nil { signatureMatch = true } - + result := map[string]bool{ - "subject_match": parentIssuerName.String() == issuerName.String(), // TODO: No Equals Defined on Name, Check This - "path_match": path_match, + // This comparison isn't strictly correct, despite a standard ordering these are sets + "subject_match": bytes.Equal(issuerCertBundle.Certificate.RawSubject, issuedCertBundle.Certificate.RawIssuer), + "path_match": pathMatch, "trust_match": trust, // TODO: Refactor into a reasonable function - "key_id_match": isKeyIDEqual(parentKeyId, issuerKeyId), - "signature_match": signatureMatch, // TODO: Checking the Signature Has to Be Done Per-Algorithm + "key_id_match": bytes.Equal(parentKeyId, issuerKeyId), + "signature_match": signatureMatch, } return nil, result } -func isKeyIDEqual(first, second []byte) bool { - // TODO: Check if Trimming Makes Sense Here - Eg. Could this Be Padded? - // TODO: There has to be a library that does this - if len(first) != len(second) { - return false - } - for i, byteOfFirst := range first { - if byteOfFirst != second[i] { - return false - } - } - return true -} - func (c *PKIVerifySignCommand) outputResults(results map[string]bool) error { switch Format(c.UI) { case "", "table": From 0733ea8d81af1011ca8ea03616f947ce01e0e6ff Mon Sep 17 00:00:00 2001 From: kitography Date: Fri, 16 Dec 2022 10:35:33 -0500 Subject: [PATCH 04/21] make fmt --- command/pki_verify_sign_command.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/command/pki_verify_sign_command.go b/command/pki_verify_sign_command.go index dd3613299eae..f6b11c64733d 100644 --- a/command/pki_verify_sign_command.go +++ b/command/pki_verify_sign_command.go @@ -5,12 +5,13 @@ import ( "crypto/x509" "encoding/json" "fmt" + "strconv" + "strings" + "github.com/ghodss/yaml" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/sdk/helper/certutil" "github.com/ryanuber/columnize" - "strconv" - "strings" ) type PKIVerifySignCommand struct { From 6e2c578aa704af4095ae36ebec0d1eeb484429a7 Mon Sep 17 00:00:00 2001 From: kitography Date: Fri, 16 Dec 2022 11:11:29 -0500 Subject: [PATCH 05/21] Git CI caught some stuff. --- changelog/18437.txt | 3 +++ command/pki_verify_sign_command.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changelog/18437.txt diff --git a/changelog/18437.txt b/changelog/18437.txt new file mode 100644 index 000000000000..9ca8a8dc3bb3 --- /dev/null +++ b/changelog/18437.txt @@ -0,0 +1,3 @@ +```release-note:improvement +client/pki: Add a new command verify-sign which checks the relationship between two certificates. +``` diff --git a/command/pki_verify_sign_command.go b/command/pki_verify_sign_command.go index f6b11c64733d..658c212375ae 100644 --- a/command/pki_verify_sign_command.go +++ b/command/pki_verify_sign_command.go @@ -76,7 +76,7 @@ func (c *PKIVerifySignCommand) Run(args []string) int { client, err := c.Client() if err != nil { - c.UI.Error(fmt.Sprintf("Failed to obtain client: %w", err)) + c.UI.Error(fmt.Sprintf("Failed to obtain client: %v", err)) return 1 } From 456a2ecd1b8c4c92d8e9ca9c0c3e8bc18763b1cd Mon Sep 17 00:00:00 2001 From: kitography Date: Mon, 19 Dec 2022 08:28:44 -0500 Subject: [PATCH 06/21] Base functionality. --- command/commands.go | 15 +- command/pki_list_children_command.go | 239 +++++++++++++++++++++++++++ command/pki_verify_sign_command.go | 13 +- 3 files changed, 259 insertions(+), 8 deletions(-) create mode 100644 command/pki_list_children_command.go diff --git a/command/commands.go b/command/commands.go index 903bf5fad637..6323d6ee9284 100644 --- a/command/commands.go +++ b/command/commands.go @@ -526,6 +526,16 @@ func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) { BaseCommand: getBaseCommand(), }, nil }, + "pki health-check": func() (cli.Command, error) { + return &PKIHealthCheckCommand{ + BaseCommand: getBaseCommand(), + }, nil + }, + "pki list-intermediates": func() (cli.Command, error) { + return &PKIListChildrenCommand{ + BaseCommand: getBaseCommand(), + }, nil + }, "pki verify-sign": func() (cli.Command, error) { return &PKIVerifySignCommand{ BaseCommand: getBaseCommand(), @@ -802,11 +812,6 @@ func initCommands(ui, serverCmdUi cli.Ui, runOpts *RunOptions) { ShutdownCh: MakeShutdownCh(), }, nil }, - "pki health-check": func() (cli.Command, error) { - return &PKIHealthCheckCommand{ - BaseCommand: getBaseCommand(), - }, nil - }, } initCommandsEnt(ui, serverCmdUi, runOpts) diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go new file mode 100644 index 000000000000..9643de778979 --- /dev/null +++ b/command/pki_list_children_command.go @@ -0,0 +1,239 @@ +package command + +import ( + "encoding/json" + "fmt" + "github.com/ghodss/yaml" + "github.com/ryanuber/columnize" + "strconv" + "strings" +) + +type PKIListChildrenCommand struct { + *BaseCommand + + flagConfig string + flagReturnIndicator string + flagDefaultDisabled bool + flagList bool + + flagSignatureMatch bool + flagIndirectSignMatch bool + flagKeyIdMatch bool + flagSubjectMatch bool + flagPathMatch bool +} + +func (c *PKIListChildrenCommand) Synopsis() string { + return "Determine Which (of a List) of Certificates Were Issued by A Given Parent Certificate" +} + +func (c *PKIListChildrenCommand) Help() string { + helpText := ` +Usage: vault pki verify-sign PARENT CHILD +Returns four fields of information: +- signature_match: was the key of the issuer used to sign the issued +- path_match: the possible issuer appears in the valid certificate chain of the issued +- key_id_match: does the key-id of the issuer match the key_id of the subject +- subject_match: does the subject name of the issuer match the issuer subject of the issued +` + return strings.TrimSpace(helpText) +} + +func (c *PKIListChildrenCommand) Flags() *FlagSets { + set := c.flagSet(FlagSetHTTP | FlagSetOutputFormat) + f := set.NewFlagSet("Command Options") + + f.BoolVar(&BoolVar{ + Name: "subject_match", + Target: &c.flagSubjectMatch, + Default: true, + EnvVar: "", + Usage: `Whether the subject key_id of the potential parent cert matches the issuing key id of the child cert`, + }) + + f.BoolVar(&BoolVar{ + Name: "key_id_match", + Target: &c.flagKeyIdMatch, + Default: true, + EnvVar: "", + Usage: `Whether the subject key_id of the potential parent cert matches the issuing key id of the child cert`, + }) + + f.BoolVar(&BoolVar{ + Name: "path_match", + Target: &c.flagPathMatch, + Default: false, + EnvVar: "", + Usage: `Whether the this potential parent appears in the certificate chain of the issued cert`, + }) + + f.BoolVar(&BoolVar{ + Name: "direct_sign", + Target: &c.flagSignatureMatch, + Default: true, + EnvVar: "", + Usage: `Whether the key of the potential parent signed this issued certificate`, + }) + + f.BoolVar(&BoolVar{ + Name: "indirect_sign", + Target: &c.flagIndirectSignMatch, + Default: true, + EnvVar: "", + Usage: `Whether trusting the parent certificate is sufficient to trust the child certificate`, + }) + + return set + +} + +func (c *PKIListChildrenCommand) Run(args []string) int { + f := c.Flags() + if err := f.Parse(args); err != nil { + c.UI.Error(err.Error()) + return 1 + } + + args = f.Args() + + if len(args) < 1 { + c.UI.Error("Not enough arguments (expected potential parent, got nothing)") + return 1 + } else if len(args) > 2 { + c.UI.Error(fmt.Sprintf("Too many arguments (expected only potential issuer and issued, got %d arguments)", len(args))) + for _, arg := range args { + if strings.HasPrefix(arg, "-") { + c.UI.Warn(fmt.Sprintf("Options (%v) must be specified before positional arguments (%v)", arg, args[0])) + break + } + } + return 1 + } + + client, err := c.Client() + if err != nil { + c.UI.Error(fmt.Sprintf("Failed to obtain client: %w", err)) + return 1 + } + + issuer := sanitizePath(args[0]) + issued := "" + if len(args) > 1 { + issued = sanitizePath(args[1]) + } else { + mountListRaw, err := client.Logical().Read("/sys/mounts/") + if err != nil { + c.UI.Error(fmt.Sprintf("Failed to Read List of Mounts With Potential Issuers: %v", err)) + return 1 + } + for path, rawValueMap := range mountListRaw.Data { + valueMap := rawValueMap.(map[string]interface{}) + if valueMap["type"].(string) == "pki" { + issuerListEndpoint := sanitizePath(path) + "/issuers" + rawIssuersResp, err := client.Logical().List(issuerListEndpoint) + if err != nil { + c.UI.Error(fmt.Sprintf("Failed to Read List of Issuers within Mount %v: %v", path, err)) + return 1 + } + issuersMap := rawIssuersResp.Data["keys"] + if issuersMap == nil { + continue // TODO: Add a Warning Here + } + certList := issuersMap.([]interface{}) + for _, certId := range certList { + if len(issued) == 0 { + issued = sanitizePath(path) + "/issuer/" + certId.(string) + } else { + issued = issued + "," + sanitizePath(path) + "/issuer/" + certId.(string) + } + } + } + } + } + + var childrenMatches = make(map[string]bool) + + constraintMap := map[string]bool{ + // This comparison isn't strictly correct, despite a standard ordering these are sets + "subject_match": c.flagSubjectMatch, + "path_match": c.flagPathMatch, + "trust_match": c.flagIndirectSignMatch, + "key_id_match": c.flagKeyIdMatch, + "signature_match": c.flagSignatureMatch, + } + + for _, child := range strings.Split(issued, ",") { + path := sanitizePath(child) + if path != "" { + err, verifyResults := verifySignBetween(client, issuer, path) + if err != nil { + c.UI.Error(fmt.Sprintf("Failed to run verification on path %v: %v", path, err)) + return 1 + } + childrenMatches[path] = checkIfResultsMatchFilters(verifyResults, constraintMap) + } + } + + c.outputResults(childrenMatches) + + return 0 +} + +func checkIfResultsMatchFilters(verifyResults, constraintMap map[string]bool) bool { + for key, required := range constraintMap { + if required == true { + if verifyResults[key] == false { + return false + } + } + } + return true +} + +func (c *PKIListChildrenCommand) outputResults(results map[string]bool) error { + switch Format(c.UI) { + case "", "table": + return c.outputResultsTable(results) + case "json": + return c.outputResultsJSON(results) + case "yaml": + return c.outputResultsYAML(results) + default: + return fmt.Errorf("unknown output format: %v", Format(c.UI)) + } +} + +func (c *PKIListChildrenCommand) outputResultsTable(results map[string]bool) error { + data := []string{"intermediate" + hopeDelim + "match?"} + for field, finding := range results { + row := field + hopeDelim + strconv.FormatBool(finding) + data = append(data, row) + } + c.UI.Output(tableOutput(data, &columnize.Config{ + Delim: hopeDelim, + })) + c.UI.Output("\n") + + return nil +} + +func (c *PKIListChildrenCommand) outputResultsJSON(results map[string]bool) error { + bytes, err := json.MarshalIndent(results, "", " ") + if err != nil { + return err + } + + c.UI.Output(string(bytes)) + return nil +} + +func (c *PKIListChildrenCommand) outputResultsYAML(results map[string]bool) error { + bytes, err := yaml.Marshal(results) + if err != nil { + return err + } + + c.UI.Output(string(bytes)) + return nil +} diff --git a/command/pki_verify_sign_command.go b/command/pki_verify_sign_command.go index f6b11c64733d..6e3460318f12 100644 --- a/command/pki_verify_sign_command.go +++ b/command/pki_verify_sign_command.go @@ -111,9 +111,16 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) if err != nil { return fmt.Errorf("error: unable to fetch issuer %v: %w", issuerPath, err), nil } - caChainRaw := issuedCertResp.Data["ca_chain"].([]interface{}) - caChain := make([]string, len(caChainRaw)) - for i, cert := range caChainRaw { + if len(issuedPath) <= 2 { + return fmt.Errorf(fmt.Sprintf("%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) From 7c1fc9373eed6fac87fa72ffc731740f9fa86acb Mon Sep 17 00:00:00 2001 From: kitography Date: Mon, 19 Dec 2022 08:36:58 -0500 Subject: [PATCH 07/21] make fmt; changelog --- changelog/18463.txt | 3 +++ command/pki_list_children_command.go | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 changelog/18463.txt diff --git a/changelog/18463.txt b/changelog/18463.txt new file mode 100644 index 000000000000..7c521794c21d --- /dev/null +++ b/changelog/18463.txt @@ -0,0 +1,3 @@ +```release-note:improvement +cli/pki: Add List-Children functionality to pki client. +``` diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go index 9643de778979..130ba7701274 100644 --- a/command/pki_list_children_command.go +++ b/command/pki_list_children_command.go @@ -3,10 +3,11 @@ package command import ( "encoding/json" "fmt" - "github.com/ghodss/yaml" - "github.com/ryanuber/columnize" "strconv" "strings" + + "github.com/ghodss/yaml" + "github.com/ryanuber/columnize" ) type PKIListChildrenCommand struct { @@ -85,7 +86,6 @@ func (c *PKIListChildrenCommand) Flags() *FlagSets { }) return set - } func (c *PKIListChildrenCommand) Run(args []string) int { @@ -152,7 +152,7 @@ func (c *PKIListChildrenCommand) Run(args []string) int { } } - var childrenMatches = make(map[string]bool) + childrenMatches := make(map[string]bool) constraintMap := map[string]bool{ // This comparison isn't strictly correct, despite a standard ordering these are sets From dab9f2360a1394654fd4379c0f4fcafc4c18a0c6 Mon Sep 17 00:00:00 2001 From: kitography Date: Wed, 25 Jan 2023 11:54:20 -0500 Subject: [PATCH 08/21] What I thought empty issuers response fix would be. --- command/pki_list_children_command.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go index 130ba7701274..c6199b44a154 100644 --- a/command/pki_list_children_command.go +++ b/command/pki_list_children_command.go @@ -136,10 +136,10 @@ func (c *PKIListChildrenCommand) Run(args []string) int { c.UI.Error(fmt.Sprintf("Failed to Read List of Issuers within Mount %v: %v", path, err)) return 1 } - issuersMap := rawIssuersResp.Data["keys"] - if issuersMap == nil { - continue // TODO: Add a Warning Here + if rawIssuersResp.Data == nil { + continue // TODO: Empty Issuers Response this throws an error } + issuersMap := rawIssuersResp.Data["keys"] certList := issuersMap.([]interface{}) for _, certId := range certList { if len(issued) == 0 { From c258cf0adb6f293f29b8df904a0285be2b26c7c5 Mon Sep 17 00:00:00 2001 From: kitography Date: Wed, 25 Jan 2023 12:56:49 -0500 Subject: [PATCH 09/21] Some tests --- command/pki_verify_sign_command.go | 17 +- command/pki_verify_sign_test.go | 438 +++++++++++++++++++++++++++++ 2 files changed, 450 insertions(+), 5 deletions(-) create mode 100644 command/pki_verify_sign_test.go diff --git a/command/pki_verify_sign_command.go b/command/pki_verify_sign_command.go index 658c212375ae..47486b8c0e18 100644 --- a/command/pki_verify_sign_command.go +++ b/command/pki_verify_sign_command.go @@ -76,7 +76,7 @@ func (c *PKIVerifySignCommand) Run(args []string) int { client, err := c.Client() if err != nil { - c.UI.Error(fmt.Sprintf("Failed to obtain client: %v", err)) + c.UI.Error(fmt.Sprintf("Failed to obtain client: %s", err)) return 1 } @@ -111,9 +111,16 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) if err != nil { return fmt.Errorf("error: unable to fetch issuer %v: %w", issuerPath, err), nil } - caChainRaw := issuedCertResp.Data["ca_chain"].([]interface{}) - caChain := make([]string, len(caChainRaw)) - for i, cert := range caChainRaw { + if len(issuedPath) <= 2 { + return fmt.Errorf(fmt.Sprintf("%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) @@ -121,7 +128,7 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) if err != nil { return err, nil } - parentKeyId := issuerCertBundle.Certificate.AuthorityKeyId + parentKeyId := issuedCertBundle.Certificate.AuthorityKeyId // Check the Chain-Match rootCertPool := x509.NewCertPool() diff --git a/command/pki_verify_sign_test.go b/command/pki_verify_sign_test.go new file mode 100644 index 000000000000..86f2b5f70c77 --- /dev/null +++ b/command/pki_verify_sign_test.go @@ -0,0 +1,438 @@ +package command + +import ( + "bytes" + "encoding/json" + "fmt" + "github.com/hashicorp/vault/api" + "strings" + "testing" +) + +func TestPKIVerifySign(t *testing.T) { + t.Parallel() + + client, closer := testVaultServer(t) + defer closer() + + // Relationship Map to Create + // pki-root | pki-newroot + // RootX1 RootX2 RootX4 RootX3 + // | | + // ---------------------------------------------- + // v v + // IntX1 IntX2 pki-int + // | | + // v v + // IntX3 (-----------------------) IntX3 + // + // Here X1,X2 have the same name (same mount) + // RootX4 uses the same key as RootX1 (but a different common_name/subject) + // RootX3 has the same name, and is on a different mount + // RootX1 has issued IntX1; RootX3 has issued IntX2 + createComplicatedIssuerSetUp(t, client) + + cases := []struct { + name string + args []string + expectedMatches map[string]bool + jsonOut bool + shouldError bool + expectErrorCont string + expectErrorNotCont string + nonJsonOutputCont string + }{ + { + "rootX1-matches-rootX1", + []string{"pki", "verify-sign", "-format=json", "pki-root/issuer/rootX1", "pki-root/issuer/rootX1"}, + map[string]bool{ + "key_id_match": true, + "path_match": true, + "signature_match": true, + "subject_match": true, + "trust_match": true, + }, + true, + false, + "", + "", + "", + }, + { + "rootX1-on-rootX2-onlySameName", + []string{"pki", "verify-sign", "-format=json", "pki-root/issuer/rootX1", "pki-root/issuer/rootX2"}, + map[string]bool{ + "key_id_match": false, + "path_match": false, + "signature_match": false, + "subject_match": true, + "trust_match": false, + }, + true, + false, + "", + "", + "", + }, + } + for _, testCase := range cases { + var errString string + var results map[string]interface{} + var stdOut string + + if testCase.jsonOut { + results, errString = execPKIVerifyJson(t, client, false, testCase.shouldError, testCase.args) + } else { + stdOut, errString = execPKIVerifyNonJson(t, client, testCase.shouldError, testCase.args) + } + + // Verify Error Behavior + if testCase.shouldError { + if errString == "" { + t.Fatalf("Expected error in Testcase %s : no error produced, got results %s", testCase.name, results) + } + if testCase.expectErrorCont != "" && !strings.Contains(errString, testCase.expectErrorCont) { + t.Fatalf("Expected error in Testcase %s to contain %s, but got error %s", testCase.name, testCase.expectErrorCont, errString) + } + if testCase.expectErrorNotCont != "" && strings.Contains(errString, testCase.expectErrorNotCont) { + t.Fatalf("Expected error in Testcase %s to not contain %s, but got error %s", testCase.name, testCase.expectErrorNotCont, errString) + } + } else { + if errString != "" { + t.Fatalf("Error in Testcase %s : no error expected, but got error: %s", testCase.name, errString) + } + } + + // Verify Output + if testCase.jsonOut { + isMatch, errString := verifyExpectedJson(testCase.expectedMatches, results) + if !isMatch { + t.Fatalf("Expected Results for Testcase %s, do not match returned results %s", testCase.name, errString) + } + } else { + if !strings.Contains(stdOut, testCase.nonJsonOutputCont) { + t.Fatalf("Expected standard output for Testcase %s to contain %s, but got %s", testCase.name, testCase.nonJsonOutputCont, stdOut) + } + } + + } + +} + +func execPKIVerifyJson(t *testing.T, client *api.Client, expectErrorUnmarshalling bool, expectErrorOut bool, callArgs []string) (map[string]interface{}, string) { + stdout, stderr := execPKIVerifyNonJson(t, client, expectErrorOut, callArgs) + + var results map[string]interface{} + if err := json.Unmarshal([]byte(stdout), &results); err != nil && !expectErrorUnmarshalling { + t.Fatalf("failed to decode json response : %v \n json: \n%v", err, stdout) + } + + return results, stderr +} + +func execPKIVerifyNonJson(t *testing.T, client *api.Client, expectErrorOut bool, callArgs []string) (string, string) { + stdout := bytes.NewBuffer(nil) + stderr := bytes.NewBuffer(nil) + runOpts := &RunOptions{ + Stdout: stdout, + Stderr: stderr, + Client: client, + } + + code := RunCustom(callArgs, runOpts) + if !expectErrorOut && code != 0 { + t.Fatalf("running command `%v` unsuccessful (ret %v)\nerr: %v", strings.Join(callArgs, " "), code, stderr.String()) + } + + t.Log(stdout.String() + stderr.String()) + + return stdout.String(), stderr.String() +} + +func convertListOfInterfaceToString(list []interface{}, sep string) string { + newList := make([]string, len(list)) + for i, interfa := range list { + newList[i] = interfa.(string) + } + return strings.Join(newList, sep) +} + +func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { + // Relationship Map to Create + // pki-root | pki-newroot + // RootX1 RootX2 RootX4 RootX3 + // | | + // ---------------------------------------------- + // v v + // IntX1 IntX2 pki-int + // | | + // v v + // IntX3 (-----------------------) IntX3 + // + // Here X1,X2 have the same name (same mount) + // RootX4 uses the same key as RootX1 (but a different common_name/subject) + // RootX3 has the same name, and is on a different mount + // RootX1 has issued IntX1; RootX3 has issued IntX2 + + if err := client.Sys().Mount("pki-root", &api.MountInput{ + Type: "pki", + Config: api.MountConfigInput{ + MaxLeaseTTL: "36500d", + }, + }); err != nil { + t.Fatalf("pki mount error: %#v", err) + } + + if err := client.Sys().Mount("pki-newroot", &api.MountInput{ + Type: "pki", + Config: api.MountConfigInput{ + MaxLeaseTTL: "36500d", + }, + }); err != nil { + t.Fatalf("pki mount error: %#v", err) + } + + if err := client.Sys().Mount("pki-int", &api.MountInput{ + Type: "pki", + Config: api.MountConfigInput{ + MaxLeaseTTL: "36500d", + }, + }); err != nil { + t.Fatalf("pki mount error: %#v", err) + } + + resp, err := client.Logical().Write("pki-root/root/generate/internal", map[string]interface{}{ + "key_type": "ec", + "common_name": "Root X", + "ttl": "3650d", + "issuer_name": "rootX1", + "key_name": "rootX1", + }) + t.Logf("%s", resp.Data) + if err != nil || resp == nil { + t.Fatalf("failed to prime CA: %v", err) + } + + resp, err = client.Logical().Write("pki-root/root/generate/internal", map[string]interface{}{ + "key_type": "ec", + "common_name": "Root X", + "ttl": "3650d", + "issuer_name": "rootX2", + }) + t.Logf("%s", resp.Data) + if err != nil || resp == nil { + t.Fatalf("failed to prime CA: %v", err) + } + + if resp, err := client.Logical().Write("pki-newroot/root/generate/internal", map[string]interface{}{ + "key_type": "ec", + "common_name": "Root X", + "ttl": "3650d", + "issuer_name": "rootX3", + }); err != nil || resp == nil { + t.Fatalf("failed to prime CA: %v", err) + } + + if resp, err := client.Logical().Write("pki-root/root/generate/existing", map[string]interface{}{ + "common_name": "Root X4", + "ttl": "3650d", + "issuer_name": "rootX4", + "key_ref": "rootX1", + }); err != nil || resp == nil { + t.Fatalf("failed to prime CA: %v", err) + } + + // Intermediate X1 + int1CsrResp, err := client.Logical().Write("pki-int/intermediate/generate/internal", map[string]interface{}{ + "key_type": "rsa", + "common_name": "Int X1", + "ttl": "3650d", + }) + if err != nil || int1CsrResp == nil { + t.Fatalf("failed to generate CSR: %v", err) + } + int1CsrRaw, ok := int1CsrResp.Data["csr"] + if !ok { + t.Fatalf("no csr produced when generating intermediate, resp: %v", int1CsrResp) + } + int1Csr := int1CsrRaw.(string) + int1CertResp, err := client.Logical().Write("pki-root/issuer/rootX1/sign-intermediate", map[string]interface{}{ + "csr": int1Csr, + }) + if err != nil || int1CertResp == nil { + t.Fatalf("failed to sign CSR: %v", err) + } + int1CertChainRaw, ok := int1CertResp.Data["ca_chain"] + if !ok { + t.Fatalf("no ca_chain produced when signing intermediate, resp: %v", int1CertResp) + } + int1CertChain := convertListOfInterfaceToString(int1CertChainRaw.([]interface{}), "\n") + importInt1Resp, err := client.Logical().Write("pki-int/issuers/import/cert", map[string]interface{}{ + "pem_bundle": int1CertChain, + }) + if err != nil || importInt1Resp == nil { + t.Fatalf("failed to import certificate: %v", err) + } + importIssuerIdMap, ok := importInt1Resp.Data["mapping"] + if !ok { + t.Fatalf("no mapping data returned on issuer import: %v", importInt1Resp) + } + importIssuerId := "" + for key, value := range importIssuerIdMap.(map[string]interface{}) { + if value != nil && len(value.(string)) > 0 { + importIssuerId = key + break + } + } + if resp, err := client.Logical().Write("pki-int/issuer/"+importIssuerId, map[string]interface{}{ + "issuer_name": "intX1", + }); err != nil || resp == nil { + t.Fatalf("error naming issuer %v", err) + } + + // Intermediate X2 + int2CsrResp, err := client.Logical().Write("pki-int/intermediate/generate/internal", map[string]interface{}{ + "key_type": "ed25519", + "common_name": "Int X2", + "ttl": "3650d", + }) + if err != nil || int2CsrResp == nil { + t.Fatalf("failed to generate CSR: %v", err) + } + int2CsrRaw, ok := int2CsrResp.Data["csr"] + if !ok { + t.Fatalf("no csr produced when generating intermediate, resp: %v", int2CsrResp) + } + int2Csr := int2CsrRaw.(string) + int2CertResp, err := client.Logical().Write("pki-newroot/issuer/rootX3/sign-intermediate", map[string]interface{}{ + "csr": int2Csr, + }) + if err != nil || int2CertResp == nil { + t.Fatalf("failed to sign CSR: %v", err) + } + int2CertChainRaw, ok := int2CertResp.Data["ca_chain"] + if !ok { + t.Fatalf("no ca_chain produced when signing intermediate, resp: %v", int2CertResp) + } + int2CertChain := convertListOfInterfaceToString(int2CertChainRaw.([]interface{}), "\n") + importInt2Resp, err := client.Logical().Write("pki-int/issuers/import/cert", map[string]interface{}{ + "pem_bundle": int2CertChain, + "issuer_name": "intX2", + }) + if err != nil || importInt2Resp == nil { + t.Fatalf("failed to import certificate: %v", err) + } + importIssuer2IdMap, ok := importInt2Resp.Data["mapping"] + if !ok { + t.Fatalf("no mapping data returned on issuer import: %v", importInt2Resp) + } + importIssuer2Id := "" + for key, value := range importIssuer2IdMap.(map[string]interface{}) { + if value != nil && len(value.(string)) > 0 { + importIssuer2Id = key + break + } + } + if resp, err := client.Logical().Write("pki-int/issuer/"+importIssuer2Id, map[string]interface{}{ + "issuer_name": "intX2", + }); err != nil || resp == nil { + t.Fatalf("error naming issuer %v", err) + } + + // Intermediate X3 + int3CsrResp, err := client.Logical().Write("pki-int/intermediate/generate/internal", map[string]interface{}{ + "key_type": "rsa", + "common_name": "Int X3", + "ttl": "3650d", + }) + if err != nil || int3CsrResp == nil { + t.Fatalf("failed to generate CSR: %v", err) + } + int3CsrRaw, ok := int3CsrResp.Data["csr"] + if !ok { + t.Fatalf("no csr produced when generating intermediate, resp: %v", int3CsrResp) + } + int3Csr := int3CsrRaw.(string) + // sign by intX1 and import + int3CertResp1, err := client.Logical().Write("pki-int/issuer/intX1/sign-intermediate", map[string]interface{}{ + "csr": int3Csr, + }) + if err != nil || int3CertResp1 == nil { + t.Fatalf("failed to sign CSR: %v", err) + } + int3CertChainRaw1, ok := int3CertResp1.Data["ca_chain"] + if !ok { + t.Fatalf("no ca_chain produced when signing intermediate, resp: %v", int3CertResp1) + } + int3CertChain1 := convertListOfInterfaceToString(int3CertChainRaw1.([]interface{}), "\n") + importInt3Resp1, err := client.Logical().Write("pki-int/issuers/import/cert", map[string]interface{}{ + "pem_bundle": int3CertChain1, + }) + if err != nil || importInt3Resp1 == nil { + t.Fatalf("failed to import certificate: %v", err) + } + importIssuer3IdMap1, ok := importInt3Resp1.Data["mapping"] + if !ok { + t.Fatalf("no mapping data returned on issuer import: %v", importInt2Resp) + } + importIssuer3Id1 := "" + for key, value := range importIssuer3IdMap1.(map[string]interface{}) { + if value != nil && len(value.(string)) > 0 { + importIssuer3Id1 = key + break + } + } + if resp, err := client.Logical().Write("pki-int/issuer/"+importIssuer3Id1, map[string]interface{}{ + "issuer_name": "intX3", + }); err != nil || resp == nil { + t.Fatalf("error naming issuer %v", err) + } + // sign by intX2 and import + int3CertResp2, err := client.Logical().Write("pki-int/issuer/intX2/sign-intermediate", map[string]interface{}{ + "csr": int3Csr, + }) + if err != nil || int3CertResp2 == nil { + t.Fatalf("failed to sign CSR: %v", err) + } + int3CertChainRaw2, ok := int3CertResp2.Data["ca_chain"] + if !ok { + t.Fatalf("no ca_chain produced when signing intermediate, resp: %v", int3CertResp2) + } + int3CertChain2 := convertListOfInterfaceToString(int3CertChainRaw2.([]interface{}), "\n") + importInt3Resp2, err := client.Logical().Write("pki-int/issuers/import/cert", map[string]interface{}{ + "pem_bundle": int3CertChain2, + }) + if err != nil || importInt3Resp2 == nil { + t.Fatalf("failed to import certificate: %v", err) + } + importIssuer3IdMap2, ok := importInt3Resp2.Data["mapping"] + if !ok { + t.Fatalf("no mapping data returned on issuer import: %v", importInt2Resp) + } + importIssuer3Id2 := "" + for key, value := range importIssuer3IdMap2.(map[string]interface{}) { + if value != nil && len(value.(string)) > 0 { + importIssuer3Id2 = key + break + } + } + if resp, err := client.Logical().Write("pki-int/issuer/"+importIssuer3Id2, map[string]interface{}{ + "issuer_name": "intX3also", + }); err != nil || resp == nil { + t.Fatalf("error naming issuer %v", err) + } + +} + +func verifyExpectedJson(expectedResults map[string]bool, results map[string]interface{}) (isMatch bool, error string) { + if len(expectedResults) != len(results) { + return false, fmt.Sprintf("Different Number of Keys in Expected Results (%d), than results (%d)", + len(expectedResults), len(results)) + } + for key, value := range expectedResults { + if results[key].(bool) != value { + return false, fmt.Sprintf("Different value for key %s : expected %t got %s", key, value, results[key]) + } + } + return true, "" +} From 77d1351e28f5dc71a52950a44cda1f042df07455 Mon Sep 17 00:00:00 2001 From: kitography Date: Wed, 25 Jan 2023 13:18:02 -0500 Subject: [PATCH 10/21] PR-review updates. --- command/pki_verify_sign_command.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/command/pki_verify_sign_command.go b/command/pki_verify_sign_command.go index 47486b8c0e18..ff2976cd1b47 100644 --- a/command/pki_verify_sign_command.go +++ b/command/pki_verify_sign_command.go @@ -30,11 +30,14 @@ func (c *PKIVerifySignCommand) Synopsis() string { func (c *PKIVerifySignCommand) Help() string { helpText := ` Usage: vault pki verify-sign POSSIBLE-ISSUER POSSIBLE-ISSUED -Returns four fields of information: +Here POSSIBLE-ISSUER and POSSIBLE-ISSUED are the fully name-spaced path to the certificate, +for instance: 'ns1/mount1/issuer/issuerName/json' +Returns five fields of information: - signature_match: was the key of the issuer used to sign the issued - path_match: the possible issuer appears in the valid certificate chain of the issued - key_id_match: does the key-id of the issuer match the key_id of the subject - subject_match: does the subject name of the issuer match the issuer subject of the issued +- trust_match: if someone trusted the parent issuer, is the chain provided sufficient to trust the child issued ` return strings.TrimSpace(helpText) } @@ -86,13 +89,13 @@ func (c *PKIVerifySignCommand) Run(args []string) int { return pkiRetUsage } - c.outputResults(results) + c.outputResults(results, issuer, issued) return 0 } func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) (error, map[string]bool) { - // TODO: Stop Eating Warnings Here + // Note that this eats warnings // Fetch and Parse the Potential Issuer: issuerResp, err := client.Logical().Read(issuerPath) @@ -178,10 +181,10 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) return nil, result } -func (c *PKIVerifySignCommand) outputResults(results map[string]bool) error { +func (c *PKIVerifySignCommand) outputResults(results map[string]bool, potentialParent, potentialChild string) error { switch Format(c.UI) { case "", "table": - return c.outputResultsTable(results) + return c.outputResultsTable(results, potentialParent, potentialChild) case "json": return c.outputResultsJSON(results) case "yaml": @@ -191,7 +194,9 @@ func (c *PKIVerifySignCommand) outputResults(results map[string]bool) error { } } -func (c *PKIVerifySignCommand) outputResultsTable(results map[string]bool) error { +func (c *PKIVerifySignCommand) outputResultsTable(results map[string]bool, potentialParent, potentialChild string) error { + c.UI.Output("issuer:" + potentialParent) + c.UI.Output("issued:" + potentialChild + "\n") data := []string{"field" + hopeDelim + "value"} for field, finding := range results { row := field + hopeDelim + strconv.FormatBool(finding) From be41954e5022159993e3f934fddaebafb1d11763 Mon Sep 17 00:00:00 2001 From: kitography Date: Wed, 25 Jan 2023 13:24:47 -0500 Subject: [PATCH 11/21] make fmt. --- command/pki_verify_sign_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/command/pki_verify_sign_test.go b/command/pki_verify_sign_test.go index 86f2b5f70c77..0a3b239bb760 100644 --- a/command/pki_verify_sign_test.go +++ b/command/pki_verify_sign_test.go @@ -4,9 +4,10 @@ import ( "bytes" "encoding/json" "fmt" - "github.com/hashicorp/vault/api" "strings" "testing" + + "github.com/hashicorp/vault/api" ) func TestPKIVerifySign(t *testing.T) { @@ -116,7 +117,6 @@ func TestPKIVerifySign(t *testing.T) { } } - } func execPKIVerifyJson(t *testing.T, client *api.Client, expectErrorUnmarshalling bool, expectErrorOut bool, callArgs []string) (map[string]interface{}, string) { @@ -421,7 +421,6 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { }); err != nil || resp == nil { t.Fatalf("error naming issuer %v", err) } - } func verifyExpectedJson(expectedResults map[string]bool, results map[string]interface{}) (isMatch bool, error string) { From 9f4f9cfb76cccc5089ea8e4ff92236720b3e5379 Mon Sep 17 00:00:00 2001 From: kitography Date: Wed, 25 Jan 2023 13:32:06 -0500 Subject: [PATCH 12/21] Fix null response data for listing empty issuers causing a crash. --- command/pki_list_children_command.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go index c6199b44a154..5c936ac9599b 100644 --- a/command/pki_list_children_command.go +++ b/command/pki_list_children_command.go @@ -113,7 +113,7 @@ func (c *PKIListChildrenCommand) Run(args []string) int { client, err := c.Client() if err != nil { - c.UI.Error(fmt.Sprintf("Failed to obtain client: %w", err)) + c.UI.Error(fmt.Sprintf("Failed to obtain client: %s", err)) return 1 } @@ -136,7 +136,7 @@ func (c *PKIListChildrenCommand) Run(args []string) int { c.UI.Error(fmt.Sprintf("Failed to Read List of Issuers within Mount %v: %v", path, err)) return 1 } - if rawIssuersResp.Data == nil { + if rawIssuersResp == nil { continue // TODO: Empty Issuers Response this throws an error } issuersMap := rawIssuersResp.Data["keys"] From 20d96a48dfa6eb6f05ce52517a696b1ea3a8bdb6 Mon Sep 17 00:00:00 2001 From: Kit Haines Date: Thu, 26 Jan 2023 10:34:50 -0500 Subject: [PATCH 13/21] Update command/pki_list_children_command.go Fix double specifier Co-authored-by: Steven Clark --- command/pki_list_children_command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go index 5c936ac9599b..90148fe39dfe 100644 --- a/command/pki_list_children_command.go +++ b/command/pki_list_children_command.go @@ -66,7 +66,7 @@ func (c *PKIListChildrenCommand) Flags() *FlagSets { Target: &c.flagPathMatch, Default: false, EnvVar: "", - Usage: `Whether the this potential parent appears in the certificate chain of the issued cert`, + Usage: `Whether the potential parent appears in the certificate chain of the issued cert`, }) f.BoolVar(&BoolVar{ From 87e9dab17d6ce59730d1f7745d75c51108b7db71 Mon Sep 17 00:00:00 2001 From: kitography Date: Thu, 26 Jan 2023 12:09:56 -0500 Subject: [PATCH 14/21] Add test for pki_list_children. --- command/pki_list_children_command.go | 29 +++++++- command/pki_list_children_test.go | 102 +++++++++++++++++++++++++++ command/pki_verify_sign_command.go | 2 +- command/pki_verify_sign_test.go | 24 ++++--- 4 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 command/pki_list_children_test.go diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go index 90148fe39dfe..f2dc2fc4c575 100644 --- a/command/pki_list_children_command.go +++ b/command/pki_list_children_command.go @@ -18,6 +18,8 @@ type PKIListChildrenCommand struct { flagDefaultDisabled bool flagList bool + flagUseNames bool + flagSignatureMatch bool flagIndirectSignMatch bool flagKeyIdMatch bool @@ -50,7 +52,7 @@ func (c *PKIListChildrenCommand) Flags() *FlagSets { Target: &c.flagSubjectMatch, Default: true, EnvVar: "", - Usage: `Whether the subject key_id of the potential parent cert matches the issuing key id of the child cert`, + Usage: `Whether the subject name of the potential parent cert matches the issuer name of the child cert`, }) f.BoolVar(&BoolVar{ @@ -85,6 +87,14 @@ func (c *PKIListChildrenCommand) Flags() *FlagSets { Usage: `Whether trusting the parent certificate is sufficient to trust the child certificate`, }) + f.BoolVar(&BoolVar{ + Name: "use_names", + Target: &c.flagUseNames, + Default: false, + EnvVar: "", + Usage: `Whether the list of issuers returned is referred to by name when it exists rather than uuid`, + }) + return set } @@ -142,10 +152,23 @@ func (c *PKIListChildrenCommand) Run(args []string) int { issuersMap := rawIssuersResp.Data["keys"] certList := issuersMap.([]interface{}) for _, certId := range certList { + identifier := certId.(string) + if c.flagUseNames { + issuerReadResp, err := client.Logical().Read(sanitizePath(path) + "/issuer/" + identifier) + if err != nil { + c.UI.Warn(fmt.Sprintf("Unable to Fetch Issuer to Recover Name at: %v", sanitizePath(path)+"/issuer/"+identifier)) + } + if issuerReadResp != nil { + issuerName := issuerReadResp.Data["issuer_name"].(string) + if issuerName != "" { + identifier = issuerName + } + } + } if len(issued) == 0 { - issued = sanitizePath(path) + "/issuer/" + certId.(string) + issued = sanitizePath(path) + "/issuer/" + identifier } else { - issued = issued + "," + sanitizePath(path) + "/issuer/" + certId.(string) + issued = issued + "," + sanitizePath(path) + "/issuer/" + identifier } } } diff --git a/command/pki_list_children_test.go b/command/pki_list_children_test.go new file mode 100644 index 000000000000..570e124732ed --- /dev/null +++ b/command/pki_list_children_test.go @@ -0,0 +1,102 @@ +package command + +import ( + "strings" + "testing" +) + +func TestPKIListChildren(t *testing.T) { + t.Parallel() + + client, closer := testVaultServer(t) + defer closer() + + // Relationship Map to Create + // pki-root | pki-newroot | pki-empty + // RootX1 RootX2 RootX4 RootX3 + // | | + // ---------------------------------------------- + // v v + // IntX1 IntX2 pki-int + // | | + // v v + // IntX3 (-----------------------) IntX3(also) + // + // Here X1,X2 have the same name (same mount) + // RootX4 uses the same key as RootX1 (but a different common_name/subject) + // RootX3 has the same name, and is on a different mount + // RootX1 has issued IntX1; RootX3 has issued IntX2 + createComplicatedIssuerSetUp(t, client) + + cases := []struct { + name string + args []string + expectedMatches map[string]bool + jsonOut bool + shouldError bool + expectErrorCont string + expectErrorNotCont string + nonJsonOutputCont string + }{ + { + "rootX1-default-children", + []string{"pki", "list-intermediates", "-format=json", "-use_names=true", "pki-root/issuer/rootX1"}, + map[string]bool{ + "pki-root/issuer/rootX1": true, + "pki-root/issuer/rootX2": false, + "pki-newroot/issuer/rootX3": false, + "pki-root/issuer/rootX4": false, + "pki-int/issuer/intX1": true, + "pki-int/issuer/intX2": false, + "pki-int/issuer/intX3": false, + "pki-int/issuer/intX3also": false, + }, + true, + false, + "", + "", + "", + }, + } + for _, testCase := range cases { + var errString string + var results map[string]interface{} + var stdOut string + + if testCase.jsonOut { + results, errString = execPKIVerifyJson(t, client, false, testCase.shouldError, testCase.args) + } else { + stdOut, errString = execPKIVerifyNonJson(t, client, testCase.shouldError, testCase.args) + } + + // Verify Error Behavior + if testCase.shouldError { + if errString == "" { + t.Fatalf("Expected error in Testcase %s : no error produced, got results %s", testCase.name, results) + } + if testCase.expectErrorCont != "" && !strings.Contains(errString, testCase.expectErrorCont) { + t.Fatalf("Expected error in Testcase %s to contain %s, but got error %s", testCase.name, testCase.expectErrorCont, errString) + } + if testCase.expectErrorNotCont != "" && strings.Contains(errString, testCase.expectErrorNotCont) { + t.Fatalf("Expected error in Testcase %s to not contain %s, but got error %s", testCase.name, testCase.expectErrorNotCont, errString) + } + } else { + if errString != "" { + t.Fatalf("Error in Testcase %s : no error expected, but got error: %s", testCase.name, errString) + } + } + + // Verify Output + if testCase.jsonOut { + isMatch, errString := verifyExpectedJson(testCase.expectedMatches, results) + if !isMatch { + t.Fatalf("Expected Results for Testcase %s, do not match returned results %s", testCase.name, errString) + } + } else { + if !strings.Contains(stdOut, testCase.nonJsonOutputCont) { + t.Fatalf("Expected standard output for Testcase %s to contain %s, but got %s", testCase.name, testCase.nonJsonOutputCont, stdOut) + } + } + + } +} diff --git a/command/pki_verify_sign_command.go b/command/pki_verify_sign_command.go index ff2976cd1b47..ce00eeee55d4 100644 --- a/command/pki_verify_sign_command.go +++ b/command/pki_verify_sign_command.go @@ -115,7 +115,7 @@ func verifySignBetween(client *api.Client, issuerPath string, issuedPath string) return fmt.Errorf("error: unable to fetch issuer %v: %w", issuerPath, err), nil } if len(issuedPath) <= 2 { - return fmt.Errorf(fmt.Sprintf("%v", issuedPath)), nil + return fmt.Errorf("%v", issuedPath), nil } caChainRaw := issuedCertResp.Data["ca_chain"] if caChainRaw == nil { diff --git a/command/pki_verify_sign_test.go b/command/pki_verify_sign_test.go index 0a3b239bb760..6d2f0b3ef3f1 100644 --- a/command/pki_verify_sign_test.go +++ b/command/pki_verify_sign_test.go @@ -2,6 +2,7 @@ package command import ( "bytes" + "context" "encoding/json" "fmt" "strings" @@ -159,7 +160,7 @@ func convertListOfInterfaceToString(list []interface{}, sep string) string { func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { // Relationship Map to Create - // pki-root | pki-newroot + // pki-root | pki-newroot | pki-empty // RootX1 RootX2 RootX4 RootX3 // | | // ---------------------------------------------- @@ -201,6 +202,14 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { t.Fatalf("pki mount error: %#v", err) } + // Used to check handling empty list responses: Not Used for Any Issuers / Certificates + if err := client.Sys().Mount("pki-empty", &api.MountInput{ + Type: "pki", + Config: api.MountConfigInput{}, + }); err != nil { + t.Fatalf("pki mount error: %#v", err) + } + resp, err := client.Logical().Write("pki-root/root/generate/internal", map[string]interface{}{ "key_type": "ec", "common_name": "Root X", @@ -208,7 +217,6 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { "issuer_name": "rootX1", "key_name": "rootX1", }) - t.Logf("%s", resp.Data) if err != nil || resp == nil { t.Fatalf("failed to prime CA: %v", err) } @@ -219,7 +227,6 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { "ttl": "3650d", "issuer_name": "rootX2", }) - t.Logf("%s", resp.Data) if err != nil || resp == nil { t.Fatalf("failed to prime CA: %v", err) } @@ -284,7 +291,7 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { break } } - if resp, err := client.Logical().Write("pki-int/issuer/"+importIssuerId, map[string]interface{}{ + if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+importIssuerId, map[string]interface{}{ "issuer_name": "intX1", }); err != nil || resp == nil { t.Fatalf("error naming issuer %v", err) @@ -316,8 +323,7 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { } int2CertChain := convertListOfInterfaceToString(int2CertChainRaw.([]interface{}), "\n") importInt2Resp, err := client.Logical().Write("pki-int/issuers/import/cert", map[string]interface{}{ - "pem_bundle": int2CertChain, - "issuer_name": "intX2", + "pem_bundle": int2CertChain, }) if err != nil || importInt2Resp == nil { t.Fatalf("failed to import certificate: %v", err) @@ -333,7 +339,7 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { break } } - if resp, err := client.Logical().Write("pki-int/issuer/"+importIssuer2Id, map[string]interface{}{ + if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+importIssuer2Id, map[string]interface{}{ "issuer_name": "intX2", }); err != nil || resp == nil { t.Fatalf("error naming issuer %v", err) @@ -382,7 +388,7 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { break } } - if resp, err := client.Logical().Write("pki-int/issuer/"+importIssuer3Id1, map[string]interface{}{ + if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+importIssuer3Id1, map[string]interface{}{ "issuer_name": "intX3", }); err != nil || resp == nil { t.Fatalf("error naming issuer %v", err) @@ -416,7 +422,7 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { break } } - if resp, err := client.Logical().Write("pki-int/issuer/"+importIssuer3Id2, map[string]interface{}{ + if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+importIssuer3Id2, map[string]interface{}{ "issuer_name": "intX3also", }); err != nil || resp == nil { t.Fatalf("error naming issuer %v", err) From 46138486494818d22cc08d7c8780b9a7160d380a Mon Sep 17 00:00:00 2001 From: kitography Date: Thu, 26 Jan 2023 16:01:47 -0500 Subject: [PATCH 15/21] Fix tests. --- command/pki_list_children_test.go | 71 ++++++++++++++++++++++++++ command/pki_verify_sign_test.go | 85 +++++++++++++++++++------------ 2 files changed, 123 insertions(+), 33 deletions(-) diff --git a/command/pki_list_children_test.go b/command/pki_list_children_test.go index 570e124732ed..f5bbceee4037 100644 --- a/command/pki_list_children_test.go +++ b/command/pki_list_children_test.go @@ -38,6 +38,29 @@ func TestPKIListChildren(t *testing.T) { expectErrorNotCont string nonJsonOutputCont string }{ + { + "rootX1-match-everything-no-constraints", + []string{"pki", "list-intermediates", "-format=json", "-use_names=true", + "-subject_match=false", "-key_id_match=false", "-direct_sign=false", "-indirect_sign=false", "-path_match=false", + "pki-root/issuer/rootX1"}, + map[string]bool{ + "pki-root/issuer/rootX1": true, + "pki-root/issuer/rootX2": true, + "pki-newroot/issuer/rootX3": true, + "pki-root/issuer/rootX4": true, + "pki-int/issuer/intX1": true, + "pki-int/issuer/intX2": true, + "pki-int/issuer/intX3": true, + "pki-int/issuer/intX3also": true, + "pki-int/issuer/rootX1": true, + "pki-int/issuer/rootX3": true, + }, + true, + false, + "", + "", + "", + }, { "rootX1-default-children", []string{"pki", "list-intermediates", "-format=json", "-use_names=true", "pki-root/issuer/rootX1"}, @@ -50,6 +73,54 @@ func TestPKIListChildren(t *testing.T) { "pki-int/issuer/intX2": false, "pki-int/issuer/intX3": false, "pki-int/issuer/intX3also": false, + "pki-int/issuer/rootX1": true, + "pki-int/issuer/rootX3": false, + }, + true, + false, + "", + "", + "", + }, + { + "rootX1-subject-match-only", + []string{"pki", "list-intermediates", "-format=json", "-use_names=true", + "-key_id_match=false", "-direct_sign=false", "-indirect_sign=false", + "pki-root/issuer/rootX1"}, + map[string]bool{ + "pki-root/issuer/rootX1": true, + "pki-root/issuer/rootX2": true, + "pki-newroot/issuer/rootX3": true, + "pki-root/issuer/rootX4": false, + "pki-int/issuer/intX1": true, + "pki-int/issuer/intX2": true, + "pki-int/issuer/intX3": false, + "pki-int/issuer/intX3also": false, + "pki-int/issuer/rootX1": true, + "pki-int/issuer/rootX3": true, + }, + true, + false, + "", + "", + "", + }, + { + "rootX1-in-path", + []string{"pki", "list-intermediates", "-format=json", "-use_names=true", + "-subject_match=false", "-key_id_match=false", "-direct_sign=false", "-indirect_sign=false", "-path_match=true", + "pki-root/issuer/rootX1"}, + map[string]bool{ + "pki-root/issuer/rootX1": true, + "pki-root/issuer/rootX2": false, + "pki-newroot/issuer/rootX3": false, + "pki-root/issuer/rootX4": false, + "pki-int/issuer/intX1": true, + "pki-int/issuer/intX2": false, + "pki-int/issuer/intX3": true, + "pki-int/issuer/intX3also": false, + "pki-int/issuer/rootX1": true, + "pki-int/issuer/rootX3": false, }, true, false, diff --git a/command/pki_verify_sign_test.go b/command/pki_verify_sign_test.go index 6d2f0b3ef3f1..843fab6b00b2 100644 --- a/command/pki_verify_sign_test.go +++ b/command/pki_verify_sign_test.go @@ -258,6 +258,10 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { if err != nil || int1CsrResp == nil { t.Fatalf("failed to generate CSR: %v", err) } + int1KeyId, ok := int1CsrResp.Data["key_id"] + if !ok { + t.Fatalf("no key_id produced when generating csr, response %v", int1CsrResp.Data) + } int1CsrRaw, ok := int1CsrResp.Data["csr"] if !ok { t.Fatalf("no csr produced when generating intermediate, resp: %v", int1CsrResp) @@ -284,18 +288,24 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { if !ok { t.Fatalf("no mapping data returned on issuer import: %v", importInt1Resp) } - importIssuerId := "" for key, value := range importIssuerIdMap.(map[string]interface{}) { if value != nil && len(value.(string)) > 0 { - importIssuerId = key - break + if value != int1KeyId { + t.Fatalf("Expected exactly one key_match to %v, got multiple: %v", int1KeyId, importIssuerIdMap) + } + if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+key, map[string]interface{}{ + "issuer_name": "intX1", + }); err != nil || resp == nil { + t.Fatalf("error naming issuer %v", err) + } + } else { + if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+key, map[string]interface{}{ + "issuer_name": "rootX1", + }); err != nil || resp == nil { + t.Fatalf("error naming issuer parent %v", err) + } } } - if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+importIssuerId, map[string]interface{}{ - "issuer_name": "intX1", - }); err != nil || resp == nil { - t.Fatalf("error naming issuer %v", err) - } // Intermediate X2 int2CsrResp, err := client.Logical().Write("pki-int/intermediate/generate/internal", map[string]interface{}{ @@ -306,6 +316,10 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { if err != nil || int2CsrResp == nil { t.Fatalf("failed to generate CSR: %v", err) } + int2KeyId, ok := int2CsrResp.Data["key_id"] + if !ok { + t.Fatalf("no key material returned from producing csr, resp: %v", int2CsrResp) + } int2CsrRaw, ok := int2CsrResp.Data["csr"] if !ok { t.Fatalf("no csr produced when generating intermediate, resp: %v", int2CsrResp) @@ -332,18 +346,24 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { if !ok { t.Fatalf("no mapping data returned on issuer import: %v", importInt2Resp) } - importIssuer2Id := "" for key, value := range importIssuer2IdMap.(map[string]interface{}) { if value != nil && len(value.(string)) > 0 { - importIssuer2Id = key - break + if value != int2KeyId { + t.Fatalf("unexpected key_match with ca_chain, expected only %v, got %v", int2KeyId, importIssuer2IdMap) + } + if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+key, map[string]interface{}{ + "issuer_name": "intX2", + }); err != nil || resp == nil { + t.Fatalf("error naming issuer %v", err) + } + } else { + if resp, err := client.Logical().Write("pki-int/issuer/"+key, map[string]interface{}{ + "issuer_name": "rootX3", + }); err != nil || resp == nil { + t.Fatalf("error naming parent issuer %v", err) + } } } - if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+importIssuer2Id, map[string]interface{}{ - "issuer_name": "intX2", - }); err != nil || resp == nil { - t.Fatalf("error naming issuer %v", err) - } // Intermediate X3 int3CsrResp, err := client.Logical().Write("pki-int/intermediate/generate/internal", map[string]interface{}{ @@ -354,6 +374,7 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { if err != nil || int3CsrResp == nil { t.Fatalf("failed to generate CSR: %v", err) } + int3KeyId, ok := int3CsrResp.Data["key_id"] int3CsrRaw, ok := int3CsrResp.Data["csr"] if !ok { t.Fatalf("no csr produced when generating intermediate, resp: %v", int3CsrResp) @@ -381,18 +402,17 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { if !ok { t.Fatalf("no mapping data returned on issuer import: %v", importInt2Resp) } - importIssuer3Id1 := "" for key, value := range importIssuer3IdMap1.(map[string]interface{}) { - if value != nil && len(value.(string)) > 0 { - importIssuer3Id1 = key + if value != nil && len(value.(string)) > 0 && value == int3KeyId { + if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+key, map[string]interface{}{ + "issuer_name": "intX3", + }); err != nil || resp == nil { + t.Fatalf("error naming issuer %v", err) + } break } } - if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+importIssuer3Id1, map[string]interface{}{ - "issuer_name": "intX3", - }); err != nil || resp == nil { - t.Fatalf("error naming issuer %v", err) - } + // sign by intX2 and import int3CertResp2, err := client.Logical().Write("pki-int/issuer/intX2/sign-intermediate", map[string]interface{}{ "csr": int3Csr, @@ -415,18 +435,17 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { if !ok { t.Fatalf("no mapping data returned on issuer import: %v", importInt2Resp) } - importIssuer3Id2 := "" for key, value := range importIssuer3IdMap2.(map[string]interface{}) { - if value != nil && len(value.(string)) > 0 { - importIssuer3Id2 = key - break + if value != nil && len(value.(string)) > 0 && value == int3KeyId { + if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+key, map[string]interface{}{ + "issuer_name": "intX3also", + }); err != nil || resp == nil { + t.Fatalf("error naming issuer %v", err) + } + break // Parent Certs Already Named } } - if resp, err := client.Logical().JSONMergePatch(context.Background(), "pki-int/issuer/"+importIssuer3Id2, map[string]interface{}{ - "issuer_name": "intX3also", - }); err != nil || resp == nil { - t.Fatalf("error naming issuer %v", err) - } + } func verifyExpectedJson(expectedResults map[string]bool, results map[string]interface{}) (isMatch bool, error string) { From 77f79a585cef1d1d580fe28bbdcdcad5e98448d1 Mon Sep 17 00:00:00 2001 From: kitography Date: Thu, 26 Jan 2023 16:55:59 -0500 Subject: [PATCH 16/21] Update descriptions for correctness based on PR reviews. --- command/pki_list_children_command.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go index f2dc2fc4c575..7516f6078dcb 100644 --- a/command/pki_list_children_command.go +++ b/command/pki_list_children_command.go @@ -33,12 +33,13 @@ func (c *PKIListChildrenCommand) Synopsis() string { func (c *PKIListChildrenCommand) Help() string { helpText := ` -Usage: vault pki verify-sign PARENT CHILD -Returns four fields of information: -- signature_match: was the key of the issuer used to sign the issued -- path_match: the possible issuer appears in the valid certificate chain of the issued -- key_id_match: does the key-id of the issuer match the key_id of the subject -- subject_match: does the subject name of the issuer match the issuer subject of the issued +Usage: vault pki list-intermediates PARENT CHILD-LIST +PARENT is the certificate that might be the issuer that everything should be verified against. +CHILD-LIST is a comma-separated list of intermediates to be compared to the PARENT. It can be omitted, in which case a +list will be constructed from all accessible pki mounts. +This returns a list of issuing certificates, and whether they are a match. +By default, the type of match required is whether the PARENT has the expected subject, key_id, and could have (directly) +signed this issuer. This can be updated by changed the corresponding flag. ` return strings.TrimSpace(helpText) } From 42ba023bb0816a4f71bfb15c461cc70909984eb8 Mon Sep 17 00:00:00 2001 From: kitography Date: Thu, 26 Jan 2023 17:00:05 -0500 Subject: [PATCH 17/21] make fmt. --- command/pki_list_children_test.go | 18 ++++++++++++------ command/pki_verify_sign_test.go | 1 - 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/command/pki_list_children_test.go b/command/pki_list_children_test.go index f5bbceee4037..78ce1fdb8757 100644 --- a/command/pki_list_children_test.go +++ b/command/pki_list_children_test.go @@ -40,9 +40,11 @@ func TestPKIListChildren(t *testing.T) { }{ { "rootX1-match-everything-no-constraints", - []string{"pki", "list-intermediates", "-format=json", "-use_names=true", + []string{ + "pki", "list-intermediates", "-format=json", "-use_names=true", "-subject_match=false", "-key_id_match=false", "-direct_sign=false", "-indirect_sign=false", "-path_match=false", - "pki-root/issuer/rootX1"}, + "pki-root/issuer/rootX1", + }, map[string]bool{ "pki-root/issuer/rootX1": true, "pki-root/issuer/rootX2": true, @@ -84,9 +86,11 @@ func TestPKIListChildren(t *testing.T) { }, { "rootX1-subject-match-only", - []string{"pki", "list-intermediates", "-format=json", "-use_names=true", + []string{ + "pki", "list-intermediates", "-format=json", "-use_names=true", "-key_id_match=false", "-direct_sign=false", "-indirect_sign=false", - "pki-root/issuer/rootX1"}, + "pki-root/issuer/rootX1", + }, map[string]bool{ "pki-root/issuer/rootX1": true, "pki-root/issuer/rootX2": true, @@ -107,9 +111,11 @@ func TestPKIListChildren(t *testing.T) { }, { "rootX1-in-path", - []string{"pki", "list-intermediates", "-format=json", "-use_names=true", + []string{ + "pki", "list-intermediates", "-format=json", "-use_names=true", "-subject_match=false", "-key_id_match=false", "-direct_sign=false", "-indirect_sign=false", "-path_match=true", - "pki-root/issuer/rootX1"}, + "pki-root/issuer/rootX1", + }, map[string]bool{ "pki-root/issuer/rootX1": true, "pki-root/issuer/rootX2": false, diff --git a/command/pki_verify_sign_test.go b/command/pki_verify_sign_test.go index 843fab6b00b2..838e1c7408c2 100644 --- a/command/pki_verify_sign_test.go +++ b/command/pki_verify_sign_test.go @@ -445,7 +445,6 @@ func createComplicatedIssuerSetUp(t *testing.T, client *api.Client) { break // Parent Certs Already Named } } - } func verifyExpectedJson(expectedResults map[string]bool, results map[string]interface{}) (isMatch bool, error string) { From f9ac1959327dba5d73a831b7a60d612aace66bd6 Mon Sep 17 00:00:00 2001 From: kitography Date: Thu, 26 Jan 2023 17:12:03 -0500 Subject: [PATCH 18/21] Updates based on PR feedback. --- changelog/18463.txt | 2 +- command/pki_list_children_command.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/changelog/18463.txt b/changelog/18463.txt index 7c521794c21d..538f66eb1dc7 100644 --- a/changelog/18463.txt +++ b/changelog/18463.txt @@ -1,3 +1,3 @@ ```release-note:improvement -cli/pki: Add List-Children functionality to pki client. +cli/pki: Add List-Intermediates functionality to pki client. ``` diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go index 7516f6078dcb..b5e7432dea04 100644 --- a/command/pki_list_children_command.go +++ b/command/pki_list_children_command.go @@ -148,7 +148,7 @@ func (c *PKIListChildrenCommand) Run(args []string) int { return 1 } if rawIssuersResp == nil { - continue // TODO: Empty Issuers Response this throws an error + continue } issuersMap := rawIssuersResp.Data["keys"] certList := issuersMap.([]interface{}) From 90eb1bc3f708f38e566e073f89b7a2f6456be07a Mon Sep 17 00:00:00 2001 From: kitography Date: Fri, 27 Jan 2023 08:25:26 -0500 Subject: [PATCH 19/21] Allow multiple arguements (space separated) --- command/pki_list_children_command.go | 98 +++++++++++++++++----------- command/pki_list_children_test.go | 58 ++++++++++++++++ 2 files changed, 119 insertions(+), 37 deletions(-) diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go index b5e7432dea04..33bde49e5015 100644 --- a/command/pki_list_children_command.go +++ b/command/pki_list_children_command.go @@ -6,6 +6,8 @@ import ( "strconv" "strings" + "github.com/hashicorp/vault/api" + "github.com/ghodss/yaml" "github.com/ryanuber/columnize" ) @@ -33,10 +35,10 @@ func (c *PKIListChildrenCommand) Synopsis() string { func (c *PKIListChildrenCommand) Help() string { helpText := ` -Usage: vault pki list-intermediates PARENT CHILD-LIST +Usage: vault pki list-intermediates PARENT [CHILD] [CHILD] [CHILD] ... PARENT is the certificate that might be the issuer that everything should be verified against. -CHILD-LIST is a comma-separated list of intermediates to be compared to the PARENT. It can be omitted, in which case a -list will be constructed from all accessible pki mounts. +CHILD is a list of paths to certificates to be compared to the PARENT, or pki mounts to look for certificates on. +If CHILD is omitted entirely, w list will be constructed from all accessible pki mounts. This returns a list of issuing certificates, and whether they are a match. By default, the type of match required is whether the PARENT has the expected subject, key_id, and could have (directly) signed this issuer. This can be updated by changed the corresponding flag. @@ -112,14 +114,12 @@ func (c *PKIListChildrenCommand) Run(args []string) int { c.UI.Error("Not enough arguments (expected potential parent, got nothing)") return 1 } else if len(args) > 2 { - c.UI.Error(fmt.Sprintf("Too many arguments (expected only potential issuer and issued, got %d arguments)", len(args))) for _, arg := range args { if strings.HasPrefix(arg, "-") { c.UI.Warn(fmt.Sprintf("Options (%v) must be specified before positional arguments (%v)", arg, args[0])) break } } - return 1 } client, err := c.Client() @@ -129,9 +129,23 @@ func (c *PKIListChildrenCommand) Run(args []string) int { } issuer := sanitizePath(args[0]) - issued := "" + var issued []string if len(args) > 1 { - issued = sanitizePath(args[1]) + for _, arg := range args[1:] { + // Arg Might be a Fully Qualified Path + if strings.Contains(sanitizePath(arg), "/issuer/") || + strings.Contains(sanitizePath(arg), "/certs/") || + strings.Contains(sanitizePath(arg), "/revoked/") { + issued = append(issued, sanitizePath(arg)) + } else { // Or Arg Might be a Mount + mountCaList, err := c.getIssuerListFromMount(client, arg) + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + issued = append(issued, mountCaList...) + } + } } else { mountListRaw, err := client.Logical().Read("/sys/mounts/") if err != nil { @@ -141,37 +155,12 @@ func (c *PKIListChildrenCommand) Run(args []string) int { for path, rawValueMap := range mountListRaw.Data { valueMap := rawValueMap.(map[string]interface{}) if valueMap["type"].(string) == "pki" { - issuerListEndpoint := sanitizePath(path) + "/issuers" - rawIssuersResp, err := client.Logical().List(issuerListEndpoint) + mountCaList, err := c.getIssuerListFromMount(client, sanitizePath(path)) if err != nil { - c.UI.Error(fmt.Sprintf("Failed to Read List of Issuers within Mount %v: %v", path, err)) + c.UI.Error(err.Error()) return 1 } - if rawIssuersResp == nil { - continue - } - issuersMap := rawIssuersResp.Data["keys"] - certList := issuersMap.([]interface{}) - for _, certId := range certList { - identifier := certId.(string) - if c.flagUseNames { - issuerReadResp, err := client.Logical().Read(sanitizePath(path) + "/issuer/" + identifier) - if err != nil { - c.UI.Warn(fmt.Sprintf("Unable to Fetch Issuer to Recover Name at: %v", sanitizePath(path)+"/issuer/"+identifier)) - } - if issuerReadResp != nil { - issuerName := issuerReadResp.Data["issuer_name"].(string) - if issuerName != "" { - identifier = issuerName - } - } - } - if len(issued) == 0 { - issued = sanitizePath(path) + "/issuer/" + identifier - } else { - issued = issued + "," + sanitizePath(path) + "/issuer/" + identifier - } - } + issued = append(issued, mountCaList...) } } } @@ -187,7 +176,7 @@ func (c *PKIListChildrenCommand) Run(args []string) int { "signature_match": c.flagSignatureMatch, } - for _, child := range strings.Split(issued, ",") { + for _, child := range issued { path := sanitizePath(child) if path != "" { err, verifyResults := verifySignBetween(client, issuer, path) @@ -199,11 +188,46 @@ func (c *PKIListChildrenCommand) Run(args []string) int { } } - c.outputResults(childrenMatches) + err = c.outputResults(childrenMatches) + if err != nil { + c.UI.Error(err.Error()) + return 1 + } return 0 } +func (c *PKIListChildrenCommand) getIssuerListFromMount(client *api.Client, mountString string) ([]string, error) { + var issuerList []string + issuerListEndpoint := sanitizePath(mountString) + "/issuers" + rawIssuersResp, err := client.Logical().List(issuerListEndpoint) + if err != nil { + return issuerList, fmt.Errorf("failed to read list of issuers within mount %v: %v", mountString, err) + } + if rawIssuersResp == nil { // No Issuers (Empty Mount) + return issuerList, nil + } + issuersMap := rawIssuersResp.Data["keys"] + certList := issuersMap.([]interface{}) + for _, certId := range certList { + identifier := certId.(string) + if c.flagUseNames { + issuerReadResp, err := client.Logical().Read(sanitizePath(mountString) + "/issuer/" + identifier) + if err != nil { + c.UI.Warn(fmt.Sprintf("Unable to Fetch Issuer to Recover Name at: %v", sanitizePath(mountString)+"/issuer/"+identifier)) + } + if issuerReadResp != nil { + issuerName := issuerReadResp.Data["issuer_name"].(string) + if issuerName != "" { + identifier = issuerName + } + } + } + issuerList = append(issuerList, sanitizePath(mountString)+"/issuer/"+identifier) + } + return issuerList, nil +} + func checkIfResultsMatchFilters(verifyResults, constraintMap map[string]bool) bool { for key, required := range constraintMap { if required == true { diff --git a/command/pki_list_children_test.go b/command/pki_list_children_test.go index 78ce1fdb8757..23f642fd65aa 100644 --- a/command/pki_list_children_test.go +++ b/command/pki_list_children_test.go @@ -134,6 +134,64 @@ func TestPKIListChildren(t *testing.T) { "", "", }, + { + "rootX1-only-int-mount", + []string{ + "pki", "list-intermediates", "-format=json", "-use_names=true", + "-subject_match=false", "-key_id_match=false", "-direct_sign=false", "-indirect_sign=false", "-path_match=true", + "pki-root/issuer/rootX1", "pki-int/", + }, + map[string]bool{ + "pki-int/issuer/intX1": true, + "pki-int/issuer/intX2": false, + "pki-int/issuer/intX3": true, + "pki-int/issuer/intX3also": false, + "pki-int/issuer/rootX1": true, + "pki-int/issuer/rootX3": false, + }, + true, + false, + "", + "", + "", + }, + { + "rootX1-subject-match-root-mounts-only", + []string{ + "pki", "list-intermediates", "-format=json", "-use_names=true", + "-key_id_match=false", "-direct_sign=false", "-indirect_sign=false", + "pki-root/issuer/rootX1", "pki-root/", "pki-newroot", "pki-empty", + }, + map[string]bool{ + "pki-root/issuer/rootX1": true, + "pki-root/issuer/rootX2": true, + "pki-newroot/issuer/rootX3": true, + "pki-root/issuer/rootX4": false, + }, + true, + false, + "", + "", + "", + }, + { + "rootX1-subject-match-these-certs-only", + []string{ + "pki", "list-intermediates", "-format=json", "-use_names=true", + "-key_id_match=false", "-direct_sign=false", "-indirect_sign=false", + "pki-root/issuer/rootX1", "pki-root/issuer/rootX2", "pki-newroot/issuer/rootX3", "pki-root/issuer/rootX4", + }, + map[string]bool{ + "pki-root/issuer/rootX2": true, + "pki-newroot/issuer/rootX3": true, + "pki-root/issuer/rootX4": false, + }, + true, + false, + "", + "", + "", + }, } for _, testCase := range cases { var errString string From 97a068da56a2cf1950c778bb54d7c7e4e1615164 Mon Sep 17 00:00:00 2001 From: kitography Date: Fri, 27 Jan 2023 08:45:17 -0500 Subject: [PATCH 20/21] Typo-fix from PR review. --- command/pki_list_children_command.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go index 33bde49e5015..e8fbd973740d 100644 --- a/command/pki_list_children_command.go +++ b/command/pki_list_children_command.go @@ -38,10 +38,10 @@ func (c *PKIListChildrenCommand) Help() string { Usage: vault pki list-intermediates PARENT [CHILD] [CHILD] [CHILD] ... PARENT is the certificate that might be the issuer that everything should be verified against. CHILD is a list of paths to certificates to be compared to the PARENT, or pki mounts to look for certificates on. -If CHILD is omitted entirely, w list will be constructed from all accessible pki mounts. +If CHILD is omitted entirely, the list will be constructed from all accessible pki mounts. This returns a list of issuing certificates, and whether they are a match. By default, the type of match required is whether the PARENT has the expected subject, key_id, and could have (directly) -signed this issuer. This can be updated by changed the corresponding flag. +signed this issuer. The match criteria can be updated by changed the corresponding flag. ` return strings.TrimSpace(helpText) } From 5028ef15bba7f07f9b4286acd114876ddd97a32a Mon Sep 17 00:00:00 2001 From: kitography Date: Fri, 27 Jan 2023 08:49:09 -0500 Subject: [PATCH 21/21] sanitizePath once. --- command/pki_list_children_command.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/command/pki_list_children_command.go b/command/pki_list_children_command.go index e8fbd973740d..23a390a7ad4a 100644 --- a/command/pki_list_children_command.go +++ b/command/pki_list_children_command.go @@ -132,11 +132,12 @@ func (c *PKIListChildrenCommand) Run(args []string) int { var issued []string if len(args) > 1 { for _, arg := range args[1:] { + cleanPath := sanitizePath(arg) // Arg Might be a Fully Qualified Path - if strings.Contains(sanitizePath(arg), "/issuer/") || - strings.Contains(sanitizePath(arg), "/certs/") || - strings.Contains(sanitizePath(arg), "/revoked/") { - issued = append(issued, sanitizePath(arg)) + if strings.Contains(cleanPath, "/issuer/") || + strings.Contains(cleanPath, "/certs/") || + strings.Contains(cleanPath, "/revoked/") { + issued = append(issued, cleanPath) } else { // Or Arg Might be a Mount mountCaList, err := c.getIssuerListFromMount(client, arg) if err != nil {