Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion app/controllers/v3/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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_key_derivation_error
rescue_from VCAP::CloudController::Encryptor::EncryptorError, with: :handle_key_derivation_error

def configuration
Config.config
Expand Down
2 changes: 1 addition & 1 deletion errors/v3.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
http_code: 429
message: "The UAA is currently rate limited. Please try again later"

10001:
10081:
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There exists already error with that code, I thought it would probably better to have an unique error code to identify them more easily.

name: InternalServerError
http_code: 500
message: "%s"
4 changes: 4 additions & 0 deletions lib/cloud_controller/encryptor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ module VCAP::CloudController
module Encryptor
ENCRYPTION_ITERATIONS = 2048

class EncryptorError < StandardError; end

class << self
ALGORITHM = 'AES-128-CBC'.freeze

Expand Down Expand Up @@ -87,6 +89,8 @@ def run_cipher(cipher, input, salt, key, iterations:)
cipher.iv = salt
end
cipher.update(input) << cipher.final
rescue OpenSSL::Cipher::CipherError, TypeError => e
raise EncryptorError.new("Encryption/Decryption failed: #{e.message}")
end

def deprecated_short_salt?(salt)
Expand Down
3 changes: 2 additions & 1 deletion lib/cloud_controller/validate_database_keys.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ def validate_encryption_key_values_unchanged!(config)
iterations: sentinel_model.encryption_iterations
)
# A failed decryption occasionally results in a CipherError: bad decrypt instead of a garbled string
rescue OpenSSL::Cipher::CipherError
# This is now caught inside Encryptor and re-raised as EncryptorError
rescue VCAP::CloudController::Encryptor::EncryptorError
labels_with_changed_keys << label_string
next
end
Expand Down
2 changes: 1 addition & 1 deletion spec/request/apps_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3406,7 +3406,7 @@

it 'fails to decrypt the environment variables and returns a 500 error' do
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)
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(500)
Expand Down
2 changes: 1 addition & 1 deletion spec/request/service_brokers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -920,7 +920,7 @@ def expect_empty_list(user_headers)

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)
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(500)
Expand Down
2 changes: 1 addition & 1 deletion spec/request/service_credential_bindings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ def check_filtered_bindings(*bindings)

it 'fails to decrypt the credentials and returns a 500 error' do
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)
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(500)
Expand Down
2 changes: 1 addition & 1 deletion spec/support/shared_examples/models/encrypted_attribute.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def last_row

begin
decrypted_value = Encryptor.decrypt(saved_attribute, model.send(attr_salt), label: model.encryption_key_label, iterations: model.encryption_iterations)
rescue OpenSSL::Cipher::CipherError
rescue VCAP::CloudController::Encryptor::EncryptorError
errored = true
end

Expand Down
4 changes: 2 additions & 2 deletions spec/unit/controllers/v3/application_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def not_found
end

def key_derivation_error
raise OpenSSL::Cipher::CipherError
raise VCAP::CloudController::Encryptor::EncryptorError
end

def db_disconnect_error
Expand Down Expand Up @@ -334,7 +334,7 @@ def warnings_incorrect_type
end
end

it 'rescues from OpenSSL::Cipher::CipherError and renders an error presenter' do
it 'rescues from EncryptorError 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(/Error while processing encrypted data/)
Expand Down
13 changes: 11 additions & 2 deletions spec/unit/lib/cloud_controller/encryptor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ module VCAP::CloudController

result = begin
Encryptor.decrypt(encrypted_string, salt, iterations: encryption_iterations)
rescue OpenSSL::Cipher::CipherError => e
rescue VCAP::CloudController::Encryptor::EncryptorError => e
e.message
end

Expand All @@ -169,12 +169,21 @@ module VCAP::CloudController

result = begin
Encryptor.decrypt(encrypted_string, salt, label: 'death', iterations: encryption_iterations)
rescue OpenSSL::Cipher::CipherError => e
rescue VCAP::CloudController::Encryptor::EncryptorError => e
e.message
end

expect(result).not_to eq(unencrypted_string)
end

it 'raises an EncryptorError' do
allow(Encryptor).to receive(:current_encryption_key_label).and_return('foo')
encrypted_string = Encryptor.encrypt(unencrypted_string, salt)

expect do
Encryptor.decrypt(encrypted_string, salt, label: 'bar', iterations: encryption_iterations)
end.to raise_error(VCAP::CloudController::Encryptor::EncryptorError, %r{Encryption/Decryption failed: })
end
end
end
end
Expand Down