Skip to content

Commit

Permalink
Remove callback support, increase coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
@WillChilds-Klein
WillChilds-Klein committed Sep 29, 2024
1 parent 94322c6 commit 9b7deca
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 60 deletions.
53 changes: 44 additions & 9 deletions crypto/pkcs7/bio/bio_deprecated_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,18 @@ struct MessageDigestParams {
};

static const struct MessageDigestParams MessageDigests[] = {
{"SHA_256", EVP_sha256},
{"MD5", EVP_md5},
{"SHA1", EVP_sha1},
{"SHA224", EVP_sha224},
{"SHA256", EVP_sha256},
{"SHA284", EVP_sha384},
{"SHA512", EVP_sha512},
{"SHA512_224", EVP_sha512_224},
{"SHA512_256", EVP_sha512_256},
{"SHA3_224", EVP_sha3_224},
{"SHA3_256", EVP_sha3_256},
{"SHA3_384", EVP_sha3_384},
{"SHA3_512", EVP_sha3_512},
};

class BIODeprecatedTest : public testing::TestWithParam<MessageDigestParams> {};
Expand Down Expand Up @@ -44,6 +55,19 @@ TEST_P(BIODeprecatedTest, MessageDigest) {

// TODO [childw] bad input
// - ptr to non-const MD
bio_md.reset(BIO_new(BIO_f_md()));
ASSERT_TRUE(bio_md);
EXPECT_TRUE(BIO_set_md(bio_md.get(), (EVP_MD *)md));
EVP_MD_CTX *ctx_tmp; // |bio_md| owns the context, we just take a ref here
EXPECT_TRUE(BIO_get_md_ctx(bio_md.get(), &ctx_tmp));
EXPECT_EQ(EVP_MD_type(md), EVP_MD_CTX_type(ctx_tmp));
EXPECT_EQ(md, EVP_MD_CTX_md(ctx_tmp)); // for static *EVP_MD_CTX, ptrs equal
EXPECT_FALSE(BIO_ctrl(bio_md.get(), BIO_C_GET_MD, 0, nullptr));
EXPECT_FALSE(BIO_ctrl(bio_md.get(), BIO_C_SET_MD_CTX, 0, nullptr));
EXPECT_FALSE(BIO_ctrl(bio_md.get(), BIO_C_DO_STATE_MACHINE, 0, nullptr));
EXPECT_FALSE(BIO_ctrl(bio_md.get(), BIO_CTRL_DUP, 0, nullptr));
EXPECT_FALSE(BIO_ctrl(bio_md.get(), BIO_CTRL_GET_CALLBACK, 0, nullptr));
EXPECT_FALSE(BIO_ctrl(bio_md.get(), BIO_CTRL_SET_CALLBACK, 0, nullptr));

// Write-through digest BIO
bio_md.reset(BIO_new(BIO_f_md()));
Expand All @@ -54,7 +78,7 @@ TEST_P(BIODeprecatedTest, MessageDigest) {
bio.reset(BIO_push(bio_md.get(), bio_mem.get()));
ASSERT_TRUE(bio);
EXPECT_TRUE(BIO_write(bio.get(), message, sizeof(message)));
unsigned digest_len = BIO_gets(bio_md.get(), (char*) buf, sizeof(buf));
unsigned digest_len = BIO_gets(bio_md.get(), (char *)buf, sizeof(buf));
buf_vec.clear();
buf_vec.insert(buf_vec.begin(), buf, buf + digest_len);
OPENSSL_memset(buf, '\0', sizeof(buf));
Expand All @@ -65,9 +89,10 @@ TEST_P(BIODeprecatedTest, MessageDigest) {
}
ctx.reset(EVP_MD_CTX_new());
ASSERT_TRUE(EVP_DigestInit_ex(ctx.get(), md, NULL));
ASSERT_TRUE(EVP_DigestUpdate(ctx.get(), message_vec.data(), message_vec.size()));
ASSERT_TRUE(
EVP_DigestUpdate(ctx.get(), message_vec.data(), message_vec.size()));
ASSERT_TRUE(EVP_DigestFinal_ex(ctx.get(), buf, &digest_len));
EXPECT_EQ(Bytes(buf, digest_len), Bytes(buf_vec.data(), buf_vec.size()));
EXPECT_EQ(Bytes(buf_vec.data(), buf_vec.size()), Bytes(buf, digest_len));
bio_md.release(); // |bio| took ownership
bio_mem.release(); // |bio| took ownership

Expand All @@ -82,17 +107,27 @@ TEST_P(BIODeprecatedTest, MessageDigest) {
message_vec.clear();
OPENSSL_memset(buf, '\0', sizeof(buf));
while ((rsize = BIO_read(bio.get(), buf, sizeof(buf))) > 0) {
message_vec.insert(message_vec.end(), buf, buf + rsize);
message_vec.insert(message_vec.begin(), buf, buf + rsize);
}
EXPECT_EQ(Bytes(message_vec.data(), message_vec.size()), Bytes(message, sizeof(message)));
digest_len = BIO_gets(bio_md.get(), (char*) buf, sizeof(buf));
EXPECT_EQ(Bytes(message_vec.data(), message_vec.size()),
Bytes(message, sizeof(message)));
digest_len = BIO_gets(bio_md.get(), (char *)buf, sizeof(buf));
buf_vec.clear();
buf_vec.insert(buf_vec.begin(), buf, buf + digest_len);
ctx.reset(EVP_MD_CTX_new());
ASSERT_TRUE(EVP_DigestInit_ex(ctx.get(), md, NULL));
ASSERT_TRUE(EVP_DigestUpdate(ctx.get(), message_vec.data(), message_vec.size()));
ASSERT_TRUE(
EVP_DigestUpdate(ctx.get(), message_vec.data(), message_vec.size()));
ASSERT_TRUE(EVP_DigestFinal_ex(ctx.get(), buf, &digest_len));
EXPECT_EQ(Bytes(buf, digest_len), Bytes(buf_vec.data(), buf_vec.size()));
EXPECT_EQ(Bytes(buf_vec.data(), buf_vec.size()), Bytes(buf, digest_len));
// Resetting |bio_md| should reset digest state, elicit different digest
// output
EXPECT_TRUE(BIO_reset(bio.get()));
digest_len = BIO_gets(bio_md.get(), (char *)buf, sizeof(buf));
EXPECT_NE(Bytes(buf_vec.data(), buf_vec.size()), Bytes(buf, digest_len));
bio_md.release(); // |bio| took ownership
bio_mem.release(); // |bio| took ownership
}

// Callback tests
}
68 changes: 18 additions & 50 deletions crypto/pkcs7/bio/md.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@ static int md_gets(BIO *h, char *str, int size);
static long md_ctrl(BIO *h, int cmd, long arg1, void *arg2);
static int md_new(BIO *h);
static int md_free(BIO *data);
static long md_callback_ctrl(BIO *h, int cmd, bio_info_cb fp);


static const BIO_METHOD methods_md = {
BIO_TYPE_MD, "message digest", md_write, md_read, NULL, /* md_puts, */
md_gets, md_ctrl, md_new, md_free, md_callback_ctrl,
BIO_TYPE_MD, "message digest", md_write, md_read, /*puts*/ NULL,
md_gets, md_ctrl, md_new, md_free, /*callback_ctrl*/ NULL,
};
const BIO_METHOD *BIO_f_md(void) { return &methods_md; }

Expand Down Expand Up @@ -68,7 +67,7 @@ static int md_read(BIO *b, char *out, int outl) {
ret = BIO_read(next, out, outl);
if (BIO_get_init(b)) {
if (ret > 0) {
if (EVP_DigestUpdate(ctx, (unsigned char *)out, (unsigned int)ret) <= 0) {
if (EVP_DigestUpdate(ctx, (unsigned char *)out, ret) <= 0) {
return -1;
}
}
Expand Down Expand Up @@ -97,8 +96,7 @@ static int md_write(BIO *b, const char *in, int inl) {

if (BIO_get_init(b)) {
if (ret > 0) {
if (!EVP_DigestUpdate(ctx, (const unsigned char *)in,
(unsigned int)ret)) {
if (!EVP_DigestUpdate(ctx, (const unsigned char *)in, ret)) {
BIO_clear_retry_flags(b);
return 0;
}
Expand All @@ -113,11 +111,10 @@ static int md_write(BIO *b, const char *in, int inl) {

static long md_ctrl(BIO *b, int cmd, long num, void *ptr) {
GUARD_PTR(b);
EVP_MD_CTX *ctx, *dctx, **pctx;
const EVP_MD **ppmd;
EVP_MD_CTX *ctx, **pctx;
EVP_MD *md;
long ret = 1;
BIO *dbio, *next;
BIO *next;

ctx = BIO_get_data(b);
next = BIO_next(b);
Expand All @@ -133,66 +130,37 @@ static long md_ctrl(BIO *b, int cmd, long num, void *ptr) {
ret = BIO_ctrl(next, cmd, num, ptr);
}
break;
case BIO_C_GET_MD:
if (BIO_get_init(b)) {
ppmd = ptr;
*ppmd = EVP_MD_CTX_md(ctx);
} else {
ret = 0;
}
break;
case BIO_C_GET_MD_CTX:
pctx = ptr;
*pctx = ctx;
BIO_set_init(b, 1);
break;
case BIO_C_SET_MD_CTX:
if (BIO_get_init(b)) {
BIO_set_data(b, ptr);
} else {
ret = 0;
}
break;
case BIO_C_DO_STATE_MACHINE:
BIO_clear_retry_flags(b);
ret = BIO_ctrl(next, cmd, num, ptr);
BIO_copy_next_retry(b);
break;
case BIO_C_SET_MD:
md = ptr;
ret = EVP_DigestInit_ex(ctx, md, NULL);
if (ret > 0) {
BIO_set_init(b, 1);
}
break;
// OpenSSL implements these, but because we don't need them and cipher BIO
// is internal, we can fail loudly if they're called. If this case is hit,
// it likely means you're making a change that will require implementing
// these.
case BIO_C_GET_MD:
case BIO_C_SET_MD_CTX:
case BIO_C_DO_STATE_MACHINE:
case BIO_CTRL_DUP:
dbio = ptr;
dctx = BIO_get_data(dbio);
if (!EVP_MD_CTX_copy_ex(dctx, ctx)) {
return 0;
}
BIO_set_init(b, 1);
break;
case BIO_CTRL_GET_CALLBACK:
case BIO_CTRL_SET_CALLBACK:
OPENSSL_PUT_ERROR(PKCS7, ERR_R_BIO_LIB);
return 0;
default:
ret = BIO_ctrl(next, cmd, num, ptr);
break;
}
return ret;
}

static long md_callback_ctrl(BIO *b, int cmd, bio_info_cb fp) {
GUARD_PTR(b);
BIO *next;

next = BIO_next(b);

if (next == NULL) {
return 0;
}

return BIO_callback_ctrl(next, cmd, fp);
}

static int md_gets(BIO *b, char *buf, int size) {
GUARD_PTR(b);
GUARD_PTR(buf);
Expand All @@ -210,4 +178,4 @@ static int md_gets(BIO *b, char *buf, int size) {
}

return ret;
}
}
1 change: 0 additions & 1 deletion crypto/pkcs7/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ int pkcs7_add_signed_data(CBB *out,
OPENSSL_EXPORT const BIO_METHOD *BIO_f_md(void);
#define BIO_get_md_ctx(b, mdcp) BIO_ctrl(b, BIO_C_GET_MD_CTX, 0, mdcp)
#define BIO_set_md(b, md) BIO_ctrl(b, BIO_C_SET_MD, 0, md)
#define BIO_set_md_ctx(b, ctx) BIO_ctrl(b, BIO_C_SET_MD_CTX, 0, ctx)

#if defined(__cplusplus)
} // extern C
Expand Down

0 comments on commit 9b7deca

Please sign in to comment.