AES-256 and 192 produce garbage output with -engine padlock (VIA Padlock)

[ValdikSS]

OpenSSL fail to properly encrypt or decrypt AES-192 and AES-256 data (tested CTR and CBC modes), producing garbage output, without any errors, when using VIA Padlock hardware accelerator on VIA Eden Esther CPU.
128 key size modes return correct results.

Tested on OpenSSL 1.1.1n, 1.1.1s, 3.0.7

Without -engine padlock:

$ echo 12345678 | openssl enc -aes-256-ctr -e -nosalt -K 8d06b54b75a39a2ec1147871b4cdcb589525cde68034d39439183feedcf5e79a -iv 84eb471441a65cf6a27b3d6976728227 | hexdump -C
00000000  67 5b 8d ba 61 e2 07 84  79                       |g[..a...y|

$ echo -ne "\x67\x5b\x8d\xba\x61\xe2\x07\x84\x79" | openssl enc -aes-256-ctr -d -nosalt -K 8d06b54b75a39a2ec1147871b4cdcb589525cde68034d39439183feedcf5e79a -iv 84eb471441a65cf6a27b3d6976728227 | hexdump -C
00000000  31 32 33 34 35 36 37 38  0a                       |12345678.|

With -engine padlock:

# Encryption WITH engine padlock
$ echo 12345678 | openssl enc -aes-256-ctr -e -nosalt -K 8d06b54b75a39a2ec1147871b4cdcb589525cde68034d39439183feedcf5e79a -iv 84eb471441a65cf6a27b3d6976728227 -engine padlock | hexdump -C
engine "padlock" set.
00000000  79 86 a0 04 c7 c6 9f 36  80                       |y......6.|

# Decrypting result produced by engine padlock (garbage) WITH engine padlock
$ echo -ne "\x79\x86\xa0\x04\xc7\xc6\x9f\x36\x80" | openssl enc -aes-256-ctr -d -nosalt -K 8d06b54b75a39a2ec1147871b4cdcb589525cde68034d39439183feedcf5e79a -iv 84eb471441a65cf6a27b3d6976728227 -engine padlock | hexdump -C
engine "padlock" set.
00000000  31 32 33 34 35 36 37 38  0a                       |12345678.|

# Decrypting result produced by engine padlock (garbage) WITHOUT engine padlock
$ echo -ne "\x79\x86\xa0\x04\xc7\xc6\x9f\x36\x80" | openssl enc -aes-256-ctr -d -nosalt -K 8d06b54b75a39a2ec1147871b4cdcb589525cde68034d39439183feedcf5e79a -iv 84eb471441a65cf6a27b3d6976728227 | hexdump -C
00000000  2f ef 1e 8a 93 12 af 8a  f3                       |/........|

# Decrypting result produced WITHOUT engine padlock (proper AES) WITH engine padlock
$ echo -ne "\x67\x5b\x8d\xba\x61\xe2\x07\x84\x79" | openssl enc -aes-256-ctr -d -nosalt -K 8d06b54b75a39a2ec1147871b4cdcb589525cde68034d39439183feedcf5e79a -iv 84eb471441a65cf6a27b3d6976728227 -engine padlock | hexdump -C
engine "padlock" set.
00000000  2f ef 1e 8a 93 12 af 8a  f3                       |/........|

[bernd-edlinger]

Hello @ValdikSS
I was not aware that padlock hardware is still in use.

I think there is likely another problem if you configure
the 1.1.1 version with ./config -DOPENSSL_AES_CONST_TIME,
then my constant-time AES implementation replaces the AES_set_encrypt_key
and AES_set_decrypt_key functions with a different encoding, so I'd expect
the encryption to fail in a different way.

Since I am not able to do any testing with padlock, may I ask you
to test if my assumption about the AES CONST TIME code is correct.
And if that is the case if the following patch would fix it?

diff --git a/engines/e_padlock.c b/engines/e_padlock.c
index a82c07e813..bbff13b29c 100644
--- a/engines/e_padlock.c
+++ b/engines/e_padlock.c
@@ -221,6 +221,19 @@ void padlock_sha1(void *ctx, const void *inp, size_t len);
 void padlock_sha256_oneshot(void *ctx, const void *inp, size_t len);
 void padlock_sha256(void *ctx, const void *inp, size_t len);
 
+#  ifndef AES_ASM
+static int padlock_aes_set_encrypt_key(const unsigned char *userKey,
+                                       const int bits,
+                                       AES_KEY *key);
+static int padlock_aes_set_decrypt_key(const unsigned char *userKey,
+                                       const int bits,
+                                       AES_KEY *key);
+#   define AES_ASM
+#   define AES_set_encrypt_key padlock_aes_set_encrypt_key
+#   define AES_set_decrypt_key padlock_aes_set_decrypt_key
+#   include "../crypto/aes/aes_core.c"
+#endif
+
 /*
  * Load supported features of the CPU to see if the PadLock is available.
  */

While it is unfortunate that this uses non-constant time code for the key derivation
it is the most simple fix I could come up without access to real hardware.

[ValdikSS]

@bernd-edlinger, unfortunately it seem to produce exactly the same incorrect results with both with and without the patch when compiling with -DOPENSSL_AES_CONST_TIME.
If you have time and desire to fix this issue, I can provide you an SSH access to this machine. This is WYSE C10LE running Debian 11.
ssh -J valdikss@ssh-j.com user@via
Your SSH key is already added to the machine. Password for user and root is 1. You can do everything it needs.
The machine is slow, compilation takes a lot of time. You'd better compile .deb packages in a Debian Bullseye x86 (i386) container on your machine and transfer it to VIA.

[bernd-edlinger]

Ah, okay, the byte-swap is always necessary....
so make that

diff --git a/engines/e_padlock.c b/engines/e_padlock.c
index a82c07e..38327df 100644
--- a/engines/e_padlock.c
+++ b/engines/e_padlock.c
@@ -144,6 +144,19 @@ static int padlock_init(ENGINE *e)
     return (padlock_use_rng || padlock_use_ace);
 }
 
+#  ifndef AES_ASM
+static int padlock_aes_set_encrypt_key(const unsigned char *userKey,
+                                       const int bits,
+                                       AES_KEY *key);
+static int padlock_aes_set_decrypt_key(const unsigned char *userKey,
+                                       const int bits,
+                                       AES_KEY *key);
+#   define AES_ASM
+#   define AES_set_encrypt_key padlock_aes_set_encrypt_key
+#   define AES_set_decrypt_key padlock_aes_set_decrypt_key
+#   include "../crypto/aes/aes_core.c"
+#  endif
+
 /*
  * This stuff is needed if this ENGINE is being compiled into a
  * self-contained shared-library.
@@ -639,12 +652,10 @@ padlock_aes_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key,
             AES_set_decrypt_key(key, key_len, &cdata->ks);
         else
             AES_set_encrypt_key(key, key_len, &cdata->ks);
-#   ifndef AES_ASM
         /*
          * OpenSSL C functions use byte-swapped extended key.
          */
         padlock_key_bswap(&cdata->ks);
-#   endif
         cdata->cword.b.keygen = 1;
         break;
 

with that patch I get the following:

 echo -n 1234567812345678 | OPENSSL_ENGINES=../engines ../util/shlib_wrap.sh ./openssl enc -aes-256-cbc -e -nopad -K 8d06b54b75a39a2ec1147871b4cdcb589525cde68034d39439183feedcf5e79a -iv 84eb471441a65cf6a27b3d6976728227 | hexdump -C
00000000  26 e8 bf 94 1a 6f 5f 05  49 3e 0c 6e d8 0c 64 58  |&....o_.I>.n..dX|
00000010
$ echo -n 1234567812345678 | OPENSSL_ENGINES=../engines ../util/shlib_wrap.sh ./openssl enc -aes-256-cbc -e -nopad -K 8d06b54b75a39a2ec1147871b4cdcb589525cde68034d39439183feedcf5e79a -iv 84eb471441a65cf6a27b3d6976728227 -engine padlock | hexdump -C
engine "padlock" set.
00000000  26 e8 bf 94 1a 6f 5f 05  49 3e 0c 6e d8 0c 64 58  |&....o_.I>.n..dX|
00000010
$ echo -ne "\x26\xe8\xbf\x94\x1a\x6f\x5f\x05\x49\x3e\x0c\x6e\xd8\x0c\x64\x58" | OPENSSL_ENGINES=../engines ../util/shlib_wrap.sh ./openssl enc -aes-256-cbc -d -nopad -K 8d06b54b75a39a2ec1147871b4cdcb589525cde68034d39439183feedcf5e79a -iv 84eb471441a65cf6a27b3d6976728227 | hexdump -C
00000000  31 32 33 34 35 36 37 38  31 32 33 34 35 36 37 38  |1234567812345678|
00000010
$ echo -ne "\x26\xe8\xbf\x94\x1a\x6f\x5f\x05\x49\x3e\x0c\x6e\xd8\x0c\x64\x58" | OPENSSL_ENGINES=../engines ../util/shlib_wrap.sh ./openssl enc -aes-256-cbc -d -nopad -K 8d06b54b75a39a2ec1147871b4cdcb589525cde68034d39439183feedcf5e79a -iv 84eb471441a65cf6a27b3d6976728227 -engine padlock | hexdump -C
engine "padlock" set.
00000000  31 32 33 34 35 36 37 38  31 32 33 34 35 36 37 38  |1234567812345678|
00000010

Thanks a lot for your help!
I will send a pull request later today or tomorrow...

[ValdikSS]

Can confirm, when ifndef AES_ASM is removed for bswap it works with both -DOPENSSL_AES_CONST_TIME and without it.
Thank you!