Bug #71929 [Csd]: Certification information (CERTINFO) data parsing error

From: Date: Thu, 28 Jul 2016 03:46:35 +0000
Subject: Bug #71929 [Csd]: Certification information (CERTINFO) data parsing error
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-202650@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=71929&edit=1

 ID:                 71929
 Updated by:         pierrick@php.net
 Reported by:        asmqb7 at gmail dot com
 Summary:            Certification information (CERTINFO) data parsing
                     error
 Status:             Closed
 Type:               Bug
 Package:            cURL related
 Operating System:   Linux (Arch Linux)
 PHP Version:        7.0.4
 Assigned To:        pierrick
 Block user comment: N
 Private report:     N

 New Comment:

CURLINFO_CERTINFO now returns Subject and Issuer as string since this is how libcurl returns this
information. There is no clear définition on how the certificate issuer and subjects are
formatting so we should not try to parse this.


Previous Comments:
------------------------------------------------------------------------
[2016-07-28 03:44:13] pierrick@php.net

Automatic comment on behalf of pierrick
Revision: http://git.php.net/?p=php-src.git;a=commit;h=30a5ed3a7979f1b865f6633cb16b5f3e78371df1
Log: Fixed bug #71929 (CURLINFO_CERTINFO data parsing error).

------------------------------------------------------------------------
[2016-04-25 01:34:14] asmqb7 at gmail dot com

In a side project I'm currently working on I'm using...

  $m = preg_match('/O = (.+?(?=, [A-Z]+ = ))/',
    $info['certinfo'][0]['Subject'], $tls_org);

...a mildly involved regular expression to (hopefully) extract useful info from the Organization
field, which I thought I'd include for fun in my logging output. That said, my cURL code is
only being used for one site right now, so I'm just messing around and having fun testing
functionality - I have VERIFYPEER=2 and VERIFYHOST=1 doing the heavy lifting.

But I think the fact that I'm reluctant to use this for production functionality says
something. I think there are two main aspects to this bug.

1. First of all, the practical aspect.

It's unfortunate GitHub and Searchcode strips quotes from searches; if only we could search for
"curlinfo" (including quotes), we'd find all the array index references :P. I poked
around for a bit but didn't find much.

I think that it's reasonable to assume that these fields have never been used for anything
noteworthy because a bug report like this has never been raised before.

With this in mind, fixing thing and putting a "there was a bug" in the changelog for
curl_getinfo seems like a good way to notify anybody using this that the function works properly
now.

For my own part, I'm fixing the bug this way:

foreach ($curlinfo['certinfo'] as &$certinfo) {
    foreach (['Subject', 'Issuer'] as $n) {
        if (is_array($certinfo[$n])) {
            $certinfo[$n] = array_keys($certinfo[$n])[0].
                '='.array_values($certinfo[$n])[0];
        }
    }
}

This does use a couple moderately magic/new PHP features such as foreach pass-by-reference and
function indices (if I've used the right feature names) to keep the code size down and this
snippet may need to be further simplified for extra backward compatibility (I'm not sure if the
features I mentioned work in PHP 5.3 - I think they do?).

FWIW, this looks solid enough to me to be able to confidently hand it off to anybody and say
"don't worry about how this works, just run it immediately after you run curl_getinfo()
when you want the Subject and Issuer."

The noteworthy part is the is_array(), so the code will be a no-op when this is fixed. The loops
will always run and this is noted, but I doubt curl_getinfo() is unlikely to be used in
time-critical sections of PHP so that should be fine.

For applications with logging capabilities, this variant might be interesting:

foreach ($curlinfo['certinfo'] as &$certinfo) {
    foreach (['Subject', 'Issuer'] as $n) {
        if (is_array($certinfo[$n])) {
            $certinfo[$n] = array_keys($certinfo[$n])[0].
                '='.array_values($certinfo[$n])[0];
        } else {
            print "PHP is fixed!\n";
            break;
        }
    }
}

So that's the practical aspect.

2. Now for the fundamental (or "philosophical," for want of a better word) aspect of this
bug.

I think that nobody is using this information because it's too arbitrarily formatted. I would
not be surprised if nobody uses cURL's own info data structures for anything much beyond
verifying the key or hash or whatnot, because there's too much room for error in a string like:

  Subject:C = US, ST = California, L = Los Angeles, O = Internet Corporation for Assigned Names and
Numbers, OU = Technology, CN = www.example.org

I don't consider the above unambiguously parseable.

One possibility might be the ability to retrieve a binary info structure from cURL that could be
passed to openssl_x509_parse() would, in my opinion, be the real solution to this bug. I mean,
it's amazing - that function gives back an array with the Subject and Issuer fields both broken
down into array key/value pairs! (:O)

Alternatively, cURL could be altered to return this structure in such a way that it's
unambiguous, and then represented in PHP as such as well.

Comments/feedback/critique/insight welcome.

------------------------------------------------------------------------
[2016-04-24 16:24:50] pajoye@php.net

And the cert info is used quite a lot, not sure if they actually use this part of the info (less
than pkey or signature but still):

https://github.com/search?l=php&q=CURLOPT_CERTINFO+&type=Code&utf8=%E2%9C%93

------------------------------------------------------------------------
[2016-04-24 16:22:48] pajoye@php.net

Alternatively explode each part of the subject and issuer into an associative array, but that could
break more code out there.

Comments&suggestions welcome :)

------------------------------------------------------------------------
[2016-04-24 04:18:14] pajoye@php.net

Do you refer to the
  [C ] =>  US, 

Part? C= should be kept as part of the content and not use as index.

I cannot remember having written the cert info split code but the slit cert function must be the
place to look at if we are willing to fix that.

I think it should ve fixed. And it should also match the openssl output

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=71929


--
Edit this bug report at https://bugs.php.net/bug.php?id=71929&edit=1


Thread (8 messages)

« previous php.bugs (#202650) next »