Fix false success on zero BIO write

The TLS record layer treated BIO_write returning 0 as success unless
the BIO retry flag was set. For a write with data pending in the write
buffer, this could report success to the application while leaving record
data pending internally.

Capture the pending buffer length before the transport write. If a
positive length write returns zero, return retry when the BIO retry flag
is set and fatal otherwise. Do not queue an SSL reason for the fatal
zero-without-retry case: a custom BIO can return zero without retry
without this implying an SSL library or protocol error, so SSL_get_error()
reports SSL_ERROR_SYSCALL with an empty error queue.

Preserve the KTLS empty fragment case where the pending length is already
zero and a zero byte write is expected to succeed.

For DTLS, the same zero return without retry condition is now treated as
fatal; the failed buffer is still dropped in the existing DTLS write error
path. This intentionally tightens the previous behavior, which could treat
the zero return as success.

Add regression coverage with a custom BIO that returns 0 without setting
retry for a positive length write.

Fixes #31009

Reviewed-by: Jakub Zelenka <jakub.zelenka@openssl.foundation>
Reviewed-by: Matt Caswell <matt@openssl.foundation>
Reviewed-by: Andrew Dinh <andrewd@openssl.org>
MergeDate: Mon Aug  3 12:37:56 2026
(Merged from https://github.com/openssl/openssl/pull/31021)

diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
index 0363bf23bf2b0ff3dce459ad44da2661db6aa391..36304496e836c27a0f0023ea465d3a6ba06254c8 100644 (file)
--- a/ssl/record/methods/tls_common.c
+++ b/ssl/record/methods/tls_common.c
@@ -1849,13 +1849,14 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
 {
     int i, ret;
     TLS_BUFFER *thiswb;
-    size_t tmpwrit = 0;
+    size_t tmpwrit = 0, left;
 
     if (rl->nextwbuf >= rl->numwpipes)
         return OSSL_RECORD_RETURN_SUCCESS;
 
     for (;;) {
         thiswb = &rl->wbuf[rl->nextwbuf];
+        left = TLS_BUFFER_get_left(thiswb);
 
         clear_sys_error();
         if (rl->bio != NULL) {
@@ -1865,13 +1866,24 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
                     return ret;
             }
             i = BIO_write(rl->bio, (char *)&(TLS_BUFFER_get_buf(thiswb)[TLS_BUFFER_get_offset(thiswb)]),
-                (unsigned int)TLS_BUFFER_get_left(thiswb));
+                (unsigned int)left);
             if (i >= 0) {
                 tmpwrit = i;
-                if (i == 0 && BIO_should_retry(rl->bio))
-                    ret = OSSL_RECORD_RETURN_RETRY;
-                else
+                if (i == 0 && left != 0) {
+                    if (BIO_should_retry(rl->bio)) {
+                        ret = OSSL_RECORD_RETURN_RETRY;
+                    } else {
+                        /*
+                         * Treat this as a fatal I/O condition. Do not queue an
+                         * SSL reason: a zero return with no retry flag may come
+                         * from a custom BIO and does not imply an SSL library
+                         * or protocol error.
+                         */
+                        ret = OSSL_RECORD_RETURN_FATAL;
+                    }
+                } else {
                     ret = OSSL_RECORD_RETURN_SUCCESS;
+                }
             } else {
                 if (BIO_should_retry(rl->bio)) {
                     ret = OSSL_RECORD_RETURN_RETRY;
@@ -1894,7 +1906,7 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
          * Treat i == 0 as success rather than an error for zero byte
          * writes to permit this case.
          */
-        if (i >= 0 && tmpwrit == TLS_BUFFER_get_left(thiswb)) {
+        if (i >= 0 && tmpwrit == left) {
             TLS_BUFFER_set_left(thiswb, 0);
             TLS_BUFFER_add_offset(thiswb, tmpwrit);
             if (++(rl->nextwbuf) < rl->numwpipes)

diff --git a/test/helpers/ssltestlib.c b/test/helpers/ssltestlib.c
index 57b8c3e0403cb5e4c738010c112e39c2b255f4d6..0d056d4ea883fa1ac2d1a52ff3a9929c7bc84529 100644 (file)
--- a/test/helpers/ssltestlib.c
+++ b/test/helpers/ssltestlib.c
@@ -34,11 +34,13 @@ static int tls_dump_puts(BIO *bp, const char *str);
 #define BIO_TYPE_MEMPACKET_TEST 0x81
 #define BIO_TYPE_ALWAYS_RETRY 0x82
 #define BIO_TYPE_MAYBE_RETRY (0x83 | BIO_TYPE_FILTER)
+#define BIO_TYPE_NO_RETRY_ZERO (0x84 | BIO_TYPE_FILTER)
 
 static BIO_METHOD *method_tls_dump = NULL;
 static BIO_METHOD *meth_mem = NULL;
 static BIO_METHOD *meth_always_retry = NULL;
 static BIO_METHOD *meth_maybe_retry = NULL;
+static BIO_METHOD *meth_no_retry_zero = NULL;
 static int retry_err = -1;
 
 /* Note: Not thread safe! */
@@ -1011,6 +1013,10 @@ static int maybe_retry_new(BIO *bi);
 static int maybe_retry_free(BIO *a);
 static int maybe_retry_write(BIO *b, const char *in, int inl);
 static long maybe_retry_ctrl(BIO *b, int cmd, long num, void *ptr);
+static int no_retry_zero_new(BIO *bi);
+static int no_retry_zero_free(BIO *a);
+static int no_retry_zero_write(BIO *b, const char *in, int inl);
+static long no_retry_zero_ctrl(BIO *b, int cmd, long num, void *ptr);
 
 const BIO_METHOD *bio_s_maybe_retry(void)
 {
@@ -1097,6 +1103,61 @@ static long maybe_retry_ctrl(BIO *bio, int cmd, long num, void *ptr)
     }
 }
 
+const BIO_METHOD *bio_s_no_retry_zero(void)
+{
+    if (meth_no_retry_zero == NULL) {
+        if (!TEST_ptr(meth_no_retry_zero = BIO_meth_new(BIO_TYPE_NO_RETRY_ZERO,
+                          "No Retry Zero"))
+            || !TEST_true(BIO_meth_set_write(meth_no_retry_zero,
+                no_retry_zero_write))
+            || !TEST_true(BIO_meth_set_ctrl(meth_no_retry_zero,
+                no_retry_zero_ctrl))
+            || !TEST_true(BIO_meth_set_create(meth_no_retry_zero,
+                no_retry_zero_new))
+            || !TEST_true(BIO_meth_set_destroy(meth_no_retry_zero,
+                no_retry_zero_free)))
+            return NULL;
+    }
+    return meth_no_retry_zero;
+}
+
+void bio_s_no_retry_zero_free(void)
+{
+    BIO_meth_free(meth_no_retry_zero);
+}
+
+static int no_retry_zero_new(BIO *bio)
+{
+    BIO_set_init(bio, 1);
+    return 1;
+}
+
+static int no_retry_zero_free(BIO *bio)
+{
+    BIO_set_data(bio, NULL);
+    BIO_set_init(bio, 0);
+    return 1;
+}
+
+static int no_retry_zero_write(BIO *bio, const char *in, int inl)
+{
+    BIO_clear_retry_flags(bio);
+    return 0;
+}
+
+static long no_retry_zero_ctrl(BIO *bio, int cmd, long num, void *ptr)
+{
+    BIO *next = BIO_next(bio);
+
+    switch (cmd) {
+    case BIO_CTRL_FLUSH:
+        return next == NULL ? 1 : BIO_ctrl(next, cmd, num, ptr);
+
+    default:
+        return next == NULL ? 0 : BIO_ctrl(next, cmd, num, ptr);
+    }
+}
+
 int create_ssl_ctx_pair(OSSL_LIB_CTX *libctx, const SSL_METHOD *sm,
     const SSL_METHOD *cm, int min_proto_version,
     int max_proto_version, SSL_CTX **sctx, SSL_CTX **cctx,

diff --git a/test/helpers/ssltestlib.h b/test/helpers/ssltestlib.h
index 68a10b24bd1aa660b69421a5e01f82a9e62646e0..37063b5018caa8554738e27d1c63dd968f1f20da 100644 (file)
--- a/test/helpers/ssltestlib.h
+++ b/test/helpers/ssltestlib.h
@@ -59,6 +59,9 @@ void set_always_retry_err_val(int err);
 const BIO_METHOD *bio_s_maybe_retry(void);
 void bio_s_maybe_retry_free(void);
 
+const BIO_METHOD *bio_s_no_retry_zero(void);
+void bio_s_no_retry_zero_free(void);
+
 /* Packet types - value 0 is reserved */
 #define INJECT_PACKET 1
 #define INJECT_PACKET_IGNORE_REC_SEQ 2

diff --git a/test/sslapitest.c b/test/sslapitest.c
index 6fc18a0dc04de461a0acc7f612cf211a3f889548..1c7620165401fa098fc9e2115bf11ef4bdaa0bec 100644 (file)
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -13556,6 +13556,70 @@ end:
     return testresult;
 }
 
+/*
+ * Test that a BIO returning 0 without a retry flag for a write with a positive
+ * length is not treated as a successful write.
+ */
+static int test_data_write_zero_no_retry(int tst)
+{
+    SSL_CTX *cctx = NULL, *sctx = NULL;
+    SSL *clientssl = NULL, *serverssl = NULL;
+    BIO *bzero = BIO_new(bio_s_no_retry_zero());
+    const SSL_METHOD *smeth = TLS_server_method();
+    const SSL_METHOD *cmeth = TLS_client_method();
+    unsigned char inbuf[1] = { 0 };
+    size_t written;
+    unsigned long errcode;
+    int err, min_version = 0, max_version = 0, testresult = 0;
+
+    if (tst == 1) {
+#if !defined(OPENSSL_NO_DTLS) && !defined(OPENSSL_NO_DTLS1_2)
+        smeth = DTLS_server_method();
+        cmeth = DTLS_client_method();
+        min_version = max_version = DTLS1_2_VERSION;
+#else
+        BIO_free(bzero);
+        return TEST_skip("DTLS 1.2 not supported");
+#endif
+    }
+
+    if (!TEST_ptr(bzero))
+        goto end;
+
+    if (!TEST_true(create_ssl_ctx_pair(libctx, smeth, cmeth, min_version,
+            max_version, &sctx, &cctx, cert, privkey)))
+        goto end;
+
+    if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, NULL,
+            NULL)))
+        goto end;
+
+    if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE)))
+        goto end;
+
+    SSL_set0_wbio(clientssl, bzero);
+    bzero = NULL;
+
+    ERR_clear_error();
+    if (!TEST_false(SSL_write_ex(clientssl, inbuf, sizeof(inbuf), &written)))
+        goto end;
+
+    err = SSL_get_error(clientssl, 0);
+    errcode = ERR_get_error();
+    if (!TEST_int_eq(err, SSL_ERROR_SYSCALL)
+        || !TEST_ulong_eq(errcode, 0))
+        goto end;
+
+    testresult = 1;
+end:
+    SSL_free(serverssl);
+    SSL_free(clientssl);
+    SSL_CTX_free(sctx);
+    SSL_CTX_free(cctx);
+    BIO_free_all(bzero);
+    return testresult;
+}
+
 struct resume_servername_cb_data {
     int i;
     SSL_CTX *cctx;
@@ -15598,6 +15662,7 @@ int setup_tests(void)
     ADD_TEST(test_rstate_string);
     ADD_ALL_TESTS(test_handshake_retry, 16);
     ADD_TEST(test_data_retry);
+    ADD_ALL_TESTS(test_data_write_zero_no_retry, 2);
     ADD_ALL_TESTS(test_multi_resume, 5);
     ADD_ALL_TESTS(test_select_next_proto, OSSL_NELEM(next_proto_tests));
 #if !defined(OPENSSL_NO_TLS1_2) && !defined(OPENSSL_NO_NEXTPROTONEG)
@@ -15649,6 +15714,7 @@ void cleanup_tests(void)
     bio_s_mempacket_test_free();
     bio_s_always_retry_free();
     bio_s_maybe_retry_free();
+    bio_s_no_retry_zero_free();
     OSSL_PROVIDER_unload(defctxnull);
     OSSL_LIB_CTX_free(libctx);
 }