Hi Emmanuel,

On Tue, Feb 13, 2018 at 05:40:00PM +0100, Emmanuel Hocdet wrote:
> Hi Olivier
> 
> > Le 13 févr. 2018 à 15:27, Olivier Houchard <ohouch...@haproxy.com> a écrit :
> > 
> > Thanks a lot for the detailed analyze, and sorry for the late answer.
> > You're probably right, SSL_ERROR_SYSCALL shouldn't be treated as an
> > unrecoverable error.
> > So, what you basically did was something equivalent to the patch attached ?
> > 
> > Thanks a lot !
> > 
> > Olivier
> > <0001-MINOR-BUG-ssl-Don-t-always-treat-SSL_ERROR_SYSCALL-a.patch>
> 
> 
> 'ret' can be unassigned
> errno must be check earlier.

I (mostly) disagree.
There are 3 goto out_error, and on the only one that mattered, ret and
errno should be fine.
However, I rewrote the patch to make it clearer what the intent is.
Do you like it better ?

Cheers,

Olivier
>From 8de2963753a33ca948beebcdd70c5f46bd01e4cf Mon Sep 17 00:00:00 2001
From: Olivier Houchard <ohouch...@haproxy.com>
Date: Tue, 13 Feb 2018 15:17:23 +0100
Subject: [PATCH] MINOR/BUG: ssl: Don't always treat SSL_ERROR_SYSCALL as
 unrecovarable.

SSL_Raad() might return <= 0, and SSL_get_erro() return SSL_ERROR_SYSCALL,
without meaning the connection is gone. Before flagging the conection
as in error, check the errno value.

This should be backported to 1.8.
---
 src/ssl_sock.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index aee3cd965..83e9b86e4 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -5313,6 +5313,7 @@ reneg_ok:
 static int ssl_sock_to_buf(struct connection *conn, struct buffer *buf, int 
count)
 {
        int ret, done = 0;
+       int set_error_flag = 1;
        int try;
 
        conn_refresh_polling_flags(conn);
@@ -5434,6 +5435,13 @@ static int ssl_sock_to_buf(struct connection *conn, 
struct buffer *buf, int coun
                                break;
                        } else if (ret == SSL_ERROR_ZERO_RETURN)
                                goto read0;
+                       /* For SSL_ERROR_SYSCALL, make sure the error is
+                        * unrecoverable before flagging the connection as
+                        * in error.
+                        */
+                       if (ret == SSL_ERROR_SYSCALL || (!errno || errno ==
+                           EAGAIN))
+                               set_error_flag = 0;
                        /* otherwise it's a real error */
                        goto out_error;
                }
@@ -5452,7 +5460,8 @@ static int ssl_sock_to_buf(struct connection *conn, 
struct buffer *buf, int coun
        ssl_sock_dump_errors(conn);
        ERR_clear_error();
 
-       conn->flags |= CO_FL_ERROR;
+       if (set_error_flag)
+               conn->flags |= CO_FL_ERROR;
        goto leave;
 }
 
-- 
2.14.3

Reply via email to