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

PR: 1949 

Submitted by: steve@openssl.org

More robust fix and workaround for PR#1949. Don't try to work out if there
is any write pending data as this can be unreliable: always flush.
parent 94137885

CHANGES

+8 −0
Original line number Diff line number Diff line
@@ -4,6 +4,14 @@ 


 Changes between 0.9.8l and 0.9.8m [xx XXX xxxx]



  *) The code that handled flusing of data in SSL/TLS originally used the

     BIO_CTRL_INFO ctrl to see if any data was pending first. This caused

     the problem outlined in PR#1949. The fix suggested there however can

     trigger problems with buggy BIO_CTRL_WPENDING (e.g. some versions

     of Apache). So instead simplify the code to flush unconditionally.

     This should be fine since flushing with no data to flush is a no op.

     [Steve Henson]



  *) Handle TLS versions 2.0 and later properly and correctly use the

     highest version of TLS/SSL supported. Although TLS >= 2.0 is some way

     off ancient servers have a habit of sticking around for a while...

ssl/d1_clnt.c

+5 −9
Original line number Diff line number Diff line
@@ -145,7 +145,6 @@ int dtls1_connect(SSL *s) 
	{

	BUF_MEM *buf=NULL;

	unsigned long Time=(unsigned long)time(NULL),l;

	long num1;

	void (*cb)(const SSL *ssl,int type,int val)=NULL;

	int ret= -1;

	int new_state,state,skip=0;;
@@ -509,16 +508,13 @@ int dtls1_connect(SSL *s) 
			break;



		case SSL3_ST_CW_FLUSH:

			/* number of bytes to be flushed */

			num1=BIO_ctrl(s->wbio,BIO_CTRL_INFO,0,NULL);

			if (num1 > 0)

				{

			s->rwstate=SSL_WRITING;

				num1=BIO_flush(s->wbio);

				if (num1 <= 0) { ret= -1; goto end; }

				s->rwstate=SSL_NOTHING;

			if (BIO_flush(s->wbio) <= 0)

				{

				ret= -1;

				goto end;

				}



			s->rwstate=SSL_NOTHING;

			s->state=s->s3->tmp.next_state;

			break;

ssl/d1_srvr.c

+6 −10
Original line number Diff line number Diff line
@@ -146,7 +146,6 @@ int dtls1_accept(SSL *s) 
	BUF_MEM *buf;

	unsigned long l,Time=(unsigned long)time(NULL);

	void (*cb)(const SSL *ssl,int type,int val)=NULL;

	long num1;

	int ret= -1;

	int new_state,state,skip=0;
@@ -441,17 +440,14 @@ int dtls1_accept(SSL *s) 
			s->init_num=0;

			break;

		

		case SSL3_ST_SW_FLUSH:

			/* number of bytes to be flushed */

			num1=BIO_ctrl(s->wbio,BIO_CTRL_INFO,0,NULL);

			if (num1 > 0)

				{

		case SSL3_ST_CW_FLUSH:

			s->rwstate=SSL_WRITING;

				num1=BIO_flush(s->wbio);

				if (num1 <= 0) { ret= -1; goto end; }

				s->rwstate=SSL_NOTHING;

			if (BIO_flush(s->wbio) <= 0)

				{

				ret= -1;

				goto end;

				}



			s->rwstate=SSL_NOTHING;

			s->state=s->s3->tmp.next_state;

			break;

ssl/s3_clnt.c

+5 −9
Original line number Diff line number Diff line
@@ -167,7 +167,6 @@ int ssl3_connect(SSL *s) 
	{

	BUF_MEM *buf=NULL;

	unsigned long Time=(unsigned long)time(NULL),l;

	long num1;

	void (*cb)(const SSL *ssl,int type,int val)=NULL;

	int ret= -1;

	int new_state,state,skip=0;
@@ -496,16 +495,13 @@ int ssl3_connect(SSL *s) 
			break;



		case SSL3_ST_CW_FLUSH:

			/* number of bytes to be flushed */

			num1=BIO_ctrl(s->wbio,BIO_CTRL_INFO,0,NULL);

			if (num1 > 0)

				{

			s->rwstate=SSL_WRITING;

				num1=BIO_flush(s->wbio);

				if (num1 <= 0) { ret= -1; goto end; }

				s->rwstate=SSL_NOTHING;

			if (BIO_flush(s->wbio) <= 0)

				{

				ret= -1;

				goto end;

				}



			s->rwstate=SSL_NOTHING;

			s->state=s->s3->tmp.next_state;

			break;

ssl/s3_srvr.c

+14 −20
Original line number Diff line number Diff line
@@ -166,7 +166,6 @@ int ssl3_accept(SSL *s) 
	BUF_MEM *buf;

	unsigned long l,Time=(unsigned long)time(NULL);

	void (*cb)(const SSL *ssl,int type,int val)=NULL;

	long num1;

	int ret= -1;

	int new_state,state,skip=0;
@@ -447,29 +446,24 @@ int ssl3_accept(SSL *s) 
			break;

		

		case SSL3_ST_SW_FLUSH:

			/* number of bytes to be flushed */

			/* This originally and incorrectly called BIO_CTRL_INFO

			 * The reason why this is wrong is mentioned in PR#1949.

			 * Unfortunately, as suggested in that bug some

			 * versions of Apache unconditionally return 0

			 * for BIO_CTRL_WPENDING meaning we don't correctly

			 * flush data and some operations, like renegotiation,

			 * don't work. Other software may also be affected so

			 * call BIO_CTRL_INFO to retain compatibility with

			 * previous behaviour and BIO_CTRL_WPENDING if we

			 * get zero to address the PR#1949 case.



			/* This code originally checked to see if

			 * any data was pending using BIO_CTRL_INFO

			 * and then flushed. This caused problems

			 * as documented in PR#1939. The proposed

			 * fix doesn't completely resolve this issue

			 * as buggy implementations of BIO_CTRL_PENDING

			 * still exist. So instead we just flush

			 * unconditionally.

			 */



			num1=BIO_ctrl(s->wbio,BIO_CTRL_INFO,0,NULL);

			if (num1 == 0)

				num1=BIO_ctrl(s->wbio,BIO_CTRL_WPENDING,0,NULL);

			if (num1 > 0)

				{

			s->rwstate=SSL_WRITING;

				num1=BIO_flush(s->wbio);

				if (num1 <= 0) { ret= -1; goto end; }

				s->rwstate=SSL_NOTHING;

			if (BIO_flush(s->wbio) <= 0)

				{

				ret= -1;

				goto end;

				}

			s->rwstate=SSL_NOTHING;



			s->state=s->s3->tmp.next_state;

			break;