Skip to content

Fix for php bug #64802 #410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Fix for php bug #64802 #410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b54d9b4
commit for php bug 61421
Sep 14, 2012
76488b7
Merge remote-tracking branch 'upstream/master'
mk-j Aug 13, 2013
3e7eba1
Fix for php bug #64802 includes test case
mk-j Aug 14, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 20 additions & 31 deletions ext/openssl/openssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ static EVP_PKEY * php_openssl_generate_private_key(struct php_x509_request * req

static void add_assoc_name_entry(zval * val, char * key, X509_NAME * name, int shortname TSRMLS_DC) /* {{{ */
{
zval **data;
zval *subitem, *subentries;
int i, j = -1, last = -1, obj_cnt = 0;
char *sname;
Expand Down Expand Up @@ -629,39 +630,27 @@ static void add_assoc_name_entry(zval * val, char * key, X509_NAME * name, int s
sname = (char *) OBJ_nid2ln(nid);
}

MAKE_STD_ZVAL(subentries);
array_init(subentries);

last = -1;
for (;;) {
j = X509_NAME_get_index_by_OBJ(name, obj, last);
if (j < 0) {
if (last != -1) break;
} else {
obj_cnt++;
ne = X509_NAME_get_entry(name, j);
str = X509_NAME_ENTRY_get_data(ne);
if (ASN1_STRING_type(str) != V_ASN1_UTF8STRING) {
to_add_len = ASN1_STRING_to_UTF8(&to_add, str);
if (to_add_len != -1) {
add_next_index_stringl(subentries, (char *)to_add, to_add_len, 1);
}
} else {
to_add = ASN1_STRING_data(str);
to_add_len = ASN1_STRING_length(str);
add_next_index_stringl(subentries, (char *)to_add, to_add_len, 1);
}
}
last = j;
str = X509_NAME_ENTRY_get_data(ne);
if (ASN1_STRING_type(str) != V_ASN1_UTF8STRING) {
to_add_len = ASN1_STRING_to_UTF8(&to_add, str);
} else {
to_add = ASN1_STRING_data(str);
to_add_len = ASN1_STRING_length(str);
}
i = last;

if (obj_cnt > 1) {
add_assoc_zval_ex(subitem, sname, strlen(sname) + 1, subentries);
} else {
zval_dtor(subentries);
FREE_ZVAL(subentries);
if (obj_cnt && str && to_add_len > -1) {
if (to_add_len != -1) {
if (zend_hash_find(Z_ARRVAL_P(subitem), sname, strlen(sname)+1, (void**)&data) == SUCCESS) {
if (Z_TYPE_PP(data) == IS_ARRAY) {
subentries = *data;
add_next_index_stringl(subentries, (char *)to_add, to_add_len, 1);
} else if (Z_TYPE_PP(data) == IS_STRING) {
MAKE_STD_ZVAL(subentries);
array_init(subentries);
add_next_index_stringl(subentries, Z_STRVAL_PP(data), Z_STRLEN_PP(data), 1);
add_next_index_stringl(subentries, (char *)to_add, to_add_len, 1);
zend_hash_update(Z_ARRVAL_P(subitem), sname, strlen(sname)+1, &subentries, sizeof(zval*), NULL);
}
} else {
add_assoc_stringl(subitem, sname, (char *)to_add, to_add_len, 1);
}
}
Expand Down
37 changes: 37 additions & 0 deletions ext/openssl/tests/bug64802.pem
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
-----BEGIN CERTIFICATE-----
MIIGfzCCBWegAwIBAgIQSVCinGH6MkvjJZjRyjK9nTANBgkqhkiG9w0BAQUFADCB
jjELMAkGA1UEBhMCR0IxGzAZBgNVBAgTEkdyZWF0ZXIgTWFuY2hlc3RlcjEQMA4G
A1UEBxMHU2FsZm9yZDEaMBgGA1UEChMRQ09NT0RPIENBIExpbWl0ZWQxNDAyBgNV
BAMTK0NPTU9ETyBFeHRlbmRlZCBWYWxpZGF0aW9uIFNlY3VyZSBTZXJ2ZXIgQ0Ew
HhcNMTIwMjI5MDAwMDAwWhcNMTQwMjI4MjM1OTU5WjCCAW8xEjAQBgNVBAMTCXd3
dy5yZC5pbzERMA8GA1UEAxMIcmRpby5jb20xDjAMBgNVBAMTBXJkLmlvMRUwEwYD
VQQDEwxhcGkucmRpby5jb20xEjAQBgNVBAMTCWFwaS5yZC5pbzEQMA4GA1UEBRMH
NDU4NjAwNzETMBEGCysGAQQBgjc8AgEDEwJVUzEZMBcGCysGAQQBgjc8AgECEwhE
ZWxhd2FyZTEdMBsGA1UEDxMUUHJpdmF0ZSBPcmdhbml6YXRpb24xCzAJBgNVBAYT
AlVTMQ4wDAYDVQQREwU5NDEwMzELMAkGA1UECBMCQ0ExFjAUBgNVBAcTDVNhbiBG
cmFuY2lzY28xFzAVBgNVBAkTDjE1NTAgQnJ5YW50IHN0MRMwEQYDVQQKEwpSZGlv
LCBJbmMuMSMwIQYDVQQLExpDT01PRE8gRVYgTXVsdGktRG9tYWluIFNTTDEVMBMG
A1UEAxMMd3d3LnJkaW8uY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC
AQEAt0AgYOe8EBJNVBAuSJFLKHRKZn0/ObCLBFG4xVH/5fb1rfYHBT1XSjjOqR3t
iGC/A3esF8YC7TuHQcTLVephx0DtJv1ASxRg3zPM8ebBRsuul18N0W+sY1aNXpkd
36quxvjg5UdBrAweuekJ7OTSZcCe2Ry/SKBeZSWWtkWsI4krCLv7JaKUwxw2h+Hn
TAZSBLVxz/mixF0WYdepYwnq2Hm7XvvVEIQ7wxOQ9bA7iCevLojZOnb39BT2QII7
cy8AB47RZdfYg7UwaO3bST2rauA4MKar7/Ozqc0aemNFpLatJfgv07cydiuj9fsd
5aE/c8is8C9M9+7MmSMkcNEgGwIDAQABo4IB8zCCAe8wHwYDVR0jBBgwFoAUiERR
/1AqaV4tiPQhutkM8s7L6nwwHQYDVR0OBBYEFCrYw8bfrYJ61NS2yYx6/CnhjzT4
MA4GA1UdDwEB/wQEAwIFoDAMBgNVHRMBAf8EAjAAMB0GA1UdJQQWMBQGCCsGAQUF
BwMBBggrBgEFBQcDAjBGBgNVHSAEPzA9MDsGDCsGAQQBsjEBAgEFATArMCkGCCsG
AQUFBwIBFh1odHRwczovL3NlY3VyZS5jb21vZG8uY29tL0NQUzBTBgNVHR8ETDBK
MEigRqBEhkJodHRwOi8vY3JsLmNvbW9kb2NhLmNvbS9DT01PRE9FeHRlbmRlZFZh
bGlkYXRpb25TZWN1cmVTZXJ2ZXJDQS5jcmwwgYQGCCsGAQUFBwEBBHgwdjBOBggr
BgEFBQcwAoZCaHR0cDovL2NydC5jb21vZG9jYS5jb20vQ09NT0RPRXh0ZW5kZWRW
YWxpZGF0aW9uU2VjdXJlU2VydmVyQ0EuY3J0MCQGCCsGAQUFBzABhhhodHRwOi8v
b2NzcC5jb21vZG9jYS5jb20wTAYDVR0RBEUwQ4IMd3d3LnJkaW8uY29tgglhcGku
cmQuaW+CDGFwaS5yZGlvLmNvbYIFcmQuaW+CCHJkaW8uY29tggl3d3cucmQuaW8w
DQYJKoZIhvcNAQEFBQADggEBAKFd4bPVFRyrlqIKPtrtMuqGqid6685ohxf0cv52
sjdRYwLVTjnZOrmkDdNaF3R2A1ZlVMRN+67rK+qfY5sTeijFcudV3/i0PDtOFRwP
6yYVD2uZmYkxfPiW309HPmDF+EzhxpVjWlTQEOwkfFLTmJmwl3Qu2Kffp8F1ENXW
OTVNvj5VtMghvzu68PpzKl1VjlOR4Ej9NCwh1dUjNKEoTPzvpehXsIZ7jHSpX/T1
wSSt9ckiechDdpgZXTzHgbxHNibK0Uhh+QhkBgYMj5F8qj5BlBhWAWqQa/VnEdmr
Pfo7U+QmadoqQd7qt06hE2hG1nfZ0vPJDbWV3oVSwG2Yt7I=
-----END CERTIFICATE-----
14 changes: 14 additions & 0 deletions ext/openssl/tests/bug64802.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
--TEST--
Bug #64802: openssl_x509_parse fails to parse subject properly in some cases
--SKIPIF--
<?php
if (!extension_loaded("openssl")) die("skip");
?>
--FILE--
<?php
$cert = file_get_contents(__DIR__.'/bug64802.pem');
$r = openssl_x509_parse($cert,$use_short_names=true);
var_dump( count($r['subject'])>1 );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to have actual list or count to be output. True/false tests are very hard to debug - if you get false, you have no idea what the actual count was. That's why we compare the output instead of making true/false tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had another version of the test that tested for actual keys in the response, like 'CN', 'O', 'L', 'ST' etc, but the return value of openssl_x509_parse states: "The structure of the returned data is (deliberately) not yet documented, as it is still subject to change.", so I submitted it as you saw it above.

But if you think its better, I can test previous for actual keys 'CN', 'O', etc, because the "subject to change" clause, probably has more to do with the X509v3 extensions, than the x509 subject.

?>
--EXPECTF--
bool(true)