Skip to content

Commit

Permalink
Make CMP server use same protection for response as for request
Browse files Browse the repository at this point in the history
Also adds ossl_cmp_hdr_get_protection_nid() simplifying cmp_vfy.c

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #11998)
  • Loading branch information
DDvO committed Jun 13, 2020
1 parent 5aed178 commit 12bbcee
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 31 deletions.
8 changes: 8 additions & 0 deletions crypto/cmp/cmp_hdr.c
Expand Up @@ -41,6 +41,14 @@ int ossl_cmp_hdr_get_pvno(const OSSL_CMP_PKIHEADER *hdr)
return (int)pvno;
}

int ossl_cmp_hdr_get_protection_nid(const OSSL_CMP_PKIHEADER *hdr)
{
if (!ossl_assert(hdr != NULL)
|| hdr->protectionAlg == NULL)
return NID_undef;
return OBJ_obj2nid(hdr->protectionAlg->algorithm);
}

ASN1_OCTET_STRING *OSSL_CMP_HDR_get0_transactionID(const
OSSL_CMP_PKIHEADER *hdr)
{
Expand Down
1 change: 1 addition & 0 deletions crypto/cmp/cmp_local.h
Expand Up @@ -796,6 +796,7 @@ int ossl_cmp_pkisi_check_pkifailureinfo(const OSSL_CMP_PKISI *si, int index);
/* from cmp_hdr.c */
int ossl_cmp_hdr_set_pvno(OSSL_CMP_PKIHEADER *hdr, int pvno);
int ossl_cmp_hdr_get_pvno(const OSSL_CMP_PKIHEADER *hdr);
int ossl_cmp_hdr_get_protection_nid(const OSSL_CMP_PKIHEADER *hdr);
ASN1_OCTET_STRING *ossl_cmp_hdr_get0_senderNonce(const OSSL_CMP_PKIHEADER *hdr);
int ossl_cmp_general_name_is_NULL_DN(GENERAL_NAME *name);
int ossl_cmp_hdr_set1_sender(OSSL_CMP_PKIHEADER *hdr, const X509_NAME *nm);
Expand Down
6 changes: 2 additions & 4 deletions crypto/cmp/cmp_protect.c
Expand Up @@ -259,10 +259,8 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
goto err;
}

if (msg->header->protectionAlg == NULL)
if ((msg->header->protectionAlg = X509_ALGOR_new()) == NULL)
goto err;

if ((msg->header->protectionAlg = X509_ALGOR_new()) == NULL)
goto err;
if (!OBJ_find_sigid_by_algs(&algNID, ctx->digest,
EVP_PKEY_id(ctx->pkey))) {
CMPerr(0, CMP_R_UNSUPPORTED_KEY_TYPE);
Expand Down
32 changes: 22 additions & 10 deletions crypto/cmp/cmp_server.c
Expand Up @@ -452,8 +452,10 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
const OSSL_CMP_MSG *req)
{
OSSL_CMP_CTX *ctx;
ASN1_OCTET_STRING *backup_secret;
OSSL_CMP_PKIHEADER *hdr;
int req_type, rsp_type;
int res;
OSSL_CMP_MSG *rsp = NULL;

if (srv_ctx == NULL || srv_ctx->ctx == NULL
Expand All @@ -463,7 +465,12 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
return 0;
}
ctx = srv_ctx->ctx;
backup_secret = ctx->secretValue;

/*
* Some things need to be done already before validating the message in
* order to be able to send an error message as far as needed and possible.
*/
if (hdr->sender->type != GEN_DIRNAME) {
CMPerr(0, CMP_R_SENDER_GENERALNAME_TYPE_NOT_SUPPORTED);
goto err;
Expand Down Expand Up @@ -506,8 +513,12 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
}
}

if (!ossl_cmp_msg_check_update(ctx, req, unprotected_exception,
srv_ctx->acceptUnprotected))
res = ossl_cmp_msg_check_update(ctx, req, unprotected_exception,
srv_ctx->acceptUnprotected);
if (ctx->secretValue != NULL && ctx->pkey != NULL
&& ossl_cmp_hdr_get_protection_nid(hdr) != NID_id_PasswordBasedMAC)
ctx->secretValue = NULL; /* use MSG_SIG_ALG when protecting rsp */
if (!res)
goto err;

switch (req_type) {
Expand Down Expand Up @@ -573,15 +584,16 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
}

if ((si = OSSL_CMP_STATUSINFO_new(OSSL_CMP_PKISTATUS_rejection,
fail_info, NULL)) == NULL)
return 0;
if (err != 0 && (flags & ERR_TXT_STRING) != 0)
data = ERR_reason_error_string(err);
rsp = ossl_cmp_error_new(srv_ctx->ctx, si,
err != 0 ? ERR_GET_REASON(err) : -1,
data, srv_ctx->sendUnprotectedErrors);
OSSL_CMP_PKISI_free(si);
fail_info, NULL)) != NULL) {
if (err != 0 && (flags & ERR_TXT_STRING) != 0)
data = ERR_reason_error_string(err);
rsp = ossl_cmp_error_new(srv_ctx->ctx, si,
err != 0 ? ERR_GET_REASON(err) : -1,
data, srv_ctx->sendUnprotectedErrors);
OSSL_CMP_PKISI_free(si);
}
}
ctx->secretValue = backup_secret;

/* possibly close the transaction */
rsp_type =
Expand Down
20 changes: 4 additions & 16 deletions crypto/cmp/cmp_vfy.c
Expand Up @@ -70,7 +70,7 @@ static int verify_signature(const OSSL_CMP_CTX *cmp_ctx,
prot_part_der_len = (size_t) len;

/* verify signature of protected part */
if (!OBJ_find_sigid_algs(OBJ_obj2nid(msg->header->protectionAlg->algorithm),
if (!OBJ_find_sigid_algs(ossl_cmp_hdr_get_protection_nid(msg->header),
&digest_nid, &pk_nid)
|| digest_nid == NID_undef || pk_nid == NID_undef
|| (digest = EVP_get_digestbynid(digest_nid)) == NULL) {
Expand Down Expand Up @@ -574,9 +574,6 @@ static int check_msg_find_cert(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
*/
int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
{
X509_ALGOR *alg;
int nid = NID_undef, pk_nid = NID_undef;
const ASN1_OBJECT *algorOID = NULL;
X509 *scrt;
const X509_NAME *expected_sender;

Expand Down Expand Up @@ -605,17 +602,13 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
return 0;
/* Note: if recipient was NULL-DN it could be learned here if needed */

if ((alg = msg->header->protectionAlg) == NULL /* unprotected message */
if (msg->header->protectionAlg == NULL /* unprotected message */
|| msg->protection == NULL || msg->protection->data == NULL) {
CMPerr(0, CMP_R_MISSING_PROTECTION);
return 0;
}

/* determine the nid for the used protection algorithm */
X509_ALGOR_get0(&algorOID, NULL, NULL, alg);
nid = OBJ_obj2nid(algorOID);

switch (nid) {
switch (ossl_cmp_hdr_get_protection_nid(msg->header)) {
/* 5.1.3.1. Shared Secret Information */
case NID_id_PasswordBasedMAC:
if (ctx->secretValue == 0) {
Expand Down Expand Up @@ -665,11 +658,6 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
* 5.1.3.3. Signature
*/
default:
if (!OBJ_find_sigid_algs(OBJ_obj2nid(alg->algorithm), NULL, &pk_nid)
|| pk_nid == NID_undef) {
CMPerr(0, CMP_R_UNKNOWN_ALGORITHM_ID);
break;
}
scrt = ctx->srvCert;
if (scrt == NULL) {
if (check_msg_find_cert(ctx, msg))
Expand Down Expand Up @@ -727,7 +715,7 @@ int ossl_cmp_msg_check_update(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
return 0;

/* validate message protection */
if (msg->header->protectionAlg != 0) {
if (msg->header->protectionAlg != NULL) {
/* detect explicitly permitted exceptions for invalid protection */
if (!OSSL_CMP_validate_msg(ctx, msg)
&& (cb == NULL || (*cb)(ctx, msg, 1, cb_arg) <= 0)) {
Expand Down
9 changes: 8 additions & 1 deletion doc/internal/man3/ossl_cmp_hdr_init.pod
Expand Up @@ -4,6 +4,7 @@

ossl_cmp_hdr_set_pvno,
ossl_cmp_hdr_get_pvno,
ossl_cmp_hdr_get_protection_nid,
ossl_cmp_hdr_get0_sendernonce,
ossl_cmp_general_name_is_NULL_DN,
ossl_cmp_hdr_set1_sender,
Expand All @@ -25,6 +26,7 @@ ossl_cmp_hdr_init

int ossl_cmp_hdr_set_pvno(OSSL_CMP_PKIHEADER *hdr, int pvno);
int ossl_cmp_hdr_get_pvno(const OSSL_CMP_PKIHEADER *hdr);
int ossl_cmp_hdr_get_protection_nid(const OSSL_CMP_PKIHEADER *hdr);
ASN1_OCTET_STRING
*ossl_cmp_hdr_get0_sendernonce(const OSSL_CMP_PKIHEADER *hdr);
int ossl_cmp_general_name_is_NULL_DN(GENERAL_NAME *name);
Expand Down Expand Up @@ -52,6 +54,9 @@ ossl_cmp_hdr_set_pvno() sets hdr->pvno to the given B<pvno>.

ossl_cmp_hdr_get_pvno() returns the pvno of the given B<hdr> or -1 on error.

ossl_cmp_hdr_get_protection_nid returns the NID of the protection algorithm
in B<hdr> or NID_undef on error.

ossl_cmp_hdr_get0_sendernonce() returns the sender nonce of the given PKIHeader.

ossl_cmp_general_name_is_NULL_DN() determines if the given GENERAL_NAME
Expand Down Expand Up @@ -110,7 +115,9 @@ CMP is defined in RFC 4210 (and CRMF in RFC 4211).

ossl_cmp_hdr_get_pvno() returns the pvno of the given B<hdr> or -1 on error.

ossl_cmp_hdr_get0_sendernonce() returns the respective nonce.
ossl_cmp_hdr_get_protection_nid returns the respective NID, NID_undef on error.

ossl_cmp_hdr_get0_sendernonce() returns the respective nonce, or NULL.

ossl_cmp_general_name_is_NULL_DN() returns 1 given a NULL-DN, else 0.

Expand Down

0 comments on commit 12bbcee

Please sign in to comment.