PROXY: fix v2 protocol decode. Bugs 2003, 1747

10 files changed, 307 insertions, 54 deletions

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index 75f28ef67..4b1e5af3e 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -38489,9 +38489,9 @@ To include this support, include &"SUPPORT_PROXY=yes"&
 in Local/Makefile.
 
 It was built on specifications from:
-http://haproxy.1wt.eu/download/1.5/doc/proxy-protocol.txt
+(&url(http://haproxy.1wt.eu/download/1.5/doc/proxy-protocol.txt)).
 That URL was revised in May 2014 to version 2 spec:
-http://git.1wt.eu/web?p=haproxy.git;a=commitdiff;h=afb768340c9d7e50d8e
+(&url(http://git.1wt.eu/web?p=haproxy.git;a=commitdiff;h=afb768340c9d7e50d8e)).
 
 The purpose of this facility is so that an application load balancer,
 such as HAProxy, can sit in front of several Exim servers
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 7e02d30bc..8ef8b0b6c 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -16,6 +16,9 @@ JH/02 The path option on a pipe transport is now expanded before use.
 PP/01 GitHub PR 50: Do not call ldap_start_tls_s on ldapi:// connections.
       Patch provided by "Björn", documentation fix added too.
 
+JH/03 Bug 2003: fix Proxy Protocol v2 handling: the address size field was
+      missing a wire-to-host endian conversion.
+
 
 Exim version 4.88
 -----------------
diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index 148486161..0935d212b 100644
--- a/src/src/smtp_in.c
+++ b/src/src/smtp_in.c
@@ -878,17 +878,15 @@ do
 if (ret == -1)
   goto proxyfail;
 
-if (ret >= 16 &&
-    memcmp(&hdr.v2, v2sig, 12) == 0)
+if (ret >= 16 && memcmp(&hdr.v2, v2sig, 12) == 0)
   {
-  uint8_t ver, cmd;
+  uint8_t ver = (hdr.v2.ver_cmd & 0xf0) >> 4;
+  uint8_t cmd = (hdr.v2.ver_cmd & 0x0f);
 
   /* May 2014: haproxy combined the version and command into one byte to
      allow two full bytes for the length field in order to proxy SSL
      connections.  SSL Proxy is not supported in this version of Exim, but
      must still seperate values here. */
-  ver = (hdr.v2.ver_cmd & 0xf0) >> 4;
-  cmd = (hdr.v2.ver_cmd & 0x0f);
 
   if (ver != 0x02)
     {
@@ -897,7 +895,7 @@ if (ret >= 16 &&
     }
   DEBUG(D_receive) debug_printf("Detected PROXYv2 header\n");
   /* The v2 header will always be 16 bytes per the spec. */
-  size = 16 + hdr.v2.len;
+  size = 16 + ntohs(hdr.v2.len);
   if (ret < size)
     {
     DEBUG(D_receive) debug_printf("Truncated or too large PROXYv2 header (%d/%d)\n",
@@ -912,8 +910,8 @@ if (ret >= 16 &&
         case 0x11:  /* TCPv4 address type */
           iptype = US"IPv4";
           tmpaddr.sin_addr.s_addr = hdr.v2.addr.ip4.src_addr;
-          inet_ntop(AF_INET, &(tmpaddr.sin_addr), (char *)&tmpip, sizeof(tmpip));
-          if (!string_is_ip_address(US tmpip,NULL))
+          inet_ntop(AF_INET, &tmpaddr.sin_addr, CS &tmpip, sizeof(tmpip));
+          if (!string_is_ip_address(US tmpip, NULL))
             {
             DEBUG(D_receive) debug_printf("Invalid %s source IP\n", iptype);
             goto proxyfail;
@@ -925,8 +923,8 @@ if (ret >= 16 &&
           sender_host_port    = tmpport;
           /* Save dest ip/port */
           tmpaddr.sin_addr.s_addr = hdr.v2.addr.ip4.dst_addr;
-          inet_ntop(AF_INET, &(tmpaddr.sin_addr), (char *)&tmpip, sizeof(tmpip));
-          if (!string_is_ip_address(US tmpip,NULL))
+          inet_ntop(AF_INET, &tmpaddr.sin_addr, CS &tmpip, sizeof(tmpip));
+          if (!string_is_ip_address(US tmpip, NULL))
             {
             DEBUG(D_receive) debug_printf("Invalid %s dest port\n", iptype);
             goto proxyfail;
@@ -938,8 +936,8 @@ if (ret >= 16 &&
         case 0x21:  /* TCPv6 address type */
           iptype = US"IPv6";
           memmove(tmpaddr6.sin6_addr.s6_addr, hdr.v2.addr.ip6.src_addr, 16);
-          inet_ntop(AF_INET6, &(tmpaddr6.sin6_addr), (char *)&tmpip6, sizeof(tmpip6));
-          if (!string_is_ip_address(US tmpip6,NULL))
+          inet_ntop(AF_INET6, &tmpaddr6.sin6_addr, CS &tmpip6, sizeof(tmpip6));
+          if (!string_is_ip_address(US tmpip6, NULL))
             {
             DEBUG(D_receive) debug_printf("Invalid %s source IP\n", iptype);
             goto proxyfail;
@@ -951,8 +949,8 @@ if (ret >= 16 &&
           sender_host_port    = tmpport;
           /* Save dest ip/port */
           memmove(tmpaddr6.sin6_addr.s6_addr, hdr.v2.addr.ip6.dst_addr, 16);
-          inet_ntop(AF_INET6, &(tmpaddr6.sin6_addr), (char *)&tmpip6, sizeof(tmpip6));
-          if (!string_is_ip_address(US tmpip6,NULL))
+          inet_ntop(AF_INET6, &tmpaddr6.sin6_addr, CS &tmpip6, sizeof(tmpip6));
+          if (!string_is_ip_address(US tmpip6, NULL))
             {
             DEBUG(D_receive) debug_printf("Invalid %s dest port\n", iptype);
             goto proxyfail;
@@ -978,8 +976,7 @@ if (ret >= 16 &&
       goto proxyfail;
     }
   }
-else if (ret >= 8 &&
-         memcmp(hdr.v1.line, "PROXY", 5) == 0)
+else if (ret >= 8 && memcmp(hdr.v1.line, "PROXY", 5) == 0)
   {
   uschar *p = string_copy(hdr.v1.line);
   uschar *end = memchr(p, '\r', ret - 1);
@@ -1032,7 +1029,7 @@ else if (ret >= 8 &&
     goto proxyfail;
     }
   *sp = '\0';
-  if(!string_is_ip_address(p,NULL))
+  if(!string_is_ip_address(p, NULL))
     {
     DEBUG(D_receive)
       debug_printf("Proxied src arg is not an %s address\n", iptype);
@@ -1048,7 +1045,7 @@ else if (ret >= 8 &&
     goto proxyfail;
     }
   *sp = '\0';
-  if(!string_is_ip_address(p,NULL))
+  if(!string_is_ip_address(p, NULL))
     {
     DEBUG(D_receive)
       debug_printf("Proxy dest arg is not an %s address\n", iptype);
@@ -1062,7 +1059,7 @@ else if (ret >= 8 &&
     goto proxyfail;
     }
   *sp = '\0';
-  tmp_port = strtol(CCS p,&endc,10);
+  tmp_port = strtol(CCS p, &endc, 10);
   if (*endc || tmp_port == 0)
     {
     DEBUG(D_receive)
@@ -1077,7 +1074,7 @@ else if (ret >= 8 &&
     DEBUG(D_receive) debug_printf("Did not find proxy dest port\n");
     goto proxyfail;
     }
-  tmp_port = strtol(CCS p,&endc,10);
+  tmp_port = strtol(CCS p, &endc, 10);
   if (*endc || tmp_port == 0)
     {
     DEBUG(D_receive)
diff --git a/test/README b/test/README
index 1a300663b..a600756be 100644
--- a/test/README
+++ b/test/README
@@ -1023,9 +1023,15 @@ Lines in client scripts are of two kinds:
 (2) If a line starts with three plus signs followed by a space, the rest of the
     line specifies a number of seconds to sleep for before proceeding.
 
-(3) Otherwise, the line is an input line line that is sent to the server. Any
+(3) If a line begins with three '>' characters and a space, the rest of the
+    line is input to be sent to the server.  Backslash escaping is done as
+    described below, but no trailing "\r\n" is sent.
+
+(4) Otherwise, the line is an input line line that is sent to the server. Any
     occurrences of \r and \n in the line are turned into carriage return and
     linefeed, respectively. This is used for testing PIPELINING.
+    Any sequences of \x followed by two hex digits are converted to the equvalent
+    byte value.  Any other character following a \ is sent verbatim.
 
 Here is a simple example:
 
diff --git a/test/confs/4030 b/test/confs/4030
new file mode 100644
index 000000000..e64859769
--- /dev/null
+++ b/test/confs/4030
@@ -0,0 +1,36 @@
+# Exim test configuration 4030
+# Proxy Protocol
+
+.include DIR/aux-var/std_conf_prefix
+
+primary_hostname = myhost.test.ex
+hosts_proxy = HOSTIPV4
+queue_only
+
+# ----- Main settings -----
+
+log_selector = +proxy +incoming_port
+
+acl_smtp_rcpt = r_acl
+
+
+begin acl
+
+r_acl:
+  accept
+	logwrite = proxy session: $proxy_session
+	logwrite = local          [$received_ip_address]:$received_port
+	logwrite = proxy internal [$proxy_local_address]:$proxy_local_port
+	logwrite = proxy external [$proxy_external_address]:$proxy_external_port
+	logwrite = remote         [$sender_host_address]:$sender_host_port
+
+
+# ----- Routers -----
+
+begin routers
+
+dump:
+  driver = redirect
+  data = :blackhole:
+
+# End
diff --git a/test/log/4030 b/test/log/4030
new file mode 100644
index 000000000..5fda8874c
--- /dev/null
+++ b/test/log/4030
@@ -0,0 +1,23 @@
+
+******** SERVER ********
+1999-03-02 09:44:33 exim x.yz daemon started: pid=pppp, no queue runs, listening for SMTP on port 1225
+1999-03-02 09:44:33 proxy session: no
+1999-03-02 09:44:33 local          [127.0.0.1]:1111
+1999-03-02 09:44:33 proxy internal []:0
+1999-03-02 09:44:33 proxy external []:0
+1999-03-02 09:44:33 remote         [127.0.0.1]:1112
+1999-03-02 09:44:33 10HmaX-0005vi-00 <= a@test.ex H=(clientname) [127.0.0.1]:1112 P=smtp S=sss
+1999-03-02 09:44:33 no host name found for IP address 127.0.0.2
+1999-03-02 09:44:33 proxy session: yes
+1999-03-02 09:44:33 local          [ip4.ip4.ip4.ip4]:1111
+1999-03-02 09:44:33 proxy internal [ip4.ip4.ip4.ip4]:1113
+1999-03-02 09:44:33 proxy external [127.42.42.42]:1114
+1999-03-02 09:44:33 remote         [127.0.0.2]:1115
+1999-03-02 09:44:33 10HmaY-0005vi-00 <= c@test.ex H=(clientname) [127.0.0.2]:1115 P=smtp PRX=ip4.ip4.ip4.ip4 S=sss
+1999-03-02 09:44:33 no host name found for IP address 192.168.0.15
+1999-03-02 09:44:33 proxy session: yes
+1999-03-02 09:44:33 local          [ip4.ip4.ip4.ip4]:1111
+1999-03-02 09:44:33 proxy internal [ip4.ip4.ip4.ip4]:1116
+1999-03-02 09:44:33 proxy external [192.168.0.48]:1117
+1999-03-02 09:44:33 remote         [192.168.0.15]:1118
+1999-03-02 09:44:33 10HmaZ-0005vi-00 <= e@test.ex H=(clientname) [192.168.0.15]:1118 P=smtp PRX=ip4.ip4.ip4.ip4 S=sss
diff --git a/test/scripts/4030-proxy-protocol/4030 b/test/scripts/4030-proxy-protocol/4030
new file mode 100644
index 000000000..8b560bcbc
--- /dev/null
+++ b/test/scripts/4030-proxy-protocol/4030
@@ -0,0 +1,75 @@
+# proxy-protocol proxy on inbound smtp
+#
+munge loopback
+#
+exim -bd -DSERVER=server -oX PORT_D
+****
+#
+# non-prox plain receive
+client 127.0.0.1 PORT_D
+??? 220
+HELO clientname
+??? 250
+MAIL FROM:<a@test.ex>
+??? 250
+RCPT TO:<b@test.ex>
+??? 250
+DATA
+??? 354
+Subject: test
+
+body
+.
+??? 250
+QUIT
+??? 221
+****
+#
+# protocol v1 plain receive
+client HOSTIPV4 PORT_D
+PROXY TCP4 127.0.0.2 127.42.42.42 64000 25
+??? 220
+HELO clientname
+??? 250
+MAIL FROM:<c@test.ex>
+??? 250
+RCPT TO:<d@test.ex>
+??? 250
+DATA
+??? 354
+Subject: test
+
+body
+.
+??? 250
+QUIT
+??? 221
+****
+#
+#
+#
+# protocol v2 plain receive
+client HOSTIPV4 PORT_D
+>>> \x0D\x0A\x0D\x0A\x00\x0D\x0A\x51\x55\x49\x54\x0A\x21\x11\x00\x0c\xc0\xa8\x00\x0f\xc0\xa8\x00\05\xc2\x95\x04\01
+??? 220
+HELO clientname
+??? 250
+MAIL FROM:<e@test.ex>
+??? 250
+RCPT TO:<f@test.ex>
+??? 250
+DATA
+??? 354
+Subject: test
+
+body
+.
+??? 250
+QUIT
+??? 221
+****
+#
+#
+#
+killdaemon
+no_msglog_check
diff --git a/test/scripts/4030-proxy-protocol/REQUIRES b/test/scripts/4030-proxy-protocol/REQUIRES
new file mode 100644
index 000000000..244d6e168
--- /dev/null
+++ b/test/scripts/4030-proxy-protocol/REQUIRES
@@ -0,0 +1 @@
+support PROXY
diff --git a/test/src/client.c b/test/src/client.c
index 4a0a1bac4..c1ac6b7f1 100644
--- a/test/src/client.c
+++ b/test/src/client.c
@@ -476,6 +476,44 @@ return session;
 
 
 /****************************************************************************/
+/* Turn "\n" and "\r" into the relevant characters. This is a hack. */
+
+static int
+unescape_buf(unsigned char * buf, int len)
+{
+unsigned char * s;
+unsigned char c, t;
+unsigned shift;
+
+for (s = buf; s < buf+len; s++) if (*s == '\\')
+  {
+  switch (s[1])
+    {
+    default:	c = s[1]; shift = 1; break;
+    case 'n':	c = '\n'; shift = 1; break;
+    case 'r':	c = '\r'; shift = 1; break;
+    case 'x':
+		t = s[2];
+    		if (t >= 'A' && t <= 'F') t -= 'A'-'9'-1;
+		else if (t >= 'a' && t <= 'f') t -= 'a'-'9'-1;
+		t -= '0';
+		c = (t<<4) & 0xf0;
+		t = s[3];
+    		if (t >= 'A' && t <= 'F') t -= 'A'-'9'-1;
+		else if (t >= 'a' && t <= 'f') t -= 'a'-'9'-1;
+		t -= '0';
+		c |= t & 0xf;
+		shift = 3;
+		break;
+    }
+  *s = c;
+  memmove(s+1, s+shift+1, len-shift);
+  len -= shift;
+  }
+return len;
+}
+
+
 /****************************************************************************/
 
 
@@ -501,7 +539,8 @@ Usage: client\n"
           [<key file>]\n\
 \n";
 
-int main(int argc, char **argv)
+int
+main(int argc, char **argv)
 {
 struct sockaddr *s_ptr;
 struct sockaddr_in s_in4;
@@ -852,6 +891,7 @@ if (tls_on_connect)
 while (fgets(CS outbuffer, sizeof(outbuffer), stdin) != NULL)
   {
   int n = (int)strlen(CS outbuffer);
+  int crlf = 1;
 
   /* Strip trailing newline */
   if (outbuffer[n-1] == '\n') outbuffer[--n] = 0;
@@ -866,6 +906,7 @@ while (fgets(CS outbuffer, sizeof(outbuffer), stdin) != NULL)
     unsigned exp_eof = outbuffer[3] == '*';
 
     printf("%s\n", outbuffer);
+    n = unescape_buf(outbuffer, n);
 
     if (*inptr == 0)   /* Refill input buffer */
       {
@@ -924,7 +965,7 @@ while (fgets(CS outbuffer, sizeof(outbuffer), stdin) != NULL)
       }
 
     printf("<<< %s\n", lineptr);
-    if (strncmp(CS lineptr, CS outbuffer + 4, (int)strlen(CS outbuffer) - 4) != 0)
+    if (strncmp(CS lineptr, CS outbuffer + 4, n - 4) != 0)
       {
       printf("\n******** Input mismatch ********\n");
       exit(79);
@@ -1024,12 +1065,19 @@ int rc;
 
   else
     {
-    unsigned char *escape;
+    unsigned char * out = outbuffer;
+
+    if (strncmp(CS outbuffer, ">>> ", 4) == 0)
+      {
+      crlf = 0;
+      out += 4;
+      n -= 4;
+      }
 
     if (*inptr != 0)
       {
       printf("Unconsumed input: %s", inptr);
-      printf("   About to send: %s\n", outbuffer);
+      printf("   About to send: %s\n", out);
       exit(78);
       }
 
@@ -1037,8 +1085,8 @@ int rc;
 
     /* Shutdown TLS */
 
-    if (strcmp(CS outbuffer, "stoptls") == 0 ||
-        strcmp(CS outbuffer, "STOPTLS") == 0)
+    if (strcmp(CS out, "stoptls") == 0 ||
+        strcmp(CS out, "STOPTLS") == 0)
       {
       if (!tls_active)
         {
@@ -1065,38 +1113,28 @@ int rc;
 
     /* Remember that we sent STARTTLS */
 
-    sent_starttls = (strcmp(CS outbuffer, "starttls") == 0 ||
-                     strcmp(CS outbuffer, "STARTTLS") == 0);
+    sent_starttls = (strcmp(CS out, "starttls") == 0 ||
+                     strcmp(CS out, "STARTTLS") == 0);
 
     /* Fudge: if the command is "starttls_wait", we send the starttls bit,
     but we haven't set the flag, so that there is no negotiation. This is for
     testing the server's timeout. */
 
-    if (strcmp(CS outbuffer, "starttls_wait") == 0)
+    if (strcmp(CS out, "starttls_wait") == 0)
       {
-      outbuffer[8] = 0;
+      out[8] = 0;
       n = 8;
       }
     #endif
 
-    printf(">>> %s\n", outbuffer);
-    strcpy(CS outbuffer + n, "\r\n");
-
-    /* Turn "\n" and "\r" into the relevant characters. This is a hack. */
-
-    while ((escape = US strstr(CS outbuffer, "\\r")) != NULL)
+    printf(">>> %s\n", out);
+    if (crlf)
       {
-      *escape = '\r';
-      memmove(escape + 1, escape + 2,  (n + 2) - (escape - outbuffer) - 2);
-      n--;
+      strcpy(CS out + n, "\r\n");
+      n += 2;
       }
 
-    while ((escape = US strstr(CS outbuffer, "\\n")) != NULL)
-      {
-      *escape = '\n';
-      memmove(escape + 1, escape + 2,  (n + 2) - (escape - outbuffer) - 2);
-      n--;
-      }
+    n = unescape_buf(out, n);
 
     /* OK, do it */
 
@@ -1104,11 +1142,10 @@ int rc;
     if (tls_active)
       {
       #ifdef HAVE_OPENSSL
-        rc = SSL_write (ssl, outbuffer, n + 2);
+        rc = SSL_write (ssl, out, n);
       #endif
       #ifdef HAVE_GNUTLS
-        rc = gnutls_record_send(tls_session, CS outbuffer, n + 2);
-        if (rc < 0)
+        if ((rc = gnutls_record_send(tls_session, CS out, n)) < 0)
           {
           printf("GnuTLS write error: %s\n", gnutls_strerror(rc));
           exit(76);
@@ -1116,9 +1153,7 @@ int rc;
       #endif
       }
     else
-      {
-      rc = write(sock, outbuffer, n + 2);
-      }
+      rc = write(sock, out, n);
     alarm(0);
 
     if (rc < 0)
diff --git a/test/stdout/4030 b/test/stdout/4030
new file mode 100644
index 000000000..b0d28b233
--- /dev/null
+++ b/test/stdout/4030
@@ -0,0 +1,77 @@
+Connecting to 127.0.0.1 port 1225 ... connected
+??? 220
+<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000
+>>> HELO clientname
+??? 250
+<<< 250 myhost.test.ex Hello clientname [IP_LOOPBACK_ADDR]
+>>> MAIL FROM:<a@test.ex>
+??? 250
+<<< 250 OK
+>>> RCPT TO:<b@test.ex>
+??? 250
+<<< 250 Accepted
+>>> DATA
+??? 354
+<<< 354 Enter message, ending with "." on a line by itself
+>>> Subject: test
+>>> 
+>>> body
+>>> .
+??? 250
+<<< 250 OK id=10HmaX-0005vi-00
+>>> QUIT
+??? 221
+<<< 221 myhost.test.ex closing connection
+End of script
+Connecting to ip4.ip4.ip4.ip4 port 1225 ... connected
+>>> PROXY TCP4 127.0.0.2 127.42.42.42 64000 25
+??? 220
+<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000
+>>> HELO clientname
+??? 250
+<<< 250 myhost.test.ex Hello clientname [127.0.0.2]
+>>> MAIL FROM:<c@test.ex>
+??? 250
+<<< 250 OK
+>>> RCPT TO:<d@test.ex>
+??? 250
+<<< 250 Accepted
+>>> DATA
+??? 354
+<<< 354 Enter message, ending with "." on a line by itself
+>>> Subject: test
+>>> 
+>>> body
+>>> .
+??? 250
+<<< 250 OK id=10HmaY-0005vi-00
+>>> QUIT
+??? 221
+<<< 221 myhost.test.ex closing connection
+End of script
+Connecting to ip4.ip4.ip4.ip4 port 1225 ... connected
+>>> \x0D\x0A\x0D\x0A\x00\x0D\x0A\x51\x55\x49\x54\x0A\x21\x11\x00\x0c\xc0\xa8\x00\x0f\xc0\xa8\x00\05\xc2\x95\x04\01
+??? 220
+<<< 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000
+>>> HELO clientname
+??? 250
+<<< 250 myhost.test.ex Hello clientname [192.168.0.15]
+>>> MAIL FROM:<e@test.ex>
+??? 250
+<<< 250 OK
+>>> RCPT TO:<f@test.ex>
+??? 250
+<<< 250 Accepted
+>>> DATA
+??? 354
+<<< 354 Enter message, ending with "." on a line by itself
+>>> Subject: test
+>>> 
+>>> body
+>>> .
+??? 250
+<<< 250 OK id=10HmaZ-0005vi-00
+>>> QUIT
+??? 221
+<<< 221 myhost.test.ex closing connection
+End of script