bn/bn_lib.c: make BN_bn2binpad computationally constant-time.

"Computationally constant-time" means that it might still leak
information about input's length, but only in cases when input
is missing complete BN_ULONG limbs. But even then leak is possible
only if attacker can observe memory access pattern with limb
granularity.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5254)

(cherry picked from commit 89d8aade5f4011ddeea7827f08ec544c914f275a)

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index 8fa9f2f09f4588e329c3c183db50c62252b11c18..ebad255e5bfb5f2149ebe4e69cb2dc4c2b37d346 100644 (file)
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -12,6 +12,7 @@
 #include "internal/cryptlib.h"
 #include "bn_lcl.h"
 #include <openssl/opensslconf.h>
+#include "internal/constant_time_locl.h"
 
 /* This stuff appears to be completely unused, so is deprecated */
 #if OPENSSL_API_COMPAT < 0x00908000L
@@ -497,24 +498,30 @@ BIGNUM *BN_bin2bn(const unsigned char *s, int len, BIGNUM *ret)
 /* ignore negative */
 static int bn2binpad(const BIGNUM *a, unsigned char *to, int tolen)
 {
-    int i;
+    int i, j, top;
     BN_ULONG l;
 
-    bn_check_top(a);
     i = BN_num_bytes(a);
     if (tolen == -1)
         tolen = i;
     else if (tolen < i)
         return -1;
-    /* Add leading zeroes if necessary */
-    if (tolen > i) {
-        memset(to, 0, tolen - i);
-        to += tolen - i;
+
+    if (i == 0) {
+        OPENSSL_cleanse(to, tolen);
+        return tolen;
     }
-    while (i--) {
+
+    top = a->top * BN_BYTES;
+    for (i = 0, j = tolen; j > 0; i++) {
+        unsigned int mask;
+
+        mask = constant_time_lt(i, top);
+        i -= 1 & ~mask; /* stay on top limb */
         l = a->d[i / BN_BYTES];
-        *(to++) = (unsigned char)(l >> (8 * (i % BN_BYTES))) & 0xff;
+        to[--j] = (unsigned char)(l >> (8 * (i % BN_BYTES)) & mask);
     }
+
     return tolen;
 }