Read the request body before replying, and generate certificates portably

httpsrv parsed Content-Length and never read what followed. The reply is
followed by a close, and closing a socket that still holds unread data
resets the connection instead of ending it, so a POST could cost the client
the reply it was about to read. Windows does that reliably; the same test
passes on Linux and macOS, which is why it looked flaky. Drain the body,
bounded at a megabyte.

The test CA was built with -addext, which LibreSSL - the openssl on a stock
macOS - does not apply the same way, leaving a certificate that is not
usable as a CA and a client that cannot build a chain to it. Put the
extensions in a file both accept, and verify the generated chain before any
of it is handed to a proxy, so a failure there is not read as a fault in
the proxy.

src/httpsrv.c

@@ -27,6 +27,7 @@
 #define HTTPSRV_LINE	1024
 #define HTTPSRV_BLOCK	8192
 #define HTTPSRV_MAXHDR	64
+#define HTTPSRV_MAXBODY	1048576
 
 
 /* Returns the value of a query parameter, or def when it is missing or not a
@@ -363,6 +364,28 @@ int httpopbyname(const unsigned char *name)
 	return -1;
 }
 
+/* Read and discard a request body.
+
+   The reply is followed by a close, and closing a socket that still holds
+   unread data resets the connection rather than ending it, which costs the
+   client the reply it was about to read. Bounded, so a client cannot keep
+   the server reading.
+ */
+static void httpsrv_drain(struct clientparam *param, unsigned long len)
+{
+	char buf[HTTPSRV_BLOCK];
+
+	if(len > HTTPSRV_MAXBODY) len = HTTPSRV_MAXBODY;
+	while(len){
+		int want = (len > (unsigned long)sizeof(buf))? (int)sizeof(buf) : (int)len;
+		int got = sockgetlinebuf(param, CLIENT, (unsigned char *)buf, want, EOF,
+			conf.timeouts[STRING_S]);
+
+		if(got <= 0) break;
+		len -= (unsigned long)got;
+	}
+}
+
 void * httpsrvchild(struct clientparam *param)
 {
 	struct httpreq r;
@@ -451,6 +474,8 @@ void * httpsrvchild(struct clientparam *param)
 		}
 	}
 
+	if(r.contentlen) httpsrv_drain(param, r.contentlen);
+
 	if(r.host[0]){
 		char host[sizeof(r.host)];
 		char *colon;

tests/cases/ssl.py

@@ -16,6 +16,14 @@ def run(t):
         t.skip("TLS (openssl is not available to generate certificates)")
         return
 
+    # The key material has to be sound before anything is asked of the
+    # proxy, or every failure below points at the wrong thing.
+    if not certs.verified:
+        t.fail("the generated certificate chain verifies", "OK",
+               certs.verify_output or "openssl verify failed")
+        return
+    t.ok("the generated certificate chain verifies")
+
     # --- a proxy wrapped in TLS (ssl_serv) ----------------------------
     origin = t.free_port()
     tlsproxy = t.free_port()

tests/harness.py

@@ -104,6 +104,8 @@ class Certs:
         self.other = self.dir + "/other.pem"
         self.other_key = self.dir + "/other.key"
         self.cache = self.dir + "/cache/"
+        self.verified = False
+        self.verify_output = ""
 
 
 class Failure(Exception):
@@ -435,28 +437,38 @@ class Tester:
         os.makedirs(c.cache, exist_ok=True)
         csr = c.dir + "/server.csr"
         ext = c.dir + "/server.ext"
+        ca_ext = c.dir + "/ca.ext"
         with open(ext, "w") as fp:
             fp.write("subjectAltName=IP:127.0.0.1,DNS:localhost\n")
+        # A CA without these is not usable as one. They go in a file rather
+        # than in -addext, which LibreSSL - the openssl on a stock macOS -
+        # does not apply the same way.
+        with open(ca_ext, "w") as fp:
+            fp.write("basicConstraints=critical,CA:TRUE\n"
+                     "keyUsage=critical,keyCertSign,cRLSign\n"
+                     "subjectKeyIdentifier=hash\n")
 
-        # OpenSSL 3 refuses to trust a CA without these extensions
+        def ca_steps(key, csr_path, out, name):
-        ca_ext = ["-addext", "basicConstraints=critical,CA:TRUE",
+            return [
-                  "-addext", "keyUsage=critical,keyCertSign,cRLSign"]
+                ["openssl", "genrsa", "-out", key, "2048"],
-        steps = [
+                ["openssl", "req", "-new", "-nodes", "-key", key,
-            ["openssl", "genrsa", "-out", c.ca_key, "2048"],
+                 "-subj", "/CN=" + name, "-out", csr_path],
-            ["openssl", "req", "-x509", "-new", "-nodes", "-key", c.ca_key,
+                ["openssl", "x509", "-req", "-in", csr_path, "-signkey", key,
-             "-sha256", "-days", "3650", "-subj", "/CN=3proxy-test-ca",
+                 "-days", "3650", "-sha256", "-extfile", ca_ext, "-out", out],
-             "-out", c.ca] + ca_ext,
+            ]
-            ["openssl", "genrsa", "-out", c.other_key, "2048"],
+
-            ["openssl", "req", "-x509", "-new", "-nodes", "-key", c.other_key,
+        steps = (
-             "-sha256", "-days", "3650", "-subj", "/CN=3proxy-test-other-ca",
+            ca_steps(c.ca_key, c.dir + "/ca.csr", c.ca, "3proxy-test-ca") +
-             "-out", c.other] + ca_ext,
+            ca_steps(c.other_key, c.dir + "/other.csr", c.other,
-            ["openssl", "genrsa", "-out", c.server_key, "2048"],
+                     "3proxy-test-other-ca") +
-            ["openssl", "req", "-new", "-key", c.server_key,
+            [
-             "-subj", "/CN=127.0.0.1", "-out", csr],
+                ["openssl", "genrsa", "-out", c.server_key, "2048"],
-            ["openssl", "x509", "-req", "-in", csr, "-CA", c.ca,
+                ["openssl", "req", "-new", "-key", c.server_key,
-             "-CAkey", c.ca_key, "-CAcreateserial", "-out", c.server,
+                 "-subj", "/CN=127.0.0.1", "-out", csr],
-             "-days", "3650", "-sha256", "-extfile", ext],
+                ["openssl", "x509", "-req", "-in", csr, "-CA", c.ca,
-        ]
+                 "-CAkey", c.ca_key, "-CAcreateserial", "-out", c.server,
+                 "-days", "3650", "-sha256", "-extfile", ext],
+            ])
         for step in steps:
             done = subprocess.run(step, stdout=subprocess.PIPE,
                                   stderr=subprocess.STDOUT, timeout=60)
@@ -464,6 +476,14 @@ class Tester:
                 self._certs = False
                 return None
 
+        # If the chain does not verify, the fault is in the generation, not
+        # in whatever is about to present it.
+        check = subprocess.run(["openssl", "verify", "-CAfile", c.ca, c.server],
+                               stdout=subprocess.PIPE,
+                               stderr=subprocess.STDOUT, timeout=60)
+        c.verified = check.returncode == 0
+        c.verify_output = check.stdout.decode("utf-8", "replace").strip()
+
         self._certs = c
         return c