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

Author: bukka

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:

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);
 				}

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)