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

…1790)

* Rewrite the encryption layer for match, keeping backwards compatibility

* match: add companion script to enc

* Update documentation related to encryption

* rubocop

* Rename match_enc file

* Remove deprecation warning from OpenSSL

* Attempt at lower casing the cipher to allow older rubies to find it

* address code review

* Adjust CLI usage and arg checks

* more doc cleanups

* rubocop

* Update match/lib/match/encryption/encryption.rb

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

* Update match/lib/match/encryption/encryption.rb

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

* Update bin/match_file

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

* Update match/lib/match/encryption/encryption.rb

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

* Update match/lib/match/encryption/encryption.rb

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

* Monkey patch match_file to support control-c :)

* Fix typo

* Use keyword parameters in main entry point for encryption

* Rubocop

* ctrl-c support requires ruby 2.7.1 apparently, so aborting instead

* encryption: keyword parameters all the way, also add the test that got lost

---------

Co-authored-by: Roger Oba <rogerluan.oba@gmail.com>

bin/match_file

@@ -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
+  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?
+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

fastlane/lib/fastlane/actions/docs/sync_code_signing.md

@@ -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

match/lib/match/encryption.rb

@@ -1,5 +1,6 @@
 require_relative 'encryption/interface'
 require_relative 'encryption/openssl'
+require_relative 'encryption/encryption'
 
 module Match
   module Encryption

match/lib/match/encryption/encryption.rb

@@ -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)
+      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
+      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
+            # see https://github.com/fastlane/fastlane/issues/21663
+            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

match/lib/match/encryption/openssl.rb

@@ -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

match/spec/encryption/encryption_spec.rb

@@ -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