From 1b3c931bffbac8ffb3efb1b489f7572be219e6e8 Mon Sep 17 00:00:00 2001 From: Olivier Houchard Date: Fri, 5 Mar 2021 23:37:48 +0100 Subject: [PATCH] MEDIUM: connections: Introduce a new XPRT method, start(). Introduce a new XPRT method, start(). The init() method will now only initialize whatever is needed for the XPRT to run, but any action the XPRT has to do before being ready, such as handshakes, will be done in the new start() method. That way, we will be sure the full stack of xprt will be initialized before attempting to do anything. The init() call is also moved to conn_prepare(). There's no longer any reason to wait for the ctrl to be ready, any action will be deferred until start(), anyway. This means conn_xprt_init() is no longer needed. --- include/haproxy/connection-t.h | 3 ++- include/haproxy/connection.h | 43 +++++++++++++++++++++------------- src/backend.c | 22 +++++++++-------- src/proto_quic.c | 6 ----- src/proto_sockpair.c | 6 ----- src/proto_tcp.c | 6 ----- src/proto_uxst.c | 6 ----- src/quic_sock.c | 10 ++++---- src/session.c | 10 ++++---- src/ssl_sock.c | 3 --- src/tcpcheck.c | 11 ++++++++- src/xprt_handshake.c | 2 -- src/xprt_quic.c | 3 --- 13 files changed, 63 insertions(+), 68 deletions(-) diff --git a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h index 8e0ee7d4b8..4635700d65 100644 --- a/include/haproxy/connection-t.h +++ b/include/haproxy/connection-t.h @@ -128,7 +128,7 @@ enum { /* These flags indicate whether the Control and Transport layers are initialized */ CO_FL_CTRL_READY = 0x00000100, /* FD was registered, fd_delete() needed */ - CO_FL_XPRT_READY = 0x00000200, /* xprt_init() done, xprt_close() needed */ + CO_FL_XPRT_READY = 0x00000200, /* xprt_start() done, xprt can be used */ /* unused : 0x00000400 */ @@ -374,6 +374,7 @@ struct xprt_ops { void (*shutw)(struct connection *conn, void *xprt_ctx, int); /* shutw function */ void (*close)(struct connection *conn, void *xprt_ctx); /* close the transport layer */ int (*init)(struct connection *conn, void **ctx); /* initialize the transport layer */ + int (*start)(struct connection *conn, void *ctx); /* Start the transport layer, if needed */ int (*prepare_bind_conf)(struct bind_conf *conf); /* prepare a whole bind_conf */ void (*destroy_bind_conf)(struct bind_conf *conf); /* destroy a whole bind_conf */ int (*prepare_srv)(struct server *srv); /* prepare a server context */ diff --git a/include/haproxy/connection.h b/include/haproxy/connection.h index 739d6a636e..7370263314 100644 --- a/include/haproxy/connection.h +++ b/include/haproxy/connection.h @@ -74,28 +74,29 @@ int conn_create_mux(struct connection *conn); extern struct idle_conns idle_conns[MAX_THREADS]; -/* returns true is the transport layer is ready */ +/* returns true if the transport layer is ready */ static inline int conn_xprt_ready(const struct connection *conn) { return (conn->flags & CO_FL_XPRT_READY); } -/* returns true is the control layer is ready */ +/* returns true if the control layer is ready */ static inline int conn_ctrl_ready(const struct connection *conn) { return (conn->flags & CO_FL_CTRL_READY); } -/* Calls the init() function of the transport layer if any and if not done yet, - * and sets the CO_FL_XPRT_READY flag to indicate it was properly initialized. - * Returns <0 in case of error. - */ -static inline int conn_xprt_init(struct connection *conn) +/* + * Calls the start() function of the transport layer, if needed. + * Returns < 0 in case of error. +*/ + +static inline int conn_xprt_start(struct connection *conn) { int ret = 0; - if (!conn_xprt_ready(conn) && conn->xprt && conn->xprt->init) - ret = conn->xprt->init(conn, &conn->xprt_ctx); + if (!conn_xprt_ready(conn) && conn->xprt && conn->xprt->start) + ret = conn->xprt->start(conn, conn->xprt_ctx); if (ret >= 0) conn->flags |= CO_FL_XPRT_READY; @@ -104,17 +105,18 @@ static inline int conn_xprt_init(struct connection *conn) } /* Calls the close() function of the transport layer if any and if not done - * yet, and clears the CO_FL_XPRT_READY flag. However this is not done if the - * CO_FL_XPRT_TRACKED flag is set, which allows logs to take data from the - * transport layer very late if needed. + * yet, and clears the CO_FL_XPRT_READY flags + * However this is not done if the CO_FL_XPRT_TRACKED flag is set, + * which allows logs to take data from the transport layer very late if needed. */ static inline void conn_xprt_close(struct connection *conn) { - if ((conn->flags & (CO_FL_XPRT_READY|CO_FL_XPRT_TRACKED)) == CO_FL_XPRT_READY) { + if (conn->xprt && !(conn->flags & CO_FL_XPRT_TRACKED)) { if (conn->xprt->close) conn->xprt->close(conn, conn->xprt_ctx); conn->xprt_ctx = NULL; conn->flags &= ~CO_FL_XPRT_READY; + conn->xprt = NULL; } } @@ -138,7 +140,7 @@ static inline void conn_ctrl_init(struct connection *conn) */ static inline void conn_ctrl_close(struct connection *conn) { - if ((conn->flags & (CO_FL_XPRT_READY|CO_FL_CTRL_READY)) == CO_FL_CTRL_READY) { + if (!conn->xprt && (conn->flags & CO_FL_CTRL_READY)) { conn->flags &= ~CO_FL_CTRL_READY; if (conn->ctrl->ctrl_close) conn->ctrl->ctrl_close(conn); @@ -313,13 +315,21 @@ static inline int conn_xprt_read0_pending(struct connection *c) * set prior to calling this function so that the function may make use of it * in the future to refine the mux choice if needed. */ -static inline void conn_prepare(struct connection *conn, const struct protocol *proto, const struct xprt_ops *xprt) +static inline int conn_prepare(struct connection *conn, const struct protocol *proto, const struct xprt_ops *xprt) { + int ret = 0; + conn->ctrl = proto; conn->xprt = xprt; conn->mux = NULL; conn->xprt_ctx = NULL; conn->ctx = NULL; + if (xprt->init) { + ret = xprt->init(conn, &conn->xprt_ctx); + if (ret < 0) + conn->xprt = NULL; + } + return ret; } /* @@ -359,6 +369,7 @@ static inline void conn_init(struct connection *conn, void *target) conn->proxy_unique_id = IST_NULL; conn->qc = NULL; conn->hash_node = NULL; + conn->xprt = NULL; } static inline struct conn_hash_node *conn_alloc_hash_node(struct connection *conn) @@ -795,7 +806,7 @@ static inline const char *conn_get_ctrl_name(const struct connection *conn) static inline const char *conn_get_xprt_name(const struct connection *conn) { - if (!conn || !conn_xprt_ready(conn)) + if (!conn || !conn->xprt) return "NONE"; return conn->xprt->name; } diff --git a/src/backend.c b/src/backend.c index 67a7625574..80f00f2586 100644 --- a/src/backend.c +++ b/src/backend.c @@ -1535,14 +1535,19 @@ skip_reuse: /* Copy network namespace from client connection */ srv_conn->proxy_netns = cli_conn ? cli_conn->proxy_netns : NULL; - if (!conn_xprt_ready(srv_conn) && !srv_conn->mux) { + if (!srv_conn->xprt) { /* set the correct protocol on the output stream interface */ - if (srv) - conn_prepare(srv_conn, protocol_by_family(srv_conn->dst->ss_family), srv->xprt); - else if (obj_type(s->target) == OBJ_TYPE_PROXY) { + if (srv) { + if (conn_prepare(srv_conn, protocol_by_family(srv_conn->dst->ss_family), srv->xprt)) { + conn_free(srv_conn); + return SF_ERR_INTERNAL; + } + } else if (obj_type(s->target) == OBJ_TYPE_PROXY) { + int ret; + /* proxies exclusively run on raw_sock right now */ - conn_prepare(srv_conn, protocol_by_family(srv_conn->dst->ss_family), xprt_get(XPRT_RAW)); - if (!(srv_conn->ctrl)) { + ret = conn_prepare(srv_conn, protocol_by_family(srv_conn->dst->ss_family), xprt_get(XPRT_RAW)); + if (ret < 0 || !(srv_conn->ctrl)) { conn_free(srv_conn); return SF_ERR_INTERNAL; } @@ -1580,10 +1585,6 @@ skip_reuse: srv_conn->flags |= CO_FL_SOCKS4; } } - else if (!conn_xprt_ready(srv_conn)) { - if (srv_conn->mux->reset) - srv_conn->mux->reset(srv_conn); - } else { /* Currently there seems to be no known cases of xprt ready * without the mux installed here. @@ -1633,6 +1634,7 @@ skip_reuse: return SF_ERR_INTERNAL; } } + conn_xprt_start(srv_conn); /* We have to defer the mux initialization until after si_connect() * has been called, as we need the xprt to have been properly diff --git a/src/proto_quic.c b/src/proto_quic.c index fa36ba7072..c52d7927d5 100644 --- a/src/proto_quic.c +++ b/src/proto_quic.c @@ -505,12 +505,6 @@ int quic_connect_server(struct connection *conn, int flags) fd_cant_recv(fd); } - if (conn_xprt_init(conn) < 0) { - conn_full_close(conn); - conn->flags |= CO_FL_ERROR; - return SF_ERR_RESOURCE; - } - return SF_ERR_NONE; /* connection is OK */ } diff --git a/src/proto_sockpair.c b/src/proto_sockpair.c index 306213e77d..a1e9d2540e 100644 --- a/src/proto_sockpair.c +++ b/src/proto_sockpair.c @@ -364,12 +364,6 @@ static int sockpair_connect_server(struct connection *conn, int flags) conn_ctrl_init(conn); /* registers the FD */ fdtab[fd].linger_risk = 0; /* no need to disable lingering */ - if (conn_xprt_init(conn) < 0) { - conn_full_close(conn); - conn->flags |= CO_FL_ERROR; - return SF_ERR_RESOURCE; - } - return SF_ERR_NONE; /* connection is OK */ } diff --git a/src/proto_tcp.c b/src/proto_tcp.c index 53569a9989..cef2ff890c 100644 --- a/src/proto_tcp.c +++ b/src/proto_tcp.c @@ -565,12 +565,6 @@ int tcp_connect_server(struct connection *conn, int flags) fd_cant_recv(fd); } - if (conn_xprt_init(conn) < 0) { - conn_full_close(conn); - conn->flags |= CO_FL_ERROR; - return SF_ERR_RESOURCE; - } - return SF_ERR_NONE; /* connection is OK */ } diff --git a/src/proto_uxst.c b/src/proto_uxst.c index f33099e702..8156eb8049 100644 --- a/src/proto_uxst.c +++ b/src/proto_uxst.c @@ -354,12 +354,6 @@ static int uxst_connect_server(struct connection *conn, int flags) fd_cant_recv(fd); } - if (conn_xprt_init(conn) < 0) { - conn_full_close(conn); - conn->flags |= CO_FL_ERROR; - return SF_ERR_RESOURCE; - } - return SF_ERR_NONE; /* connection is OK */ } diff --git a/src/quic_sock.c b/src/quic_sock.c index 5d030b7cd5..571c5db3c2 100644 --- a/src/quic_sock.c +++ b/src/quic_sock.c @@ -35,7 +35,8 @@ int quic_session_accept(struct connection *cli_conn) struct session *sess; cli_conn->proxy_netns = l->rx.settings->netns; - conn_prepare(cli_conn, l->rx.proto, l->bind_conf->xprt); + if (conn_prepare(cli_conn, l->rx.proto, l->bind_conf->xprt) < 0) + goto out_free_conn; /* This flag is ordinarily set by conn_ctrl_init() which cannot * be called for now. @@ -50,14 +51,15 @@ int quic_session_accept(struct connection *cli_conn) if (l->options & LI_O_ACC_CIP) cli_conn->flags |= CO_FL_ACCEPT_CIP; - if (conn_xprt_init(cli_conn) < 0) - goto out_free_conn; - /* Add the handshake pseudo-XPRT */ if (cli_conn->flags & (CO_FL_ACCEPT_PROXY | CO_FL_ACCEPT_CIP)) { if (xprt_add_hs(cli_conn) != 0) goto out_free_conn; } + + if (conn_xpt_start(cli_conn < 0)) + goto out_free_conn; + sess = session_new(p, l, &cli_conn->obj_type); if (!sess) goto out_free_conn; diff --git a/src/session.c b/src/session.c index daf3995dd7..f78c2dfa4d 100644 --- a/src/session.c +++ b/src/session.c @@ -148,7 +148,9 @@ int session_accept_fd(struct connection *cli_conn) cli_conn->proxy_netns = l->rx.settings->netns; - conn_prepare(cli_conn, l->rx.proto, l->bind_conf->xprt); + if (conn_prepare(cli_conn, l->rx.proto, l->bind_conf->xprt) < 0) + goto out_free_conn; + conn_ctrl_init(cli_conn); /* wait for a PROXY protocol header */ @@ -159,9 +161,6 @@ int session_accept_fd(struct connection *cli_conn) if (l->options & LI_O_ACC_CIP) cli_conn->flags |= CO_FL_ACCEPT_CIP; - if (conn_xprt_init(cli_conn) < 0) - goto out_free_conn; - /* Add the handshake pseudo-XPRT */ if (cli_conn->flags & (CO_FL_ACCEPT_PROXY | CO_FL_ACCEPT_CIP)) { if (xprt_add_hs(cli_conn) != 0) @@ -182,6 +181,9 @@ int session_accept_fd(struct connection *cli_conn) ret = 0; /* successful termination */ goto out_free_sess; } + /* TCP rules may flag the connection as needing proxy protocol, now that it's done we can start ourxprt */ + if (conn_xprt_start(cli_conn) < 0) + goto out_free_conn; /* Adjust some socket options */ if (l->rx.addr.ss_family == AF_INET || l->rx.addr.ss_family == AF_INET6) { diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 1544397440..ec74a0a221 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -5226,9 +5226,6 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx) if (*xprt_ctx) return 0; - if (!conn_ctrl_ready(conn)) - return 0; - ctx = pool_alloc(ssl_sock_ctx_pool); if (!ctx) { conn->err_code = CO_ER_SSL_NO_MEM; diff --git a/src/tcpcheck.c b/src/tcpcheck.c index 55ecd2383e..90f1ee953a 100644 --- a/src/tcpcheck.c +++ b/src/tcpcheck.c @@ -1072,7 +1072,11 @@ enum tcpcheck_eval_ret tcpcheck_eval_connect(struct check *check, struct tcpchec ? xprt_get(XPRT_SSL) : ((connect->options & TCPCHK_OPT_DEFAULT_CONNECT) ? check->xprt : xprt_get(XPRT_RAW))); - conn_prepare(conn, proto, xprt); + if (conn_prepare(conn, proto, xprt) < 0) { + status = SF_ERR_RESOURCE; + goto fail_check; + } + cs_attach(cs, check, &check_conn_cb); if ((connect->options & TCPCHK_OPT_SOCKS4) && s && (s->flags & SRV_F_SOCKS4_PROXY)) { @@ -1132,6 +1136,11 @@ enum tcpcheck_eval_ret tcpcheck_eval_connect(struct check *check, struct tcpchec status = SF_ERR_RESOURCE; } + if (conn_xprt_start(conn) < 0) { + status = SF_ERR_RESOURCE; + goto fail_check; + } + /* The mux may be initialized now if there isn't server attached to the * check (email alerts) or if there is a mux proto specified or if there * is no alpn. diff --git a/src/xprt_handshake.c b/src/xprt_handshake.c index 81f65060b2..61d7c33df7 100644 --- a/src/xprt_handshake.c +++ b/src/xprt_handshake.c @@ -133,8 +133,6 @@ static int xprt_handshake_init(struct connection *conn, void **xprt_ctx) /* already initialized */ if (*xprt_ctx) return 0; - if (!conn_ctrl_ready(conn)) - return 0; ctx = pool_alloc(xprt_handshake_ctx_pool); if (!ctx) { diff --git a/src/xprt_quic.c b/src/xprt_quic.c index b447efe726..be15af90d2 100644 --- a/src/xprt_quic.c +++ b/src/xprt_quic.c @@ -4063,9 +4063,6 @@ static int qc_conn_init(struct connection *conn, void **xprt_ctx) if (*xprt_ctx) goto out; - if (!conn_ctrl_ready(conn)) - goto out; - ctx = pool_alloc(pool_head_quic_conn_ctx); if (!ctx) { conn->err_code = CO_ER_SYS_MEMLIM; -- 2.39.5