From f226d0103f9a953cfbbcbc8abfd40748b4f82e3b Mon Sep 17 00:00:00 2001 From: Scott Miller Date: Tue, 8 Feb 2022 10:37:40 -0600 Subject: [PATCH] Add duration/count metrics to PKI issue and revoke flows (#13889) * Add duration/count metrics to PKI issue and revoke flows * docs, changelog * tidy * last tidy * remove err * Update callsites * Simple returns * Handle the fact that test cases don't have namespaces * Add mount point to the request * fmt * Handle empty mount point, and add it to unit tests * improvement * Turns out sign-verbatim is tricky, it can take a role but doesn't have to * Get around the field schema problem --- builtin/logical/pki/backend.go | 72 ++++++++++++++++- builtin/logical/pki/backend_test.go | 106 +++++++++++++++---------- builtin/logical/pki/path_issue_sign.go | 42 ++-------- builtin/logical/pki/path_revoke.go | 4 +- changelog/13889.txt | 3 + website/content/docs/secrets/pki.mdx | 6 ++ 6 files changed, 152 insertions(+), 81 deletions(-) create mode 100644 changelog/13889.txt diff --git a/builtin/logical/pki/backend.go b/builtin/logical/pki/backend.go index 9f72d09e0..61b241a44 100644 --- a/builtin/logical/pki/backend.go +++ b/builtin/logical/pki/backend.go @@ -2,14 +2,24 @@ package pki import ( "context" + "fmt" "strings" "sync" "time" + "github.com/armon/go-metrics" + "github.com/hashicorp/vault/helper/metricsutil" + "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" ) +const ( + noRole = 0 + roleOptional = 1 + roleRequired = 2 +) + // Factory creates a new backend implementing the logical.Backend interface func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) { b := Backend(conf) @@ -106,7 +116,10 @@ type backend struct { tidyStatus *tidyStatus } -type tidyStatusState int +type ( + tidyStatusState int + roleOperation func(ctx context.Context, req *logical.Request, data *framework.FieldData, role *roleEntry) (*logical.Response, error) +) const ( tidyStatusInactive tidyStatusState = iota @@ -137,3 +150,60 @@ The PKI backend dynamically generates X509 server and client certificates. After mounting this backend, configure the CA using the "pem_bundle" endpoint within the "config/" path. ` + +func metricsKey(req *logical.Request, extra ...string) []string { + if req == nil || req.MountPoint == "" { + return extra + } + key := make([]string, len(extra)+1) + key[0] = req.MountPoint[:len(req.MountPoint)-1] + copy(key[1:], extra) + return key +} + +func (b *backend) metricsWrap(callType string, roleMode int, ofunc roleOperation) framework.OperationFunc { + return func(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { + key := metricsKey(req, callType) + var role *roleEntry + var labels []metrics.Label + var err error + + var roleName string + switch roleMode { + case roleRequired: + roleName = data.Get("role").(string) + case roleOptional: + r, ok := data.GetOk("role") + if ok { + roleName = r.(string) + } + } + if roleMode > noRole { + // Get the role + role, err = b.getRole(ctx, req.Storage, roleName) + if err != nil { + return nil, err + } + if role == nil && roleMode == roleRequired { + return logical.ErrorResponse(fmt.Sprintf("unknown role: %s", roleName)), nil + } + labels = []metrics.Label{{"role", roleName}} + } + + ns, err := namespace.FromContext(ctx) + if err == nil { + labels = append(labels, metricsutil.NamespaceLabel(ns)) + } + + start := time.Now() + defer metrics.MeasureSinceWithLabels(key, start, labels) + resp, err := ofunc(ctx, req, data, role) + + if err != nil || resp.IsError() { + metrics.IncrCounterWithLabels(append(key, "failure"), 1.0, labels) + } else { + metrics.IncrCounterWithLabels(key, 1.0, labels) + } + return resp, err + } +} diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 1098b5409..4be56d9cf 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -1792,10 +1792,11 @@ func TestBackend_PathFetchCertList(t *testing.T) { } resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: "root/generate/internal", - Storage: storage, - Data: rootData, + Operation: logical.UpdateOperation, + Path: "root/generate/internal", + Storage: storage, + Data: rootData, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to generate root, %#v", resp) @@ -1811,10 +1812,11 @@ func TestBackend_PathFetchCertList(t *testing.T) { } resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: "config/urls", - Storage: storage, - Data: urlsData, + Operation: logical.UpdateOperation, + Path: "config/urls", + Storage: storage, + Data: urlsData, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to config urls, %#v", resp) @@ -1831,10 +1833,11 @@ func TestBackend_PathFetchCertList(t *testing.T) { } resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: "roles/test-example", - Storage: storage, - Data: roleData, + Operation: logical.UpdateOperation, + Path: "roles/test-example", + Storage: storage, + Data: roleData, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to create a role, %#v", resp) @@ -1850,10 +1853,11 @@ func TestBackend_PathFetchCertList(t *testing.T) { "common_name": "example.test.com", } resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: "issue/test-example", - Storage: storage, - Data: certData, + Operation: logical.UpdateOperation, + Path: "issue/test-example", + Storage: storage, + Data: certData, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to issue a cert, %#v", resp) @@ -1867,9 +1871,10 @@ func TestBackend_PathFetchCertList(t *testing.T) { // list certs resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.ListOperation, - Path: "certs", - Storage: storage, + Operation: logical.ListOperation, + Path: "certs", + Storage: storage, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to list certs, %#v", resp) @@ -1884,9 +1889,10 @@ func TestBackend_PathFetchCertList(t *testing.T) { // list certs/ resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.ListOperation, - Path: "certs/", - Storage: storage, + Operation: logical.ListOperation, + Path: "certs/", + Storage: storage, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to list certs, %#v", resp) @@ -1919,10 +1925,11 @@ func TestBackend_SignVerbatim(t *testing.T) { } resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: "root/generate/internal", - Storage: storage, - Data: rootData, + Operation: logical.UpdateOperation, + Path: "root/generate/internal", + Storage: storage, + Data: rootData, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to generate root, %#v", *resp) @@ -1963,6 +1970,7 @@ func TestBackend_SignVerbatim(t *testing.T) { Data: map[string]interface{}{ "csr": pemCSR, }, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to sign-verbatim basic CSR: %#v", *resp) @@ -1980,10 +1988,11 @@ func TestBackend_SignVerbatim(t *testing.T) { "max_ttl": "8h", } resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: "roles/test", - Storage: storage, - Data: roleData, + Operation: logical.UpdateOperation, + Path: "roles/test", + Storage: storage, + Data: roleData, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to create a role, %#v", *resp) @@ -1999,6 +2008,7 @@ func TestBackend_SignVerbatim(t *testing.T) { "csr": pemCSR, "ttl": "5h", }, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to sign-verbatim ttl'd CSR: %#v", *resp) @@ -2017,6 +2027,7 @@ func TestBackend_SignVerbatim(t *testing.T) { "csr": pemCSR, "ttl": "12h", }, + MountPoint: "pki/", }) if err != nil { t.Fatal(err) @@ -2053,6 +2064,7 @@ func TestBackend_SignVerbatim(t *testing.T) { "csr": pemCSR, "not_after": "9999-12-31T23:59:59Z", }, + MountPoint: "pki/", }) if err != nil { t.Fatal(err) @@ -2088,10 +2100,11 @@ func TestBackend_SignVerbatim(t *testing.T) { "generate_lease": true, } resp, err = b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: "roles/test", - Storage: storage, - Data: roleData, + Operation: logical.UpdateOperation, + Path: "roles/test", + Storage: storage, + Data: roleData, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to create a role, %#v", *resp) @@ -2107,6 +2120,7 @@ func TestBackend_SignVerbatim(t *testing.T) { "csr": pemCSR, "ttl": "5h", }, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to sign-verbatim role-leased CSR: %#v", *resp) @@ -2336,10 +2350,11 @@ func TestBackend_SignSelfIssued(t *testing.T) { } resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: "root/generate/internal", - Storage: storage, - Data: rootData, + Operation: logical.UpdateOperation, + Path: "root/generate/internal", + Storage: storage, + Data: rootData, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to generate root, %#v", *resp) @@ -2370,6 +2385,7 @@ func TestBackend_SignSelfIssued(t *testing.T) { Data: map[string]interface{}{ "certificate": ss, }, + MountPoint: "pki/", }) if err != nil { t.Fatal(err) @@ -2400,6 +2416,7 @@ func TestBackend_SignSelfIssued(t *testing.T) { Data: map[string]interface{}{ "certificate": ss, }, + MountPoint: "pki/", }) if err != nil { t.Fatal(err) @@ -2419,6 +2436,7 @@ func TestBackend_SignSelfIssued(t *testing.T) { Data: map[string]interface{}{ "certificate": ss, }, + MountPoint: "pki/", }) if err != nil { t.Fatal(err) @@ -2481,10 +2499,11 @@ func TestBackend_SignSelfIssued_DifferentTypes(t *testing.T) { } resp, err := b.HandleRequest(context.Background(), &logical.Request{ - Operation: logical.UpdateOperation, - Path: "root/generate/internal", - Storage: storage, - Data: rootData, + Operation: logical.UpdateOperation, + Path: "root/generate/internal", + Storage: storage, + Data: rootData, + MountPoint: "pki/", }) if resp != nil && resp.IsError() { t.Fatalf("failed to generate root, %#v", *resp) @@ -2516,6 +2535,7 @@ func TestBackend_SignSelfIssued_DifferentTypes(t *testing.T) { Data: map[string]interface{}{ "certificate": ss, }, + MountPoint: "pki/", }) if err != nil { t.Fatal(err) @@ -2537,6 +2557,7 @@ func TestBackend_SignSelfIssued_DifferentTypes(t *testing.T) { "certificate": ss, "require_matching_certificate_algorithms": false, }, + MountPoint: "pki/", }) if err != nil { t.Fatal(err) @@ -2555,6 +2576,7 @@ func TestBackend_SignSelfIssued_DifferentTypes(t *testing.T) { "certificate": ss, "require_matching_certificate_algorithms": true, }, + MountPoint: "pki/", }) if err == nil { t.Fatal("expected error due to mismatched algorithms") diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index 97b99f56b..cffba8f3c 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -19,7 +19,7 @@ func pathIssue(b *backend) *framework.Path { Pattern: "issue/" + framework.GenericNameRegex("role"), Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.UpdateOperation: b.pathIssue, + logical.UpdateOperation: b.metricsWrap("issue", roleRequired, b.pathIssue), }, HelpSynopsis: pathIssueHelpSyn, @@ -35,7 +35,7 @@ func pathSign(b *backend) *framework.Path { Pattern: "sign/" + framework.GenericNameRegex("role"), Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.UpdateOperation: b.pathSign, + logical.UpdateOperation: b.metricsWrap("sign", roleRequired, b.pathSign), }, HelpSynopsis: pathSignHelpSyn, @@ -58,7 +58,7 @@ func pathSignVerbatim(b *backend) *framework.Path { Pattern: "sign-verbatim" + framework.OptionalParamRegex("role"), Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.UpdateOperation: b.pathSignVerbatim, + logical.UpdateOperation: b.metricsWrap("sign-verbatim", roleOptional, b.pathSignVerbatim), }, HelpSynopsis: pathSignHelpSyn, @@ -106,18 +106,7 @@ this value to an empty list.`, // pathIssue issues a certificate and private key from given parameters, // subject to role restrictions -func (b *backend) pathIssue(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - roleName := data.Get("role").(string) - - // Get the role - role, err := b.getRole(ctx, req.Storage, roleName) - if err != nil { - return nil, err - } - if role == nil { - return logical.ErrorResponse(fmt.Sprintf("unknown role: %s", roleName)), nil - } - +func (b *backend) pathIssue(ctx context.Context, req *logical.Request, data *framework.FieldData, role *roleEntry) (*logical.Response, error) { if role.KeyType == "any" { return logical.ErrorResponse("role key type \"any\" not allowed for issuing certificates, only signing"), nil } @@ -127,32 +116,13 @@ func (b *backend) pathIssue(ctx context.Context, req *logical.Request, data *fra // pathSign issues a certificate from a submitted CSR, subject to role // restrictions -func (b *backend) pathSign(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - roleName := data.Get("role").(string) - - // Get the role - role, err := b.getRole(ctx, req.Storage, roleName) - if err != nil { - return nil, err - } - if role == nil { - return logical.ErrorResponse(fmt.Sprintf("unknown role: %s", roleName)), nil - } - +func (b *backend) pathSign(ctx context.Context, req *logical.Request, data *framework.FieldData, role *roleEntry) (*logical.Response, error) { return b.pathIssueSignCert(ctx, req, data, role, true, false) } // pathSignVerbatim issues a certificate from a submitted CSR, *not* subject to // role restrictions -func (b *backend) pathSignVerbatim(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - roleName := data.Get("role").(string) - - // Get the role if one was specified - role, err := b.getRole(ctx, req.Storage, roleName) - if err != nil { - return nil, err - } - +func (b *backend) pathSignVerbatim(ctx context.Context, req *logical.Request, data *framework.FieldData, role *roleEntry) (*logical.Response, error) { entry := &roleEntry{ AllowLocalhost: true, AllowAnyName: true, diff --git a/builtin/logical/pki/path_revoke.go b/builtin/logical/pki/path_revoke.go index 547927717..c2726475f 100644 --- a/builtin/logical/pki/path_revoke.go +++ b/builtin/logical/pki/path_revoke.go @@ -23,7 +23,7 @@ hyphen-separated octal`, }, Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.UpdateOperation: b.pathRevokeWrite, + logical.UpdateOperation: b.metricsWrap("revoke", noRole, b.pathRevokeWrite), }, HelpSynopsis: pathRevokeHelpSyn, @@ -44,7 +44,7 @@ func pathRotateCRL(b *backend) *framework.Path { } } -func (b *backend) pathRevokeWrite(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { +func (b *backend) pathRevokeWrite(ctx context.Context, req *logical.Request, data *framework.FieldData, _ *roleEntry) (*logical.Response, error) { serial := data.Get("serial_number").(string) if len(serial) == 0 { return logical.ErrorResponse("The serial number must be provided"), nil diff --git a/changelog/13889.txt b/changelog/13889.txt new file mode 100644 index 000000000..8d98d4a7e --- /dev/null +++ b/changelog/13889.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/pki: Add count and duration metrics to PKI issue and revoke calls. +``` diff --git a/website/content/docs/secrets/pki.mdx b/website/content/docs/secrets/pki.mdx index fb69dbfa3..5f61018b2 100644 --- a/website/content/docs/secrets/pki.mdx +++ b/website/content/docs/secrets/pki.mdx @@ -211,6 +211,12 @@ associated leases, to prevent unintended revocation when not using a token with a long enough lifetime. To revoke these certificates, use the `pki/revoke` endpoint. +### Telemetry + +Beyond Vault's default telemetry around request processing, PKI exposes count and + duration metrics for the issue, sign, sign-verbatim, and revoke calls. The +metrics keys take the form `mount-path,operation,[failure]` with labels for namespace and role name. + ## Quick Start #### Mount the backend