New load_csr() in apps.c simplifiying req, x509, and ca apps and allowing DER input to ca#4940
New load_csr() in apps.c simplifiying req, x509, and ca apps and allowing DER input to ca#4940DDvO wants to merge 4 commits intoopenssl:masterfrom
Conversation
|
@bernd-edlinger, thanks for your review and comments. Indeed the output in case of unsupported format could be more helpful. I've now done this not only for Doing so, I found that the use of the |
|
Nice work, @DDvO |
|
Hmm, it looks like the Travis CI build https://travis-ci.org/openssl/openssl/jobs/321919199 failed due to some Debian package (update) issue? |
|
At this point, I think we need to post-pone this PR to be post-1.1.1, as it's very much an added features thing. Also, I'm thinking that this kind of change should really make use of the |
|
I've meanwhile rebased also this PR such that we can hopefully integrate it with #11470 soon. |
|
Hopefully nobody depends on exact wording (and case) for the error messages in scripts. Could you please amend the commit message of the last commit so the subject is shorter and you add description of the changes in message body. |
It would be pretty unwise to do so.
Sure - done. |
|
I think if CI passes this could be approved unless you plan further changes. |
|
Pleased to hear. I don't plan to do further changes unless request by a review. |
Make use of new load_csr() in 'ca', 'req', and 'x509' app Add '-inform' and '-certform' option to 'ca' app Add 'desc' parameter to load_crl() function defined in apps/lib/apps.c Allow 'desc' parameter to be NULL (gives option to suppress error output)
Also make sure that all error messages in apps.c consistently begin upper-case.
Changed files: apps/lib/apps.c and apps/{req.c,s_client.c,s_server.c,x509.c}
|
Since Travis CI went fine I've already updated the PR to the final commit structure I propose. |
t8m
added
approval: done
|
24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually. |
DDvO
added
the
approval: ready to merge
|
For some reason AppVeyor is still queued on this PR (and others as well) since about two days, |
Make use of new load_csr() in 'ca', 'req', and 'x509' app Add '-inform' and '-certform' option to 'ca' app Add 'desc' parameter to load_crl() function defined in apps/lib/apps.c Allow 'desc' parameter to be NULL (gives option to suppress error output) Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #4940)
…s/lib/opt.c Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #4940)
Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #4940)
Also make sure that all error messages in apps.c consistently begin upper-case.
Changed files: apps/lib/apps.c and apps/{req.c,s_client.c,s_server.c,x509.c}
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from #4940)
|
Merged - thanks @t8m for your swift review! |
9 participants
New helper function
X509_REQ *load_csr(const char *file, int format, const char *desc)in
apps/apps.csimplifying and generalizing CSR loading inreq.c,x509.c, andca.c.Moreover, new
-informand-certformCLI options to ca app now allowing DER input.Tested with DER (rather than PEM) CSR input format in
80-test_ca.t.Checklist