Skip to content
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.

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

[match] improve encryption internals, solving flaky test (#21663) #21790

Merged
merged 22 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f6aeb5c
Rewrite the encryption layer for match, keeping backwards compatibility
lacostej Jan 10, 2024
1828882
match: add companion script to enc
lacostej Jan 10, 2024
4e68ebb
Update documentation related to encryption
lacostej Jan 10, 2024
c5d33cd
rubocop
lacostej Jan 10, 2024
e5b271a
Rename match_enc file
lacostej Jan 10, 2024
4332cc9
Remove deprecation warning from OpenSSL
lacostej Jan 10, 2024
8c04014
Attempt at lower casing the cipher to allow older rubies to find it
lacostej Jan 10, 2024
e07a6f2
address code review
lacostej Jan 11, 2024
7170033
Adjust CLI usage and arg checks
lacostej Jan 11, 2024
91f0b00
more doc cleanups
lacostej Jan 11, 2024
133d090
rubocop
lacostej Jan 11, 2024
471c3a0
Update match/lib/match/encryption/encryption.rb
lacostej Jan 15, 2024
cb496ec
Update match/lib/match/encryption/encryption.rb
lacostej Jan 15, 2024
850a472
Update bin/match_file
lacostej Jan 15, 2024
0d864ca
Update match/lib/match/encryption/encryption.rb
lacostej Jan 15, 2024
0394f34
Update match/lib/match/encryption/encryption.rb
lacostej Jan 15, 2024
035cbd1
Monkey patch match_file to support control-c :)
lacostej Jan 15, 2024
10405ee
Fix typo
lacostej Jan 15, 2024
237adc0
Use keyword parameters in main entry point for encryption
lacostej Jan 15, 2024
56578d8
Rubocop
lacostej Jan 16, 2024
7ed2e06
ctrl-c support requires ruby 2.7.1 apparently, so aborting instead
lacostej Jan 16, 2024
4c6f360
encryption: keyword parameters all the way, also add the test that go…
lacostej Jan 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 60 additions & 0 deletions bin/match_file
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#!/usr/bin/env ruby
require 'match'

# CLI to encrypt/decrypt files using fastlane match encryption layer

def usage
puts("USAGE: [encrypt|decrypt] input_path [output_path]")
exit(-1)
end

if ARGV.count < 2 || ARGV.count > 3
usage
end

method_name = ARGV.shift
unless ['encrypt', 'decrypt'].include?(method_name)
usage
end

input_file = ARGV.shift

if ARGV.count > 0
output_file = ARGV.shift
else
output_file = input_file
end

def ask_password(msg)
ask(msg) do |q|
q.whitespace = :chomp
q.echo = "*"
end
end

def ask_password_twice
password = ask_password("Enter the password: ")
return "" if password.empty? || password == "\u0003" # CTRL-C char
rogerluan marked this conversation as resolved.
Show resolved Hide resolved
other = ask_password("Enter the password again: ")
if other == password
return password
else
return nil
end
end

# read the password
password = nil
loop do
password = ask_password_twice
break unless password.nil?
rogerluan marked this conversation as resolved.
Show resolved Hide resolved
end

exit if password.empty?

begin
Match::Encryption::MatchFileEncryption.new.send(method_name, file_path: input_file, password: password, output_path: output_file)
rescue => e
puts("ERROR #{method_name}ing. [#{e}]. Check your password")
usage
end
8 changes: 6 additions & 2 deletions fastlane/lib/fastlane/actions/docs/sync_code_signing.md
Original file line number Diff line number Diff line change
Expand Up @@ -509,12 +509,16 @@ Please be careful when using this option and ensure the certificates and profile

### Manual Decrypt

If you want to manually decrypt a file you can.
If you want to manually decrypt or encrypt a file, you can use the companion script `match_file`:

```no-highlight
openssl aes-256-cbc -k "<password>" -in "<fileYouWantToDecryptPath>" -out "<decryptedFilePath>" -a -d -md [md5|sha256]
match_file encrypt "<fileYouWantToEncryptPath>" ["<encryptedFilePath>"]

match_file decrypt "<fileYouWantToDecryptPath>" ["<decryptedFilePath>"]
```

The password will be asked interactively.

_**Note:** You may need to swap double quotes `"` for single quotes `'` if your match password contains an exclamation mark `!`._

#### Export Distribution Certificate and Private Key as Single .p12 File
Expand Down
1 change: 1 addition & 0 deletions match/lib/match/encryption.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require_relative 'encryption/interface'
require_relative 'encryption/openssl'
require_relative 'encryption/encryption'

module Match
module Encryption
Expand Down
153 changes: 153 additions & 0 deletions match/lib/match/encryption/encryption.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
require 'base64'
require 'openssl'
require 'securerandom'

module Match
module Encryption
# This is to keep backwards compatibility with the old fastlane version which used the local openssl installation.
# The encryption parameters in this implementation reflect the old behavior which was the most common default value in those versions.
# As for decryption, 1.0.x OpenSSL and earlier versions use MD5, 1.1.0c and newer uses SHA256, we try both before giving an error
class EncryptionV1
ALGORITHM = 'aes-256-cbc'

def encrypt(data:, password:, salt:, hash_algorithm: "MD5")
cipher = ::OpenSSL::Cipher.new(ALGORITHM)
cipher.encrypt

keyivgen(cipher, password, salt, hash_algorithm)

encrypted_data = cipher.update(data)
encrypted_data << cipher.final
{ encrypted_data: encrypted_data }
end

def decrypt(encrypted_data:, password:, salt:, hash_algorithm: "MD5")
cipher = ::OpenSSL::Cipher.new(ALGORITHM)
cipher.decrypt

keyivgen(cipher, password, salt, hash_algorithm)

data = cipher.update(encrypted_data)
data << cipher.final
end

private

def keyivgen(cipher, password, salt, hash_algorithm)
cipher.pkcs5_keyivgen(password, salt, 1, hash_algorithm)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I considered increasing this very low iteration value, but changing the value requires changing the encryption format anyway, so let's just change the cipher etc.

end
end

# The newer encryption mechanism, which features a more secure key and IV generation.
#
# The IV is randomly generated and provided unencrypted.
# The salt should be randomly generated and provided unencrypted (like in the current implementation).
# The key is generated with OpenSSL::KDF::pbkdf2_hmac with properly chosen parameters.
#
# Short explanation about salt and IV: https://stackoverflow.com/a/1950674/6324550
class EncryptionV2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just as a suggestion but nothing blocking, I think this could be refactored so that the IV gen happens in one class per algorithm, and then there's a single encrypt/decrypt function that takes in the IV gen class. I think overall it would just cut down on the duplication a little bit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean as passing the ivgen function as a "strategy" to the same code that is shared between V1 and V2?

I thought about it, but decided against for now because

  • I preferred to keep V2 on its own, completely unrelated to common code with V1, to make it more readable.
  • There used to be a few lines, and now there are already 3 classes. I felt it was maybe enough abstractions

ALGORITHM = 'aes-256-gcm'

def encrypt(data:, password:, salt:)
cipher = ::OpenSSL::Cipher.new(ALGORITHM)
cipher.encrypt

keyivgen(cipher, password, salt)

encrypted_data = cipher.update(data)
encrypted_data << cipher.final

auth_tag = cipher.auth_tag

{ encrypted_data: encrypted_data, auth_tag: auth_tag }
end

def decrypt(encrypted_data:, password:, salt:, auth_tag:)
cipher = ::OpenSSL::Cipher.new(ALGORITHM)
cipher.decrypt

keyivgen(cipher, password, salt)

cipher.auth_tag = auth_tag

data = cipher.update(encrypted_data)
data << cipher.final
end

private

def keyivgen(cipher, password, salt)
keyIv = ::OpenSSL::KDF.pbkdf2_hmac(password, salt: salt, iterations: 10_000, length: 32 + 12 + 24, hash: "sha256")
key = keyIv[0..31]
iv = keyIv[32..43]
auth_data = keyIv[44..-1]
cipher.key = key
cipher.iv = iv
cipher.auth_data = auth_data
end
end

class MatchDataEncryption
V1_PREFIX = "Salted__"
V2_PREFIX = "match_encrypted_v2__"

def encrypt(data:, password:, version: 2)
salt = SecureRandom.random_bytes(8)
if version == 2
e = EncryptionV2.new
encryption = e.encrypt(data: data, password: password, salt: salt)
encrypted_data = V2_PREFIX + salt + encryption[:auth_tag] + encryption[:encrypted_data]
else
e = EncryptionV1.new
encryption = e.encrypt(data: data, password: password, salt: salt)
encrypted_data = V1_PREFIX + salt + encryption[:encrypted_data]
end
Base64.encode64(encrypted_data)
end

def decrypt(base64encoded_encrypted:, password:)
stored_data = Base64.decode64(base64encoded_encrypted)
if stored_data.start_with?(V2_PREFIX)
salt = stored_data[20..27]
auth_tag = stored_data[28..43]
data_to_decrypt = stored_data[44..-1]
e = EncryptionV2.new
e.decrypt(encrypted_data: data_to_decrypt, password: password, salt: salt, auth_tag: auth_tag)
else
salt = stored_data[8..15]
data_to_decrypt = stored_data[16..-1]
e = EncryptionV1.new
begin
e.decrypt(encrypted_data: data_to_decrypt, password: password, salt: salt)
rescue => _ex
# Note that we are not guaranteed to catch the decryption errors here if the password is wrong
# as there's no integrity checks.
# With a wrong password, there's a 0.4% chance it will decrypt garbage and not fail
lacostej marked this conversation as resolved.
Show resolved Hide resolved
# see https://github.com/fastlane/fastlane/issues/21663
lacostej marked this conversation as resolved.
Show resolved Hide resolved
fallback_hash_algorithm = "SHA256"
e.decrypt(encrypted_data: data_to_decrypt, password: password, salt: salt, hash_algorithm: fallback_hash_algorithm)
end
end
end
end

# The methods of this class will encrypt or decrypt files in place, by default.
class MatchFileEncryption
def encrypt(file_path:, password:, output_path: nil)
output_path = file_path unless output_path
data_to_encrypt = File.binread(file_path)
e = MatchDataEncryption.new
data = e.encrypt(data: data_to_encrypt, password: password)
File.write(output_path, data)
end

def decrypt(file_path:, password:, output_path: nil)
output_path = file_path unless output_path
content = File.read(file_path)
e = MatchDataEncryption.new
decrypted_data = e.decrypt(base64encoded_encrypted: content, password: password)
File.binwrite(output_path, decrypted_data)
end
end
end
end
45 changes: 7 additions & 38 deletions match/lib/match/encryption/openssl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,54 +109,23 @@ def fetch_password!
return password
end

# We encrypt with MD5 because that was the most common default value in older fastlane versions which used the local OpenSSL installation
# A more secure key and IV generation is needed in the future
# IV should be randomly generated and provided unencrypted
# salt should be randomly generated and provided unencrypted (like in the current implementation)
# key should be generated with OpenSSL::KDF::pbkdf2_hmac with properly chosen parameters
# Short explanation about salt and IV: https://stackoverflow.com/a/1950674/6324550
def encrypt_specific_file(path: nil, password: nil)
UI.user_error!("No password supplied") if password.to_s.strip.length == 0

data_to_encrypt = File.binread(path)
salt = SecureRandom.random_bytes(8)

# The :: is important, as there is a name clash
cipher = ::OpenSSL::Cipher.new('AES-256-CBC')
cipher.encrypt
cipher.pkcs5_keyivgen(password, salt, 1, "MD5")
encrypted_data = "Salted__" + salt + cipher.update(data_to_encrypt) + cipher.final

File.write(path, Base64.encode64(encrypted_data))
e = MatchFileEncryption.new
e.encrypt(file_path: path, password: password)
rescue FastlaneCore::Interface::FastlaneError
raise
rescue => error
UI.error(error.to_s)
UI.crash!("Error encrypting '#{path}'")
end

# The encryption parameters in this implementations reflect the old behavior which depended on the users' local OpenSSL version
# 1.0.x OpenSSL and earlier versions use MD5, 1.1.0c and newer uses SHA256, we try both before giving an error
def decrypt_specific_file(path: nil, password: nil, hash_algorithm: "MD5")
stored_data = Base64.decode64(File.read(path))
salt = stored_data[8..15]
data_to_decrypt = stored_data[16..-1]

decipher = ::OpenSSL::Cipher.new('AES-256-CBC')
decipher.decrypt
decipher.pkcs5_keyivgen(password, salt, 1, hash_algorithm)

decrypted_data = decipher.update(data_to_decrypt) + decipher.final

File.binwrite(path, decrypted_data)
def decrypt_specific_file(path: nil, password: nil)
e = MatchFileEncryption.new
e.decrypt(file_path: path, password: password)
rescue => error
fallback_hash_algorithm = "SHA256"
if hash_algorithm != fallback_hash_algorithm
decrypt_specific_file(path: path, password: password, hash_algorithm: fallback_hash_algorithm)
else
UI.error(error.to_s)
UI.crash!("Error decrypting '#{path}'")
end
UI.error(error.to_s)
UI.crash!("Error decrypting '#{path}'")
end
end
end
Expand Down
27 changes: 27 additions & 0 deletions match/spec/encryption/encryption_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
describe Match do
describe Match::Encryption::MatchDataEncryption do
let(:v1) { Match::Encryption::EncryptionV1.new }
let(:v2) { Match::Encryption::EncryptionV2.new }
let(:e) { Match::Encryption::MatchDataEncryption.new }
let(:salt) { salt = SecureRandom.random_bytes(8) }

let(:data) { "Hello World" }
let(:password) { '2"QAHg@v(Qp{=*n^' }

it "decrypts V1 Encryption with default hash" do
encryption = v1.encrypt(data: data, password: password, salt: salt)
encrypted_data = Match::Encryption::MatchDataEncryption::V1_PREFIX + salt + encryption[:encrypted_data]
encoded_encrypted_data = Base64.encode64(encrypted_data)

expect(e.decrypt(base64encoded_encrypted: encoded_encrypted_data, password: password)).to eq(data)
end

it "decrypts V1 Encryption with SHA256 hash" do
encryption = v1.encrypt(data: data, password: password, salt: salt, hash_algorithm: "SHA256")
encrypted_data = Match::Encryption::MatchDataEncryption::V1_PREFIX + salt + encryption[:encrypted_data]
encoded_encrypted_data = Base64.encode64(encrypted_data)

expect(e.decrypt(base64encoded_encrypted: encoded_encrypted_data, password: password)).to eq(data)
end
end
end