From 54310d95f91bcd286e4bae67891402f782c1e767 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sun, 12 Jun 2016 17:57:08 +0100 Subject: [PATCH] Fix bug #72336 (openssl_pkey_new does not fail for invalid DSA params) --- NEWS | 4 +++ ext/openssl/openssl.c | 51 +++++++++++++++++++++++++++------ ext/openssl/tests/bug72336.phpt | 24 ++++++++++++++++ 3 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 ext/openssl/tests/bug72336.phpt diff --git a/NEWS b/NEWS index e24b39a5cc..b730703bc9 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? 2016, PHP 5.6.24 +- OpenSSL: + . Fixed bug #72336 (openssl_pkey_new does not fail for invalid DSA params). + (Jakub Zelenka) + 23 Jun 2016, PHP 5.6.23 - GD: diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c index 79f666acc5..da71d718ff 100644 --- a/ext/openssl/openssl.c +++ b/ext/openssl/openssl.c @@ -3531,6 +3531,44 @@ static int php_openssl_is_private_key(EVP_PKEY* pkey TSRMLS_DC) } \ } while (0); +/* {{{ php_openssl_pkey_init_dsa */ +zend_bool php_openssl_pkey_init_dsa(DSA *dsa) +{ + if (!dsa->p || !dsa->q || !dsa->g) { + return 0; + } + if (dsa->priv_key || dsa->pub_key) { + return 1; + } + if (!DSA_generate_key(dsa)) { + return 0; + } + /* if BN_mod_exp return -1, then DSA_generate_key succeed for failed key + * so we need to double check that public key is created */ + if (!dsa->pub_key || BN_is_zero(dsa->pub_key)) { + return 0; + } + /* all good */ + return 1; +} +/* }}} */ + +/* {{{ php_openssl_pkey_init_dh */ +zend_bool php_openssl_pkey_init_dh(DH *dh) +{ + if (!dh->p || !dh->g) { + return 0; + } + if (dh->pub_key) { + return 1; + } + if (!DH_generate_key(dh)) { + return 0; + } + /* all good */ + return 1; +} +/* }}} */ /* {{{ proto resource openssl_pkey_new([array configargs]) Generates a new private key */ @@ -3583,10 +3621,7 @@ PHP_FUNCTION(openssl_pkey_new) OPENSSL_PKEY_SET_BN(Z_ARRVAL_PP(data), dsa, g); OPENSSL_PKEY_SET_BN(Z_ARRVAL_PP(data), dsa, priv_key); OPENSSL_PKEY_SET_BN(Z_ARRVAL_PP(data), dsa, pub_key); - if (dsa->p && dsa->q && dsa->g) { - if (!dsa->priv_key && !dsa->pub_key) { - DSA_generate_key(dsa); - } + if (php_openssl_pkey_init_dsa(dsa)) { if (EVP_PKEY_assign_DSA(pkey, dsa)) { RETURN_RESOURCE(zend_list_insert(pkey, le_key TSRMLS_CC)); } @@ -3606,10 +3641,10 @@ PHP_FUNCTION(openssl_pkey_new) OPENSSL_PKEY_SET_BN(Z_ARRVAL_PP(data), dh, g); OPENSSL_PKEY_SET_BN(Z_ARRVAL_PP(data), dh, priv_key); OPENSSL_PKEY_SET_BN(Z_ARRVAL_PP(data), dh, pub_key); - if (dh->p && dh->g && - (dh->pub_key || DH_generate_key(dh)) && - EVP_PKEY_assign_DH(pkey, dh)) { - RETURN_RESOURCE(zend_list_insert(pkey, le_key TSRMLS_CC)); + if (php_openssl_pkey_init_dh(dh)) { + if (EVP_PKEY_assign_DH(pkey, dh)) { + RETURN_RESOURCE(zend_list_insert(pkey, le_key TSRMLS_CC)); + } } DH_free(dh); } diff --git a/ext/openssl/tests/bug72336.phpt b/ext/openssl/tests/bug72336.phpt new file mode 100644 index 0000000000..893b51838d --- /dev/null +++ b/ext/openssl/tests/bug72336.phpt @@ -0,0 +1,24 @@ +--TEST-- +Bug #72336 (openssl_pkey_new does not fail for invalid DSA params) +--SKIPIF-- + +--FILE-- + array('p' => $p, 'q' => $q, 'g' => $g)))); +?> +--EXPECT-- +bool(false) -- 2.40.0