CMP extension to OpenSSL, incremental PR chunk 2: CRMF code#7646
CMP extension to OpenSSL, incremental PR chunk 2: CRMF code#7646DDvO wants to merge 47 commits intoopenssl:masterfrom
Conversation
DDvO
force-pushed
the
cmp_incremental2
branch
from
93a8482 to
9303dd8
Compare
DDvO
force-pushed
the
cmp_incremental2
branch
from
6e2f8a6 to
3a76b60
Compare
|
@bernd-edlinger and @FdaSilvaYY, thank you for your review comments. |
|
I've just rebased this PR again due to those ever-annoying conflicts in the auto-generated |
|
@bernd-edlinger, do you see anything more to do after the recent minor improvements, or can you meanwhile approve this? |
|
I wanted to approve already, but found a few things I overlooked before. |
I've meanwhile handled all of them, and Travis CI and AppVeyor have passed again :) |
|
Very pleased that this is now fully approved :-) |
mattcaswell
added
approval: done
in INSTALL, Configure, crypto/build.info, include/openssl/crmferr.h, crypto/err/, include/openssl/err.h, and (to be updated:) util/libcrypto.num Reviewed-by: Bernd Edlinger <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #7646)
|
Woop! Pushed! Thanks all! So...chunk 3 now? |
Basically yes :) Martin and me just need some time now to rebase our CMP code on this And I think we need to clarify first what exactly is going to be part of chunk 3: |
|
Choose the smallest self-contained piece that you can find. Ideally one that can be committed as-is so that we don't have multiple PRs in-flight that depend on each other. |
Here it is: #8669 |
| if (EVP_PKEY_decrypt(pkctx, NULL, &eksize, encKey->data, encKey->length) | ||
| <= 0 | ||
| || (ek = OPENSSL_malloc(eksize)) == NULL | ||
| || EVP_PKEY_decrypt(pkctx, ek, &eksize, encKey->data, |
There was a problem hiding this comment.
you don't use eksize, you dont cleat the ek which is secret, right?
There was a problem hiding this comment.
is the encKey->data authenticated, or could this be used for a padding oracle?
There was a problem hiding this comment.
Fixed: eksize is now used for clearing ek in #9774 .
If CRMF (and therefore encSymmKey as part of EncryptedValue) is used in context of a CMP PKIMessage, the PKIMessage has to be protected in the most use cases. Protections means applying password based or signature based message integrity. See RFC4210, chapter 5.1.3 .
| goto oom; | ||
| EVP_CIPHER_CTX_set_padding(evp_ctx, 0); | ||
|
|
||
| if (!EVP_DecryptInit(evp_ctx, cipher, ek, iv) |
There was a problem hiding this comment.
you don't check that eksize is equal to EVP_CIPHER_key_length
or try to set EVP_CIPHER_CTX_set_key_length in case it is a variable key length?
| EVP_PKEY_CTX_free(pkctx); | ||
| OPENSSL_free(outbuf); | ||
| EVP_CIPHER_CTX_free(evp_ctx); | ||
| OPENSSL_free(ek); |
9 participants
Certificate Management Protocol (CMP, RFC 4210) extension to OpenSSL.
Also includes CRMF (RFC 4211) and HTTP transfer (RFC 6712).
CMP and CRMF functionality is added to
libcrypto, and thecmpapp to theopensslCLI.Adds extensive man pages and tests. Integration into build scripts.
Incremental pull request as suggested by @mattcaswell: #6811 (comment).
Initial chunk, reviewed and approved in #7328, consists of the CRMF API
as declared in the header file
include/openssl/crmf.hand its documentation in
doc/man3/OSSL_CRMF_*.podThis second chunk adds the CRMF implementation (
crypto/crmf/)and integrates it in
INSTALL,Configure,crypto/build.info,crypto/err/err.c,crypto/err/err_all.c,crypto/err/openssl.ec,crypto/err/openssl.txt,include/openssl/err.h,include/openssl/crmferr.h, andutil/libcrypto.num.Checklist