autolab / Autolab-CLI

Command line autolab client that uses the Autolab API
3 stars 2 forks source link

Padding issue #29

Closed damianhxy closed 10 months ago

damianhxy commented 1 year ago

Occurring on AWS Autolab.

Error

pygmyshark:/afs/cs/academic/class/18213-m23/private/Autolab-CLI/build% src/autolab setup
read size 96
fatal: OpenSSL error
001E482E667F0000:error:1C800064:Provider routines:ossl_cipher_unpadblock:bad decrypt:../providers/implementations/ciphers/ciphercommon_block.c:124:

Fix

/afs/cs/academic/class/18213-m23/private/Autolab-CLI/src/crypto/pseudocrypto.cpp:
std::string decrypt_string(char *srctext, size_t srclength, unsigned char *key,
    unsigned char *iv) {
  check_key_and_iv_lengths(key, iv);
  EVP_CIPHER_CTX *ctx;
  unsigned char plaintext[MAX_CIPHERTEXT_LEN];
  int total_len = 0;
  int temp_len = 0;
  unsigned char *ciphertext = (unsigned char *)srctext;
  int input_len = (int)srclength;
  if (!(ctx = EVP_CIPHER_CTX_new()))
    exit_with_crypto_error();
  EVP_CIPHER_CTX_set_padding(ctx, 0); // GMK
  if (1 != EVP_DecryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, key, iv))
    exit_with_crypto_error();

Potential Explanation

I think the problem may be related to encrypt_string(...) at pseudocrypto.cpp:95: if (1 != EVP_EncryptInit_ex(ctx, EVP_aes_256_cbc(), NULL, key, iv)) exit_with_crypto_error(); I think EVP_aes_256_cbc() may be a legacy encoding. I don't know exactly how it pads, but I don't think it will add a block if ((SIZE/BLOCK_SIZE)==0). The decryption with padding uses padding a bit like a checksum. It expects each block to be padded with N bytes of value N, where N is the number of bytes needed to fill out the 256-byte block. So, for example, if the last block in the message has 250 bytes, 5 bytes would be added, each with a value of 0x5. But, to make this work there needs to be a checksum even if the data size is a multiple of the block size, so, in this case, it expects that, during the encryption, an entirely empty block, fully padded with 256 0xFF bytes, was added to the end of the message. It is this step that I don't think is being performed by the legacy encoding used in the encrypt_string(...) function. The result is that it looks at the last byte of the last block it gets and expects to see the number of bytes of padding in the block, but it sees, instead, the last byte of data. I think it then expects to find exactly that many bytes of exactly that value as the tail values of the block, but instead finds a bunch of arbitrary values, ie. the last bytes of the actual data. So, I think we can either turn off the check on padding or switch encryption to a more modern one. But, if we don't I think that, at least, 1 time out of 256 (and maybe every time), this error will occur. I haven't dug into this deeply. But, this is my hot take from a quick look at the encrypt_string() and decrypt_string() functions. Maybe this helps?

damianhxy commented 10 months ago

As far as I can tell, there shouldn't be a need to add the line: https://wiki.openssl.org/index.php/EVP_Symmetric_Encryption_and_Decryption

Furthermore, I am unable to reproduce the error even after removing the line.