Fix bug #80269: OpenSSL sets Subject wrong with extraattribs parameter

Closes phpGH-12979
devnexen · Dec 21, 2023 · e8fde6b · e8fde6b
1 parent 6c0d559
commit e8fde6b
Show file tree

Hide file tree

Showing 6 changed files with 138 additions and 5 deletions.
diff --git a/NEWS b/NEWS
@@ -41,6 +41,10 @@ Opcache:
   . If JIT is enabled, PHP will now exit with a fatal error on startup in case
     of JIT startup initialization issues. (danog)
 
+OpenSSL:
+  . Fixed bug #80269 (OpenSSL sets Subject wrong with extraattribs parameter).
+    (Jakub Zelenka)
+
 PDO:
   . Fixed setAttribute and getAttribute (SakiTakamachi)
 

diff --git a/UPGRADING b/UPGRADING
@@ -226,6 +226,10 @@ PHP 8.4 UPGRADE NOTES
   . The behavior of mb_strcut is more consistent now on invalid UTF-8 and UTF-16
     strings. (For valid UTF-8 and UTF-16 strings, there is no change.)
 
+- OpenSSL:
+  . The extra_attributes parameter in openssl_csr_new sets CSR attributes
+    instead of subject DN which was incorrectly done previously.
+
 - PDO:
   . getAttribute, enabled to get the value of ATTR_STRINGIFY_FETCHES.
 

diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c
@@ -2981,7 +2981,7 @@ static int php_openssl_make_REQ(struct php_x509_request * req, X509_REQ * csr, z
 				int nid;
 
 				if (NULL == strindex) {
-					php_error_docref(NULL, E_WARNING, "dn: numeric fild names are not supported");
+					php_error_docref(NULL, E_WARNING, "attributes: numeric fild names are not supported");
 					continue;
 				}
 
@@ -2991,15 +2991,15 @@ static int php_openssl_make_REQ(struct php_x509_request * req, X509_REQ * csr, z
 					if (UNEXPECTED(!str_item)) {
 						return FAILURE;
 					}
-					if (!X509_NAME_add_entry_by_NID(subj, nid, MBSTRING_UTF8, (unsigned char*)ZSTR_VAL(str_item), -1, -1, 0)) {
+					if (!X509_REQ_add1_attr_by_NID(csr, nid, MBSTRING_UTF8, (unsigned char*)ZSTR_VAL(str_item), (int)ZSTR_LEN(str_item))) {
 						php_openssl_store_errors();
-						php_error_docref(NULL, E_WARNING, "attribs: add_entry_by_NID %d -> %s (failed)", nid, ZSTR_VAL(str_item));
+						php_error_docref(NULL, E_WARNING, "attributes: add_attr_by_NID %d -> %s (failed)", nid, ZSTR_VAL(str_item));
 						zend_string_release(str_item);
 						return FAILURE;
 					}
 					zend_string_release(str_item);
 				} else {
-					php_error_docref(NULL, E_WARNING, "dn: %s is not a recognized name", ZSTR_VAL(strindex));
+					php_error_docref(NULL, E_WARNING, "attributes: %s is not a recognized attribute name", ZSTR_VAL(strindex));
 				}
 			} ZEND_HASH_FOREACH_END();
 			for (i = 0; i < sk_CONF_VALUE_num(attr_sk); i++) {

diff --git a/ext/openssl/tests/bug72165.phpt b/ext/openssl/tests/bug72165.phpt
@@ -9,6 +9,6 @@ $options = ['config' => __DIR__ . DIRECTORY_SEPARATOR . 'openssl.cnf'];
 $var2 = openssl_csr_new([0], $var0, $options, [0]);
 ?>
 --EXPECTF--
-Warning: openssl_csr_new(): dn: numeric fild names are not supported in %sbug72165.php on line %d
+Warning: openssl_csr_new(): attributes: numeric fild names are not supported in %sbug72165.php on line %d
 
 Warning: openssl_csr_new(): add1_attr_by_txt challengePassword_min -> 4 (failed; check error queue and value of string_mask OpenSSL option if illegal characters are reported) in %sbug72165.php on line %d
diff --git a/ext/openssl/tests/openssl_csr_attribs.cnf b/ext/openssl/tests/openssl_csr_attribs.cnf
@@ -0,0 +1,31 @@
+oid_section = new_oids
+[ new_oids ]
+aansluitNummer = 1.3.6.1.4.1.11278.1150.2.1
+kvkNummer = 1.3.6.1.4.1.11278.1150.2.2
+
+[ req ]
+default_bits = 2048
+distinguished_name = req_distinguished_name
+attributes = req_attributes
+req_extensions = v3_req
+prompt = no
+
+[ req_distinguished_name ]
+C = NL
+ST = ST
+L = L
+O = O
+CN = test
+
+[ req_attributes ]
+facsimileTelephoneNumber =
+postalCode =
+streetAddress =
+name = Organisation
+telephoneNumber = 012345678
+aansluitNummer = 1234
+kvkNummer = 12345678
+emailAddress = [email protected]
+
+[ v3_req ]
+basicConstraints = CA:FALSE
diff --git a/ext/openssl/tests/openssl_csr_new_with_attribs.phpt b/ext/openssl/tests/openssl_csr_new_with_attribs.phpt
@@ -0,0 +1,94 @@
+--TEST--
+openssl_csr_new() attributes setting tests
+--EXTENSIONS--
+openssl
+--FILE--
+<?php
+
+$dn = array(
+    "countryName" => "UK",
+    "stateOrProvinceName" => "England",
+    "localityName" => "London",
+    "commonName" => "test.php.net",
+    "emailAddress" => "[email protected]"
+);
+
+
+$config = __DIR__ . DIRECTORY_SEPARATOR . 'openssl_csr_attribs.cnf';
+
+$config_arg = array('config' => $config);
+
+$args = array(
+    "digest_alg" => "sha256",
+    "private_key_bits" => 2048,
+    "private_key_type" => OPENSSL_KEYTYPE_DSA,
+    "encrypt_key" => true,
+    "config" => $config,
+);
+
+$privkey = 'file://' . __DIR__ . '/private_rsa_2048.key';
+
+$csr = openssl_csr_new(
+    $dn,
+    $privkey,
+    $args,
+    [
+        'emailAddress' => '[email protected]',
+        'aansluitNummer' => '11112222',
+        'postalCode' => 'N11',
+    ]
+);
+
+
+var_dump(openssl_csr_get_subject($csr));
+var_dump(openssl_csr_export($csr, $output));
+var_dump($output);
+
+var_dump(openssl_csr_new(
+    $dn,
+    $privkey,
+    $args,
+    ['wrong' => '[email protected]']
+));
+
+?>
+--EXPECTF--
+array(5) {
+  ["C"]=>
+  string(2) "UK"
+  ["ST"]=>
+  string(7) "England"
+  ["L"]=>
+  string(6) "London"
+  ["CN"]=>
+  string(12) "test.php.net"
+  ["emailAddress"]=>
+  string(16) "[email protected]"
+}
+bool(true)
+string(1269) "-----BEGIN CERTIFICATE REQUEST-----
+MIIDcDCCAlgCAQAwaDELMAkGA1UEBhMCVUsxEDAOBgNVBAgMB0VuZ2xhbmQxDzAN
+BgNVBAcMBkxvbmRvbjEVMBMGA1UEAwwMdGVzdC5waHAubmV0MR8wHQYJKoZIhvcN
+AQkBFhB0ZXN0LnBocEBwaHAubmV0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB
+CgKCAQEArbUmVW1Y+rJzZRC3DYB0kdIgvk7MAday78ybGPPDhVlbAb4CjWbaPs4n
+yUCTEt9KVG0H7pXHxDbWSsC2974zdvqlP0L2op1/M2SteTcGCBOdwGH2jORVAZL8
+/WbTOf9IpKAM77oN14scsyOlQBJqhh+xrLg8ksB2dOos54yDqo0Tq7R5tldV+alK
+ZXWlJnqRCfFuxvqtfWI5nGTAedVZhvjQfLQQgujfXHoFWoGbXn2buzfwKGJEeqWP
+bQOZF/FeOJPlgOBhhDb3BAFNVCtM3k71Rblj54pNd3yvq152xsgFd0o3s15fuSwZ
+gerUjeEuw/wTK9k7vyp+MrIQHQmPdQIDAQABoIHCMAkGA1UECTECDAAwCQYDVQQX
+MQIMADAMBgNVBBExBQwDTjExMBIGA1UEFDELDAkwMTIzNDU2NzgwFQYDVQQpMQ4M
+DE9yZ2FuaXNhdGlvbjAZBgsrBgEEAdgOiH4CATEKDAgxMTExMjIyMjAZBgsrBgEE
+AdgOiH4CAjEKDAgxMjM0NTY3ODAaBgkqhkiG9w0BCQ4xDTALMAkGA1UdEwQCMAAw
+HwYJKoZIhvcNAQkBMRIWEGluZm9AZXhhbXBsZS5jb20wDQYJKoZIhvcNAQELBQAD
+ggEBAAoPI/sWY0QKPMEBuRp6MHcvWgSExwkkQfRJQZlYdepu6Tw0iZwYRTOR4sEn
+Vz95qsrWqHp6QkXxdFG9FPHi4N66OX2Xb5TtHgDGMxrJTwbH+7VdsJiXLkWbeLuo
+zKv8BsrhLRYiZkl+VWIrNyOcK7ao2sD+D3YkCBA4JK4OFhfhxY43D2sme7aEQVjr
+S+UvEjuIALN0AP6gO2AMiUODPBrjsPI3NpN40VUvVU+Hsp1Tlqvth/AYASuGT2yt
+M5YdcSm7JwaGAwIgOv8XPUQGem52yMEvzySRC4ZyTddfiZAkeTLmbh+SMVbHXXOk
+UeEz+fvmQ4L+sc3RE8u+M8g31LM=
+-----END CERTIFICATE REQUEST-----
+"
+
+Warning: openssl_csr_new(): attributes: wrong is not a recognized attribute name in %s on line %d
+object(OpenSSLCertificateSigningRequest)#%d (0) {
+}