The branch master has been updated
via bb30bce22b1f1e0dd6e3e33f28ddb24dc5b285ab (commit)
via 543a802fabc6e53cd7b50b5561b9b0abbf769667 (commit)
via 61994781011ba4dde5b546971623ce6590d5d60f (commit)
from 7eb48cfc66372772c088c7ef1f443432a36e8a5c (commit)
- Log -----------------------------------------------------------------
commit bb30bce22b1f1e0dd6e3e33f28ddb24dc5b285ab
Author: Dr. David von Oheimb <david.von.ohe...@siemens.com>
Date: Tue Sep 8 15:30:33 2020 +0200
bugfix in apps/cmp.c and cmp_client.c: inconsistencies on retrieving
extraCerts in code and doc
Reviewed-by: Tim Hudson <t...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/12822)
commit 543a802fabc6e53cd7b50b5561b9b0abbf769667
Author: Dr. David von Oheimb <david.von.ohe...@siemens.com>
Date: Fri Sep 4 17:09:13 2020 +0200
bugfix in ossl_cmp_msg_protect(): set senderKID and extend extraCerts also
for unprotected CMP requests
Reviewed-by: Tim Hudson <t...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/12822)
commit 61994781011ba4dde5b546971623ce6590d5d60f
Author: Dr. David von Oheimb <david.von.ohe...@siemens.com>
Date: Fri Sep 4 10:58:26 2020 +0200
bugfix in ossl_cmp_msg_add_extraCerts(): should include cert chain when
using PBM
Reviewed-by: Tim Hudson <t...@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/12822)
-----------------------------------------------------------------------
Summary of changes:
apps/cmp.c | 40 ++++++++++++++++++++++------------------
crypto/cmp/cmp_client.c | 15 ++++++++-------
crypto/cmp/cmp_protect.c | 27 ++++++++++++++-------------
doc/man1/openssl-cmp.pod.in | 8 ++++----
doc/man3/OSSL_CMP_CTX_new.pod | 10 +++++-----
5 files changed, 53 insertions(+), 47 deletions(-)
diff --git a/apps/cmp.c b/apps/cmp.c
index 3e7b010fcb..1c3b592f7f 100644
--- a/apps/cmp.c
+++ b/apps/cmp.c
@@ -2818,27 +2818,27 @@ int cmp_main(int argc, char **argv)
switch (opt_cmd) {
case CMP_IR:
newcert = OSSL_CMP_exec_IR_ses(cmp_ctx);
- if (newcert == NULL)
- goto err;
+ if (newcert != NULL)
+ ret = 1;
break;
case CMP_KUR:
newcert = OSSL_CMP_exec_KUR_ses(cmp_ctx);
- if (newcert == NULL)
- goto err;
+ if (newcert != NULL)
+ ret = 1;
break;
case CMP_CR:
newcert = OSSL_CMP_exec_CR_ses(cmp_ctx);
- if (newcert == NULL)
- goto err;
+ if (newcert != NULL)
+ ret = 1;
break;
case CMP_P10CR:
newcert = OSSL_CMP_exec_P10CR_ses(cmp_ctx);
- if (newcert == NULL)
- goto err;
+ if (newcert != NULL)
+ ret = 1;
break;
case CMP_RR:
- if (OSSL_CMP_exec_RR_ses(cmp_ctx) == NULL)
- goto err;
+ if (OSSL_CMP_exec_RR_ses(cmp_ctx) != NULL)
+ ret = 1;
break;
case CMP_GENM:
{
@@ -2852,10 +2852,11 @@ int cmp_main(int argc, char **argv)
OSSL_CMP_CTX_push0_genm_ITAV(cmp_ctx, itav);
}
- if ((itavs = OSSL_CMP_exec_GENM_ses(cmp_ctx)) == NULL)
- goto err;
- print_itavs(itavs);
- sk_OSSL_CMP_ITAV_pop_free(itavs, OSSL_CMP_ITAV_free);
+ if ((itavs = OSSL_CMP_exec_GENM_ses(cmp_ctx)) != NULL) {
+ print_itavs(itavs);
+ sk_OSSL_CMP_ITAV_pop_free(itavs, OSSL_CMP_ITAV_free);
+ ret = 1;
+ }
break;
}
default:
@@ -2863,7 +2864,7 @@ int cmp_main(int argc, char **argv)
}
{
- /* print PKIStatusInfo (this is in case there has been no error) */
+ /* print PKIStatusInfo */
int status = OSSL_CMP_CTX_get_status(cmp_ctx);
char *buf = app_malloc(OSSL_CMP_PKISI_BUFLEN, "PKIStatusInfo buf");
const char *string =
@@ -2885,11 +2886,14 @@ int cmp_main(int argc, char **argv)
OPENSSL_free(buf);
}
- if (save_free_certs(cmp_ctx, OSSL_CMP_CTX_get1_caPubs(cmp_ctx),
- opt_cacertsout, "CA") < 0)
- goto err;
if (save_free_certs(cmp_ctx, OSSL_CMP_CTX_get1_extraCertsIn(cmp_ctx),
opt_extracertsout, "extra") < 0)
+ ret = 0;
+ if (!ret)
+ goto err;
+ ret = 0;
+ if (save_free_certs(cmp_ctx, OSSL_CMP_CTX_get1_caPubs(cmp_ctx),
+ opt_cacertsout, "CA") < 0)
goto err;
if (newcert != NULL) {
STACK_OF(X509) *certs = sk_X509_new_null();
diff --git a/crypto/cmp/cmp_client.c b/crypto/cmp/cmp_client.c
index 4f8a9e2444..fe7168916a 100644
--- a/crypto/cmp/cmp_client.c
+++ b/crypto/cmp/cmp_client.c
@@ -190,6 +190,11 @@ static int send_receive_check(OSSL_CMP_CTX *ctx, const
OSSL_CMP_MSG *req,
*/
ossl_cmp_log1(INFO, ctx, "received %s", ossl_cmp_bodytype_to_string(bt));
+ /* copy received extraCerts to ctx->extraCertsIn so they can be retrieved
*/
+ if (bt != OSSL_CMP_PKIBODY_POLLREP && bt != OSSL_CMP_PKIBODY_PKICONF
+ && !ossl_cmp_ctx_set1_extraCertsIn(ctx, (*rep)->extraCerts))
+ return 0;
+
if (!ossl_cmp_msg_check_update(ctx, *rep, unprotected_exception,
expected_type))
return 0;
@@ -470,7 +475,7 @@ static X509 *get1_cert_status(OSSL_CMP_CTX *ctx, int
bodytype,
/*-
* Callback fn validating that the new certificate can be verified, using
* ctx->certConf_cb_arg, which has been initialized using opt_out_trusted, and
- * ctx->untrusted, which at this point already contains ctx->extraCertsIn.
+ * ctx->untrusted, which at this point already contains msg->extraCerts.
* Returns 0 on acceptance, else a bit field reflecting PKIFailureInfo.
* Quoting from RFC 4210 section 5.1. Overall PKI Message:
* The extraCerts field can contain certificates that may be useful to
@@ -595,10 +600,6 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int
rid,
&& !ossl_cmp_ctx_set1_caPubs(ctx, crepmsg->caPubs))
return 0;
- /* copy received extraCerts to ctx->extraCertsIn so they can be retrieved
*/
- if (!ossl_cmp_ctx_set1_extraCertsIn(ctx, (*resp)->extraCerts))
- return 0;
-
subj = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0);
if (rkey != NULL
/* X509_check_private_key() also works if rkey is just public key */
@@ -606,8 +607,8 @@ static int cert_response(OSSL_CMP_CTX *ctx, int sleep, int
rid,
fail_info = 1 << OSSL_CMP_PKIFAILUREINFO_incorrectData;
txt = "public key in new certificate does not match our enrollment
key";
/*-
- * not callling (void)ossl_cmp_exchange_error(ctx,
- * OSSL_CMP_PKISTATUS_rejection, fail_info, txt)
+ * not calling (void)ossl_cmp_exchange_error(ctx,
+ * OSSL_CMP_PKISTATUS_rejection, fail_info, txt)
* not throwing CMP_R_CERTIFICATE_NOT_ACCEPTED with txt
* not returning 0
* since we better leave this for the certConf_cb to decide
diff --git a/crypto/cmp/cmp_protect.c b/crypto/cmp/cmp_protect.c
index 6313cc94ce..a6a0f9f9e0 100644
--- a/crypto/cmp/cmp_protect.c
+++ b/crypto/cmp/cmp_protect.c
@@ -140,7 +140,8 @@ int ossl_cmp_msg_add_extraCerts(OSSL_CMP_CTX *ctx,
OSSL_CMP_MSG *msg)
return 0;
/* Add first ctx->cert and its chain if using signature-based protection */
- if (!ctx->unprotectedSend && ctx->secretValue == NULL) {
+ if (!ctx->unprotectedSend && ctx->secretValue == NULL
+ && ctx->cert != NULL && ctx->pkey != NULL) {
int flags_prepend = X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP
| X509_ADD_FLAG_PREPEND | X509_ADD_FLAG_NO_SS;
@@ -178,7 +179,7 @@ int ossl_cmp_msg_add_extraCerts(OSSL_CMP_CTX *ctx,
OSSL_CMP_MSG *msg)
X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
return 0;
- /* if none was found avoid empty ASN.1 sequence */
+ /* in case extraCerts are empty list avoid empty ASN.1 sequence */
if (sk_X509_num(msg->extraCerts) == 0) {
sk_X509_free(msg->extraCerts);
msg->extraCerts = NULL;
@@ -271,11 +272,11 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG
*msg)
ASN1_BIT_STRING_free(msg->protection);
msg->protection = NULL;
- if (ctx->unprotectedSend)
- return 1;
-
- /* use PasswordBasedMac according to 5.1.3.1 if secretValue is given */
- if (ctx->secretValue != NULL) {
+ if (ctx->unprotectedSend) {
+ if (!set_senderKID(ctx, msg, NULL))
+ goto err;
+ } else if (ctx->secretValue != NULL) {
+ /* use PasswordBasedMac according to 5.1.3.1 if secretValue is given */
if (!set_pbmac_algor(ctx, &msg->header->protectionAlg))
goto err;
if (!set_senderKID(ctx, msg, NULL))
@@ -309,11 +310,12 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG
*msg)
CMPerr(0, CMP_R_MISSING_KEY_INPUT_FOR_CREATING_PROTECTION);
goto err;
}
- if ((msg->protection = ossl_cmp_calc_protection(ctx, msg)) == NULL)
+ if (!ctx->unprotectedSend
+ && ((msg->protection = ossl_cmp_calc_protection(ctx, msg)) ==
NULL))
goto err;
/*
- * If present, add ctx->cert followed by its chain as far as possible.
+ * For signature-based protection add ctx->cert followed by its chain.
* Finally add any additional certificates from ctx->extraCertsOut;
* even if not needed to validate the protection
* the option to do this might be handy for certain use cases.
@@ -326,11 +328,10 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG
*msg)
* to the client it set to NULL-DN. In this case for identification at
least
* the senderKID must be set, where we took the referenceValue as fallback.
*/
- if (ossl_cmp_general_name_is_NULL_DN(msg->header->sender)
- && msg->header->senderKID == NULL)
- CMPerr(0, CMP_R_MISSING_SENDER_IDENTIFICATION);
- else
+ if (!(ossl_cmp_general_name_is_NULL_DN(msg->header->sender)
+ && msg->header->senderKID == NULL))
return 1;
+ CMPerr(0, CMP_R_MISSING_SENDER_IDENTIFICATION);
err:
CMPerr(0, CMP_R_ERROR_PROTECTING_MESSAGE);
diff --git a/doc/man1/openssl-cmp.pod.in b/doc/man1/openssl-cmp.pod.in
index 75ee82211d..9389701893 100644
--- a/doc/man1/openssl-cmp.pod.in
+++ b/doc/man1/openssl-cmp.pod.in
@@ -590,13 +590,13 @@ with a signature key."
=item B<-extracertsout> I<filename>
-The file where to save any extra certificates received in the extraCerts field
-of response messages.
+The file where to save all certificates contained in the extraCerts field
+of the last received response message (except for pollRep and PKIConf).
=item B<-cacertsout> I<filename>
-The file where to save any CA certificates received in the caPubs field of
-Initialization Response (IP) messages.
+The file where to save any CA certificates contained in the caPubs field of
+the last received certificate response (i.e., IP, CP, or KUP) message.
=back
diff --git a/doc/man3/OSSL_CMP_CTX_new.pod b/doc/man3/OSSL_CMP_CTX_new.pod
index f619a65d3f..d581556ff1 100644
--- a/doc/man3/OSSL_CMP_CTX_new.pod
+++ b/doc/man3/OSSL_CMP_CTX_new.pod
@@ -617,14 +617,14 @@ OSSL_CMP_CTX_get1_newChain() returns a pointer to a
duplicate of the stack of
X.509 certificates computed by OSSL_CMP_certConf_cb() (if this function has
been called) on the last received certificate response message IP/CP/KUP.
-OSSL_CMP_CTX_get1_caPubs() returns a pointer to a duplicate of the stack of
+OSSL_CMP_CTX_get1_caPubs() returns a pointer to a duplicate of the list of
X.509 certificates received in the caPubs field of last received certificate
response message IP/CP/KUP.
-OSSL_CMP_CTX_get1_extraCertsIn() returns a pointer to a duplicate of the stack
-of X.509 certificates received in the last received nonempty extraCerts field.
-Returns an empty stack if no extraCerts have been received in the current
-transaction.
+OSSL_CMP_CTX_get1_extraCertsIn() returns a pointer to a duplicate of the list
+of X.509 certificates contained in the extraCerts field of the last received
+response message (except for pollRep and PKIConf), or
+an empty stack if no extraCerts have been received in the current transaction.
OSSL_CMP_CTX_set1_transactionID() sets the given transaction ID in the given
OSSL_CMP_CTX structure.
