Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:99353
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.213.178 as permitted sender)
MIME-Version: 1.0
Sender: jakub.php@gmail.com
In-Reply-To: <20170604182219.GA6723@gmail.com>
References: <20170531101952.GB26690@gmail.com> <CAEKnhAF-NnxvwmgVzZuKitKZJu1cUTK+9dKzuido22s373m73w@mail.gmail.com>
 <20170604182219.GA6723@gmail.com>
Date: Sun, 4 Jun 2017 20:02:15 +0100
Message-ID: <CAEKnhAG=i0DKfa8E9cfFqE2DoO8p08WcYkm00+Z=ib5fEfGsig@mail.gmail.com>
To: Jelle van der Waa <jelle@vdwaa.nl>
Cc: php-internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="94eb2c1358f45c81970551270639"
Subject: Re: [PHP-DEV] [RFC] Add openssl_pkcs7_read and extend openssl_pkcs7_verify
From: bukka@php.net (Jakub Zelenka)

--94eb2c1358f45c81970551270639
Content-Type: text/plain; charset="UTF-8"

On Sun, Jun 4, 2017 at 7:22 PM, Jelle van der Waa <jelle@vdwaa.nl> wrote:

> On 06/04/17 at 07:00pm, Jakub Zelenka wrote:
> > On Wed, May 31, 2017 at 11:19 AM, Jelle van der Waa <jelle@vdwaa.nl>
> wrote:
> >
> > > I would like to propose the addition of openssl_pkcs7_read and
> extending
> > > openssl_pkcs7_verify to also return a PKCS7 structure. The reasoning
> for
> > > the addition of these functions is the requirement at work to obtain
> the
> > > CA certificates usually send along with a signed email. The CA
> > > certificates are required for OCSP verification (which is currently
> done
> > > in pure PHP, I also would like to see this added in PHP in the future).
> > >
> > > It is currently impossible to acquire the CA certificates with the
> > > openssl functions which PHP provides, I've also found a bug report
> > > requesting the ability to read a PKCS7 blob. [1]
> > >
> > > To summarize, I would propose to add an optional parameter to
> > > openssl_pkcs7_verify which takes a string that defines the location
> > > where the PKCS7 blob should be stored.
> > >
> > > $pkcs7 = "chain.pk7";
> > > openssl_pkcs7_verify($file, PKCS7_NOVERIFY, $outfile, [], $outfile,
> > > $content, $pkcs7);
> > >
> > > To be able to read the blob, I would propose a new function
> > > openssl_pkcs7_read which returns an array of strings containing the PEM
> > > certificates in the PKCS7 blob. I've based the naming and behaviour on
> > > openssl_pkcs12_read.
> > >
> > > openssl_pkcs7_read($pkcs7, $data);
> > > var_dump($data);
> > >
> > > I've implemented the above mentioned changes in my fork of PHP, mind
> > > that the code isn't ready for a PR yet since there are some styling
> > > issues, possible memory leaks and of course missing tests. The code
> > > however works as a proof of concept. [2]
> > >
> > > For further background information, obtaining the pk7 output can be
> done
> > > with the 'openssl' tool:
> > >
> > > openssl smime -verify -pk7out  -in signed_email.eml  > foo.pkcs7
> > > openssl pkcs7 -print_certs -in foo.pkcs7
> > >
> > >
> > It seems reasonable from the quick look.
> >
> > I don't think we need RFC unless there are some objections. Once it's
> > ready, PR should be enough IMHO.
>
> Thanks, this is my first contributing a feature to PHP and from reading
> the wiki an RFC was the method to introduce new features.
>
> Offtopic, I also want to later add string based instead of file based
> openssl_pkcs7_* functions. Do I need to create an RFC for it?
>
>
First of all, it's really great that you contribute to this so thanks!

I think that for changes like this, it's best to first open PR. When the PR
is ready and all issues resolved, then it should be announced on this
mailing list and then see if there are no objections. If there are not, it
should be fine to merge it IMHO. Otherwise if there is no agreement, it
probably needs RFC. :) I think that RFC makes sense either when the feature
is complex or when people are not in agreement about it.

This is of course just my view and others might disagree... :)

Cheers

Jakub

--94eb2c1358f45c81970551270639--