Skip to content

[CDRIVER-4568] Limit options available for createEncryptedCollection #1218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 29, 2023
Merged

[CDRIVER-4568] Limit options available for createEncryptedCollection #1218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
168c1da
Modify create-encrypted-collection to only accept a masterKey option
vector-of-bool Feb 23, 2023
9621f0d
Parameterise CEC tests:
vector-of-bool Mar 13, 2023
4e098c0
Merge branch 'master' into CDRIVER-4568-cec-no-keyAlt
vector-of-bool Mar 13, 2023
77d9dba
Use existing KMS conveniences
vector-of-bool Mar 13, 2023
c27c438
Remove comment
vector-of-bool Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/libmongoc/doc/mongoc_client_encryption_create_encrypted_collection.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Synopsis
const bson_t *in_options,
bson_t *out_options,
const char *kms_provider,
const mongoc_client_encryption_datakey_opts_t *dk_opts,
const bson_t *opt_masterKey,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding a way to extend options. When this API becomes stable, it may be helpful to have a way to add supported options. Consider adding a mongoc_client_encryption_cec_opts_t type to support only this option:

:man_page: mongoc_client_encryption_cec_opts_t

mongoc_client_encryption_cec_opts_t
===================================

Synopsis
--------

.. code-block:: c

  typedef struct _mongoc_client_encryption_cec_opts_t mongoc_client_encryption_cec_opts_t;

Used to set options for :symbol:`mongoc_client_encryption_create_encrypted_collection()`.

.. only:: html

  Functions
  ---------

  .. toctree::
    :titlesonly:
    :maxdepth: 1

    mongoc_client_encryption_cec_opts_new
    mongoc_client_encryption_cec_opts_destroy
    mongoc_client_encryption_cec_opts_set_masterkey

.. seealso::

  | :symbol:`mongoc_client_encryption_create_encrypted_collection()`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this pattern throughout the C driver and I question its ergonomics over using _v2/_v3 interfaces that add the additional options, which is used in most C APIs I'm familiar with. Using dynamically allocated options-objects adds significant complexity at call sites.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. The option structs may not add much benefit. Adding options to the structs still requires an API addition of the function call. The alternative API addition of a _v2 / _v3 may be preferable.

bson_error_t *error)
BSON_GNUC_WARN_UNUSED_RESULT;

Expand Down Expand Up @@ -47,8 +47,9 @@ Parameters
must be destroyed by the caller. If ``NULL``, has no effect.
* ``kms_provider``: The name of the KMS provider to use for generating new data
encryption keys for encrypted fields within the collection.
* ``dk_opts``: Additional options to be used when creating data encryption keys
for the collection.
* ``opt_masterKey``: If provided, used as the masterkey option when data
encryption keys need to be created. (See:
:doc:`mongoc_client_encryption_datakey_opts_set_masterkey`)
* ``error``: Optional output parameter pointing to storage for a
:symbol:`bson_error_t`. If an error occurs, will be initialized with error
information.
Expand Down
16 changes: 12 additions & 4 deletions src/libmongoc/src/mongoc/mongoc-client-side-encryption.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,7 @@ mongoc_client_encryption_create_encrypted_collection (
const bson_t *in_options,
bson_t *out_options,
const char *const kms_provider,
const mongoc_client_encryption_datakey_opts_t *dk_opts,
const bson_t *opt_masterkey,
bson_error_t *error)
{
BSON_UNUSED (enc);
Expand All @@ -983,7 +983,7 @@ mongoc_client_encryption_create_encrypted_collection (
BSON_UNUSED (in_options);
BSON_UNUSED (out_options);
BSON_UNUSED (kms_provider);
BSON_UNUSED (dk_opts);
BSON_UNUSED (opt_masterkey);

_disabled_error (error);
return NULL;
Expand Down Expand Up @@ -2932,16 +2932,16 @@ mongoc_client_encryption_create_encrypted_collection (
const bson_t *in_options,
bson_t *out_options,
const char *const kms_provider,
const mongoc_client_encryption_datakey_opts_t *dk_opts,
const bson_t *opt_masterkey,
bson_error_t *error)
{
BSON_ASSERT_PARAM (enc);
BSON_ASSERT_PARAM (database);
BSON_ASSERT_PARAM (name);
BSON_ASSERT_PARAM (in_options);
BSON_ASSERT (out_options || true);
BSON_ASSERT (opt_masterkey || true);
BSON_ASSERT_PARAM (kms_provider);
BSON_ASSERT_PARAM (dk_opts);
BSON_ASSERT (error || true);

mongoc_collection_t *ret = NULL;
Expand All @@ -2950,6 +2950,13 @@ mongoc_client_encryption_create_encrypted_collection (
bson_t new_encryptedFields = BSON_INITIALIZER;
bson_t local_new_options = BSON_INITIALIZER;

mongoc_client_encryption_datakey_opts_t *dk_opts =
mongoc_client_encryption_datakey_opts_new ();
if (opt_masterkey) {
mongoc_client_encryption_datakey_opts_set_masterkey (dk_opts,
opt_masterkey);
}

if (!out_options) {
// We'll use our own storage for the new options
out_options = &local_new_options;
Expand Down Expand Up @@ -3036,6 +3043,7 @@ mongoc_client_encryption_create_encrypted_collection (
done:
bson_destroy (&new_encryptedFields);
bson_destroy (&in_encryptedFields);
mongoc_client_encryption_datakey_opts_destroy (dk_opts);
// Destroy the local options, which may or may not have been used. If unused,
// the new options are now owned by the caller and this is a no-op.
bson_destroy (&local_new_options);
Expand Down
2 changes: 1 addition & 1 deletion src/libmongoc/src/mongoc/mongoc-client-side-encryption.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ mongoc_client_encryption_create_encrypted_collection (
const bson_t *in_options,
bson_t *opt_out_options,
const char *const kms_provider,
const mongoc_client_encryption_datakey_opts_t *dk_opts,
const bson_t *opt_masterkey,
bson_error_t *error) BSON_GNUC_WARN_UNUSED_RESULT;

BSON_END_DECLS
Expand Down
106 changes: 63 additions & 43 deletions src/libmongoc/tests/test-mongoc-client-side-encryption.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6218,10 +6218,9 @@ test_auto_datakeys (void *unused)
require (
keyWithType ("0", doc), //
parse (require (allOf (key ("keyId"), strEqual ("keepme")), nop))),
require (
keyWithType ("1", doc),
parse (require (allOf (keyWithType ("keyId", int32)),
do (ASSERT_CMPINT32 (bsonAs (int32), ==, 42))))));
require (keyWithType ("1", doc),
parse (require (allOf (keyWithType ("keyId", int32)),
do(ASSERT_CMPINT32 (bsonAs (int32), ==, 42))))));
ASSERT (bsonParseError == NULL);
bson_destroy (&out_fields);

Expand All @@ -6245,12 +6244,28 @@ test_auto_datakeys (void *unused)
}

static void
test_create_encrypted_collection_simple (void *unused)
_do_cec_test (void (*test) (const char *kmsProvider))
{
test ("local");
test ("aws");
}

// Declare a createEncryptedCollection test case (See usage below)
#define CEC_TEST(name, ...) \
static void name##_impl (__VA_ARGS__); \
static void name (void *unused) \
{ \
BSON_UNUSED (unused); \
_do_cec_test (name##_impl); \
} \
static void name##_impl (__VA_ARGS__)

CEC_TEST (test_create_encrypted_collection_simple, const char *kmsProvider)
{
BSON_UNUSED (unused);
bson_error_t error = {0};
mongoc_client_t *const client = test_framework_new_default_client ();
bson_t *const kmsProviders = _make_kms_providers (false, true);
bson_t *const kmsProviders = _make_kms_providers (true, true);
bson_t *const tlsOptions = _make_tls_opts ();

const char *const dbName = "cec-test-db";

Expand All @@ -6275,6 +6290,7 @@ test_create_encrypted_collection_simple (void *unused)
mongoc_client_encryption_opts_t *const ceOpts =
mongoc_client_encryption_opts_new ();
mongoc_client_encryption_opts_set_kms_providers (ceOpts, kmsProviders);
mongoc_client_encryption_opts_set_tls_opts (ceOpts, tlsOptions);
mongoc_client_encryption_opts_set_keyvault_namespace (
ceOpts, "keyvault", "datakeys");
mongoc_client_encryption_opts_set_keyvault_client (ceOpts, client);
Expand All @@ -6291,13 +6307,13 @@ test_create_encrypted_collection_simple (void *unused)
kv ("bsonType", cstr ("string")),
kv ("keyId", null)))))));
mongoc_database_t *const db = mongoc_client_get_database (client, dbName);
mongoc_client_encryption_datakey_opts_t *const dkOpts =
mongoc_client_encryption_datakey_opts_new ();
bson_t *const mkey = _make_kms_masterkey (kmsProvider);
mongoc_collection_t *const coll =
mongoc_client_encryption_create_encrypted_collection (
ce, db, "test-coll", &ccOpts, NULL, "local", dkOpts, &error);
ce, db, "test-coll", &ccOpts, NULL, kmsProvider, mkey, &error);
ASSERT_OR_PRINT (coll, error);
bson_destroy (&ccOpts);
bson_destroy (mkey);

bsonBuildDecl (doc, kv ("ssn", cstr ("123-45-6789")));
const bool okay =
Expand All @@ -6310,7 +6326,7 @@ test_create_encrypted_collection_simple (void *unused)
bson_destroy (&doc);

bson_destroy (kmsProviders);
mongoc_client_encryption_datakey_opts_destroy (dkOpts);
bson_destroy (tlsOptions);
mongoc_collection_destroy (coll);
mongoc_database_drop (db, &error);
mongoc_database_destroy (db);
Expand All @@ -6320,10 +6336,14 @@ test_create_encrypted_collection_simple (void *unused)

static void
test_create_encrypted_collection_no_encryptedFields_helper (
mongoc_client_t *client, const char *dbName, const char *collName)
mongoc_client_t *client,
const char *dbName,
const char *collName,
const char *kmsProvider)
{
bson_error_t error = {0};
bson_t *const kmsProviders = _make_kms_providers (false, true);
bson_t *const kmsProviders = _make_kms_providers (true, true);
bson_t *const tlsOptions = _make_tls_opts ();

// Drop prior data
{
Expand All @@ -6346,6 +6366,7 @@ test_create_encrypted_collection_no_encryptedFields_helper (
mongoc_client_encryption_opts_t *const ceOpts =
mongoc_client_encryption_opts_new ();
mongoc_client_encryption_opts_set_kms_providers (ceOpts, kmsProviders);
mongoc_client_encryption_opts_set_tls_opts (ceOpts, tlsOptions);
mongoc_client_encryption_opts_set_keyvault_namespace (
ceOpts, "keyvault", "datakeys");
mongoc_client_encryption_opts_set_keyvault_client (ceOpts, client);
Expand All @@ -6355,41 +6376,38 @@ test_create_encrypted_collection_no_encryptedFields_helper (
ASSERT_OR_PRINT (ce, error);

// Create the encrypted collection
bsonBuildDecl (ccOpts, do ());
bsonBuildDecl (ccOpts, do());
mongoc_database_t *const db = mongoc_client_get_database (client, dbName);
mongoc_client_encryption_datakey_opts_t *const dkOpts =
mongoc_client_encryption_datakey_opts_new ();
bson_t *const mkey = _make_kms_masterkey (kmsProvider);
mongoc_collection_t *const coll =
mongoc_client_encryption_create_encrypted_collection (
ce, db, collName, &ccOpts, NULL, "local", dkOpts, &error);
ce, db, collName, &ccOpts, NULL, kmsProvider, mkey, &error);
ASSERT_ERROR_CONTAINS (error,
MONGOC_ERROR_COMMAND,
MONGOC_ERROR_COMMAND_INVALID_ARG,
"No 'encryptedFields' are defined");
bson_destroy (&ccOpts);
bson_destroy (mkey);

bson_destroy (kmsProviders);
mongoc_client_encryption_datakey_opts_destroy (dkOpts);
bson_destroy (tlsOptions);
mongoc_collection_destroy (coll);
mongoc_database_drop (db, &error);
mongoc_database_destroy (db);
mongoc_client_encryption_destroy (ce);
}


static void
test_create_encrypted_collection_no_encryptedFields (void *unused)
CEC_TEST (test_create_encrypted_collection_no_encryptedFields,
const char *kmsProvider)
{
BSON_UNUSED (unused);

const char *dbName = "cec-test-db";
const char *collName = "test-coll";

// Test with a default client.
{
mongoc_client_t *const client = test_framework_new_default_client ();
test_create_encrypted_collection_no_encryptedFields_helper (
client, dbName, collName);
client, dbName, collName, kmsProvider);
mongoc_client_destroy (client);
}

Expand All @@ -6401,7 +6419,7 @@ test_create_encrypted_collection_no_encryptedFields (void *unused)
mongoc_auto_encryption_opts_t *aeOpts =
mongoc_auto_encryption_opts_new ();
bson_t *const kmsProviders =
_make_kms_providers (false /* with aws */, true /* with local */);
_make_kms_providers (true /* with aws */, true /* with local */);
char *namespace = bson_strdup_printf ("%s.%s", dbName, collName);
bson_t *encryptedFieldsMap =
tmp_bson ("{'%s': {'fields': []}}", namespace);
Expand All @@ -6416,7 +6434,7 @@ test_create_encrypted_collection_no_encryptedFields (void *unused)
mongoc_client_enable_auto_encryption (client, aeOpts, &error), error);

test_create_encrypted_collection_no_encryptedFields_helper (
client, dbName, collName);
client, dbName, collName, kmsProvider);

bson_free (namespace);
bson_destroy (kmsProviders);
Expand All @@ -6425,13 +6443,13 @@ test_create_encrypted_collection_no_encryptedFields (void *unused)
}
}

static void
test_create_encrypted_collection_bad_keyId (void *unused)
CEC_TEST (test_create_encrypted_collection_bad_keyId,
const char *const kmsProvider)
{
BSON_UNUSED (unused);
bson_error_t error = {0};
mongoc_client_t *const client = test_framework_new_default_client ();
bson_t *const kmsProviders = _make_kms_providers (false, true);
bson_t *const kmsProviders = _make_kms_providers (true, true);
bson_t *const tlsOptions = _make_tls_opts ();

const char *const dbName = "cec-test-db";

Expand All @@ -6456,6 +6474,7 @@ test_create_encrypted_collection_bad_keyId (void *unused)
mongoc_client_encryption_opts_t *const ceOpts =
mongoc_client_encryption_opts_new ();
mongoc_client_encryption_opts_set_kms_providers (ceOpts, kmsProviders);
mongoc_client_encryption_opts_set_tls_opts (ceOpts, tlsOptions);
mongoc_client_encryption_opts_set_keyvault_namespace (
ceOpts, "keyvault", "datakeys");
mongoc_client_encryption_opts_set_keyvault_client (ceOpts, client);
Expand All @@ -6472,19 +6491,19 @@ test_create_encrypted_collection_bad_keyId (void *unused)
kv ("bsonType", cstr ("string")),
kv ("keyId", bool (true))))))));
mongoc_database_t *const db = mongoc_client_get_database (client, dbName);
mongoc_client_encryption_datakey_opts_t *const dkOpts =
mongoc_client_encryption_datakey_opts_new ();
bson_t *const mkey = _make_kms_masterkey (kmsProvider);
mongoc_collection_t *const coll =
mongoc_client_encryption_create_encrypted_collection (
ce, db, "test-coll", &ccOpts, NULL, "local", dkOpts, &error);
ce, db, "test-coll", &ccOpts, NULL, kmsProvider, mkey, &error);
ASSERT_ERROR_CONTAINS (error,
MONGOC_ERROR_QUERY,
MONGOC_ERROR_PROTOCOL_INVALID_REPLY,
"create.encryptedFields.fields.keyId");
bson_destroy (&ccOpts);
bson_destroy (mkey);

bson_destroy (kmsProviders);
mongoc_client_encryption_datakey_opts_destroy (dkOpts);
bson_destroy (tlsOptions);
mongoc_collection_destroy (coll);
mongoc_database_drop (db, &error);
mongoc_database_destroy (db);
Expand All @@ -6493,13 +6512,13 @@ test_create_encrypted_collection_bad_keyId (void *unused)
}

// Implements Prose Test 21. Case: 4.
static void
test_create_encrypted_collection_insert (void *unused)
CEC_TEST (test_create_encrypted_collection_insert,
const char *const kmsProvider)
{
BSON_UNUSED (unused);
bson_error_t error = {0};
mongoc_client_t *const client = test_framework_new_default_client ();
bson_t *const kmsProviders = _make_kms_providers (false, true);
bson_t *const kmsProviders = _make_kms_providers (true, true);
bson_t *const tlsOptions = _make_tls_opts ();

const char *const dbName = "cec-test-db";

Expand All @@ -6524,6 +6543,7 @@ test_create_encrypted_collection_insert (void *unused)
mongoc_client_encryption_opts_t *const ceOpts =
mongoc_client_encryption_opts_new ();
mongoc_client_encryption_opts_set_kms_providers (ceOpts, kmsProviders);
mongoc_client_encryption_opts_set_tls_opts (ceOpts, tlsOptions);
mongoc_client_encryption_opts_set_keyvault_namespace (
ceOpts, "keyvault", "datakeys");
mongoc_client_encryption_opts_set_keyvault_client (ceOpts, client);
Expand All @@ -6540,14 +6560,14 @@ test_create_encrypted_collection_insert (void *unused)
kv ("bsonType", cstr ("string")),
kv ("keyId", null)))))));
mongoc_database_t *const db = mongoc_client_get_database (client, dbName);
mongoc_client_encryption_datakey_opts_t *const dkOpts =
mongoc_client_encryption_datakey_opts_new ();
bson_t new_opts;
bson_t *const mkey = _make_kms_masterkey (kmsProvider);
mongoc_collection_t *const coll =
mongoc_client_encryption_create_encrypted_collection (
ce, db, "testing1", &ccOpts, &new_opts, "local", dkOpts, &error);
ce, db, "testing1", &ccOpts, &new_opts, kmsProvider, mkey, &error);
ASSERT_OR_PRINT (coll, error);
bson_destroy (&ccOpts);
bson_destroy (mkey);

// Extract the encryption key ID that was generated by
// CreateEncryptedCollection:
Expand All @@ -6561,7 +6581,7 @@ test_create_encrypted_collection_insert (void *unused)
visitEach (require (type (doc)),
parse (require (key ("keyId"),
require (type (binary)),
do ({
do({
bson_value_copy (
bson_iter_value (
(bson_iter_t *) &bsonVisitIter),
Expand Down Expand Up @@ -6598,7 +6618,7 @@ test_create_encrypted_collection_insert (void *unused)
bson_destroy (&doc);
bson_value_destroy (&ciphertext);
bson_destroy (kmsProviders);
mongoc_client_encryption_datakey_opts_destroy (dkOpts);
bson_destroy (tlsOptions);
mongoc_collection_destroy (coll);
mongoc_database_drop (db, &error);
mongoc_database_destroy (db);
Expand Down