From 5f2d9c4d26c923ece6ee04616a2d32a1afc124c8 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Sun, 8 Oct 2017 21:04:05 +0100 Subject: [PATCH] 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 (Merged from https://github.com/openssl/openssl/pull/4485) --- crypto/bn/bn_lib.c | 21 +++++---------------- crypto/dh/dh_ameth.c | 18 ++++++++++++------ 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; } -- 2.25.1