--- crypto/openssl/crypto/cmp/cmp_protect.c.orig +++ crypto/openssl/crypto/cmp/cmp_protect.c @@ -63,7 +63,7 @@ ERR_raise(ERR_LIB_CMP, CMP_R_MISSING_PBM_SECRET); return NULL; } - if (ppval == NULL) { + if (pptype != V_ASN1_SEQUENCE || ppval == NULL) { ERR_raise(ERR_LIB_CMP, CMP_R_ERROR_CALCULATING_PROTECTION); return NULL; } --- crypto/openssl/crypto/cmp/cmp_vfy.c.orig +++ crypto/openssl/crypto/cmp/cmp_vfy.c @@ -666,6 +666,7 @@ { OSSL_CMP_PKIHEADER *hdr; const X509_NAME *expected_sender; + int num_extra_before, num_extra_after, num_added; if (!ossl_assert(ctx != NULL && msg != NULL && msg->header != NULL)) return 0; @@ -700,17 +701,27 @@ * extraCerts because they do not belong to the protected msg part anyway. * For efficiency, the extraCerts are prepended so they get used first. */ + num_extra_before = sk_X509_num(ctx->untrusted); if (!X509_add_certs(ctx->untrusted, msg->extraCerts, /* this allows self-signed certs */ X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP | X509_ADD_FLAG_PREPEND)) return 0; - + num_extra_after = sk_X509_num(ctx->untrusted); + num_added = num_extra_after - num_extra_before; /* validate message protection */ if (hdr->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)) { + /* + * remove extraCerts again if not caching + * or if we failed validation above, lest a remote user + * starts sending us lots of certificate in invalid messages + * leading to a DOS from unbounded certificate stack growth + */ + while (num_added-- > 0) + X509_free(sk_X509_shift(ctx->untrusted)); #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION ERR_raise(ERR_LIB_CMP, CMP_R_ERROR_VALIDATING_PROTECTION); return 0; @@ -719,6 +730,8 @@ } else { /* detect explicitly permitted exceptions for missing protection */ if (cb == NULL || (*cb)(ctx, msg, 0, cb_arg) <= 0) { + while (num_added-- > 0) + X509_free(sk_X509_shift(ctx->untrusted)); #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION ERR_raise(ERR_LIB_CMP, CMP_R_MISSING_PROTECTION); return 0; --- crypto/openssl/crypto/cms/cms_kari.c.orig +++ crypto/openssl/crypto/cms/cms_kari.c @@ -217,6 +217,7 @@ int rv = 0; unsigned char *out = NULL; int outlen; + size_t outsize; keklen = EVP_CIPHER_CTX_get_key_length(kari->ctx); if (keklen > EVP_MAX_KEY_LENGTH) @@ -230,7 +231,13 @@ /* obtain output length of ciphered key */ if (!EVP_CipherUpdate(kari->ctx, NULL, &outlen, in, inlen)) goto err; - out = OPENSSL_malloc(outlen); + /* + * On its integrity-failure paths that primitive writes and cleanses up to + * inlen bytes of the output buffer. Size the buffer for that worst case so + * a failed unwrap cannot write past the allocation. + */ + outsize = (size_t)outlen < inlen ? inlen : (size_t)outlen; + out = OPENSSL_malloc(outsize); if (out == NULL) goto err; if (!EVP_CipherUpdate(kari->ctx, out, &outlen, in, inlen)) --- crypto/openssl/ssl/record/rec_layer_d1.c.orig +++ crypto/openssl/ssl/record/rec_layer_d1.c @@ -70,7 +70,7 @@ while ((item = pqueue_pop(d->unprocessed_rcds.q)) != NULL) { rdata = (DTLS1_RECORD_DATA *)item->data; - OPENSSL_free(rdata->rbuf.buf); + OPENSSL_free(rdata->packet); OPENSSL_free(item->data); pitem_free(item); } @@ -78,8 +78,8 @@ while ((item = pqueue_pop(d->processed_rcds.q)) != NULL) { rdata = (DTLS1_RECORD_DATA *)item->data; if (rl->s->options & SSL_OP_CLEANSE_PLAINTEXT) - OPENSSL_cleanse(rdata->rbuf.buf, rdata->rbuf.len); - OPENSSL_free(rdata->rbuf.buf); + OPENSSL_cleanse(rdata->packet, rdata->packet_length); + OPENSSL_free(rdata->packet); OPENSSL_free(item->data); pitem_free(item); } @@ -87,8 +87,8 @@ while ((item = pqueue_pop(d->buffered_app_data.q)) != NULL) { rdata = (DTLS1_RECORD_DATA *)item->data; if (rl->s->options & SSL_OP_CLEANSE_PLAINTEXT) - OPENSSL_cleanse(rdata->rbuf.buf, rdata->rbuf.len); - OPENSSL_free(rdata->rbuf.buf); + OPENSSL_cleanse(rdata->packet, rdata->packet_length); + OPENSSL_free(rdata->packet); OPENSSL_free(item->data); pitem_free(item); } @@ -130,26 +130,60 @@ rdata = (DTLS1_RECORD_DATA *)item->data; - SSL3_BUFFER_release(&s->rlayer.rbuf); + /* + * The record was read into a read buffer at least this size, so it must fit + * - see the length checks in ssl3_read_n(). Verify it rather than risk + * overrunning the buffer if that ever ceases to hold. + */ + if (!ossl_assert(rdata->packet_length <= s->rlayer.rbuf.len)) { + OPENSSL_free(rdata->packet); + return 0; + } - s->rlayer.packet = rdata->packet; + /* + * rdata->packet is a standalone copy of this record's on-wire bytes (see + * dtls1_buffer_record()). Copy it into the live read buffer so that + * s->rlayer.packet continues to point inside s->rlayer.rbuf.buf, as it + * does for every other record, then free our standalone copy. + */ + memcpy(s->rlayer.rbuf.buf, rdata->packet, rdata->packet_length); + s->rlayer.rbuf.offset = 0; + s->rlayer.rbuf.left = 0; + s->rlayer.packet = s->rlayer.rbuf.buf; s->rlayer.packet_length = rdata->packet_length; - memcpy(&s->rlayer.rbuf, &(rdata->rbuf), sizeof(SSL3_BUFFER)); memcpy(&s->rlayer.rrec, &(rdata->rrec), sizeof(SSL3_RECORD)); + /* + * dtls1_buffer_record() rebased rrec.data/input onto rdata->packet if they + * pointed into this record's own bytes, so translate them again onto the + * record's new location in the read buffer. Anything still pointing + * outside rdata->packet is either a separate allocation (rr->comp) or a + * leftover from a previously processed record that will be overwritten + * before use, so leave it alone. + */ + if (rdata->rrec.data >= rdata->packet + && rdata->rrec.data < rdata->packet + rdata->packet_length) + s->rlayer.rrec[0].data = s->rlayer.packet + (rdata->rrec.data - rdata->packet); + if (rdata->rrec.input >= rdata->packet + && rdata->rrec.input < rdata->packet + rdata->packet_length) + s->rlayer.rrec[0].input = s->rlayer.packet + (rdata->rrec.input - rdata->packet); + + OPENSSL_free(rdata->packet); + /* Set proper sequence number for mac calculation */ - memcpy(&(s->rlayer.read_sequence[2]), &(rdata->packet[5]), 6); + memcpy(&(s->rlayer.read_sequence[2]), &(s->rlayer.packet[5]), 6); return 1; } -int dtls1_buffer_record(SSL *s, record_pqueue *queue, unsigned char *priority) +int dtls1_buffer_record(SSL *s, record_pqueue *queue, unsigned char *priority, + size_t limit) { DTLS1_RECORD_DATA *rdata; pitem *item; /* Limit the size of the queue to prevent DOS attacks */ - if (pqueue_size(queue->q) >= 100) + if (pqueue_size(queue->q) >= limit) return 0; rdata = OPENSSL_malloc(sizeof(*rdata)); @@ -161,11 +195,37 @@ return -1; } - rdata->packet = s->rlayer.packet; + /* + * Take a copy of just this record's own on-wire bytes - the header plus the + * record body, which for an already-processed record holds the plaintext + * decrypted in place - rather than the whole (much larger) read buffer. The + * live s->rlayer.rbuf is left untouched and continues to be used for + * subsequent reads. + */ rdata->packet_length = s->rlayer.packet_length; - memcpy(&(rdata->rbuf), &s->rlayer.rbuf, sizeof(SSL3_BUFFER)); + rdata->packet = OPENSSL_memdup(s->rlayer.packet, s->rlayer.packet_length); + if (rdata->packet == NULL) { + OPENSSL_free(rdata); + pitem_free(item); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE); + return -1; + } memcpy(&(rdata->rrec), &s->rlayer.rrec, sizeof(SSL3_RECORD)); + /* + * The copied rrec.data/input still point into the live read buffer. Rebase + * any that point within this record's own bytes onto our standalone copy, + * so that dtls1_copy_record() can translate them again on retrieval. + * Pointers elsewhere (e.g. into rr->comp, or left over from a previously + * processed record) are not ours to move and are left alone. + */ + if (rdata->rrec.data >= s->rlayer.packet + && rdata->rrec.data < s->rlayer.packet + s->rlayer.packet_length) + rdata->rrec.data = rdata->packet + (rdata->rrec.data - s->rlayer.packet); + if (rdata->rrec.input >= s->rlayer.packet + && rdata->rrec.input < s->rlayer.packet + s->rlayer.packet_length) + rdata->rrec.input = rdata->packet + (rdata->rrec.input - s->rlayer.packet); + item->data = rdata; #ifndef OPENSSL_NO_SCTP @@ -176,22 +236,9 @@ } #endif - s->rlayer.packet = NULL; - s->rlayer.packet_length = 0; - memset(&s->rlayer.rbuf, 0, sizeof(s->rlayer.rbuf)); - memset(&s->rlayer.rrec, 0, sizeof(s->rlayer.rrec)); - - if (!ssl3_setup_buffers(s)) { - /* SSLfatal() already called */ - OPENSSL_free(rdata->rbuf.buf); - OPENSSL_free(rdata); - pitem_free(item); - return -1; - } - if (pqueue_insert(queue->q, item) == NULL) { /* Must be a duplicate so ignore it */ - OPENSSL_free(rdata->rbuf.buf); + OPENSSL_free(rdata->packet); OPENSSL_free(rdata); pitem_free(item); } @@ -202,15 +249,16 @@ int dtls1_retrieve_buffered_record(SSL *s, record_pqueue *queue) { pitem *item; + int ret; item = pqueue_pop(queue->q); if (item) { - dtls1_copy_record(s, item); + ret = dtls1_copy_record(s, item); OPENSSL_free(item->data); pitem_free(item); - return 1; + return ret; } return 0; @@ -255,7 +303,13 @@ /* Process all the records. */ while (pqueue_peek(s->rlayer.d->unprocessed_rcds.q)) { - dtls1_get_unprocessed_record(s); + if (!dtls1_get_unprocessed_record(s)) { + /* + * Should not happen. The record has been dropped, so move on + * to the next one. + */ + continue; + } bitmap = dtls1_get_bitmap(s, rr, &is_next_epoch); if (bitmap == NULL) { /* @@ -292,7 +346,8 @@ } if (dtls1_buffer_record(s, &(s->rlayer.d->processed_rcds), - SSL3_RECORD_get_seq_num(s->rlayer.rrec)) + SSL3_RECORD_get_seq_num(s->rlayer.rrec), + DTLS1_MAX_BUFFERED_RECORDS) < 0) { /* SSLfatal() already called */ return 0; @@ -398,6 +453,7 @@ } #endif + /* On failure the record is simply dropped */ dtls1_copy_record(s, item); OPENSSL_free(item->data); @@ -451,7 +507,7 @@ * data for later processing rather than dropping the connection. */ if (dtls1_buffer_record(s, &(s->rlayer.d->buffered_app_data), - SSL3_RECORD_get_seq_num(rr)) + SSL3_RECORD_get_seq_num(rr), DTLS1_MAX_BUFFERED_RECORDS) < 0) { /* SSLfatal() already called */ return -1; --- crypto/openssl/ssl/record/record.h.orig +++ crypto/openssl/ssl/record/record.h @@ -85,7 +85,6 @@ typedef struct dtls1_record_data_st { unsigned char *packet; size_t packet_length; - SSL3_BUFFER rbuf; SSL3_RECORD rrec; #ifndef OPENSSL_NO_SCTP struct bio_dgram_sctp_rcvinfo recordinfo; --- crypto/openssl/ssl/record/record_local.h.orig +++ crypto/openssl/ssl/record/record_local.h @@ -41,9 +41,18 @@ DTLS1_BITMAP *dtls1_get_bitmap(SSL *s, SSL3_RECORD *rr, unsigned int *is_next_epoch); +/* + * Limits on the number of records dtls1_buffer_record() will hold, to prevent + * DOS attacks. Records arriving early for the next epoch get a tighter limit: + * a legitimate peer only ever has a small burst of those in flight. + */ +#define DTLS1_MAX_UNPROCESSED_RECORDS 16 +#define DTLS1_MAX_BUFFERED_RECORDS 100 + int dtls1_process_buffered_records(SSL *s); int dtls1_retrieve_buffered_record(SSL *s, record_pqueue *queue); -int dtls1_buffer_record(SSL *s, record_pqueue *q, unsigned char *priority); +int dtls1_buffer_record(SSL *s, record_pqueue *q, unsigned char *priority, + size_t limit); void ssl3_record_sequence_update(unsigned char *seq); /* Functions provided by the DTLS1_BITMAP component */ --- crypto/openssl/ssl/record/ssl3_record.c.orig +++ crypto/openssl/ssl/record/ssl3_record.c @@ -1683,8 +1683,11 @@ * after use :-). */ - /* we have pulled in a full packet so zero things */ - RECORD_LAYER_reset_packet_length(&s->rlayer); + /* + * Leave s->rlayer.packet_length alone: ssl3_read_n() starts each new record + * by resetting it, and it must still describe this record's on-wire bytes + * for dtls1_buffer_record() should this record end up being buffered. + */ /* Mark receipt of record. */ dtls1_record_bitmap_update(s, bitmap); @@ -1887,7 +1890,7 @@ if ((SSL_in_init(s) || ossl_statem_get_in_handshake(s))) { if (dtls1_buffer_record(s, &(DTLS_RECORD_LAYER_get_unprocessed_rcds(&s->rlayer)), - rr->seq_num) + rr->seq_num, DTLS1_MAX_UNPROCESSED_RECORDS) < 0) { /* SSLfatal() already called */ return -1; @@ -1930,7 +1933,8 @@ rr->data = s->rlayer.packet + DTLS1_RT_HEADER_LENGTH; if (dtls1_buffer_record(s, &(s->rlayer.d->processed_rcds), - SSL3_RECORD_get_seq_num(s->rlayer.rrec)) + SSL3_RECORD_get_seq_num(s->rlayer.rrec), + DTLS1_MAX_BUFFERED_RECORDS) <= 0) { /* SSLfatal() already called */ return 0; --- crypto/openssl/test/build.info.orig +++ crypto/openssl/test/build.info @@ -564,7 +564,7 @@ IF[{- !$disabled{cmp} -}] PROGRAMS{noinst}=cmp_asn_test cmp_ctx_test cmp_status_test cmp_hdr_test \ cmp_protect_test cmp_msg_test cmp_vfy_test \ - cmp_server_test cmp_client_test + cmp_server_test cmp_client_test cmp_extracerts_dos_test ENDIF SOURCE[cmp_asn_test]=cmp_asn_test.c helpers/cmp_testlib.c @@ -591,6 +591,10 @@ INCLUDE[cmp_msg_test]=.. ../include ../apps/include DEPEND[cmp_msg_test]=../libcrypto.a libtestutil.a + SOURCE[cmp_extracerts_dos_test]=cmp_extracerts_dos_test.c helpers/cmp_testlib.c + INCLUDE[cmp_extracerts_dos_test]=.. ../include ../apps/include + DEPEND[cmp_extracerts_dos_test]=../libcrypto.a libtestutil.a + SOURCE[cmp_vfy_test]=cmp_vfy_test.c helpers/cmp_testlib.c INCLUDE[cmp_vfy_test]=.. ../include ../apps/include DEPEND[cmp_vfy_test]=../libcrypto.a libtestutil.a --- /dev/null +++ crypto/openssl/test/cmp_extracerts_dos_test.c @@ -0,0 +1,348 @@ +/* + * Copyright 2026 The OpenSSL Project Authors. All Rights Reserved. + * + * Licensed under the Apache License 2.0 (the "License"). You may not use + * this file except in compliance with the License. You can obtain a copy + * in the file LICENSE in the source distribution or at + * https://www.openssl.org/source/license.html + */ + +/* + * Regression test for: CMP server unauthenticated memory/CPU DoS via + * cached extraCerts on failed protection checks. + * + * Root cause (crypto/cmp/cmp_vfy.c, ossl_cmp_msg_check_update(), current + * master as of this writing): + * + * res = ossl_x509_add_certs_new(&ctx->untrusted, msg->extraCerts, ...); + * ... + * res = OSSL_CMP_validate_msg(ctx, msg) || (cb...); // may be 0 (rejected) + * + * if (ctx->noCacheExtraCerts) // <-- rollback is + * while (num_added-- > 0) // gated on this + * X509_free(sk_X509_shift(ctx->untrusted)); // flag only, NOT + * // on the + * // validation + * // result (res) + * + * if (!res) { ...; return 0; } // certs from a REJECTED msg are kept + * + * This test exercises ossl_cmp_msg_check_update() directly -- no sockets, + * no HTTP server, no apps/cmp.c -- and asserts on the resulting size of + * ctx->untrusted. It builds a genuinely PBM-protected OSSL_CMP_MSG using + * the project's own internal message-creation function + * (ossl_cmp_genm_new(), same one exercised in test/cmp_msg_test.c) so the + * message is not hand-crafted to "look" rejectable -- it is rejected for a + * real reason (the receiving ctx has no matching secret configured), the + * same way OSSL_CMP_validate_msg() would reject any unauthenticated CMP + * request in the field. + * + * Expected results: + * - BEFORE the fix: untrusted_count_after == untrusted_count_before + N + * (every rejected message's extraCerts persist) + * - AFTER the fix: untrusted_count_after == untrusted_count_before + * (rejected messages leave no residue) + */ + +#include "helpers/cmp_testlib.h" + +#define NUM_REJECTED_REQUESTS 25 /* "attacker" sends this many distinct certs */ + +typedef struct test_fixture { + const char *test_case_name; + OSSL_CMP_CTX *server_ctx; /* long-lived ctx under test, mirrors srv_ctx->ctx */ +} CMP_DOS_TEST_FIXTURE; + +static OSSL_LIB_CTX *libctx = NULL; + +static CMP_DOS_TEST_FIXTURE *set_up(const char *const test_case_name) +{ + CMP_DOS_TEST_FIXTURE *fixture; + + if (!TEST_ptr(fixture = OPENSSL_zalloc(sizeof(*fixture)))) + return NULL; + fixture->test_case_name = test_case_name; + + if (!TEST_ptr(fixture->server_ctx = OSSL_CMP_CTX_new(libctx, NULL))) { + OPENSSL_free(fixture); + return NULL; + } + /* + * Deliberately do NOT call OSSL_CMP_CTX_set1_secretValue() on the + * server ctx. Per OSSL_CMP_validate_msg() (crypto/cmp/cmp_vfy.c): + * case NID_id_PasswordBasedMAC: + * if (ctx->secretValue == NULL) { + * ossl_cmp_info(ctx, "no secret available for verifying.."); + * ERR_raise(ERR_LIB_CMP, CMP_R_ERROR_VALIDATING_PROTECTION); + * return 0; + * } + * so every PBM-protected message this ctx receives is unconditionally + * rejected -- a deterministic, content-independent rejection path that + * models "missing or invalid protection" from the report's repro + * steps, without needing to forge a bad MAC by hand. + * ctx->noCacheExtraCerts is left at its default (0), exactly as in the + * vulnerable deployment ("not setting -no_cache_extracerts"). + */ + return fixture; +} + +static void tear_down(CMP_DOS_TEST_FIXTURE *fixture) +{ + if (fixture == NULL) + return; + OSSL_CMP_CTX_free(fixture->server_ctx); + OPENSSL_free(fixture); +} + +/* Generates a throwaway EC P-256 keypair; cheap, and key strength is + * irrelevant to this test. */ +static EVP_PKEY *generate_throwaway_keypair(void) +{ + EVP_PKEY_CTX *pctx = NULL; + EVP_PKEY *pkey = NULL; + + if (!TEST_ptr(pctx = EVP_PKEY_CTX_new_from_name(libctx, "EC", NULL))) + return NULL; + if (!TEST_int_gt(EVP_PKEY_keygen_init(pctx), 0) + || !TEST_int_gt(EVP_PKEY_CTX_set_group_name(pctx, "P-256"), 0) + || !TEST_int_gt(EVP_PKEY_generate(pctx, &pkey), 0)) + pkey = NULL; + EVP_PKEY_CTX_free(pctx); + return pkey; +} + +/* + * Builds a minimal, self-signed, syntactically valid X509 with a unique + * subject/issuer per index, so X509_ADD_FLAG_NO_DUP cannot collapse it + * with any other generated cert (matching the report's exploitation + * requirement of "unique certificates across requests"). + */ +static X509 *generate_unique_self_signed_cert(EVP_PKEY *pkey, int index) +{ + X509 *cert = NULL; + X509_NAME *name = NULL; + ASN1_INTEGER *serial = NULL; + char cn[64]; + + BIO_snprintf(cn, sizeof(cn), "attacker-cert-%d", index); + + if (!TEST_ptr(cert = X509_new()) + || !TEST_true(X509_set_version(cert, X509_VERSION_3))) + goto err; + + if (!TEST_ptr(serial = ASN1_INTEGER_new()) + || !TEST_true(ASN1_INTEGER_set(serial, 1000L + index)) + || !TEST_true(X509_set_serialNumber(cert, serial))) + goto err; + + if (!TEST_ptr(X509_gmtime_adj(X509_getm_notBefore(cert), 0)) + || !TEST_ptr(X509_gmtime_adj(X509_getm_notAfter(cert), + 60L * 60L * 24L * 365L))) + goto err; + + if (!TEST_true(X509_set_pubkey(cert, pkey))) + goto err; + + if (!TEST_ptr(name = X509_NAME_new()) + || !TEST_true(X509_NAME_add_entry_by_txt(name, "O", MBSTRING_ASC, + (unsigned char *)"cmp-dos-test", + -1, -1, 0)) + || !TEST_true(X509_NAME_add_entry_by_txt(name, "CN", MBSTRING_ASC, + (unsigned char *)cn, + -1, -1, 0)) + || !TEST_true(X509_set_subject_name(cert, name)) + || !TEST_true(X509_set_issuer_name(cert, name))) + goto err; + + if (!TEST_int_gt(X509_sign(cert, pkey, EVP_sha256()), 0)) + goto err; + + X509_NAME_free(name); + ASN1_INTEGER_free(serial); + return cert; + +err: + X509_NAME_free(name); + ASN1_INTEGER_free(serial); + X509_free(cert); + return NULL; +} + +/* + * Builds a real, internally consistent, PBM-protected CMP GenMsg carrying + * exactly one never-before-seen self-signed cert as its sole extraCert. + * Uses a throwaway *client*-side OSSL_CMP_CTX purely to drive message + * creation/protection (ossl_cmp_genm_new() both builds the body and calls + * ossl_cmp_msg_protect() internally, same as in test/cmp_msg_test.c). The + * client ctx's secret is intentionally never shared with the server ctx + * under test, so the message is protected (syntactically well-formed, + * non-empty protection field) but NOT verifiable by the receiver -- this + * is what "missing or invalid protection" means for a real attacker who + * has no credentials, not an empty/garbage protection field. + */ +static OSSL_CMP_MSG *build_rejectable_msg_with_unique_cert(int index) +{ + OSSL_CMP_CTX *client_ctx = NULL; + OSSL_CMP_MSG *msg = NULL; + EVP_PKEY *pkey = NULL; + X509 *fresh_cert = NULL; + STACK_OF(X509) *extra = NULL; + unsigned char ref[16], secret[16]; + + if (!TEST_ptr(client_ctx = OSSL_CMP_CTX_new(libctx, NULL))) + goto err; + + if (!TEST_ptr(pkey = generate_throwaway_keypair()) + || !TEST_ptr(fresh_cert = generate_unique_self_signed_cert(pkey, index))) + goto err; + + if (!TEST_ptr(extra = sk_X509_new_null()) + || !TEST_true(sk_X509_push(extra, fresh_cert))) + goto err; + fresh_cert = NULL; /* ownership now with the stack */ + + if (!TEST_true(OSSL_CMP_CTX_set1_extraCertsOut(client_ctx, extra))) + goto err; + + /* PBM protection with a secret the server ctx will never be given */ + memset(ref, (unsigned char)(0xA0 + (index & 0x0F)), sizeof(ref)); + memset(secret, (unsigned char)(0x50 + (index & 0x0F)), sizeof(secret)); + if (!TEST_true(OSSL_CMP_CTX_set_option(client_ctx, + OSSL_CMP_OPT_UNPROTECTED_SEND, 0)) + || !TEST_true(OSSL_CMP_CTX_set1_referenceValue(client_ctx, ref, + sizeof(ref))) + || !TEST_true(OSSL_CMP_CTX_set1_secretValue(client_ctx, secret, + sizeof(secret)))) + goto err; + + /* GenMsg is the lightest standard body type for this purpose */ + if (!TEST_ptr(msg = ossl_cmp_genm_new(client_ctx))) + goto err; + + sk_X509_pop_free(extra, X509_free); + X509_free(fresh_cert); + EVP_PKEY_free(pkey); + OSSL_CMP_CTX_free(client_ctx); + return msg; + +err: + sk_X509_pop_free(extra, X509_free); + X509_free(fresh_cert); + EVP_PKEY_free(pkey); + OSSL_CMP_CTX_free(client_ctx); + OSSL_CMP_MSG_free(msg); + return NULL; +} + +/* + * Core assertion: N distinct rejected requests must not grow + * server_ctx->untrusted at all. + * + * Before the fix this fails with e.g.: + * ERROR: untrusted count after (25) != count before (0) + */ +static int execute_no_unbounded_growth_test(CMP_DOS_TEST_FIXTURE *fixture) +{ + OSSL_CMP_CTX *server_ctx = fixture->server_ctx; + int count_before, count_after, i; + +#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION + TEST_skip("The unbounded growth test is invalid when FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION is defined\n"); + return 1; +#endif + + count_before = sk_X509_num(OSSL_CMP_CTX_get0_untrusted(server_ctx)); + if (count_before < 0) + count_before = 0; + + for (i = 0; i < NUM_REJECTED_REQUESTS; i++) { + OSSL_CMP_MSG *msg = build_rejectable_msg_with_unique_cert(i); + int check_result; + + if (!TEST_ptr(msg)) + return 0; + + check_result = ossl_cmp_msg_check_update(server_ctx, msg, NULL, 0); + OSSL_CMP_MSG_free(msg); + + if (!TEST_int_eq(check_result, 0)) { + TEST_note("expected request #%d to be rejected (server ctx has" + " no matching PBM secret) but it was accepted -- test" + " setup is wrong, not exercising the rejection path", + i); + return 0; + } + } + + count_after = sk_X509_num(OSSL_CMP_CTX_get0_untrusted(server_ctx)); + if (count_after < 0) + count_after = 0; + + if (!TEST_int_eq(count_after, count_before)) { + TEST_note("server_ctx->untrusted grew from %d to %d after %d" + " rejected requests -- failed-request extraCerts caching" + " bug is present (see ossl_cmp_msg_check_update() in" + " crypto/cmp/cmp_vfy.c)", + count_before, count_after, + NUM_REJECTED_REQUESTS); + return 0; + } + return 1; +} + +/* + * Single-request variant of the same check, useful in isolation since it + * pins down that even ONE rejected request leaves no residue -- ruling out + * X509_ADD_FLAG_NO_DUP coincidentally masking the bug in the N-request test. + */ +static int execute_single_rejected_request_test(CMP_DOS_TEST_FIXTURE *fixture) +{ + OSSL_CMP_CTX *server_ctx = fixture->server_ctx; + OSSL_CMP_MSG *msg = build_rejectable_msg_with_unique_cert(999); + int count_before, count_after; + +#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION + TEST_skip("The cmp reject test is invalid when FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION is defined\n"); + OSSL_CMP_MSG_free(msg); + return 1; +#endif + if (!TEST_ptr(msg)) + return 0; + + count_before = sk_X509_num(OSSL_CMP_CTX_get0_untrusted(server_ctx)); + if (count_before < 0) + count_before = 0; + + if (!TEST_int_eq(ossl_cmp_msg_check_update(server_ctx, msg, NULL, 0), 0)) { + OSSL_CMP_MSG_free(msg); + return 0; + } + OSSL_CMP_MSG_free(msg); + + count_after = sk_X509_num(OSSL_CMP_CTX_get0_untrusted(server_ctx)); + if (count_after < 0) + count_after = 0; + + return TEST_int_eq(count_after, count_before); +} + +static int test_single_rejected_request_leaves_no_residue(void) +{ + SETUP_TEST_FIXTURE(CMP_DOS_TEST_FIXTURE, set_up); + EXECUTE_TEST(execute_single_rejected_request_test, tear_down); + return result; +} + +static int test_no_unbounded_growth_on_rejected_requests(void) +{ + SETUP_TEST_FIXTURE(CMP_DOS_TEST_FIXTURE, set_up); + EXECUTE_TEST(execute_no_unbounded_growth_test, tear_down); + return result; +} + +int setup_tests(void) +{ + ADD_TEST(test_single_rejected_request_leaves_no_residue); + ADD_TEST(test_no_unbounded_growth_on_rejected_requests); + return 1; +} --- crypto/openssl/test/recipes/65-test_cmp_msg.t.orig +++ crypto/openssl/test/recipes/65-test_cmp_msg.t @@ -20,17 +20,22 @@ use lib bldtop_dir('.'); my $no_fips = disabled('fips') || ($ENV{NO_FIPS} // 0); +my $no_ec = disabled('ec'); plan skip_all => "This test is not supported in a no-cmp build" if disabled("cmp"); -plan tests => 2 + ($no_fips ? 0 : 1); #fips test +plan tests => 2 + ($no_fips ? 0 : 1) + ($no_ec ? 0 : 1); #fips test and ec test my @basic_cmd = ("cmp_msg_test", data_file("new.key"), data_file("server.crt"), data_file("pkcs10.der")); +unless ($no_ec) { + ok(run(test(["cmp_extracerts_dos_test"]))); +} + ok(run(test([@basic_cmd, "none"]))); ok(run(test([@basic_cmd, "default", srctop_file("test", "default.cnf")])));