As has been discussed several times in (at least) the past two years,
there's a serious problem with SSL_shutdown() for nonblocking connections:
it discards the underlying BIO's WANT_READ/WANT_WRITE information so that
the calling application cannot know in which direction it must wait for
I/O; so the choices are to spin (by waiting for I/O in either direction),
to use a timeout (not always practical in an event-driven application) and
re-check, or to switch to blocking mode -- again not really a good option.
Heuristics can reduce the chance of spinning or missing an I/O ready event
to a very small chance, but they cannot eliminate it. This patch makes the
problem go away.
The attached patch was sent to openssl-dev by Darryl Miles two years
ago. I strongly believe it implements correct behavior and is not
really a change to the API -- it just uses WANT_READ/WANT_WRITE
appropriately rather than returning SSL_ERROR_SYSCALL when it should
not. This is *not* the patch offered by another frustrated nbio user
last year which added a new error code to the library -- it uses the
existing ones as it should.
Would it be possible to get some feedback from the OpenSSL team? I
feel quite strongly this patch is needed and am sorely tempted to just
check it in to the copy of OpenSSL in NetBSD's tree, but would prefer
to not do so if there's any chance I can get OpenSSL developer comment
on the patch or the underlying problem, to avoid a local patch that
might not then be needed.
Darryl's description of the patch is at
http://marc.info/?l=openssl-dev&m=115153998821797&w=2 but one should
note it addresses other problems with the API that may be more serious
in practice than the one he originally observed (e.g. the blocking/
spinning described above).
Thor
diff -u -r -N -x '*~' -x Makefile openssl-0.9.8b/ssl/s3_lib.c
openssl-0.9.8b-me/ssl/s3_lib.c
--- openssl-0.9.8b/ssl/s3_lib.c 2006-01-15 07:14:38.000000000 +0000
+++ openssl-0.9.8b-me/ssl/s3_lib.c 2006-06-29 00:31:50.000000000 +0100
@@ -2071,6 +2071,7 @@
int ssl3_shutdown(SSL *s)
{
+ int ret;
/* Don't do anything much if we have not done the handshake or
* we don't want to send messages :-) */
@@ -2088,18 +2089,32 @@
#endif
/* our shutdown alert has been sent now, and if it still needs
* to be written, s->s3->alert_dispatch will be true */
+ if (s->s3->alert_dispatch)
+ return(-1); /* return WANT_WRITE */
}
else if (s->s3->alert_dispatch)
{
/* resend it if not sent */
#if 1
- s->method->ssl_dispatch_alert(s);
+ ret=s->method->ssl_dispatch_alert(s);
+ if(ret == -1)
+ {
+ /* we only get to return -1 here the 2nd/Nth
+ * invocation, we must have already signalled
+ * return 0 upon a previous invoation,
+ * return WANT_WRITE */
+ return(ret);
+ }
#endif
}
else if (!(s->shutdown & SSL_RECEIVED_SHUTDOWN))
{
/* If we are waiting for a close from our peer, we are closed */
s->method->ssl_read_bytes(s,0,NULL,0,0);
+ if(!(s->shutdown & SSL_RECEIVED_SHUTDOWN))
+ {
+ return(-1); /* return WANT_READ */
+ }
}
if ((s->shutdown == (SSL_SENT_SHUTDOWN|SSL_RECEIVED_SHUTDOWN)) &&
diff -u -r -N -x '*~' -x Makefile openssl-0.9.8b/ssl/s3_pkt.c
openssl-0.9.8b-me/ssl/s3_pkt.c
--- openssl-0.9.8b/ssl/s3_pkt.c 2005-10-01 00:38:20.000000000 +0100
+++ openssl-0.9.8b-me/ssl/s3_pkt.c 2006-06-22 17:45:46.000000000 +0100
@@ -1258,13 +1258,13 @@
return(1);
}
-void ssl3_send_alert(SSL *s, int level, int desc)
+int ssl3_send_alert(SSL *s, int level, int desc)
{
/* Map tls/ssl alert value to correct one */
desc=s->method->ssl3_enc->alert_value(desc);
if (s->version == SSL3_VERSION && desc == SSL_AD_PROTOCOL_VERSION)
desc = SSL_AD_HANDSHAKE_FAILURE; /* SSL 3.0 does not have
protocol_version alerts */
- if (desc < 0) return;
+ if (desc < 0) return -1;
/* If a fatal one, remove from cache */
if ((level == 2) && (s->session != NULL))
SSL_CTX_remove_session(s->ctx,s->session);
@@ -1273,9 +1273,10 @@
s->s3->send_alert[0]=level;
s->s3->send_alert[1]=desc;
if (s->s3->wbuf.left == 0) /* data still being written out? */
- s->method->ssl_dispatch_alert(s);
+ return s->method->ssl_dispatch_alert(s);
/* else data is still being written out, we will get written
* some time in the future */
+ return -1;
}
int ssl3_dispatch_alert(SSL *s)
diff -u -r -N -x '*~' -x Makefile openssl-0.9.8b/ssl/ssl_locl.h
openssl-0.9.8b-me/ssl/ssl_locl.h
--- openssl-0.9.8b/ssl/ssl_locl.h 2005-08-22 00:06:51.000000000 +0100
+++ openssl-0.9.8b-me/ssl/ssl_locl.h 2006-06-22 17:45:46.000000000 +0100
@@ -780,7 +780,7 @@
int ssl3_change_cipher_state(SSL *s,int which);
void ssl3_cleanup_key_block(SSL *s);
int ssl3_do_write(SSL *s,int type);
-void ssl3_send_alert(SSL *s,int level, int desc);
+int ssl3_send_alert(SSL *s,int level, int desc);
int ssl3_generate_master_secret(SSL *s, unsigned char *out,
unsigned char *p, int len);
int ssl3_get_req_cert_type(SSL *s,unsigned char *p);
