Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:102910
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.218.54 as permitted sender)
MIME-Version: 1.0
References: <8bb21db9-1f64-6520-68af-23840bba77ea@snowgarden.ch>
In-Reply-To: <8bb21db9-1f64-6520-68af-23840bba77ea@snowgarden.ch>
Date: Thu, 19 Jul 2018 00:05:24 +0100
Message-ID: <CAEKnhAHWhZ+EghPv5BGhSLpk5FDme3aHB9-w5=AO+bY1WUJhtg@mail.gmail.com>
To: Dominic Luechinger <dol+php@snowgarden.ch>
Cc: PHP internals list <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000053660e05714e198e"
Subject: Re: [PHP-DEV] Improvements to openssl_crs_new. Need advice
From: bukka@php.net (Jakub Zelenka)

--00000000000053660e05714e198e
Content-Type: text/plain; charset="UTF-8"

Hi,

On Wed, 18 Jul 2018, 00:15 Dominic Luechinger, <dol+php@snowgarden.ch>
wrote:

> I'd like to improve the openssl_csr_new function to add any X509
> "Requested Extensions" [1] to a CSR.
> My motivation to improve this functionality is to avoid workarounds like
> altering a openssl.cnf file and pass some ENV variable to it [2].
>
> I already implemented the following new functionality:
>
> Old:
> mixed openssl_csr_new ( array $dn , resource &$privkey [, array
> $configargs [, array $extraattribs ]] )
>
> New (I can provide a patch, needs cleanup and testing):
> mixed openssl_csr_new ( array $dn , resource &$privkey [, array
> $configargs [, array $extraattribs[, array $extraexts ]]] )
>
> E.g:
> ```
> $privkey = openssl_pkey_new();
> $csr = openssl_csr_new([], $privkey, null, [], [
>     'subjectAltName' => 'DNS:example.tld',
> ]);
>
> ```
>
> While implementing the new functionality I realized that the 'Requested
> Extensions' are represented as a CSR attribute and it contains the ASN1
> structure of multiple extensions and their definitions. With the
> following example the declaration of the extension should be possible
> without the new argument $extraexts in openssl_csr_new.
>
> ```
> $privkey = openssl_pkey_new();
> // Use OID of ExtensionRequest
> $csr = openssl_csr_new([], $privkey, null, ['1.2.840.113549.1.9.14' =>
> 0xDEADBEEF]);
> ```
>
> This won't work because the argument $extraattribs only applies to
> subject attributes. The argument name is kind of misleading. See the
> following bug report [3] from 2008 that describes the issue in a good
> manor. IMHO this bug report is valid and the bug should be fixed in a
> way that the attributes are added to the certificationRequestInfo [4]
> instead being merged into the subject. This might break some existing
> usage of this argument. With this bug fixed 'Requested Extensions' can
> be added in a programmatic way. To generate the DER encoded content of
> 'Requested Extensions' a ASN1 library should be used.
>
>
> Now comes to tricky part about supporting my initial goal to add
> additional'Requested Extensions' to an CSR.
>
> Should I summit my patch with the extra argument as a PR or should I fix
> the bug 45076 or should I do both?
>
> extraexts VS bug fix:
> - No BC break VS BC break
> - No need for a ASN1 library VS working with ASN1 DER encoded data
> - Default extensions from openssl.cnf are preserved and can be
> overwritten VS definition of 'Requested Extensions' in DER overwrites
> default extensions from openssl.cnf
>
> Looking at the pros and cons my guts tells my to do both. Patch and bug
> fix.
> Any other suggestions/thoughts?
>

I haven't had time to properly look into it but from the quick read, I
would prefer not to break BC even though the behaviour seems incorrect. I
think that changing that can cause some issues potentially. But I might
need more time to think about. Anyway the proposed patch seems reasonable
so it would be great if you can create a PR that.

Cheers

Jakub

>

--00000000000053660e05714e198e--