From ca06765b1b580282bbbb2c0d9466de591149b7d7 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Mon, 28 Apr 2025 17:25:08 +0200 Subject: [PATCH 01/11] Add error handling for invalid encryption keys with logging - Wrapped pbkdf2_hmac in a begin-rescue block to catch encryption key errors. - Added detailed error logging for failed key derivation. --- lib/cloud_controller/encryptor.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/cloud_controller/encryptor.rb b/lib/cloud_controller/encryptor.rb index 60a4a754f6d..7f88026665b 100644 --- a/lib/cloud_controller/encryptor.rb +++ b/lib/cloud_controller/encryptor.rb @@ -83,7 +83,20 @@ def run_cipher(cipher, input, salt, key, iterations:) if deprecated_short_salt?(salt) cipher.pkcs5_keyivgen(key, salt) else - cipher.key = OpenSSL::PKCS5.pbkdf2_hmac(key, salt, iterations, 16, OpenSSL::Digest.new('SHA256')) + begin + cipher.key = OpenSSL::PKCS5.pbkdf2_hmac(key, salt, iterations, 16, OpenSSL::Digest.new('SHA256')) + rescue StandardError => e + if key.nil? + logger.error("Failed to derive cipher key due to missing key for encryption_key_label=#{current_encryption_key_label}: #{e.class}: #{e.message}") + elsif salt.nil? + logger.error("Failed to derive cipher key due to missing salt: #{e.class}: #{e.message}") + elsif input.nil? + logger.error("Failed to derive cipher key due to missing input: #{e.class}: #{e.message}") + elsif !iterations.is_a?(Integer) + logger.error("Failed to derive cipher key due to wrong type of iterations (must be integer): #{e.class}: #{e.message}") + end + raise + end cipher.iv = salt end cipher.update(input) << cipher.final From f984d22d94ece7aed86260cd5b9162021085f081 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 29 Apr 2025 10:36:29 +0200 Subject: [PATCH 02/11] New log only for key=nil --- lib/cloud_controller/encryptor.rb | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/cloud_controller/encryptor.rb b/lib/cloud_controller/encryptor.rb index 7f88026665b..071a37f4ad6 100644 --- a/lib/cloud_controller/encryptor.rb +++ b/lib/cloud_controller/encryptor.rb @@ -86,15 +86,7 @@ def run_cipher(cipher, input, salt, key, iterations:) begin cipher.key = OpenSSL::PKCS5.pbkdf2_hmac(key, salt, iterations, 16, OpenSSL::Digest.new('SHA256')) rescue StandardError => e - if key.nil? - logger.error("Failed to derive cipher key due to missing key for encryption_key_label=#{current_encryption_key_label}: #{e.class}: #{e.message}") - elsif salt.nil? - logger.error("Failed to derive cipher key due to missing salt: #{e.class}: #{e.message}") - elsif input.nil? - logger.error("Failed to derive cipher key due to missing input: #{e.class}: #{e.message}") - elsif !iterations.is_a?(Integer) - logger.error("Failed to derive cipher key due to wrong type of iterations (must be integer): #{e.class}: #{e.message}") - end + logger.error("Failed to derive cipher key due to missing key for encryption_key_label=#{current_encryption_key_label}: #{e.class}: #{e.message}") if key.nil? raise end cipher.iv = salt From d227ca5d7f03923c839880fdaa0d24823d885d80 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Wed, 30 Apr 2025 17:06:32 +0200 Subject: [PATCH 03/11] Throw appropriate error instead of UnknownError for GET /v3/service_credential_bindings/:binding_guid/details when encryption-key-label is invalid --- .../service_credential_bindings_controller.rb | 10 ++++++++- lib/cloud_controller/encryptor.rb | 7 +++++-- .../service_credential_bindings_spec.rb | 21 +++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/app/controllers/v3/service_credential_bindings_controller.rb b/app/controllers/v3/service_credential_bindings_controller.rb index 8549251f7c0..775f863a7e4 100644 --- a/app/controllers/v3/service_credential_bindings_controller.rb +++ b/app/controllers/v3/service_credential_bindings_controller.rb @@ -133,7 +133,15 @@ def details credentials = if service_credential_binding.is_a?(ServiceKey) && service_credential_binding.credhub_reference? fetch_credentials_value(service_credential_binding.credhub_reference) else - service_credential_binding.credentials + begin + service_credential_binding.credentials + rescue StandardError => e + logger.error("Failed to decrypt credentials: #{e.message}") + raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', + "Cannot read credentials for \ + service_credential_binding \ + with guid: #{service_credential_binding.guid}") + end end details = Presenters::V3::ServiceCredentialBindingDetailsPresenter.new( diff --git a/lib/cloud_controller/encryptor.rb b/lib/cloud_controller/encryptor.rb index 071a37f4ad6..3178fdc1394 100644 --- a/lib/cloud_controller/encryptor.rb +++ b/lib/cloud_controller/encryptor.rb @@ -47,8 +47,11 @@ def decrypt(encrypted_input, salt, iterations:, label: nil) return unless encrypted_input key = key_to_use(label) - - decrypt_raw(encrypted_input, key, salt, iterations:) + begin + decrypt_raw(encrypted_input, key, salt, iterations:) + rescue OpenSSL::Cipher::CipherError, StandardError => e + raise StandardError.new("Decryption failed: #{e.message}") + end end def decrypt_raw(encrypted_input, key, salt, iterations:) diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb index f1f5d3c4ba2..cbf0934520e 100644 --- a/spec/request/service_credential_bindings_spec.rb +++ b/spec/request/service_credential_bindings_spec.rb @@ -624,6 +624,27 @@ def check_filtered_bindings(*bindings) } end + context 'when the encryption_key_label is invalid' do + before do + VCAP::CloudController::Encryptor.database_encryption_keys = { + encryption_key_0: 'somevalidkeyvalue', + foo: 'fooencryptionkey', + death: 'headbangingdeathmetalkey', 'invalid-key-label': 'fakekey' + } + end + + it 'fails to decrypt the credentials and returns a 500 error' do + app_binding.class.db[:service_bindings].where(id: app_binding.id).update(encryption_key_label: 'invalid-key-label') + + allow(VCAP::CloudController::Encryptor).to receive(:decrypt_raw).and_raise(StandardError.new('Decryption failed')) + + api_call.call(admin_headers) + + expect(last_response).to have_status_code(422) + expect(parsed_response['errors'].first['detail']).to match(/Cannot read credentials/i) + end + end + context "last binding operation is in 'create succeeded' state" do before do app_binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' }) From ec884502034ac844752147932bf681af551f705c Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Mon, 5 May 2025 14:38:21 +0200 Subject: [PATCH 04/11] enhance error handling --- app/controllers/v3/application_controller.rb | 6 ++++++ .../v3/service_credential_bindings_controller.rb | 7 ++----- lib/cloud_controller/encryptor.rb | 13 +++++-------- spec/request/service_credential_bindings_spec.rb | 8 ++++---- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/app/controllers/v3/application_controller.rb b/app/controllers/v3/application_controller.rb index a2da1d11d46..b2323a89a04 100644 --- a/app/controllers/v3/application_controller.rb +++ b/app/controllers/v3/application_controller.rb @@ -93,6 +93,7 @@ class ApplicationController < ActionController::Base rescue_from CloudController::Errors::CompoundError, with: :handle_compound_error rescue_from ActionDispatch::Http::Parameters::ParseError, with: :handle_invalid_request_body rescue_from Sequel::DatabaseConnectionError, Sequel::DatabaseDisconnectError, with: :handle_db_connection_error + rescue_from OpenSSL::Cipher::CipherError, with: :handle_decryption_error def configuration Config.config @@ -219,6 +220,11 @@ def handle_db_connection_error(_) handle_api_error(error) end + def handle_decryption_error(_) + error = CloudController::Errors::ApiError.new_from_details('InternalServerError', 'Failed to decrypt credentials') + handle_api_error(error) + end + def handle_exception(error) presenter = ErrorPresenter.new(error, Rails.env.test?, V3ErrorHasher.new(error)) logger.info(presenter.log_message) diff --git a/app/controllers/v3/service_credential_bindings_controller.rb b/app/controllers/v3/service_credential_bindings_controller.rb index 775f863a7e4..45df74bbaf8 100644 --- a/app/controllers/v3/service_credential_bindings_controller.rb +++ b/app/controllers/v3/service_credential_bindings_controller.rb @@ -135,12 +135,9 @@ def details else begin service_credential_binding.credentials - rescue StandardError => e + rescue VCAP::CloudController::Encryptor::EncryptorError => e logger.error("Failed to decrypt credentials: #{e.message}") - raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', - "Cannot read credentials for \ - service_credential_binding \ - with guid: #{service_credential_binding.guid}") + raise CloudController::Errors::V3::ApiError.new_from_details('InternalServerError', 'Failed to decrypt credentials') end end diff --git a/lib/cloud_controller/encryptor.rb b/lib/cloud_controller/encryptor.rb index 3178fdc1394..3e5a1220228 100644 --- a/lib/cloud_controller/encryptor.rb +++ b/lib/cloud_controller/encryptor.rb @@ -8,7 +8,7 @@ module VCAP::CloudController module Encryptor ENCRYPTION_ITERATIONS = 2048 - + class EncryptorError < StandardError; end class << self ALGORITHM = 'AES-128-CBC'.freeze @@ -47,11 +47,8 @@ def decrypt(encrypted_input, salt, iterations:, label: nil) return unless encrypted_input key = key_to_use(label) - begin - decrypt_raw(encrypted_input, key, salt, iterations:) - rescue OpenSSL::Cipher::CipherError, StandardError => e - raise StandardError.new("Decryption failed: #{e.message}") - end + + decrypt_raw(encrypted_input, key, salt, iterations:) end def decrypt_raw(encrypted_input, key, salt, iterations:) @@ -88,9 +85,9 @@ def run_cipher(cipher, input, salt, key, iterations:) else begin cipher.key = OpenSSL::PKCS5.pbkdf2_hmac(key, salt, iterations, 16, OpenSSL::Digest.new('SHA256')) - rescue StandardError => e + rescue OpenSSL::Cipher::CipherError => e logger.error("Failed to derive cipher key due to missing key for encryption_key_label=#{current_encryption_key_label}: #{e.class}: #{e.message}") if key.nil? - raise + raise EncryptorError end cipher.iv = salt end diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb index cbf0934520e..648c4dd26f4 100644 --- a/spec/request/service_credential_bindings_spec.rb +++ b/spec/request/service_credential_bindings_spec.rb @@ -631,17 +631,17 @@ def check_filtered_bindings(*bindings) foo: 'fooencryptionkey', death: 'headbangingdeathmetalkey', 'invalid-key-label': 'fakekey' } + allow_any_instance_of(ErrorPresenter).to receive(:raise_500?).and_return(false) end it 'fails to decrypt the credentials and returns a 500 error' do app_binding.class.db[:service_bindings].where(id: app_binding.id).update(encryption_key_label: 'invalid-key-label') - allow(VCAP::CloudController::Encryptor).to receive(:decrypt_raw).and_raise(StandardError.new('Decryption failed')) - + allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(VCAP::CloudController::Encryptor::EncryptorError) api_call.call(admin_headers) - expect(last_response).to have_status_code(422) - expect(parsed_response['errors'].first['detail']).to match(/Cannot read credentials/i) + expect(last_response).to have_status_code(500) + expect(parsed_response['errors'].first['detail']).to match(/Failed/i) end end From 7e061a21cc6217e4e34f77f4744ab0a9cfc0a08a Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Mon, 5 May 2025 16:57:05 +0200 Subject: [PATCH 05/11] add test for rescue_from decryption error --- .../v3/application_controller_spec.rb | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/spec/unit/controllers/v3/application_controller_spec.rb b/spec/unit/controllers/v3/application_controller_spec.rb index a25111cf67d..a7b396ff376 100644 --- a/spec/unit/controllers/v3/application_controller_spec.rb +++ b/spec/unit/controllers/v3/application_controller_spec.rb @@ -38,14 +38,18 @@ def not_found raise CloudController::Errors::NotFound.new_from_details('NotFound') end - def db_connection_error - raise Sequel::DatabaseConnectionError.new + def decryption_error + raise OpenSSL::Cipher::CipherError.new end def db_disconnect_error raise Sequel::DatabaseDisconnectError.new end + def db_connection_error + raise Sequel::DatabaseConnectionError.new + end + def warnings_is_nil add_warning_headers(nil) render status: :ok, json: {} @@ -320,6 +324,23 @@ def warnings_incorrect_type end end + describe '#handle_decryption_error' do + let!(:user) { set_current_user(VCAP::CloudController::User.make) } + + before do + allow_any_instance_of(ErrorPresenter).to receive(:raise_500?).and_return(false) + routes.draw do + get 'decryption_error' => 'anonymous#decryption_error' + end + end + + it 'rescues from OpenSSL::Cipher::CipherError and renders an error presenter1' do + get :decryption_error + expect(response).to have_http_status(:internal_server_error) + expect(response).to have_error_message(/Failed to decrypt credentials/) + end + end + describe '#add_warning_headers' do let!(:user) { set_current_user(VCAP::CloudController::User.make) } From 3e15dbf984426c3f7b6a1c1a6d973a87c6d791c8 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Mon, 5 May 2025 17:07:28 +0200 Subject: [PATCH 06/11] change error name --- app/controllers/v3/application_controller.rb | 6 +++--- .../v3/service_credential_bindings_controller.rb | 2 +- errors/v3.yml | 5 +++++ lib/cloud_controller/encryptor.rb | 4 ++-- spec/request/service_credential_bindings_spec.rb | 2 +- .../controllers/v3/application_controller_spec.rb | 12 ++++++------ 6 files changed, 18 insertions(+), 13 deletions(-) diff --git a/app/controllers/v3/application_controller.rb b/app/controllers/v3/application_controller.rb index b2323a89a04..7399af9c08b 100644 --- a/app/controllers/v3/application_controller.rb +++ b/app/controllers/v3/application_controller.rb @@ -93,7 +93,7 @@ class ApplicationController < ActionController::Base rescue_from CloudController::Errors::CompoundError, with: :handle_compound_error rescue_from ActionDispatch::Http::Parameters::ParseError, with: :handle_invalid_request_body rescue_from Sequel::DatabaseConnectionError, Sequel::DatabaseDisconnectError, with: :handle_db_connection_error - rescue_from OpenSSL::Cipher::CipherError, with: :handle_decryption_error + rescue_from OpenSSL::Cipher::CipherError, with: :handle_key_derivation_error def configuration Config.config @@ -220,8 +220,8 @@ def handle_db_connection_error(_) handle_api_error(error) end - def handle_decryption_error(_) - error = CloudController::Errors::ApiError.new_from_details('InternalServerError', 'Failed to decrypt credentials') + def handle_key_derivation_error(_) + error = CloudController::Errors::V3::ApiError.new_from_details('InternalServerError', 'Failed to decrypt credentials') handle_api_error(error) end diff --git a/app/controllers/v3/service_credential_bindings_controller.rb b/app/controllers/v3/service_credential_bindings_controller.rb index 45df74bbaf8..f2b263fd188 100644 --- a/app/controllers/v3/service_credential_bindings_controller.rb +++ b/app/controllers/v3/service_credential_bindings_controller.rb @@ -135,7 +135,7 @@ def details else begin service_credential_binding.credentials - rescue VCAP::CloudController::Encryptor::EncryptorError => e + rescue VCAP::CloudController::Encryptor::KeyDerivationError => e logger.error("Failed to decrypt credentials: #{e.message}") raise CloudController::Errors::V3::ApiError.new_from_details('InternalServerError', 'Failed to decrypt credentials') end diff --git a/errors/v3.yml b/errors/v3.yml index 3022af70ad4..d533a567049 100644 --- a/errors/v3.yml +++ b/errors/v3.yml @@ -17,3 +17,8 @@ name: UaaRateLimited http_code: 429 message: "The UAA is currently rate limited. Please try again later" + +10001: + name: InternalServerError + http_code: 500 + message: "%s" diff --git a/lib/cloud_controller/encryptor.rb b/lib/cloud_controller/encryptor.rb index 3e5a1220228..e207934fa2f 100644 --- a/lib/cloud_controller/encryptor.rb +++ b/lib/cloud_controller/encryptor.rb @@ -8,7 +8,7 @@ module VCAP::CloudController module Encryptor ENCRYPTION_ITERATIONS = 2048 - class EncryptorError < StandardError; end + class KeyDerivationError < StandardError; end class << self ALGORITHM = 'AES-128-CBC'.freeze @@ -87,7 +87,7 @@ def run_cipher(cipher, input, salt, key, iterations:) cipher.key = OpenSSL::PKCS5.pbkdf2_hmac(key, salt, iterations, 16, OpenSSL::Digest.new('SHA256')) rescue OpenSSL::Cipher::CipherError => e logger.error("Failed to derive cipher key due to missing key for encryption_key_label=#{current_encryption_key_label}: #{e.class}: #{e.message}") if key.nil? - raise EncryptorError + raise KeyDerivationError end cipher.iv = salt end diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb index 648c4dd26f4..c80b4d5bf31 100644 --- a/spec/request/service_credential_bindings_spec.rb +++ b/spec/request/service_credential_bindings_spec.rb @@ -637,7 +637,7 @@ def check_filtered_bindings(*bindings) it 'fails to decrypt the credentials and returns a 500 error' do app_binding.class.db[:service_bindings].where(id: app_binding.id).update(encryption_key_label: 'invalid-key-label') - allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(VCAP::CloudController::Encryptor::EncryptorError) + allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(VCAP::CloudController::Encryptor::KeyDerivationError) api_call.call(admin_headers) expect(last_response).to have_status_code(500) diff --git a/spec/unit/controllers/v3/application_controller_spec.rb b/spec/unit/controllers/v3/application_controller_spec.rb index a7b396ff376..08336d06d2b 100644 --- a/spec/unit/controllers/v3/application_controller_spec.rb +++ b/spec/unit/controllers/v3/application_controller_spec.rb @@ -38,8 +38,8 @@ def not_found raise CloudController::Errors::NotFound.new_from_details('NotFound') end - def decryption_error - raise OpenSSL::Cipher::CipherError.new + def key_derivation_error + raise OpenSSL::Cipher::CipherError end def db_disconnect_error @@ -324,18 +324,18 @@ def warnings_incorrect_type end end - describe '#handle_decryption_error' do + describe '#handle_key_derivation_error' do let!(:user) { set_current_user(VCAP::CloudController::User.make) } before do allow_any_instance_of(ErrorPresenter).to receive(:raise_500?).and_return(false) routes.draw do - get 'decryption_error' => 'anonymous#decryption_error' + get 'key_derivation_error' => 'anonymous#key_derivation_error' end end - it 'rescues from OpenSSL::Cipher::CipherError and renders an error presenter1' do - get :decryption_error + it 'rescues from OpenSSL::Cipher::CipherError and renders an error presenter' do + get :key_derivation_error expect(response).to have_http_status(:internal_server_error) expect(response).to have_error_message(/Failed to decrypt credentials/) end From e90425e605b53498524d6a076355c4a84350984b Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 6 May 2025 10:19:11 +0200 Subject: [PATCH 07/11] handle cipher error in central place in applications controller --- .../v3/service_credential_bindings_controller.rb | 7 +------ lib/cloud_controller/encryptor.rb | 9 ++------- spec/request/service_credential_bindings_spec.rb | 2 +- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/app/controllers/v3/service_credential_bindings_controller.rb b/app/controllers/v3/service_credential_bindings_controller.rb index f2b263fd188..8549251f7c0 100644 --- a/app/controllers/v3/service_credential_bindings_controller.rb +++ b/app/controllers/v3/service_credential_bindings_controller.rb @@ -133,12 +133,7 @@ def details credentials = if service_credential_binding.is_a?(ServiceKey) && service_credential_binding.credhub_reference? fetch_credentials_value(service_credential_binding.credhub_reference) else - begin - service_credential_binding.credentials - rescue VCAP::CloudController::Encryptor::KeyDerivationError => e - logger.error("Failed to decrypt credentials: #{e.message}") - raise CloudController::Errors::V3::ApiError.new_from_details('InternalServerError', 'Failed to decrypt credentials') - end + service_credential_binding.credentials end details = Presenters::V3::ServiceCredentialBindingDetailsPresenter.new( diff --git a/lib/cloud_controller/encryptor.rb b/lib/cloud_controller/encryptor.rb index e207934fa2f..60a4a754f6d 100644 --- a/lib/cloud_controller/encryptor.rb +++ b/lib/cloud_controller/encryptor.rb @@ -8,7 +8,7 @@ module VCAP::CloudController module Encryptor ENCRYPTION_ITERATIONS = 2048 - class KeyDerivationError < StandardError; end + class << self ALGORITHM = 'AES-128-CBC'.freeze @@ -83,12 +83,7 @@ def run_cipher(cipher, input, salt, key, iterations:) if deprecated_short_salt?(salt) cipher.pkcs5_keyivgen(key, salt) else - begin - cipher.key = OpenSSL::PKCS5.pbkdf2_hmac(key, salt, iterations, 16, OpenSSL::Digest.new('SHA256')) - rescue OpenSSL::Cipher::CipherError => e - logger.error("Failed to derive cipher key due to missing key for encryption_key_label=#{current_encryption_key_label}: #{e.class}: #{e.message}") if key.nil? - raise KeyDerivationError - end + cipher.key = OpenSSL::PKCS5.pbkdf2_hmac(key, salt, iterations, 16, OpenSSL::Digest.new('SHA256')) cipher.iv = salt end cipher.update(input) << cipher.final diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb index c80b4d5bf31..6f5a4094fac 100644 --- a/spec/request/service_credential_bindings_spec.rb +++ b/spec/request/service_credential_bindings_spec.rb @@ -637,7 +637,7 @@ def check_filtered_bindings(*bindings) it 'fails to decrypt the credentials and returns a 500 error' do app_binding.class.db[:service_bindings].where(id: app_binding.id).update(encryption_key_label: 'invalid-key-label') - allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(VCAP::CloudController::Encryptor::KeyDerivationError) + allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(OpenSSL::Cipher::CipherError) api_call.call(admin_headers) expect(last_response).to have_status_code(500) From 18e747a4dcbfc2ee9752aec9179dd06a52534c8f Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 6 May 2025 11:25:20 +0200 Subject: [PATCH 08/11] add test for GET /v3/apps/:guid/environment_variables when the encryption_key_label is invalid --- spec/request/apps_spec.rb | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/spec/request/apps_spec.rb b/spec/request/apps_spec.rb index 8afc4591ff0..69765ff0b67 100644 --- a/spec/request/apps_spec.rb +++ b/spec/request/apps_spec.rb @@ -3398,6 +3398,38 @@ end end end + + context 'when the encryption_key_label is invalid' do + let(:instance) { VCAP::CloudController::ManagedServiceInstance.make(space:) } + let(:app_binding) do + VCAP::CloudController::ServiceBinding.make( + app: app_model, + service_instance_guid: instance.guid, + credentials: { key: 'value' }, + syslog_drain_url: 'syslog-url', + volume_mounts: %w[volume1 volume2] + ) + end + + before do + VCAP::CloudController::Encryptor.database_encryption_keys = { + encryption_key_0: 'somevalidkeyvalue', + foo: 'fooencryptionkey', + death: 'headbangingdeathmetalkey', 'invalid-key-label': 'fakekey' + } + allow_any_instance_of(ErrorPresenter).to receive(:raise_500?).and_return(false) + end + + it 'fails to decrypt the environment variables and returns a 500 error' do + app_binding.class.db[:service_bindings].where(id: app_binding.id).update(encryption_key_label: 'invalid-key-label') + + allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(OpenSSL::Cipher::CipherError) + api_call.call(admin_headers) + + expect(last_response).to have_status_code(500) + expect(parsed_response['errors'].first['detail']).to match(/Failed/i) + end + end end describe 'GET /v3/apps/:guid/permissions' do From 06131362cdd18e9dec4f8d30b6156daa79ddff6e Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Tue, 6 May 2025 14:03:09 +0200 Subject: [PATCH 09/11] add test for service borker update when the encryption_key_label is invalid --- spec/request/service_brokers_spec.rb | 33 ++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/spec/request/service_brokers_spec.rb b/spec/request/service_brokers_spec.rb index 45226eb3351..bcbcb5461bd 100644 --- a/spec/request/service_brokers_spec.rb +++ b/spec/request/service_brokers_spec.rb @@ -899,6 +899,39 @@ def expect_empty_list(user_headers) expect(response).to include('detail' => 'Service broker not found') end end + + context 'when updating credentials and the encryption_key_label is invalid' do + let(:broker) { VCAP::CloudController::ServiceBroker.make } + let(:api_call) do + lambda { |headers| + patch "/v3/service_brokers/#{broker.guid}", { authentication: { + type: 'basic', + credentials: { + username: 'your-username', + password: 'your-password' + } + } }.to_json, headers + } + end + + before do + VCAP::CloudController::Encryptor.database_encryption_keys = { + encryption_key_0: 'somevalidkeyvalue', + foo: 'fooencryptionkey', + death: 'headbangingdeathmetalkey', 'invalid-key-label': 'fakekey' + } + broker.class.db[:service_brokers].where(id: broker.id).update(encryption_key_label: 'invalid-key-label') + allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(OpenSSL::Cipher::CipherError) + allow_any_instance_of(ErrorPresenter).to receive(:raise_500?).and_return(false) + end + + it 'fails to decrypt the broker data and returns a 500 error' do + api_call.call(admin_headers) + + expect(last_response).to have_status_code(500) + expect(parsed_response['errors'].first['detail']).to match(/Failed/i) + end + end end describe 'POST /v3/service_brokers' do From 6244510a7e92a613b665b000c4589eb1a1a6e54a Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 8 May 2025 13:30:16 +0200 Subject: [PATCH 10/11] rename error message, remove obsolete parts in test setups --- app/controllers/v3/application_controller.rb | 2 +- spec/request/apps_spec.rb | 19 ++----------------- spec/request/service_brokers_spec.rb | 11 +++-------- .../service_credential_bindings_spec.rb | 10 ++-------- .../v3/application_controller_spec.rb | 2 +- 5 files changed, 9 insertions(+), 35 deletions(-) diff --git a/app/controllers/v3/application_controller.rb b/app/controllers/v3/application_controller.rb index 7399af9c08b..b992d0199e7 100644 --- a/app/controllers/v3/application_controller.rb +++ b/app/controllers/v3/application_controller.rb @@ -221,7 +221,7 @@ def handle_db_connection_error(_) end def handle_key_derivation_error(_) - error = CloudController::Errors::V3::ApiError.new_from_details('InternalServerError', 'Failed to decrypt credentials') + error = CloudController::Errors::V3::ApiError.new_from_details('InternalServerError', 'Error while processing encrypted data') handle_api_error(error) end diff --git a/spec/request/apps_spec.rb b/spec/request/apps_spec.rb index 69765ff0b67..29cf157c39d 100644 --- a/spec/request/apps_spec.rb +++ b/spec/request/apps_spec.rb @@ -3401,33 +3401,18 @@ context 'when the encryption_key_label is invalid' do let(:instance) { VCAP::CloudController::ManagedServiceInstance.make(space:) } - let(:app_binding) do - VCAP::CloudController::ServiceBinding.make( - app: app_model, - service_instance_guid: instance.guid, - credentials: { key: 'value' }, - syslog_drain_url: 'syslog-url', - volume_mounts: %w[volume1 volume2] - ) - end before do - VCAP::CloudController::Encryptor.database_encryption_keys = { - encryption_key_0: 'somevalidkeyvalue', - foo: 'fooencryptionkey', - death: 'headbangingdeathmetalkey', 'invalid-key-label': 'fakekey' - } allow_any_instance_of(ErrorPresenter).to receive(:raise_500?).and_return(false) end it 'fails to decrypt the environment variables and returns a 500 error' do - app_binding.class.db[:service_bindings].where(id: app_binding.id).update(encryption_key_label: 'invalid-key-label') - + app_model # ensure that app model is created before run_cipher is mocked to throw an error allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(OpenSSL::Cipher::CipherError) api_call.call(admin_headers) expect(last_response).to have_status_code(500) - expect(parsed_response['errors'].first['detail']).to match(/Failed/i) + expect(parsed_response['errors'].first['detail']).to match(/Error while processing encrypted data/i) end end end diff --git a/spec/request/service_brokers_spec.rb b/spec/request/service_brokers_spec.rb index bcbcb5461bd..265cfc596bd 100644 --- a/spec/request/service_brokers_spec.rb +++ b/spec/request/service_brokers_spec.rb @@ -915,21 +915,16 @@ def expect_empty_list(user_headers) end before do - VCAP::CloudController::Encryptor.database_encryption_keys = { - encryption_key_0: 'somevalidkeyvalue', - foo: 'fooencryptionkey', - death: 'headbangingdeathmetalkey', 'invalid-key-label': 'fakekey' - } - broker.class.db[:service_brokers].where(id: broker.id).update(encryption_key_label: 'invalid-key-label') - allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(OpenSSL::Cipher::CipherError) allow_any_instance_of(ErrorPresenter).to receive(:raise_500?).and_return(false) end it 'fails to decrypt the broker data and returns a 500 error' do + broker # ensure the broker is created before run_cipher is mocked to throw an error + allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(OpenSSL::Cipher::CipherError) api_call.call(admin_headers) expect(last_response).to have_status_code(500) - expect(parsed_response['errors'].first['detail']).to match(/Failed/i) + expect(parsed_response['errors'].first['detail']).to match(/Error while processing encrypted data/i) end end end diff --git a/spec/request/service_credential_bindings_spec.rb b/spec/request/service_credential_bindings_spec.rb index 6f5a4094fac..7176af5f6a8 100644 --- a/spec/request/service_credential_bindings_spec.rb +++ b/spec/request/service_credential_bindings_spec.rb @@ -626,22 +626,16 @@ def check_filtered_bindings(*bindings) context 'when the encryption_key_label is invalid' do before do - VCAP::CloudController::Encryptor.database_encryption_keys = { - encryption_key_0: 'somevalidkeyvalue', - foo: 'fooencryptionkey', - death: 'headbangingdeathmetalkey', 'invalid-key-label': 'fakekey' - } allow_any_instance_of(ErrorPresenter).to receive(:raise_500?).and_return(false) end it 'fails to decrypt the credentials and returns a 500 error' do - app_binding.class.db[:service_bindings].where(id: app_binding.id).update(encryption_key_label: 'invalid-key-label') - + app_binding # ensure that binding is created before run_cipher is mocked to throw an error allow(VCAP::CloudController::Encryptor).to receive(:run_cipher).and_raise(OpenSSL::Cipher::CipherError) api_call.call(admin_headers) expect(last_response).to have_status_code(500) - expect(parsed_response['errors'].first['detail']).to match(/Failed/i) + expect(parsed_response['errors'].first['detail']).to match(/Error while processing encrypted data/i) end end diff --git a/spec/unit/controllers/v3/application_controller_spec.rb b/spec/unit/controllers/v3/application_controller_spec.rb index 08336d06d2b..e8c15173345 100644 --- a/spec/unit/controllers/v3/application_controller_spec.rb +++ b/spec/unit/controllers/v3/application_controller_spec.rb @@ -337,7 +337,7 @@ def warnings_incorrect_type it 'rescues from OpenSSL::Cipher::CipherError and renders an error presenter' do get :key_derivation_error expect(response).to have_http_status(:internal_server_error) - expect(response).to have_error_message(/Failed to decrypt credentials/) + expect(response).to have_error_message(/Error while processing encrypted data/) end end From ffba137c2e4b8717f0d4723ff3729baf43ba7ab5 Mon Sep 17 00:00:00 2001 From: Philipp Thun Date: Fri, 9 May 2025 11:48:11 +0200 Subject: [PATCH 11/11] Update spec/request/apps_spec.rb --- spec/request/apps_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/request/apps_spec.rb b/spec/request/apps_spec.rb index 29cf157c39d..37a5998861e 100644 --- a/spec/request/apps_spec.rb +++ b/spec/request/apps_spec.rb @@ -3400,8 +3400,6 @@ end context 'when the encryption_key_label is invalid' do - let(:instance) { VCAP::CloudController::ManagedServiceInstance.make(space:) } - before do allow_any_instance_of(ErrorPresenter).to receive(:raise_500?).and_return(false) end