From 0a024b25949406c5007030198ac6dc2c1354c80f Mon Sep 17 00:00:00 2001
From: Maarten Billemont <lhunath@lyndir.com>
Date: Sun, 24 Sep 2017 13:06:19 -0400
Subject: [PATCH] AES-CBC needs PKCS#7 padding.

---
 core/c/mpw-algorithm_v0.c | 10 +++++-----
 core/c/mpw-util.c         | 33 +++++++++++++++++++++++----------
 core/c/mpw-util.h         |  4 ++--
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/core/c/mpw-algorithm_v0.c b/core/c/mpw-algorithm_v0.c
index f7e93c0b..a6226edc 100644
--- a/core/c/mpw-algorithm_v0.c
+++ b/core/c/mpw-algorithm_v0.c
@@ -165,7 +165,7 @@ static const char *mpw_sitePasswordFromCrypt_v0(
 
     // Base64-decode
     uint8_t *cipherBuf = calloc( 1, mpw_base64_decode_max( cipherText ) );
-    size_t bufSize = (size_t)mpw_base64_decode( cipherBuf, cipherText );
+    size_t bufSize = (size_t)mpw_base64_decode( cipherBuf, cipherText ), cipherBufSize = bufSize;
     if ((int)bufSize < 0) {
         err( "Base64 decoding error." );
         mpw_free( &cipherBuf, mpw_base64_decode_max( cipherText ) );
@@ -174,13 +174,13 @@ static const char *mpw_sitePasswordFromCrypt_v0(
     trc( "b64 decoded: %zu bytes = %s\n", bufSize, mpw_hex( cipherBuf, bufSize ) );
 
     // Decrypt
-    const uint8_t *plainBytes = mpw_aes_decrypt( masterKey, MPMasterKeySize, cipherBuf, bufSize );
-    mpw_free( &cipherBuf, bufSize );
+    const uint8_t *plainBytes = mpw_aes_decrypt( masterKey, MPMasterKeySize, cipherBuf, &bufSize );
+    mpw_free( &cipherBuf, cipherBufSize );
     const char *plainText = strndup( (char *)plainBytes, bufSize );
     mpw_free( &plainBytes, bufSize );
     if (!plainText)
         err( "AES decryption error: %s\n", strerror( errno ) );
-    trc( "decrypted -> plainText: %zu bytes = %s = %s\n", sizeof( plainText ), plainText, mpw_hex( plainText, sizeof( plainText ) ) );
+    trc( "decrypted -> plainText: %zu bytes = %s = %s\n", strlen( plainText ), plainText, mpw_hex( plainText, strlen( plainText ) ) );
 
     return plainText;
 }
@@ -235,7 +235,7 @@ static const char *mpw_siteState_v0(
 
     // Encrypt
     size_t bufSize = strlen( plainText );
-    const uint8_t *cipherBuf = mpw_aes_encrypt( masterKey, MPMasterKeySize, (const uint8_t *)plainText, bufSize );
+    const uint8_t *cipherBuf = mpw_aes_encrypt( masterKey, MPMasterKeySize, (const uint8_t *)plainText, &bufSize );
     if (!cipherBuf) {
         err( "AES encryption error: %s\n", strerror( errno ) );
         return NULL;
diff --git a/core/c/mpw-util.c b/core/c/mpw-util.c
index 73c746c7..c07746d3 100644
--- a/core/c/mpw-util.c
+++ b/core/c/mpw-util.c
@@ -267,33 +267,46 @@ uint8_t const *mpw_hash_hmac_sha256(const uint8_t *key, const size_t keySize, co
     return mac;
 }
 
-static uint8_t const *mpw_aes(bool encrypt, const uint8_t *key, const size_t keySize, const uint8_t *buf, const size_t bufSize) {
+static uint8_t const *mpw_aes(bool encrypt, const uint8_t *key, const size_t keySize, const uint8_t *buf, size_t *bufSize) {
 
     if (!key || keySize < 16)
         return NULL;
 
+    // IV = zero
     uint8_t iv[16];
     bzero( (void *)iv, sizeof( iv ) );
-    uint8_t aesBuf[bufSize];
-    memcpy( aesBuf, buf, bufSize );
-    uint8_t *resultBuf = malloc( bufSize );
+
+    // Add PKCS#7 padding
+    uint32_t aesSize = (uint32_t)*bufSize;
+    if (encrypt)
+        aesSize = (aesSize / 16) * 16 + 16;
+    uint8_t aesBuf[aesSize];
+    memcpy( aesBuf, buf, *bufSize );
+    memset( aesBuf + *bufSize, aesSize - *bufSize, aesSize - *bufSize );
+    uint8_t *resultBuf = malloc( aesSize );
 
     if (encrypt)
-        AES_CBC_encrypt_buffer( resultBuf, aesBuf, (uint32_t)bufSize, key, iv );
+        AES_CBC_encrypt_buffer( resultBuf, aesBuf, aesSize, key, iv );
     else
-        AES_CBC_decrypt_buffer( resultBuf, aesBuf, (uint32_t)bufSize, key, iv );
-    bzero( aesBuf, bufSize );
-    bzero( iv, bufSize );
+        AES_CBC_decrypt_buffer( resultBuf, aesBuf, aesSize, key, iv );
+    bzero( aesBuf, aesSize );
+    bzero( iv, 16 );
+
+    // Truncate PKCS#7 padding
+    if (encrypt)
+        *bufSize = aesSize;
+    else
+        *bufSize -= resultBuf[aesSize - 1];
 
     return resultBuf;
 }
 
-uint8_t const *mpw_aes_encrypt(const uint8_t *key, const size_t keySize, const uint8_t *plainBuf, const size_t bufSize) {
+uint8_t const *mpw_aes_encrypt(const uint8_t *key, const size_t keySize, const uint8_t *plainBuf, size_t *bufSize) {
 
     return mpw_aes( true, key, keySize, plainBuf, bufSize );
 }
 
-uint8_t const *mpw_aes_decrypt(const uint8_t *key, const size_t keySize, const uint8_t *cipherBuf, const size_t bufSize) {
+uint8_t const *mpw_aes_decrypt(const uint8_t *key, const size_t keySize, const uint8_t *cipherBuf, size_t *bufSize) {
 
     return mpw_aes( false, key, keySize, cipherBuf, bufSize );
 }
diff --git a/core/c/mpw-util.h b/core/c/mpw-util.h
index 896ac215..2187ad78 100644
--- a/core/c/mpw-util.h
+++ b/core/c/mpw-util.h
@@ -173,11 +173,11 @@ uint8_t const *mpw_hash_hmac_sha256(
 /** Encrypt a plainBuf with the given key using AES-128-CBC.
   * @return A new bufSize allocated buffer containing the cipherBuf. */
 uint8_t const *mpw_aes_encrypt(
-        const uint8_t *key, const size_t keySize, const uint8_t *plainBuf, const size_t bufSize);
+        const uint8_t *key, const size_t keySize, const uint8_t *plainBuf, size_t *bufSize);
 /** Decrypt a cipherBuf with the given key using AES-128-CBC.
   * @return A new bufSize allocated buffer containing the plainBuf. */
 uint8_t const *mpw_aes_decrypt(
-        const uint8_t *key, const size_t keySize, const uint8_t *cipherBuf, const size_t bufSize);
+        const uint8_t *key, const size_t keySize, const uint8_t *cipherBuf, size_t *bufSize);
 /** Calculate an OTP using RFC-4226.
   * @return A newly allocated string containing exactly `digits` decimal OTP digits. */
 #if UNUSED