Fix BIO_get_new_index() to return an error when it is exhausted.
authorslontis <shane.lontis@oracle.com>
Mon, 4 Mar 2024 02:08:08 +0000 (13:08 +1100)
committerMatt Caswell <matt@openssl.org>
Mon, 11 Mar 2024 11:34:25 +0000 (11:34 +0000)
Fixes #23655

BIO_get_new_index() returns a range of 129..255.

It is set to BIO_TYPE_START (128) initially and is incremented on each
call.
>= 256 is reserved for the class type flags (BIO_TYPE_DESCRIPTOR) so it
should error if it reaches the upper bound.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23732)

CHANGES.md
crypto/bio/bio_lib.c
crypto/bio/bio_meth.c
doc/man3/BIO_find_type.pod
doc/man3/BIO_meth_new.pod
include/openssl/bio.h.in
test/bio_meth_test.c [new file with mode: 0644]
test/build.info
test/recipes/61-test_bio_meth.t [new file with mode: 0644]

index 322fd521779b7e76ea28e07ce5ec35834763fbaf..ac6b7525bf2551f5d72f1af160dd3e2f7725189a 100644 (file)
@@ -28,6 +28,14 @@ OpenSSL 3.3
 
 ### Changes between 3.2 and 3.3 [xx XXX xxxx]
 
+ * The BIO_get_new_index() function can only be called 127 times before it
+   reaches its upper bound of BIO_TYPE_MASK. It will now correctly return an
+   error of -1 once it is exhausted. Users may need to reserve using this
+   function for cases where BIO_find_type() is required. Either BIO_TYPE_NONE
+   or BIO_get_new_index() can be used to supply a type to BIO_meth_new().
+
+   *Shane Lontis*
+
  * Added API functions SSL_SESSION_get_time_ex(), SSL_SESSION_set_time_ex()
    using time_t which is Y2038 safe on 32 bit systems when 64 bit time
    is enabled (e.g via setting glibc macro _TIME_BITS=64).
index e4f72bcd1b8a140cd85f5cfc0f39d47bd28dc7d9..8bcf666e3c3290ea63815df0f21d95a2b048fcc7 100644 (file)
@@ -817,7 +817,7 @@ BIO *BIO_find_type(BIO *bio, int type)
         ERR_raise(ERR_LIB_BIO, ERR_R_PASSED_NULL_PARAMETER);
         return NULL;
     }
-    mask = type & 0xff;
+    mask = type & BIO_TYPE_MASK;
     do {
         if (bio->method != NULL) {
             mt = bio->method->type;
index 30b1db5aa8d46006f8fbc4d03b35a4fb2e1074b5..5102a54e99f37a54999f97505850e6952d9d462b 100644 (file)
@@ -29,6 +29,8 @@ int BIO_get_new_index(void)
     }
     if (!CRYPTO_UP_REF(&bio_type_count, &newval))
         return -1;
+    if (newval > BIO_TYPE_MASK)
+        return -1;
     return newval;
 }
 
index 99bab8cce85c23eee80e1ff2356286b6ddbaca34..452c29c1bf4118a7698f49040dc07a21972a14ad 100644 (file)
@@ -14,12 +14,12 @@ BIO_find_type, BIO_next, BIO_method_type - BIO chain traversal
 
 =head1 DESCRIPTION
 
-The BIO_find_type() searches for a BIO of a given type in a chain, starting
-at BIO B<b>. If B<type> is a specific type (such as B<BIO_TYPE_MEM>) then a search
-is made for a BIO of that type. If B<type> is a general type (such as
-B<BIO_TYPE_SOURCE_SINK>) then the next matching BIO of the given general type is
-searched for. BIO_find_type() returns the next matching BIO or NULL if none is
-found.
+The BIO_find_type() searches for a B<BIO> of a given type in a chain, starting
+at B<BIO> I<b>. If I<type> is a specific type (such as B<BIO_TYPE_MEM>) then a
+search is made for a B<BIO> of that type. If I<type> is a general type (such as
+B<BIO_TYPE_SOURCE_SINK>) then the next matching B<BIO> of the given general type is
+searched for. BIO_find_type() returns the next matching B<BIO> or NULL if none is
+found. If I<type> is B<BIO_TYPE_NONE> it will not find a match.
 
 The following general types are defined:
 B<BIO_TYPE_DESCRIPTOR>, B<BIO_TYPE_FILTER>, and B<BIO_TYPE_SOURCE_SINK>.
@@ -38,7 +38,7 @@ BIO_find_type() returns a matching BIO or NULL for no match.
 
 BIO_next() returns the next BIO in a chain.
 
-BIO_method_type() returns the type of the BIO B<b>.
+BIO_method_type() returns the type of the BIO I<b>.
 
 =head1 EXAMPLES
 
index 5be14b2a28f7993937a46b707d0502c5c1c27025..589c1b18fc81b3756c107cc4c1c5c7f7e819b1a0 100644 (file)
@@ -82,9 +82,13 @@ The B<BIO_METHOD> type is a structure used for the implementation of new BIO
 types. It provides a set of functions used by OpenSSL for the implementation
 of the various BIO capabilities. See the L<bio(7)> page for more information.
 
-BIO_meth_new() creates a new B<BIO_METHOD> structure. It should be given a
-unique integer B<type> and a string that represents its B<name>.
-Use BIO_get_new_index() to get the value for B<type>.
+BIO_meth_new() creates a new B<BIO_METHOD> structure that contains a type
+identifier I<type> and a string that represents its B<name>.
+B<type> can be set to either B<BIO_TYPE_NONE> or via BIO_get_new_index() if
+a unique type is required for searching (See L<BIO_find_type(3)>)
+
+Note that BIO_get_new_index() can only be used 127 times before it returns an
+error.
 
 The set of
 standard OpenSSL provided BIO types is provided in F<< <openssl/bio.h> >>.
index d1a29bee32887a1ccd6dcafc991346bdb0c81c1b..a118aeeecedb4161f264e16ecd65b8cf74972422 100644 (file)
@@ -71,7 +71,10 @@ extern "C" {
 # define BIO_TYPE_DGRAM_PAIR     (26|BIO_TYPE_SOURCE_SINK)
 # define BIO_TYPE_DGRAM_MEM      (27|BIO_TYPE_SOURCE_SINK)
 
+/* Custom type starting index returned by BIO_get_new_index() */
 #define BIO_TYPE_START           128
+/* Custom type maximum index that can be returned by BIO_get_new_index() */
+#define BIO_TYPE_MASK            0xFF
 
 /*
  * BIO_FILENAME_READ|BIO_CLOSE to open or close on free.
diff --git a/test/bio_meth_test.c b/test/bio_meth_test.c
new file mode 100644 (file)
index 0000000..3eeafe3
--- /dev/null
@@ -0,0 +1,71 @@
+/*
+ * Copyright 2024 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
+ */
+
+#include <openssl/bio.h>
+#include "testutil.h"
+
+static int test_bio_meth(void)
+{
+    int i, ret = 0, id;
+    BIO_METHOD *meth1 = NULL, *meth2 = NULL, *meth3 = NULL;
+    BIO *membio = NULL, *bio1 = NULL, *bio2 = NULL, *bio3 = NULL;
+
+    id = BIO_get_new_index();
+    if (!TEST_int_eq(id, BIO_TYPE_START + 1))
+        goto err;
+
+    if (!TEST_ptr(meth1 = BIO_meth_new(id, "Method1"))
+        || !TEST_ptr(meth2 = BIO_meth_new(BIO_TYPE_NONE, "Method2"))
+        || !TEST_ptr(meth3 = BIO_meth_new(BIO_TYPE_NONE|BIO_TYPE_FILTER, "Method3"))
+        || !TEST_ptr(bio1 = BIO_new(meth1))
+        || !TEST_ptr(bio2 = BIO_new(meth2))
+        || !TEST_ptr(bio3 = BIO_new(meth3))
+        || !TEST_ptr(membio = BIO_new(BIO_s_mem())))
+        goto err;
+
+    BIO_set_next(bio3, bio2);
+    BIO_set_next(bio2, bio1);
+    BIO_set_next(bio1, membio);
+
+    /* Check that we get an error if we exhaust BIO_get_new_index() */
+    for (i = id + 1; i <= BIO_TYPE_MASK; ++i) {
+        if (!TEST_int_eq(BIO_get_new_index(), i))
+            goto err;
+    }
+    if (!TEST_int_eq(BIO_get_new_index(), -1))
+        goto err;
+
+    /* test searching works */
+    if (!TEST_ptr_eq(BIO_find_type(bio3, BIO_TYPE_MEM), membio)
+        || !TEST_ptr_eq(BIO_find_type(bio3, id), bio1))
+        goto err;
+
+    /* Check searching for BIO_TYPE_NONE returns NULL */
+    if (!TEST_ptr_null(BIO_find_type(bio3, BIO_TYPE_NONE)))
+        goto err;
+    /* Check searching for BIO_TYPE_NONE + BIO_TYPE_FILTER works */
+    if (!TEST_ptr_eq(BIO_find_type(bio3, BIO_TYPE_FILTER), bio3))
+        goto err;
+    ret = 1;
+err:
+    BIO_free(membio);
+    BIO_free(bio3);
+    BIO_free(bio2);
+    BIO_free(bio1);
+    BIO_meth_free(meth3);
+    BIO_meth_free(meth2);
+    BIO_meth_free(meth1);
+    return ret;
+}
+
+int setup_tests(void)
+{
+    ADD_TEST(test_bio_meth);
+    return 1;
+}
index 10df828764b96a9c791c39d4bd357238993870a7..969e81c2ccc0ea97b4e232499a92ca17b9143224 100644 (file)
@@ -63,7 +63,7 @@ IF[{- !$disabled{tests} -}]
           provfetchtest prov_config_test rand_test \
           ca_internals_test bio_tfo_test membio_test bio_dgram_test list_test \
           fips_version_test x509_test hpke_test pairwise_fail_test \
-          nodefltctxtest evp_xof_test x509_load_cert_file_test
+          nodefltctxtest evp_xof_test x509_load_cert_file_test bio_meth_test
 
   IF[{- !$disabled{'rpk'} -}]
     PROGRAMS{noinst}=rpktest
@@ -489,6 +489,10 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[bio_memleak_test]=../include ../apps/include
   DEPEND[bio_memleak_test]=../libcrypto libtestutil.a
 
+  SOURCE[bio_meth_test]=bio_meth_test.c
+  INCLUDE[bio_meth_test]=../include ../apps/include
+  DEPEND[bio_meth_test]=../libcrypto libtestutil.a
+
   SOURCE[bioprinttest]=bioprinttest.c
   INCLUDE[bioprinttest]=../include ../apps/include
   DEPEND[bioprinttest]=../libcrypto libtestutil.a
diff --git a/test/recipes/61-test_bio_meth.t b/test/recipes/61-test_bio_meth.t
new file mode 100644 (file)
index 0000000..15207f8
--- /dev/null
@@ -0,0 +1,12 @@
+#! /usr/bin/env perl
+# Copyright 2024 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
+
+use OpenSSL::Test::Simple;
+
+simple_test("test_bio_meth", "bio_meth_test");
+