From 43cea98eef18372a246d2b1d03b646602182a4ea Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Tue, 23 May 2023 16:35:12 -0600 Subject: [PATCH 1/7] RUBY 3156 enforce that masterKey requires provider --- lib/mongo/crypt/explicit_encrypter.rb | 15 ++++++++++++++- .../client_side_encryption/rewrap_prose_spec.rb | 8 ++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/mongo/crypt/explicit_encrypter.rb b/lib/mongo/crypt/explicit_encrypter.rb index cce570e441..2421b337b3 100644 --- a/lib/mongo/crypt/explicit_encrypter.rb +++ b/lib/mongo/crypt/explicit_encrypter.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:todo all # Copyright (C) 2020 MongoDB Inc. # @@ -257,6 +256,8 @@ def remove_key_alt_name(id, key_alt_name) # # @return [ Crypt::RewrapManyDataKeyResult ] Result of the operation. def rewrap_many_data_key(filter, opts = {}) + validate_rewrap_options!(opts) + master_key_document = if opts[:provider] options = opts.dup provider = options.delete(:provider) @@ -291,6 +292,18 @@ def rewrap_many_data_key(filter, opts = {}) @encryption_io.update_data_keys(updates) ) end + + # Ensures the consistency of the options passed to #rewrap_many_data_keys. + # + # @param [Hash] opts the options hash to validate + # + # @raise [ ArgumentError ] if the options are not consistent or + # compatible. + def validate_rewrap_options!(opts) + if opts.key?(:master_key) && !opts.key?(:provider) + raise ArgumentError, 'If :master_key is specified, :provider must also be given' + end + end end end end diff --git a/spec/integration/client_side_encryption/rewrap_prose_spec.rb b/spec/integration/client_side_encryption/rewrap_prose_spec.rb index fada5ee6e7..46603665b0 100644 --- a/spec/integration/client_side_encryption/rewrap_prose_spec.rb +++ b/spec/integration/client_side_encryption/rewrap_prose_spec.rb @@ -93,6 +93,14 @@ expect(client_encryption_1.decrypt(ciphertext)).to eq('test') expect(client_encryption_2.decrypt(ciphertext)).to eq('test') end + + + context 'when master_key is present without provider' do + it 'raises an exception' do + expect { client_encryption_1.rewrap_many_data_key({}, master_key: {}) } + .to raise_error(ArgumentError, /provider/) + end + end end end end From eebfb051adff809133ed96ababb8c2a25d8eb35e Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Fri, 26 May 2023 14:10:53 -0600 Subject: [PATCH 2/7] linter appeasement --- lib/mongo/crypt/explicit_encrypter.rb | 105 ++++++++++-------- .../rewrap_prose_spec.rb | 1 - 2 files changed, 59 insertions(+), 47 deletions(-) diff --git a/lib/mongo/crypt/explicit_encrypter.rb b/lib/mongo/crypt/explicit_encrypter.rb index 2421b337b3..a38c2705a1 100644 --- a/lib/mongo/crypt/explicit_encrypter.rb +++ b/lib/mongo/crypt/explicit_encrypter.rb @@ -16,12 +16,13 @@ module Mongo module Crypt - # An ExplicitEncrypter is an object that performs explicit encryption # operations and handles all associated options and instance variables. # # @api private class ExplicitEncrypter + extend Forwardable + # Create a new ExplicitEncrypter object. # # @param [ Mongo::Client ] key_vault_client An instance of Mongo::Client @@ -43,7 +44,7 @@ def initialize(key_vault_client, key_vault_namespace, kms_providers, kms_tls_opt @encryption_io = EncryptionIO.new( key_vault_client: key_vault_client, metadata_client: nil, - key_vault_namespace: key_vault_namespace, + key_vault_namespace: key_vault_namespace ) end @@ -107,7 +108,7 @@ def encrypt(value, options) Crypt::ExplicitEncryptionContext.new( @crypt_handle, @encryption_io, - { 'v': value }, + { v: value }, options ).run_state_machine['v'] end @@ -166,7 +167,7 @@ def encrypt_expression(expression, options) Crypt::ExplicitEncryptionExpressionContext.new( @crypt_handle, @encryption_io, - { 'v': expression }, + { v: expression }, options ).run_state_machine['v'] end @@ -178,10 +179,10 @@ def encrypt_expression(expression, options) # # @return [ Object ] The decrypted value def decrypt(value) - result = Crypt::ExplicitDecryptionContext.new( + Crypt::ExplicitDecryptionContext.new( @crypt_handle, @encryption_io, - { 'v': value }, + { v: value } ).run_state_machine['v'] end @@ -202,9 +203,7 @@ def add_key_alt_name(id, key_alt_name) # # @return [ Operation::Result ] The response from the database for the delete_one # operation that deletes the key. - def delete_key(id) - @encryption_io.delete_key(id) - end + def_delegators :@encryption_io, :delete_key # Finds a single key with the given id. # @@ -212,9 +211,7 @@ def delete_key(id) # # @return [ BSON::Document | nil ] The found key document or nil # if not found. - def get_key(id) - @encryption_io.get_key(id) - end + def_delegators :@encryption_io, :get_key # Returns a key in the key vault collection with the given key_alt_name. # @@ -222,16 +219,12 @@ def get_key(id) # # @return [ BSON::Document | nil ] The found key document or nil # if not found. - def get_key_by_alt_name(key_alt_name) - @encryption_io.get_key_by_alt_name(key_alt_name) - end + def_delegators :@encryption_io, :get_key_by_alt_name # Returns all keys in the key vault collection. # # @return [ Collection::View ] Keys in the key vault collection. - def get_keys - @encryption_io.get_keys - end + def_delegators :@encryption_io, :get_keys # Removes a key_alt_name from a key in the key vault collection with the given id. # @@ -240,9 +233,7 @@ def get_keys # # @return [ BSON::Document | nil ] Document describing the identified key # before removing the key alt name, or nil if no such key. - def remove_key_alt_name(id, key_alt_name) - @encryption_io.remove_key_alt_name(id, key_alt_name) - end + def_delegators :@encryption_io, :remove_key_alt_name # Decrypts multiple data keys and (re-)encrypts them with a new master_key, # or with their current master_key if a new one is not given. @@ -258,11 +249,7 @@ def remove_key_alt_name(id, key_alt_name) def rewrap_many_data_key(filter, opts = {}) validate_rewrap_options!(opts) - master_key_document = if opts[:provider] - options = opts.dup - provider = options.delete(:provider) - KMS::MasterKeyDocument.new(provider, options) - end + master_key_document = master_key_for_provider(opts) rewrap_result = Crypt::RewrapManyDataKeyContext.new( @crypt_handle, @@ -270,11 +257,52 @@ def rewrap_many_data_key(filter, opts = {}) filter, master_key_document ).run_state_machine - if rewrap_result.nil? - return RewrapManyDataKeyResult.new(nil) - end - data_key_documents = rewrap_result.fetch('v') - updates = data_key_documents.map do |doc| + + return RewrapManyDataKeyResult.new(nil) if rewrap_result.nil? + + updates = updates_from_data_key_documents(rewrap_result.fetch('v')) + RewrapManyDataKeyResult.new(@encryption_io.update_data_keys(updates)) + end + + private + + # Ensures the consistency of the options passed to #rewrap_many_data_keys. + # + # @param [ Hash ] opts the options hash to validate + # + # @raise [ ArgumentError ] if the options are not consistent or + # compatible. + def validate_rewrap_options!(opts) + return unless opts.key?(:master_key) && !opts.key?(:provider) + + raise ArgumentError, 'If :master_key is specified, :provider must also be given' + end + + # If a :provider is given, construct a new master key document + # with that provider. + # + # @param [ Hash ] opts the options hash + # + # @option [ String ] :provider KMS provider to encrypt keys. + # + # @return [ KMS::MasterKeyDocument | nil ] the new master key document, + # or nil if no provider was given. + def master_key_for_provider(opts) + return nil unless opts[:provider] + + options = opts.dup + provider = options.delete(:provider) + KMS::MasterKeyDocument.new(provider, options) + end + + # Returns the corresponding update document for each for of the given + # data key documents. + # + # @param [ Array ] documents the data key documents + # + # @return [ Array ] the update documents + def updates_from_data_key_documents(documents) + documents.map do |doc| { update_one: { filter: { _id: doc[:_id] }, @@ -288,21 +316,6 @@ def rewrap_many_data_key(filter, opts = {}) } } end - RewrapManyDataKeyResult.new( - @encryption_io.update_data_keys(updates) - ) - end - - # Ensures the consistency of the options passed to #rewrap_many_data_keys. - # - # @param [Hash] opts the options hash to validate - # - # @raise [ ArgumentError ] if the options are not consistent or - # compatible. - def validate_rewrap_options!(opts) - if opts.key?(:master_key) && !opts.key?(:provider) - raise ArgumentError, 'If :master_key is specified, :provider must also be given' - end end end end diff --git a/spec/integration/client_side_encryption/rewrap_prose_spec.rb b/spec/integration/client_side_encryption/rewrap_prose_spec.rb index ddd25f8da4..a5e5580e33 100644 --- a/spec/integration/client_side_encryption/rewrap_prose_spec.rb +++ b/spec/integration/client_side_encryption/rewrap_prose_spec.rb @@ -102,7 +102,6 @@ expect(client_encryption2.decrypt(ciphertext)).to eq('test') end - context 'when master_key is present without provider' do it 'raises an exception' do expect { client_encryption_1.rewrap_many_data_key({}, master_key: {}) } From fad2da2dc42a2fd86f3953dcb9cbe93b698440d2 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Fri, 26 May 2023 14:12:52 -0600 Subject: [PATCH 3/7] reference the correct variable --- spec/integration/client_side_encryption/rewrap_prose_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/integration/client_side_encryption/rewrap_prose_spec.rb b/spec/integration/client_side_encryption/rewrap_prose_spec.rb index ddd25f8da4..86958929f2 100644 --- a/spec/integration/client_side_encryption/rewrap_prose_spec.rb +++ b/spec/integration/client_side_encryption/rewrap_prose_spec.rb @@ -102,10 +102,9 @@ expect(client_encryption2.decrypt(ciphertext)).to eq('test') end - context 'when master_key is present without provider' do it 'raises an exception' do - expect { client_encryption_1.rewrap_many_data_key({}, master_key: {}) } + expect { client_encryption1.rewrap_many_data_key({}, master_key: {}) } .to raise_error(ArgumentError, /provider/) end end From 369c7a2f91429e8c78a7e9dee8f22defd5bedac9 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Fri, 26 May 2023 14:18:06 -0600 Subject: [PATCH 4/7] rubocop work is being done in a separate branch --- lib/mongo/crypt/explicit_encrypter.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/mongo/crypt/explicit_encrypter.rb b/lib/mongo/crypt/explicit_encrypter.rb index 2421b337b3..ffe885976b 100644 --- a/lib/mongo/crypt/explicit_encrypter.rb +++ b/lib/mongo/crypt/explicit_encrypter.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true +# rubocop:todo all # Copyright (C) 2020 MongoDB Inc. # From d41cbde65b74b6a1ff7ba614493895f6085a4669 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Fri, 26 May 2023 14:18:39 -0600 Subject: [PATCH 5/7] remove the rubocop:todo for this file --- lib/mongo/crypt/explicit_encrypter.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/mongo/crypt/explicit_encrypter.rb b/lib/mongo/crypt/explicit_encrypter.rb index 61ebd8f991..a38c2705a1 100644 --- a/lib/mongo/crypt/explicit_encrypter.rb +++ b/lib/mongo/crypt/explicit_encrypter.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -# rubocop:todo all # Copyright (C) 2020 MongoDB Inc. # From 4c35e2ceaa619c9798486b3b60420d0e1939e3a9 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Wed, 31 May 2023 08:42:02 -0600 Subject: [PATCH 6/7] remove merge artifact --- lib/mongo/crypt/explicit_encrypter.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/mongo/crypt/explicit_encrypter.rb b/lib/mongo/crypt/explicit_encrypter.rb index 16f550d933..1c0699cb03 100644 --- a/lib/mongo/crypt/explicit_encrypter.rb +++ b/lib/mongo/crypt/explicit_encrypter.rb @@ -249,15 +249,7 @@ def add_key_alt_name(id, key_alt_name) def rewrap_many_data_key(filter, opts = {}) validate_rewrap_options!(opts) -<<<<<<< HEAD master_key_document = master_key_for_provider(opts) -======= - master_key_document = if opts[:provider] - options = opts.dup - provider = options.delete(:provider) - KMS::MasterKeyDocument.new(provider, options) - end ->>>>>>> master rewrap_result = Crypt::RewrapManyDataKeyContext.new( @crypt_handle, From 642f4f04e28402a78425a3630f1b97d763b61efe Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Wed, 31 May 2023 10:10:45 -0600 Subject: [PATCH 7/7] merge artifact --- lib/mongo/crypt/explicit_encrypter.rb | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/mongo/crypt/explicit_encrypter.rb b/lib/mongo/crypt/explicit_encrypter.rb index 1c0699cb03..a38c2705a1 100644 --- a/lib/mongo/crypt/explicit_encrypter.rb +++ b/lib/mongo/crypt/explicit_encrypter.rb @@ -317,18 +317,6 @@ def updates_from_data_key_documents(documents) } end end - - # Ensures the consistency of the options passed to #rewrap_many_data_keys. - # - # @param [Hash] opts the options hash to validate - # - # @raise [ ArgumentError ] if the options are not consistent or - # compatible. - def validate_rewrap_options!(opts) - if opts.key?(:master_key) && !opts.key?(:provider) - raise ArgumentError, 'If :master_key is specified, :provider must also be given' - end - end end end end