From b5c1e9440f841c68d5898464c1eaa07f6b5f4bc0 Mon Sep 17 00:00:00 2001 From: rculpepper Date: Thu, 28 Apr 2022 10:14:18 -0400 Subject: [PATCH 01/16] add import endpoint --- builtin/logical/transit/path_import.go | 147 +++++++++++++++++++++++++ sdk/helper/keysutil/lock_manager.go | 62 +++++++++++ sdk/helper/keysutil/policy.go | 101 +++++++++++++++++ 3 files changed, 310 insertions(+) create mode 100644 builtin/logical/transit/path_import.go diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go new file mode 100644 index 000000000000..35e0aaf6f38b --- /dev/null +++ b/builtin/logical/transit/path_import.go @@ -0,0 +1,147 @@ +package transit + +import ( + "context" + "crypto/rsa" + "crypto/sha256" + "encoding/base64" + "fmt" + + "github.com/google/tink/go/kwp/subtle" + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/helper/keysutil" + "github.com/hashicorp/vault/sdk/logical" + "strconv" +) + +const EncryptedKeyBytes = 512 + +func (b *backend) pathImport() *framework.Path { + return &framework.Path{ + Pattern: "keys/" + framework.GenericNameRegex("name") + "/import", + Fields: map[string]*framework.FieldSchema{ + "name": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "The name of the key", + }, + "type": { + Type: framework.TypeString, + Description: ` +The type of key being imported. Currently, "aes128-gcm96" (symmetric), "aes256-gcm96" (symmetric), "ecdsa-p256" +(asymmetric), "ecdsa-p384" (asymmetric), "ecdsa-p521" (asymmetric), "ed25519" (asymmetric), "rsa-2048" (asymmetric), "rsa-3072" +(asymmetric), "rsa-4096" (asymmetric) are supported. Defaults to "aes256-gcm96". +`, + }, + "allow_plaintext_backup": { + Type: framework.TypeBool, + Description: `Enables taking a backup of the named +key in plaintext format. Once set, +this cannot be disabled.`, + }, + "ciphertext": { + Type: framework.TypeString, + Description: `The base64-encoded ciphertext of the keys. The AES key should be encrypted using OAEP +with the wrapping key and then concatenated with the import key, wrapped by the AES key.`, + }, + "allow_rotation": { + Type: framework.TypeBool, + Description: "True if the imported key may be rotated within Vault; false otherwise.", + }, + }, + Callbacks: map[logical.Operation]framework.OperationFunc{ + logical.UpdateOperation: b.pathImportWrite, + }, + HelpSynopsis: pathImportWriteSyn, + HelpDescription: pathImportWriteDesc, + } +} + +func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + name := d.Get("name").(string) + keyType := d.Get("type").(string) + ciphertextString := d.Get("ciphertext").(string) + allowRotation := d.Get("allow_rotation").(bool) + + polReq := keysutil.PolicyRequest{ + Storage: req.Storage, + Name: name, + AllowImportedKeyRotation: allowRotation, + } + + switch keyType { + case "aes128-gcm96": + polReq.KeyType = keysutil.KeyType_AES128_GCM96 + case "aes256-gcm96": + polReq.KeyType = keysutil.KeyType_AES256_GCM96 + case "chacha20-poly1305": + polReq.KeyType = keysutil.KeyType_ChaCha20_Poly1305 + case "ecdsa-p256": + polReq.KeyType = keysutil.KeyType_ECDSA_P256 + case "ecdsa-p384": + polReq.KeyType = keysutil.KeyType_ECDSA_P384 + case "ecdsa-p521": + polReq.KeyType = keysutil.KeyType_ECDSA_P521 + case "ed25519": + polReq.KeyType = keysutil.KeyType_ED25519 + case "rsa-2048": + polReq.KeyType = keysutil.KeyType_RSA2048 + case "rsa-3072": + polReq.KeyType = keysutil.KeyType_RSA3072 + case "rsa-4096": + polReq.KeyType = keysutil.KeyType_RSA4096 + default: + return logical.ErrorResponse(fmt.Sprintf("unknown key type: %v", keyType)), logical.ErrInvalidRequest + } + + p, _, err := b.GetPolicy(ctx, polReq, b.GetRandomReader()) + if err != nil { + return nil, err + } + + if p != nil { + p.Unlock() + return nil, fmt.Errorf("the import path cannot overwrite an existing key; use import-version to rotate an imported key") + } + + ciphertext, err := base64.RawURLEncoding.DecodeString(ciphertextString) + if err != nil { + return nil, err + } + + wrappedAESKey := ciphertext[:EncryptedKeyBytes] + wrappedImportKey := ciphertext[EncryptedKeyBytes:] + + wrappingKey, err := getWrappingKey(ctx, req.Storage) + if err != nil { + return nil, err + } + if wrappingKey == nil { + return nil, fmt.Errorf("error importing key: wrapping key was nil") + } + + rsaKey := wrappingKey.Keys[strconv.Itoa(wrappingKey.LatestVersion)].RSAKey + aesKey, err := rsa.DecryptOAEP(sha256.New(), b.GetRandomReader(), rsaKey, wrappedAESKey, []byte{}) + if err != nil { + return nil, err + } + + kwp, err := subtle.NewKWP(aesKey) + if err != nil { + return nil, err + } + + importKey, err := kwp.Unwrap(wrappedImportKey) + if err != nil { + return nil, err + } + + err = b.lm.ImportPolicy(ctx, polReq, importKey, b.GetRandomReader()) + if err != nil { + return nil, err + } + + return nil, nil +} + +const pathImportWriteSyn = "Imports an externally-generated key into transit" +const pathImportWriteDesc = "" diff --git a/sdk/helper/keysutil/lock_manager.go b/sdk/helper/keysutil/lock_manager.go index a424ca9e05b4..7d46cee7556c 100644 --- a/sdk/helper/keysutil/lock_manager.go +++ b/sdk/helper/keysutil/lock_manager.go @@ -53,6 +53,9 @@ type PolicyRequest struct { // How frequently the key should automatically rotate AutoRotatePeriod time.Duration + + // AllowImportedKeyRotation indicates whether an imported key may be rotated by Vault + AllowImportedKeyRotation bool } type LockManager struct { @@ -439,6 +442,65 @@ func (lm *LockManager) GetPolicy(ctx context.Context, req PolicyRequest, rand io return } +// When the function returns, if caching was disabled, the Policy's lock must +// be unlocked when the caller is done (and it should not be re-locked). +func (lm *LockManager) ImportPolicy(ctx context.Context, req PolicyRequest, key []byte, rand io.Reader) error { + var p *Policy + var err error + var ok bool + var pRaw interface{} + + // Check if it's in our cache + if lm.useCache { + pRaw, ok = lm.cache.Load(req.Name) + } + if ok { + p = pRaw.(*Policy) + if atomic.LoadUint32(&p.deleted) == 1 { + return nil + } + } + + // We're not using the cache, or it wasn't found; get an exclusive lock. + // This ensures that any other process writing the actual storage will be + // finished before we load from storage. + lock := locksutil.LockForKey(lm.keyLocks, req.Name) + lock.Lock() + + // Load it from storage + p, err = lm.getPolicyFromStorage(ctx, req.Storage, req.Name) + if err != nil { + return err + } + + if p == nil { + p = &Policy{ + l: new(sync.RWMutex), + Name: req.Name, + Type: req.KeyType, + Derived: req.Derived, + Exportable: req.Exportable, + AllowPlaintextBackup: req.AllowPlaintextBackup, + AutoRotatePeriod: req.AutoRotatePeriod, + AllowImportedKeyRotation: req.AllowImportedKeyRotation, + Imported: true, + } + } + + err = p.Import(ctx, req.Storage, key, rand) + if err != nil { + return fmt.Errorf("error importing key: %s", err) + } + + if lm.useCache { + lm.cache.Store(req.Name, p) + } + + lock.Unlock() + + return nil +} + func (lm *LockManager) DeletePolicy(ctx context.Context, storage logical.Storage, name string) error { var p *Policy var err error diff --git a/sdk/helper/keysutil/policy.go b/sdk/helper/keysutil/policy.go index 59afa99fde23..1e256176e7ac 100644 --- a/sdk/helper/keysutil/policy.go +++ b/sdk/helper/keysutil/policy.go @@ -381,6 +381,13 @@ type Policy struct { // versionPrefixCache stores caches of version prefix strings and the split // version template. versionPrefixCache sync.Map + + // Imported indicates whether the key was generated by Vault or imported + // from an external source + Imported bool + + // AllowImportedKeyRotation indicates whether an imported key may be rotated by Vault + AllowImportedKeyRotation bool } func (p *Policy) Lock(exclusive bool) { @@ -1355,6 +1362,96 @@ func (p *Policy) VerifySignature(context, input []byte, hashAlgorithm HashType, } } +func (p *Policy) Import(ctx context.Context, storage logical.Storage, key []byte, randReader io.Reader) error { + now := time.Now() + entry := KeyEntry{ + CreationTime: now, + DeprecatedCreationTime: now.Unix(), + } + + hmacKey, err := uuid.GenerateRandomBytesWithReader(32, randReader) + if err != nil { + return err + } + entry.HMACKey = hmacKey + + if (p.Type == KeyType_AES128_GCM96 && len(key) != 16) || + ((p.Type == KeyType_AES256_GCM96 || p.Type == KeyType_ChaCha20_Poly1305) && len(key) != 32) { + return fmt.Errorf("invalid key size %d bytes for key type %s", len(key), p.Type) + } + + if p.Type == KeyType_AES128_GCM96 || p.Type == KeyType_AES256_GCM96 || p.Type == KeyType_ChaCha20_Poly1305 { + entry.Key = key + } else { + parsedPrivateKey, err := x509.ParsePKCS8PrivateKey(key) + if err != nil { + return fmt.Errorf("error parsing asymmetric key: %s", err) + } + + switch parsedPrivateKey.(type) { + case ecdsa.PrivateKey: + ecdsaKey := parsedPrivateKey.(ecdsa.PrivateKey) + curve := elliptic.P256() + if p.Type == KeyType_ECDSA_P384 { + curve = elliptic.P384() + } else if p.Type == KeyType_ECDSA_P521 { + curve = elliptic.P521() + } + + if ecdsaKey.Curve != curve { + return fmt.Errorf("key type mismatch: provided key uses %s, expected %s", ecdsaKey.Curve, curve) + } + + entry.EC_D = ecdsaKey.D + entry.EC_X = ecdsaKey.X + entry.EC_Y = ecdsaKey.Y + derBytes, err := x509.MarshalPKIXPublicKey(ecdsaKey.Public()) + if err != nil { + return errwrap.Wrapf("error marshaling public key: {{err}}", err) + } + pemBlock := &pem.Block{ + Type: "PUBLIC KEY", + Bytes: derBytes, + } + pemBytes := pem.EncodeToMemory(pemBlock) + if pemBytes == nil || len(pemBytes) == 0 { + return fmt.Errorf("error PEM-encoding public key") + } + entry.FormattedPublicKey = string(pemBytes) + case ed25519.PrivateKey: + privateKey := parsedPrivateKey.(ed25519.PrivateKey) + + entry.Key = privateKey + publicKey := privateKey.Public().(ed25519.PublicKey) + entry.FormattedPublicKey = base64.StdEncoding.EncodeToString(publicKey) + case *rsa.PrivateKey: + rsaKey := parsedPrivateKey.(*rsa.PrivateKey) + entry.RSAKey = rsaKey + default: + return fmt.Errorf("error parsing key: invalid key type %T", parsedPrivateKey) + } + } + + p.LatestVersion += 1 + + if p.Keys == nil { + // This is an initial key rotation when generating a new policy. We + // don't need to call migrate here because if we've called getPolicy to + // get the policy in the first place it will have been run. + p.Keys = keyEntryMap{} + } + p.Keys[strconv.Itoa(p.LatestVersion)] = entry + + // This ensures that with new key creations min decryption version is set + // to 1 rather than the int default of 0, since keys start at 1 (either + // fresh or after migration to the key map) + if p.MinDecryptionVersion == 0 { + p.MinDecryptionVersion = 1 + } + + return p.Persist(ctx, storage) +} + // Rotate rotates the policy and persists it to storage. // If the rotation partially fails, the policy state will be restored. func (p *Policy) Rotate(ctx context.Context, storage logical.Storage, randReader io.Reader) (retErr error) { @@ -1362,6 +1459,10 @@ func (p *Policy) Rotate(ctx context.Context, storage logical.Storage, randReader priorMinDecryptionVersion := p.MinDecryptionVersion var priorKeys keyEntryMap + if p.Imported && !p.AllowImportedKeyRotation { + return fmt.Errorf("imported key %s does not allow rotation within Vault", p.Name) + } + if p.Keys != nil { priorKeys = keyEntryMap{} for k, v := range p.Keys { From e4482b5e80b07bcf1900a123a9b4746421fcf5cf Mon Sep 17 00:00:00 2001 From: Rachel Culpepper Date: Thu, 28 Apr 2022 14:38:42 -0400 Subject: [PATCH 02/16] fix unlock --- builtin/logical/transit/path_import.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index 35e0aaf6f38b..357483f316a0 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -99,8 +99,10 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * } if p != nil { - p.Unlock() - return nil, fmt.Errorf("the import path cannot overwrite an existing key; use import-version to rotate an imported key") + if b.System().CachingDisabled() { + p.Unlock() + } + return nil, errors.New("the import path cannot overwrite an existing key; use import-version to rotate an imported key") } ciphertext, err := base64.RawURLEncoding.DecodeString(ciphertextString) From c8862677885529e81dffc6fc6a79cd128a02159a Mon Sep 17 00:00:00 2001 From: rculpepper Date: Thu, 28 Apr 2022 20:26:58 -0400 Subject: [PATCH 03/16] add import_version --- builtin/logical/transit/backend.go | 2 + builtin/logical/transit/path_import.go | 99 ++++++++++++++++--- .../logical/transit/path_import_version..go | 78 +++++++++++++++ builtin/logical/transit/path_keys.go | 5 + sdk/helper/keysutil/lock_manager.go | 2 - sdk/helper/keysutil/policy.go | 36 +++++-- 6 files changed, 200 insertions(+), 22 deletions(-) create mode 100644 builtin/logical/transit/path_import_version..go diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index f2454a80fa49..2c73a4bfe5de 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -47,6 +47,8 @@ func Backend(ctx context.Context, conf *logical.BackendConfig) (*backend, error) b.pathRotate(), b.pathRewrap(), b.pathWrappingKey(), + b.pathImport(), + b.pathImportVersion(), b.pathKeys(), b.pathListKeys(), b.pathExportKeys(), diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index 357483f316a0..ce04766d76ee 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -5,7 +5,9 @@ import ( "crypto/rsa" "crypto/sha256" "encoding/base64" + "errors" "fmt" + "time" "github.com/google/tink/go/kwp/subtle" "github.com/hashicorp/vault/sdk/framework" @@ -31,12 +33,6 @@ The type of key being imported. Currently, "aes128-gcm96" (symmetric), "aes256-g (asymmetric), "ecdsa-p384" (asymmetric), "ecdsa-p521" (asymmetric), "ed25519" (asymmetric), "rsa-2048" (asymmetric), "rsa-3072" (asymmetric), "rsa-4096" (asymmetric) are supported. Defaults to "aes256-gcm96". `, - }, - "allow_plaintext_backup": { - Type: framework.TypeBool, - Description: `Enables taking a backup of the named -key in plaintext format. Once set, -this cannot be disabled.`, }, "ciphertext": { Type: framework.TypeString, @@ -47,6 +43,58 @@ with the wrapping key and then concatenated with the import key, wrapped by the Type: framework.TypeBool, Description: "True if the imported key may be rotated within Vault; false otherwise.", }, + "derived": { + Type: framework.TypeBool, + Description: `Enables key derivation mode. This +allows for per-transaction unique +keys for encryption operations.`, + }, + + "convergent_encryption": { + Type: framework.TypeBool, + Description: `Whether to support convergent encryption. +This is only supported when using a key with +key derivation enabled and will require all +requests to carry both a context and 96-bit +(12-byte) nonce. The given nonce will be used +in place of a randomly generated nonce. As a +result, when the same context and nonce are +supplied, the same ciphertext is generated. It +is *very important* when using this mode that +you ensure that all nonces are unique for a +given context. Failing to do so will severely +impact the ciphertext's security.`, + }, + + "exportable": { + Type: framework.TypeBool, + Description: `Enables keys to be exportable. +This allows for all the valid keys +in the key ring to be exported.`, + }, + + "allow_plaintext_backup": { + Type: framework.TypeBool, + Description: `Enables taking a backup of the named +key in plaintext format. Once set, +this cannot be disabled.`, + }, + + "context": { + Type: framework.TypeString, + Description: `Base64 encoded context for key derivation. +When reading a key with key derivation enabled, +if the key type supports public keys, this will +return the public key for the given context.`, + }, + "auto_rotate_period": { + Type: framework.TypeDurationSecond, + Default: 0, + Description: `Amount of time the key should live before +being automatically rotated. A value of 0 +(default) disables automatic rotation for the +key.`, + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ logical.UpdateOperation: b.pathImportWrite, @@ -58,13 +106,27 @@ with the wrapping key and then concatenated with the import key, wrapped by the func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { name := d.Get("name").(string) + derived := d.Get("derived").(bool) + convergent := d.Get("convergent_encryption").(bool) keyType := d.Get("type").(string) + exportable := d.Get("exportable").(bool) + allowPlaintextBackup := d.Get("allow_plaintext_backup").(bool) + autoRotatePeriod := time.Second * time.Duration(d.Get("auto_rotate_period").(int)) ciphertextString := d.Get("ciphertext").(string) allowRotation := d.Get("allow_rotation").(bool) + if autoRotatePeriod > 0 && !allowRotation { + return nil, errors.New("allow_rotation must be set to true if auto-rotation is enabled") + } + polReq := keysutil.PolicyRequest{ Storage: req.Storage, Name: name, + Derived: derived, + Convergent: convergent, + Exportable: exportable, + AllowPlaintextBackup: allowPlaintextBackup, + AutoRotatePeriod: autoRotatePeriod, AllowImportedKeyRotation: allowRotation, } @@ -102,7 +164,7 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * if b.System().CachingDisabled() { p.Unlock() } - return nil, errors.New("the import path cannot overwrite an existing key; use import-version to rotate an imported key") + return nil, errors.New("the import path cannot be used with an existing key; use import-version to rotate an existing imported key") } ciphertext, err := base64.RawURLEncoding.DecodeString(ciphertextString) @@ -110,10 +172,24 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * return nil, err } + key, err := b.decryptImportedKey(ctx, req.Storage, ciphertext) + if err != nil { + return nil, err + } + + err = b.lm.ImportPolicy(ctx, polReq, key, b.GetRandomReader()) + if err != nil { + return nil, err + } + + return nil, nil +} + +func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storage, ciphertext []byte) ([]byte, error) { wrappedAESKey := ciphertext[:EncryptedKeyBytes] wrappedImportKey := ciphertext[EncryptedKeyBytes:] - wrappingKey, err := getWrappingKey(ctx, req.Storage) + wrappingKey, err := getWrappingKey(ctx, storage) if err != nil { return nil, err } @@ -137,12 +213,7 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * return nil, err } - err = b.lm.ImportPolicy(ctx, polReq, importKey, b.GetRandomReader()) - if err != nil { - return nil, err - } - - return nil, nil + return importKey, nil } const pathImportWriteSyn = "Imports an externally-generated key into transit" diff --git a/builtin/logical/transit/path_import_version..go b/builtin/logical/transit/path_import_version..go new file mode 100644 index 000000000000..33f8e4693506 --- /dev/null +++ b/builtin/logical/transit/path_import_version..go @@ -0,0 +1,78 @@ +package transit + +import ( + "context" + "encoding/base64" + "errors" + "fmt" + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/helper/keysutil" + "github.com/hashicorp/vault/sdk/logical" +) + +func (b *backend) pathImportVersion() *framework.Path { + return &framework.Path{ + Pattern: "keys/" + framework.GenericNameRegex("name") + "/import_version", + Fields: map[string]*framework.FieldSchema{ + "name": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "The name of the key", + }, + "ciphertext": { + Type: framework.TypeString, + Description: `The base64-encoded ciphertext of the keys. The AES key should be encrypted using OAEP +with the wrapping key and then concatenated with the import key, wrapped by the AES key.`, + }, + }, + Callbacks: map[logical.Operation]framework.OperationFunc{ + logical.UpdateOperation: b.pathImportVersionWrite, + }, + HelpSynopsis: pathImportVersionWriteSyn, + HelpDescription: pathImportVersionWriteDesc, + } +} + +func (b *backend) pathImportVersionWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + name := d.Get("name").(string) + ciphertextString := d.Get("ciphertext").(string) + + polReq := keysutil.PolicyRequest{ + Storage: req.Storage, + Name: name, + Upsert: false, + } + + p, _, err := b.GetPolicy(ctx, polReq, b.GetRandomReader()) + if err != nil { + return nil, err + } + if p == nil { + return nil, fmt.Errorf("no key found with name %s; to import a new key, use the import/ endpoint", name) + } + if !p.Imported { + return nil, errors.New("the import_version endpoint can only be used with an imported key") + } + if p.ConvergentEncryption { + return nil, errors.New("import_version cannot be used on keys with convergent encryption enabled") + } + + if !b.System().CachingDisabled() { + p.Lock(true) + } + defer p.Unlock() + + ciphertext, err := base64.RawURLEncoding.DecodeString(ciphertextString) + if err != nil { + return nil, err + } + importKey, err := b.decryptImportedKey(ctx, req.Storage, ciphertext) + err = p.Import(ctx, req.Storage, importKey, b.GetRandomReader()) + if err != nil { + return nil, err + } + + return nil, nil +} + +const pathImportVersionWriteSyn = "" +const pathImportVersionWriteDesc = "" diff --git a/builtin/logical/transit/path_keys.go b/builtin/logical/transit/path_keys.go index bcaf326c23dc..e64adb24a81c 100644 --- a/builtin/logical/transit/path_keys.go +++ b/builtin/logical/transit/path_keys.go @@ -239,9 +239,14 @@ func (b *backend) pathPolicyRead(ctx context.Context, req *logical.Request, d *f "supports_signing": p.Type.SigningSupported(), "supports_derivation": p.Type.DerivationSupported(), "auto_rotate_period": int64(p.AutoRotatePeriod.Seconds()), + "imported_key": p.Imported, }, } + if p.Imported { + resp.Data["imported_key_allow_rotation"] = p.AllowImportedKeyRotation + } + if p.BackupInfo != nil { resp.Data["backup_info"] = map[string]interface{}{ "time": p.BackupInfo.Time, diff --git a/sdk/helper/keysutil/lock_manager.go b/sdk/helper/keysutil/lock_manager.go index 7d46cee7556c..33b298041145 100644 --- a/sdk/helper/keysutil/lock_manager.go +++ b/sdk/helper/keysutil/lock_manager.go @@ -442,8 +442,6 @@ func (lm *LockManager) GetPolicy(ctx context.Context, req PolicyRequest, rand io return } -// When the function returns, if caching was disabled, the Policy's lock must -// be unlocked when the caller is done (and it should not be re-locked). func (lm *LockManager) ImportPolicy(ctx context.Context, req PolicyRequest, key []byte, rand io.Reader) error { var p *Policy var err error diff --git a/sdk/helper/keysutil/policy.go b/sdk/helper/keysutil/policy.go index 1e256176e7ac..07645521879d 100644 --- a/sdk/helper/keysutil/policy.go +++ b/sdk/helper/keysutil/policy.go @@ -1389,17 +1389,22 @@ func (p *Policy) Import(ctx context.Context, storage logical.Storage, key []byte } switch parsedPrivateKey.(type) { - case ecdsa.PrivateKey: - ecdsaKey := parsedPrivateKey.(ecdsa.PrivateKey) - curve := elliptic.P256() - if p.Type == KeyType_ECDSA_P384 { + case *ecdsa.PrivateKey: + ecdsaKey := parsedPrivateKey.(*ecdsa.PrivateKey) + var curve elliptic.Curve + if p.Type == KeyType_ECDSA_P256 { + curve = elliptic.P256() + } else if p.Type == KeyType_ECDSA_P384 { curve = elliptic.P384() } else if p.Type == KeyType_ECDSA_P521 { curve = elliptic.P521() } + if curve == nil { + return fmt.Errorf("invalid key type: expected %s, got %T", p.Type, parsedPrivateKey) + } if ecdsaKey.Curve != curve { - return fmt.Errorf("key type mismatch: provided key uses %s, expected %s", ecdsaKey.Curve, curve) + return fmt.Errorf("invalid curve: expected %s, got %s", p.Type, curve) } entry.EC_D = ecdsaKey.D @@ -1419,16 +1424,35 @@ func (p *Policy) Import(ctx context.Context, storage logical.Storage, key []byte } entry.FormattedPublicKey = string(pemBytes) case ed25519.PrivateKey: + if p.Type != KeyType_ED25519 { + return fmt.Errorf("invalid key type: expected %s, got %T", p.Type, parsedPrivateKey) + } privateKey := parsedPrivateKey.(ed25519.PrivateKey) entry.Key = privateKey publicKey := privateKey.Public().(ed25519.PublicKey) entry.FormattedPublicKey = base64.StdEncoding.EncodeToString(publicKey) case *rsa.PrivateKey: + var keyBits int + if p.Type == KeyType_RSA2048 { + keyBits = 2048 + } else if p.Type == KeyType_RSA3072 { + keyBits = 3072 + } else if p.Type == KeyType_RSA4096 { + keyBits = 4096 + } rsaKey := parsedPrivateKey.(*rsa.PrivateKey) + + if keyBits == 0 { + return fmt.Errorf("invalid key type: expected %s, got %T", p.Type, rsaKey) + } + if rsaKey.Size() != keyBits { + return fmt.Errorf("invalid key size: expected %s, got %d", p.Type, rsaKey.Size()) + } + entry.RSAKey = rsaKey default: - return fmt.Errorf("error parsing key: invalid key type %T", parsedPrivateKey) + return fmt.Errorf("invalid key type: expected %s, got %T", p.Type, parsedPrivateKey) } } From 423eff5be6486662e845fc02e101e9eafbe14d2b Mon Sep 17 00:00:00 2001 From: rculpepper Date: Mon, 2 May 2022 11:00:27 -0400 Subject: [PATCH 04/16] refactor import endpoints and add tests --- builtin/logical/transit/path_import.go | 67 +++++++++++ .../logical/transit/path_import_version..go | 78 ------------ sdk/helper/keysutil/policy.go | 38 +++--- sdk/helper/keysutil/policy_test.go | 112 ++++++++++++++++++ 4 files changed, 197 insertions(+), 98 deletions(-) delete mode 100644 builtin/logical/transit/path_import_version..go diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index ce04766d76ee..d1e34052f951 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -104,6 +104,28 @@ key.`, } } +func (b *backend) pathImportVersion() *framework.Path { + return &framework.Path{ + Pattern: "keys/" + framework.GenericNameRegex("name") + "/import_version", + Fields: map[string]*framework.FieldSchema{ + "name": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "The name of the key", + }, + "ciphertext": { + Type: framework.TypeString, + Description: `The base64-encoded ciphertext of the keys. The AES key should be encrypted using OAEP +with the wrapping key and then concatenated with the import key, wrapped by the AES key.`, + }, + }, + Callbacks: map[logical.Operation]framework.OperationFunc{ + logical.UpdateOperation: b.pathImportVersionWrite, + }, + HelpSynopsis: pathImportVersionWriteSyn, + HelpDescription: pathImportVersionWriteDesc, + } +} + func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { name := d.Get("name").(string) derived := d.Get("derived").(bool) @@ -185,6 +207,48 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * return nil, nil } +func (b *backend) pathImportVersionWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + name := d.Get("name").(string) + ciphertextString := d.Get("ciphertext").(string) + + polReq := keysutil.PolicyRequest{ + Storage: req.Storage, + Name: name, + Upsert: false, + } + + p, _, err := b.GetPolicy(ctx, polReq, b.GetRandomReader()) + if err != nil { + return nil, err + } + if p == nil { + return nil, fmt.Errorf("no key found with name %s; to import a new key, use the import/ endpoint", name) + } + if !p.Imported { + return nil, errors.New("the import_version endpoint can only be used with an imported key") + } + if p.ConvergentEncryption { + return nil, errors.New("import_version cannot be used on keys with convergent encryption enabled") + } + + if !b.System().CachingDisabled() { + p.Lock(true) + } + defer p.Unlock() + + ciphertext, err := base64.RawURLEncoding.DecodeString(ciphertextString) + if err != nil { + return nil, err + } + importKey, err := b.decryptImportedKey(ctx, req.Storage, ciphertext) + err = p.Import(ctx, req.Storage, importKey, b.GetRandomReader()) + if err != nil { + return nil, err + } + + return nil, nil +} + func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storage, ciphertext []byte) ([]byte, error) { wrappedAESKey := ciphertext[:EncryptedKeyBytes] wrappedImportKey := ciphertext[EncryptedKeyBytes:] @@ -218,3 +282,6 @@ func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storag const pathImportWriteSyn = "Imports an externally-generated key into transit" const pathImportWriteDesc = "" + +const pathImportVersionWriteSyn = "" +const pathImportVersionWriteDesc = "" diff --git a/builtin/logical/transit/path_import_version..go b/builtin/logical/transit/path_import_version..go deleted file mode 100644 index 33f8e4693506..000000000000 --- a/builtin/logical/transit/path_import_version..go +++ /dev/null @@ -1,78 +0,0 @@ -package transit - -import ( - "context" - "encoding/base64" - "errors" - "fmt" - "github.com/hashicorp/vault/sdk/framework" - "github.com/hashicorp/vault/sdk/helper/keysutil" - "github.com/hashicorp/vault/sdk/logical" -) - -func (b *backend) pathImportVersion() *framework.Path { - return &framework.Path{ - Pattern: "keys/" + framework.GenericNameRegex("name") + "/import_version", - Fields: map[string]*framework.FieldSchema{ - "name": &framework.FieldSchema{ - Type: framework.TypeString, - Description: "The name of the key", - }, - "ciphertext": { - Type: framework.TypeString, - Description: `The base64-encoded ciphertext of the keys. The AES key should be encrypted using OAEP -with the wrapping key and then concatenated with the import key, wrapped by the AES key.`, - }, - }, - Callbacks: map[logical.Operation]framework.OperationFunc{ - logical.UpdateOperation: b.pathImportVersionWrite, - }, - HelpSynopsis: pathImportVersionWriteSyn, - HelpDescription: pathImportVersionWriteDesc, - } -} - -func (b *backend) pathImportVersionWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - name := d.Get("name").(string) - ciphertextString := d.Get("ciphertext").(string) - - polReq := keysutil.PolicyRequest{ - Storage: req.Storage, - Name: name, - Upsert: false, - } - - p, _, err := b.GetPolicy(ctx, polReq, b.GetRandomReader()) - if err != nil { - return nil, err - } - if p == nil { - return nil, fmt.Errorf("no key found with name %s; to import a new key, use the import/ endpoint", name) - } - if !p.Imported { - return nil, errors.New("the import_version endpoint can only be used with an imported key") - } - if p.ConvergentEncryption { - return nil, errors.New("import_version cannot be used on keys with convergent encryption enabled") - } - - if !b.System().CachingDisabled() { - p.Lock(true) - } - defer p.Unlock() - - ciphertext, err := base64.RawURLEncoding.DecodeString(ciphertextString) - if err != nil { - return nil, err - } - importKey, err := b.decryptImportedKey(ctx, req.Storage, ciphertext) - err = p.Import(ctx, req.Storage, importKey, b.GetRandomReader()) - if err != nil { - return nil, err - } - - return nil, nil -} - -const pathImportVersionWriteSyn = "" -const pathImportVersionWriteDesc = "" diff --git a/sdk/helper/keysutil/policy.go b/sdk/helper/keysutil/policy.go index 07645521879d..59029756d2e1 100644 --- a/sdk/helper/keysutil/policy.go +++ b/sdk/helper/keysutil/policy.go @@ -1390,21 +1390,20 @@ func (p *Policy) Import(ctx context.Context, storage logical.Storage, key []byte switch parsedPrivateKey.(type) { case *ecdsa.PrivateKey: + if p.Type != KeyType_ECDSA_P256 && p.Type != KeyType_ECDSA_P384 && p.Type != KeyType_ECDSA_P521 { + return fmt.Errorf("invalid key type: expected %s, got %T", p.Type, parsedPrivateKey) + } + ecdsaKey := parsedPrivateKey.(*ecdsa.PrivateKey) - var curve elliptic.Curve - if p.Type == KeyType_ECDSA_P256 { - curve = elliptic.P256() - } else if p.Type == KeyType_ECDSA_P384 { + curve := elliptic.P256() + if p.Type == KeyType_ECDSA_P384 { curve = elliptic.P384() } else if p.Type == KeyType_ECDSA_P521 { curve = elliptic.P521() } - if curve == nil { - return fmt.Errorf("invalid key type: expected %s, got %T", p.Type, parsedPrivateKey) - } if ecdsaKey.Curve != curve { - return fmt.Errorf("invalid curve: expected %s, got %s", p.Type, curve) + return fmt.Errorf("invalid curve: expected %s, got %s", curve.Params().Name, ecdsaKey.Curve.Params().Name) } entry.EC_D = ecdsaKey.D @@ -1433,21 +1432,19 @@ func (p *Policy) Import(ctx context.Context, storage logical.Storage, key []byte publicKey := privateKey.Public().(ed25519.PublicKey) entry.FormattedPublicKey = base64.StdEncoding.EncodeToString(publicKey) case *rsa.PrivateKey: - var keyBits int - if p.Type == KeyType_RSA2048 { - keyBits = 2048 - } else if p.Type == KeyType_RSA3072 { - keyBits = 3072 - } else if p.Type == KeyType_RSA4096 { - keyBits = 4096 + if p.Type != KeyType_RSA2048 && p.Type != KeyType_RSA3072 && p.Type != KeyType_RSA4096 { + return fmt.Errorf("invalid key type: expected %s, got %T", p.Type, parsedPrivateKey) } - rsaKey := parsedPrivateKey.(*rsa.PrivateKey) - if keyBits == 0 { - return fmt.Errorf("invalid key type: expected %s, got %T", p.Type, rsaKey) + keyBytes := 256 + if p.Type == KeyType_RSA3072 { + keyBytes = 384 + } else if p.Type == KeyType_RSA4096 { + keyBytes = 512 } - if rsaKey.Size() != keyBits { - return fmt.Errorf("invalid key size: expected %s, got %d", p.Type, rsaKey.Size()) + rsaKey := parsedPrivateKey.(*rsa.PrivateKey) + if rsaKey.Size() != keyBytes { + return fmt.Errorf("invalid key size: expected %d bytes, got %d bytes", keyBytes, rsaKey.Size()) } entry.RSAKey = rsaKey @@ -1506,6 +1503,7 @@ func (p *Policy) Rotate(ctx context.Context, storage logical.Storage, randReader return err } + p.Imported = false return p.Persist(ctx, storage) } diff --git a/sdk/helper/keysutil/policy_test.go b/sdk/helper/keysutil/policy_test.go index e1ad6dde7c3d..d8212773641a 100644 --- a/sdk/helper/keysutil/policy_test.go +++ b/sdk/helper/keysutil/policy_test.go @@ -3,7 +3,12 @@ package keysutil import ( "bytes" "context" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" + "crypto/rsa" + "crypto/x509" + "golang.org/x/crypto/ed25519" "reflect" "strconv" "sync" @@ -615,6 +620,113 @@ func Test_BadArchive(t *testing.T) { } } +func Test_Import(t *testing.T) { + ctx := context.Background() + storage := &logical.InmemStorage{} + testKeys, err := generateTestKeys() + if err != nil { + t.Fatalf("error generating test keys: %s", err) + } + + tests := map[string]struct { + policy Policy + key []byte + shouldError bool + }{ + "import AES key": { + policy: Policy{ + Name: "test-aes-key", + Type: KeyType_AES256_GCM96, + }, + key: testKeys[KeyType_AES256_GCM96], + shouldError: false, + }, + "import RSA key": { + policy: Policy{ + Name: "test-rsa-key", + Type: KeyType_RSA2048, + }, + key: testKeys[KeyType_RSA2048], + shouldError: false, + }, + "import ECDSA key": { + policy: Policy{ + Name: "test-ecdsa-key", + Type: KeyType_ECDSA_P256, + }, + key: testKeys[KeyType_ECDSA_P256], + shouldError: false, + }, + "import ED25519 key": { + policy: Policy{ + Name: "test-ed25519-key", + Type: KeyType_ED25519, + }, + key: testKeys[KeyType_ED25519], + shouldError: false, + }, + "import incorrect key type": { + policy: Policy{ + Name: "test-ed25519-key", + Type: KeyType_ED25519, + }, + key: testKeys[KeyType_AES256_GCM96], + shouldError: true, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + if err := test.policy.Import(ctx, storage, test.key, rand.Reader); (err != nil) != test.shouldError { + t.Fatalf("error importing key: %s", err) + } + }) + } +} + +func generateTestKeys() (map[KeyType][]byte, error) { + keyMap := make(map[KeyType][]byte) + + rsaKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + return nil, err + } + rsaKeyBytes, err := x509.MarshalPKCS8PrivateKey(rsaKey) + if err != nil { + return nil, err + } + keyMap[KeyType_RSA2048] = rsaKeyBytes + + ecdsaKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + return nil, err + } + ecdsaKeyBytes, err := x509.MarshalPKCS8PrivateKey(ecdsaKey) + if err != nil { + return nil, err + } + keyMap[KeyType_ECDSA_P256] = ecdsaKeyBytes + + _, ed25519Key, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + return nil, err + } + ed25519KeyBytes, err := x509.MarshalPKCS8PrivateKey(ed25519Key) + if err != nil { + return nil, err + } + keyMap[KeyType_ED25519] = ed25519KeyBytes + + aesKey := make([]byte, 32) + _, err = rand.Read(aesKey) + if err != nil { + return nil, err + } + keyMap[KeyType_AES256_GCM96] = aesKey + + return keyMap, nil +} + func BenchmarkSymmetric(b *testing.B) { ctx := context.Background() lm, _ := NewLockManager(true, 0) From 450d3189962f93ecf3e3e55e77485b8c1ab79d6b Mon Sep 17 00:00:00 2001 From: rculpepper Date: Thu, 5 May 2022 16:34:46 -0400 Subject: [PATCH 05/16] add descriptions --- builtin/logical/transit/path_import.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index d1e34052f951..388450057fb7 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -280,8 +280,13 @@ func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storag return importKey, nil } -const pathImportWriteSyn = "Imports an externally-generated key into transit" -const pathImportWriteDesc = "" - -const pathImportVersionWriteSyn = "" -const pathImportVersionWriteDesc = "" +const pathImportWriteSyn = "Imports an externally-generated key into a new transit key" +const pathImportWriteDesc = "This path is used to import an externally-generated " + + "key into Vault. The import operation creates a new key and cannot be used to " + + "replace an existing key." + +const pathImportVersionWriteSyn = "Imports an externally-generated key into an " + + "existing imported key" +const pathImportVersionWriteDesc = "This path is used to import a new version of an " + + "externally-generated key into an existing import key. The import_version endpoint " + + "only supports importing key material into existing imported keys." From a70f4205555b8cab0f4e857c67dc391630c43134 Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Mon, 9 May 2022 15:49:30 -0500 Subject: [PATCH 06/16] Update dependencies to include tink for Transit import operations. Convert Transit wrapping key endpoint to use shared wrapping key retrieval method. Disallow import of convergent keys to Transit via BYOK process. --- builtin/logical/transit/path_import.go | 12 +++-- builtin/logical/transit/path_wrapping_key.go | 46 ++++++++++++-------- go.mod | 1 + go.sum | 4 ++ 4 files changed, 41 insertions(+), 22 deletions(-) diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index 388450057fb7..96e1cd505ac0 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -9,11 +9,12 @@ import ( "fmt" "time" + "strconv" + "github.com/google/tink/go/kwp/subtle" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/keysutil" "github.com/hashicorp/vault/sdk/logical" - "strconv" ) const EncryptedKeyBytes = 512 @@ -52,7 +53,8 @@ keys for encryption operations.`, "convergent_encryption": { Type: framework.TypeBool, - Description: `Whether to support convergent encryption. + Description: `This field is not currently supported for import operations! +Whether to support convergent encryption. This is only supported when using a key with key derivation enabled and will require all requests to carry both a context and 96-bit @@ -141,6 +143,10 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * return nil, errors.New("allow_rotation must be set to true if auto-rotation is enabled") } + if convergent { + return nil, errors.New("import cannot be used on keys with convergent encryption enabled") + } + polReq := keysutil.PolicyRequest{ Storage: req.Storage, Name: name, @@ -253,7 +259,7 @@ func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storag wrappedAESKey := ciphertext[:EncryptedKeyBytes] wrappedImportKey := ciphertext[EncryptedKeyBytes:] - wrappingKey, err := getWrappingKey(ctx, storage) + wrappingKey, err := b.getWrappingKey(ctx, storage) if err != nil { return nil, err } diff --git a/builtin/logical/transit/path_wrapping_key.go b/builtin/logical/transit/path_wrapping_key.go index e878fbfd2882..1a08318db339 100644 --- a/builtin/logical/transit/path_wrapping_key.go +++ b/builtin/logical/transit/path_wrapping_key.go @@ -26,28 +26,10 @@ func (b *backend) pathWrappingKey() *framework.Path { } func (b *backend) pathWrappingKeyRead(ctx context.Context, req *logical.Request, _ *framework.FieldData) (*logical.Response, error) { - polReq := keysutil.PolicyRequest{ - Upsert: true, - Storage: req.Storage, - Name: fmt.Sprintf("import/%s", WrappingKeyName), - KeyType: keysutil.KeyType_RSA4096, - Derived: false, - Convergent: false, - Exportable: false, - AllowPlaintextBackup: false, - AutoRotatePeriod: 0, - } - p, _, err := b.GetPolicy(ctx, polReq, b.GetRandomReader()) + p, err := b.getWrappingKey(ctx, req.Storage) if err != nil { return nil, err } - if p == nil { - return nil, fmt.Errorf("error retrieving wrapping key: returned policy was nil") - } - if b.System().CachingDisabled() { - p.Unlock() - } - wrappingKey := p.Keys[strconv.Itoa(p.LatestVersion)] derBytes, err := x509.MarshalPKIXPublicKey(wrappingKey.RSAKey.Public()) @@ -74,6 +56,32 @@ func (b *backend) pathWrappingKeyRead(ctx context.Context, req *logical.Request, return resp, nil } +func (b *backend) getWrappingKey(ctx context.Context, storage logical.Storage) (*keysutil.Policy, error) { + polReq := keysutil.PolicyRequest{ + Upsert: true, + Storage: storage, + Name: fmt.Sprintf("import/%s", WrappingKeyName), + KeyType: keysutil.KeyType_RSA4096, + Derived: false, + Convergent: false, + Exportable: false, + AllowPlaintextBackup: false, + AutoRotatePeriod: 0, + } + p, _, err := b.GetPolicy(ctx, polReq, b.GetRandomReader()) + if err != nil { + return nil, err + } + if p == nil { + return nil, fmt.Errorf("error retrieving wrapping key: returned policy was nil") + } + if b.System().CachingDisabled() { + p.Unlock() + } + + return p, nil +} + const ( pathWrappingKeyHelpSyn = "Returns the public key to use for wrapping imported keys" pathWrappingKeyHelpDesc = "This path is used to retrieve the RSA-4096 wrapping key " + diff --git a/go.mod b/go.mod index 82408f664737..f1df2ac77673 100644 --- a/go.mod +++ b/go.mod @@ -274,6 +274,7 @@ require ( github.com/google/go-querystring v1.1.0 // indirect github.com/google/gofuzz v1.1.0 // indirect github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect + github.com/google/tink/go v1.4.0 // indirect github.com/google/uuid v1.3.0 // indirect github.com/googleapis/gax-go/v2 v2.0.5 // indirect github.com/googleapis/gnostic v0.5.5 // indirect diff --git a/go.sum b/go.sum index 9e423dd0cb9a..234326cd63a3 100644 --- a/go.sum +++ b/go.sum @@ -214,6 +214,7 @@ github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgI github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a h1:idn718Q4B6AGu/h5Sxe66HYVdqdGu2l9Iebqhi/AEoA= github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY= github.com/aws/aws-sdk-go v1.15.11/go.mod h1:mFuSZ37Z9YOHbQEwBWztmVzqXrEkub65tZoCYDt7FT0= +github.com/aws/aws-sdk-go v1.25.39/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= github.com/aws/aws-sdk-go v1.25.41/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= github.com/aws/aws-sdk-go v1.30.27/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= github.com/aws/aws-sdk-go v1.37.19 h1:/xKHoSsYfH9qe16pJAHIjqTVpMM2DRSsEt8Ok1bzYiw= @@ -751,6 +752,8 @@ github.com/google/pprof v0.0.0-20200708004538-1a94d8640e99/go.mod h1:ZgVRPoUq/hf github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= +github.com/google/tink/go v1.4.0 h1:7Ihv6n6/0zPrm2GRAeeF408P9Y00HXC2J6tvUzgb2sg= +github.com/google/tink/go v1.4.0/go.mod h1:OdW+ACSIXwGiPOWJiRTdoKzStsnqo8ZOsTzchWLy2DY= github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= @@ -1654,6 +1657,7 @@ golang.org/x/crypto v0.0.0-20190611184440-5c40567a22f8/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190923035154-9ee001bba392/go.mod h1:/lpIB1dKB+9EgE3H3cr1v9wB50oz8l4C4h62xy7jSTY= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20191119213627-4f8c1d86b1ba/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20191206172530-e9b2fee46413/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200220183623-bac4c82f6975/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20200302210943-78000ba7a073/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= From 71da4e289795717fb1d7207f3687f30f5a112285 Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Tue, 10 May 2022 14:24:06 -0500 Subject: [PATCH 07/16] Include new 'hash_function' parameter on Transit import endpoints to specify OAEP random oracle hash function used to wrap ephemeral AES key. --- builtin/logical/transit/path_import.go | 48 ++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index 96e1cd505ac0..897f64f2c289 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -3,10 +3,13 @@ package transit import ( "context" "crypto/rsa" + "crypto/sha1" "crypto/sha256" + "crypto/sha512" "encoding/base64" "errors" "fmt" + "hash" "time" "strconv" @@ -29,11 +32,15 @@ func (b *backend) pathImport() *framework.Path { }, "type": { Type: framework.TypeString, - Description: ` -The type of key being imported. Currently, "aes128-gcm96" (symmetric), "aes256-gcm96" (symmetric), "ecdsa-p256" + Description: `The type of key being imported. Currently, "aes128-gcm96" (symmetric), "aes256-gcm96" (symmetric), "ecdsa-p256" (asymmetric), "ecdsa-p384" (asymmetric), "ecdsa-p521" (asymmetric), "ed25519" (asymmetric), "rsa-2048" (asymmetric), "rsa-3072" (asymmetric), "rsa-4096" (asymmetric) are supported. Defaults to "aes256-gcm96". `, + }, + "hash_function": { + Type: framework.TypeString, + Description: `The hash function used as a random oracle in the OAEP wrapping of the user-generated, +ephemeral AES key. Can be one of "SHA1", "SHA224", "SHA256" (default), "SHA384", or "SHA512"`, }, "ciphertext": { Type: framework.TypeString, @@ -133,6 +140,7 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * derived := d.Get("derived").(bool) convergent := d.Get("convergent_encryption").(bool) keyType := d.Get("type").(string) + hashFnStr := d.Get("hash_function").(string) exportable := d.Get("exportable").(bool) allowPlaintextBackup := d.Get("allow_plaintext_backup").(bool) autoRotatePeriod := time.Second * time.Duration(d.Get("auto_rotate_period").(int)) @@ -183,6 +191,11 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * return logical.ErrorResponse(fmt.Sprintf("unknown key type: %v", keyType)), logical.ErrInvalidRequest } + hashFn, err := parseHashFn(hashFnStr) + if err != nil { + return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest + } + p, _, err := b.GetPolicy(ctx, polReq, b.GetRandomReader()) if err != nil { return nil, err @@ -200,7 +213,7 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * return nil, err } - key, err := b.decryptImportedKey(ctx, req.Storage, ciphertext) + key, err := b.decryptImportedKey(ctx, req.Storage, ciphertext, hashFn) if err != nil { return nil, err } @@ -215,6 +228,7 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * func (b *backend) pathImportVersionWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { name := d.Get("name").(string) + hashFnStr := d.Get("hash_function").(string) ciphertextString := d.Get("ciphertext").(string) polReq := keysutil.PolicyRequest{ @@ -223,6 +237,11 @@ func (b *backend) pathImportVersionWrite(ctx context.Context, req *logical.Reque Upsert: false, } + hashFn, err := parseHashFn(hashFnStr) + if err != nil { + return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest + } + p, _, err := b.GetPolicy(ctx, polReq, b.GetRandomReader()) if err != nil { return nil, err @@ -246,7 +265,7 @@ func (b *backend) pathImportVersionWrite(ctx context.Context, req *logical.Reque if err != nil { return nil, err } - importKey, err := b.decryptImportedKey(ctx, req.Storage, ciphertext) + importKey, err := b.decryptImportedKey(ctx, req.Storage, ciphertext, hashFn) err = p.Import(ctx, req.Storage, importKey, b.GetRandomReader()) if err != nil { return nil, err @@ -255,7 +274,7 @@ func (b *backend) pathImportVersionWrite(ctx context.Context, req *logical.Reque return nil, nil } -func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storage, ciphertext []byte) ([]byte, error) { +func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storage, ciphertext []byte, hashFn hash.Hash) ([]byte, error) { wrappedAESKey := ciphertext[:EncryptedKeyBytes] wrappedImportKey := ciphertext[EncryptedKeyBytes:] @@ -268,7 +287,7 @@ func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storag } rsaKey := wrappingKey.Keys[strconv.Itoa(wrappingKey.LatestVersion)].RSAKey - aesKey, err := rsa.DecryptOAEP(sha256.New(), b.GetRandomReader(), rsaKey, wrappedAESKey, []byte{}) + aesKey, err := rsa.DecryptOAEP(hashFn, b.GetRandomReader(), rsaKey, wrappedAESKey, []byte{}) if err != nil { return nil, err } @@ -286,6 +305,23 @@ func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storag return importKey, nil } +func parseHashFn(hashFn string) (hash.Hash, error) { + switch hashFn { + case "sha1": + return sha1.New(), nil + case "sha224": + return sha256.New224(), nil + case "sha256": + return sha256.New(), nil + case "sha384": + return sha512.New384(), nil + case "sha512": + return sha512.New(), nil + default: + return nil, fmt.Errorf("unknown hash function: %s", hashFn) + } +} + const pathImportWriteSyn = "Imports an externally-generated key into a new transit key" const pathImportWriteDesc = "This path is used to import an externally-generated " + "key into Vault. The import operation creates a new key and cannot be used to " + From 1d808635c19c07dec616567e17f44012e60e054c Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Thu, 12 May 2022 12:43:14 -0500 Subject: [PATCH 08/16] Add default values for Transit import endpoint fields. Prevent an OOB panic in Transit import. Proactively zero out ephemeral AES key used in Transit imports. --- builtin/logical/transit/path_import.go | 33 +++++++++++++++++++------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index 897f64f2c289..5dc15b8b3e17 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "hash" + "strings" "time" "strconv" @@ -31,14 +32,16 @@ func (b *backend) pathImport() *framework.Path { Description: "The name of the key", }, "type": { - Type: framework.TypeString, + Type: framework.TypeString, + Default: "aes256-gcm96", Description: `The type of key being imported. Currently, "aes128-gcm96" (symmetric), "aes256-gcm96" (symmetric), "ecdsa-p256" (asymmetric), "ecdsa-p384" (asymmetric), "ecdsa-p521" (asymmetric), "ed25519" (asymmetric), "rsa-2048" (asymmetric), "rsa-3072" (asymmetric), "rsa-4096" (asymmetric) are supported. Defaults to "aes256-gcm96". `, }, "hash_function": { - Type: framework.TypeString, + Type: framework.TypeString, + Default: "SHA256", Description: `The hash function used as a random oracle in the OAEP wrapping of the user-generated, ephemeral AES key. Can be one of "SHA1", "SHA224", "SHA256" (default), "SHA384", or "SHA512"`, }, @@ -275,6 +278,11 @@ func (b *backend) pathImportVersionWrite(ctx context.Context, req *logical.Reque } func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storage, ciphertext []byte, hashFn hash.Hash) ([]byte, error) { + // Bounds check the ciphertext to avoid panics + if len(ciphertext) <= EncryptedKeyBytes { + return nil, errors.New("provided ciphertext is too short") + } + wrappedAESKey := ciphertext[:EncryptedKeyBytes] wrappedImportKey := ciphertext[EncryptedKeyBytes:] @@ -292,6 +300,15 @@ func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storag return nil, err } + // Zero out the ephemeral AES key just to be extra cautious. Note that this + // isn't a guarantee against memory analysis! See the documentation for the + // `vault.memzero` utility function for more information. + defer func() { + for i := range aesKey { + aesKey[i] = 0 + } + }() + kwp, err := subtle.NewKWP(aesKey) if err != nil { return nil, err @@ -306,16 +323,16 @@ func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storag } func parseHashFn(hashFn string) (hash.Hash, error) { - switch hashFn { - case "sha1": + switch strings.ToUpper(hashFn) { + case "SHA1": return sha1.New(), nil - case "sha224": + case "SHA224": return sha256.New224(), nil - case "sha256": + case "SHA256": return sha256.New(), nil - case "sha384": + case "SHA384": return sha512.New384(), nil - case "sha512": + case "SHA512": return sha512.New(), nil default: return nil, fmt.Errorf("unknown hash function: %s", hashFn) From 501a414554548a3773b9027509bbf52e57beeee7 Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Thu, 12 May 2022 13:57:19 -0500 Subject: [PATCH 09/16] Rename some Transit BYOK import variables. Ensure Transit BYOK ephemeral key is of the size specified byt the RFC. --- builtin/logical/transit/path_import.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index 5dc15b8b3e17..a44d3f36ea3e 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -283,7 +283,7 @@ func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storag return nil, errors.New("provided ciphertext is too short") } - wrappedAESKey := ciphertext[:EncryptedKeyBytes] + wrappedEphKey := ciphertext[:EncryptedKeyBytes] wrappedImportKey := ciphertext[EncryptedKeyBytes:] wrappingKey, err := b.getWrappingKey(ctx, storage) @@ -294,8 +294,8 @@ func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storag return nil, fmt.Errorf("error importing key: wrapping key was nil") } - rsaKey := wrappingKey.Keys[strconv.Itoa(wrappingKey.LatestVersion)].RSAKey - aesKey, err := rsa.DecryptOAEP(hashFn, b.GetRandomReader(), rsaKey, wrappedAESKey, []byte{}) + privWrappingKey := wrappingKey.Keys[strconv.Itoa(wrappingKey.LatestVersion)].RSAKey + ephKey, err := rsa.DecryptOAEP(hashFn, b.GetRandomReader(), privWrappingKey, wrappedEphKey, []byte{}) if err != nil { return nil, err } @@ -304,12 +304,17 @@ func (b *backend) decryptImportedKey(ctx context.Context, storage logical.Storag // isn't a guarantee against memory analysis! See the documentation for the // `vault.memzero` utility function for more information. defer func() { - for i := range aesKey { - aesKey[i] = 0 + for i := range ephKey { + ephKey[i] = 0 } }() - kwp, err := subtle.NewKWP(aesKey) + // Ensure the ephemeral AES key is 256-bit + if len(ephKey) != 32 { + return nil, errors.New("expected ephemeral AES key to be 256-bit") + } + + kwp, err := subtle.NewKWP(ephKey) if err != nil { return nil, err } From 4593cd94ddbe6c0659400ccfe6cbc6f8245e7740 Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Thu, 12 May 2022 13:57:52 -0500 Subject: [PATCH 10/16] Add unit tests for Transit BYOK import endpoint. --- builtin/logical/transit/path_import_test.go | 348 ++++++++++++++++++++ 1 file changed, 348 insertions(+) create mode 100644 builtin/logical/transit/path_import_test.go diff --git a/builtin/logical/transit/path_import_test.go b/builtin/logical/transit/path_import_test.go new file mode 100644 index 000000000000..904bea9591fc --- /dev/null +++ b/builtin/logical/transit/path_import_test.go @@ -0,0 +1,348 @@ +package transit + +import ( + "context" + "crypto/ecdsa" + "crypto/ed25519" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/base64" + "errors" + "fmt" + "strconv" + "testing" + + "github.com/google/tink/go/kwp/subtle" + uuid "github.com/hashicorp/go-uuid" + "github.com/hashicorp/vault/sdk/logical" +) + +func TestTransit_Import(t *testing.T) { + // Set up shared backend for subtests + b, s := createBackendWithStorage(t) + + t.Run( + "import into a key fails before wrapping key is read", + func(t *testing.T) { + fakeWrappingKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + t.Fatalf("failed to generate fake wrapping key: %s", err) + } + // Roll an AES256 key and import + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate key ID: %s", err) + } + targetKey, err := uuid.GenerateRandomBytes(32) + if err != nil { + t.Fatalf("failed to generate target key: %s", err) + } + importBlob, err := wrapTargetKeyForImport(&fakeWrappingKey.PublicKey, targetKey, "aes256-gcm96", "SHA256") + if err != nil { + t.Fatalf("failed to wrap target key for import: %s", err) + } + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import", keyID), + Data: map[string]interface{}{ + "ciphertext": importBlob, + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err == nil { + t.Fatal("import prior to wrapping key generation incorrectly succeeded") + } + }, + ) + + // Retrieve public wrapping key + wrappingKey, err := b.getWrappingKey(context.Background(), s) + if err != nil || wrappingKey == nil { + t.Fatalf("failed to retrieve public wrapping key: %s", err) + } + privWrappingKey := wrappingKey.Keys[strconv.Itoa(wrappingKey.LatestVersion)].RSAKey + pubWrappingKey := &privWrappingKey.PublicKey + + t.Run( + "import into an existing key fails", + func(t *testing.T) { + // Generate a key ID + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate a key ID: %s", err) + } + + // Create an AES256 key within Transit + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s", keyID), + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatalf("unexpected error creating key: %s", err) + } + + // Roll an AES256 key and import + targetKey, err := uuid.GenerateRandomBytes(32) + if err != nil { + t.Fatalf("failed to generate target key: %s", err) + } + importBlob, err := wrapTargetKeyForImport(pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") + if err != nil { + t.Fatalf("failed to wrap target key for import: %s", err) + } + req = &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import", keyID), + Data: map[string]interface{}{ + "ciphertext": importBlob, + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err == nil { + t.Fatal("import into an existing key incorrectly succeeded") + } + }, + ) + + // Check for all combinations of supported key type and hash function + keyTypes := []string{ + "aes256-gcm96", + "aes128-gcm96", + "chacha20-poly1305", + "ed25519", + "ecdsa-p256", + "ecdsa-p384", + "ecdsa-p521", + "rsa-2048", + "rsa-3072", + "rsa-4096", + } + hashFns := []string{ + "SHA256", + "SHA1", + "SHA224", + "SHA384", + "SHA512", + } + for _, keyType := range keyTypes { + priv, err := generateKey(keyType) + if err != nil { + t.Fatalf("failed to generate key: %s", err) + } + for _, hashFn := range hashFns { + t.Run( + fmt.Sprintf("%s/%s", keyType, hashFn), + func(t *testing.T) { + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate key ID: %s", err) + } + importBlob, err := wrapTargetKeyForImport(pubWrappingKey, priv, keyType, hashFn) + if err != nil { + t.Fatalf("failed to wrap target key for import: %s", err) + } + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import", keyID), + Data: map[string]interface{}{ + "type": keyType, + "hash_function": hashFn, + "ciphertext": importBlob, + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatalf("failed to import valid key: %s", err) + } + }, + ) + + // Shouldn't need to test every combination of key and hash function + if keyType != "aes256-gcm96" { + break + } + } + } + + failures := []struct { + name string + ciphertext interface{} + keyType interface{} + hashFn interface{} + }{ + { + name: "nil ciphertext", + }, + { + name: "empty string ciphertext", + ciphertext: "", + }, + { + name: "ciphertext not base64", + ciphertext: "this isn't correct", + }, + { + name: "ciphertext too short", + ciphertext: "ZmFrZSBjaXBoZXJ0ZXh0Cg", + }, + { + name: "invalid key type", + keyType: "fake-key-type", + }, + { + name: "invalid hash function", + hashFn: "fake-hash-fn", + }, + } + for _, tt := range failures { + t.Run( + tt.name, + func(t *testing.T) { + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate key ID: %s", err) + } + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import", keyID), + Data: map[string]interface{}{}, + } + if tt.ciphertext != nil { + req.Data["ciphertext"] = tt.ciphertext + } + if tt.keyType != nil { + req.Data["type"] = tt.keyType + } + if tt.hashFn != nil { + req.Data["hash_function"] = tt.hashFn + } + _, err = b.HandleRequest(context.Background(), req) + if err == nil { + t.Fatal("invalid import request incorrectly succeeded") + } + }, + ) + } + + t.Run( + "disallow import of convergent keys", + func(t *testing.T) { + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate key ID: %s", err) + } + targetKey, err := generateKey("aes256-gcm96") + if err != nil { + t.Fatalf("failed to generate key: %s", err) + } + importBlob, err := wrapTargetKeyForImport(pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") + if err != nil { + t.Fatalf("failed to wrap key: %s", err) + } + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import", keyID), + Data: map[string]interface{}{ + "convergent_encryption": true, + "ciphertext": importBlob, + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err == nil { + t.Fatal("import of convergent key incorrectly succeeded") + } + }, + ) +} + +func TestTransit_ImportVersion(t *testing.T) { +} + +func wrapTargetKeyForImport(wrappingKey *rsa.PublicKey, targetKey interface{}, targetKeyType string, hashFnName string) (string, error) { + // Generate an ephemeral AES-256 key + ephKey, err := uuid.GenerateRandomBytes(32) + if err != nil { + return "", err + } + + // Parse the hash function name into an actual function + hashFn, err := parseHashFn(hashFnName) + if err != nil { + return "", err + } + + // Wrap ephemeral AES key with public wrapping key + ephKeyWrapped, err := rsa.EncryptOAEP(hashFn, rand.Reader, wrappingKey, ephKey, []byte{}) + if err != nil { + return "", err + } + + // Create KWP instance for wrapping target key + kwp, err := subtle.NewKWP(ephKey) + if err != nil { + return "", err + } + + // Format target key for wrapping + var preppedTargetKey []byte + var ok bool + switch targetKeyType { + case "aes128-gcm96", "aes256-gcm96", "chacha20-poly1305": + preppedTargetKey, ok = targetKey.([]byte) + if !ok { + return "", errors.New("target key not provided in byte format") + } + default: + preppedTargetKey, err = x509.MarshalPKCS8PrivateKey(targetKey) + if err != nil { + return "", err + } + } + + // Wrap target key with KWP + targetKeyWrapped, err := kwp.Wrap(preppedTargetKey) + if err != nil { + return "", err + } + + // Combined wrapped keys into a single blob and base64 encode + wrappedKeys := append(ephKeyWrapped, targetKeyWrapped...) + return base64.RawURLEncoding.EncodeToString(wrappedKeys), nil +} + +func generateKey(keyType string) (interface{}, error) { + switch keyType { + case "aes128-gcm96": + return uuid.GenerateRandomBytes(16) + case "aes256-gcm96": + return uuid.GenerateRandomBytes(32) + case "chacha20-poly1305": + return uuid.GenerateRandomBytes(32) + case "ed25519": + _, priv, err := ed25519.GenerateKey(rand.Reader) + return priv, err + case "ecdsa-p256": + return ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + case "ecdsa-p384": + return ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + case "ecdsa-p521": + return ecdsa.GenerateKey(elliptic.P521(), rand.Reader) + case "rsa-2048": + return rsa.GenerateKey(rand.Reader, 2048) + case "rsa-3072": + return rsa.GenerateKey(rand.Reader, 3072) + case "rsa-4096": + return rsa.GenerateKey(rand.Reader, 4096) + default: + return nil, fmt.Errorf("failed to generate unsupported key type: %s", keyType) + } +} From bcee9ee00ca2443c48c0cdea2841a03ecff6894b Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Thu, 12 May 2022 15:35:59 -0500 Subject: [PATCH 11/16] Simplify Transit BYOK import tests. Add a conditional on auto rotation to avoid errors on BYOK keys with allow_rotation=false. --- builtin/logical/transit/backend.go | 5 + builtin/logical/transit/path_import_test.go | 150 ++++++++++++++++---- 2 files changed, 129 insertions(+), 26 deletions(-) diff --git a/builtin/logical/transit/backend.go b/builtin/logical/transit/backend.go index 2c73a4bfe5de..391b392c69b0 100644 --- a/builtin/logical/transit/backend.go +++ b/builtin/logical/transit/backend.go @@ -250,6 +250,11 @@ func (b *backend) rotateIfRequired(ctx context.Context, req *logical.Request, ke } defer p.Unlock() + // If the key is imported, it can only be rotated from within Vault if allowed. + if p.Imported && !p.AllowImportedKeyRotation { + return nil + } + // If the policy's automatic rotation period is 0, it should not // automatically rotate. if p.AutoRotatePeriod == 0 { diff --git a/builtin/logical/transit/path_import_test.go b/builtin/logical/transit/path_import_test.go index 904bea9591fc..089802e0ffcc 100644 --- a/builtin/logical/transit/path_import_test.go +++ b/builtin/logical/transit/path_import_test.go @@ -23,6 +23,36 @@ func TestTransit_Import(t *testing.T) { // Set up shared backend for subtests b, s := createBackendWithStorage(t) + keyTypes := []string{ + "aes256-gcm96", + "aes128-gcm96", + "chacha20-poly1305", + "ed25519", + "ecdsa-p256", + "ecdsa-p384", + "ecdsa-p521", + "rsa-2048", + "rsa-3072", + "rsa-4096", + } + hashFns := []string{ + "SHA256", + "SHA1", + "SHA224", + "SHA384", + "SHA512", + } + + // Generate a key for each type + keys := map[string]interface{}{} + for _, keyType := range keyTypes { + key, err := generateKey(keyType) + if err != nil { + t.Fatalf("failed to generate %s key: %s", keyType, err) + } + keys[keyType] = key + } + t.Run( "import into a key fails before wrapping key is read", func(t *testing.T) { @@ -110,30 +140,10 @@ func TestTransit_Import(t *testing.T) { }, ) - // Check for all combinations of supported key type and hash function - keyTypes := []string{ - "aes256-gcm96", - "aes128-gcm96", - "chacha20-poly1305", - "ed25519", - "ecdsa-p256", - "ecdsa-p384", - "ecdsa-p521", - "rsa-2048", - "rsa-3072", - "rsa-4096", - } - hashFns := []string{ - "SHA256", - "SHA1", - "SHA224", - "SHA384", - "SHA512", - } for _, keyType := range keyTypes { - priv, err := generateKey(keyType) - if err != nil { - t.Fatalf("failed to generate key: %s", err) + priv, ok := keys[keyType] + if !ok { + t.Fatalf("no pre-generated key for type: %s", keyType) } for _, hashFn := range hashFns { t.Run( @@ -239,9 +249,9 @@ func TestTransit_Import(t *testing.T) { if err != nil { t.Fatalf("failed to generate key ID: %s", err) } - targetKey, err := generateKey("aes256-gcm96") - if err != nil { - t.Fatalf("failed to generate key: %s", err) + targetKey, ok := keys["aes256-gcm96"] + if !ok { + t.Fatalf("no pre-generated key for type: aes256-gcm96") } importBlob, err := wrapTargetKeyForImport(pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") if err != nil { @@ -262,6 +272,94 @@ func TestTransit_Import(t *testing.T) { } }, ) + + t.Run( + "allow_rotation=true enables rotation within vault", + func(t *testing.T) { + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate key ID: %s", err) + } + targetKey, ok := keys["aes256-gcm96"] + if !ok { + t.Fatalf("no pre-generated key for type: aes256-gcm96") + } + + // Import key + importBlob, err := wrapTargetKeyForImport(pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") + if err != nil { + t.Fatalf("failed to wrap key: %s", err) + } + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import", keyID), + Data: map[string]interface{}{ + "allow_rotation": true, + "ciphertext": importBlob, + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatalf("failed to import key: %s", err) + } + + // Rotate key + req = &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/rotate", keyID), + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatalf("failed to rotate key: %s", err) + } + }, + ) + + t.Run( + "allow_rotation=false disables rotation within vault", + func(t *testing.T) { + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate key ID: %s", err) + } + targetKey, ok := keys["aes256-gcm96"] + if !ok { + t.Fatalf("no pre-generated key for type: aes256-gcm96") + } + + // Import key + importBlob, err := wrapTargetKeyForImport(pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") + if err != nil { + t.Fatalf("failed to wrap key: %s", err) + } + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import", keyID), + Data: map[string]interface{}{ + "allow_rotation": false, + "ciphertext": importBlob, + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatalf("failed to import key: %s", err) + } + + // Rotate key + req = &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/rotate", keyID), + } + _, err = b.HandleRequest(context.Background(), req) + if err == nil { + t.Fatal("rotation of key with allow_rotation incorrectly succeeded") + } + }, + ) } func TestTransit_ImportVersion(t *testing.T) { From 0b6ffbaccb16956ac567070bd5738628458111c9 Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Thu, 12 May 2022 16:47:02 -0500 Subject: [PATCH 12/16] Added hash_function field to Transit import_version endpoint. Reworked Transit import unit tests. Added unit tests for Transit import_version endpoint. --- builtin/logical/transit/path_import.go | 6 + builtin/logical/transit/path_import_test.go | 299 ++++++++++++++------ 2 files changed, 223 insertions(+), 82 deletions(-) diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index a44d3f36ea3e..93a8675fa401 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -129,6 +129,12 @@ func (b *backend) pathImportVersion() *framework.Path { Description: `The base64-encoded ciphertext of the keys. The AES key should be encrypted using OAEP with the wrapping key and then concatenated with the import key, wrapped by the AES key.`, }, + "hash_function": { + Type: framework.TypeString, + Default: "SHA256", + Description: `The hash function used as a random oracle in the OAEP wrapping of the user-generated, +ephemeral AES key. Can be one of "SHA1", "SHA224", "SHA256" (default), "SHA384", or "SHA512"`, + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ logical.UpdateOperation: b.pathImportVersionWrite, diff --git a/builtin/logical/transit/path_import_test.go b/builtin/logical/transit/path_import_test.go index 089802e0ffcc..f79c5b58bac5 100644 --- a/builtin/logical/transit/path_import_test.go +++ b/builtin/logical/transit/path_import_test.go @@ -9,9 +9,9 @@ import ( "crypto/rsa" "crypto/x509" "encoding/base64" - "errors" "fmt" "strconv" + "sync" "testing" "github.com/google/tink/go/kwp/subtle" @@ -19,32 +19,40 @@ import ( "github.com/hashicorp/vault/sdk/logical" ) -func TestTransit_Import(t *testing.T) { - // Set up shared backend for subtests - b, s := createBackendWithStorage(t) +var keyTypes = []string{ + "aes256-gcm96", + "aes128-gcm96", + "chacha20-poly1305", + "ed25519", + "ecdsa-p256", + "ecdsa-p384", + "ecdsa-p521", + "rsa-2048", + "rsa-3072", + "rsa-4096", +} - keyTypes := []string{ - "aes256-gcm96", - "aes128-gcm96", - "chacha20-poly1305", - "ed25519", - "ecdsa-p256", - "ecdsa-p384", - "ecdsa-p521", - "rsa-2048", - "rsa-3072", - "rsa-4096", - } - hashFns := []string{ - "SHA256", - "SHA1", - "SHA224", - "SHA384", - "SHA512", +var hashFns = []string{ + "SHA256", + "SHA1", + "SHA224", + "SHA384", + "SHA512", +} + +var keysLock sync.RWMutex +var keys = map[string]interface{}{} + +func generateKeys(t *testing.T) { + t.Helper() + + keysLock.Lock() + defer keysLock.Unlock() + + if len(keys) > 0 { + return } - // Generate a key for each type - keys := map[string]interface{}{} for _, keyType := range keyTypes { key, err := generateKey(keyType) if err != nil { @@ -52,6 +60,25 @@ func TestTransit_Import(t *testing.T) { } keys[keyType] = key } +} + +func getKey(t *testing.T, keyType string) interface{} { + t.Helper() + + keysLock.RLock() + defer keysLock.RUnlock() + + key, ok := keys[keyType] + if !ok { + t.Fatalf("no pre-generated key of type: %s", keyType) + } + + return key +} + +func TestTransit_Import(t *testing.T) { + generateKeys(t) + b, s := createBackendWithStorage(t) t.Run( "import into a key fails before wrapping key is read", @@ -65,14 +92,8 @@ func TestTransit_Import(t *testing.T) { if err != nil { t.Fatalf("failed to generate key ID: %s", err) } - targetKey, err := uuid.GenerateRandomBytes(32) - if err != nil { - t.Fatalf("failed to generate target key: %s", err) - } - importBlob, err := wrapTargetKeyForImport(&fakeWrappingKey.PublicKey, targetKey, "aes256-gcm96", "SHA256") - if err != nil { - t.Fatalf("failed to wrap target key for import: %s", err) - } + targetKey := getKey(t, "aes256-gcm96") + importBlob := wrapTargetKeyForImport(t, &fakeWrappingKey.PublicKey, targetKey, "aes256-gcm96", "SHA256") req := &logical.Request{ Storage: s, Operation: logical.UpdateOperation, @@ -116,15 +137,8 @@ func TestTransit_Import(t *testing.T) { t.Fatalf("unexpected error creating key: %s", err) } - // Roll an AES256 key and import - targetKey, err := uuid.GenerateRandomBytes(32) - if err != nil { - t.Fatalf("failed to generate target key: %s", err) - } - importBlob, err := wrapTargetKeyForImport(pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") - if err != nil { - t.Fatalf("failed to wrap target key for import: %s", err) - } + targetKey := getKey(t, "aes256-gcm96") + importBlob := wrapTargetKeyForImport(t, pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") req = &logical.Request{ Storage: s, Operation: logical.UpdateOperation, @@ -141,10 +155,7 @@ func TestTransit_Import(t *testing.T) { ) for _, keyType := range keyTypes { - priv, ok := keys[keyType] - if !ok { - t.Fatalf("no pre-generated key for type: %s", keyType) - } + priv := getKey(t, keyType) for _, hashFn := range hashFns { t.Run( fmt.Sprintf("%s/%s", keyType, hashFn), @@ -153,10 +164,7 @@ func TestTransit_Import(t *testing.T) { if err != nil { t.Fatalf("failed to generate key ID: %s", err) } - importBlob, err := wrapTargetKeyForImport(pubWrappingKey, priv, keyType, hashFn) - if err != nil { - t.Fatalf("failed to wrap target key for import: %s", err) - } + importBlob := wrapTargetKeyForImport(t, pubWrappingKey, priv, keyType, hashFn) req := &logical.Request{ Storage: s, Operation: logical.UpdateOperation, @@ -249,14 +257,8 @@ func TestTransit_Import(t *testing.T) { if err != nil { t.Fatalf("failed to generate key ID: %s", err) } - targetKey, ok := keys["aes256-gcm96"] - if !ok { - t.Fatalf("no pre-generated key for type: aes256-gcm96") - } - importBlob, err := wrapTargetKeyForImport(pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") - if err != nil { - t.Fatalf("failed to wrap key: %s", err) - } + targetKey := getKey(t, "aes256-gcm96") + importBlob := wrapTargetKeyForImport(t, pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") req := &logical.Request{ Storage: s, Operation: logical.UpdateOperation, @@ -280,16 +282,10 @@ func TestTransit_Import(t *testing.T) { if err != nil { t.Fatalf("failed to generate key ID: %s", err) } - targetKey, ok := keys["aes256-gcm96"] - if !ok { - t.Fatalf("no pre-generated key for type: aes256-gcm96") - } + targetKey := getKey(t, "aes256-gcm96") // Import key - importBlob, err := wrapTargetKeyForImport(pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") - if err != nil { - t.Fatalf("failed to wrap key: %s", err) - } + importBlob := wrapTargetKeyForImport(t, pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") req := &logical.Request{ Storage: s, Operation: logical.UpdateOperation, @@ -324,16 +320,10 @@ func TestTransit_Import(t *testing.T) { if err != nil { t.Fatalf("failed to generate key ID: %s", err) } - targetKey, ok := keys["aes256-gcm96"] - if !ok { - t.Fatalf("no pre-generated key for type: aes256-gcm96") - } + targetKey := getKey(t, "aes256-gcm96") // Import key - importBlob, err := wrapTargetKeyForImport(pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") - if err != nil { - t.Fatalf("failed to wrap key: %s", err) - } + importBlob := wrapTargetKeyForImport(t, pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") req := &logical.Request{ Storage: s, Operation: logical.UpdateOperation, @@ -363,31 +353,176 @@ func TestTransit_Import(t *testing.T) { } func TestTransit_ImportVersion(t *testing.T) { + generateKeys(t) + b, s := createBackendWithStorage(t) + + t.Run( + "import into a key version fails before wrapping key is read", + func(t *testing.T) { + fakeWrappingKey, err := rsa.GenerateKey(rand.Reader, 4096) + if err != nil { + t.Fatalf("failed to generate fake wrapping key: %s", err) + } + // Roll an AES256 key and import + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate key ID: %s", err) + } + targetKey := getKey(t, "aes256-gcm96") + importBlob := wrapTargetKeyForImport(t, &fakeWrappingKey.PublicKey, targetKey, "aes256-gcm96", "SHA256") + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import_version", keyID), + Data: map[string]interface{}{ + "ciphertext": importBlob, + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err == nil { + t.Fatal("import_version prior to wrapping key generation incorrectly succeeded") + } + }, + ) + + // Retrieve public wrapping key + wrappingKey, err := b.getWrappingKey(context.Background(), s) + if err != nil || wrappingKey == nil { + t.Fatalf("failed to retrieve public wrapping key: %s", err) + } + privWrappingKey := wrappingKey.Keys[strconv.Itoa(wrappingKey.LatestVersion)].RSAKey + pubWrappingKey := &privWrappingKey.PublicKey + + t.Run( + "import into a non-existent key fails", + func(t *testing.T) { + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate key ID: %s", err) + } + targetKey := getKey(t, "aes256-gcm96") + importBlob := wrapTargetKeyForImport(t, pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import_version", keyID), + Data: map[string]interface{}{ + "ciphertext": importBlob, + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err == nil { + t.Fatal("import_version into a non-existent key incorrectly succeeded") + } + }, + ) + + t.Run( + "import into an internally-generated key fails", + func(t *testing.T) { + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate key ID: %s", err) + } + + // Roll a key within Transit + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s", keyID), + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatalf("failed to generate a key within transit: %s", err) + } + + // Attempt to import into newly generated key + targetKey := getKey(t, "aes256-gcm96") + importBlob := wrapTargetKeyForImport(t, pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") + req = &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import_version", keyID), + Data: map[string]interface{}{ + "ciphertext": importBlob, + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err == nil { + t.Fatal("import_version into an internally-generated key incorrectly succeeded") + } + }, + ) + + t.Run( + "imported key version type must match existing key type", + func(t *testing.T) { + keyID, err := uuid.GenerateUUID() + if err != nil { + t.Fatalf("failed to generate key ID: %s", err) + } + + // Import an RSA key + targetKey := getKey(t, "rsa-2048") + importBlob := wrapTargetKeyForImport(t, pubWrappingKey, targetKey, "rsa-2048", "SHA256") + req := &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import", keyID), + Data: map[string]interface{}{ + "ciphertext": importBlob, + "type": "rsa-2048", + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatalf("failed to generate a key within transit: %s", err) + } + + // Attempt to import an AES key version into existing RSA key + targetKey = getKey(t, "aes256-gcm96") + importBlob = wrapTargetKeyForImport(t, pubWrappingKey, targetKey, "aes256-gcm96", "SHA256") + req = &logical.Request{ + Storage: s, + Operation: logical.UpdateOperation, + Path: fmt.Sprintf("keys/%s/import_version", keyID), + Data: map[string]interface{}{ + "ciphertext": importBlob, + }, + } + _, err = b.HandleRequest(context.Background(), req) + if err == nil { + t.Fatal("import_version into a key of a different type incorrectly succeeded") + } + }, + ) } -func wrapTargetKeyForImport(wrappingKey *rsa.PublicKey, targetKey interface{}, targetKeyType string, hashFnName string) (string, error) { +func wrapTargetKeyForImport(t *testing.T, wrappingKey *rsa.PublicKey, targetKey interface{}, targetKeyType string, hashFnName string) string { + t.Helper() + // Generate an ephemeral AES-256 key ephKey, err := uuid.GenerateRandomBytes(32) if err != nil { - return "", err + t.Fatalf("failed to wrap target key for import: %s", err) } // Parse the hash function name into an actual function hashFn, err := parseHashFn(hashFnName) if err != nil { - return "", err + t.Fatalf("failed to wrap target key for import: %s", err) } // Wrap ephemeral AES key with public wrapping key ephKeyWrapped, err := rsa.EncryptOAEP(hashFn, rand.Reader, wrappingKey, ephKey, []byte{}) if err != nil { - return "", err + t.Fatalf("failed to wrap target key for import: %s", err) } // Create KWP instance for wrapping target key kwp, err := subtle.NewKWP(ephKey) if err != nil { - return "", err + t.Fatalf("failed to wrap target key for import: %s", err) } // Format target key for wrapping @@ -397,24 +532,24 @@ func wrapTargetKeyForImport(wrappingKey *rsa.PublicKey, targetKey interface{}, t case "aes128-gcm96", "aes256-gcm96", "chacha20-poly1305": preppedTargetKey, ok = targetKey.([]byte) if !ok { - return "", errors.New("target key not provided in byte format") + t.Fatal("failed to wrap target key for import: symmetric key not provided in byte format") } default: preppedTargetKey, err = x509.MarshalPKCS8PrivateKey(targetKey) if err != nil { - return "", err + t.Fatalf("failed to wrap target key for import: %s", err) } } // Wrap target key with KWP targetKeyWrapped, err := kwp.Wrap(preppedTargetKey) if err != nil { - return "", err + t.Fatalf("failed to wrap target key for import: %s", err) } // Combined wrapped keys into a single blob and base64 encode wrappedKeys := append(ephKeyWrapped, targetKeyWrapped...) - return base64.RawURLEncoding.EncodeToString(wrappedKeys), nil + return base64.RawURLEncoding.EncodeToString(wrappedKeys) } func generateKey(keyType string) (interface{}, error) { From 41d6bc599e834a0d7eaa77ba5525cc70b7b3a61a Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Fri, 13 May 2022 10:39:44 -0500 Subject: [PATCH 13/16] Add changelog entry for Transit BYOK. --- changelog/15414.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/15414.txt diff --git a/changelog/15414.txt b/changelog/15414.txt new file mode 100644 index 000000000000..de6372bfdf1a --- /dev/null +++ b/changelog/15414.txt @@ -0,0 +1,3 @@ +```release-note:feature +**Transit BYOK**: Allow import of externally-generated keys into the Transit secrets engine. +``` From e05a5fb14a9a432e8d8f50af7ef0d61321ffcafe Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Fri, 13 May 2022 13:03:51 -0500 Subject: [PATCH 14/16] Transit BYOK formatting fixes. --- builtin/logical/transit/path_import.go | 17 +++++++++-------- builtin/logical/transit/path_import_test.go | 6 ++++-- sdk/helper/keysutil/policy_test.go | 3 ++- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index 93a8675fa401..d9dffe893a8c 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -10,11 +10,10 @@ import ( "errors" "fmt" "hash" + "strconv" "strings" "time" - "strconv" - "github.com/google/tink/go/kwp/subtle" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/helper/keysutil" @@ -27,7 +26,7 @@ func (b *backend) pathImport() *framework.Path { return &framework.Path{ Pattern: "keys/" + framework.GenericNameRegex("name") + "/import", Fields: map[string]*framework.FieldSchema{ - "name": &framework.FieldSchema{ + "name": { Type: framework.TypeString, Description: "The name of the key", }, @@ -120,7 +119,7 @@ func (b *backend) pathImportVersion() *framework.Path { return &framework.Path{ Pattern: "keys/" + framework.GenericNameRegex("name") + "/import_version", Fields: map[string]*framework.FieldSchema{ - "name": &framework.FieldSchema{ + "name": { Type: framework.TypeString, Description: "The name of the key", }, @@ -350,10 +349,12 @@ func parseHashFn(hashFn string) (hash.Hash, error) { } } -const pathImportWriteSyn = "Imports an externally-generated key into a new transit key" -const pathImportWriteDesc = "This path is used to import an externally-generated " + - "key into Vault. The import operation creates a new key and cannot be used to " + - "replace an existing key." +const ( + pathImportWriteSyn = "Imports an externally-generated key into a new transit key" + pathImportWriteDesc = "This path is used to import an externally-generated " + + "key into Vault. The import operation creates a new key and cannot be used to " + + "replace an existing key." +) const pathImportVersionWriteSyn = "Imports an externally-generated key into an " + "existing imported key" diff --git a/builtin/logical/transit/path_import_test.go b/builtin/logical/transit/path_import_test.go index f79c5b58bac5..ce3b82abcdc0 100644 --- a/builtin/logical/transit/path_import_test.go +++ b/builtin/logical/transit/path_import_test.go @@ -40,8 +40,10 @@ var hashFns = []string{ "SHA512", } -var keysLock sync.RWMutex -var keys = map[string]interface{}{} +var ( + keysLock sync.RWMutex + keys = map[string]interface{}{} +) func generateKeys(t *testing.T) { t.Helper() diff --git a/sdk/helper/keysutil/policy_test.go b/sdk/helper/keysutil/policy_test.go index d8212773641a..a2d9206a8aca 100644 --- a/sdk/helper/keysutil/policy_test.go +++ b/sdk/helper/keysutil/policy_test.go @@ -8,13 +8,14 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" - "golang.org/x/crypto/ed25519" "reflect" "strconv" "sync" "testing" "time" + "golang.org/x/crypto/ed25519" + "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/logical" "github.com/mitchellh/copystructure" From d8d310a215bf43f46e1273ce63730e552f7c7f27 Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Fri, 13 May 2022 14:16:27 -0500 Subject: [PATCH 15/16] Omit 'convergent_encryption' field from Transit BYOK import endpoint, but reject with an error when the field is provided. --- builtin/logical/transit/path_import.go | 28 +++++--------------------- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index d9dffe893a8c..8446a53e5442 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -60,23 +60,6 @@ allows for per-transaction unique keys for encryption operations.`, }, - "convergent_encryption": { - Type: framework.TypeBool, - Description: `This field is not currently supported for import operations! -Whether to support convergent encryption. -This is only supported when using a key with -key derivation enabled and will require all -requests to carry both a context and 96-bit -(12-byte) nonce. The given nonce will be used -in place of a randomly generated nonce. As a -result, when the same context and nonce are -supplied, the same ciphertext is generated. It -is *very important* when using this mode that -you ensure that all nonces are unique for a -given context. Failing to do so will severely -impact the ciphertext's security.`, - }, - "exportable": { Type: framework.TypeBool, Description: `Enables keys to be exportable. @@ -146,7 +129,6 @@ ephemeral AES key. Can be one of "SHA1", "SHA224", "SHA256" (default), "SHA384", func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { name := d.Get("name").(string) derived := d.Get("derived").(bool) - convergent := d.Get("convergent_encryption").(bool) keyType := d.Get("type").(string) hashFnStr := d.Get("hash_function").(string) exportable := d.Get("exportable").(bool) @@ -155,19 +137,19 @@ func (b *backend) pathImportWrite(ctx context.Context, req *logical.Request, d * ciphertextString := d.Get("ciphertext").(string) allowRotation := d.Get("allow_rotation").(bool) - if autoRotatePeriod > 0 && !allowRotation { - return nil, errors.New("allow_rotation must be set to true if auto-rotation is enabled") + // Ensure the caller didn't supply "convergent_encryption" as a field, since it's not supported on import. + if _, ok := d.Raw["convergent_encryption"]; ok { + return nil, errors.New("import cannot be used on keys with convergent encryption enabled") } - if convergent { - return nil, errors.New("import cannot be used on keys with convergent encryption enabled") + if autoRotatePeriod > 0 && !allowRotation { + return nil, errors.New("allow_rotation must be set to true if auto-rotation is enabled") } polReq := keysutil.PolicyRequest{ Storage: req.Storage, Name: name, Derived: derived, - Convergent: convergent, Exportable: exportable, AllowPlaintextBackup: allowPlaintextBackup, AutoRotatePeriod: autoRotatePeriod, From 592853103b66c91bfca935a805ef10849627c107 Mon Sep 17 00:00:00 2001 From: Matt Schultz Date: Fri, 13 May 2022 15:00:56 -0500 Subject: [PATCH 16/16] Minor formatting fix in Transit import. --- builtin/logical/transit/path_import.go | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/logical/transit/path_import.go b/builtin/logical/transit/path_import.go index 8446a53e5442..0f86ea3efd7f 100644 --- a/builtin/logical/transit/path_import.go +++ b/builtin/logical/transit/path_import.go @@ -340,6 +340,7 @@ const ( const pathImportVersionWriteSyn = "Imports an externally-generated key into an " + "existing imported key" + const pathImportVersionWriteDesc = "This path is used to import a new version of an " + "externally-generated key into an existing import key. The import_version endpoint " + "only supports importing key material into existing imported keys."