HTTP: Implement persistent connections (keep-alive)

Both at API and at CLI level (for the CMP app only, so far)
there is a new parameter/option: keep_alive.
* 0 means HTTP connections are not kept open after
receiving a response, which is the default behavior for HTTP 1.0.
* 1 means that persistent connections are requested.
* 2 means that persistent connections are required, i.e.,
in case the server does not grant them an error occurs.

For the CMP app the default value is 1, which means preferring to keep
the connection open. For all other internal uses of the HTTP client
(fetching an OCSP response, a cert, or a CRL) it does not matter
because these operations just take one round trip.

If the client application requested or required a persistent connection
and this was granted by the server, it can keep the OSSL_HTTP_REQ_CTX *
as long as it wants to send further requests and OSSL_HTTP_is_alive()
returns nonzero,
else it should call OSSL_HTTP_REQ_CTX_free() or OSSL_HTTP_close().
In case the client application keeps the OSSL_HTTP_REQ_CTX *
but the connection then dies for any reason at the server side, it will
notice this obtaining an I/O error when trying to send the next request.

This requires extending the HTTP header parsing and
rearranging the high-level HTTP client API. In particular:
* Split the monolithic OSSL_HTTP_transfer() into OSSL_HTTP_open(),
  OSSL_HTTP_set_request(), a lean OSSL_HTTP_transfer(), and OSSL_HTTP_close().
* Split the timeout functionality accordingly and improve default behavior.
* Extract part of OSSL_HTTP_REQ_CTX_new() to OSSL_HTTP_REQ_CTX_set_expected().
* Extend struct ossl_http_req_ctx_st accordingly.

Use the new feature for the CMP client, which requires extending
related transaction management of CMP client and test server.

Update the documentation and extend the tests accordingly.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15053)

4 files changed, 133 insertions, 35 deletions

diff --git a/apps/cmp.c b/apps/cmp.c
index d0d18c69ca..303eff10c0 100644
--- a/apps/cmp.c
+++ b/apps/cmp.c
@@ -72,6 +72,7 @@ static char *opt_path = NULL;
 static char *opt_proxy = NULL;
 static char *opt_no_proxy = NULL;
 static char *opt_recipient = NULL;
+static int opt_keep_alive = 1;
 static int opt_msg_timeout = -1;
 static int opt_total_timeout = -1;
 
@@ -205,7 +206,7 @@ typedef enum OPTION_choice {
 
     OPT_SERVER, OPT_PATH, OPT_PROXY, OPT_NO_PROXY,
     OPT_RECIPIENT,
-    OPT_MSG_TIMEOUT, OPT_TOTAL_TIMEOUT,
+    OPT_KEEP_ALIVE, OPT_MSG_TIMEOUT, OPT_TOTAL_TIMEOUT,
 
     OPT_TRUSTED, OPT_UNTRUSTED, OPT_SRVCERT,
     OPT_EXPECT_SENDER,
@@ -344,8 +345,10 @@ const OPTIONS cmp_options[] = {
      "Default from environment variable 'no_proxy', else 'NO_PROXY', else none"},
     {"recipient", OPT_RECIPIENT, 's',
      "DN of CA. Default: subject of -srvcert, -issuer, issuer of -oldcert or -cert"},
+    {"keep_alive", OPT_KEEP_ALIVE, 'N',
+     "Persistent HTTP connections. 0: no, 1 (the default): request, 2: require"},
     {"msg_timeout", OPT_MSG_TIMEOUT, 'N',
-     "Timeout per CMP message round trip (or 0 for none). Default 120 seconds"},
+     "Number of seconds allowed per CMP message round trip, or 0 for infinite"},
     {"total_timeout", OPT_TOTAL_TIMEOUT, 'N',
      "Overall time an enrollment incl. polling may take. Default 0 = infinite"},
 
@@ -530,7 +533,7 @@ static varref cmp_vars[] = { /* must be in same order as enumerated above! */
     {&opt_oldcert}, {(char **)&opt_revreason},
 
     {&opt_server}, {&opt_path}, {&opt_proxy}, {&opt_no_proxy},
-    {&opt_recipient},
+    {&opt_recipient}, {(char **)&opt_keep_alive},
     {(char **)&opt_msg_timeout}, {(char **)&opt_total_timeout},
 
     {&opt_trusted}, {&opt_untrusted}, {&opt_srvcert},
@@ -1817,6 +1820,15 @@ static int setup_client_ctx(OSSL_CMP_CTX *ctx, ENGINE *engine)
     if (!setup_verification_ctx(ctx))
         goto err;
 
+    if (opt_keep_alive != 1)
+        (void)OSSL_CMP_CTX_set_option(ctx, OSSL_CMP_OPT_KEEP_ALIVE,
+                                      opt_keep_alive);
+    if (opt_total_timeout > 0 && opt_msg_timeout > 0
+            && opt_total_timeout < opt_msg_timeout) {
+        CMP_err2("-total_timeout argument = %d must not be < %d (-msg_timeout)",
+                 opt_total_timeout, opt_msg_timeout);
+        goto err;
+    }
     if (opt_msg_timeout >= 0) /* must do this before setup_ssl_ctx() */
         (void)OSSL_CMP_CTX_set_option(ctx, OSSL_CMP_OPT_MSG_TIMEOUT,
                                       opt_msg_timeout);
@@ -2232,6 +2244,13 @@ static int get_opts(int argc, char **argv)
         case OPT_RECIPIENT:
             opt_recipient = opt_str();
             break;
+        case OPT_KEEP_ALIVE:
+            opt_keep_alive = opt_int_arg();
+            if (opt_keep_alive > 2) {
+                CMP_err("-keep_alive argument must be 0, 1, or 2");
+                goto opthelp;
+            }
+            break;
         case OPT_MSG_TIMEOUT:
             opt_msg_timeout = opt_int_arg();
             break;
@@ -2668,6 +2687,7 @@ int cmp_main(int argc, char **argv)
 #else
         BIO *acbio;
         BIO *cbio = NULL;
+        int keep_alive = 0;
         int msgs = 0;
         int retry = 1;
 
@@ -2680,7 +2700,8 @@ int cmp_main(int argc, char **argv)
 
             ret = http_server_get_asn1_req(ASN1_ITEM_rptr(OSSL_CMP_MSG),
                                            (ASN1_VALUE **)&req, &path,
-                                           &cbio, acbio, prog, 0, 0);
+                                           &cbio, acbio, &keep_alive,
+                                           prog, opt_port, 0, 0);
             if (ret == 0) { /* no request yet */
                 if (retry) {
                     sleep(1);
@@ -2712,7 +2733,8 @@ int cmp_main(int argc, char **argv)
                                                   500, "Internal Server Error");
                     break; /* treated as fatal error */
                 }
-                ret = http_server_send_asn1_resp(cbio, "application/pkixcmp",
+                ret = http_server_send_asn1_resp(cbio, keep_alive,
+                                                 "application/pkixcmp",
                                                  ASN1_ITEM_rptr(OSSL_CMP_MSG),
                                                  (const ASN1_VALUE *)resp);
                 OSSL_CMP_MSG_free(resp);
@@ -2724,8 +2746,12 @@ int cmp_main(int argc, char **argv)
                 (void)OSSL_CMP_CTX_set1_transactionID(srv_cmp_ctx, NULL);
                 (void)OSSL_CMP_CTX_set1_senderNonce(srv_cmp_ctx, NULL);
             }
-            BIO_free_all(cbio);
-            cbio = NULL;
+            if (!ret || !keep_alive
+                || OSSL_CMP_CTX_get_status(srv_cmp_ctx) == -1
+                 /* transaction closed by OSSL_CMP_CTX_server_perform() */) {
+                BIO_free_all(cbio);
+                cbio = NULL;
+            }
         }
         BIO_free_all(cbio);
         BIO_free_all(acbio);
diff --git a/apps/include/http_server.h b/apps/include/http_server.h
index 9520eb4dac..ed3f597fbd 100644
--- a/apps/include/http_server.h
+++ b/apps/include/http_server.h
@@ -67,10 +67,12 @@ BIO *http_server_init_bio(const char *prog, const char *port);
  * Accept an ASN.1-formatted HTTP request
  * it: the expected request ASN.1 type
  * preq: pointer to variable where to place the parsed request
- * pcbio: pointer to variable where to place the BIO for sending the response to
  * ppath: pointer to variable where to place the request path, or NULL
+ * pcbio: pointer to variable where to place the BIO for sending the response to
  * acbio: the listening bio (typically as returned by http_server_init_bio())
- * prog: the name of the current app
+ * found_keep_alive: for returning flag if client requests persistent connection
+ * prog: the name of the current app, for diagnostics only
+ * port: the local port listening to, for diagnostics only
  * accept_get: whether to accept GET requests (in addition to POST requests)
  * timeout: connection timeout (in seconds), or 0 for none/infinite
  * returns 0 in case caller should retry, then *preq == *ppath == *pcbio == NULL
@@ -83,19 +85,22 @@ BIO *http_server_init_bio(const char *prog, const char *port);
  */
 int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
                              char **ppath, BIO **pcbio, BIO *acbio,
-                             const char *prog, int accept_get, int timeout);
+                             int *found_keep_alive,
+                             const char *prog, const char *port,
+                             int accept_get, int timeout);
 
 /*-
  * Send an ASN.1-formatted HTTP response
  * cbio: destination BIO (typically as returned by http_server_get_asn1_req())
  *       note: cbio should not do an encoding that changes the output length
+ * keep_alive: grant persistent connnection
  * content_type: string identifying the type of the response
  * it: the response ASN.1 type
- * valit: the response ASN.1 type
  * resp: the response to send
  * returns 1 on success, 0 on failure
  */
-int http_server_send_asn1_resp(BIO *cbio, const char *content_type,
+int http_server_send_asn1_resp(BIO *cbio, int keep_alive,
+                               const char *content_type,
                                const ASN1_ITEM *it, const ASN1_VALUE *resp);
 
 /*-
diff --git a/apps/lib/http_server.c b/apps/lib/http_server.c
index 0bdc4bc316..691e5c9056 100644
--- a/apps/lib/http_server.c
+++ b/apps/lib/http_server.c
@@ -31,7 +31,14 @@
 #endif
 
 static int verbosity = LOG_INFO;
+
+#define HTTP_PREFIX "HTTP/"
+#define HTTP_VERSION_PATT "1." /* allow 1.x */
+#define HTTP_PREFIX_VERSION HTTP_PREFIX""HTTP_VERSION_PATT
+#define HTTP_1_0 HTTP_PREFIX_VERSION"0" /* "HTTP/1.0" */
+
 #ifdef HTTP_DAEMON
+
 int multi = 0; /* run multiple responder processes */
 int acfd = (int) INVALID_SOCKET;
 
@@ -262,11 +269,15 @@ static int urldecode(char *p)
     return (int)(out - save);
 }
 
+/* if *pcbio != NULL, continue given connected session, else accept new */
+/* if found_keep_alive != NULL, return this way connection persistence state */
 int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
                              char **ppath, BIO **pcbio, BIO *acbio,
-                             const char *prog, int accept_get, int timeout)
+                             int *found_keep_alive,
+                             const char *prog, const char *port,
+                             int accept_get, int timeout)
 {
-    BIO *cbio = NULL, *getbio = NULL, *b64 = NULL;
+    BIO *cbio = *pcbio, *getbio = NULL, *b64 = NULL;
     int len;
     char reqbuf[2048], inbuf[2048];
     char *meth, *url, *end;
@@ -276,14 +287,18 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
     *preq = NULL;
     if (ppath != NULL)
         *ppath = NULL;
-    *pcbio = NULL;
 
-    log_message(prog, LOG_DEBUG, "Awaiting next request...");
-/* Connection loss before accept() is routine, ignore silently */
-    if (BIO_do_accept(acbio) <= 0)
-        return ret;
+    if (cbio == NULL) {
+        log_message(prog, LOG_DEBUG,
+                    "Awaiting new connection on port %s...", port);
+        if (BIO_do_accept(acbio) <= 0)
+            /* Connection loss before accept() is routine, ignore silently */
+            return ret;
 
-    *pcbio = cbio = BIO_pop(acbio);
+        *pcbio = cbio = BIO_pop(acbio);
+    } else {
+        log_message(prog, LOG_DEBUG, "Awaiting next request...");
+    }
     if (cbio == NULL) {
         /* Cannot call http_server_send_status(cbio, ...) */
         ret = -1;
@@ -316,6 +331,9 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
     url = meth + 3;
     if ((accept_get && strncmp(meth, "GET ", 4) == 0)
             || (url++, strncmp(meth, "POST ", 5) == 0)) {
+        static const char http_version_str[] = " "HTTP_PREFIX_VERSION;
+        static const size_t http_version_str_len = sizeof(http_version_str) - 1;
+
         /* Expecting (GET|POST) {sp} /URL {sp} HTTP/1.x */
         *(url++) = '\0';
         while (*url == ' ')
@@ -333,7 +351,7 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
         for (end = url; *end != '\0'; end++)
             if (*end == ' ')
                 break;
-        if (strncmp(end, " HTTP/1.", 7) != 0) {
+        if (strncmp(end, http_version_str, http_version_str_len) != 0) {
             log_message(prog, LOG_WARNING,
                         "Invalid %s -- bad HTTP/version string: %s",
                         meth, end + 1);
@@ -341,6 +359,9 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
             goto out;
         }
         *end = '\0';
+        /* above HTTP 1.0, connection persistence is the default */
+        if (found_keep_alive != NULL)
+            *found_keep_alive = end[http_version_str_len] > '0';
 
         /*-
          * Skip "GET / HTTP..." requests often used by load-balancers.
@@ -373,7 +394,8 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
         }
     } else {
         log_message(prog, LOG_WARNING,
-                    "HTTP request does not start with GET/POST: %s", reqbuf);
+                    "HTTP request does not begin with %sPOST: %s",
+                    accept_get ? "GET or " : "", reqbuf);
         /* TODO provide better diagnosis in case client tries TLS */
         (void)http_server_send_status(cbio, 400, "Bad Request");
         goto out;
@@ -388,15 +410,50 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
 
     /* Read and skip past the headers. */
     for (;;) {
+        char *key, *value, *line_end = NULL;
+
         len = BIO_gets(cbio, inbuf, sizeof(inbuf));
         if (len <= 0) {
-            log_message(prog, LOG_WARNING,
-                        "Error skipping remaining HTTP headers");
+            log_message(prog, LOG_WARNING, "Error reading HTTP header");
             (void)http_server_send_status(cbio, 400, "Bad Request");
             goto out;
         }
-        if ((inbuf[0] == '\r') || (inbuf[0] == '\n'))
+
+        if (inbuf[0] == '\r' || inbuf[0] == '\n')
             break;
+
+        key = inbuf;
+        value = strchr(key, ':');
+        if (value != NULL) {
+            *(value++) = '\0';
+            while (*value == ' ')
+                value++;
+            line_end = strchr(value, '\r');
+            if (line_end == NULL)
+                line_end = strchr(value, '\n');
+            if (line_end != NULL)
+                *line_end = '\0';
+        } else {
+            log_message(prog, LOG_WARNING,
+                        "Error parsing HTTP header: missing ':'");
+            (void)http_server_send_status(cbio, 400, "Bad Request");
+            goto out;
+        }
+        if (value != NULL && line_end != NULL) {
+            /* https://tools.ietf.org/html/rfc7230#section-6.3 Persistence */
+            if (found_keep_alive != NULL && strcasecmp(key, "Connection") == 0) {
+                if (strcasecmp(value, "keep-alive") == 0)
+                    *found_keep_alive = 1;
+                if (strcasecmp(value, "close") == 0)
+                    *found_keep_alive = 0;
+            }
+        } else {
+            log_message(prog, LOG_WARNING,
+                        "Error parsing HTTP header: missing end of line");
+            (void)http_server_send_status(cbio, 400, "Bad Request");
+            goto out;
+        }
+
     }
 
 # ifdef HTTP_DAEMON
@@ -408,7 +465,8 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
     /* Try to read and parse request */
     req = ASN1_item_d2i_bio(it, getbio != NULL ? getbio : cbio, NULL);
     if (req == NULL) {
-        log_message(prog, LOG_WARNING, "Error parsing DER-encoded request content");
+        log_message(prog, LOG_WARNING,
+                    "Error parsing DER-encoded request content");
         (void)http_server_send_status(cbio, 400, "Bad Request");
     } else if (ppath != NULL && (*ppath = OPENSSL_strdup(url)) == NULL) {
         log_message(prog, LOG_ERR,
@@ -441,11 +499,15 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
 }
 
 /* assumes that cbio does not do an encoding that changes the output length */
-int http_server_send_asn1_resp(BIO *cbio, const char *content_type,
+int http_server_send_asn1_resp(BIO *cbio, int keep_alive,
+                               const char *content_type,
                                const ASN1_ITEM *it, const ASN1_VALUE *resp)
 {
-    int ret = BIO_printf(cbio, "HTTP/1.0 200 OK\r\nContent-type: %s\r\n"
-                         "Content-Length: %d\r\n\r\n", content_type,
+    int ret = BIO_printf(cbio, HTTP_1_0" 200 OK\r\n%s"
+                         "Content-type: %s\r\n"
+                         "Content-Length: %d\r\n\r\n",
+                         keep_alive ? "Connection: keep-alive\r\n" : "",
+                         content_type,
                          ASN1_item_i2d(resp, NULL, it)) > 0
             && ASN1_item_i2d_bio(it, cbio, resp) > 0;
 
@@ -455,7 +517,9 @@ int http_server_send_asn1_resp(BIO *cbio, const char *content_type,
 
 int http_server_send_status(BIO *cbio, int status, const char *reason)
 {
-    int ret = BIO_printf(cbio, "HTTP/1.0 %d %s\r\n\r\n", status, reason) > 0;
+    int ret = BIO_printf(cbio, HTTP_1_0" %d %s\r\n\r\n",
+                         /* This implicitly cancels keep-alive */
+                         status, reason) > 0;
 
     (void)BIO_flush(cbio);
     return ret;
diff --git a/apps/ocsp.c b/apps/ocsp.c
index 355b4127c8..694855fe09 100644
--- a/apps/ocsp.c
+++ b/apps/ocsp.c
@@ -76,7 +76,7 @@ static void make_ocsp_response(BIO *err, OCSP_RESPONSE **resp, OCSP_REQUEST *req
 
 static char **lookup_serial(CA_DB *db, ASN1_INTEGER *ser);
 static int do_responder(OCSP_REQUEST **preq, BIO **pcbio, BIO *acbio,
-                        int timeout);
+                        const char *port, int timeout);
 static int send_ocsp_response(BIO *cbio, const OCSP_RESPONSE *resp);
 static char *prog;
 
@@ -631,7 +631,7 @@ redo_accept:
 #endif
 
         req = NULL;
-        res = do_responder(&req, &cbio, acbio, req_timeout);
+        res = do_responder(&req, &cbio, acbio, port, req_timeout);
         if (res == 0)
             goto redo_accept;
 
@@ -1162,12 +1162,13 @@ static char **lookup_serial(CA_DB *db, ASN1_INTEGER *ser)
 }
 
 static int do_responder(OCSP_REQUEST **preq, BIO **pcbio, BIO *acbio,
-                        int timeout)
+                        const char *port, int timeout)
 {
 #ifndef OPENSSL_NO_SOCK
     return http_server_get_asn1_req(ASN1_ITEM_rptr(OCSP_REQUEST),
                                     (ASN1_VALUE **)preq, NULL, pcbio, acbio,
-                                    prog, 1 /* accept_get */, timeout);
+                                    NULL /* found_keep_alive */,
+                                    prog, port, 1 /* accept_get */, timeout);
 #else
     BIO_printf(bio_err,
                "Error getting OCSP request - sockets not supported\n");
@@ -1179,7 +1180,9 @@ static int do_responder(OCSP_REQUEST **preq, BIO **pcbio, BIO *acbio,
 static int send_ocsp_response(BIO *cbio, const OCSP_RESPONSE *resp)
 {
 #ifndef OPENSSL_NO_SOCK
-    return http_server_send_asn1_resp(cbio, "application/ocsp-response",
+    return http_server_send_asn1_resp(cbio,
+                                      0 /* no keep-alive */,
+                                      "application/ocsp-response",
                                       ASN1_ITEM_rptr(OCSP_RESPONSE),
                                       (const ASN1_VALUE *)resp);
 #else