BUG/MEDIUM: ssl: always send a full buffer after EAGAIN

Igor Chan reported a very interesting bug which was triggered by the
recent dynamic size change in SSL.

The OpenSSL API refuses to send less data than any failed previous
attempt. So what's happening is that if an SSL_write() in streaming
mode sends 5kB of data and the openssl layer cannot send them all,
it returns SSL_ERROR_WANT_WRITE, which haproxy reacts to by enabling
polling on the file descriptor. In the mean time, haproxy may detect
that the buffer was almost full and will disable streaming mode. Upon
write notification, it will try to send again, but less data this
time (limited to tune.ssl_max_record). OpenSSL disagrees with this
and returns a generic error SSL_ERROR_SSL.

The solution which was found consists in adding a flag to the SSL
context to remind that we must not shrink writes after a failed
attempt. Thus, if EAGAIN is encountered, the next send() will not
be limited in order to retry the same size as before.

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 404b203cb1f23b771e2467696ad5e8083fe5d20e..b095f8c1a58cbdd5f9d31bbc0bb655614705820c 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -74,8 +74,10 @@
 #include <proto/ssl_sock.h>
 #include <proto/task.h>
 
+/* Warning, these are bits, not integers! */
 #define SSL_SOCK_ST_FL_VERIFY_DONE  0x00000001
 #define SSL_SOCK_ST_FL_16K_WBFSIZE  0x00000002
+#define SSL_SOCK_SEND_UNLIMITED     0x00000004
 /* bits 0xFFFF0000 are reserved to store verify errors */
 
 /* Verify errors macros */
@@ -1533,15 +1535,27 @@ static int ssl_sock_from_buf(struct connection *conn, struct buffer *buf, int fl
                try = bo_contig_data(buf);
 
                if (!(flags & CO_SFL_STREAMER) &&
-                   global.tune.ssl_max_record && try > global.tune.ssl_max_record)
+                   !(conn->xprt_st & SSL_SOCK_SEND_UNLIMITED) &&
+                   global.tune.ssl_max_record && try > global.tune.ssl_max_record) {
                        try = global.tune.ssl_max_record;
+               }
+               else {
+                       /* we need to keep the information about the fact that
+                        * we're not limiting the upcoming send(), because if it
+                        * fails, we'll have to retry with at least as many data.
+                        */
+                       conn->xprt_st |= SSL_SOCK_SEND_UNLIMITED;
+               }
 
                ret = SSL_write(conn->xprt_ctx, bo_ptr(buf), try);
+
                if (conn->flags & CO_FL_ERROR) {
                        /* CO_FL_ERROR may be set by ssl_sock_infocbk */
                        goto out_error;
                }
                if (ret > 0) {
+                       conn->xprt_st &= ~SSL_SOCK_SEND_UNLIMITED;
+
                        buf->o -= ret;
                        done += ret;