Add purpose codesign#18567
Add purpose codesign#18567ljaenicke wants to merge 6 commits intoopenssl:masterfrom PHOENIXCONTACT:purpose_codesign
Conversation
|
This pull request is part of issue #18491 |
FdaSilvaYY
left a comment
There was a problem hiding this comment.
A few typos on 'signing' word.
Good work over all
|
We will need tests to be able to merge this according to our policy. |
|
I spent some time trying to reverse engineer the content of test/certs and the other subcomponents of the test/ directory. Is there any README I have missed? |
I suppose you did not miss anything. There is not much documentation, unfortunately. I'd put some new test certs to the |
|
Updated pull request to resolve merge conflicts |
|
CMS is something of a general-purpose content format these days, getting used not just for S/MIME email protection and code signing, but also used as parts of bootstrapping autonomic network equipment (RFC 8995), Kerberos public-key initial authentication (RFC 4556), RPKI (many RFCs), etc. It seems like we're setting ourselves up for a maintenance headache if we add individual flags for "use this purpose value", "use that purpose value", etc. I think it would be better to just have a |
|
Note: adding this to the flags looked like the least intrusive change. From my perspective your proposal could be achieved in two ways. The simple way would be to extend CMS_verify() with an additional argument providing the purpose as a string, just as I did with the internal function cms_signerinfo_verify_cert(), passing the purpose string down from the command line argument. |
Put "}," on separate lines as suggested in PR #18567
|
I have reworked the pull request after digging deeper. It is not necessary to set the purpose within the CMS_verify() call tree affecting the X509_STORE_CTX. It is sufficient to set the purpose when setting up the X509_STORE which can be done with the command line application(s) -purpose option. |
|
Fix typo spottet by @t-j-h |
|
This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago |
|
This pull request is ready to merge |
DDvO
left a comment
There was a problem hiding this comment.
Thanks for very swiftly handling my first comment.
Meanwhile I've done a full review - please see the individual new comments.
Your test cases are commendable.
| if ((x->ex_xkusage & (XKU_ANYEKU | XKU_SSL_SERVER)) != 0) | ||
| return 0; |
There was a problem hiding this comment.
And how about the requirements demanded by CA/BF on CA certs,
and the further ones on EE certs regarding policies, CDPs, and AIA?
|
BTW, when making further adaptations to already (at least partly) reviewed PRs, |
|
Agreed. There was no "--fixup" that many years ago : -). |
|
This pull request is ready to merge |
|
Merged to master branch. Thank you for your contribution! |
Put "}," on separate lines as suggested in PR #18567 Reviewed-by: Paul Dale <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #18567)
Code signing certificates have other properties as for example described in CA Browser Forum documents. This leads to "unsupported certificate purpose" errors when verifying signed objects. This patch adds the purpose "codesign" to the table in X.509 certificate verification and the verification parameter "code_sign" to X509_VERIFY_PARAM. Reviewed-by: Paul Dale <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #18567)
Correct configuration according to CA Browser forum: KU: critical,digitalSignature XKU: codeSiging Note: I did not find any other document formally defining the requirements for code signing certificates. Some combinations are explicitly forbidden, some flags can be ignored Reviewed-by: Paul Dale <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #18567)
The tests only cover the correct handling of the codesigning purpose in the certificates in the context of the cms command line tool. The interpretation of the certificate purpose is tested in the context of the "verify" app. The correct handling of the cms objects is tested by other tests in 80-test_cms.t. Reviewed-by: Paul Dale <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from #18567)
Put "}," on separate lines as suggested in PR openssl#18567 Reviewed-by: Paul Dale <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from openssl#18567)
Code signing certificates have other properties as for example described in CA Browser Forum documents. This leads to "unsupported certificate purpose" errors when verifying signed objects. This patch adds the purpose "codesign" to the table in X.509 certificate verification and the verification parameter "code_sign" to X509_VERIFY_PARAM. Reviewed-by: Paul Dale <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from openssl#18567)
Correct configuration according to CA Browser forum: KU: critical,digitalSignature XKU: codeSiging Note: I did not find any other document formally defining the requirements for code signing certificates. Some combinations are explicitly forbidden, some flags can be ignored Reviewed-by: Paul Dale <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from openssl#18567)
The tests only cover the correct handling of the codesigning purpose in the certificates in the context of the cms command line tool. The interpretation of the certificate purpose is tested in the context of the "verify" app. The correct handling of the cms objects is tested by other tests in 80-test_cms.t. Reviewed-by: Paul Dale <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> (Merged from openssl#18567)
…RAM default_table From OpenSSL commit 1a68a3e42142a2c188f4b69c7337438c89502143 openssl/openssl#18567 Put "}," on separate lines as suggested in PR #18567
…rtificates From OpenSSL commit 178696d6020878361a088086243d56203e0beaa9 openssl/openssl#18567 Code signing certificates have other properties as for example described in CA Browser Forum documents. This leads to "unsupported certificate purpose" errors when verifying signed objects. This patch adds the purpose "codesign" to the table in X.509 certificate verification and the verification parameter "code_sign" to X509_VERIFY_PARAM.
…ication From OpenSSL commit 61a97676914df358dd014a9b6fe2ba01b0ebe508 openssl/openssl#18567 Correct configuration according to CA Browser forum: KU: critical,digitalSignature XKU: codeSiging Note: I did not find any other document formally defining the requirements for code signing certificates. Some combinations are explicitly forbidden, some flags can be ignored
…plication From OpenSSL commit 19914fec9bac08ca7c7917eddc1b7d1dba67e4a7 openssl/openssl#18567 The tests only cover the correct handling of the codesigning purpose in the certificates in the context of the cms command line tool. The interpretation of the certificate purpose is tested in the context of the "verify" app. The correct handling of the cms objects is tested by other tests in 80-test_cms.t.
This PR adds the purpose "Code Signing" to allow verification of certificate purposes in the context of code signing. Code signing certificates have the critical keyUsage "digitalSignature" and the non-critical extendedKeyUsage "id-kp-codeSigning".
Checklist