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
