From f804c087e41d47aad0999984d35e52672f4dccaa Mon Sep 17 00:00:00 2001 From: George Thessalonikefs Date: Fri, 29 Sep 2023 17:31:52 +0200 Subject: [PATCH] proxy-protocol, review comments: - more generic switch statement for address families; - comment the protocol values as such in their definitions; - less hardcoded values for address family and protocol combinations. --- util/netevent.c | 12 ++++++------ util/proxy_protocol.c | 27 ++++++++++++++++----------- util/proxy_protocol.h | 21 +++++++++++++++++---- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/util/netevent.c b/util/netevent.c index edb9da8b5..6a455e858 100644 --- a/util/netevent.c +++ b/util/netevent.c @@ -772,7 +772,7 @@ static int consume_pp2_header(struct sldns_buffer* buf, struct comm_reply* rep, * No need to do anything with addresses. */ goto done; } - if(header->fam_prot == 0x00) { + if(header->fam_prot == PP2_UNSPEC_UNSPEC) { /* Unspecified family and protocol. This could be used for * health checks by proxies. * No need to do anything with addresses. */ @@ -780,8 +780,8 @@ static int consume_pp2_header(struct sldns_buffer* buf, struct comm_reply* rep, } /* Read the proxied address */ switch(header->fam_prot) { - case 0x11: /* AF_INET|STREAM */ - case 0x12: /* AF_INET|DGRAM */ + case PP2_INET_STREAM: + case PP2_INET_DGRAM: { struct sockaddr_in* addr = (struct sockaddr_in*)&rep->client_addr; @@ -792,8 +792,8 @@ static int consume_pp2_header(struct sldns_buffer* buf, struct comm_reply* rep, } /* Ignore the destination address; it should be us. */ break; - case 0x21: /* AF_INET6|STREAM */ - case 0x22: /* AF_INET6|DGRAM */ + case PP2_INET6_STREAM: + case PP2_INET6_DGRAM: { struct sockaddr_in6* addr = (struct sockaddr_in6*)&rep->client_addr; @@ -808,7 +808,7 @@ static int consume_pp2_header(struct sldns_buffer* buf, struct comm_reply* rep, break; default: log_err("proxy_protocol: unsupported family and " - "protocol"); + "protocol 0x%x", (int)header->fam_prot); return 0; } rep->is_proxied = 1; diff --git a/util/proxy_protocol.c b/util/proxy_protocol.c index 03db06037..a18804974 100644 --- a/util/proxy_protocol.c +++ b/util/proxy_protocol.c @@ -105,7 +105,8 @@ pp2_write_to_buf(uint8_t* buf, size_t buflen, /* version and command */ *buf = (PP2_VERSION << 4) | PP2_CMD_PROXY; buf++; - if(af==AF_INET) { + switch(af) { + case AF_INET: /* family and protocol */ *buf = (PP2_AF_INET<<4) | (stream?PP2_PROT_STREAM:PP2_PROT_DGRAM); @@ -127,8 +128,9 @@ pp2_write_to_buf(uint8_t* buf, size_t buflen, /* dst addr */ /* dst port */ (*pp_data.write_uint16)(buf, 12); - } else { + break; #ifdef INET6 + case AF_INET6: /* family and protocol */ *buf = (PP2_AF_INET6<<4) | (stream?PP2_PROT_STREAM:PP2_PROT_DGRAM); @@ -148,9 +150,12 @@ pp2_write_to_buf(uint8_t* buf, size_t buflen, buf += 2; /* dst port */ (*pp_data.write_uint16)(buf, 0); -#else - return 0; + break; #endif /* INET6 */ + case AF_UNIX: + /* fallthrough */ + default: + return 0; } return expected_size; } @@ -180,13 +185,13 @@ pp2_read_header(uint8_t* buf, size_t buflen) return PP_PARSE_UNKNOWN_CMD; } /* Check for supported family and protocol */ - if(header->fam_prot != 0x00 /* AF_UNSPEC|UNSPEC */ && - header->fam_prot != 0x11 /* AF_INET|STREAM */ && - header->fam_prot != 0x12 /* AF_INET|DGRAM */ && - header->fam_prot != 0x21 /* AF_INET6|STREAM */ && - header->fam_prot != 0x22 /* AF_INET6|DGRAM */ && - header->fam_prot != 0x31 /* AF_UNIX|STREAM */ && - header->fam_prot != 0x32 /* AF_UNIX|DGRAM */) { + if(header->fam_prot != PP2_UNSPEC_UNSPEC && + header->fam_prot != PP2_INET_STREAM && + header->fam_prot != PP2_INET_DGRAM && + header->fam_prot != PP2_INET6_STREAM && + header->fam_prot != PP2_INET6_DGRAM && + header->fam_prot != PP2_UNIX_STREAM && + header->fam_prot != PP2_UNIX_DGRAM) { return PP_PARSE_UNKNOWN_FAM_PROT; } /* We have a correct header */ diff --git a/util/proxy_protocol.h b/util/proxy_protocol.h index 58d3f8d57..ca81065bf 100644 --- a/util/proxy_protocol.h +++ b/util/proxy_protocol.h @@ -51,11 +51,11 @@ #define PP2_SIG "\x0D\x0A\x0D\x0A\x00\x0D\x0A\x51\x55\x49\x54\x0A" #define PP2_SIG_LEN 12 -/** PROXYv2 version */ +/** PROXYv2 version (protocol value) */ #define PP2_VERSION 0x2 /** - * PROXYv2 command. + * PROXYv2 command (protocol value). */ enum pp2_command { PP2_CMD_LOCAL = 0x0, @@ -63,7 +63,7 @@ enum pp2_command { }; /** - * PROXYv2 address family. + * PROXYv2 address family (protocol value). */ enum pp2_af { PP2_AF_UNSPEC = 0x0, @@ -73,7 +73,7 @@ enum pp2_af { }; /** - * PROXYv2 protocol. + * PROXYv2 protocol (protocol value). */ enum pp2_protocol { PP2_PROT_UNSPEC = 0x0, @@ -81,6 +81,19 @@ enum pp2_protocol { PP2_PROT_DGRAM = 0x2 }; +/** + * Expected combinations of address family and protocol values used in checks. + */ +enum pp2_af_protocol_combination { + PP2_UNSPEC_UNSPEC = (PP2_AF_UNSPEC<<4)|PP2_PROT_UNSPEC, + PP2_INET_STREAM = (PP2_AF_INET<<4)|PP2_PROT_STREAM, + PP2_INET_DGRAM = (PP2_AF_INET<<4)|PP2_PROT_DGRAM, + PP2_INET6_STREAM = (PP2_AF_INET6<<4)|PP2_PROT_STREAM, + PP2_INET6_DGRAM = (PP2_AF_INET6<<4)|PP2_PROT_DGRAM, + PP2_UNIX_STREAM = (PP2_AF_UNIX<<4)|PP2_PROT_STREAM, + PP2_UNIX_DGRAM = (PP2_AF_UNIX<<4)|PP2_PROT_DGRAM +}; + /** * PROXYv2 header. */ -- 2.39.5