Support constant BN for DH parameters

If BN_FLG_STATIC_DATA is set don't cleanse a->d as it will reside
in read only memory. If BN_FLG_MALLOCED is not set don't modify the
BIGNUM at all.

This change applies to BN_clear_free() and BN_free(). Now the BIGNUM
structure is opaque applications cannot create a BIGNUM structure
without BN_FLG_MALLOCED being set so they are unaffected.

Update internal DH routines so they only copy pointers for read only
parameters.

Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4485)

2 files changed, 17 insertions, 22 deletions

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index d22a83fc34..ad2a47e09f 100644
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -179,37 +179,26 @@ static void bn_free_d(BIGNUM *a)
 
 void BN_clear_free(BIGNUM *a)
 {
-    int i;
-
     if (a == NULL)
         return;
-    bn_check_top(a);
-    if (a->d != NULL) {
+    if (a->d != NULL && !BN_get_flags(a, BN_FLG_STATIC_DATA)) {
         OPENSSL_cleanse(a->d, a->dmax * sizeof(a->d[0]));
-        if (!BN_get_flags(a, BN_FLG_STATIC_DATA))
-            bn_free_d(a);
+        bn_free_d(a);
     }
-    i = BN_get_flags(a, BN_FLG_MALLOCED);
-    OPENSSL_cleanse(a, sizeof(*a));
-    if (i)
+    if (BN_get_flags(a, BN_FLG_MALLOCED)) {
+        OPENSSL_cleanse(a, sizeof(*a));
         OPENSSL_free(a);
+    }
 }
 
 void BN_free(BIGNUM *a)
 {
     if (a == NULL)
         return;
-    bn_check_top(a);
     if (!BN_get_flags(a, BN_FLG_STATIC_DATA))
         bn_free_d(a);
     if (a->flags & BN_FLG_MALLOCED)
         OPENSSL_free(a);
-    else {
-#if OPENSSL_API_COMPAT < 0x00908000L
-        a->flags |= BN_FLG_FREE;
-#endif
-        a->d = NULL;
-    }
 }
 
 void bn_init(BIGNUM *a)
diff --git a/crypto/dh/dh_ameth.c b/crypto/dh/dh_ameth.c
index cd77867dee..abf68aa25d 100644
--- a/crypto/dh/dh_ameth.c
+++ b/crypto/dh/dh_ameth.c
@@ -374,13 +374,19 @@ static int dh_cmp_parameters(const EVP_PKEY *a, const EVP_PKEY *b)
 static int int_dh_bn_cpy(BIGNUM **dst, const BIGNUM *src)
 {
     BIGNUM *a;
-    if (src) {
-        a = BN_dup(src);
-        if (!a)
-            return 0;
-    } else
+
+    /*
+     * If source is read only just copy the pointer, so
+     * we don't have to reallocate it.
+     */
+    if (src == NULL)
         a = NULL;
-    BN_free(*dst);
+    else if (BN_get_flags(src, BN_FLG_STATIC_DATA)
+                && !BN_get_flags(src, BN_FLG_MALLOCED))
+        a = (BIGNUM *)src;
+    else if ((a = BN_dup(src)) == NULL)
+        return 0;
+    BN_clear_free(*dst);
     *dst = a;
     return 1;
 }