[PATCH] Fix buggy RC4 AMD64 assembly and add test to notice similar issues
Jussi Kivilinna
jussi.kivilinna at iki.fi
Thu Apr 30 16:04:05 CEST 2015
* cipher/arcfour-amd64.S (_gcry_arcfour_amd64): Fix swapped store of
'x' and 'y'.
* tests/basic.c (get_algo_mode_blklen): New.
(check_one_cipher_core): Add new tests for split buffer input on
encryption and decryption.
--
Reported-by: Dima Kukulniak <dima.ky at gmail.com>
Signed-off-by: Jussi Kivilinna <jussi.kivilinna at iki.fi>
---
cipher/arcfour-amd64.S | 4 +
tests/basic.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 154 insertions(+), 3 deletions(-)
diff --git a/cipher/arcfour-amd64.S b/cipher/arcfour-amd64.S
index c32cd6f..8b8031a 100644
--- a/cipher/arcfour-amd64.S
+++ b/cipher/arcfour-amd64.S
@@ -85,8 +85,8 @@ _gcry_arcfour_amd64:
.Lfinished:
dec %rcx # x--
- movb %dl, (4*256)(%rbp) # key->y = y
- movb %cl, (4*256+4)(%rbp) # key->x = x
+ movb %cl, (4*256)(%rbp) # key->y = y
+ movb %dl, (4*256+4)(%rbp) # key->x = x
pop %rbx
pop %rbp
ret
diff --git a/tests/basic.c b/tests/basic.c
index 1175b38..3d370d1 100644
--- a/tests/basic.c
+++ b/tests/basic.c
@@ -4676,6 +4676,26 @@ check_bulk_cipher_modes (void)
}
+static unsigned int get_algo_mode_blklen(int algo, int mode)
+{
+ unsigned int blklen = gcry_cipher_get_algo_blklen(algo);
+
+ /* Some modes override blklen. */
+ switch (mode)
+ {
+ case GCRY_CIPHER_MODE_STREAM:
+ case GCRY_CIPHER_MODE_OFB:
+ case GCRY_CIPHER_MODE_CTR:
+ case GCRY_CIPHER_MODE_CCM:
+ case GCRY_CIPHER_MODE_GCM:
+ case GCRY_CIPHER_MODE_POLY1305:
+ return 1;
+ }
+
+ return blklen;
+}
+
+
/* The core of the cipher check. In addition to the parameters passed
to check_one_cipher it also receives the KEY and the plain data.
PASS is printed with error messages. The function returns 0 on
@@ -4688,14 +4708,27 @@ check_one_cipher_core (int algo, int mode, int flags,
{
gcry_cipher_hd_t hd;
unsigned char in_buffer[1040+1], out_buffer[1040+1];
+ unsigned char enc_result[1040];
unsigned char *in, *out;
int keylen;
gcry_error_t err = 0;
+ unsigned int blklen;
+ unsigned int piecelen;
+ unsigned int pos;
+
+ blklen = get_algo_mode_blklen(algo, mode);
assert (nkey == 32);
assert (nplain == 1040);
assert (sizeof(in_buffer) == nplain + 1);
assert (sizeof(out_buffer) == sizeof(in_buffer));
+ assert (blklen > 0);
+
+ if (mode == GCRY_CIPHER_MODE_CBC && (flags & GCRY_CIPHER_CBC_CTS))
+ {
+ /* TODO: examine why CBC with CTS fails. */
+ blklen = nplain;
+ }
if (!bufshift)
{
@@ -4758,6 +4791,8 @@ check_one_cipher_core (int algo, int mode, int flags,
return -1;
}
+ memcpy (enc_result, out, nplain);
+
gcry_cipher_reset (hd);
err = gcry_cipher_decrypt (hd, in, nplain, out, nplain);
@@ -4787,6 +4822,10 @@ check_one_cipher_core (int algo, int mode, int flags,
return -1;
}
+ if (memcmp (enc_result, out, nplain))
+ fail ("pass %d, algo %d, mode %d, in-place, encrypt mismatch\n",
+ pass, algo, mode);
+
gcry_cipher_reset (hd);
err = gcry_cipher_decrypt (hd, out, nplain, NULL, 0);
@@ -4803,6 +4842,119 @@ check_one_cipher_core (int algo, int mode, int flags,
fail ("pass %d, algo %d, mode %d, in-place, encrypt-decrypt mismatch\n",
pass, algo, mode);
+ /* Again, splitting encryption in multiple operations. */
+ gcry_cipher_reset (hd);
+
+ piecelen = blklen;
+ pos = 0;
+ while (pos < nplain)
+ {
+ if (piecelen > nplain - pos)
+ piecelen = nplain - pos;
+
+ err = gcry_cipher_encrypt (hd, out + pos, piecelen, plain + pos,
+ piecelen);
+ if (err)
+ {
+ fail ("pass %d, algo %d, mode %d, split-buffer (pos: %d, "
+ "piecelen: %d), gcry_cipher_encrypt failed: %s\n",
+ pass, algo, mode, pos, piecelen, gpg_strerror (err));
+ gcry_cipher_close (hd);
+ return -1;
+ }
+
+ pos += piecelen;
+ piecelen = piecelen * 2 - ((piecelen != blklen) ? blklen : 0);
+ }
+
+ if (memcmp (enc_result, out, nplain))
+ fail ("pass %d, algo %d, mode %d, split-buffer, encrypt mismatch\n",
+ pass, algo, mode);
+
+ gcry_cipher_reset (hd);
+
+ piecelen = blklen;
+ pos = 0;
+ while (pos < nplain)
+ {
+ if (piecelen > nplain - pos)
+ piecelen = nplain - pos;
+
+ err = gcry_cipher_decrypt (hd, in + pos, piecelen, out + pos, piecelen);
+ if (err)
+ {
+ fail ("pass %d, algo %d, mode %d, split-buffer (pos: %d, "
+ "piecelen: %d), gcry_cipher_decrypt failed: %s\n",
+ pass, algo, mode, pos, piecelen, gpg_strerror (err));
+ gcry_cipher_close (hd);
+ return -1;
+ }
+
+ pos += piecelen;
+ piecelen = piecelen * 2 - ((piecelen != blklen) ? blklen : 0);
+ }
+
+ if (memcmp (plain, in, nplain))
+ fail ("pass %d, algo %d, mode %d, split-buffer, encrypt-decrypt mismatch\n",
+ pass, algo, mode);
+
+ /* Again, using in-place encryption and splitting encryption in multiple
+ * operations. */
+ gcry_cipher_reset (hd);
+
+ piecelen = blklen;
+ pos = 0;
+ while (pos < nplain)
+ {
+ if (piecelen > nplain - pos)
+ piecelen = nplain - pos;
+
+ memcpy (out + pos, plain + pos, piecelen);
+ err = gcry_cipher_encrypt (hd, out + pos, piecelen, NULL, 0);
+ if (err)
+ {
+ fail ("pass %d, algo %d, mode %d, in-place split-buffer (pos: %d, "
+ "piecelen: %d), gcry_cipher_encrypt failed: %s\n",
+ pass, algo, mode, pos, piecelen, gpg_strerror (err));
+ gcry_cipher_close (hd);
+ return -1;
+ }
+
+ pos += piecelen;
+ piecelen = piecelen * 2 - ((piecelen != blklen) ? blklen : 0);
+ }
+
+ if (memcmp (enc_result, out, nplain))
+ fail ("pass %d, algo %d, mode %d, in-place split-buffer, encrypt mismatch\n",
+ pass, algo, mode);
+
+ gcry_cipher_reset (hd);
+
+ piecelen = blklen;
+ pos = 0;
+ while (pos < nplain)
+ {
+ if (piecelen > nplain - pos)
+ piecelen = nplain - pos;
+
+ err = gcry_cipher_decrypt (hd, out + pos, piecelen, NULL, 0);
+ if (err)
+ {
+ fail ("pass %d, algo %d, mode %d, in-place split-buffer (pos: %d, "
+ "piecelen: %d), gcry_cipher_decrypt failed: %s\n",
+ pass, algo, mode, pos, piecelen, gpg_strerror (err));
+ gcry_cipher_close (hd);
+ return -1;
+ }
+
+ pos += piecelen;
+ piecelen = piecelen * 2 - ((piecelen != blklen) ? blklen : 0);
+ }
+
+ if (memcmp (plain, out, nplain))
+ fail ("pass %d, algo %d, mode %d, in-place split-buffer, encrypt-decrypt"
+ " mismatch\n", pass, algo, mode);
+
gcry_cipher_close (hd);
@@ -4810,7 +4962,6 @@ check_one_cipher_core (int algo, int mode, int flags,
}
-
static void
check_one_cipher (int algo, int mode, int flags)
{
More information about the Gcrypt-devel
mailing list