Revert BIO_push(3) cycle prevention (bio_lib.c rev. 1.42).

jsing@ worries that cycle prevention might increase risk because
software that is not checking return values (and indeed, not checking
is likely common in practice) might silently behave incorrectly
with cycle prevention whereas without, it will likely either crash
right away through infinite recursion or at least hang in an infinite
loop when trying to use the cyclic chain, in both cases making it
likely that the bug will be found and fixed.

Besides, tb@ points out that BIO_set_next(3) ought to behave as
similarly as possible to BIO_push(3), but adding cycle prevention
to BIO_set_next(3) would be even less convincing because that
function does not provide a return value, encouraging users to
expect that it will always succeed.  While a safe idiom for checking
the success of BIO_set_next(3) could easily be designed, let's be
realistic: application software would be highly unlikely to pick up
such an idiom.

diff --git a/lib/libcrypto/bio/bio_lib.c b/lib/libcrypto/bio/bio_lib.c
index 3eb0869..31b1e73 100644 (file)
--- a/lib/libcrypto/bio/bio_lib.c
+++ b/lib/libcrypto/bio/bio_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: bio_lib.c,v 1.42 2022/12/07 23:08:47 schwarze Exp $ */
+/* $OpenBSD: bio_lib.c,v 1.43 2022/12/16 13:41:55 schwarze Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -637,12 +637,6 @@ BIO_push(BIO *b, BIO *bio)
 
        if (b == NULL)
                return (bio);
-
-       /* If this would create a cycle, change nothing and fail. */
-       for (lb = bio; lb != NULL; lb = lb->next_bio)
-               if (lb == b)
-                       return NULL;
-
        lb = b;
        while (lb->next_bio != NULL)
                lb = lb->next_bio;

diff --git a/lib/libcrypto/man/BIO_push.3 b/lib/libcrypto/man/BIO_push.3
index 35c0c5b..413f824 100644 (file)
--- a/lib/libcrypto/man/BIO_push.3
+++ b/lib/libcrypto/man/BIO_push.3
@@ -1,4 +1,4 @@
-.\" $OpenBSD: BIO_push.3,v 1.12 2022/12/08 11:33:58 tb Exp $
+.\" $OpenBSD: BIO_push.3,v 1.13 2022/12/16 13:41:55 schwarze Exp $
 .\" full merge up to:
 .\" OpenSSL doc/man3/BIO_push.pod 791bfd91 Nov 19 20:38:27 2021 +0100
 .\" OpenSSL doc/man7/bio.pod 1cb7eff4 Sep 10 13:56:40 2019 +0100
@@ -67,7 +67,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
 .\" OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: December 8 2022 $
+.Dd $Mdocdate: December 16 2022 $
 .Dt BIO_PUSH 3
 .Os
 .Sh NAME
@@ -141,11 +141,6 @@ For portability, it is best to make sure that
 is at the beginning of its chain before calling
 .Fn BIO_push .
 .Pp
-The LibreSSL implementation of
-.Fn BIO_push
-never creates cycles.
-If a call would result in a cycle, nothing is changed and the call fails.
-.Pp
 .Fn BIO_pop
 removes the BIO
 .Fa b
@@ -214,16 +209,11 @@ have any effect is
 .Fn BIO_push
 returns
 .Fa b
-for success or a different pointer for failure.
-In particular, it fails and returns
-.Fa new_tail
-if
-.Fa b
-is
-.Dv NULL .
-In LibreSSL, it fails and returns
+if it is not
 .Dv NULL
-if appending would create a cycle.
+or
+.Fa new_tail
+if it is.
 .Pp
 .Fn BIO_pop
 returns the BIO that followed
@@ -294,22 +284,6 @@ and the new chain will be
 data can be written to
 .Sy md1
 as before.
-.Pp
-Even though LibreSSL handles some of the edge cases gracefully,
-the following idiom is recommended for portable error checking:
-.Bd -literal -offset indent
-if (b == NULL || new_tail == NULL || b == new_tail)
-       /* Report the problem and bail out. */
-if (BIO_push(b, new_tail) != b)
-       /* Report that nothing was changed
-        * because it would have created a cycle. */
-.Ed
-.Pp
-Even with the portable idiom, old and non-LibreSSL library implementations
-may silently attempt to create cycles instead of rejecting them and returning
-.Dv NULL ,
-which may result in infinite loops, infinite recursion, or segmentation
-faults either right away or later on.
 .Sh SEE ALSO
 .Xr BIO_find_type 3 ,
 .Xr BIO_new 3 ,