From 905a32998fc46eea222fff851aab69eea4ac8e8a Mon Sep 17 00:00:00 2001 From: Omar Polo Date: Sat, 3 Aug 2024 14:40:17 +0000 Subject: proxy-protocol: accept cross-family proxying Due to a strict interpretation of the spec if "TCP4" is used we expect two ipv4 addresses (and similar for "TCP6" and ipv6 addresses). However, the family specified in the proxy header matters only for the first address (the source), not the destination! After all, it's not strange to proxy from/to ipv4 and ipv6. Use getaddrinfo(NI_NUMERICHOST) to parse the IP addresses since inet_pton() is too strict. --- gmid.h | 13 +++---- proxy-proto.c | 97 +++++++++++++++++++++++++-------------------------- regress/fuzz/Makefile | 2 +- server.c | 33 +++++++++--------- 4 files changed, 72 insertions(+), 73 deletions(-) diff --git a/gmid.h b/gmid.h index 3a28c29..d68e0e3 100644 --- a/gmid.h +++ b/gmid.h @@ -290,12 +290,13 @@ enum proto { }; struct proxy_protocol_v1 { - enum proto proto; - union { - struct in_addr v4; - struct in6_addr v6; - } srcaddr, dstaddr; - uint16_t srcport, dstport; + enum proto proto; + struct sockaddr_storage srcaddr; + socklen_t srclen; + struct sockaddr_storage dstaddr; + socklen_t dstlen; + uint16_t srcport; + uint16_t dstport; }; #define BUFLAYER_MAX 108 diff --git a/proxy-proto.c b/proxy-proto.c index ecf885f..f7c00d1 100644 --- a/proxy-proto.c +++ b/proxy-proto.c @@ -19,6 +19,8 @@ #include #include +#include "log.h" + #define MIN(a, b) (a) < (b) ? (a) : (b) static int @@ -88,17 +90,29 @@ check_crlf_v1(char *const *buf, size_t buflen) } static int -check_ip_v1(int af, void *addr, char **buf) +check_ip_v1(int af, char **buf, struct sockaddr_storage *addr, socklen_t *len) { - char *spc; + struct addrinfo hints, *res; + char *spc; + int err; if ((spc = strchr(*buf, ' ')) == NULL) return (-1); *spc++ = '\0'; - if (inet_pton(af, *buf, addr) != 1) + memset(&hints, 0, sizeof(hints)); + hints.ai_family = af; + hints.ai_flags = AI_NUMERICHOST; + err = getaddrinfo(*buf, NULL, &hints, &res); + if (err) { + log_warnx("getaddrinfo(%s): %s", *buf, gai_strerror(err)); return (-1); + } + + memcpy(addr, res->ai_addr, res->ai_addrlen); + *len = res->ai_addrlen; + freeaddrinfo(res); *buf = spc; @@ -132,6 +146,7 @@ proxy_proto_v1_parse(struct proxy_protocol_v1 *s, char *buf, size_t buflen, size_t *consumed) { const char *begin = buf; + int af; if (check_crlf_v1(&buf, buflen) == -1 || check_prefix_v1(&buf) == -1) @@ -149,24 +164,13 @@ proxy_proto_v1_parse(struct proxy_protocol_v1 *s, char *buf, size_t buflen, return (-1); } - switch (s->proto) { - case PROTO_V4: - if (check_ip_v1(AF_INET, &s->srcaddr.v4, &buf) == -1 || - check_ip_v1(AF_INET, &s->dstaddr.v4, &buf) == -1) - return (-1); - break; + af = AF_INET; + if (s->proto == PROTO_V6) + af = AF_INET6; - case PROTO_V6: - if (check_ip_v1(AF_INET6, &s->srcaddr.v6, &buf) == -1 || - check_ip_v1(AF_INET6, &s->dstaddr.v6, &buf) == -1) - return (-1); - break; - - default: - return (-1); - } - - if (check_port_v1(&s->srcport, &buf) == -1 || + if (check_ip_v1(af, &buf, &s->srcaddr, &s->srclen) == -1 || + check_ip_v1(AF_UNSPEC, &buf, &s->dstaddr, &s->dstlen) == -1 || + check_port_v1(&s->srcport, &buf) == -1 || check_port_v1(&s->dstport, &buf) == -1) return (-1); @@ -182,36 +186,31 @@ int proxy_proto_v1_string(const struct proxy_protocol_v1 *s, char *buf, size_t buflen) { - // "0000:0000:0000:0000:0000:0000:0000:0000\0" - char srcaddrbuf[40], dstaddrbuf[40]; + char srcaddr[NI_MAXHOST], dstaddr[NI_MAXHOST]; int ret; - switch (s->proto) { - case PROTO_UNKNOWN: - ret = snprintf(buf, buflen, "unknown"); - goto fin; - case PROTO_V4: - inet_ntop(AF_INET, &s->srcaddr.v4, srcaddrbuf, - sizeof(srcaddrbuf)); - inet_ntop(AF_INET, &s->dstaddr.v4, dstaddrbuf, - sizeof(dstaddrbuf)); - break; - case PROTO_V6: - inet_ntop(AF_INET6, &s->srcaddr.v6, srcaddrbuf, - sizeof(srcaddrbuf)); - inet_ntop(AF_INET6, &s->dstaddr.v6, dstaddrbuf, - sizeof(dstaddrbuf)); - break; + + if (s->proto == PROTO_UNKNOWN) + return strlcpy(buf, "unknown", buflen); + + ret = getnameinfo((struct sockaddr *)&s->srcaddr, s->srclen, + srcaddr, sizeof(srcaddr), NULL, 0, + NI_NUMERICHOST); + if (ret) { + log_warnx("getnameinfo: %s", gai_strerror(ret)); + return (-1); } - ret = snprintf( - buf, - buflen, - "from %s port %u via %s port %u", - srcaddrbuf, - s->srcport, - dstaddrbuf, - s->dstport); - -fin: - return ret; + ret = getnameinfo((struct sockaddr *)&s->dstaddr, s->dstlen, + dstaddr, sizeof(dstaddr), NULL, 0, + NI_NUMERICHOST); + if (ret) { + log_warnx("getnameinfo: %s", gai_strerror(ret)); + return (-1); + } + + ret = snprintf(buf, buflen, "from %s port %u via %s port %u", + srcaddr, s->srcport, dstaddr, s->dstport); + if (ret < 0 || (size_t)ret >= buflen) + return (-1); + return (ret); } diff --git a/regress/fuzz/Makefile b/regress/fuzz/Makefile index e8906b3..339c031 100644 --- a/regress/fuzz/Makefile +++ b/regress/fuzz/Makefile @@ -11,7 +11,7 @@ REG_COMPATS = ${COBJS:%=../../%} IRI_SRCS = iri.c ../../iri.c ../../utf8.c ../../log.c IRI_OBJS = ${IRI_SRCS:.c=.o} ${REG_COMPATS} -PROXY_SRCS = proxy.c ../../proxy-proto.c +PROXY_SRCS = proxy.c ../../proxy-proto.c ../../log.c PROXY_OBJS = ${PROXY_SRCS:.c=.o} ${REG_COMPATS} .PHONY: all data clean dist diff --git a/server.c b/server.c index aed958d..296fb12 100644 --- a/server.c +++ b/server.c @@ -604,7 +604,8 @@ proxy_socket(struct client *c, struct proxy *p) to, sizeof(to), to_port, sizeof(to_port), NI_NUMERICHOST|NI_NUMERICSERV); if (err != 0) { - log_warnx("getnameinfo failed: %s", gai_strerror(err)); + log_warnx("%s: getnameinfo failed: %s", __func__, + gai_strerror(err)); strlcpy(to, c->rhost, sizeof(to)); strlcpy(to_port, c->rserv, sizeof(to_port)); } @@ -1324,7 +1325,7 @@ read_cb(struct tls *ctx, void *buf, size_t buflen, void *cb_arg) char protostr[1024]; ssize_t ret; size_t left, avail, copy, consumed; - int status; + int err, status; if (!c->proxy_proto) { /* no buffer to cache into, read into libtls buffer */ @@ -1370,22 +1371,19 @@ read_cb(struct tls *ctx, void *buf, size_t buflen, void *cb_arg) return -1; } - switch (pp1.proto) { - case PROTO_V4: - inet_ntop(AF_INET, &pp1.srcaddr.v4, c->rhost, - sizeof(c->rhost)); - break; - case PROTO_V6: - inet_ntop(AF_INET6, &pp1.srcaddr.v6, c->rhost, - sizeof(c->rhost)); - break; - case PROTO_UNKNOWN: + if (pp1.proto == PROTO_UNKNOWN) strlcpy(c->rhost, "UNKNOWN", sizeof(c->rhost)); - break; - } - - if (pp1.proto != PROTO_UNKNOWN) + else { + err = getnameinfo((struct sockaddr *)&pp1.srcaddr, pp1.srclen, + c->rhost, sizeof(c->rhost), NULL, 0, + NI_NUMERICHOST); + if (err) { + log_warn("%s: getnameinfo failed: %s", __func__, + gai_strerror(err)); + return -1; + } snprintf(c->rserv, sizeof(c->rserv), "%u", pp1.srcport); + } proxy_proto_v1_string(&pp1, protostr, sizeof(protostr)); log_debug("proxy-protocol v1: %s", protostr); @@ -1450,7 +1448,8 @@ server_accept(int sock, short et, void *d) e = getnameinfo(sraddr, len, c->rhost, sizeof(c->rhost), c->rserv, sizeof(c->rserv), NI_NUMERICHOST | NI_NUMERICSERV); if (e != 0) { - log_warnx("getnameinfo failed: %s", gai_strerror(e)); + log_warnx("%s: getnameinfo failed: %s", __func__, + gai_strerror(e)); close(c->fd); free(c); return; -- cgit v1.2.3