Skip to content

Commit

Permalink
cms encrypt, better OBJ_nid2obj() return check
Browse files Browse the repository at this point in the history
Fixes #22225

In OBJ_nid2obj(), if the NID does not have an OID, then a pointer to
the special "undefined" ASN1_OBJECT is returned.  Check for the
undefined-ASN1_OBJECT and return an error.  Also, add a test for this
in 80-test_cms.t.

Testing:

  #!/bin/bash -x

  shopt -s expand_aliases

  alias openssl="LD_LIBRARY_PATH=~/git/openssl ~/git/openssl/apps/openssl"

  echo "This is a confidential message.  It should be encrypted." > msg.txt

  ## this should fail b/c there is no OID for aes-256-ctr
  openssl cms -encrypt -in msg.txt -aes-256-ctr -out msg.txt.cms -recip demos/cms/signer.pem
  echo $?

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #22392)
  • Loading branch information
James Muir authored and t8m committed Oct 18, 2023
1 parent a47fc4e commit bd16091
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
2 changes: 1 addition & 1 deletion crypto/cms/cms_enc.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ BIO *ossl_cms_EncryptedContent_init_bio(CMS_EncryptedContentInfo *ec,

if (enc) {
calg->algorithm = OBJ_nid2obj(EVP_CIPHER_CTX_get_type(ctx));
if (calg->algorithm == NULL) {
if (calg->algorithm == NULL || calg->algorithm->nid == NID_undef) {

This comment has been minimized.

Copy link
@baentsch

baentsch Oct 19, 2023

Contributor

@t8m @jamuir Am I right thinking that this line crashes CI --both in openssl master as well as in oqs-provider? What about changing to ...|| EVP_CIPHER_CTX_get_type(ctx) == NID_undef? Should I put in a PR for this or is this already underway?

This comment has been minimized.

Copy link
@t8m

t8m Oct 19, 2023

Member

IMO that wouldn't be the right fix. #22432 is the right fix IMO and it is merged now.

This comment has been minimized.

Copy link
@jamuir

jamuir Oct 19, 2023

Member

thanks for pointing this out, Michael, and thanks for fixing this, Tomas.

ERR_raise(ERR_LIB_CMS, CMS_R_UNSUPPORTED_CONTENT_ENCRYPTION_ALGORITHM);
goto err;
}
Expand Down
13 changes: 12 additions & 1 deletion test/recipes/80-test_cms.t
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ my ($no_des, $no_dh, $no_dsa, $no_ec, $no_ec2m, $no_rc2, $no_zlib)

$no_rc2 = 1 if disabled("legacy");

plan tests => 21;
plan tests => 22;

ok(run(test(["pkcs7_test"])), "test pkcs7");

Expand Down Expand Up @@ -1154,3 +1154,14 @@ with({ exit_checker => sub { return shift == 3; } },
"issue#21986");
}
});

# Test for problem reported in #22225
with({ exit_checker => sub { return shift == 3; } },
sub {
ok(run(app(['openssl', 'cms', '-encrypt',
'-in', srctop_file("test", "smcont.txt"),
'-aes-256-ctr', '-recip',
catfile($smdir, "smec1.pem"),
])),
"Check for failure when cipher does not have an assigned OID (issue#22225)");
});

0 comments on commit bd16091

Please sign in to comment.