From: Billy Brumley <bbrumley@gmail.com>
Date: Fri, 8 Jan 2021 11:45:49 +0000 (+0200)
Subject: [crypto/dh] side channel hardening for computing DH shared keys (1.1.1)
X-Git-Tag: OpenSSL_1_1_1j~31
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6e3ba20dc49ccbf12ff4c27a4d8b84dcbeb71654;p=thirdparty%2Fopenssl.git

[crypto/dh] side channel hardening for computing DH shared keys (1.1.1)

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/13772)
---

diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c
index daffdf74dd3..ccf51b3546e 100644
--- a/crypto/dh/dh_key.c
+++ b/crypto/dh/dh_key.c
@@ -25,18 +25,45 @@ int DH_generate_key(DH *dh)
     return dh->meth->generate_key(dh);
 }
 
+/*-
+ * NB: This function is inherently not constant time due to the
+ * RFC 5246 (8.1.2) padding style that strips leading zero bytes.
+ */
 int DH_compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh)
 {
-    return dh->meth->compute_key(key, pub_key, dh);
+    int ret = 0, i;
+    volatile size_t npad = 0, mask = 1;
+
+    /* compute the key; ret is constant unless compute_key is external */
+    if ((ret = dh->meth->compute_key(key, pub_key, dh)) <= 0)
+        return ret;
+
+    /* count leading zero bytes, yet still touch all bytes */
+    for (i = 0; i < ret; i++) {
+        mask &= !key[i];
+        npad += mask;
+    }
+
+    /* unpad key */
+    ret -= npad;
+    /* key-dependent memory access, potentially leaking npad / ret */
+    memmove(key, key + npad, ret);
+    /* key-dependent memory access, potentially leaking npad / ret */
+    memset(key + ret, 0, npad);
+
+    return ret;
 }
 
 int DH_compute_key_padded(unsigned char *key, const BIGNUM *pub_key, DH *dh)
 {
     int rv, pad;
+
+    /* rv is constant unless compute_key is external */
     rv = dh->meth->compute_key(key, pub_key, dh);
     if (rv <= 0)
         return rv;
     pad = BN_num_bytes(dh->p) - rv;
+    /* pad is constant (zero) unless compute_key is external */
     if (pad > 0) {
         memmove(key + pad, key, rv);
         memset(key, 0, pad);
@@ -212,7 +239,7 @@ static int compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh)
         goto err;
     }
 
-    ret = BN_bn2bin(tmp, key);
+    ret = BN_bn2binpad(tmp, key, BN_num_bytes(dh->p));
  err:
     BN_CTX_end(ctx);
     BN_CTX_free(ctx);
diff --git a/doc/man3/DH_generate_key.pod b/doc/man3/DH_generate_key.pod
index 297e7fbf47b..fab14d77e8b 100644
--- a/doc/man3/DH_generate_key.pod
+++ b/doc/man3/DH_generate_key.pod
@@ -2,7 +2,8 @@
 
 =head1 NAME
 
-DH_generate_key, DH_compute_key - perform Diffie-Hellman key exchange
+DH_generate_key, DH_compute_key, DH_compute_key_padded - perform
+Diffie-Hellman key exchange
 
 =head1 SYNOPSIS
 
@@ -10,14 +11,16 @@ DH_generate_key, DH_compute_key - perform Diffie-Hellman key exchange
 
  int DH_generate_key(DH *dh);
 
- int DH_compute_key(unsigned char *key, BIGNUM *pub_key, DH *dh);
+ int DH_compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh);
+
+ int DH_compute_key_padded(unsigned char *key, const BIGNUM *pub_key, DH *dh);
 
 =head1 DESCRIPTION
 
 DH_generate_key() performs the first step of a Diffie-Hellman key
 exchange by generating private and public DH values. By calling
-DH_compute_key(), these are combined with the other party's public
-value to compute the shared key.
+DH_compute_key() or DH_compute_key_padded(), these are combined with
+the other party's public value to compute the shared key.
 
 DH_generate_key() expects B<dh> to contain the shared parameters
 B<dh-E<gt>p> and B<dh-E<gt>g>. It generates a random private DH value
@@ -28,6 +31,14 @@ published.
 DH_compute_key() computes the shared secret from the private DH value
 in B<dh> and the other party's public value in B<pub_key> and stores
 it in B<key>. B<key> must point to B<DH_size(dh)> bytes of memory.
+The padding style is RFC 5246 (8.1.2) that strips leading zero bytes.
+It is not constant time due to the leading zero bytes being stripped.
+The return value should be considered public.
+
+DH_compute_key_padded() is similar but stores a fixed number of bytes.
+The padding style is NIST SP 800-56A (C.1) that retains leading zero bytes.
+It is constant time due to the leading zero bytes being retained.
+The return value should be considered public.
 
 =head1 RETURN VALUES
 
@@ -36,12 +47,18 @@ DH_generate_key() returns 1 on success, 0 otherwise.
 DH_compute_key() returns the size of the shared secret on success, -1
 on error.
 
+DH_compute_key_padded() returns B<DH_size(dh)> on success, -1 on error.
+
 The error codes can be obtained by L<ERR_get_error(3)>.
 
 =head1 SEE ALSO
 
 L<DH_new(3)>, L<ERR_get_error(3)>, L<RAND_bytes(3)>, L<DH_size(3)>
 
+=head1 HISTORY
+
+DH_compute_key_padded() was added in OpenSSL 1.0.2.
+
 =head1 COPYRIGHT
 
 Copyright 2000-2016 The OpenSSL Project Authors. All Rights Reserved.