Add X509 purpose constant

[vjardin]

X509_PURPOSE_OCSP_HELPER, X509_PURPOSE_TIMESTAMP_SIGN are available from
OpenSSL for many years:

• X509_PURPOSE_OCSP_HELPER, since 2001
• X509_PURPOSE_TIMESTAMP_SIGN, since 2006

[nikic]

Looks like a reasonable addition. Any concerns from @bukka?

[vjardin]

What's about a backport for previous PHP Releases: 7.4, 8.0 ? If you want, I can build the patch, it'll be an easy one.

[bukka]

Just a minor comment but seems reasonable. It's quite straight forward and self contained so I would be fine with targeting 7.4 if there are no objections.

@vjardin you don't need to provide extra patch. You can just change the base branch which should do.

[vjardin]

@bukka Thanks regarding 7.4 I am not used with PHP maintenance process: should I wait for these 2 patches to be applied to openssl/master first before getting it in 7.4 ?

Regarding the change of the base branch, how does it work with github ? Should I provide it thru a specific pull request ? I'll maybe wrong, but it seem easier for a maintainer to apply it, doesn't it ?

I am ready to help...

[Girgias]

@bukka Thanks regarding 7.4 I am not used with PHP maintenance process: should I wait for these 2 patches to be applied to openssl/master first before getting it in 7.4 ?

Regarding the change of the base branch, how does it work with github ? Should I provide it thru a specific pull request ? I'll maybe wrong, but it seem easier for a maintainer to apply it, doesn't it ?

I am ready to help...

Change the base branch of this PR in the GitHub interface. However GitHub doesn't seem to be smart enough to do a rebase. After having done it next to the title of the PR there is an edit button which allows you to change the base branch.

We need to merge up anyway so it makes no sense to add this first on master (which is now PHP 8.1) when the whole point is to backport it. The person merging will take care of merging it into all the maintained version branches.

[nikic]

I've applied this patch to PHP-7.4 and upwards.

[nikic]

I had to revert this change, because it fails on Travis on the PHP-7.4 branch:

========DIFF========
046+ bool(true)
053- bool(false)
069+ bool(true)
069- bool(false)
========DONE========
FAIL int openssl_x509_checkpurpose ( mixed $x509cert , int $purpose [, array $cainfo = array() [, string $untrustedfile ]] ) function [ext/openssl/tests/openssl_x509_checkpurpose_basic.phpt]

It does work locally and on Azure though. Possibly there is an OpenSSL version dependence in the test?

[vjardin]

Any openssl since 2006 should be good enough. By the way, from the Travis logs I can see, it is OK. Please, do you have a URL with some traces I can analyse ?

[nikic]

@vjardin The failing build was https://travis-ci.org/github/php/php-src/builds/734957779.

[vjardin]

Thanks,:the latest one https://travis-ci.org/github/php/php-src/builds/734972098 has been OK. How can I run my own Travis environment, close to the Travis one in order to analyse the gap with the failing one https://travis-ci.org/github/php/php-src/builds/734957779 ?

[nikic]

@vjardin The relevant difference (I assume) is that this PR against master uses bionic, while PHP-7.4 uses xenial.

[vjardin]

Right, it fails on Xenial (16.04) while it is OK on 18.04 on 20.04. It seems that OpenSSL 1.1+ is required which is strange since these features are defined for years within OpenSSL. I could not find the proper bug fix into OpenSSL's git yet.

• 16.04/openssl: OpenSSL 1.0.2g 1 Mar 2016

• 18.04/openssl: OpenSSL 1.1.1 11 Sep 2018

• 20.04/openssl: OpenSSL 1.1.1f 31 Mar 2020

Should I "ifdef" it so it compiles only when OpenSSL 1.1 is available ?

[bukka]

The constants are present as it wouldn't compile otherwise. I think this will be more about the checking logic that might have changed in OpenSSL 1.1.0.

It would be good if you can compile it against 1.0.2 and check which constant is causing that and what was changed in OpenSSL to cause that. Then you can modify the test accordingly if it's expected.

[cmb69]

I wanted to suggest to just merge that in a branch which requires OpenSSL ≥ 1.1, but even master still supports OpenSSL ≥ 1.0.2. Does that make sense?

[bukka]

I wanted to suggest to just merge that in a branch which requires OpenSSL ≥ 1.1, but even master still supports OpenSSL ≥ 1.0.2. Does that make sense?

@cmb69 yeah we have discussed it and think Remi was against dropping the support as 1.0.2 is still supported by RHEL 7 IIRC.

[cmb69]

@cmb69 yeah we have discussed it and think Remi was against dropping the support as 1.0.2 is still supported by RHEL 7 IIRC.

@remicollet, can you confirm? Is this still relevant for PHP 8.2?

[cmb69]

Oh, indeed, CentOS 7 defaults to OpenSSL 1.0.2, and receives maintainance updates till 2024-06-30. :(

Anyhow, @vjardin, are you planning to work on this PR?

[vjardin]

Oh, indeed, CentOS 7 defaults to OpenSSL 1.0.2, and receives maintainance updates till 2024-06-30. :(

Anyhow, @vjardin, are you planning to work on this PR?

No, I do not plan. It has been lasting for too long. You are welcomed to take over this topic or to close it.

[cmb69]

Yeah, sorry, we're bad at addressing PRs.

Anyhow, @bukka, what do you think about this? Is this worth pursuing?

[bukka]

Yeah I think it's useful. It's fine to leave it open. I have got it on my list but there are lots of other higher priority things before that so it might take some time.

[vjardin]

let's close without merging this pull request, more to come with #13149