Hi, On Wed, 18 Jul 2018, 00:15 Dominic Luechinger, <dol+...@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 >
