Fix bug #72336 (openssl_pkey_new does not fail for invalid DSA params)

diff --git a/NEWS b/NEWS
index e24b39a5cc56edfc8d80b5c4da578d681798463a..b730703bc996bc9d0ef8deeff132a3b74cdcbe50 100644 (file)
--- 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 79f666acc5a6421eff70764446f5038e8c660907..da71d718ff2aa1ced7110866eea63672ae5f2f97 100644 (file)
--- 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 (file)
index 0000000..893b518
--- /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--
+<?php if (!extension_loaded("openssl")) print "skip"; ?>
+--FILE--
+<?php
+$p = '00f8000ae45b2dacb47dd977d58b719d097bdf07cb2c17660ad898518c08' .
+        '1a61659a16daadfaa406a0a994c743df5eda07e36bd0adcad921b77432ff' .
+        '24ccc31e782d647e66768122b578857e9293df78387dc8b44af2a4a3f305' .
+        '1f236b1000a3e31da489c6681b0031f7ec37c2e1091bdb698e7660f135b6' .
+        '996def90090303b7ad';
+
+$q = '009b3734fc9f7a4a9d6437ec314e0a78c2889af64b';
+
+$g = '00b320300a0bc55b8f0ec6edc218e2185250f38fbb8291db8a89227f6e41' .
+        '00d47d6ccb9c7d42fc43280ecc2ed386e81ff65bc5d6a2ae78db7372f5dc' .
+        'f780f4558e7ed3dd0c96a1b40727ac56c5165aed700a3b63997893a1fb21' .
+        '4e882221f0dd9604820dc34e2725dd6901c93e0ca56f6d76d495c332edc5' .
+        'b81747c4c447a941f3';
+
+var_dump(openssl_pkey_new(array('dsa' => array('p' => $p, 'q' => $q, 'g' => $g))));
+?>
+--EXPECT--
+bool(false)