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 <agl@google.com>
This commit is contained in:
parent
8cfd8ad144
commit
ec48af40a7
@ -508,9 +508,6 @@ 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
|
||||
@ -518,6 +515,10 @@ struct ssl_session_st
|
||||
* 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.
|
||||
|
33
ssl/d1_pkt.c
33
ssl/d1_pkt.c
@ -976,23 +976,6 @@ start:
|
||||
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 @@ start:
|
||||
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;
|
||||
}
|
||||
|
||||
|
33
ssl/s3_pkt.c
33
ssl/s3_pkt.c
@ -1151,23 +1151,6 @@ start:
|
||||
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 @@ start:
|
||||
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;
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user