Make SSL_MODE_AUTO_RETRY the default.

Without SSL_MODE_AUTO_RETRY, even blocking mode will return
SSL_ERROR_WANT_{READ|WRITE} in the event of a renegotiation.

The comments in the code speak only of "nasty problems" unless this is
done. The original commit that added SSL_MODE_AUTO_RETRY
(54f10e6adce56eb2e59936e32216162aadc5d050) gives a little more detail:

    The [...] behaviour is needed by applications such as s_client and
    s_server that use select() to determine when to use SSL_read.

Without the -nbio flag, s_client will use select() to find when the
socket is readable and then call SSL_read with a blocking socket.
However, this will still block in the event of an incomplete record, so
the delay is already unbounded. This it's very unclear what the point of
this behaviour ever was.

Perhaps if the read and write paths were different sockets where the
read socket was non-blocking but the write socket was blocking. But that
seems like an implausible situation to worry too much about.

Change-Id: I9d9f2526afc2e0fd0e5440e9a047f419a2d61afa
Reviewed-on: https://boringssl-review.googlesource.com/2140
Reviewed-by: Adam Langley <[email protected]>

Author: agl

include/openssl/ssl.h

@@ -508,16 +508,17 @@ struct ssl_session_st
  * the misconception that non-blocking SSL_write() behaves like
  * non-blocking write(): */
 #define SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER 0x00000002L
-/* Never bother the application with retries if the transport
- * is blocking: */
-#define SSL_MODE_AUTO_RETRY 0x00000004L
 /* Don't attempt to automatically build certificate chain */
 #define SSL_MODE_NO_AUTO_CHAIN 0x00000008L
 /* Save RAM by releasing read and write buffers when they're empty. (SSL3 and
  * TLS only.)  "Released" buffers are put onto a free-list in the context or
  * just freed (depending on the context's setting for freelist_max_len). */
 #define SSL_MODE_RELEASE_BUFFERS 0x00000010L
 
+/* The following flags do nothing and are included only to make it easier to
+ * compile code with BoringSSL. */
+#define SSL_MODE_AUTO_RETRY 0
+
 /* Send the current time in the Random fields of the ClientHello and
  * ServerHello records for compatibility with hypothetical implementations
  * that require it.

ssl/d1_pkt.c

@@ -976,23 +976,6 @@ int dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
 					OPENSSL_PUT_ERROR(SSL, dtls1_read_bytes, SSL_R_SSL_HANDSHAKE_FAILURE);
 					return(-1);
 					}
-
-				if (!(s->mode & SSL_MODE_AUTO_RETRY))
-					{
-					if (s->s3->rbuf.left == 0) /* no read-ahead left? */
-						{
-						BIO *bio;
-						/* In the case where we try to read application data,
-						 * but we trigger an SSL handshake, we return -1 with
-						 * the retry option set.  Otherwise renegotiation may
-						 * cause nasty problems in the blocking world */
-						s->rwstate=SSL_READING;
-						bio=SSL_get_rbio(s);
-						BIO_clear_retry_flags(bio);
-						BIO_set_retry_read(bio);
-						return(-1);
-						}
-					}
 				}
 			}
 		/* we either finished a handshake or ignored the request,
@@ -1155,22 +1138,6 @@ int dtls1_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
 			return(-1);
 			}
 
-		if (!(s->mode & SSL_MODE_AUTO_RETRY))
-			{
-			if (s->s3->rbuf.left == 0) /* no read-ahead left? */
-				{
-				BIO *bio;
-				/* In the case where we try to read application data,
-				 * but we trigger an SSL handshake, we return -1 with
-				 * the retry option set.  Otherwise renegotiation may
-				 * cause nasty problems in the blocking world */
-				s->rwstate=SSL_READING;
-				bio=SSL_get_rbio(s);
-				BIO_clear_retry_flags(bio);
-				BIO_set_retry_read(bio);
-				return(-1);
-				}
-			}
 		goto start;
 		}
 

ssl/s3_pkt.c

@@ -1151,23 +1151,6 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
 					OPENSSL_PUT_ERROR(SSL, ssl3_read_bytes, SSL_R_SSL_HANDSHAKE_FAILURE);
 					return(-1);
 					}
-
-				if (!(s->mode & SSL_MODE_AUTO_RETRY))
-					{
-					if (s->s3->rbuf.left == 0) /* no read-ahead left? */
-						{
-						BIO *bio;
-						/* In the case where we try to read application data,
-						 * but we trigger an SSL handshake, we return -1 with
-						 * the retry option set.  Otherwise renegotiation may
-						 * cause nasty problems in the blocking world */
-						s->rwstate=SSL_READING;
-						bio=SSL_get_rbio(s);
-						BIO_clear_retry_flags(bio);
-						BIO_set_retry_read(bio);
-						return(-1);
-						}
-					}
 				}
 			}
 		/* we either finished a handshake or ignored the request,
@@ -1335,22 +1318,6 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
 			return(-1);
 			}
 
-		if (!(s->mode & SSL_MODE_AUTO_RETRY))
-			{
-			if (s->s3->rbuf.left == 0) /* no read-ahead left? */
-				{
-				BIO *bio;
-				/* In the case where we try to read application data,
-				 * but we trigger an SSL handshake, we return -1 with
-				 * the retry option set.  Otherwise renegotiation may
-				 * cause nasty problems in the blocking world */
-				s->rwstate=SSL_READING;
-				bio=SSL_get_rbio(s);
-				BIO_clear_retry_flags(bio);
-				BIO_set_retry_read(bio);
-				return(-1);
-				}
-			}
 		goto start;
 		}