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

@@ -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

proxy-proto.c

@@ -19,6 +19,8 @@
 #include <stdint.h>
 #include <string.h>
 
+#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);
 }

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

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;