match encryption is outdated and doesn't guarantee it detects invalid passwords. Causes flaky test.

[lacostej]

I've seen this fail twice already on master.

E.g. https://app.circleci.com/pipelines/github/fastlane/fastlane/5764/workflows/bec22718-cc50-4a05-9952-1615d1d76389/jobs/92676

 1) Match Match::Encryption::OpenSSL raises an exception if invalid password is passed
     Failure/Error:
       expect do
         @e.decrypt_files
       end.to raise_error("Invalid password passed via 'MATCH_PASSWORD'")

       expected Exception with "Invalid password passed via 'MATCH_PASSWORD'" but nothing was raised
     # ./match/spec/encryption/openssl_spec.rb:33:in `block (3 levels) in <top (required)>'
     # ./.bundle/ruby/2.6.0/gems/webmock-3.12.1/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'

Finished in 2 minutes 5.9 seconds (files took 3.34 seconds to load)
7267 examples, 1 failure, 329 pending

Failed examples:

rspec ./match/spec/encryption/openssl_spec.rb:27 # Match Match::Encryption::OpenSSL raises an exception if invalid password is passed

Investigating...

[lacostej]

Failed again in https://ci.appveyor.com/project/KrauseFx/fastlane/builds/48636131 as part of #21672

[lacostej]

Just saw this fail during the last stable release.

I have a WIP branch to reproduce the issue, which can be isolated to that very test.

see https://github.com/lacostej/fastlane/tree/fix/investigate_openssl_issue

Run DEBUG=y bundle exec rspec match/spec/encryption/openssl_spec.rb

It opens the interactive prompt when the test is about to fail.

Not yet sure why.

It looks like

• only this test is needed to fail. Just run it many times in a row and it will eventually fail
• the right parameters are passed
• the decryption doesn't fail

But it looks like we are getting garbage out of the decryption. I used a plain text file, and it produces garbage out.

[lacostej]

I’ve found something. It is related to the salt and the password length. For passwords smaller than 8, some given salts will 100% cause decryption to be garbage.

For our 7 letters wrong password, I’ve run the test 100000 times and it generated about 800 salts that break the decryption. The same salts do not break with a different password.

[lacostej]

require 'openssl'
require 'securerandom'
require 'base64'

class Encryption
	attr_accessor :algorithm

	def initialize
		self.algorithm = 'aes-256-cbc'
	end

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

		cipher.pkcs5_keyivgen(password, salt, 1, "MD5")
		encrypted_data = cipher.update(data)
		encrypted_data << cipher.final
	end
	def decrypt(encrypted_data, password, salt)
		cipher = OpenSSL::Cipher::Cipher.new(self.algorithm)
		cipher.decrypt
		cipher.pkcs5_keyivgen(password, salt, 1, "MD5")
		data = cipher.update(encrypted_data)
		data << cipher.final
	end
end


def run_test0(data)
	password='2"QAHg@v(Qp{=*n^'
	wrong_password="#{password}x"

	salt = SecureRandom.random_bytes(8)
	# test with a specific known broken salt with the given small wrong password
	#salt = [247, 107, 154, 246, 0, 151, 25, 194].pack("C*")

	e = Encryption.new
	encrypted_data = e.encrypt(data, password, salt)

	# uncomment to verify that decryption works
	# decrypted_data = e.decrypt(encrypted_data, password, salt)
	# raise "Wrong data" unless decrypted_data == data

	begin
		decrypted_data = e.decrypt(encrypted_data, wrong_password, salt)
		raise "ERROR: salt #{salt.unpack("C*")}"
	rescue OpenSSL::Cipher::CipherError => e
		# expected "bad decrypt"
		raise "ERROR: #{e} salt #{salt.unpack("C*")}" unless e.to_s == "bad decrypt"
	end
end

text='a text file with multiple lines'
DATA = 10.times.map{|i| text}.join("\n")

RUNS=2000

puts "Running test 0"
RUNS.times do |i|
	begin
		run_test0(DATA)	
	rescue => e
		puts "#{i}: #{e}"
	end
end

The password length doesn't matter. This code reproduces the problem, and it fails around 400 times per 100000 runs.

[lacostej]

#include <openssl/ssl.h>
#include <openssl/conf.h>
#include <openssl/err.h>
#include <openssl/crypto.h>
#include <openssl/evp.h>
#include <openssl/rand.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <time.h>

#define AES_256_KEY_SIZE 32
#define AES_BLOCK_SIZE 16
#define BUFSIZE 1024

void handleErrors() {
    // ERR_print_errors_fp(stderr);
    //abort();
}

int encrypt(EVP_CIPHER_CTX * ctx, unsigned char *plaintext, int plaintext_len, unsigned char *key,
            unsigned char *iv, unsigned char *ciphertext) {
    int len;
    int ciphertext_len;

    if(1 != EVP_EncryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, key, iv)) {
        handleErrors();
        return -1;
    }

    if(1 != EVP_EncryptUpdate(ctx, ciphertext, &len, plaintext, plaintext_len)) {
        handleErrors();
        return -1;
    }
    ciphertext_len = len;

    if(1 != EVP_EncryptFinal_ex(ctx, ciphertext + len, &len)) {
        handleErrors();
        return -1;
    }        
    ciphertext_len += len;

    return ciphertext_len;
}

int decrypt(EVP_CIPHER_CTX * ctx, unsigned char *ciphertext, int ciphertext_len, unsigned char *key,
            unsigned char *iv, unsigned char *plaintext) {
    int len;
    int plaintext_len;

    if(1 != EVP_DecryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, key, iv)) {
        handleErrors();
        return -1;
    }

    if(1 != EVP_DecryptUpdate(ctx, plaintext, &len, ciphertext, ciphertext_len)) {
        handleErrors();
        return -1;
    }
    plaintext_len = len;

    if(1 != EVP_DecryptFinal_ex(ctx, plaintext + len, &len)) {
        handleErrors();
        return -1;
    }
    plaintext_len += len;

    return plaintext_len;
}

void pkcs5_keyivgen(EVP_CIPHER_CTX * ctx, unsigned char * pass, unsigned char * salt, unsigned char *key, unsigned char *iv) {

    int iter = 1;
    const EVP_MD * digest = EVP_md5();

    EVP_BytesToKey(EVP_aes_256_cbc(), digest, salt,
                   pass, strlen((const char *)pass), iter, key, iv);
}

void random_keyivgen(unsigned char *key, unsigned char *iv) {
    // Generating a random key
    if(1 != RAND_bytes(key, sizeof(key))) {
        handleErrors();
    }

    // Generating a random IV
    if(1 != RAND_bytes(iv, sizeof(iv))) {
        handleErrors();
    }
}



void generateSaltString(unsigned char *salt, int saltSize, char *output) {
    int offset = 0;
    offset += sprintf(output + offset, "[");

    for (int i = 0; i < saltSize; ++i) {
        offset += sprintf(output + offset, "%d", salt[i]);
        if (i < saltSize - 1) {
            offset += sprintf(output + offset, ", ");
        }
    }

    sprintf(output + offset, "]");
}


void run_test0(unsigned char *data) {
    unsigned char key[AES_256_KEY_SIZE]; // 32 bytes for AES-256 key
    unsigned char iv[AES_BLOCK_SIZE]; // IV for AES
    unsigned char *encrypted_data;
    unsigned char *decrypted_data;

    unsigned char salt[8];

    EVP_CIPHER_CTX *ctx;

    unsigned char *password = (unsigned char *) "apassword"; //"2\"QAHg@v(Qp{=*n^";
    unsigned char *wrong_password = (unsigned char *)"invalid";

    // Generating a random salt
    if(1 != RAND_bytes(salt, sizeof(salt))) {
        handleErrors();
    }

    encrypted_data = (unsigned char *)malloc(BUFSIZE);
    decrypted_data = (unsigned char *)malloc(BUFSIZE);

    if(!(ctx = EVP_CIPHER_CTX_new())) handleErrors();

    //random_keyivgen(key, iv);
    pkcs5_keyivgen(ctx, password, salt, key, iv);

    // Encrypt the data
    int encrypted_data_len = encrypt(ctx, data, strlen((char *)data), key, iv, encrypted_data);

    EVP_CIPHER_CTX_free(ctx);

    if(!(ctx = EVP_CIPHER_CTX_new())) handleErrors();

    pkcs5_keyivgen(ctx, wrong_password, salt, key, iv);

    // Decrypt the data
    int decrypted_data_len = decrypt(ctx, encrypted_data, encrypted_data_len, key, iv, decrypted_data);

    if (decrypted_data_len != -1) {
        char saltString[100];
        generateSaltString(salt, sizeof(salt), saltString);
        printf("ERROR: salt: %s\n", saltString);
    }

    EVP_CIPHER_CTX_free(ctx);

    decrypted_data[decrypted_data_len] = '\0';

    printf("Original: %s\n", data);
    printf("Decrypted: %s\n", decrypted_data);

    free(encrypted_data);
    free(decrypted_data);
}

int main(void) {
    // Initialize OpenSSL
    ERR_load_crypto_strings();
    OpenSSL_add_all_algorithms();
#if OPENSSL_VERSION_NUMBER >= 0x10100003L
    if (OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL) == 0) {
        printf("Failed init");
        return -1;
    }
#else
    OPENSSL_config(NULL);
#endif

    unsigned char *data = (unsigned char *)"Your data here";

    for (int i = 0; i < 10000; i++) {
        run_test0(data);
    }

    // Clean up
    EVP_cleanup();
    ERR_free_strings();

    return 0;
}

Same code in C:

openssldir=`brew --prefix openssl`
gcc test_openssl.c -o test_openssl -L$openssldir/lib -I$openssldir/include -lcrypto -lssl

sh build && ./test_openssl  | grep ERROR | wc -l

gives me about 40 errors on each run.

[lacostej]

Here’s an incomplete proposed migration to our current encryption mechanics. I’ve asked feedback from openssl team to see if the security makes sense.

Anyone else has some feedback?

[lacostej]

One thing is that the documentation contains some hints on how to manually decrypt files, which wouldn't work in the new implementation. Is it an important feature to preserve?

https://github.com/fastlane/fastlane/blob/master/fastlane/lib/fastlane/actions/docs/sync_code_signing.md#manual-decrypt

[triplef]

FYI I got an error wrong final block length after updating my certificates using fastlane 2.220.0 but then using that repo with 2.219.0 on our CI. Updating fastlane on the CI fixed this for me. I am guessing this was caused by this change, so I just wanted to leave this here in case anyone else is running into this.

[kahest]

This is probably related to ^: #21973

[airmikec]

### Azure DevOps API Key Issues for deploying to Apple
To get this to successfully work on Azure DevOps Pipeline with an API Key. I ran into so many problems and tried everything I could find and this finally worked. Azure DevOps says you need use the base64 key. Well if you open up the .p8 and copy the base64 encoded string in there, that isn't what they want. They literally want you to copy all of the contenxt in the .p8 file to a base64 string. Don't modify the .p8 file. I found the solution at the follow source, and once you get the data.b64 file open it and copy the base64 string in that file. In the following article is discusses how to do this for Windows, Mac, and Ubuntu.

#21531 (comment)