BUG/MINOR: quic: Possible crash when verifying certificates

This verification is done by ssl_sock_bind_verifycbk() which is set at different
locations in the ssl_sock.c code . About QUIC connections, there are a lot of chances
the connection object is not initialized when entering this function. What must
be accessed is the SSL object to retrieve the connection or quic_conn objects,
then the bind_conf object of the listener. If the connection object is not found,
we try to find the quic_conn object.

Modify ssl_sock_dump_errors() interface which takes a connection object as parameter
to also passed a quic_conn object as parameter. Again this function try first
to access the connection object if not NULL or the quic_conn object if not.

There is a remaining thing to do for QUIC: store the certificate verification error
code as it is currently stored in the connection object. This error code is at least
used by the "bc_err" and "fc_err" sample fetches.

There are chances this bug is in relation with GH #1851. Thank you to @tasavis
for the report.

Must be merged into 2.6.

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 0edad3b0824b9b80e29506ec3ddcec27fbb2ae84..caf41eb9a08bbb712827aa37614adc8005504613 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -638,7 +638,8 @@ SSL *ssl_sock_get_ssl_object(struct connection *conn)
  * if the debug mode and the verbose mode are activated. It dump all
  * the SSL error until the stack was empty.
  */
-static forceinline void ssl_sock_dump_errors(struct connection *conn)
+static forceinline void ssl_sock_dump_errors(struct connection *conn,
+                                             struct quic_conn *qc)
 {
        unsigned long ret;
 
@@ -650,9 +651,18 @@ static forceinline void ssl_sock_dump_errors(struct connection *conn)
                        ret = ERR_get_error();
                        if (ret == 0)
                                return;
-                       fprintf(stderr, "fd[%#x] OpenSSL error[0x%lx] %s: %s\n",
-                               conn_fd(conn), ret,
-                               func, ERR_reason_error_string(ret));
+                       if (conn) {
+                               fprintf(stderr, "fd[%#x] OpenSSL error[0x%lx] %s: %s\n",
+                                       conn_fd(conn), ret,
+                                       func, ERR_reason_error_string(ret));
+                       }
+#ifdef USE_QUIC
+                       else {
+                               /* TODO: we are not sure <conn> is always initialized for QUIC connections */
+                               fprintf(stderr, "qc @%p OpenSSL error[0x%lx] %s: %s\n", qc, ret,
+                                       func, ERR_reason_error_string(ret));
+                       }
+#endif
                }
        }
 }
@@ -1699,16 +1709,36 @@ int ssl_sock_bind_verifycbk(int ok, X509_STORE_CTX *x_store)
 {
        SSL *ssl;
        struct connection *conn;
-       struct ssl_sock_ctx *ctx;
+       struct ssl_sock_ctx *ctx = NULL;
        int err, depth;
        X509 *client_crt;
        STACK_OF(X509) *certs;
+       struct bind_conf *bind_conf;
+       struct quic_conn *qc = NULL;
 
        ssl = X509_STORE_CTX_get_ex_data(x_store, SSL_get_ex_data_X509_STORE_CTX_idx());
        conn = SSL_get_ex_data(ssl, ssl_app_data_index);
        client_crt = SSL_get_ex_data(ssl, ssl_client_crt_ref_index);
 
-       ctx = __conn_get_ssl_sock_ctx(conn);
+       if (conn) {
+               bind_conf = __objt_listener(conn->target)->bind_conf;
+               ctx = __conn_get_ssl_sock_ctx(conn);
+       }
+#ifdef USE_QUIC
+       else {
+               qc = SSL_get_ex_data(ssl, ssl_qc_app_data_index);
+               if (qc) {
+                       bind_conf = qc->li->bind_conf;
+                       ctx = qc->xprt_ctx;
+               }
+       }
+#endif
+
+       if (!ctx || !bind_conf) {
+               /* Must never happen */
+               ABORT_NOW();
+       }
+
        ctx->xprt_st |= SSL_SOCK_ST_FL_VERIFY_DONE;
 
        depth = X509_STORE_CTX_get_error_depth(x_store);
@@ -1751,13 +1781,12 @@ int ssl_sock_bind_verifycbk(int ok, X509_STORE_CTX *x_store)
                        ctx->xprt_st |= SSL_SOCK_CAEDEPTH_TO_ST(depth);
                }
 
-               if (err < 64 && __objt_listener(conn->target)->bind_conf->ca_ignerr & (1ULL << err)) {
-                       ssl_sock_dump_errors(conn);
-                       ERR_clear_error();
-                       return 1;
-               }
+               if (err < 64 && bind_conf->ca_ignerr & (1ULL << err))
+                       goto err_ignored;
 
-               conn->err_code = CO_ER_SSL_CA_FAIL;
+               /* TODO: for QUIC connection, this error code is lost */
+               if (conn)
+                       conn->err_code = CO_ER_SSL_CA_FAIL;
                return 0;
        }
 
@@ -1765,14 +1794,18 @@ int ssl_sock_bind_verifycbk(int ok, X509_STORE_CTX *x_store)
                ctx->xprt_st |= SSL_SOCK_CRTERROR_TO_ST(err);
 
        /* check if certificate error needs to be ignored */
-       if (err < 64 && __objt_listener(conn->target)->bind_conf->crt_ignerr & (1ULL << err)) {
-               ssl_sock_dump_errors(conn);
-               ERR_clear_error();
-               return 1;
-       }
+       if (err < 64 && bind_conf->crt_ignerr & (1ULL << err))
+               goto err_ignored;
 
-       conn->err_code = CO_ER_SSL_CRT_FAIL;
+       /* TODO: for QUIC connection, this error code is lost */
+       if (conn)
+               conn->err_code = CO_ER_SSL_CRT_FAIL;
        return 0;
+
+ err_ignored:
+       ssl_sock_dump_errors(conn, qc);
+       ERR_clear_error();
+       return 1;
 }
 
 #ifdef TLS1_RT_HEARTBEAT
@@ -6229,7 +6262,7 @@ reneg_ok:
 
  out_error:
        /* Clear openssl global errors stack */
-       ssl_sock_dump_errors(conn);
+       ssl_sock_dump_errors(conn, NULL);
        ERR_clear_error();
 
        /* free resumed session if exists */
@@ -6594,7 +6627,7 @@ static size_t ssl_sock_to_buf(struct connection *conn, void *xprt_ctx, struct bu
 
  clear_ssl_error:
        /* Clear openssl global errors stack */
-       ssl_sock_dump_errors(conn);
+       ssl_sock_dump_errors(conn, NULL);
        ERR_clear_error();
  read0:
        conn_sock_read0(conn);
@@ -6603,7 +6636,7 @@ static size_t ssl_sock_to_buf(struct connection *conn, void *xprt_ctx, struct bu
  out_error:
        conn->flags |= CO_FL_ERROR;
        /* Clear openssl global errors stack */
-       ssl_sock_dump_errors(conn);
+       ssl_sock_dump_errors(conn, NULL);
        ERR_clear_error();
        goto leave;
 }
@@ -6759,7 +6792,7 @@ static size_t ssl_sock_from_buf(struct connection *conn, void *xprt_ctx, const s
 
  out_error:
        /* Clear openssl global errors stack */
-       ssl_sock_dump_errors(conn);
+       ssl_sock_dump_errors(conn, NULL);
        ERR_clear_error();
 
        conn->flags |= CO_FL_ERROR;
@@ -6858,7 +6891,7 @@ static void ssl_sock_shutw(struct connection *conn, void *xprt_ctx, int clean)
        /* no handshake was in progress, try a clean ssl shutdown */
        if (SSL_shutdown(ctx->ssl) <= 0) {
                /* Clear openssl global errors stack */
-               ssl_sock_dump_errors(conn);
+               ssl_sock_dump_errors(conn, NULL);
                ERR_clear_error();
        }
 }