rpms/openssl
8
1
Fork 1
Code Issues Pull Requests Releases Activity
a10s
openssl/0058-asn1_d2i_read_bio_EOF.patch
Pavol Žáčik d8bc2379c7
asn1: Do not raise NOT_ENOUGH_DATA at object boundary 
Resolves: RHEL-192992
2026-07-07 21:36:00 +02:00

305 lines
10 KiB
Diff
Raw Permalink Blame History

From 836406633204bccb6d7a8718e15de8c3c2213a8e Mon Sep 17 00:00:00 2001
From: icanhasmath <marcg@activestate.com>
Date: Tue, 30 Jun 2026 16:55:40 -0500
Subject: [PATCH 1/2] 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, or when EOF is
reached in the middle of an object (diff != 0), which is genuine truncation.
A clean EOF on an object boundary again fails without queuing an error,
restoring the long-standing behaviour that looping callers depend on.
Add regression tests for both the clean-EOF and truncated cases, and a
CHANGES.md entry.
Fixes #31807
Assisted-by: Claude:claude-opus-4-8
---
crypto/asn1/a_d2i_fp.c | 15 ++++-
test/asn1_decode_test.c | 130 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 144 insertions(+), 1 deletion(-)
diff --git a/crypto/asn1/a_d2i_fp.c b/crypto/asn1/a_d2i_fp.c
index a23dea8ebd..19595683ef 100644
--- a/crypto/asn1/a_d2i_fp.c
+++ b/crypto/asn1/a_d2i_fp.c
@@ -139,7 +139,20 @@ int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
}
i = BIO_read(in, &(b->data[len]), 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;
}
if (i > 0) {
diff --git a/test/asn1_decode_test.c b/test/asn1_decode_test.c
index 6e00d40896..ec20d6ebbc 100644
--- a/test/asn1_decode_test.c
+++ b/test/asn1_decode_test.c
@@ -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,129 @@ 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;
+
+ if (!TEST_ptr(bio = BIO_new_mem_buf(truncated_indefinite,
+ sizeof(truncated_indefinite))))
+ 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 +406,8 @@ 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);
return 1;
}
--
2.54.0
From a7272fcb339ec8e11578a23fcd667167b6dbab49 Mon Sep 17 00:00:00 2001
From: icanhasmath <marcg@activestate.com>
Date: Fri, 3 Jul 2026 09:32:10 -0500
Subject: [PATCH 2/2] test: cover the partial-header EOF case in
asn1_d2i_read_bio
The existing asn1_d2i_read_bio tests cover a clean EOF at an object
boundary, an EOF in the middle of an object body, and an EOF inside an
unterminated indefinite-length value. None exercised an EOF reached
part-way through an object header, with header bytes already buffered,
which is the "diff != 0" arm of the header-read truncation check.
Add test_d2i_read_bio_partial_header feeding a SEQUENCE whose 2-byte
long-form length is truncated after the first byte, and assert it reports
ASN1_R_NOT_ENOUGH_DATA.
Assisted-by: Claude:claude-opus-4-8
---
test/asn1_decode_test.c | 39 +++++++++++++++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/test/asn1_decode_test.c b/test/asn1_decode_test.c
index ec20d6ebbc..8a9629c21d 100644
--- a/test/asn1_decode_test.c
+++ b/test/asn1_decode_test.c
@@ -374,8 +374,42 @@ static int test_d2i_read_bio_indefinite_truncated(void)
unsigned long e;
int ret = 0;
- if (!TEST_ptr(bio = BIO_new_mem_buf(truncated_indefinite,
- sizeof(truncated_indefinite))))
+ 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();
@@ -409,5 +443,6 @@ int setup_tests(void)
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;
}
--
2.54.0
Reference in New Issue View Git Blame Copy Permalink