From 3903eab1414e08744c946f332652382f2a04a762 Mon Sep 17 00:00:00 2001 From: Steve Clark Date: Fri, 8 Apr 2022 12:51:59 -0400 Subject: [PATCH 1/7] Add support root issuer generation --- builtin/logical/pki/backend.go | 1 + builtin/logical/pki/backend_test.go | 95 +++++++++++++ builtin/logical/pki/ca_util.go | 155 +++++++++++++++------ builtin/logical/pki/config_util.go | 19 +++ builtin/logical/pki/fields.go | 12 ++ builtin/logical/pki/managed_key_util.go | 34 ++++- builtin/logical/pki/path_manage_issuers.go | 26 +++- builtin/logical/pki/path_root.go | 21 +-- builtin/logical/pki/storage.go | 53 ++++++- builtin/logical/pki/storage_migrations.go | 33 +---- builtin/logical/pki/storage_test.go | 6 +- builtin/logical/pki/util.go | 31 ++++- 12 files changed, 396 insertions(+), 90 deletions(-) diff --git a/builtin/logical/pki/backend.go b/builtin/logical/pki/backend.go index 6ab418f03bbf..98257e0c51eb 100644 --- a/builtin/logical/pki/backend.go +++ b/builtin/logical/pki/backend.go @@ -116,6 +116,7 @@ func Backend(conf *logical.BackendConfig) *backend { pathIssuerSignIntermediate(&b), pathIssuerSignSelfIssued(&b), pathIssuerSignVerbatim(&b), + pathIssuerGenerateRoot(&b), pathConfigIssuers(&b), // Fetch APIs have been lowered to favor the newer issuer API endpoints diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 53abd6128bcb..8120936d4af8 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -4613,6 +4613,101 @@ func TestBackend_Roles_KeySizeRegression(t *testing.T) { t.Log(fmt.Sprintf("Key size regression expanded matrix test scenarios: %d", tested)) } +func TestRootWithExistingKey(t *testing.T) { + coreConfig := &vault.CoreConfig{ + LogicalBackends: map[string]logical.Factory{ + "pki": Factory, + }, + } + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + + client := cluster.Cores[0].Client + var err error + + mountPKIEndpoint(t, client, "pki-root") + + // Fail requests if type is existing, and we specify the key_type param + ctx := context.Background() + _, err = client.Logical().WriteWithContext(ctx, "pki-root/root/generate/existing", map[string]interface{}{ + "common_name": "root myvault.com", + "key_type": "rsa", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "key_type nor key_bits arguments can be set in this mode") + + // Fail requests if type is existing, and we specify the key_bits param + _, err = client.Logical().WriteWithContext(ctx, "pki-root/root/generate/existing", map[string]interface{}{ + "common_name": "root myvault.com", + "key_bits": "2048", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "key_type nor key_bits arguments can be set in this mode") + + // Fail if the specified key does not exist. + _, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/existing", map[string]interface{}{ + "common_name": "root myvault.com", + "id": "my-issuer1", + "key_id": "my-key1", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "unable to find PKI key for reference: my-key1") + + // Create the first CA + resp, err := client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/internal", map[string]interface{}{ + "common_name": "root myvault.com", + "key_type": "rsa", + "id": "my-issuer1", + }) + require.NoError(t, err) + require.NotNil(t, resp.Data["certificate"]) + myIssuerId1 := resp.Data["id"] + myKeyId1 := resp.Data["key_id"] + require.NotEmpty(t, myIssuerId1) + require.NotEmpty(t, myKeyId1) + + // Create the second CA + resp, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/internal", map[string]interface{}{ + "common_name": "root myvault.com", + "key_type": "rsa", + "id": "my-issuer2", + }) + require.NoError(t, err) + require.NotNil(t, resp.Data["certificate"]) + myIssuerId2 := resp.Data["id"] + myKeyId2 := resp.Data["key_id"] + require.NotEmpty(t, myIssuerId2) + require.NotEmpty(t, myKeyId2) + + // Create a third CA re-using key from CA 1 + resp, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/existing", map[string]interface{}{ + "common_name": "root myvault.com", + "id": "my-issuer3", + "key_id": myKeyId1, + }) + require.NoError(t, err) + require.NotNil(t, resp.Data["certificate"]) + myIssuerId3 := resp.Data["id"] + myKeyId3 := resp.Data["key_id"] + require.NotEmpty(t, myIssuerId3) + require.NotEmpty(t, myKeyId3) + + require.NotEqual(t, myIssuerId1, myIssuerId2) + require.NotEqual(t, myIssuerId1, myIssuerId3) + require.NotEqual(t, myKeyId1, myKeyId2) + require.Equal(t, myKeyId1, myKeyId3) + + resp, err = client.Logical().ListWithContext(ctx, "pki-root/issuers") + require.NoError(t, err) + require.Equal(t, 3, len(resp.Data["keys"].([]interface{}))) + require.Contains(t, resp.Data["keys"], myIssuerId1) + require.Contains(t, resp.Data["keys"], myIssuerId2) + require.Contains(t, resp.Data["keys"], myIssuerId3) +} + var ( initTest sync.Once rsaCAKey string diff --git a/builtin/logical/pki/ca_util.go b/builtin/logical/pki/ca_util.go index e7a9e8700488..b9d6616f2736 100644 --- a/builtin/logical/pki/ca_util.go +++ b/builtin/logical/pki/ca_util.go @@ -2,8 +2,10 @@ package pki import ( "context" + "crypto" "crypto/ecdsa" "crypto/rsa" + "errors" "fmt" "time" @@ -14,18 +16,17 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -func (b *backend) getGenerationParams(ctx context.Context, - data *framework.FieldData, mountPoint string, -) (exported bool, format string, role *roleEntry, errorResp *logical.Response) { +func (b *backend) getGenerationParams(ctx context.Context, data *framework.FieldData, mountPoint string) (exported bool, format string, role *roleEntry, errorResp *logical.Response) { exportedStr := data.Get("exported").(string) switch exportedStr { case "exported": exported = true case "internal": + case "existing": case "kms": default: errorResp = logical.ErrorResponse( - `the "exported" path parameter must be "internal", "exported" or "kms"`) + `the "exported" path parameter must be "internal", "existing", exported" or "kms"`) return } @@ -36,46 +37,10 @@ func (b *backend) getGenerationParams(ctx context.Context, return } - keyType := data.Get("key_type").(string) - keyBits := data.Get("key_bits").(int) - if exportedStr == "kms" { - _, okKeyType := data.Raw["key_type"] - _, okKeyBits := data.Raw["key_bits"] - - if okKeyType || okKeyBits { - errorResp = logical.ErrorResponse( - `invalid parameter for the kms path parameter, key_type nor key_bits arguments can be set in this mode`) - return - } - - keyId, err := getManagedKeyId(data) - if err != nil { - errorResp = logical.ErrorResponse("unable to determine managed key id") - return - } - // Determine key type and key bits from the managed public key - err = withManagedPKIKey(ctx, b, keyId, mountPoint, func(ctx context.Context, key logical.ManagedSigningKey) error { - pubKey, err := key.GetPublicKey(ctx) - if err != nil { - return err - } - switch pubKey.(type) { - case *rsa.PublicKey: - keyType = "rsa" - keyBits = pubKey.(*rsa.PublicKey).Size() * 8 - case *ecdsa.PublicKey: - keyType = "ec" - case *ed25519.PublicKey: - keyType = "ed25519" - default: - return fmt.Errorf("unsupported public key: %#v", pubKey) - } - return nil - }) - if err != nil { - errorResp = logical.ErrorResponse("failed to lookup public key from managed key: %s", err.Error()) - return - } + keyType, keyBits, err := getKeyTypeAndBitsForRole(ctx, b, data, mountPoint) + if err != nil { + errorResp = logical.ErrorResponse(err.Error()) + return } role = &roleEntry{ @@ -101,10 +66,110 @@ func (b *backend) getGenerationParams(ctx context.Context, } *role.AllowWildcardCertificates = true - var err error if role.KeyBits, role.SignatureBits, err = certutil.ValidateDefaultOrValueKeyTypeSignatureLength(role.KeyType, role.KeyBits, role.SignatureBits); err != nil { errorResp = logical.ErrorResponse(err.Error()) } return } + +func getKeyTypeAndBitsForRole(ctx context.Context, b *backend, data *framework.FieldData, mountPoint string) (string, int, error) { + exportedStr := data.Get("exported").(string) + var keyType string + var keyBits int + + switch exportedStr { + case "internal": + fallthrough + case "exported": + keyType = data.Get("key_type").(string) + keyBits = data.Get("key_bits").(int) + return keyType, keyBits, nil + } + + // existing and kms types don't support providing the key_type and key_bits args. + _, okKeyType := data.Raw["key_type"] + _, okKeyBits := data.Raw["key_bits"] + + if okKeyType || okKeyBits { + return "", 0, errors.New("invalid parameter for the kms/existing path parameter, key_type nor key_bits arguments can be set in this mode") + } + + var pubKey crypto.PublicKey + if kmsRequestedFromFieldData(data) { + pubKeyManagedKey, err := getManagedKeyPublicKey(ctx, b, data, mountPoint) + if err != nil { + return "", 0, errors.New("failed to lookup public key from managed key: " + err.Error()) + } + pubKey = pubKeyManagedKey + } + + if existingKeyRequestedFromFieldData(data) { + existingPubKey, err := getExistingPublicKey(ctx, b.storage, data) + if err != nil { + return "", 0, errors.New("failed to lookup public key from existing key: " + err.Error()) + } + pubKey = existingPubKey + } + + return getKeyTypeAndBitsFromPublicKeyForRole(pubKey) +} + +func getExistingPublicKey(ctx context.Context, s logical.Storage, data *framework.FieldData) (crypto.PublicKey, error) { + keyRef, err := getExistingKeyRef(data) + if err != nil { + return nil, err + } + id, err := resolveKeyReference(ctx, s, keyRef) + if err != nil { + return nil, err + } + key, err := fetchKeyById(ctx, s, id) + if err != nil { + return nil, err + } + signer, err := key.GetSigner() + if err != nil { + return nil, err + } + return signer.Public(), nil +} + +func getKeyTypeAndBitsFromPublicKeyForRole(pubKey crypto.PublicKey) (string, int, error) { + var keyType string + var keyBits int + + switch pubKey.(type) { + case *rsa.PublicKey: + keyType = "rsa" + keyBits = certutil.GetPublicKeySize(pubKey) + case *ecdsa.PublicKey: + keyType = "ec" + case *ed25519.PublicKey: + keyType = "ed25519" + default: + return "", 0, fmt.Errorf("unsupported public key: %#v", pubKey) + } + return keyType, keyBits, nil +} + +func getManagedKeyPublicKey(ctx context.Context, b *backend, data *framework.FieldData, mountPoint string) (crypto.PublicKey, error) { + keyId, err := getManagedKeyId(data) + if err != nil { + return nil, errors.New("unable to determine managed key id") + } + // Determine key type and key bits from the managed public key + var pubKey crypto.PublicKey + err = withManagedPKIKey(ctx, b, keyId, mountPoint, func(ctx context.Context, key logical.ManagedSigningKey) error { + pubKey, err = key.GetPublicKey(ctx) + if err != nil { + return err + } + + return nil + }) + if err != nil { + return nil, errors.New("failed to lookup public key from managed key: " + err.Error()) + } + return pubKey, nil +} diff --git a/builtin/logical/pki/config_util.go b/builtin/logical/pki/config_util.go index 2ba36fe9d0fd..830590fbd8b0 100644 --- a/builtin/logical/pki/config_util.go +++ b/builtin/logical/pki/config_util.go @@ -2,10 +2,29 @@ package pki import ( "context" + "strings" "github.com/hashicorp/vault/sdk/logical" ) +func isKeyDefaultSet(ctx context.Context, s logical.Storage) (bool, error) { + config, err := getKeysConfig(ctx, s) + if err != nil { + return false, err + } + + return strings.TrimSpace(config.DefaultKeyId.String()) != "", nil +} + +func isIssuerDefaultSet(ctx context.Context, s logical.Storage) (bool, error) { + config, err := getIssuersConfig(ctx, s) + if err != nil { + return false, err + } + + return strings.TrimSpace(config.DefaultIssuerId.String()) != "", nil +} + func updateDefaultKeyId(ctx context.Context, s logical.Storage, id keyId) error { config, err := getKeysConfig(ctx, s) if err != nil { diff --git a/builtin/logical/pki/fields.go b/builtin/logical/pki/fields.go index 4593d6d9a7cc..fa94751451b7 100644 --- a/builtin/logical/pki/fields.go +++ b/builtin/logical/pki/fields.go @@ -314,6 +314,18 @@ SHA-2-512. Defaults to 0 to automatically detect based on key length Value: "rsa", }, } + + fields["key_id"] = &framework.FieldSchema{ + Type: framework.TypeString, + Description: `Reference to a existing key; either "default" +for the configured default key, an identifier or the name assigned +to the key. Note this is only used for the existing generation type.`, + } + + fields["id"] = &framework.FieldSchema{ + Type: framework.TypeString, + Description: `Assign a name to the generated issuer.`, + } return fields } diff --git a/builtin/logical/pki/managed_key_util.go b/builtin/logical/pki/managed_key_util.go index 4c16d0d2a410..acb11e624065 100644 --- a/builtin/logical/pki/managed_key_util.go +++ b/builtin/logical/pki/managed_key_util.go @@ -4,6 +4,7 @@ package pki import ( "context" + "encoding/pem" "errors" "io" @@ -13,10 +14,17 @@ import ( var errEntOnly = errors.New("managed keys are supported within enterprise edition only") -func generateCABundle(_ context.Context, _ *backend, input *inputBundle, data *certutil.CreationBundle, randomSource io.Reader) (*certutil.ParsedCertBundle, error) { +func generateCABundle(ctx context.Context, _ *backend, input *inputBundle, data *certutil.CreationBundle, randomSource io.Reader) (*certutil.ParsedCertBundle, error) { if kmsRequested(input) { return nil, errEntOnly } + if existingKeyRequested(input) { + keyRef, err := getExistingKeyRef(input.apiData) + if err != nil { + return nil, err + } + return certutil.CreateCertificateWithKeyGenerator(data, randomSource, existingGeneratePrivateKey(ctx, input.req.Storage, keyRef)) + } return certutil.CreateCertificateWithRandomSource(data, randomSource) } @@ -35,3 +43,27 @@ func parseCABundle(_ context.Context, _ *backend, _ *logical.Request, bundle *ce func withManagedPKIKey(_ context.Context, _ *backend, _ managedKeyId, _ string, _ logical.ManagedSigningKeyConsumer) error { return errEntOnly } + +func existingGeneratePrivateKey(ctx context.Context, s logical.Storage, keyRef string) certutil.KeyGenerator { + return func(keyType string, keyBits int, container certutil.ParsedPrivateKeyContainer, _ io.Reader) error { + keyId, err := resolveKeyReference(ctx, s, keyRef) + if err != nil { + return err + } + key, err := fetchKeyById(ctx, s, keyId) + if err != nil { + return err + } + signer, err := key.GetSigner() + if err != nil { + return err + } + privateKeyType := certutil.GetPrivateKeyTypeFromSigner(signer) + if privateKeyType == certutil.UnknownPrivateKey { + return errors.New("unknown private key type loaded from key id: " + keyId.String()) + } + blk, _ := pem.Decode([]byte(key.PrivateKey)) + container.SetParsedPrivateKey(signer, privateKeyType, blk.Bytes) + return nil + } +} diff --git a/builtin/logical/pki/path_manage_issuers.go b/builtin/logical/pki/path_manage_issuers.go index 907f8aa8da4b..b9ac8bc5740b 100644 --- a/builtin/logical/pki/path_manage_issuers.go +++ b/builtin/logical/pki/path_manage_issuers.go @@ -11,6 +11,30 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) +func pathIssuerGenerateRoot(b *backend) *framework.Path { + ret := &framework.Path{ + Pattern: "issuers/generate/root/" + framework.GenericNameRegex("exported"), + + Operations: map[logical.Operation]framework.OperationHandler{ + logical.UpdateOperation: &framework.PathOperation{ + Callback: b.pathCAGenerateRoot, + // Read more about why these flags are set in backend.go + ForwardPerformanceStandby: true, + ForwardPerformanceSecondary: true, + }, + }, + + HelpSynopsis: pathGenerateRootHelpSyn, + HelpDescription: pathGenerateRootHelpDesc, + } + + ret.Fields = addCACommonFields(map[string]*framework.FieldSchema{}) + ret.Fields = addCAKeyGenerationFields(ret.Fields) + ret.Fields = addCAIssueFields(ret.Fields) + + return ret +} + func pathImportIssuer(b *backend) *framework.Path { return &framework.Path{ Pattern: "issuers/import/(cert|bundle)", @@ -88,7 +112,7 @@ func (b *backend) pathImportIssuers(ctx context.Context, req *logical.Request, d } for _, certPem := range issuers { - cert, existing, err := importIssuer(ctx, req.Storage, certPem) + cert, existing, err := importIssuer(ctx, req.Storage, certPem, "") if err != nil { return logical.ErrorResponse(err.Error()), nil } diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 75d1dfda002f..5f84c1152358 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -97,6 +97,12 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, role.MaxPathLength = &maxPathLength } + issuerName := "" + issuerNameIface, ok := data.GetOk("id") + if ok { + issuerName = issuerNameIface.(string) + } + input := &inputBundle{ req: req, apiData: data, @@ -163,14 +169,12 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, } // Store it as the CA bundle - entry, err = logical.StorageEntryJSON("config/ca_bundle", cb) - if err != nil { - return nil, err - } - err = req.Storage.Put(ctx, entry) + myIssuer, myKey, err := writeCaBundle(ctx, req.Storage, cb, issuerName) if err != nil { return nil, err } + resp.Data["id"] = myIssuer.ID + resp.Data["key_id"] = myKey.ID // Also store it as just the certificate identified by serial number, so it // can be revoked @@ -184,9 +188,10 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, // For ease of later use, also store just the certificate at a known // location - entry.Key = "ca" - entry.Value = parsedBundle.CertificateBytes - err = req.Storage.Put(ctx, entry) + err = req.Storage.Put(ctx, &logical.StorageEntry{ + Key: "ca", + Value: parsedBundle.CertificateBytes, + }) if err != nil { return nil, err } diff --git a/builtin/logical/pki/storage.go b/builtin/logical/pki/storage.go index 4b6d5b23930e..8799caf1112e 100644 --- a/builtin/logical/pki/storage.go +++ b/builtin/logical/pki/storage.go @@ -306,7 +306,7 @@ func deleteIssuer(ctx context.Context, s logical.Storage, id issuerId) error { return s.Delete(ctx, issuerPrefix+id.String()) } -func importIssuer(ctx context.Context, s logical.Storage, certValue string) (*issuer, bool, error) { +func importIssuer(ctx context.Context, s logical.Storage, certValue string, issuerName string) (*issuer, bool, error) { // importIssuers imports the specified PEM-format certificate (from // certValue) into the new PKI storage format. The first return field is a // reference to the new issuer; the second is whether or not the issuer @@ -349,6 +349,7 @@ func importIssuer(ctx context.Context, s logical.Storage, certValue string) (*is // storage. var result issuer result.ID = genIssuerId() + result.Name = issuerName result.Certificate = certValue result.CAChain = []string{certValue} @@ -518,6 +519,56 @@ func fetchCertBundleByIssuerId(ctx context.Context, s logical.Storage, id issuer return &bundle, nil } +func writeCaBundle(ctx context.Context, s logical.Storage, caBundle *certutil.CertBundle, issuerName string) (*issuer, *key, error) { + allKeyIds, err := listKeys(ctx, s) + if err != nil { + return nil, nil, err + } + + allIssuerIds, err := listIssuers(ctx, s) + if err != nil { + return nil, nil, err + } + + myKey, _, err := importKey(ctx, s, caBundle.PrivateKey) + if err != nil { + return nil, nil, err + } + + myIssuer, _, err := importIssuer(ctx, s, caBundle.Certificate, issuerName) + if err != nil { + return nil, nil, err + } + + for _, cert := range caBundle.CAChain { + if _, _, err = importIssuer(ctx, s, cert, ""); err != nil { + return nil, nil, err + } + } + + keyDefaultSet, err := isKeyDefaultSet(ctx, s) + if err != nil { + return nil, nil, err + } + if len(allKeyIds) == 0 || !keyDefaultSet { + if err = updateDefaultKeyId(ctx, s, myKey.ID); err != nil { + return nil, nil, err + } + } + + issuerDefaultSet, err := isIssuerDefaultSet(ctx, s) + if err != nil { + return nil, nil, err + } + if len(allIssuerIds) == 0 || !issuerDefaultSet { + if err = updateDefaultIssuerId(ctx, s, myIssuer.ID); err != nil { + return nil, nil, err + } + } + + return myIssuer, myKey, nil +} + func genIssuerId() issuerId { return issuerId(genUuid()) } diff --git a/builtin/logical/pki/storage_migrations.go b/builtin/logical/pki/storage_migrations.go index 73882f57461f..c1e05263a77e 100644 --- a/builtin/logical/pki/storage_migrations.go +++ b/builtin/logical/pki/storage_migrations.go @@ -50,10 +50,12 @@ func migrateStorage(ctx context.Context, req *logical.InitializationRequest, log logger.Warn("performing PKI migration to new keys/issuers layout") - err = migrateToIssuers(ctx, s, legacyBundle) + anIssuer, aKey, err := writeCaBundle(ctx, s, legacyBundle, "") if err != nil { return err } + logger.Info("Migration generated the following ids and set them as defaults", + "issuer id", anIssuer.ID, "key id", aKey.ID) err = setLegacyBundleMigrationLog(ctx, s, &legacyBundleMigration{ hash: hash, @@ -82,35 +84,6 @@ func computeHashOfLegacyBundle(bundle *certutil.CertBundle) (string, error) { return hex.EncodeToString(hasher.Sum(nil)), nil } -func migrateToIssuers(ctx context.Context, s logical.Storage, bundle *certutil.CertBundle) error { - defaultKey, _, err := importKey(ctx, s, bundle.PrivateKey) - if err != nil { - return err - } - - defaultIssuer, _, err := importIssuer(ctx, s, bundle.Certificate) - if err != nil { - return err - } - - for _, cert := range bundle.CAChain { - if _, _, err = importIssuer(ctx, s, cert); err != nil { - return err - } - } - - if err = updateDefaultKeyId(ctx, s, defaultKey.ID); err != nil { - return err - } - - if err = updateDefaultIssuerId(ctx, s, defaultIssuer.ID); err != nil { - return err - } - - // FIXME: Call function that will recompute the CAChain on issuers here. - return nil -} - type legacyBundleMigration struct { hash string created time.Time diff --git a/builtin/logical/pki/storage_test.go b/builtin/logical/pki/storage_test.go index f8ec0e8978a4..b96118a44827 100644 --- a/builtin/logical/pki/storage_test.go +++ b/builtin/logical/pki/storage_test.go @@ -113,13 +113,13 @@ func Test_KeysIssuerImport(t *testing.T) { require.Equal(t, key1.PrivateKey, key1_ref1.PrivateKey) require.Equal(t, key1_ref1.ID, key1_ref2.ID) - issuer1_ref1, existing, err := importIssuer(ctx, s, issuer1.Certificate) + issuer1_ref1, existing, err := importIssuer(ctx, s, issuer1.Certificate, "") require.NoError(t, err) require.False(t, existing) require.Equal(t, issuer1.Certificate, issuer1_ref1.Certificate) require.Equal(t, key1_ref1.ID, issuer1_ref1.KeyID) - issuer1_ref2, existing, err := importIssuer(ctx, s, issuer1.Certificate) + issuer1_ref2, existing, err := importIssuer(ctx, s, issuer1.Certificate, "") require.NoError(t, err) require.True(t, existing) require.Equal(t, issuer1.Certificate, issuer1_ref1.Certificate) @@ -132,7 +132,7 @@ func Test_KeysIssuerImport(t *testing.T) { err = writeKey(ctx, s, key2) require.NoError(t, err) - issuer2_ref, existing, err := importIssuer(ctx, s, issuer2.Certificate) + issuer2_ref, existing, err := importIssuer(ctx, s, issuer2.Certificate, "") require.NoError(t, err) require.True(t, existing) require.Equal(t, issuer2.Certificate, issuer2_ref.Certificate) diff --git a/builtin/logical/pki/util.go b/builtin/logical/pki/util.go index 3ed0f14bc673..f6737bdbd149 100644 --- a/builtin/logical/pki/util.go +++ b/builtin/logical/pki/util.go @@ -23,13 +23,29 @@ func denormalizeSerial(serial string) string { } func kmsRequested(input *inputBundle) bool { - exportedStr, ok := input.apiData.GetOk("exported") + return kmsRequestedFromFieldData(input.apiData) +} + +func kmsRequestedFromFieldData(data *framework.FieldData) bool { + exportedStr, ok := data.GetOk("exported") if !ok { return false } return exportedStr.(string) == "kms" } +func existingKeyRequested(input *inputBundle) bool { + return existingKeyRequestedFromFieldData(input.apiData) +} + +func existingKeyRequestedFromFieldData(data *framework.FieldData) bool { + exportedStr, ok := data.GetOk("exported") + if !ok { + return false + } + return exportedStr.(string) == "existing" +} + type managedKeyId interface { String() string } @@ -63,6 +79,19 @@ func getManagedKeyId(data *framework.FieldData) (managedKeyId, error) { return keyId, nil } +func getExistingKeyRef(data *framework.FieldData) (string, error) { + keyRef, ok := data.GetOk("key_id") + if !ok { + return "", errutil.UserError{Err: fmt.Sprintf("missing argument key_id for existing type")} + } + trimmedKeyRef := strings.TrimSpace(keyRef.(string)) + if len(trimmedKeyRef) == 0 { + return "", errutil.UserError{Err: fmt.Sprintf("missing argument key_id for existing type")} + } + + return trimmedKeyRef, nil +} + func getManagedKeyNameOrUUID(data *framework.FieldData) (name string, UUID string, err error) { getApiData := func(argName string) (string, error) { arg, ok := data.GetOk(argName) From 5f2179f4c7699f391f1fae0e4925f8afee8a451c Mon Sep 17 00:00:00 2001 From: Steve Clark Date: Fri, 8 Apr 2022 15:38:07 -0400 Subject: [PATCH 2/7] Add support for issuer generate intermediate end-point --- builtin/logical/pki/backend.go | 1 + builtin/logical/pki/backend_test.go | 76 ++++++++++++++++++++++ builtin/logical/pki/fields.go | 5 -- builtin/logical/pki/managed_key_util.go | 9 ++- builtin/logical/pki/path_intermediate.go | 38 +---------- builtin/logical/pki/path_manage_issuers.go | 45 ++++++++++++- builtin/logical/pki/path_root.go | 22 +------ 7 files changed, 133 insertions(+), 63 deletions(-) diff --git a/builtin/logical/pki/backend.go b/builtin/logical/pki/backend.go index 98257e0c51eb..da60099b358a 100644 --- a/builtin/logical/pki/backend.go +++ b/builtin/logical/pki/backend.go @@ -117,6 +117,7 @@ func Backend(conf *logical.BackendConfig) *backend { pathIssuerSignSelfIssued(&b), pathIssuerSignVerbatim(&b), pathIssuerGenerateRoot(&b), + pathIssuerGenerateIntermediate(&b), pathConfigIssuers(&b), // Fetch APIs have been lowered to favor the newer issuer API endpoints diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 8120936d4af8..e5f59d41038c 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -4708,6 +4708,82 @@ func TestRootWithExistingKey(t *testing.T) { require.Contains(t, resp.Data["keys"], myIssuerId3) } +func TestIntermediateWithExistingKey(t *testing.T) { + coreConfig := &vault.CoreConfig{ + LogicalBackends: map[string]logical.Factory{ + "pki": Factory, + }, + } + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + + client := cluster.Cores[0].Client + var err error + + mountPKIEndpoint(t, client, "pki-root") + + // Fail requests if type is existing, and we specify the key_type param + ctx := context.Background() + _, err = client.Logical().WriteWithContext(ctx, "pki-root/intermediate/generate/existing", map[string]interface{}{ + "common_name": "root myvault.com", + "key_type": "rsa", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "key_type nor key_bits arguments can be set in this mode") + + // Fail requests if type is existing, and we specify the key_bits param + _, err = client.Logical().WriteWithContext(ctx, "pki-root/intermediate/generate/existing", map[string]interface{}{ + "common_name": "root myvault.com", + "key_bits": "2048", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "key_type nor key_bits arguments can be set in this mode") + + // Fail if the specified key does not exist. + _, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/intermediate/existing", map[string]interface{}{ + "common_name": "root myvault.com", + "key_id": "my-key1", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "unable to find PKI key for reference: my-key1") + + // Create the first intermediate CA + resp, err := client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/intermediate/internal", map[string]interface{}{ + "common_name": "root myvault.com", + "key_type": "rsa", + }) + require.NoError(t, err) + // csr1 := resp.Data["csr"] + myKeyId1 := resp.Data["key_id"] + require.NotEmpty(t, myKeyId1) + + // Create the second intermediate CA + resp, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/intermediate/internal", map[string]interface{}{ + "common_name": "root myvault.com", + "key_type": "rsa", + }) + require.NoError(t, err) + // csr2 := resp.Data["csr"] + myKeyId2 := resp.Data["key_id"] + require.NotEmpty(t, myKeyId2) + + // Create a third intermediate CA re-using key from intermediate CA 1 + resp, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/intermediate/existing", map[string]interface{}{ + "common_name": "root myvault.com", + "key_id": myKeyId1, + }) + require.NoError(t, err) + // csr3 := resp.Data["csr"] + myKeyId3 := resp.Data["key_id"] + require.NotEmpty(t, myKeyId3) + + require.NotEqual(t, myKeyId1, myKeyId2) + require.Equal(t, myKeyId1, myKeyId3) +} + var ( initTest sync.Once rsaCAKey string diff --git a/builtin/logical/pki/fields.go b/builtin/logical/pki/fields.go index fa94751451b7..5e9d29f8cfb0 100644 --- a/builtin/logical/pki/fields.go +++ b/builtin/logical/pki/fields.go @@ -321,11 +321,6 @@ SHA-2-512. Defaults to 0 to automatically detect based on key length for the configured default key, an identifier or the name assigned to the key. Note this is only used for the existing generation type.`, } - - fields["id"] = &framework.FieldSchema{ - Type: framework.TypeString, - Description: `Assign a name to the generated issuer.`, - } return fields } diff --git a/builtin/logical/pki/managed_key_util.go b/builtin/logical/pki/managed_key_util.go index acb11e624065..caff650958d7 100644 --- a/builtin/logical/pki/managed_key_util.go +++ b/builtin/logical/pki/managed_key_util.go @@ -28,10 +28,17 @@ func generateCABundle(ctx context.Context, _ *backend, input *inputBundle, data return certutil.CreateCertificateWithRandomSource(data, randomSource) } -func generateCSRBundle(_ context.Context, _ *backend, input *inputBundle, data *certutil.CreationBundle, addBasicConstraints bool, randomSource io.Reader) (*certutil.ParsedCSRBundle, error) { +func generateCSRBundle(ctx context.Context, _ *backend, input *inputBundle, data *certutil.CreationBundle, addBasicConstraints bool, randomSource io.Reader) (*certutil.ParsedCSRBundle, error) { if kmsRequested(input) { return nil, errEntOnly } + if existingKeyRequested(input) { + keyRef, err := getExistingKeyRef(input.apiData) + if err != nil { + return nil, err + } + return certutil.CreateCSRWithKeyGenerator(data, addBasicConstraints, randomSource, existingGeneratePrivateKey(ctx, input.req.Storage, keyRef)) + } return certutil.CreateCSRWithRandomSource(data, addBasicConstraints, randomSource) } diff --git a/builtin/logical/pki/path_intermediate.go b/builtin/logical/pki/path_intermediate.go index 4e1e766eae35..6828c20b0a00 100644 --- a/builtin/logical/pki/path_intermediate.go +++ b/builtin/logical/pki/path_intermediate.go @@ -12,32 +12,7 @@ import ( ) func pathGenerateIntermediate(b *backend) *framework.Path { - ret := &framework.Path{ - Pattern: "intermediate/generate/" + framework.GenericNameRegex("exported"), - Operations: map[logical.Operation]framework.OperationHandler{ - logical.UpdateOperation: &framework.PathOperation{ - Callback: b.pathGenerateIntermediate, - // Read more about why these flags are set in backend.go - ForwardPerformanceStandby: true, - ForwardPerformanceSecondary: true, - }, - }, - - HelpSynopsis: pathGenerateIntermediateHelpSyn, - HelpDescription: pathGenerateIntermediateHelpDesc, - } - - ret.Fields = addCACommonFields(map[string]*framework.FieldSchema{}) - ret.Fields = addCAKeyGenerationFields(ret.Fields) - ret.Fields["add_basic_constraints"] = &framework.FieldSchema{ - Type: framework.TypeBool, - Description: `Whether to add a Basic Constraints -extension with CA: true. Only needed as a -workaround in some compatibility scenarios -with Active Directory Certificate Services.`, - } - - return ret + return commonGenerateIntermediate(b, "intermediate/generate/"+framework.GenericNameRegex("exported")) } func pathSetSignedIntermediate(b *backend) *framework.Path { @@ -135,18 +110,11 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req } } - cb := &certutil.CertBundle{} - cb.PrivateKey = csrb.PrivateKey - cb.PrivateKeyType = csrb.PrivateKeyType - - entry, err := logical.StorageEntryJSON("config/ca_bundle", cb) - if err != nil { - return nil, err - } - err = req.Storage.Put(ctx, entry) + myKey, _, err := importKey(ctx, req.Storage, csrb.PrivateKey) if err != nil { return nil, err } + resp.Data["key_id"] = myKey.ID return resp, nil } diff --git a/builtin/logical/pki/path_manage_issuers.go b/builtin/logical/pki/path_manage_issuers.go index b9ac8bc5740b..58f2a3ade714 100644 --- a/builtin/logical/pki/path_manage_issuers.go +++ b/builtin/logical/pki/path_manage_issuers.go @@ -12,8 +12,12 @@ import ( ) func pathIssuerGenerateRoot(b *backend) *framework.Path { + return commonGenerateRoot(b, "issuers/generate/root/"+framework.GenericNameRegex("exported")) +} + +func commonGenerateRoot(b *backend, pattern string) *framework.Path { ret := &framework.Path{ - Pattern: "issuers/generate/root/" + framework.GenericNameRegex("exported"), + Pattern: pattern, Operations: map[logical.Operation]framework.OperationHandler{ logical.UpdateOperation: &framework.PathOperation{ @@ -32,6 +36,45 @@ func pathIssuerGenerateRoot(b *backend) *framework.Path { ret.Fields = addCAKeyGenerationFields(ret.Fields) ret.Fields = addCAIssueFields(ret.Fields) + ret.Fields["id"] = &framework.FieldSchema{ + Type: framework.TypeString, + Description: `Assign a name to the generated issuer.`, + } + + return ret +} + +func pathIssuerGenerateIntermediate(b *backend) *framework.Path { + return commonGenerateIntermediate(b, + "issuers/generate/intermediate/"+framework.GenericNameRegex("exported")) +} + +func commonGenerateIntermediate(b *backend, pattern string) *framework.Path { + ret := &framework.Path{ + Pattern: pattern, + Operations: map[logical.Operation]framework.OperationHandler{ + logical.UpdateOperation: &framework.PathOperation{ + Callback: b.pathGenerateIntermediate, + // Read more about why these flags are set in backend.go + ForwardPerformanceStandby: true, + ForwardPerformanceSecondary: true, + }, + }, + + HelpSynopsis: pathGenerateIntermediateHelpSyn, + HelpDescription: pathGenerateIntermediateHelpDesc, + } + + ret.Fields = addCACommonFields(map[string]*framework.FieldSchema{}) + ret.Fields = addCAKeyGenerationFields(ret.Fields) + ret.Fields["add_basic_constraints"] = &framework.FieldSchema{ + Type: framework.TypeBool, + Description: `Whether to add a Basic Constraints +extension with CA: true. Only needed as a +workaround in some compatibility scenarios +with Active Directory Certificate Services.`, + } + return ret } diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 5f84c1152358..18ee65f96ed6 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -26,27 +26,7 @@ import ( ) func pathGenerateRoot(b *backend) *framework.Path { - ret := &framework.Path{ - Pattern: "root/generate/" + framework.GenericNameRegex("exported"), - - Operations: map[logical.Operation]framework.OperationHandler{ - logical.UpdateOperation: &framework.PathOperation{ - Callback: b.pathCAGenerateRoot, - // Read more about why these flags are set in backend.go - ForwardPerformanceStandby: true, - ForwardPerformanceSecondary: true, - }, - }, - - HelpSynopsis: pathGenerateRootHelpSyn, - HelpDescription: pathGenerateRootHelpDesc, - } - - ret.Fields = addCACommonFields(map[string]*framework.FieldSchema{}) - ret.Fields = addCAKeyGenerationFields(ret.Fields) - ret.Fields = addCAIssueFields(ret.Fields) - - return ret + return commonGenerateRoot(b, "root/generate/"+framework.GenericNameRegex("exported")) } func pathDeleteRoot(b *backend) *framework.Path { From e301ef456879a1df8655d9405b6aed7ef0b98cbe Mon Sep 17 00:00:00 2001 From: Steve Clark Date: Mon, 11 Apr 2022 10:49:10 -0400 Subject: [PATCH 3/7] Update issuer and key arguments to consistent values - Update all new API endpoints to use the new agreed upon argument names. - issuer_ref & key_ref to refer to existing - issuer_name & key_name for new definitions - Update returned values to always user issuer_id and key_id --- builtin/logical/pki/backend_test.go | 22 +++---- builtin/logical/pki/fields.go | 70 ++++++++++++++++++---- builtin/logical/pki/path_fetch_issuers.go | 38 +++++------- builtin/logical/pki/path_issue_sign.go | 8 +-- builtin/logical/pki/path_manage_issuers.go | 6 -- builtin/logical/pki/path_root.go | 6 +- builtin/logical/pki/path_sign_issuers.go | 8 +-- builtin/logical/pki/util.go | 6 +- 8 files changed, 99 insertions(+), 65 deletions(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index e5f59d41038c..823dcf427dcb 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -4650,8 +4650,8 @@ func TestRootWithExistingKey(t *testing.T) { // Fail if the specified key does not exist. _, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/existing", map[string]interface{}{ "common_name": "root myvault.com", - "id": "my-issuer1", - "key_id": "my-key1", + "issuer_name": "my-issuer1", + "key_ref": "my-key1", }) require.Error(t, err) require.Contains(t, err.Error(), "unable to find PKI key for reference: my-key1") @@ -4660,11 +4660,11 @@ func TestRootWithExistingKey(t *testing.T) { resp, err := client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/internal", map[string]interface{}{ "common_name": "root myvault.com", "key_type": "rsa", - "id": "my-issuer1", + "issuer_name": "my-issuer1", }) require.NoError(t, err) require.NotNil(t, resp.Data["certificate"]) - myIssuerId1 := resp.Data["id"] + myIssuerId1 := resp.Data["issuer_id"] myKeyId1 := resp.Data["key_id"] require.NotEmpty(t, myIssuerId1) require.NotEmpty(t, myKeyId1) @@ -4673,11 +4673,11 @@ func TestRootWithExistingKey(t *testing.T) { resp, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/internal", map[string]interface{}{ "common_name": "root myvault.com", "key_type": "rsa", - "id": "my-issuer2", + "issuer_name": "my-issuer2", }) require.NoError(t, err) require.NotNil(t, resp.Data["certificate"]) - myIssuerId2 := resp.Data["id"] + myIssuerId2 := resp.Data["issuer_id"] myKeyId2 := resp.Data["key_id"] require.NotEmpty(t, myIssuerId2) require.NotEmpty(t, myKeyId2) @@ -4685,12 +4685,12 @@ func TestRootWithExistingKey(t *testing.T) { // Create a third CA re-using key from CA 1 resp, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/existing", map[string]interface{}{ "common_name": "root myvault.com", - "id": "my-issuer3", - "key_id": myKeyId1, + "issuer_name": "my-issuer3", + "key_ref": myKeyId1, }) require.NoError(t, err) require.NotNil(t, resp.Data["certificate"]) - myIssuerId3 := resp.Data["id"] + myIssuerId3 := resp.Data["issuer_id"] myKeyId3 := resp.Data["key_id"] require.NotEmpty(t, myIssuerId3) require.NotEmpty(t, myKeyId3) @@ -4745,7 +4745,7 @@ func TestIntermediateWithExistingKey(t *testing.T) { // Fail if the specified key does not exist. _, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/intermediate/existing", map[string]interface{}{ "common_name": "root myvault.com", - "key_id": "my-key1", + "key_ref": "my-key1", }) require.Error(t, err) require.Contains(t, err.Error(), "unable to find PKI key for reference: my-key1") @@ -4773,7 +4773,7 @@ func TestIntermediateWithExistingKey(t *testing.T) { // Create a third intermediate CA re-using key from intermediate CA 1 resp, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/intermediate/existing", map[string]interface{}{ "common_name": "root myvault.com", - "key_id": myKeyId1, + "key_ref": myKeyId1, }) require.NoError(t, err) // csr3 := resp.Data["csr"] diff --git a/builtin/logical/pki/fields.go b/builtin/logical/pki/fields.go index 5e9d29f8cfb0..d9f23fe58a20 100644 --- a/builtin/logical/pki/fields.go +++ b/builtin/logical/pki/fields.go @@ -132,11 +132,7 @@ be larger than the role max TTL.`, The value format should be given in UTC format YYYY-MM-ddTHH:MM:SSZ`, } - fields["ref"] = &framework.FieldSchema{ - Type: framework.TypeString, - Description: `Reference to issuer; either "default" for the configured default issuer, an identifier of an issuer, or the name assigned to the issuer.`, - Default: "default", - } + fields = addIssuerRefField(fields) return fields } @@ -315,12 +311,8 @@ SHA-2-512. Defaults to 0 to automatically detect based on key length }, } - fields["key_id"] = &framework.FieldSchema{ - Type: framework.TypeString, - Description: `Reference to a existing key; either "default" -for the configured default key, an identifier or the name assigned -to the key. Note this is only used for the existing generation type.`, - } + fields = addKeyRefNameFields(fields) + return fields } @@ -341,5 +333,61 @@ func addCAIssueFields(fields map[string]*framework.FieldSchema) map[string]*fram }, } + fields = addIssuerNameField(fields) + + return fields +} + +func addKeyRefNameFields(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { + fields = addKeyNameField(fields) + fields = addKeyRefField(fields) + return fields +} + +func addIssuerRefNameFields(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { + fields = addIssuerNameField(fields) + fields = addIssuerRefField(fields) + return fields +} + +func addIssuerRefField(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { + fields["issuer_ref"] = &framework.FieldSchema{ + Type: framework.TypeString, + Description: `Reference to a existing issuer; either "default" +for the configured default issuer, an identifier or the name assigned +to the issuer.`, + Default: "default", + } + return fields +} + +func addIssuerNameField(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { + fields["issuer_name"] = &framework.FieldSchema{ + Type: framework.TypeString, + Description: `Provide a name to the generated issuer, the name +must be unique across all issuers and not be the reserved value 'default'`, + } + return fields +} + +func addKeyNameField(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { + fields["key_name"] = &framework.FieldSchema{ + Type: framework.TypeString, + Description: `Provide a name for the key that will be generated, +the name must be unique across all keys and not be the reserved value +'default'`, + } + + return fields +} + +func addKeyRefField(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { + fields["key_ref"] = &framework.FieldSchema{ + Type: framework.TypeString, + Description: `Reference to a existing key; either "default" +for the configured default key, an identifier or the name assigned +to the key.`, + Default: "default", + } return fields } diff --git a/builtin/logical/pki/path_fetch_issuers.go b/builtin/logical/pki/path_fetch_issuers.go index 94f34edefa85..a85eb81da457 100644 --- a/builtin/logical/pki/path_fetch_issuers.go +++ b/builtin/logical/pki/path_fetch_issuers.go @@ -10,7 +10,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -var nameMatcher = regexp.MustCompile("^" + framework.GenericNameRegex("ref") + "$") +var nameMatcher = regexp.MustCompile("^" + framework.GenericNameRegex("issuer_ref") + "$") func pathListIssuers(b *backend) *framework.Path { return &framework.Path{ @@ -45,7 +45,7 @@ func (b *backend) pathListIssuersHandler(ctx context.Context, req *logical.Reque responseKeys = append(responseKeys, string(identifier)) responseInfo[string(identifier)] = map[string]interface{}{ - "name": issuer.Name, + "issuer_name": issuer.Name, } } @@ -61,25 +61,17 @@ their identifier and their name (if set). ) func pathGetIssuer(b *backend) *framework.Path { - pattern := "issuer/" + framework.GenericNameRegex("ref") + "(/der|/pem)?" + pattern := "issuer/" + framework.GenericNameRegex("issuer_ref") + "(/der|/pem)?" return buildPathGetIssuer(b, pattern) } func buildPathGetIssuer(b *backend, pattern string) *framework.Path { + fields := map[string]*framework.FieldSchema{} + fields = addIssuerRefNameFields(fields) return &framework.Path{ // Returns a JSON entry. Pattern: pattern, - Fields: map[string]*framework.FieldSchema{ - "ref": { - Type: framework.TypeString, - Description: `Reference to issuer; either "default" for the configured default issuer, an identifier of an issuer, or the name assigned to the issuer.`, - Default: "default", - }, - "name": { - Type: framework.TypeString, - Description: `Human-readable name for this issuer.`, - }, - }, + Fields: fields, Callbacks: map[logical.Operation]framework.OperationFunc{ logical.ReadOperation: b.pathGetIssuer, @@ -98,7 +90,7 @@ func (b *backend) pathGetIssuer(ctx context.Context, req *logical.Request, data return b.pathGetRawIssuer(ctx, req, data) } - issuerName := data.Get("ref").(string) + issuerName := data.Get("issuer_ref").(string) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } @@ -118,8 +110,8 @@ func (b *backend) pathGetIssuer(ctx context.Context, req *logical.Request, data return &logical.Response{ Data: map[string]interface{}{ - "id": issuer.ID, - "name": issuer.Name, + "issuer_id": issuer.ID, + "issuer_name": issuer.Name, "key_id": issuer.KeyID, "certificate": issuer.Certificate, }, @@ -127,12 +119,12 @@ func (b *backend) pathGetIssuer(ctx context.Context, req *logical.Request, data } func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - issuerName := data.Get("ref").(string) + issuerName := data.Get("issuer_ref").(string) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } - newName := data.Get("name").(string) + newName := data.Get("issuer_name").(string) if len(newName) > 0 && !nameMatcher.MatchString(newName) { return logical.ErrorResponse("new issuer name outside of valid character limits"), nil } @@ -161,8 +153,8 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da return &logical.Response{ Data: map[string]interface{}{ - "id": issuer.ID, - "name": issuer.Name, + "issuer_id": issuer.ID, + "issuer_name": issuer.Name, "key_id": issuer.KeyID, "certificate": issuer.Certificate, }, @@ -170,7 +162,7 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da } func (b *backend) pathGetRawIssuer(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - issuerName := data.Get("ref").(string) + issuerName := data.Get("issuer_ref").(string) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } @@ -217,7 +209,7 @@ func (b *backend) pathGetRawIssuer(ctx context.Context, req *logical.Request, da } func (b *backend) pathDeleteIssuer(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - issuerName := data.Get("ref").(string) + issuerName := data.Get("issuer_ref").(string) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index 1a485189ecf0..6d72278b4067 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -20,7 +20,7 @@ func pathIssue(b *backend) *framework.Path { } func pathIssuerIssue(b *backend) *framework.Path { - pattern := "issuer/" + framework.GenericNameRegex("ref") + "/issue/" + framework.GenericNameRegex("role") + pattern := "issuer/" + framework.GenericNameRegex("issuer_ref") + "/issue/" + framework.GenericNameRegex("role") return buildPathIssue(b, pattern) } @@ -46,7 +46,7 @@ func pathSign(b *backend) *framework.Path { } func pathIssuerSign(b *backend) *framework.Path { - pattern := "issuer/" + framework.GenericNameRegex("ref") + "/sign/" + framework.GenericNameRegex("role") + pattern := "issuer/" + framework.GenericNameRegex("issuer_ref") + "/sign/" + framework.GenericNameRegex("role") return buildPathSign(b, pattern) } @@ -74,7 +74,7 @@ func buildPathSign(b *backend, pattern string) *framework.Path { } func pathIssuerSignVerbatim(b *backend) *framework.Path { - pattern := "issuers/" + framework.GenericNameRegex("ref") + "/sign-verbatim" + pattern := "issuers/" + framework.GenericNameRegex("issuer_ref") + "/sign-verbatim" return buildPathIssuerSignVerbatim(b, pattern) } @@ -218,7 +218,7 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d return nil, logical.ErrReadOnly } - issuerName := data.Get("ref").(string) + issuerName := data.Get("issuer_ref").(string) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } diff --git a/builtin/logical/pki/path_manage_issuers.go b/builtin/logical/pki/path_manage_issuers.go index 58f2a3ade714..2357352fbf0a 100644 --- a/builtin/logical/pki/path_manage_issuers.go +++ b/builtin/logical/pki/path_manage_issuers.go @@ -35,12 +35,6 @@ func commonGenerateRoot(b *backend, pattern string) *framework.Path { ret.Fields = addCACommonFields(map[string]*framework.FieldSchema{}) ret.Fields = addCAKeyGenerationFields(ret.Fields) ret.Fields = addCAIssueFields(ret.Fields) - - ret.Fields["id"] = &framework.FieldSchema{ - Type: framework.TypeString, - Description: `Assign a name to the generated issuer.`, - } - return ret } diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 18ee65f96ed6..8aa25558a859 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -153,7 +153,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, if err != nil { return nil, err } - resp.Data["id"] = myIssuer.ID + resp.Data["issuer_id"] = myIssuer.ID resp.Data["key_id"] = myKey.ID // Also store it as just the certificate identified by serial number, so it @@ -192,7 +192,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { var err error - issuerName := data.Get("ref").(string) + issuerName := data.Get("issuer_ref").(string) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } @@ -341,7 +341,7 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R func (b *backend) pathIssuerSignSelfIssued(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { var err error - issuerName := data.Get("ref").(string) + issuerName := data.Get("issuer_ref").(string) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } diff --git a/builtin/logical/pki/path_sign_issuers.go b/builtin/logical/pki/path_sign_issuers.go index c073bce48f2e..7ae493c72c22 100644 --- a/builtin/logical/pki/path_sign_issuers.go +++ b/builtin/logical/pki/path_sign_issuers.go @@ -6,7 +6,7 @@ import ( ) func pathIssuerSignIntermediate(b *backend) *framework.Path { - pattern := "issuers/" + framework.GenericNameRegex("ref") + "/sign-intermediate" + pattern := "issuers/" + framework.GenericNameRegex("issuer_ref") + "/sign-intermediate" return pathIssuerSignIntermediateRaw(b, pattern) } @@ -19,7 +19,7 @@ func pathIssuerSignIntermediateRaw(b *backend, pattern string) *framework.Path { path := &framework.Path{ Pattern: pattern, Fields: map[string]*framework.FieldSchema{ - "ref": { + "issuer_ref": { Type: framework.TypeString, Description: `Reference to issuer; either "default" for the configured default issuer, an identifier of an issuer, or the name assigned to the issuer.`, Default: "default", @@ -79,7 +79,7 @@ See the API documentation for more information about required parameters. ) func pathIssuerSignSelfIssued(b *backend) *framework.Path { - pattern := "issuers/" + framework.GenericNameRegex("ref") + "/sign-self-issued" + pattern := "issuers/" + framework.GenericNameRegex("issuer_ref") + "/sign-self-issued" return buildPathIssuerSignSelfIssued(b, pattern) } @@ -92,7 +92,7 @@ func buildPathIssuerSignSelfIssued(b *backend, pattern string) *framework.Path { path := &framework.Path{ Pattern: pattern, Fields: map[string]*framework.FieldSchema{ - "ref": { + "issuer_ref": { Type: framework.TypeString, Description: `Reference to issuer; either "default" for the configured default issuer, an identifier of an issuer, or the name assigned to the issuer.`, Default: "default", diff --git a/builtin/logical/pki/util.go b/builtin/logical/pki/util.go index f6737bdbd149..03413ee6e1b1 100644 --- a/builtin/logical/pki/util.go +++ b/builtin/logical/pki/util.go @@ -80,13 +80,13 @@ func getManagedKeyId(data *framework.FieldData) (managedKeyId, error) { } func getExistingKeyRef(data *framework.FieldData) (string, error) { - keyRef, ok := data.GetOk("key_id") + keyRef, ok := data.GetOk("key_ref") if !ok { - return "", errutil.UserError{Err: fmt.Sprintf("missing argument key_id for existing type")} + return "", errutil.UserError{Err: fmt.Sprintf("missing argument key_ref for existing type")} } trimmedKeyRef := strings.TrimSpace(keyRef.(string)) if len(trimmedKeyRef) == 0 { - return "", errutil.UserError{Err: fmt.Sprintf("missing argument key_id for existing type")} + return "", errutil.UserError{Err: fmt.Sprintf("missing argument key_ref for existing type")} } return trimmedKeyRef, nil From d2f24d7371e073cff231c19de8375430825a3a78 Mon Sep 17 00:00:00 2001 From: Steve Clark Date: Mon, 11 Apr 2022 12:49:39 -0400 Subject: [PATCH 4/7] Add utility methods to fetch common ref and name arguments - Add utility methods to fetch the issuer_name, issuer_ref, key_name and key_ref arguments from data fields. - Centralize the logic to clean up these inputs and apply various validations to all of them. --- builtin/logical/pki/backend_test.go | 36 +++++++++ builtin/logical/pki/ca_util.go | 2 +- builtin/logical/pki/managed_key_util.go | 4 +- builtin/logical/pki/path_fetch_issuers.go | 16 ++-- builtin/logical/pki/path_intermediate.go | 6 +- builtin/logical/pki/path_issue_sign.go | 2 +- builtin/logical/pki/path_manage_issuers.go | 2 +- builtin/logical/pki/path_root.go | 17 +++-- builtin/logical/pki/storage.go | 16 ++-- builtin/logical/pki/storage_migrations.go | 2 +- builtin/logical/pki/storage_test.go | 23 +++--- builtin/logical/pki/util.go | 88 ++++++++++++++++++++-- 12 files changed, 170 insertions(+), 44 deletions(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 823dcf427dcb..2ac08590767a 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -4656,6 +4656,23 @@ func TestRootWithExistingKey(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "unable to find PKI key for reference: my-key1") + // Fail if the specified key name is default. + _, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/internal", map[string]interface{}{ + "common_name": "root myvault.com", + "issuer_name": "my-issuer1", + "key_name": "Default", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "reserved keyword 'default' can not be used as key name") + + // Fail if the specified issuer name is default. + _, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/internal", map[string]interface{}{ + "common_name": "root myvault.com", + "issuer_name": "DEFAULT", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "reserved keyword 'default' can not be used as issuer name") + // Create the first CA resp, err := client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/internal", map[string]interface{}{ "common_name": "root myvault.com", @@ -4669,11 +4686,20 @@ func TestRootWithExistingKey(t *testing.T) { require.NotEmpty(t, myIssuerId1) require.NotEmpty(t, myKeyId1) + // Fail if the specified issuer name is re-used. + _, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/internal", map[string]interface{}{ + "common_name": "root myvault.com", + "issuer_name": "my-issuer1", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "issuer name already used") + // Create the second CA resp, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/internal", map[string]interface{}{ "common_name": "root myvault.com", "key_type": "rsa", "issuer_name": "my-issuer2", + "key_name": "root-key2", }) require.NoError(t, err) require.NotNil(t, resp.Data["certificate"]) @@ -4682,6 +4708,15 @@ func TestRootWithExistingKey(t *testing.T) { require.NotEmpty(t, myIssuerId2) require.NotEmpty(t, myKeyId2) + // Fail if the specified key name is re-used. + _, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/internal", map[string]interface{}{ + "common_name": "root myvault.com", + "issuer_name": "my-issuer3", + "key_name": "root-key2", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "key name already used") + // Create a third CA re-using key from CA 1 resp, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/root/existing", map[string]interface{}{ "common_name": "root myvault.com", @@ -4764,6 +4799,7 @@ func TestIntermediateWithExistingKey(t *testing.T) { resp, err = client.Logical().WriteWithContext(ctx, "pki-root/issuers/generate/intermediate/internal", map[string]interface{}{ "common_name": "root myvault.com", "key_type": "rsa", + "key_name": "interkey1", }) require.NoError(t, err) // csr2 := resp.Data["csr"] diff --git a/builtin/logical/pki/ca_util.go b/builtin/logical/pki/ca_util.go index b9d6616f2736..41987217c030 100644 --- a/builtin/logical/pki/ca_util.go +++ b/builtin/logical/pki/ca_util.go @@ -116,7 +116,7 @@ func getKeyTypeAndBitsForRole(ctx context.Context, b *backend, data *framework.F } func getExistingPublicKey(ctx context.Context, s logical.Storage, data *framework.FieldData) (crypto.PublicKey, error) { - keyRef, err := getExistingKeyRef(data) + keyRef, err := getKeyRefWithErr(data) if err != nil { return nil, err } diff --git a/builtin/logical/pki/managed_key_util.go b/builtin/logical/pki/managed_key_util.go index caff650958d7..e3569cf3433a 100644 --- a/builtin/logical/pki/managed_key_util.go +++ b/builtin/logical/pki/managed_key_util.go @@ -19,7 +19,7 @@ func generateCABundle(ctx context.Context, _ *backend, input *inputBundle, data return nil, errEntOnly } if existingKeyRequested(input) { - keyRef, err := getExistingKeyRef(input.apiData) + keyRef, err := getKeyRefWithErr(input.apiData) if err != nil { return nil, err } @@ -33,7 +33,7 @@ func generateCSRBundle(ctx context.Context, _ *backend, input *inputBundle, data return nil, errEntOnly } if existingKeyRequested(input) { - keyRef, err := getExistingKeyRef(input.apiData) + keyRef, err := getKeyRefWithErr(input.apiData) if err != nil { return nil, err } diff --git a/builtin/logical/pki/path_fetch_issuers.go b/builtin/logical/pki/path_fetch_issuers.go index a85eb81da457..28e5f46f3ca4 100644 --- a/builtin/logical/pki/path_fetch_issuers.go +++ b/builtin/logical/pki/path_fetch_issuers.go @@ -25,7 +25,7 @@ func pathListIssuers(b *backend) *framework.Path { } } -func (b *backend) pathListIssuersHandler(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { +func (b *backend) pathListIssuersHandler(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) { var responseKeys []string responseInfo := make(map[string]interface{}) @@ -90,7 +90,7 @@ func (b *backend) pathGetIssuer(ctx context.Context, req *logical.Request, data return b.pathGetRawIssuer(ctx, req, data) } - issuerName := data.Get("issuer_ref").(string) + issuerName := getIssuerRef(data) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } @@ -119,14 +119,14 @@ func (b *backend) pathGetIssuer(ctx context.Context, req *logical.Request, data } func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - issuerName := data.Get("issuer_ref").(string) + issuerName := getIssuerRef(data) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } - newName := data.Get("issuer_name").(string) - if len(newName) > 0 && !nameMatcher.MatchString(newName) { - return logical.ErrorResponse("new issuer name outside of valid character limits"), nil + newName, err := getIssuerName(ctx, req.Storage, data) + if err != nil { + return logical.ErrorResponse(err.Error()), nil } ref, err := resolveIssuerReference(ctx, req.Storage, issuerName) @@ -162,7 +162,7 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da } func (b *backend) pathGetRawIssuer(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - issuerName := data.Get("issuer_ref").(string) + issuerName := getIssuerRef(data) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } @@ -209,7 +209,7 @@ func (b *backend) pathGetRawIssuer(ctx context.Context, req *logical.Request, da } func (b *backend) pathDeleteIssuer(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - issuerName := data.Get("issuer_ref").(string) + issuerName := getIssuerRef(data) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } diff --git a/builtin/logical/pki/path_intermediate.go b/builtin/logical/pki/path_intermediate.go index 6828c20b0a00..88dbef9c5e64 100644 --- a/builtin/logical/pki/path_intermediate.go +++ b/builtin/logical/pki/path_intermediate.go @@ -52,6 +52,10 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req return errorResp, nil } + keyName, err := getKeyName(ctx, req.Storage, data) + if err != nil { + return logical.ErrorResponse(err.Error()), nil + } var resp *logical.Response input := &inputBundle{ role: role, @@ -110,7 +114,7 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req } } - myKey, _, err := importKey(ctx, req.Storage, csrb.PrivateKey) + myKey, _, err := importKey(ctx, req.Storage, csrb.PrivateKey, keyName) if err != nil { return nil, err } diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index 6d72278b4067..4aae8c278cdd 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -218,7 +218,7 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d return nil, logical.ErrReadOnly } - issuerName := data.Get("issuer_ref").(string) + issuerName := getIssuerRef(data) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } diff --git a/builtin/logical/pki/path_manage_issuers.go b/builtin/logical/pki/path_manage_issuers.go index 2357352fbf0a..d4bb435a7580 100644 --- a/builtin/logical/pki/path_manage_issuers.go +++ b/builtin/logical/pki/path_manage_issuers.go @@ -138,7 +138,7 @@ func (b *backend) pathImportIssuers(ctx context.Context, req *logical.Request, d for _, keyPem := range keys { // Handle import of private key. - key, existing, err := importKey(ctx, req.Storage, keyPem) + key, existing, err := importKey(ctx, req.Storage, keyPem, "") if err != nil { return logical.ErrorResponse(err.Error()), nil } diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 8aa25558a859..8b650b78043a 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -77,10 +77,13 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, role.MaxPathLength = &maxPathLength } - issuerName := "" - issuerNameIface, ok := data.GetOk("id") - if ok { - issuerName = issuerNameIface.(string) + issuerName, err := getIssuerName(ctx, req.Storage, data) + if err != nil { + return logical.ErrorResponse(err.Error()), nil + } + keyName, err := getKeyName(ctx, req.Storage, data) + if err != nil { + return logical.ErrorResponse(err.Error()), nil } input := &inputBundle{ @@ -149,7 +152,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, } // Store it as the CA bundle - myIssuer, myKey, err := writeCaBundle(ctx, req.Storage, cb, issuerName) + myIssuer, myKey, err := writeCaBundle(ctx, req.Storage, cb, issuerName, keyName) if err != nil { return nil, err } @@ -192,7 +195,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { var err error - issuerName := data.Get("issuer_ref").(string) + issuerName := getIssuerRef(data) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } @@ -341,7 +344,7 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R func (b *backend) pathIssuerSignSelfIssued(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { var err error - issuerName := data.Get("issuer_ref").(string) + issuerName := getIssuerRef(data) if len(issuerName) == 0 { return logical.ErrorResponse("missing issuer reference"), nil } diff --git a/builtin/logical/pki/storage.go b/builtin/logical/pki/storage.go index 8799caf1112e..17451e68eab4 100644 --- a/builtin/logical/pki/storage.go +++ b/builtin/logical/pki/storage.go @@ -36,6 +36,11 @@ func (p issuerId) String() string { return string(p) } +const ( + IssuerRefNotFound = issuerId("not-found") + KeyRefNotFound = keyId("not-found") +) + type key struct { ID keyId `json:"id" structs:"id" mapstructure:"id"` Name string `json:"name" structs:"name" mapstructure:"name"` @@ -112,7 +117,7 @@ func deleteKey(ctx context.Context, s logical.Storage, id keyId) error { return s.Delete(ctx, keyPrefix+id.String()) } -func importKey(ctx context.Context, s logical.Storage, keyValue string) (*key, bool, error) { +func importKey(ctx context.Context, s logical.Storage, keyValue string, keyName string) (*key, bool, error) { // importKey imports the specified PEM-format key (from keyValue) into // the new PKI storage format. The first return field is a reference to // the new key; the second is whether or not the key already existed @@ -154,6 +159,7 @@ func importKey(ctx context.Context, s logical.Storage, keyValue string) (*key, b // Haven't found a key, so we've gotta create it and write it into storage. var result key result.ID = genKeyId() + result.Name = keyName result.PrivateKey = keyValue // Extracting the signer is necessary for two reasons: first, to get the @@ -270,7 +276,7 @@ func resolveKeyReference(ctx context.Context, s logical.Storage, reference strin } // Otherwise, we must not have found the key. - return keyId("not-found"), errutil.UserError{Err: fmt.Sprintf("unable to find PKI key for reference: %v", reference)} + return KeyRefNotFound, errutil.UserError{Err: fmt.Sprintf("unable to find PKI key for reference: %v", reference)} } func fetchIssuerById(ctx context.Context, s logical.Storage, issuerId issuerId) (*issuer, error) { @@ -488,7 +494,7 @@ func resolveIssuerReference(ctx context.Context, s logical.Storage, reference st } // Otherwise, we must not have found the issuer. - return issuerId("not-found"), errutil.UserError{Err: fmt.Sprintf("unable to find PKI issuer for reference: %v", reference)} + return IssuerRefNotFound, errutil.UserError{Err: fmt.Sprintf("unable to find PKI issuer for reference: %v", reference)} } // Builds a certutil.CertBundle from the specified issuer identifier, @@ -519,7 +525,7 @@ func fetchCertBundleByIssuerId(ctx context.Context, s logical.Storage, id issuer return &bundle, nil } -func writeCaBundle(ctx context.Context, s logical.Storage, caBundle *certutil.CertBundle, issuerName string) (*issuer, *key, error) { +func writeCaBundle(ctx context.Context, s logical.Storage, caBundle *certutil.CertBundle, issuerName string, keyName string) (*issuer, *key, error) { allKeyIds, err := listKeys(ctx, s) if err != nil { return nil, nil, err @@ -530,7 +536,7 @@ func writeCaBundle(ctx context.Context, s logical.Storage, caBundle *certutil.Ce return nil, nil, err } - myKey, _, err := importKey(ctx, s, caBundle.PrivateKey) + myKey, _, err := importKey(ctx, s, caBundle.PrivateKey, keyName) if err != nil { return nil, nil, err } diff --git a/builtin/logical/pki/storage_migrations.go b/builtin/logical/pki/storage_migrations.go index c1e05263a77e..23c1b2399700 100644 --- a/builtin/logical/pki/storage_migrations.go +++ b/builtin/logical/pki/storage_migrations.go @@ -50,7 +50,7 @@ func migrateStorage(ctx context.Context, req *logical.InitializationRequest, log logger.Warn("performing PKI migration to new keys/issuers layout") - anIssuer, aKey, err := writeCaBundle(ctx, s, legacyBundle, "") + anIssuer, aKey, err := writeCaBundle(ctx, s, legacyBundle, "", "") if err != nil { return err } diff --git a/builtin/logical/pki/storage_test.go b/builtin/logical/pki/storage_test.go index b96118a44827..b1b4e277b6ac 100644 --- a/builtin/logical/pki/storage_test.go +++ b/builtin/logical/pki/storage_test.go @@ -102,29 +102,32 @@ func Test_KeysIssuerImport(t *testing.T) { issuer1.ID = "" issuer1.KeyID = "" - key1_ref1, existing, err := importKey(ctx, s, key1.PrivateKey) + key1_ref1, existing, err := importKey(ctx, s, key1.PrivateKey, "key1") require.NoError(t, err) require.False(t, existing) require.Equal(t, key1.PrivateKey, key1_ref1.PrivateKey) - key1_ref2, existing, err := importKey(ctx, s, key1.PrivateKey) + key1_ref2, existing, err := importKey(ctx, s, key1.PrivateKey, "ignore-me") require.NoError(t, err) require.True(t, existing) require.Equal(t, key1.PrivateKey, key1_ref1.PrivateKey) require.Equal(t, key1_ref1.ID, key1_ref2.ID) + require.Equal(t, key1_ref1.Name, key1_ref2.Name) - issuer1_ref1, existing, err := importIssuer(ctx, s, issuer1.Certificate, "") + issuer1_ref1, existing, err := importIssuer(ctx, s, issuer1.Certificate, "issuer1") require.NoError(t, err) require.False(t, existing) require.Equal(t, issuer1.Certificate, issuer1_ref1.Certificate) require.Equal(t, key1_ref1.ID, issuer1_ref1.KeyID) + require.Equal(t, "issuer1", issuer1_ref1.Name) - issuer1_ref2, existing, err := importIssuer(ctx, s, issuer1.Certificate, "") + issuer1_ref2, existing, err := importIssuer(ctx, s, issuer1.Certificate, "ignore-me") require.NoError(t, err) require.True(t, existing) require.Equal(t, issuer1.Certificate, issuer1_ref1.Certificate) require.Equal(t, issuer1_ref1.ID, issuer1_ref2.ID) require.Equal(t, key1_ref1.ID, issuer1_ref2.KeyID) + require.Equal(t, issuer1_ref1.Name, issuer1_ref2.Name) err = writeIssuer(ctx, s, &issuer2) require.NoError(t, err) @@ -132,18 +135,20 @@ func Test_KeysIssuerImport(t *testing.T) { err = writeKey(ctx, s, key2) require.NoError(t, err) - issuer2_ref, existing, err := importIssuer(ctx, s, issuer2.Certificate, "") + issuer2_ref, existing, err := importIssuer(ctx, s, issuer2.Certificate, "ignore-me") require.NoError(t, err) require.True(t, existing) require.Equal(t, issuer2.Certificate, issuer2_ref.Certificate) - require.Equal(t, issuer2_ref.ID, issuer2.ID) - require.Equal(t, issuer2_ref.KeyID, issuer2.KeyID) + require.Equal(t, issuer2.ID, issuer2_ref.ID) + require.Equal(t, "", issuer2_ref.Name) + require.Equal(t, issuer2.KeyID, issuer2_ref.KeyID) - key2_ref, existing, err := importKey(ctx, s, key2.PrivateKey) + key2_ref, existing, err := importKey(ctx, s, key2.PrivateKey, "ignore-me") require.NoError(t, err) require.True(t, existing) require.Equal(t, key2.PrivateKey, key2_ref.PrivateKey) - require.Equal(t, key2_ref.ID, key2.ID) + require.Equal(t, key2.ID, key2_ref.ID) + require.Equal(t, "", key2_ref.Name) } func genIssuerAndKey(t *testing.T, b *backend) (issuer, key) { diff --git a/builtin/logical/pki/util.go b/builtin/logical/pki/util.go index 03413ee6e1b1..5e92134739ac 100644 --- a/builtin/logical/pki/util.go +++ b/builtin/logical/pki/util.go @@ -1,9 +1,12 @@ package pki import ( + "context" "fmt" "strings" + "github.com/hashicorp/vault/sdk/logical" + "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/errutil" @@ -79,17 +82,14 @@ func getManagedKeyId(data *framework.FieldData) (managedKeyId, error) { return keyId, nil } -func getExistingKeyRef(data *framework.FieldData) (string, error) { - keyRef, ok := data.GetOk("key_ref") - if !ok { - return "", errutil.UserError{Err: fmt.Sprintf("missing argument key_ref for existing type")} - } - trimmedKeyRef := strings.TrimSpace(keyRef.(string)) - if len(trimmedKeyRef) == 0 { +func getKeyRefWithErr(data *framework.FieldData) (string, error) { + keyRef := getKeyRef(data) + + if len(keyRef) == 0 { return "", errutil.UserError{Err: fmt.Sprintf("missing argument key_ref for existing type")} } - return trimmedKeyRef, nil + return keyRef, nil } func getManagedKeyNameOrUUID(data *framework.FieldData) (name string, UUID string, err error) { @@ -122,3 +122,75 @@ func getManagedKeyNameOrUUID(data *framework.FieldData) (name string, UUID strin return keyName, keyUUID, nil } + +func getIssuerName(ctx context.Context, s logical.Storage, data *framework.FieldData) (string, error) { + issuerName := "" + issuerNameIface, ok := data.GetOk("issuer_name") + if ok { + issuerName = strings.TrimSpace(issuerNameIface.(string)) + + if strings.ToLower(issuerName) == "default" { + return "", errutil.UserError{Err: "reserved keyword 'default' can not be used as issuer name"} + } + + if !nameMatcher.MatchString(issuerName) { + return "", errutil.UserError{Err: "issuer name contained invalid characters"} + } + issuer_id, err := resolveIssuerReference(ctx, s, issuerName) + if err == nil { + return "", errutil.UserError{Err: "issuer name already used."} + } + + if err != nil && issuer_id != IssuerRefNotFound { + return "", errutil.InternalError{Err: err.Error()} + } + } + return issuerName, nil +} + +func getKeyName(ctx context.Context, s logical.Storage, data *framework.FieldData) (string, error) { + keyName := "" + keyNameIface, ok := data.GetOk("key_name") + if ok { + keyName = strings.TrimSpace(keyNameIface.(string)) + + if strings.ToLower(keyName) == "default" { + return "", errutil.UserError{Err: "reserved keyword 'default' can not be used as key name"} + } + + if !nameMatcher.MatchString(keyName) { + return "", errutil.UserError{Err: "key name contained invalid characters"} + } + key_id, err := resolveKeyReference(ctx, s, keyName) + if err == nil { + return "", errutil.UserError{Err: "key name already used."} + } + + if err != nil && key_id != KeyRefNotFound { + return "", errutil.InternalError{Err: err.Error()} + } + } + return keyName, nil +} + +func getIssuerRef(data *framework.FieldData) string { + return extractRef(data, "issuer_ref") +} + +func getKeyRef(data *framework.FieldData) string { + return extractRef(data, "key_ref") +} + +func extractRef(data *framework.FieldData, paramName string) string { + value := "" + issuerNameIface, ok := data.GetOk(paramName) + if ok { + value = strings.TrimSpace(issuerNameIface.(string)) + if strings.ToLower(value) == "default" { + return "default" + } + return value + } + + return value +} From 58115c0f3ec8bfb6330648e4dd2fd8f26338d000 Mon Sep 17 00:00:00 2001 From: Steve Clark Date: Mon, 11 Apr 2022 14:16:49 -0400 Subject: [PATCH 5/7] Rename common PKI backend handlers - Use the buildPath convention for the function name instead of common... --- builtin/logical/pki/path_intermediate.go | 2 +- builtin/logical/pki/path_manage_issuers.go | 8 ++++---- builtin/logical/pki/path_root.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/logical/pki/path_intermediate.go b/builtin/logical/pki/path_intermediate.go index 88dbef9c5e64..2783b08cb9d3 100644 --- a/builtin/logical/pki/path_intermediate.go +++ b/builtin/logical/pki/path_intermediate.go @@ -12,7 +12,7 @@ import ( ) func pathGenerateIntermediate(b *backend) *framework.Path { - return commonGenerateIntermediate(b, "intermediate/generate/"+framework.GenericNameRegex("exported")) + return buildPathGenerateIntermediate(b, "intermediate/generate/"+framework.GenericNameRegex("exported")) } func pathSetSignedIntermediate(b *backend) *framework.Path { diff --git a/builtin/logical/pki/path_manage_issuers.go b/builtin/logical/pki/path_manage_issuers.go index d4bb435a7580..eb04a21ef48a 100644 --- a/builtin/logical/pki/path_manage_issuers.go +++ b/builtin/logical/pki/path_manage_issuers.go @@ -12,10 +12,10 @@ import ( ) func pathIssuerGenerateRoot(b *backend) *framework.Path { - return commonGenerateRoot(b, "issuers/generate/root/"+framework.GenericNameRegex("exported")) + return buildPathGenerateRoot(b, "issuers/generate/root/"+framework.GenericNameRegex("exported")) } -func commonGenerateRoot(b *backend, pattern string) *framework.Path { +func buildPathGenerateRoot(b *backend, pattern string) *framework.Path { ret := &framework.Path{ Pattern: pattern, @@ -39,11 +39,11 @@ func commonGenerateRoot(b *backend, pattern string) *framework.Path { } func pathIssuerGenerateIntermediate(b *backend) *framework.Path { - return commonGenerateIntermediate(b, + return buildPathGenerateIntermediate(b, "issuers/generate/intermediate/"+framework.GenericNameRegex("exported")) } -func commonGenerateIntermediate(b *backend, pattern string) *framework.Path { +func buildPathGenerateIntermediate(b *backend, pattern string) *framework.Path { ret := &framework.Path{ Pattern: pattern, Operations: map[logical.Operation]framework.OperationHandler{ diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 8b650b78043a..297393c854c4 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -26,7 +26,7 @@ import ( ) func pathGenerateRoot(b *backend) *framework.Path { - return commonGenerateRoot(b, "root/generate/"+framework.GenericNameRegex("exported")) + return buildPathGenerateRoot(b, "root/generate/"+framework.GenericNameRegex("exported")) } func pathDeleteRoot(b *backend) *framework.Path { From 5d5e0be507d1c49d73e299ca98eee2e9f4c2ec67 Mon Sep 17 00:00:00 2001 From: Steve Clark Date: Mon, 11 Apr 2022 14:35:26 -0400 Subject: [PATCH 6/7] Move setting PKI defaults from writeCaBundle to proper import{keys,issuer} methods - PR feedback, move setting up the default configuration references within the import methods instead of within the writeCaBundle method. This should now cover all use cases of us setting up the defaults properly. --- builtin/logical/pki/storage.go | 56 +++++++++++++++------------------- builtin/logical/pki/util.go | 12 ++------ 2 files changed, 28 insertions(+), 40 deletions(-) diff --git a/builtin/logical/pki/storage.go b/builtin/logical/pki/storage.go index 17451e68eab4..e0e64ac45459 100644 --- a/builtin/logical/pki/storage.go +++ b/builtin/logical/pki/storage.go @@ -213,6 +213,18 @@ func importKey(ctx context.Context, s logical.Storage, keyValue string, keyName } } + // If there was no prior default value set and/or we had no known + // keys when we started, set this key as default. + keyDefaultSet, err := isKeyDefaultSet(ctx, s) + if err != nil { + return nil, false, err + } + if len(knownKeys) == 0 || !keyDefaultSet { + if err = updateDefaultKeyId(ctx, s, result.ID); err != nil { + return nil, false, err + } + } + // All done; return our new key reference. return &result, false, nil } @@ -399,11 +411,23 @@ func importIssuer(ctx context.Context, s logical.Storage, certValue string, issu } } - // Finally we can write the issuer to storage. + // We can write the issuer to storage. if err := writeIssuer(ctx, s, &result); err != nil { return nil, false, err } + // If there was no prior default value set and/or we had no known + // issuers when we started, set this issuer as default. + issuerDefaultSet, err := isIssuerDefaultSet(ctx, s) + if err != nil { + return nil, false, err + } + if len(knownIssuers) == 0 || !issuerDefaultSet { + if err = updateDefaultIssuerId(ctx, s, result.ID); err != nil { + return nil, false, err + } + } + // All done; return our new key reference. return &result, false, nil } @@ -526,16 +550,6 @@ func fetchCertBundleByIssuerId(ctx context.Context, s logical.Storage, id issuer } func writeCaBundle(ctx context.Context, s logical.Storage, caBundle *certutil.CertBundle, issuerName string, keyName string) (*issuer, *key, error) { - allKeyIds, err := listKeys(ctx, s) - if err != nil { - return nil, nil, err - } - - allIssuerIds, err := listIssuers(ctx, s) - if err != nil { - return nil, nil, err - } - myKey, _, err := importKey(ctx, s, caBundle.PrivateKey, keyName) if err != nil { return nil, nil, err @@ -552,26 +566,6 @@ func writeCaBundle(ctx context.Context, s logical.Storage, caBundle *certutil.Ce } } - keyDefaultSet, err := isKeyDefaultSet(ctx, s) - if err != nil { - return nil, nil, err - } - if len(allKeyIds) == 0 || !keyDefaultSet { - if err = updateDefaultKeyId(ctx, s, myKey.ID); err != nil { - return nil, nil, err - } - } - - issuerDefaultSet, err := isIssuerDefaultSet(ctx, s) - if err != nil { - return nil, nil, err - } - if len(allIssuerIds) == 0 || !issuerDefaultSet { - if err = updateDefaultIssuerId(ctx, s, myIssuer.ID); err != nil { - return nil, nil, err - } - } - return myIssuer, myKey, nil } diff --git a/builtin/logical/pki/util.go b/builtin/logical/pki/util.go index 5e92134739ac..4c524e1dd000 100644 --- a/builtin/logical/pki/util.go +++ b/builtin/logical/pki/util.go @@ -182,15 +182,9 @@ func getKeyRef(data *framework.FieldData) string { } func extractRef(data *framework.FieldData, paramName string) string { - value := "" - issuerNameIface, ok := data.GetOk(paramName) - if ok { - value = strings.TrimSpace(issuerNameIface.(string)) - if strings.ToLower(value) == "default" { - return "default" - } - return value + value := strings.TrimSpace(data.Get(paramName).(string)) + if strings.ToLower(value) == "default" { + return "default" } - return value } From ce4769add6a75721314e61c326fb54847980c4be Mon Sep 17 00:00:00 2001 From: Steve Clark Date: Mon, 11 Apr 2022 17:22:28 -0400 Subject: [PATCH 7/7] Introduce constants for issuer_ref, rename isKeyDefaultSet... --- builtin/logical/pki/backend_test.go | 3 +- builtin/logical/pki/config_util.go | 4 +- builtin/logical/pki/fields.go | 30 ++++++----- builtin/logical/pki/path_config_ca.go | 66 +---------------------- builtin/logical/pki/path_fetch_issuers.go | 4 +- builtin/logical/pki/path_issue_sign.go | 6 +-- builtin/logical/pki/path_root.go | 11 ---- builtin/logical/pki/path_sign_issuers.go | 8 +-- builtin/logical/pki/storage.go | 4 +- builtin/logical/pki/storage_test.go | 6 +++ builtin/logical/pki/util.go | 2 +- 11 files changed, 39 insertions(+), 105 deletions(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 2ac08590767a..ce3327bfabe9 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -2275,7 +2275,6 @@ func TestBackend_Root_Idempotency(t *testing.T) { }) cluster.Start() defer cluster.Cleanup() - client := cluster.Cores[0].Client var err error err = client.Sys().MountWithContext(context.Background(), "pki", &api.MountInput{ @@ -4817,7 +4816,7 @@ func TestIntermediateWithExistingKey(t *testing.T) { require.NotEmpty(t, myKeyId3) require.NotEqual(t, myKeyId1, myKeyId2) - require.Equal(t, myKeyId1, myKeyId3) + require.Equal(t, myKeyId1, myKeyId3, "our new ca did not seem to reuse the key as we expected.") } var ( diff --git a/builtin/logical/pki/config_util.go b/builtin/logical/pki/config_util.go index 830590fbd8b0..0dddc620ab27 100644 --- a/builtin/logical/pki/config_util.go +++ b/builtin/logical/pki/config_util.go @@ -7,7 +7,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -func isKeyDefaultSet(ctx context.Context, s logical.Storage) (bool, error) { +func isDefaultKeySet(ctx context.Context, s logical.Storage) (bool, error) { config, err := getKeysConfig(ctx, s) if err != nil { return false, err @@ -16,7 +16,7 @@ func isKeyDefaultSet(ctx context.Context, s logical.Storage) (bool, error) { return strings.TrimSpace(config.DefaultKeyId.String()) != "", nil } -func isIssuerDefaultSet(ctx context.Context, s logical.Storage) (bool, error) { +func isDefaultIssuerSet(ctx context.Context, s logical.Storage) (bool, error) { config, err := getIssuersConfig(ctx, s) if err != nil { return false, err diff --git a/builtin/logical/pki/fields.go b/builtin/logical/pki/fields.go index d9f23fe58a20..afd872396453 100644 --- a/builtin/logical/pki/fields.go +++ b/builtin/logical/pki/fields.go @@ -2,6 +2,10 @@ package pki import "github.com/hashicorp/vault/sdk/framework" +const ( + issuerRefParam = "issuer_ref" +) + // addIssueAndSignCommonFields adds fields common to both CA and non-CA issuing // and signing func addIssueAndSignCommonFields(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { @@ -338,20 +342,23 @@ func addCAIssueFields(fields map[string]*framework.FieldSchema) map[string]*fram return fields } -func addKeyRefNameFields(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { - fields = addKeyNameField(fields) - fields = addKeyRefField(fields) - return fields -} - func addIssuerRefNameFields(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { fields = addIssuerNameField(fields) fields = addIssuerRefField(fields) return fields } +func addIssuerNameField(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { + fields["issuer_name"] = &framework.FieldSchema{ + Type: framework.TypeString, + Description: `Provide a name to the generated issuer, the name +must be unique across all issuers and not be the reserved value 'default'`, + } + return fields +} + func addIssuerRefField(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { - fields["issuer_ref"] = &framework.FieldSchema{ + fields[issuerRefParam] = &framework.FieldSchema{ Type: framework.TypeString, Description: `Reference to a existing issuer; either "default" for the configured default issuer, an identifier or the name assigned @@ -361,12 +368,9 @@ to the issuer.`, return fields } -func addIssuerNameField(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { - fields["issuer_name"] = &framework.FieldSchema{ - Type: framework.TypeString, - Description: `Provide a name to the generated issuer, the name -must be unique across all issuers and not be the reserved value 'default'`, - } +func addKeyRefNameFields(fields map[string]*framework.FieldSchema) map[string]*framework.FieldSchema { + fields = addKeyNameField(fields) + fields = addKeyRefField(fields) return fields } diff --git a/builtin/logical/pki/path_config_ca.go b/builtin/logical/pki/path_config_ca.go index d16137ea3c5b..6b4590344966 100644 --- a/builtin/logical/pki/path_config_ca.go +++ b/builtin/logical/pki/path_config_ca.go @@ -2,11 +2,8 @@ package pki import ( "context" - "fmt" "github.com/hashicorp/vault/sdk/framework" - "github.com/hashicorp/vault/sdk/helper/certutil" - "github.com/hashicorp/vault/sdk/helper/errutil" "github.com/hashicorp/vault/sdk/logical" ) @@ -22,7 +19,7 @@ secret key and certificate.`, }, Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.UpdateOperation: b.pathCAWrite, + logical.UpdateOperation: b.pathImportIssuers, }, HelpSynopsis: pathConfigCAHelpSyn, @@ -30,67 +27,6 @@ secret key and certificate.`, } } -func (b *backend) pathCAWrite(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - pemBundle := data.Get("pem_bundle").(string) - - if pemBundle == "" { - return logical.ErrorResponse("'pem_bundle' was empty"), nil - } - - parsedBundle, err := certutil.ParsePEMBundle(pemBundle) - if err != nil { - switch err.(type) { - case errutil.InternalError: - return nil, err - default: - return logical.ErrorResponse(err.Error()), nil - } - } - - if parsedBundle.PrivateKey == nil { - return logical.ErrorResponse("private key not found in the PEM bundle"), nil - } - - if parsedBundle.PrivateKeyType == certutil.UnknownPrivateKey { - return logical.ErrorResponse("unknown private key found in the PEM bundle"), nil - } - - if parsedBundle.Certificate == nil { - return logical.ErrorResponse("no certificate found in the PEM bundle"), nil - } - - if !parsedBundle.Certificate.IsCA { - return logical.ErrorResponse("the given certificate is not marked for CA use and cannot be used with this backend"), nil - } - - cb, err := parsedBundle.ToCertBundle() - if err != nil { - return nil, fmt.Errorf("error converting raw values into cert bundle: %w", err) - } - - entry, err := logical.StorageEntryJSON("config/ca_bundle", cb) - if err != nil { - return nil, err - } - err = req.Storage.Put(ctx, entry) - if err != nil { - return nil, err - } - - // For ease of later use, also store just the certificate at a known - // location, plus a fresh CRL - entry.Key = "ca" - entry.Value = parsedBundle.CertificateBytes - err = req.Storage.Put(ctx, entry) - if err != nil { - return nil, err - } - - err = buildCRL(ctx, b, req, true) - - return nil, err -} - const pathConfigCAHelpSyn = ` Set the CA certificate and private key used for generated credentials. ` diff --git a/builtin/logical/pki/path_fetch_issuers.go b/builtin/logical/pki/path_fetch_issuers.go index 28e5f46f3ca4..6f15761d31fb 100644 --- a/builtin/logical/pki/path_fetch_issuers.go +++ b/builtin/logical/pki/path_fetch_issuers.go @@ -10,7 +10,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -var nameMatcher = regexp.MustCompile("^" + framework.GenericNameRegex("issuer_ref") + "$") +var nameMatcher = regexp.MustCompile("^" + framework.GenericNameRegex(issuerRefParam) + "$") func pathListIssuers(b *backend) *framework.Path { return &framework.Path{ @@ -61,7 +61,7 @@ their identifier and their name (if set). ) func pathGetIssuer(b *backend) *framework.Path { - pattern := "issuer/" + framework.GenericNameRegex("issuer_ref") + "(/der|/pem)?" + pattern := "issuer/" + framework.GenericNameRegex(issuerRefParam) + "(/der|/pem)?" return buildPathGetIssuer(b, pattern) } diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index 4aae8c278cdd..e77a983fa61e 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -20,7 +20,7 @@ func pathIssue(b *backend) *framework.Path { } func pathIssuerIssue(b *backend) *framework.Path { - pattern := "issuer/" + framework.GenericNameRegex("issuer_ref") + "/issue/" + framework.GenericNameRegex("role") + pattern := "issuer/" + framework.GenericNameRegex(issuerRefParam) + "/issue/" + framework.GenericNameRegex("role") return buildPathIssue(b, pattern) } @@ -46,7 +46,7 @@ func pathSign(b *backend) *framework.Path { } func pathIssuerSign(b *backend) *framework.Path { - pattern := "issuer/" + framework.GenericNameRegex("issuer_ref") + "/sign/" + framework.GenericNameRegex("role") + pattern := "issuer/" + framework.GenericNameRegex(issuerRefParam) + "/sign/" + framework.GenericNameRegex("role") return buildPathSign(b, pattern) } @@ -74,7 +74,7 @@ func buildPathSign(b *backend, pattern string) *framework.Path { } func pathIssuerSignVerbatim(b *backend) *framework.Path { - pattern := "issuers/" + framework.GenericNameRegex("issuer_ref") + "/sign-verbatim" + pattern := "issuers/" + framework.GenericNameRegex(issuerRefParam) + "/sign-verbatim" return buildPathIssuerSignVerbatim(b, pattern) } diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 297393c854c4..41b2734cea2b 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -55,17 +55,6 @@ func (b *backend) pathCADeleteRoot(ctx context.Context, req *logical.Request, da func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { var err error - entry, err := req.Storage.Get(ctx, "config/ca_bundle") - if err != nil { - return nil, err - } - if entry != nil { - resp := &logical.Response{} - resp.AddWarning(fmt.Sprintf("Refusing to generate a root certificate over an existing root certificate. "+ - "If you really want to destroy the original root certificate, please issue a delete against %s root.", req.MountPoint)) - return resp, nil - } - exported, format, role, errorResp := b.getGenerationParams(ctx, data, req.MountPoint) if errorResp != nil { return errorResp, nil diff --git a/builtin/logical/pki/path_sign_issuers.go b/builtin/logical/pki/path_sign_issuers.go index 7ae493c72c22..7afa1c752f7a 100644 --- a/builtin/logical/pki/path_sign_issuers.go +++ b/builtin/logical/pki/path_sign_issuers.go @@ -6,7 +6,7 @@ import ( ) func pathIssuerSignIntermediate(b *backend) *framework.Path { - pattern := "issuers/" + framework.GenericNameRegex("issuer_ref") + "/sign-intermediate" + pattern := "issuers/" + framework.GenericNameRegex(issuerRefParam) + "/sign-intermediate" return pathIssuerSignIntermediateRaw(b, pattern) } @@ -19,7 +19,7 @@ func pathIssuerSignIntermediateRaw(b *backend, pattern string) *framework.Path { path := &framework.Path{ Pattern: pattern, Fields: map[string]*framework.FieldSchema{ - "issuer_ref": { + issuerRefParam: { Type: framework.TypeString, Description: `Reference to issuer; either "default" for the configured default issuer, an identifier of an issuer, or the name assigned to the issuer.`, Default: "default", @@ -79,7 +79,7 @@ See the API documentation for more information about required parameters. ) func pathIssuerSignSelfIssued(b *backend) *framework.Path { - pattern := "issuers/" + framework.GenericNameRegex("issuer_ref") + "/sign-self-issued" + pattern := "issuers/" + framework.GenericNameRegex(issuerRefParam) + "/sign-self-issued" return buildPathIssuerSignSelfIssued(b, pattern) } @@ -92,7 +92,7 @@ func buildPathIssuerSignSelfIssued(b *backend, pattern string) *framework.Path { path := &framework.Path{ Pattern: pattern, Fields: map[string]*framework.FieldSchema{ - "issuer_ref": { + issuerRefParam: { Type: framework.TypeString, Description: `Reference to issuer; either "default" for the configured default issuer, an identifier of an issuer, or the name assigned to the issuer.`, Default: "default", diff --git a/builtin/logical/pki/storage.go b/builtin/logical/pki/storage.go index e0e64ac45459..e341f1fb8817 100644 --- a/builtin/logical/pki/storage.go +++ b/builtin/logical/pki/storage.go @@ -215,7 +215,7 @@ func importKey(ctx context.Context, s logical.Storage, keyValue string, keyName // If there was no prior default value set and/or we had no known // keys when we started, set this key as default. - keyDefaultSet, err := isKeyDefaultSet(ctx, s) + keyDefaultSet, err := isDefaultKeySet(ctx, s) if err != nil { return nil, false, err } @@ -418,7 +418,7 @@ func importIssuer(ctx context.Context, s logical.Storage, certValue string, issu // If there was no prior default value set and/or we had no known // issuers when we started, set this issuer as default. - issuerDefaultSet, err := isIssuerDefaultSet(ctx, s) + issuerDefaultSet, err := isDefaultIssuerSet(ctx, s) if err != nil { return nil, false, err } diff --git a/builtin/logical/pki/storage_test.go b/builtin/logical/pki/storage_test.go index b1b4e277b6ac..a47a324c3c41 100644 --- a/builtin/logical/pki/storage_test.go +++ b/builtin/logical/pki/storage_test.go @@ -107,6 +107,8 @@ func Test_KeysIssuerImport(t *testing.T) { require.False(t, existing) require.Equal(t, key1.PrivateKey, key1_ref1.PrivateKey) + // Make sure if we attempt to re-import the same private key, no import/updates occur. + // So the existing flag should be set to true and we do not update the existing Name field. key1_ref2, existing, err := importKey(ctx, s, key1.PrivateKey, "ignore-me") require.NoError(t, err) require.True(t, existing) @@ -121,6 +123,8 @@ func Test_KeysIssuerImport(t *testing.T) { require.Equal(t, key1_ref1.ID, issuer1_ref1.KeyID) require.Equal(t, "issuer1", issuer1_ref1.Name) + // Make sure if we attempt to re-import the same issuer, no import/updates occur. + // So the existing flag should be set to true and we do not update the existing Name field. issuer1_ref2, existing, err := importIssuer(ctx, s, issuer1.Certificate, "ignore-me") require.NoError(t, err) require.True(t, existing) @@ -135,6 +139,7 @@ func Test_KeysIssuerImport(t *testing.T) { err = writeKey(ctx, s, key2) require.NoError(t, err) + // Same double import tests as above, but make sure if the previous was created through writeIssuer not importIssuer. issuer2_ref, existing, err := importIssuer(ctx, s, issuer2.Certificate, "ignore-me") require.NoError(t, err) require.True(t, existing) @@ -143,6 +148,7 @@ func Test_KeysIssuerImport(t *testing.T) { require.Equal(t, "", issuer2_ref.Name) require.Equal(t, issuer2.KeyID, issuer2_ref.KeyID) + // Same double import tests as above, but make sure if the previous was created through writeKey not importKey. key2_ref, existing, err := importKey(ctx, s, key2.PrivateKey, "ignore-me") require.NoError(t, err) require.True(t, existing) diff --git a/builtin/logical/pki/util.go b/builtin/logical/pki/util.go index 4c524e1dd000..cabf9f0b5ada 100644 --- a/builtin/logical/pki/util.go +++ b/builtin/logical/pki/util.go @@ -174,7 +174,7 @@ func getKeyName(ctx context.Context, s logical.Storage, data *framework.FieldDat } func getIssuerRef(data *framework.FieldData) string { - return extractRef(data, "issuer_ref") + return extractRef(data, issuerRefParam) } func getKeyRef(data *framework.FieldData) string {