Welcome! Log In Create A New Profile

Advanced

[PATCH] MEDIUM: proxy_protocol: Send IPv4 addresses when possible

Posted by Tim Duesterhus 
This patch changes the sending side of proxy protocol to convert IP
addresses to IPv4 when possible (and converts them IPv6 otherwise).

Previously the code failed to properly provide information under
certain circumstances:

1. haproxy is being accessed using IPv4, http-request set-src sets
a IPv6 address.
2. haproxy is being accessed using IPv6, http-request set-src sets
a IPv4 address.
3. haproxy listens on `::` with v4v6 and is accessed using IPv4:
It would send a TCP6 line, instead of a proper TCP4 line, because
the IP addresses are representing as a mapped IPv4 address internally.

Once correctness of this patch has been verified it should be evaluated
whether it should be backported, as (1) and (2) are bugs. (3) is an
enhancement.
---
include/common/standard.h | 6 ++++++
include/proto/connection.h | 2 +-
src/connection.c | 29 ++++++++++++++++++++++++-----
src/standard.c | 21 +++++++++++++++++++++
4 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/include/common/standard.h b/include/common/standard.h
index 6542759d..eb92b22b 100644
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -1047,6 +1047,12 @@ extern void v4tov6(struct in6_addr *sin6_addr, struct in_addr *sin_addr);
*/
extern int v6tov4(struct in_addr *sin_addr, struct in6_addr *sin6_addr);

+/* Calls v4tov6 on the addr in v4. Copies v4 to v6 v6 already is of type AF_INET6 */
+extern void sockaddr_v4tov6(struct sockaddr_storage *v6, struct sockaddr_storage *v4);
+
+/* Calls v6tov4 on the addr in v6. Copies v6 to v4 if conversion fails or v6 already is of type AF_INET */
+extern int sockaddr_v6tov4(struct sockaddr_storage *v4, struct sockaddr_storage *v6);
+
/* compare two struct sockaddr_storage and return:
* 0 (true) if the addr is the same in both
* 1 (false) if the addr is not the same in both
diff --git a/include/proto/connection.h b/include/proto/connection.h
index 8566736f..4e19756b 100644
--- a/include/proto/connection.h
+++ b/include/proto/connection.h
@@ -46,7 +46,7 @@ void conn_fd_handler(int fd);
/* receive a PROXY protocol header over a connection */
int conn_recv_proxy(struct connection *conn, int flag);
int make_proxy_line(char *buf, int buf_len, struct server *srv, struct connection *remote);
-int make_proxy_line_v1(char *buf, int buf_len, struct sockaddr_storage *src, struct sockaddr_storage *dst);
+int make_proxy_line_v1(char *buf, int buf_len, struct connection *remote);
int make_proxy_line_v2(char *buf, int buf_len, struct server *srv, struct connection *remote);

/* receive a NetScaler Client IP insertion header over a connection */
diff --git a/src/connection.c b/src/connection.c
index 1ea96ae3..e8e48d69 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -888,14 +888,24 @@ int make_proxy_line(char *buf, int buf_len, struct server *srv, struct connectio
{
int ret = 0;

+ struct connection tmp;
+ if (remote) {
+ memcpy(&tmp, remote, sizeof(tmp));
+
+ if (!sockaddr_v6tov4(&tmp.addr.from, &remote->addr.from) || !sockaddr_v6tov4(&tmp.addr.to, &remote->addr.to)) {
+ sockaddr_v4tov6(&tmp.addr.from, &remote->addr.from);
+ sockaddr_v4tov6(&tmp.addr.to, &remote->addr.to);
+ }
+
+ remote = &tmp;
+ }
+
+
if (srv && (srv->pp_opts & SRV_PP_V2)) {
ret = make_proxy_line_v2(buf, buf_len, srv, remote);
}
else {
- if (remote)
- ret = make_proxy_line_v1(buf, buf_len, &remote->addr.from, &remote->addr.to);
- else
- ret = make_proxy_line_v1(buf, buf_len, NULL, NULL);
+ ret = make_proxy_line_v1(buf, buf_len, remote);
}

return ret;
@@ -908,10 +918,19 @@ int make_proxy_line(char *buf, int buf_len, struct server *srv, struct connectio
* TCP6 and "UNKNOWN" formats. If any of <src> or <dst> is null, UNKNOWN is
* emitted as well.
*/
-int make_proxy_line_v1(char *buf, int buf_len, struct sockaddr_storage *src, struct sockaddr_storage *dst)
+int make_proxy_line_v1(char *buf, int buf_len, struct connection *remote)
{
int ret = 0;

+ struct sockaddr_storage null_addr = { .ss_family = 0 };
+ struct sockaddr_storage *src = &null_addr;
+ struct sockaddr_storage *dst = &null_addr;
+
+ if (remote) {
+ src = &remote->addr.from;
+ dst = &remote->addr.to;
+ }
+
if (src && dst && src->ss_family == dst->ss_family && src->ss_family == AF_INET) {
ret = snprintf(buf + ret, buf_len - ret, "PROXY TCP4 ");
if (ret >= buf_len)
diff --git a/src/standard.c b/src/standard.c
index ebe043f1..51fd2cc4 100644
--- a/src/standard.c
+++ b/src/standard.c
@@ -2693,6 +2693,27 @@ int v6tov4(struct in_addr *sin_addr, struct in6_addr *sin6_addr)
return 0;
}

+void sockaddr_v4tov6(struct sockaddr_storage *v6, struct sockaddr_storage *v4) {
+ if (v4->ss_family == AF_INET) {
+ v6->ss_family = AF_INET6;
+ v4tov6(&((struct sockaddr_in6 *) v6)->sin6_addr, &((struct sockaddr_in *) v4)->sin_addr);
+ return;
+ }
+
+ memcpy(v6, v4, sizeof(struct sockaddr_storage));
+}
+
+int sockaddr_v6tov4(struct sockaddr_storage *v4, struct sockaddr_storage *v6) {
+ if (v6->ss_family == AF_INET6 && v6tov4(&((struct sockaddr_in *) v4)->sin_addr, &((struct sockaddr_in6 *) v6)->sin6_addr)) {
+ v4->ss_family = AF_INET;
+ return 1;
+ }
+
+ memcpy(v4, v6, sizeof(struct sockaddr_storage));
+
+ return v4->ss_family == AF_INET;
+}
+
/* compare two struct sockaddr_storage and return:
* 0 (true) if the addr is the same in both
* 1 (false) if the addr is not the same in both
--
2.18.0
Willy,

Am 29.06.2018 um 20:59 schrieb Tim Duesterhus:
> This patch changes the sending side of proxy protocol to convert IP
> addresses to IPv4 when possible (and converts them IPv6 otherwise).
>
> Previously the code failed to properly provide information under
> certain circumstances:
>
> 1. haproxy is being accessed using IPv4, http-request set-src sets
> a IPv6 address.
> 2. haproxy is being accessed using IPv6, http-request set-src sets
> a IPv4 address.
> 3. haproxy listens on `::` with v4v6 and is accessed using IPv4:
> It would send a TCP6 line, instead of a proper TCP4 line, because
> the IP addresses are representing as a mapped IPv4 address internally.
>
> Once correctness of this patch has been verified it should be evaluated
> whether it should be backported, as (1) and (2) are bugs. (3) is an
> enhancement.

based on the overall lack of responses I assume that you are busy. I
just want to make sure that this patch / bug report did not slip through
the cracks. A short acknowledgement that you received it would be great,
if you are currently unable to take a deeper look at it.

Best regards
Tim Düsterhus
Hello Tim,


On Fri, 29 Jun 2018 at 21:00, Tim Duesterhus <[email protected]> wrote:
>
> This patch changes the sending side of proxy protocol to convert IP
> addresses to IPv4 when possible (and converts them IPv6 otherwise).
>
> Previously the code failed to properly provide information under
> certain circumstances:
>
> 1. haproxy is being accessed using IPv4, http-request set-src sets
> a IPv6 address.
> 2. haproxy is being accessed using IPv6, http-request set-src sets
> a IPv4 address.
> 3. haproxy listens on `::` with v4v6 and is accessed using IPv4:
> It would send a TCP6 line, instead of a proper TCP4 line, because
> the IP addresses are representing as a mapped IPv4 address internally.
>
> Once correctness of this patch has been verified it should be evaluated
> whether it should be backported, as (1) and (2) are bugs. (3) is an
> enhancement.

Thanks for this, just a comment about nr 3:

A backend may rely on v4-mapped addresses for various reason, consider
a backend that to simplify its handling of IP addresses only handles
IPv6 and expects IPv4 addresses to be mapped.
Also consider that to send native v4 addresses the admin only has to
make a small adjustment in the bind configuration.

So since this would be a breaking change, and that the admin can
easily reconfigure the bind line any time, I would advise against this
and vote for maintaining the current behavior (where the bind
configuration controls this behavior).

I assume the X-Forwarded-For header behaves similar in this regard.



cheers,
lukas
Sorry, only registered users may post in this forum.

Click here to login