[RFC] Add openssl_pkcs7_read and extend openssl_pkcs7_verify

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!

No problem, we've so far build our application with S/MIME support on
PHP and it worked so far :-)

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.

Ok, I'm open to feedback! I've opened the PR https://github.com/php/php-src/pull/2563

Another thing I would want to work on is making the openssl_pkcs7_*
functions in memory instead of file based. This would increase the
security concerns since that means it won't save plaintext temporarily to
file.

An attempt for that has been made in a PR, but was closed. Is the PR on
the right 'track' or can the current functions change to accept either a
string or a filename? (I'm not 100% if PHP internals allow that) or if
it's really wanted. [1]

[1] https://github.com/php/php-src/pull/560

--
Jelle van der Waa