Commit d7e77b66ca for openssl.org

commit d7e77b66cabb770932091ed5986221e0d1e57144
Author: Marc Gutman <marcg@activestate.com>
Date:   Thu Jul 9 15:23:28 2026 -0500

    Don't raise NOT_ENOUGH_DATA on a clean EOF at an object boundary

    asn1_d2i_read_bio() reads one ASN.1 object at a time from a BIO.  Callers
    commonly loop, decoding concatenated DER values until the call fails, and
    rely on a failure with no queued error to recognise a clean end of input.
    CPython's ssl module does this in _add_ca_certs() when loading the Windows
    certificate store via SSLContext.load_verify_locations(cadata=...); it
    re-raises any leftover ASN.1 error other than ASN1_R_HEADER_TOO_LONG as
    fatal.

    Commit 9eb6922c59 ("asn1: raise NOT_ENOUGH_DATA on header EOF") changed the
    BIO_read() check from "i < 0" to "i <= 0", so a clean EOF (BIO_read()
    returning 0, as an exhausted BIO_new_mem_buf does) on an object boundary now
    raises ASN1_R_NOT_ENOUGH_DATA instead of failing with an empty error queue.
    The rewrite in commit 35852da1d9 carried this behaviour forward.  As a
    result Python 3 on Windows fails to initialise an SSLContext with:

        ssl.SSLError: [ASN1: NOT_ENOUGH_DATA] not enough data

    Raise ASN1_R_NOT_ENOUGH_DATA only on an actual read error, on an EOF in the
    middle of an object (some bytes already buffered), or on an EOF while still
    inside an indefinite-length value awaiting its end-of-contents octets - all
    of which are genuine truncation.  A clean EOF at a top-level object boundary
    again fails without queuing an error, restoring the long-standing behaviour
    that looping callers depend on.

    Add regression tests covering the clean-EOF, truncated, indefinite-length
    truncation and partial-header cases, and document the read behaviour in
    ASN1_item_d2i_bio(3).

    Fixes #31807

    Assisted-by: Claude:claude-opus-4-8

    Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
    Reviewed-by: Igor Ustinov <igus@openssl.foundation>
    Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
    MergeDate: Mon Jul 13 08:05:03 2026
    (Merged from https://github.com/openssl/openssl/pull/31818)

diff --git a/crypto/asn1/a_d2i_fp.c b/crypto/asn1/a_d2i_fp.c
index 8f9e267689..41491b92a1 100644
--- a/crypto/asn1/a_d2i_fp.c
+++ b/crypto/asn1/a_d2i_fp.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1995-2025 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-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
@@ -141,7 +141,20 @@ int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
             i = BIO_read(in, &(b->data[len]), (int)want);

             if (i <= 0) {
-                ERR_raise(ERR_LIB_ASN1, ASN1_R_NOT_ENOUGH_DATA);
+                /*
+                 * A read error (i < 0), an EOF in the middle of an object
+                 * (diff != 0, some bytes already buffered), or an EOF while
+                 * still inside an indefinite-length constructed value awaiting
+                 * its end-of-contents octets (eos != 0) all mean the input is
+                 * truncated.  Only a clean EOF at a top-level object boundary
+                 * (i == 0, diff == 0, eos == 0) is the normal end of input:
+                 * fail without queuing an error so that callers looping over
+                 * concatenated DER values (e.g. the libcrypto d2i_*_bio()
+                 * consumers in CPython's ssl module) terminate cleanly instead
+                 * of seeing a spurious ASN1_R_NOT_ENOUGH_DATA.
+                 */
+                if (i < 0 || diff != 0 || eos != 0)
+                    ERR_raise(ERR_LIB_ASN1, ASN1_R_NOT_ENOUGH_DATA);
                 goto err;
             }

diff --git a/doc/man3/ASN1_item_d2i_bio.pod b/doc/man3/ASN1_item_d2i_bio.pod
index f8e4678367..9b3f389a23 100644
--- a/doc/man3/ASN1_item_d2i_bio.pod
+++ b/doc/man3/ASN1_item_d2i_bio.pod
@@ -59,6 +59,16 @@ B<OSSL_LIB_CTX> provided in the I<libctx> parameter and the property query
 string in I<propq>. See L<crypto(7)/ALGORITHM FETCHING> for more information
 about algorithm fetching.

+When reading from I<in>, decoding consumes one complete DER-encoded structure
+and leaves any following bytes in the BIO, so concatenated structures can be
+read with successive calls. Reaching the end of the input cleanly, at a
+structure boundary, is not treated as an error: the function returns NULL
+without adding to the error queue. If the end of the input is reached in the
+middle of a structure, or an indefinite-length value is missing its
+end-of-contents octets (that is, the input is truncated), an error is queued
+with reason code B<ASN1_R_NOT_ENOUGH_DATA>. The same applies to
+ASN1_item_d2i_fp_ex().
+
 ASN1_item_d2i_bio() is the same as ASN1_item_d2i_bio_ex() except that the
 default B<OSSL_LIB_CTX> is used (i.e. NULL) and with a NULL property query
 string.
@@ -92,6 +102,12 @@ that the I<libctx> and I<propq> can be used when doing algorithm fetching.
 ASN1_item_d2i_bio(), ASN1_item_unpack_ex() and ASN1_item_unpack() return a pointer to
 an B<ASN1_VALUE> or NULL on error.

+The ASN1_item_d2i_bio() and ASN1_item_d2i_fp() functions, including their
+B<_ex> variants, also return NULL at a clean end of input. In that case the
+error queue is left unchanged, so a caller reading concatenated structures in
+a loop can distinguish a clean end of input from a decoding error by
+inspecting the error queue, for example with L<ERR_peek_error(3)>.
+
 ASN1_item_i2d_mem_bio() returns a pointer to a memory BIO or NULL on error.

 ASN1_item_pack() returns a pointer to an B<ASN1_STRING> or NULL on error.
@@ -105,7 +121,7 @@ The function ASN1_item_unpack_ex() was added in OpenSSL 3.2.

 =head1 COPYRIGHT

-Copyright 2021-2023 The OpenSSL Project Authors. All Rights Reserved.
+Copyright 2021-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
diff --git a/test/asn1_decode_test.c b/test/asn1_decode_test.c
index d7377c8ef7..8a9629c21d 100644
--- a/test/asn1_decode_test.c
+++ b/test/asn1_decode_test.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2017-2024 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2017-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
@@ -14,7 +14,11 @@
 #include <openssl/asn1.h>
 #include <openssl/asn1t.h>
 #include <openssl/obj_mac.h>
+#include <openssl/bio.h>
+#include <openssl/buffer.h>
+#include <openssl/err.h>
 #include "internal/numbers.h"
+#include "internal/asn1.h"
 #include "testutil.h"

 #ifdef __GNUC__
@@ -266,6 +270,163 @@ err:
     return ret;
 }

+/*
+ * A minimal, complete DER object: SEQUENCE { INTEGER 0 }.
+ * asn1_d2i_read_bio() should consume exactly these bytes.
+ */
+static const unsigned char one_obj[] = {
+    0x30, 0x03, /* SEQUENCE, length 3 */
+    0x02, 0x01, 0x00 /*   INTEGER 0        */
+};
+
+/*
+ * Reading concatenated DER objects from a BIO must stop cleanly at EOF:
+ * once the input is exhausted on an object boundary, asn1_d2i_read_bio()
+ * returns < 0 and must NOT leave an error on the queue.  Callers that loop
+ * over concatenated values (e.g. CPython's ssl module loading the Windows
+ * certificate store via d2i_X509_bio()) rely on this to detect end-of-input;
+ * a spurious ASN1_R_NOT_ENOUGH_DATA there is reported as a fatal error.
+ */
+static int test_d2i_read_bio_clean_eof(void)
+{
+    unsigned char two_objs[sizeof(one_obj) * 2];
+    BIO *bio = NULL;
+    BUF_MEM *buf = NULL;
+    int ret = 0;
+
+    memcpy(two_objs, one_obj, sizeof(one_obj));
+    memcpy(two_objs + sizeof(one_obj), one_obj, sizeof(one_obj));
+
+    if (!TEST_ptr(bio = BIO_new_mem_buf(two_objs, sizeof(two_objs))))
+        goto err;
+    ERR_clear_error();
+
+    /* Both complete objects are read, one per call. */
+    if (!TEST_int_eq(asn1_d2i_read_bio(bio, &buf), (int)sizeof(one_obj)))
+        goto err;
+    BUF_MEM_free(buf);
+    buf = NULL;
+    if (!TEST_int_eq(asn1_d2i_read_bio(bio, &buf), (int)sizeof(one_obj)))
+        goto err;
+    BUF_MEM_free(buf);
+    buf = NULL;
+
+    /* Clean EOF: failure return, but no error must be queued. */
+    if (!TEST_int_lt(asn1_d2i_read_bio(bio, &buf), 0))
+        goto err;
+    if (!TEST_ulong_eq(ERR_peek_error(), 0))
+        goto err;
+
+    ret = 1;
+err:
+    BUF_MEM_free(buf);
+    BIO_free(bio);
+    return ret;
+}
+
+/*
+ * In contrast, hitting EOF in the middle of an object is genuine truncation
+ * and must still be reported as ASN1_R_NOT_ENOUGH_DATA.
+ */
+static int test_d2i_read_bio_truncated(void)
+{
+    static const unsigned char truncated[] = {
+        0x30, 0x05, /* SEQUENCE claims 5 content bytes ... */
+        0x02, 0x01 /* ... but only 2 are present         */
+    };
+    BIO *bio = NULL;
+    BUF_MEM *buf = NULL;
+    unsigned long e;
+    int ret = 0;
+
+    if (!TEST_ptr(bio = BIO_new_mem_buf(truncated, sizeof(truncated))))
+        goto err;
+    ERR_clear_error();
+
+    if (!TEST_int_lt(asn1_d2i_read_bio(bio, &buf), 0))
+        goto err;
+    e = ERR_peek_last_error();
+    if (!TEST_int_eq(ERR_GET_LIB(e), ERR_LIB_ASN1)
+        || !TEST_int_eq(ERR_GET_REASON(e), ASN1_R_NOT_ENOUGH_DATA))
+        goto err;
+
+    ret = 1;
+err:
+    BUF_MEM_free(buf);
+    BIO_free(bio);
+    return ret;
+}
+
+/*
+ * An EOF reached while still inside an indefinite-length constructed value,
+ * before its end-of-contents octets, is truncation too (not a clean boundary),
+ * so it must also report ASN1_R_NOT_ENOUGH_DATA rather than an empty queue.
+ */
+static int test_d2i_read_bio_indefinite_truncated(void)
+{
+    /* SEQUENCE (indefinite) { INTEGER 0 } with the 00 00 EOC missing */
+    static const unsigned char truncated_indefinite[] = {
+        0x30, 0x80, /* SEQUENCE, indefinite length */
+        0x02, 0x01, 0x00 /* INTEGER 0; no end-of-contents octets follow */
+    };
+    BIO *bio = NULL;
+    BUF_MEM *buf = NULL;
+    unsigned long e;
+    int ret = 0;
+
+    bio = BIO_new_mem_buf(truncated_indefinite, sizeof(truncated_indefinite));
+    if (!TEST_ptr(bio))
+        goto err;
+    ERR_clear_error();
+
+    if (!TEST_int_lt(asn1_d2i_read_bio(bio, &buf), 0))
+        goto err;
+    e = ERR_peek_last_error();
+    if (!TEST_int_eq(ERR_GET_LIB(e), ERR_LIB_ASN1)
+        || !TEST_int_eq(ERR_GET_REASON(e), ASN1_R_NOT_ENOUGH_DATA))
+        goto err;
+
+    ret = 1;
+err:
+    BUF_MEM_free(buf);
+    BIO_free(bio);
+    return ret;
+}
+
+/*
+ * An EOF reached part-way through an object's header, with some header bytes
+ * already buffered, is truncation as well.  This exercises the "diff != 0" arm
+ * of the header-read check (distinct from the body read handled elsewhere).
+ */
+static int test_d2i_read_bio_partial_header(void)
+{
+    /* SEQUENCE with a 2-byte long-form length, but only one length byte given */
+    static const unsigned char partial_header[] = {
+        0x30, 0x82, 0x01 /* SEQUENCE, length declared as 2 bytes, 1 present */
+    };
+    BIO *bio = NULL;
+    BUF_MEM *buf = NULL;
+    unsigned long e;
+    int ret = 0;
+
+    if (!TEST_ptr(bio = BIO_new_mem_buf(partial_header, sizeof(partial_header))))
+        goto err;
+    ERR_clear_error();
+
+    if (!TEST_int_lt(asn1_d2i_read_bio(bio, &buf), 0))
+        goto err;
+    e = ERR_peek_last_error();
+    if (!TEST_int_eq(ERR_GET_LIB(e), ERR_LIB_ASN1)
+        || !TEST_int_eq(ERR_GET_REASON(e), ASN1_R_NOT_ENOUGH_DATA))
+        goto err;
+
+    ret = 1;
+err:
+    BUF_MEM_free(buf);
+    BIO_free(bio);
+    return ret;
+}
+
 int setup_tests(void)
 {
 #ifndef OPENSSL_NO_DEPRECATED_3_0
@@ -279,5 +440,9 @@ int setup_tests(void)
     ADD_TEST(test_utctime);
     ADD_TEST(test_invalid_template);
     ADD_TEST(test_reuse_asn1_object);
+    ADD_TEST(test_d2i_read_bio_clean_eof);
+    ADD_TEST(test_d2i_read_bio_truncated);
+    ADD_TEST(test_d2i_read_bio_indefinite_truncated);
+    ADD_TEST(test_d2i_read_bio_partial_header);
     return 1;
 }