Willy,

attached is an updated patch that:

1. Only converts the addresses to IPv6 if at least one of them is IPv6.
   But it does not convert them to IPv4 if both of them can be converted to 
IPv4.
2. Does not copy the whole `struct connection`, but performs the conversion 
inside
   `make_proxy_line_v?`.

I'm not sure whether I like this better than my first attempt at it. Proxy 
protocol
v2 was rather easy to modify, but proxy protocol v1 required a complete 
restructuring
to not create a new case for each of the 4 address combinations (44, 46, 64, 
66).

I performed a manual test running using both send-proxy as well as 
send-proxy-v2 inside
of valgrind. It sent the expected values. Valgrind did not report any memory 
corruption
or memory leaks.

So I believe this patch is good, but you want to double check my logic. 
Especially inside
`make_proxy_line_v1`.

Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --

http-request set-src possibly creates a situation where src and dst
are from different address families. Convert both addresses to IPv6
to avoid a PROXY UNKNOWN.

This patch should be backported to haproxy 1.8.
---
 src/connection.c | 173 +++++++++++++++++++++++++++--------------------
 1 file changed, 98 insertions(+), 75 deletions(-)

diff --git a/src/connection.c b/src/connection.c
index 4b1e066e..8826706f 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -964,73 +964,71 @@ int make_proxy_line(char *buf, int buf_len, struct server 
*srv, struct connectio
 int make_proxy_line_v1(char *buf, int buf_len, struct sockaddr_storage *src, 
struct sockaddr_storage *dst)
 {
        int ret = 0;
-
-       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)
-                       return 0;
-
-               /* IPv4 src */
-               if (!inet_ntop(src->ss_family, &((struct sockaddr_in 
*)src)->sin_addr, buf + ret, buf_len - ret))
-                       return 0;
-
-               ret += strlen(buf + ret);
+       char * protocol;
+       char src_str[MAX(INET_ADDRSTRLEN, INET6_ADDRSTRLEN)];
+       char dst_str[MAX(INET_ADDRSTRLEN, INET6_ADDRSTRLEN)];
+       in_port_t src_port;
+       in_port_t dst_port;
+
+       if (   !src
+           || !dst
+           || (src->ss_family != AF_INET && src->ss_family != AF_INET6)
+           || (dst->ss_family != AF_INET && dst->ss_family != AF_INET6)) {
+               /* unknown family combination */
+               ret = snprintf(buf, buf_len, "PROXY UNKNOWN\r\n");
                if (ret >= buf_len)
                        return 0;
 
-               buf[ret++] = ' ';
-
-               /* IPv4 dst */
-               if (!inet_ntop(dst->ss_family, &((struct sockaddr_in 
*)dst)->sin_addr, buf + ret, buf_len - ret))
-                       return 0;
+               return ret;
+       }
 
-               ret += strlen(buf + ret);
-               if (ret >= buf_len)
+       /* IPv4 for both src and dst */
+       if (src->ss_family == AF_INET && dst->ss_family == AF_INET) {
+               protocol = "TCP4";
+               if (!inet_ntop(AF_INET, &((struct sockaddr_in *)src)->sin_addr, 
src_str, sizeof(src_str)))
                        return 0;
-
-               /* source and destination ports */
-               ret += snprintf(buf + ret, buf_len - ret, " %u %u\r\n",
-                               ntohs(((struct sockaddr_in *)src)->sin_port),
-                               ntohs(((struct sockaddr_in *)dst)->sin_port));
-               if (ret >= buf_len)
+               src_port = ((struct sockaddr_in *)src)->sin_port;
+               if (!inet_ntop(AF_INET, &((struct sockaddr_in *)dst)->sin_addr, 
dst_str, sizeof(dst_str)))
                        return 0;
+               dst_port = ((struct sockaddr_in *)dst)->sin_port;
        }
-       else if (src && dst && src->ss_family == dst->ss_family && 
src->ss_family == AF_INET6) {
-               ret = snprintf(buf + ret, buf_len - ret, "PROXY TCP6 ");
-               if (ret >= buf_len)
-                       return 0;
-
-               /* IPv6 src */
-               if (!inet_ntop(src->ss_family, &((struct sockaddr_in6 
*)src)->sin6_addr, buf + ret, buf_len - ret))
-                       return 0;
+       /* IPv6 for at least one of src and dst */
+       else {
+               struct in6_addr tmp;
 
-               ret += strlen(buf + ret);
-               if (ret >= buf_len)
-                       return 0;
+               protocol = "TCP6";
 
-               buf[ret++] = ' ';
+               if (src->ss_family == AF_INET) {
+                       /* Convert src to IPv6 */
+                       v4tov6(&tmp, &((struct sockaddr_in *)src)->sin_addr);
+                       src_port = ((struct sockaddr_in *)src)->sin_port;
+               }
+               else {
+                       tmp = ((struct sockaddr_in6 *)src)->sin6_addr;
+                       src_port = ((struct sockaddr_in6 *)src)->sin6_port;
+               }
 
-               /* IPv6 dst */
-               if (!inet_ntop(dst->ss_family, &((struct sockaddr_in6 
*)dst)->sin6_addr, buf + ret, buf_len - ret))
+               if (!inet_ntop(AF_INET6, &tmp, src_str, sizeof(src_str)))
                        return 0;
 
-               ret += strlen(buf + ret);
-               if (ret >= buf_len)
-                       return 0;
+               if (dst->ss_family == AF_INET) {
+                       /* Convert dst to IPv6 */
+                       v4tov6(&tmp, &((struct sockaddr_in *)dst)->sin_addr);
+                       dst_port = ((struct sockaddr_in *)dst)->sin_port;
+               }
+               else {
+                       tmp = ((struct sockaddr_in6 *)dst)->sin6_addr;
+                       dst_port = ((struct sockaddr_in6 *)dst)->sin6_port;
+               }
 
-               /* source and destination ports */
-               ret += snprintf(buf + ret, buf_len - ret, " %u %u\r\n",
-                               ntohs(((struct sockaddr_in6 *)src)->sin6_port),
-                               ntohs(((struct sockaddr_in6 *)dst)->sin6_port));
-               if (ret >= buf_len)
-                       return 0;
-       }
-       else {
-               /* unknown family combination */
-               ret = snprintf(buf, buf_len, "PROXY UNKNOWN\r\n");
-               if (ret >= buf_len)
+               if (!inet_ntop(AF_INET6, &tmp, dst_str, sizeof(dst_str)))
                        return 0;
        }
+
+       ret = snprintf(buf, buf_len, "PROXY %s %s %s %u %u\r\n", protocol, 
src_str, dst_str, ntohs(src_port), ntohs(dst_port));
+       if (ret >= buf_len)
+               return 0;
+
        return ret;
 }
 
@@ -1071,35 +1069,60 @@ int make_proxy_line_v2(char *buf, int buf_len, struct 
server *srv, struct connec
                dst = &remote->addr.to;
        }
 
-       if (src && dst && src->ss_family == dst->ss_family && src->ss_family == 
AF_INET) {
-               if (buf_len < PP2_HDR_LEN_INET)
-                       return 0;
-               hdr->ver_cmd = PP2_VERSION | PP2_CMD_PROXY;
-               hdr->fam = PP2_FAM_INET | PP2_TRANS_STREAM;
-               hdr->addr.ip4.src_addr = ((struct sockaddr_in 
*)src)->sin_addr.s_addr;
-               hdr->addr.ip4.dst_addr = ((struct sockaddr_in 
*)dst)->sin_addr.s_addr;
-               hdr->addr.ip4.src_port = ((struct sockaddr_in *)src)->sin_port;
-               hdr->addr.ip4.dst_port = ((struct sockaddr_in *)dst)->sin_port;
-               ret = PP2_HDR_LEN_INET;
-       }
-       else if (src && dst && src->ss_family == dst->ss_family && 
src->ss_family == AF_INET6) {
-               if (buf_len < PP2_HDR_LEN_INET6)
-                       return 0;
-               hdr->ver_cmd = PP2_VERSION | PP2_CMD_PROXY;
-               hdr->fam = PP2_FAM_INET6 | PP2_TRANS_STREAM;
-               memcpy(hdr->addr.ip6.src_addr, &((struct sockaddr_in6 
*)src)->sin6_addr, 16);
-               memcpy(hdr->addr.ip6.dst_addr, &((struct sockaddr_in6 
*)dst)->sin6_addr, 16);
-               hdr->addr.ip6.src_port = ((struct sockaddr_in6 
*)src)->sin6_port;
-               hdr->addr.ip6.dst_port = ((struct sockaddr_in6 
*)dst)->sin6_port;
-               ret = PP2_HDR_LEN_INET6;
-       }
-       else {
+       /* At least one of src or dst is not of AF_INET or AF_INET6 */
+       if (  !src
+          || !dst
+          || (src->ss_family != AF_INET && src->ss_family != AF_INET6)
+          || (dst->ss_family != AF_INET && dst->ss_family != AF_INET6)) {
                if (buf_len < PP2_HDR_LEN_UNSPEC)
                        return 0;
                hdr->ver_cmd = PP2_VERSION | PP2_CMD_LOCAL;
                hdr->fam = PP2_FAM_UNSPEC | PP2_TRANS_UNSPEC;
                ret = PP2_HDR_LEN_UNSPEC;
        }
+       else {
+               /* IPv4 for both src and dst */
+               if (src->ss_family == AF_INET && dst->ss_family == AF_INET) {
+                       if (buf_len < PP2_HDR_LEN_INET)
+                               return 0;
+                       hdr->ver_cmd = PP2_VERSION | PP2_CMD_PROXY;
+                       hdr->fam = PP2_FAM_INET | PP2_TRANS_STREAM;
+                       hdr->addr.ip4.src_addr = ((struct sockaddr_in 
*)src)->sin_addr.s_addr;
+                       hdr->addr.ip4.src_port = ((struct sockaddr_in 
*)src)->sin_port;
+                       hdr->addr.ip4.dst_addr = ((struct sockaddr_in 
*)dst)->sin_addr.s_addr;
+                       hdr->addr.ip4.dst_port = ((struct sockaddr_in 
*)dst)->sin_port;
+                       ret = PP2_HDR_LEN_INET;
+               }
+               /* IPv6 for at least one of src and dst */
+               else {
+                       struct in6_addr tmp;
+
+                       if (buf_len < PP2_HDR_LEN_INET6)
+                               return 0;
+                       hdr->ver_cmd = PP2_VERSION | PP2_CMD_PROXY;
+                       hdr->fam = PP2_FAM_INET6 | PP2_TRANS_STREAM;
+                       if (src->ss_family == AF_INET) {
+                               v4tov6(&tmp, &((struct sockaddr_in 
*)src)->sin_addr);
+                               memcpy(hdr->addr.ip6.src_addr, &tmp, 16);
+                               hdr->addr.ip6.src_port = ((struct sockaddr_in 
*)src)->sin_port;
+                       }
+                       else {
+                               memcpy(hdr->addr.ip6.src_addr, &((struct 
sockaddr_in6 *)src)->sin6_addr, 16);
+                               hdr->addr.ip6.src_port = ((struct sockaddr_in6 
*)src)->sin6_port;
+                       }
+                       if (dst->ss_family == AF_INET) {
+                               v4tov6(&tmp, &((struct sockaddr_in 
*)dst)->sin_addr);
+                               memcpy(hdr->addr.ip6.dst_addr, &tmp, 16);
+                               hdr->addr.ip6.src_port = ((struct sockaddr_in 
*)src)->sin_port;
+                       }
+                       else {
+                               memcpy(hdr->addr.ip6.dst_addr, &((struct 
sockaddr_in6 *)dst)->sin6_addr, 16);
+                               hdr->addr.ip6.dst_port = ((struct sockaddr_in6 
*)dst)->sin6_port;
+                       }
+
+                       ret = PP2_HDR_LEN_INET6;
+               }
+       }
 
        if (srv->pp_opts & SRV_PP_V2_CRC32C) {
                uint32_t zero_crc32c = 0;
-- 
2.18.0


Reply via email to