Improve error when EncryptedFile key length wrong

When given a key of invalid length, OpenSSL::Cipher raises a "key must
be X bytes" error.  However, EncryptedFile keys are packed before they
are passed to OpenSSL::Cipher, so the actual length requirement is "2*X
characters".

This commit checks the key length, and raises a more helpful error if
the key length is invalid.

Closes #39528.
This commit is contained in:
Jonathan Hefner 2020-06-05 16:34:07 -05:00
parent f624ed09ab
commit 0fc7448fc5
2 changed files with 33 additions and 1 deletions

@ -20,12 +20,22 @@ def initialize(key_path:, env_key:)
end
end
class InvalidKeyLengthError < RuntimeError
def initialize
super "Encryption key must be exactly #{EncryptedFile.expected_key_length} characters."
end
end
CIPHER = "aes-128-gcm"
def self.generate_key
SecureRandom.hex(ActiveSupport::MessageEncryptor.key_len(CIPHER))
end
def self.expected_key_length # :nodoc:
@expected_key_length ||= generate_key.length
end
attr_reader :content_path, :key_path, :env_key, :raise_if_missing_key
@ -74,6 +84,7 @@ def writing(contents)
def encrypt(contents)
check_key_length
encryptor.encrypt_and_sign contents
end
@ -91,11 +102,16 @@ def read_env_key
end
def read_key_file
key_path.binread.strip if key_path.exist?
return @key_file_contents if defined?(@key_file_contents)
@key_file_contents = (key_path.binread.strip if key_path.exist?)
end
def handle_missing_key
raise MissingKeyError.new(key_path: key_path, env_key: env_key) if raise_if_missing_key
end
def check_key_length
raise InvalidKeyLengthError if key&.length != self.class.expected_key_length
end
end
end

@ -55,6 +55,22 @@ class EncryptedFileTest < ActiveSupport::TestCase
end
end
test "raise InvalidKeyLengthError when key is too short" do
File.write(@key_path, ActiveSupport::EncryptedFile.generate_key[0..-2])
assert_raise ActiveSupport::EncryptedFile::InvalidKeyLengthError do
@encrypted_file.write(@content)
end
end
test "raise InvalidKeyLengthError when key is too long" do
File.write(@key_path, ActiveSupport::EncryptedFile.generate_key + "0")
assert_raise ActiveSupport::EncryptedFile::InvalidKeyLengthError do
@encrypted_file.write(@content)
end
end
test "respects existing content_path symlink" do
@encrypted_file.write(@content)