diff --git a/builtin/logical/pki/path_fetch_issuers.go b/builtin/logical/pki/path_fetch_issuers.go index 92a5c97b72..352048783e 100644 --- a/builtin/logical/pki/path_fetch_issuers.go +++ b/builtin/logical/pki/path_fetch_issuers.go @@ -231,6 +231,9 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da // When the new name is in use but isn't this name, throw an error. return logical.ErrorResponse(err.Error()), nil } + if len(newName) > 0 && !nameMatcher.MatchString(newName) { + return logical.ErrorResponse("new key name outside of valid character limits"), nil + } newPath := data.Get("manual_chain").([]string) rawLeafBehavior := data.Get("leaf_not_after_behavior").(string) @@ -374,6 +377,9 @@ func (b *backend) pathPatchIssuer(ctx context.Context, req *logical.Request, dat // When the new name is in use but isn't this name, throw an error. return logical.ErrorResponse(err.Error()), nil } + if len(newName) > 0 && !nameMatcher.MatchString(newName) { + return logical.ErrorResponse("new key name outside of valid character limits"), nil + } if newName != issuer.Name { oldName = issuer.Name issuer.Name = newName diff --git a/builtin/logical/pki/path_roles_test.go b/builtin/logical/pki/path_roles_test.go index 99c72fe13b..c7035adadb 100644 --- a/builtin/logical/pki/path_roles_test.go +++ b/builtin/logical/pki/path_roles_test.go @@ -2,12 +2,14 @@ package pki import ( "context" + "fmt" "testing" "time" "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/hashicorp/vault/sdk/logical" "github.com/mitchellh/mapstructure" + "github.com/stretchr/testify/require" ) func TestPki_RoleGenerateLease(t *testing.T) { @@ -738,3 +740,282 @@ func TestPki_CertsLease(t *testing.T) { t.Fatalf("expected a response that contains a secret") } } + +func TestPki_RolePatch(t *testing.T) { + type TestCase struct { + Field string + Before interface{} + Patched interface{} + } + + testCases := []TestCase{ + { + Field: "ttl", + Before: int64(5), + Patched: int64(10), + }, + { + Field: "max_ttl", + Before: int64(5), + Patched: int64(10), + }, + { + Field: "allow_localhost", + Before: true, + Patched: false, + }, + { + Field: "allowed_domains", + Before: []string{"alex", "bob"}, + Patched: []string{"sam", "alex", "frank"}, + }, + { + Field: "allowed_domains_template", + Before: false, + Patched: true, + }, + { + Field: "allow_bare_domains", + Before: true, + Patched: false, + }, + { + Field: "allow_subdomains", + Before: false, + Patched: true, + }, + { + Field: "allow_glob_domains", + Before: true, + Patched: false, + }, + { + Field: "allow_wildcard_certificates", + Before: false, + Patched: true, + }, + { + Field: "allow_any_name", + Before: true, + Patched: false, + }, + { + Field: "enforce_hostnames", + Before: false, + Patched: true, + }, + { + Field: "allow_ip_sans", + Before: true, + Patched: false, + }, + { + Field: "allowed_uri_sans", + Before: []string{"gopher://*"}, + Patched: []string{"https://*"}, + }, + { + Field: "allowed_uri_sans_template", + Before: false, + Patched: true, + }, + { + Field: "allowed_other_sans", + Before: []string{"1.2.3.4;UTF8:magic"}, + Patched: []string{"4.3.2.1;UTF8:cigam"}, + }, + { + Field: "allowed_serial_numbers", + Before: []string{"*"}, + Patched: []string{""}, + }, + { + Field: "server_flag", + Before: true, + Patched: false, + }, + { + Field: "client_flag", + Before: false, + Patched: true, + }, + { + Field: "code_signing_flag", + Before: true, + Patched: false, + }, + { + Field: "email_protection_flag", + Before: false, + Patched: true, + }, + // key_type, key_bits, and signature_bits can't be tested in this setup + // due to their non-default stored nature. + { + Field: "key_usage", + Before: []string{"DigitialSignature"}, + Patched: []string{"DigitalSignature", "KeyAgreement"}, + }, + { + Field: "ext_key_usage", + Before: []string{"ServerAuth"}, + Patched: []string{"ClientAuth"}, + }, + { + Field: "ext_key_usage_oids", + Before: []string{"1.2.3.4"}, + Patched: []string{"4.3.2.1"}, + }, + { + Field: "use_csr_common_name", + Before: true, + Patched: false, + }, + { + Field: "use_csr_sans", + Before: false, + Patched: true, + }, + { + Field: "ou", + Before: []string{"crypto"}, + Patched: []string{"cryptosec"}, + }, + { + Field: "organization", + Before: []string{"hashicorp"}, + Patched: []string{"dadgarcorp"}, + }, + { + Field: "country", + Before: []string{"US"}, + Patched: []string{"USA"}, + }, + { + Field: "locality", + Before: []string{"Orange"}, + Patched: []string{"Blue"}, + }, + { + Field: "province", + Before: []string{"CA"}, + Patched: []string{"AC"}, + }, + { + Field: "street_address", + Before: []string{"101 First"}, + Patched: []string{"202 Second", "Unit 020"}, + }, + { + Field: "postal_code", + Before: []string{"12345"}, + Patched: []string{"54321-1234"}, + }, + { + Field: "generate_lease", + Before: false, + Patched: true, + }, + { + Field: "no_store", + Before: true, + Patched: false, + }, + { + Field: "require_cn", + Before: false, + Patched: true, + }, + { + Field: "policy_identifiers", + Before: []string{"1.2.3.4.5"}, + Patched: []string{"5.4.3.2.1"}, + }, + { + Field: "basic_constraints_valid_for_non_ca", + Before: true, + Patched: false, + }, + { + Field: "not_before_duration", + Before: int64(30), + Patched: int64(300), + }, + { + Field: "not_after", + Before: "9999-12-31T23:59:59Z", + Patched: "1230-12-31T23:59:59Z", + }, + { + Field: "issuer_ref", + Before: "default", + Patched: "missing", + }, + } + + b, storage := createBackendWithStorage(t) + + for index, testCase := range testCases { + var resp *logical.Response + var roleDataResp *logical.Response + var afterRoleDataResp *logical.Response + var err error + + // Create the role + roleData := map[string]interface{}{} + roleData[testCase.Field] = testCase.Before + + roleReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "roles/testrole", + Storage: storage, + Data: roleData, + } + + resp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad [%d/%v] create: err: %v resp: %#v", index, testCase.Field, err, resp) + } + + // Read the role after creation + roleReq.Operation = logical.ReadOperation + roleDataResp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (roleDataResp != nil && roleDataResp.IsError()) { + t.Fatalf("bad [%d/%v] read: err: %v resp: %#v", index, testCase.Field, err, resp) + } + + beforeRoleData := roleDataResp.Data + + // Patch the role + roleReq.Operation = logical.PatchOperation + roleReq.Data[testCase.Field] = testCase.Patched + resp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad [%d/%v] patch: err: %v resp: %#v", index, testCase.Field, err, resp) + } + + // Re-read and verify the role + roleReq.Operation = logical.ReadOperation + afterRoleDataResp, err = b.HandleRequest(context.Background(), roleReq) + if err != nil || (afterRoleDataResp != nil && afterRoleDataResp.IsError()) { + t.Fatalf("bad [%d/%v] read: err: %v resp: %#v", index, testCase.Field, err, resp) + } + + afterRoleData := afterRoleDataResp.Data + + for field, before := range beforeRoleData { + switch typed := before.(type) { + case *bool: + before = *typed + afterRoleData[field] = *(afterRoleData[field].(*bool)) + } + + if field != testCase.Field { + require.Equal(t, before, afterRoleData[field], fmt.Sprintf("bad [%d/%v] compare: non-modified field %v should not be changed", index, testCase.Field, field)) + } else { + require.Equal(t, before, testCase.Before, fmt.Sprintf("bad [%d] compare: modified field %v before should be correct", index, field)) + require.Equal(t, afterRoleData[field], testCase.Patched, fmt.Sprintf("bad [%d] compare: modified field %v after should be correct", index, field)) + } + } + } +} diff --git a/website/content/api-docs/secret/pki.mdx b/website/content/api-docs/secret/pki.mdx index ebf9d449de..e087d01be1 100644 --- a/website/content/api-docs/secret/pki.mdx +++ b/website/content/api-docs/secret/pki.mdx @@ -1676,13 +1676,18 @@ modes are set on this issuer. Note that it is not possible to change the certificate of this issuer; to do so, import a new issuer and a new `issuer_id` will be assigned. -| Method | Path | -| :----- | :------------------------ | -| `POST` | `/pki/issuer/:issuer_ref` | +| Method | Path | +| :------ | :------------------------ | +| `POST` | `/pki/issuer/:issuer_ref` | +| `PATCH` | `/pki/issuer/:issuer_ref` | ~> **Note** `POST`ing to this endpoint causes Vault to overwrite the previous contents of the issuer, using the provided request data (and any defaults - for elided parameters). It does not update only the provided fields. + for elided parameters). It does not update only the provided fields.

+ Since Vault 1.11.0, Vault supports the PATCH operation to this endpoint, + using the [JSON patch format](https://datatracker.ietf.org/doc/html/rfc6902) + supported by KVv2, allowing update of specific fields. Note that + `vault write` uses `POST`. #### Parameters @@ -2018,14 +2023,19 @@ multiple roles nearly any issuing policy can be accommodated. `server_flag`, requests a certificate that is not allowed by the CN policy in the role, the request is denied. -| Method | Path | -| :----- | :----------------- | -| `POST` | `/pki/roles/:name` | +| Method | Path | +| :------ | :----------------- | +| `POST` | `/pki/roles/:name` | +| `PATCH` | `/pki/roles/:name` | ~> **Note** `POST`ing to this endpoint when the role already exists causes Vault to overwrite the contents of the role, using the provided request data (and any defaults for elided parameters). It does not update only - the provided fields. + the provided fields.

+ Since Vault 1.11.0, Vault supports the PATCH operation to this endpoint, + using the [JSON patch format](https://datatracker.ietf.org/doc/html/rfc6902) + supported by KVv2, allowing update of specific fields. Note that + `vault write` uses `POST`. #### Parameters