DRIVERS-2731 support named KMS providers #1492

kevinAlbs · 2024-01-11T13:51:20Z

Summary

This PR includes tests and documentation for the new feature: named KMS providers.

mongodb/mongo-c-driver#1509 is a reference implementation in the C driver.

Named KMS providers requires libmongocrypt with MONGOCRYPT-605. libmongocrypt binaries to test are available on the upload-all task.

Background

Previously supported KMS providers were only: aws, azure, gcp, kmip, and local. The KMS provider is now expanded to support name suffixes. (e.g. local:myname).

Named KMS providers enables more than one of each KMS provider type to be configured. Example: libmongocrypt can now support more than one local KMS provider configured:

kms_providers = {
    "local": {"key": local_kek1},        # Unnamed KMS provider.
    "local:myname": {"key": local_kek2 } # Named KMS provider with name "myname".
}

Named KMS providers is further described in DBX Scope: Support Named KMS Providers.

Tests

The following specification tests are added:

namedKMS.yml tests automatic encryption.
namedKMS-rewrapManyDataKey.yml tests key rewrapping. This was noted as a motivation in DRIVERS-2731.
namedKMS-explicit.yml tests explicit encryption. This introduces use of the encrypt and decrypt operations.
namedKMS-createDataKey.yml tests creating data keys with named KMS providers.

The Unified Test Format schema 1.18 is added to allow ~~additionalProperties~~ patternProperties in kmsProviders.

Prose Test 11 (KMS TLS Options Tests) is extended to test named KMS providers. This is motivated by necessary changes in the C driver to support named KMS providers in the KMSProvidersTLSOptions.

Tests refer to additional KMS providers: local:name1, aws:name1, gcp:name1, azure:name1, and kmip:name1.

The name1 KMS providers may be configured exactly as the unnamed KMS providers. I.e. aws:name1 is configured the same as aws.

To test configuring two KMS providers of the same type referring to distinct credentials, two more test KMS providers are defined: local:name2 and aws:name2.

This PR proposes not adding azure:name2, gcp:name2, kmip:name2 with distinct credentials. I expect there is little gained value in testing all external KMS providers. And it requires managing more accounts.

Test credentials for aws:name2 are available in AWS Secrets Manager under drivers/csfle. The aws:name2 account credentials are in FLE_AWS_KEY2 and FLE_AWS_SECRET2. See https://wiki.corp.mongodb.com/display/DRIVERS/Using+AWS+Secrets+Manager+to+Store+Testing+Secrets for more background on how the secrets are managed.

Please complete the following before merging:

Update changelog.
Make sure there are generated JSON files from the YAML test files.
Test changes in at least one language driver. Tested in C
[] Test these changes against all server versions and topologies (including standalone, replica set, sharded
clusters, and serverless). C does not run Client-Side Encryption tests with sharded or serverless

Additional properties are now allowed to support specifying named KMS providers

To test automatic encryption for named KMS

AWS and local are not the only KMS providers supported. Remove sentence. Specifics of KMS providers are described in the `kmsProviders` section.

… provider name

KMSProviderName was previously one of the strings "aws", "azure", "gcp", "local", or "kmip". The term "KMS provider" consists of the same set of strings, but also includes an optional name suffix. Use KMSProvider to match the term and avoid ambiguity with "KMS provider name".

Note that "aws" is a `KMS provider type`, not a `KMS provider`.

May help readers unfamiliar with typescript interfaces.

Named KMS providers are required to be non-empty. libmongocrypt will return the error: `empty name`

jmikola · 2024-01-18T15:35:40Z

source/client-side-encryption/client-side-encryption.rst

+      "^azure:[a-zA-Z0-9_]+?$"?: AzureKMSOptions;
+      "^gcp:[a-zA-Z0-9_]+?$"?: GCPKMSOptions;
+      "^local:[a-zA-Z0-9_]+?$"?: LocalKMSOptions;
+      "^kmip:[a-zA-Z0-9_]+?$"?: KMIPKMSOptions;


source/client-side-encryption/etc/test-templates/namedKMS.yml.template

source/client-side-encryption/tests/README.rst

source/unified-test-format/unified-test-format.rst

jmikola · 2024-01-18T15:45:43Z

source/unified-test-format/schema-1.18.json

+            "additionalProperties": false,
+            "patternProperties": {
+                "^aws:[a-zA-Z0-9_]+?$": {
+                    "$ref": "#/definitions/kmsProviders/$defs/AWSKMSOptions"


Since the named providers don't support placeholders, I think this may warrant different definitions. You can probably revert to inlined definitions for "aws", "gcp" and "azure".

You may not even need $defs for the "kmip" and "local" providers. If the "kmip" and "local" providers are identical, you can just tweak just their regex patterns to make the entire suffix optional (e.g. ^local(:[a-zA-Z0-9_]+)?$) so it will apply to both cases.

Since the named providers don't support placeholders, I think this may warrant different definitions. You can probably revert to inlined definitions for "aws", "gcp" and "azure".

I think the named and unnamed can use the same definition. The aws:name2 KMS provider uses placeholders, since it is expected to be filled in with credentials for a different AWS account.

you can just tweak just their regex patterns to make the entire suffix optional (e.g. ^local(:[a-zA-Z0-9_]+)?$) so it will apply to both cases.

I like that idea. Since the named providers also support placeholders, the named and unnamed properties have been combined.

Since the named providers also support placeholders, the named and unnamed properties have been combined

Ah, I mixed up support for placeholders with automatic credentials. The automatic credentials are what the named providers don't support.

source/unified-test-format/unified-test-format.rst

jmikola · 2024-01-18T16:31:14Z

source/unified-test-format/schema-1.18.json

+            "type": "object",
+            "additionalProperties": false,
+            "patternProperties": {
+                "^aws(:[a-zA-Z0-9_]+)?$": {


Noted that the named providers don't support automatic credentials, but I don't think we need to address that in the schema. If anything, allowing an empty object would be helpful if we want to test that a driver raises a runtime error.

ShaneHarvey · 2024-01-25T18:05:34Z

source/client-side-encryption/client-side-encryption.rst

@@ -545,6 +599,11 @@ When such a state is detected, libmongocrypt_ will call back to the driver by
 entering the ``MONGOCRYPT_CTX_NEED_KMS_CREDENTIALS`` state, upon which the
 driver should fill in the KMS options automatically.

+Automatic credentials are only supported for the KMS provider types ``aws``,
+``gcp``, and ``azure``. KMS providers containing a name (e.g. ``aws:myname``) do
+not support automatic credentials. Attempting to configure a KMS provider with a


Named KMS providers do not support "Automatic Credentials".

It would be very simple to add support for this in Python, why do we have this limitation?

And by this I mean that all providers of the same type share one set of on demand creds.

See Q1: Should on-demand KMS credentials be supported for named KMS providers?.

On-demand credentials are primarily intended to support passing credentials assigned in an environment. Supporting on-demand credentials for more than one KMS provider of the same type is not expected to be useful. Supporting on-demand KMS credentials would require added work in drivers inspecting the KMS providers (example in Java) when
obtaining credentials.

Seems fine to me since we can always add support later if needed. Could we include this rationale in the spec?

Added rationale.

ShaneHarvey · 2024-01-26T16:53:08Z

source/client-side-encryption/tests/unified/namedKMS-createDataKey.yml

+          "aws:name1": { accessKeyId: { $$placeholder: 1 }, secretAccessKey: { $$placeholder: 1 } }
+          "azure:name1": { tenantId: { $$placeholder: 1 }, clientId: { $$placeholder: 1 }, clientSecret: { $$placeholder: 1 } }
+          "gcp:name1": { email: { $$placeholder: 1 }, privateKey: { $$placeholder: 1 } }
+          "kmip:name1": { endpoint: { $$placeholder: 1 } }


To test this the driver test needs to also set the KMS TLS Options for "kmip:name1". Are we supposed to set that up automatically?

Yes. From the PR description:

The name1 KMS providers may be configured exactly as the unnamed KMS providers. I.e. aws:name1 is configured the same as aws.

The C driver implementation uses the same function to parse: https://github.com/mongodb/mongo-c-driver/pull/1509/files#diff-155d86e55a8ee5230e233a43f28e041a3e0c9c24ebf2403763c2220388fa2a10R1201

ShaneHarvey · 2024-01-26T17:25:21Z

source/client-side-encryption/tests/unified/namedKMS-rewrapManyDataKey.yml

+          "local:name1": { key: { $$placeholder: 1 } }
+          # Use a different local master key for `local:name2`.
+          "local:name2": { key: "local+name2+YUJCa1kxNkVyNUR1QURhZ2h2UzR2d2RrZzh0cFBwM3R6NmdWMDFBMUN3YkQ5aXRRMkhGRGdQV09wOGVNYUMxT2k3NjZKelhaQmRCZGJkTXVyZG9uSjFk" }
+          # Use a different AWS account to test wrapping between different AWS accounts.


How does this use a different AWS account if aws:name2 uses the same placeholders as aws:name1?

The test runner is expected to use different credentials for the aws:name2 provider.

See the PR description for information on the different AWS account:

Test credentials for aws:name2 are available in AWS Secrets Manager under drivers/client-side-encryption. The aws:name2 account credentials start with AWSNAME2_. See https://wiki.corp.mongodb.com/display/DRIVERS/Using+AWS+Secrets+Manager+to+Store+Testing+Secrets for more background on how the secrets are managed.

Thanks this makes sense now. Could you update the CSE test readme to mention the aws:name2 credentials and the kmip:name1 KMS TLS options mentioned in the PR description?

ShaneHarvey · 2024-01-26T17:27:53Z

source/client-side-encryption/tests/unified/namedKMS-rewrapManyDataKey.yml

+                    upsert: { $$unsetOrMatches: false }
+                writeConcern: { w: majority }
+
+  - description: "rewrap from aws:name1 to aws:name2"


This test fails in python with:

[2024/01/26 09:02:28.906] test/unified_format.py:1280: in _clientEncryptionOperation_rewrapManyDataKey [2024/01/26 09:02:28.906] data = target.rewrap_many_data_key(*args, **kwargs) [2024/01/26 09:02:28.906] pymongo/encryption.py:1057: in rewrap_many_data_key [2024/01/26 09:02:28.906] with _wrap_encryption_errors(): [2024/01/26 09:02:28.906] /opt/python/3.10/lib/python3.10/contextlib.py:153: in __exit__ [2024/01/26 09:02:28.906] self.gen.throw(typ, value, traceback) [2024/01/26 09:02:28.906] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ [2024/01/26 09:02:28.906] @contextlib.contextmanager [2024/01/26 09:02:28.906] def _wrap_encryption_errors() -> Iterator[None]: [2024/01/26 09:02:28.906] """Context manager to wrap encryption related errors.""" [2024/01/26 09:02:28.906] try: [2024/01/26 09:02:28.906] yield [2024/01/26 09:02:28.906] except BSONError: [2024/01/26 09:02:28.906] # BSON encoding/decoding errors are unrelated to encryption so [2024/01/26 09:02:28.906] # we should propagate them unchanged. [2024/01/26 09:02:28.906] raise [2024/01/26 09:02:28.906] except Exception as exc: [2024/01/26 09:02:28.906] > raise EncryptionError(exc) from None [2024/01/26 09:02:28.906] E pymongo.errors.EncryptionError: Error in KMS response. HTTP status=400. Response body= [2024/01/26 09:02:28.906] E {"__type":"AccessDeniedException","Message":"User: arn:aws:iam::579766882180:user/BUILD-7242 is not authorized to perform: kms:Encrypt on this resource because the resource does not exist in this Region, no resource-based policies allow access, or a resource-based policy explicitly denies access"}

https://spruce.mongodb.com/task/mongo_python_driver_tests_python_version_rhel8_test_encryption__platform~rhel8_auth_ssl~noauth_nossl_python_version~3.10_encryption~encryption_crypt_shared_test_5.0_replica_set_patch_41a131ea1c15ffa969a14ce1334ce19837dc226b_65b3e49a57e85a1e221c98f3_24_01_26_16_58_03?execution=0&sortBy=STATUS&sortDir=ASC

Do you know what the issue could be?

I expect this is due to aws:name2 being configured with the credentials from aws:name1. The aws:name1 account deliberately cannot access the key in the aws:name2 account to ensure rewrapping uses separate accounts.

See the PR description for steps to get the aws:name2 credentials:

Test credentials for aws:name2 are available in AWS Secrets Manager under drivers/client-side-encryption. The aws:name2 account credentials start with AWSNAME2_. See https://wiki.corp.mongodb.com/display/DRIVERS/Using+AWS+Secrets+Manager+to+Store+Testing+Secrets for more background on how the secrets are managed.

Thanks this makes sense now.

…d `aws:name2` and `aws:name1` are different AWS accounts.

ShaneHarvey · 2024-01-26T22:06:00Z

source/client-side-encryption/tests/README.rst

+               "privateKey": <set from environment>,
+               "endpoint": "127.0.0.1:9002"
+            },
+            "kmip:no_client_cert" {


missing a colon after the name here

ShaneHarvey · 2024-01-26T22:07:51Z

source/client-side-encryption/tests/README.rst

+               "privateKey": <set from environment>,
+               "endpoint": "127.0.0.1:9002"
+            },
+            "kmip:with_tls" {


missing a colon after the name here

ShaneHarvey · 2024-01-26T22:36:49Z

source/client-side-encryption/tests/README.rst

+
+Test credentials to KMS are located in "drivers/client-side-encryption".
+
+Test credentials to create environments are available in "drivers/gcpkms" and "drivers/azurekms".


We already have these credentials in the existing "drivers/csfle" vault. Should we remove these other vaults in favor of "drivers/csfle"?

Note I just added FLE_AWS_KEY2 and FLE_AWS_SECRET2 to "drivers/csfle" today (for aws:name2).

CC: @blink1073

Note: I used drivers/csfle in mongodb-labs/drivers-evergreen-tools#390

Updated to use drivers/csfle and scheduled drivers/client-side-encryption for deletion. Thank you for adding the secrets.

Should we remove these other vaults in favor of "drivers/csfle"?

I propose keeping drivers/gcpkms and drivers/azurekms separate. Those vaults are in use by Python and Go. I expect the Azure and GCP prose tests are run in a separate task group to manage the lifetime of the created VM. I do not see much value in combining the credentials.

I agree with keeping them separate

ShaneHarvey

LGTM! Tests pass in Python.

kevinAlbs added 20 commits January 11, 2024 09:26

copy schema-1.17.json to schema-1.18.json

5336765

allow additionalProperties in kmsProviders

ad007fb

add CSE unified tests to Makefile

e1302f0

use schema 1.18 in Makefile

fcd6b27

remove clientEncryptionOpts-kmsProviders-additionalProperties test

d2d5667

Additional properties are now allowed to support specifying named KMS providers

add spec test: namedKMS-createDataKey

371a7a3

add spec test: namedKMS-rewrapManyDataKey

ac30c25

add legacy spec test: namedKMS

67926c5

To test automatic encryption for named KMS

add spec test: namedKMS-explicit

e326cde

update prose test to test TLS options are applied to named providers

102b9a4

remove out-dated info

7c893bf

AWS and local are not the only KMS providers supported. Remove sentence. Specifics of KMS providers are described in the `kmsProviders` section.

add terms for KMS providers, KMS provider, KMS provider type, and KMS…

25cc0cb

… provider name

note which KMS providers support automatic credentials

aec3bdc

revise terminology in masterKey

2afe686

Note that "aws" is a `KMS provider type`, not a `KMS provider`.

describe KMSProviders interface

ec8fab5

May help readers unfamiliar with typescript interfaces.

update CHANGELOG, Last Modified, and Version

9ee8b89

add test rewrap from aws:name1 to aws:name2

94299ac

note location of test credentials

d86a573

note schema bump in Unified Test Format

d2f71e5

kevinAlbs force-pushed the D2731 branch from 796c66d to d2f71e5 Compare January 11, 2024 14:28

kevinAlbs marked this pull request as ready for review January 17, 2024 14:30

kevinAlbs requested review from a team as code owners January 17, 2024 14:30

kevinAlbs requested review from jmikola, katcharov and ShaneHarvey and removed request for a team and katcharov January 17, 2024 14:30

kevinAlbs added 2 commits January 18, 2024 09:33

remove inline named KMS TLS tests

bb8a824

quote "arn" strings

89564c9

kevinAlbs mentioned this pull request Jan 18, 2024

quote "arn" strings #1497

Merged

1 task

kevinAlbs added 3 commits January 18, 2024 09:50

fix credentials link (hopefully)

2a70f68

update changelog format

3e67af1

fix sphinx-build warning

3e3f03c

kevinAlbs requested a review from jmikola January 18, 2024 15:36

require non-empty name in regex

3886e82

Named KMS providers are required to be non-empty. libmongocrypt will return the error: `empty name`

jmikola reviewed Jan 18, 2024

View reviewed changes

kevinAlbs added 3 commits January 18, 2024 11:04

combine named and unnamed properties

2ace264

remove no-longer-needed $defs

7f7bc23

replace tabs with spaces

81071da

kevinAlbs requested a review from jmikola January 18, 2024 16:12

kevinAlbs added 2 commits January 18, 2024 11:13

fix typo

d5081bb

fix comment wording

42ceb71

jmikola approved these changes Jan 18, 2024

View reviewed changes

ShaneHarvey reviewed Jan 25, 2024

View reviewed changes

kevinAlbs requested a review from ShaneHarvey January 25, 2024 18:55

add rationale for no on-demand KMS credentials support for named KMS

d7a7e56

ShaneHarvey mentioned this pull request Jan 26, 2024

PYTHON-4112 Support named KMS providers mongodb/mongo-python-driver#1487

Merged

ShaneHarvey reviewed Jan 26, 2024

View reviewed changes

kevinAlbs requested a review from ShaneHarvey January 26, 2024 18:17

note unnamed and *:name1 KMS providers use the same credentials, an…

4e08d63

…d `aws:name2` and `aws:name1` are different AWS accounts.

adriandole approved these changes Jan 26, 2024

View reviewed changes

ShaneHarvey reviewed Jan 26, 2024

View reviewed changes

kevinAlbs added 2 commits January 29, 2024 15:27

add missing colons

51783e1

use drivers/csfle vault

ed43ad1

kevinAlbs requested a review from ShaneHarvey January 29, 2024 20:40

ShaneHarvey approved these changes Jan 29, 2024

View reviewed changes

kevinAlbs merged commit f2e4cf1 into mongodb:master Jan 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRIVERS-2731 support named KMS providers #1492

DRIVERS-2731 support named KMS providers #1492

kevinAlbs commented Jan 11, 2024 •

edited

Loading

jmikola Jan 18, 2024

jmikola Jan 18, 2024

kevinAlbs Jan 18, 2024

jmikola Jan 18, 2024

jmikola Jan 18, 2024

ShaneHarvey Jan 25, 2024 •

edited

Loading

kevinAlbs Jan 25, 2024

ShaneHarvey Jan 25, 2024

kevinAlbs Jan 25, 2024

ShaneHarvey Jan 26, 2024

kevinAlbs Jan 26, 2024

ShaneHarvey Jan 26, 2024

kevinAlbs Jan 26, 2024

ShaneHarvey Jan 26, 2024 •

edited

Loading

kevinAlbs Jan 26, 2024

ShaneHarvey Jan 26, 2024

kevinAlbs Jan 26, 2024

ShaneHarvey Jan 26, 2024

ShaneHarvey Jan 26, 2024

kevinAlbs Jan 29, 2024

ShaneHarvey Jan 26, 2024

kevinAlbs Jan 29, 2024

ShaneHarvey Jan 26, 2024 •

edited

Loading

ShaneHarvey Jan 26, 2024

blink1073 Jan 29, 2024

kevinAlbs Jan 29, 2024

blink1073 Jan 29, 2024

ShaneHarvey left a comment


		Test credentials to KMS are located in "drivers/client-side-encryption".

		Test credentials to create environments are available in "drivers/gcpkms" and "drivers/azurekms".

DRIVERS-2731 support named KMS providers #1492

DRIVERS-2731 support named KMS providers #1492

Conversation

kevinAlbs commented Jan 11, 2024 • edited Loading

Summary

Background

Tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShaneHarvey Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShaneHarvey Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShaneHarvey Jan 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShaneHarvey left a comment

Choose a reason for hiding this comment

kevinAlbs commented Jan 11, 2024 •

edited

Loading

ShaneHarvey Jan 25, 2024 •

edited

Loading

ShaneHarvey Jan 26, 2024 •

edited

Loading

ShaneHarvey Jan 26, 2024 •

edited

Loading