Commit 0cb76e79 authored by Dr. Stephen Henson's avatar Dr. Stephen Henson
Browse files

PR: 1748 

Fix nasty SSL BIO pop bug. Since this changes the behaviour of SSL BIOs and
will break applications that worked around the bug only included in 1.0.0 and
later.
parent 6cb41967

CHANGES

+10 −0
Original line number Diff line number Diff line
@@ -4,6 +4,16 @@ 


 Changes between 0.9.8k and 1.0  [xx XXX xxxx]



  *) In BIO_pop() and BIO_push() use the ctrl argument (which was NULL) to

     indicate the initial BIO being pushed or popped. This makes it possible

     to determine whether the BIO is the one explicitly called or as a result

     of the ctrl being passed down the chain. Fix BIO_pop() and SSL BIOs so

     it handles reference counts correctly and doesn't zero out the I/O bio

     when it is not being explicitly popped. WARNING: applications which

     included workarounds for the old buggy behaviour will need to be modified

     or they could free up already freed BIOs.

     [Steve Henson]



  *) Rename uni2asc and asc2uni functions to OPENSSL_uni2asc and

     OPENSSL_asc2uni the original names were too generic and cause name

     clashes on Netware.

crypto/bio/bio_lib.c

+2 −2
Original line number Diff line number Diff line
@@ -429,7 +429,7 @@ BIO *BIO_push(BIO *b, BIO *bio) 
	if (bio != NULL)

		bio->prev_bio=lb;

	/* called to do internal processing */

	BIO_ctrl(b,BIO_CTRL_PUSH,0,NULL);

	BIO_ctrl(b,BIO_CTRL_PUSH,0,lb);

	return(b);

	}
@@ -441,7 +441,7 @@ BIO *BIO_pop(BIO *b) 
	if (b == NULL) return(NULL);

	ret=b->next_bio;



	BIO_ctrl(b,BIO_CTRL_POP,0,NULL);

	BIO_ctrl(b,BIO_CTRL_POP,0,b);



	if (b->prev_bio != NULL)

		b->prev_bio->next_bio=b->next_bio;

doc/crypto/BIO_f_ssl.pod

+9 −0
Original line number Diff line number Diff line
@@ -308,6 +308,15 @@ a client and also echoes the request to standard output. 


 BIO_free_all(sbio);



=head1 BUGS



In OpenSSL versions before 1.0.0 the BIO_pop() call was handled incorrectly,

the I/O BIO reference count was incorrectly incremented (instead of

decremented) and dissociated with the SSL BIO even if the SSL BIO was not

explicitly being popped (e.g. a pop higher up the chain). Applications which

included workarounds for this bug (e.g. freeing BIOs more than once) should

be modified to handle this fix or they may free up an already freed BIO.



=head1 SEE ALSO



TBA

ssl/bio_ssl.c

+11 −9
Original line number Diff line number Diff line
@@ -398,17 +398,19 @@ static long ssl_ctrl(BIO *b, int cmd, long num, void *ptr) 
			}

		break;

	case BIO_CTRL_POP:

		/* ugly bit of a hack */

		if (ssl->rbio != ssl->wbio) /* we are in trouble :-( */

		/* Only detach if we are the BIO explicitly being popped */

		if (b == ptr)

			{

			/* Shouldn't happen in practice because the

			 * rbio and wbio are the same when pushed.

			 */

			if (ssl->rbio != ssl->wbio)

				BIO_free_all(ssl->wbio);

			}

			if (b->next_bio != NULL)

			{

			CRYPTO_add(&b->next_bio->references,1,CRYPTO_LOCK_BIO);

			}

				CRYPTO_add(&b->next_bio->references,-1,CRYPTO_LOCK_BIO);

			ssl->wbio=NULL;

			ssl->rbio=NULL;

			}

		break;

	case BIO_C_DO_STATE_MACHINE:

		BIO_clear_retry_flags(b);