74 lines
2.8 KiB
Diff
74 lines
2.8 KiB
Diff
From 919925673d6c9cfed3c1085497f5dfbbed5fc431 Mon Sep 17 00:00:00 2001
|
|
From: Alex Chernyakhovsky <achernya@google.com>
|
|
Date: Thu, 16 Jun 2022 12:00:22 +1000
|
|
Subject: [PATCH] Fix AES OCB encrypt/decrypt for x86 AES-NI
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
aesni_ocb_encrypt and aesni_ocb_decrypt operate by having a fast-path
|
|
that performs operations on 6 16-byte blocks concurrently (the
|
|
"grandloop") and then proceeds to handle the "short" tail (which can
|
|
be anywhere from 0 to 5 blocks) that remain.
|
|
|
|
As part of initialization, the assembly initializes $len to the true
|
|
length, less 96 bytes and converts it to a pointer so that the $inp
|
|
can be compared to it. Each iteration of "grandloop" checks to see if
|
|
there's a full 96-byte chunk to process, and if so, continues. Once
|
|
this has been exhausted, it falls through to "short", which handles
|
|
the remaining zero to five blocks.
|
|
|
|
Unfortunately, the jump at the end of "grandloop" had a fencepost
|
|
error, doing a `jb` ("jump below") rather than `jbe` (jump below or
|
|
equal). This should be `jbe`, as $inp is pointing to the *end* of the
|
|
chunk currently being handled. If $inp == $len, that means that
|
|
there's a whole 96-byte chunk waiting to be handled. If $inp > $len,
|
|
then there's 5 or fewer 16-byte blocks left to be handled, and the
|
|
fall-through is intended.
|
|
|
|
The net effect of `jb` instead of `jbe` is that the last 16-byte block
|
|
of the last 96-byte chunk was completely omitted. The contents of
|
|
`out` in this position were never written to. Additionally, since
|
|
those bytes were never processed, the authentication tag generated is
|
|
also incorrect.
|
|
|
|
The same fencepost error, and identical logic, exists in both
|
|
aesni_ocb_encrypt and aesni_ocb_decrypt.
|
|
|
|
This addresses CVE-2022-2097.
|
|
|
|
Co-authored-by: Alejandro Sedeño <asedeno@google.com>
|
|
Co-authored-by: David Benjamin <davidben@google.com>
|
|
|
|
Reviewed-by: Paul Dale <pauli@openssl.org>
|
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
|
---
|
|
crypto/aes/asm/aesni-x86.pl | 4 ++--
|
|
1 file changed, 2 insertions(+), 2 deletions(-)
|
|
|
|
diff --git a/crypto/aes/asm/aesni-x86.pl b/crypto/aes/asm/aesni-x86.pl
|
|
index fe2b265..812758e 100644
|
|
--- a/crypto/aes/asm/aesni-x86.pl
|
|
+++ b/crypto/aes/asm/aesni-x86.pl
|
|
@@ -2027,7 +2027,7 @@ my ($l_,$block,$i1,$i3,$i5) = ($rounds_,$key_,$rounds,$len,$out);
|
|
&movdqu (&QWP(-16*2,$out,$inp),$inout4);
|
|
&movdqu (&QWP(-16*1,$out,$inp),$inout5);
|
|
&cmp ($inp,$len); # done yet?
|
|
- &jb (&label("grandloop"));
|
|
+ &jbe (&label("grandloop"));
|
|
|
|
&set_label("short");
|
|
&add ($len,16*6);
|
|
@@ -2453,7 +2453,7 @@ my ($l_,$block,$i1,$i3,$i5) = ($rounds_,$key_,$rounds,$len,$out);
|
|
&pxor ($rndkey1,$inout5);
|
|
&movdqu (&QWP(-16*1,$out,$inp),$inout5);
|
|
&cmp ($inp,$len); # done yet?
|
|
- &jb (&label("grandloop"));
|
|
+ &jbe (&label("grandloop"));
|
|
|
|
&set_label("short");
|
|
&add ($len,16*6);
|
|
--
|
|
1.8.3.1
|
|
|