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

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

crypto/dh/dh_key.c

@@ -86,26 +86,53 @@ static int compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh)
         goto err;
     }
 
-    ret = BN_bn2bin(tmp, key);
+    /* return the padded key, i.e. same number of bytes as the modulus */
+    ret = BN_bn2binpad(tmp, key, BN_num_bytes(dh->params.p));
  err:
     BN_CTX_end(ctx);
     BN_CTX_free(ctx);
     return ret;
 }
 
+/*-
+ * 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)
 {
+    int ret = 0, i;
+    volatile size_t npad = 0, mask = 1;
+
+    /* compute the key; ret is constant unless compute_key is external */
 #ifdef FIPS_MODULE
-    return compute_key(key, pub_key, dh);
+    ret = compute_key(key, pub_key, dh);
 #else
-    return dh->meth->compute_key(key, pub_key, dh);
+    ret = dh->meth->compute_key(key, pub_key, dh);
 #endif
+    if (ret <= 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 */
 #ifdef FIPS_MODULE
     rv = compute_key(key, pub_key, dh);
 #else
@@ -114,6 +141,7 @@ int DH_compute_key_padded(unsigned char *key, const BIGNUM *pub_key, DH *dh)
     if (rv <= 0)
         return rv;
     pad = BN_num_bytes(dh->params.p) - rv;
+    /* pad is constant (zero) unless compute_key is external */
     if (pad > 0) {
         memmove(key + pad, key, rv);
         memset(key, 0, pad);

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
 
@@ -14,18 +15,20 @@ L<openssl_user_macros(7)>:
 
  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
 
-Both of the functions described on this page are deprecated.
+All of the functions described on this page are deprecated.
 Applications should instead use L<EVP_PKEY_derive_init(3)>
 and L<EVP_PKEY_derive(3)>.
 
 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
@@ -36,6 +39,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
 
@@ -44,6 +55,8 @@ 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
@@ -53,7 +66,9 @@ L<DH_new(3)>, L<ERR_get_error(3)>, L<RAND_bytes(3)>, L<DH_size(3)>
 
 =head1 HISTORY
 
-Both of these functions were deprecated in OpenSSL 3.0.
+DH_compute_key_padded() was added in OpenSSL 1.0.2.
+
+All of these functions were deprecated in OpenSSL 3.0.
 
 =head1 COPYRIGHT