Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/wolfprovider/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ int wp_decrypt_key_pkcs8(unsigned char* data, word32* len,
int wp_read_der_bio(WOLFPROV_CTX* provCtx, OSSL_CORE_BIO *coreBio, unsigned char** data, word32* len);
int wp_read_pem_bio(WOLFPROV_CTX *provctx, OSSL_CORE_BIO *coreBio,
unsigned char** data, word32* len);
int wp_write_bio(BIO* bio, const unsigned char* data, size_t len);
BIO* wp_corebio_get_bio(WOLFPROV_CTX* provCtx, OSSL_CORE_BIO *coreBio);

#ifdef HAVE_FIPS
Expand Down
5 changes: 1 addition & 4 deletions src/wp_dh_kmgmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -2965,10 +2965,7 @@ static int wp_dh_encode(wp_DhEncDecCtx* ctx, OSSL_CORE_BIO *cBio,
}
}
if (ok) {
rc = BIO_write(out, keyData, (int)keyLen);
if (rc <= 0) {
ok = 0;
}
ok = wp_write_bio(out, keyData, keyLen);
}

if (private) {
Expand Down
5 changes: 1 addition & 4 deletions src/wp_ecc_kmgmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -3090,10 +3090,7 @@ static int wp_ecc_encode(wp_EccEncDecCtx* ctx, OSSL_CORE_BIO *cBio,
}
}
if (ok) {
rc = BIO_write(out, keyData, (int)keyLen);
if (rc <= 0) {
ok = 0;
}
ok = wp_write_bio(out, keyData, keyLen);
}

if (private) {
Expand Down
5 changes: 1 addition & 4 deletions src/wp_ecx_kmgmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -2314,10 +2314,7 @@ static int wp_ecx_encode(wp_EcxEncDecCtx* ctx, OSSL_CORE_BIO *cBio,
}
}
if (ok) {
rc = BIO_write(out, keyData, (int)keyLen);
if (rc <= 0) {
ok = 0;
}
ok = wp_write_bio(out, keyData, keyLen);
}

/* derData holds the plaintext private key material. */
Expand Down
40 changes: 40 additions & 0 deletions src/wp_internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -1447,6 +1447,46 @@ int wp_read_pem_bio(WOLFPROV_CTX *provctx, OSSL_CORE_BIO *coreBio,
return ok;
}

/**
* Write all data to a BIO.
*
* BIO_write may write fewer bytes than requested. Loop until all data is
* written so that a short write does not silently truncate the output.
*
* @param [in] bio BIO to write to.
* @param [in] data Data to write.
* @param [in] len Length of data in bytes.
* @return 1 on success.
* @return 0 on failure.
*/
int wp_write_bio(BIO* bio, const unsigned char* data, size_t len)
{
int ok = 1;
size_t off = 0;

WOLFPROV_ENTER(WP_LOG_COMP_PROVIDER, "wp_write_bio");

if ((bio == NULL) || (data == NULL)) {
ok = 0;
}

while (ok && (off < len)) {
int rc = BIO_write(bio, data + off, (int)(len - off));
if (rc > 0) {
off += (size_t)rc;
}
else {
WOLFPROV_MSG(WP_LOG_COMP_PROVIDER, "BIO_write error (%d) in %s:%d",
rc, __FILE__, __LINE__);
ok = 0;
}
}

WOLFPROV_LEAVE(WP_LOG_COMP_PROVIDER, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__),
ok);
return ok;
}

/**
* Get the underlying BIO object from the core BIO.
*
Expand Down
5 changes: 1 addition & 4 deletions src/wp_mldsa_kmgmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -1628,10 +1628,7 @@ static int wp_mldsa_encode(wp_MlDsaEncDecCtx* ctx, OSSL_CORE_BIO* cBio,
}
}
if (ok) {
rc = BIO_write(out, keyData, (int)keyLen);
if (rc <= 0) {
ok = 0;
}
ok = wp_write_bio(out, keyData, keyLen);
}

if (private) {
Expand Down
5 changes: 1 addition & 4 deletions src/wp_rsa_kmgmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -3683,10 +3683,7 @@ static int wp_rsa_encode(wp_RsaEncDecCtx* ctx, OSSL_CORE_BIO* cBio,
}
}
if (ok) {
rc = BIO_write(out, keyData, (int)keyLen);
if (rc <= 0) {
ok = 0;
}
ok = wp_write_bio(out, keyData, keyLen);
}

if (private) {
Expand Down
133 changes: 133 additions & 0 deletions test/test_ecc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,139 @@ int test_ecc_encode_epki(void *data)
}
#endif /* WP_HAVE_EPKI_TEST */

/* Sink BIO that accepts one byte per write and keeps every byte it is given.
* It reproduces a short-writing BIO so a truncated encoding is detectable. */
typedef struct {
unsigned char buf[4096];
size_t len;
} ShortWriteSink;

static int short_write_bio_write(BIO* b, const char* data, int len)
{
ShortWriteSink* sink = (ShortWriteSink*)BIO_get_data(b);

BIO_clear_retry_flags(b);
if ((sink == NULL) || (len <= 0) || (sink->len >= sizeof(sink->buf))) {
return 0;
}
/* Take a single byte so the writer must loop to make progress. */
sink->buf[sink->len++] = (unsigned char)data[0];
return 1;
}

static long short_write_bio_ctrl(BIO* b, int cmd, long num, void* ptr)
{
(void)b;
(void)num;
(void)ptr;
return (cmd == BIO_CTRL_FLUSH) ? 1 : 0;
}

static int short_write_bio_create(BIO* b)
{
BIO_set_init(b, 1);
return 1;
}

/* Encode pkey twice with the same wolfProvider encoder: once via
* OSSL_ENCODER_to_data (a normal full-writing sink) for a reference, and once
* through a one-byte-at-a-time BIO. The two encodings must be identical. */
static int test_ecc_encode_short_write(EVP_PKEY* pkey, int selection,
const char* format, const char* structure)
{
int err = 0;
OSSL_ENCODER_CTX* ectx = NULL;
unsigned char* refData = NULL;
size_t refLen = 0;
BIO_METHOD* meth = NULL;
BIO* bio = NULL;
ShortWriteSink sink;

memset(&sink, 0, sizeof(sink));

ectx = OSSL_ENCODER_CTX_new_for_pkey(pkey, selection, format, structure,
"provider=libwolfprov");
err = (ectx == NULL);
if (err == 0) {
err = (OSSL_ENCODER_to_data(ectx, &refData, &refLen) != 1);
}
OSSL_ENCODER_CTX_free(ectx);
ectx = NULL;
if (err == 0) {
err = (refLen == 0);
}

if (err == 0) {
meth = BIO_meth_new(BIO_get_new_index() | BIO_TYPE_SOURCE_SINK,
"short-write");
err = (meth == NULL);
}
if (err == 0) {
BIO_meth_set_write(meth, short_write_bio_write);
BIO_meth_set_ctrl(meth, short_write_bio_ctrl);
BIO_meth_set_create(meth, short_write_bio_create);
bio = BIO_new(meth);
err = (bio == NULL);
}
if (err == 0) {
BIO_set_data(bio, &sink);
ectx = OSSL_ENCODER_CTX_new_for_pkey(pkey, selection, format, structure,
"provider=libwolfprov");
err = (ectx == NULL);
}
if (err == 0) {
err = (OSSL_ENCODER_to_bio(ectx, bio) != 1);
}
if (err == 0) {
err = (sink.len != refLen);
if (err) {
PRINT_ERR_MSG("Short write truncated output: %lu of %lu bytes",
(unsigned long)sink.len, (unsigned long)refLen);
}
}
if (err == 0) {
err = (memcmp(sink.buf, refData, refLen) != 0);
if (err) {
PRINT_ERR_MSG("Short-write output does not match the reference");
}
}

OSSL_ENCODER_CTX_free(ectx);
BIO_free(bio);
BIO_meth_free(meth);
OPENSSL_free(refData);

return err;
}

int test_ecc_encode_short_write_bio(void *data)
{
int err = 0;
const unsigned char* p = ecc_key_der_256;
EVP_PKEY* pkey = NULL;

(void)data;

pkey = d2i_PrivateKey_ex(EVP_PKEY_EC, NULL, &p, sizeof(ecc_key_der_256),
wpLibCtx, NULL);
err = (pkey == NULL);

if (err == 0) {
PRINT_MSG("SubjectPublicKeyInfo DER survives a short-writing BIO");
err = test_ecc_encode_short_write(pkey, EVP_PKEY_PUBLIC_KEY, "DER",
"SubjectPublicKeyInfo");
}
if (err == 0) {
PRINT_MSG("SubjectPublicKeyInfo PEM survives a short-writing BIO");
err = test_ecc_encode_short_write(pkey, EVP_PKEY_PUBLIC_KEY, "PEM",
"SubjectPublicKeyInfo");
}

EVP_PKEY_free(pkey);

return err;
}

int test_ecdh_invalid_kdf_strings(void *data)
{
int err = 0;
Expand Down
1 change: 1 addition & 0 deletions test/unit.c
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ TEST_CASE test_case[] = {
#endif
#endif
#ifdef WP_HAVE_EC_P256
TEST_DECL(test_ecc_encode_short_write_bio, NULL),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New test registered outside the WP_HAVE_ECDH guard that surrounds its definition · Incorrect macro expansion

test_ecc_encode_short_write_bio is defined in test/test_ecc.c:1153, inside the #ifdef WP_HAVE_ECDH block spanning lines 721-1634, and declared in test/unit.h:473 inside the #ifdef WP_HAVE_ECDH block starting at line 430, but registered under #ifdef WP_HAVE_EC_P256 alone. A build with P-256 and ECDSA but without HAVE_ECC_DHE fails to compile and link unit.test.

Fix: Move the definition and declaration out of the WP_HAVE_ECDH blocks so all three sites are gated identically on WP_HAVE_EC_P256.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — moved the test, its helper, and the sink BIO out of the WP_HAVE_ECDH blocks so the definition, declaration, and registration are all gated on WP_HAVE_EC_P256 alone.

#ifdef WP_HAVE_EPKI_TEST
TEST_DECL(test_ecc_encode_epki, NULL),
#endif
Expand Down
1 change: 1 addition & 0 deletions test/unit.h
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ int test_ecdh_p224(void *data);
#endif /* WP_HAVE_EC_P224 */
#ifdef WP_HAVE_EC_P256
int test_ecdh_invalid_kdf_strings(void *data);
int test_ecc_encode_short_write_bio(void *data);
#ifdef WP_HAVE_EPKI_TEST
int test_ecc_encode_epki(void *data);
#endif
Expand Down
Loading