Add request URL path checking and status responses to HTTP server

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11998)

5 files changed, 139 insertions, 48 deletions

diff --git a/apps/cmp.c b/apps/cmp.c
index 6f3e7ed39e..a229485d66 100644
--- a/apps/cmp.c
+++ b/apps/cmp.c
@@ -2100,6 +2100,7 @@ static int setup_client_ctx(OSSL_CMP_CTX *ctx, ENGINE *e)
     (void)BIO_snprintf(server_buf, sizeof(server_buf), "http%s://%s%s%s/%s",
                        opt_tls_used ? "s" : "", opt_server,
                        server_port == 0 ? "" : ":", server_port_s,
+                       opt_path == NULL ? "" :
                        opt_path[0] == '/' ? opt_path + 1 : opt_path);
 
     if (opt_proxy != NULL)
@@ -2977,12 +2978,13 @@ int cmp_main(int argc, char **argv)
         if ((acbio = http_server_init_bio(prog, opt_port)) == NULL)
             goto err;
         while (opt_max_msgs <= 0 || msgs < opt_max_msgs) {
+            char *path = NULL;
             OSSL_CMP_MSG *req = NULL;
             OSSL_CMP_MSG *resp = NULL;
 
             ret = http_server_get_asn1_req(ASN1_ITEM_rptr(OSSL_CMP_MSG),
-                                           (ASN1_VALUE **)&req, &cbio, acbio,
-                                           prog, 0, 0);
+                                           (ASN1_VALUE **)&req, &path,
+                                           &cbio, acbio, prog, 0, 0);
             if (ret == 0)
                 continue;
             if (ret++ == -1)
@@ -2991,17 +2993,32 @@ int cmp_main(int argc, char **argv)
             ret = 0;
             msgs++;
             if (req != NULL) {
+                if (strcmp(path, "") != 0 && strcmp(path, "pkix/") != 0) {
+                    (void)http_server_send_status(cbio, 404, "Not Found");
+                    CMP_err1("Expecting empty path or 'pkix/' but got '%s'\n",
+                             path);
+                    OPENSSL_free(path);
+                    OSSL_CMP_MSG_free(req);
+                    goto cont;
+                }
+                OPENSSL_free(path);
                 resp = OSSL_CMP_CTX_server_perform(cmp_ctx, req);
                 OSSL_CMP_MSG_free(req);
-                if (resp == NULL)
+                if (resp == NULL) {
+                    (void)http_server_send_status(cbio,
+                                                  500, "Internal Server Error");
                     break; /* treated as fatal error */
+                }
                 ret = http_server_send_asn1_resp(cbio, "application/pkixcmp",
                                                  ASN1_ITEM_rptr(OSSL_CMP_MSG),
                                                  (const ASN1_VALUE *)resp);
                 OSSL_CMP_MSG_free(resp);
                 if (!ret)
                     break; /* treated as fatal error */
+            } else {
+                (void)http_server_send_status(cbio, 400, "Bad Request");
             }
+        cont:
             BIO_free_all(cbio);
             cbio = NULL;
         }
diff --git a/apps/include/http_server.h b/apps/include/http_server.h
index 8c65521339..1264753899 100644
--- a/apps/include/http_server.h
+++ b/apps/include/http_server.h
@@ -60,23 +60,29 @@ void log_message(const char *prog, int level, const char *fmt, ...);
  * returns a BIO for accepting requests, NULL on error
  */
 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
  * acbio: the listening bio (typically as returned by http_server_init_bio())
  * prog: the name of the current app
- * accept_get: wheter to accept GET requests (in addition to POST requests)
+ * 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 == *pcbio == NULL
- * returns -1 on fatal error; also in this case *preq == *pcbio == NULL
- * returns 1 otherwise. In this case it is guaranteed that *pcbio != NULL
- *                      while *preq == NULL if and only if request is invalid
+ * returns 0 in case caller should retry, then *preq == *ppath == *pcbio == NULL
+ * returns -1 on fatal error; also then holds *preq == *ppath == *pcbio == NULL
+ * returns 1 otherwise. In this case it is guaranteed that *pcbio != NULL while
+ * *ppath == NULL and *preq == NULL if and only if the request is invalid,
+ * On return value 1 the caller is responsible for sending an HTTP response,
+ * using http_server_send_asn1_resp() or http_server_send_status().
+ * The caller must free any non-NULL *preq, *ppath, and *pcbio pointers.
  */
 int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
-                        BIO **pcbio, BIO *acbio,
-                        const char *prog, int accept_get, int timeout);
+                             char **ppath, BIO **pcbio, BIO *acbio,
+                             const char *prog, int accept_get, int timeout);
+
 /*-
  * Send an ASN.1-formatted HTTP response
  * cbio: destination BIO (typically as returned by http_server_get_asn1_req())
@@ -89,6 +95,16 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
  */
 int http_server_send_asn1_resp(BIO *cbio, const char *content_type,
                                const ASN1_ITEM *it, const ASN1_VALUE *resp);
+
+/*-
+ * Send a trivial HTTP response, typically to report an error or OK
+ * cbio: destination BIO (typically as returned by http_server_get_asn1_req())
+ * status: the status code to send
+ * reason: the corresponding human-readable string
+ * returns 1 on success, 0 on failure
+ */
+int http_server_send_status(BIO *cbio, int status, const char *reason);
+
 # endif
 
 # ifdef HTTP_DAEMON
diff --git a/apps/lib/http_server.c b/apps/lib/http_server.c
index 2b5c9f5dcd..11f0b1fcb5 100644
--- a/apps/lib/http_server.c
+++ b/apps/lib/http_server.c
@@ -255,17 +255,19 @@ static int urldecode(char *p)
 }
 
 int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
-                             BIO **pcbio, BIO *acbio,
+                             char **ppath, BIO **pcbio, BIO *acbio,
                              const char *prog, int accept_get, int timeout)
 {
     BIO *cbio = NULL, *getbio = NULL, *b64 = NULL;
     int len;
     char reqbuf[2048], inbuf[2048];
-    char *url, *end;
+    char *meth, *url, *end;
     ASN1_VALUE *req;
     int ret = 1;
 
     *preq = NULL;
+    if (ppath != NULL)
+        *ppath = NULL;
     *pcbio = NULL;
 
     /* Connection loss before accept() is routine, ignore silently */
@@ -275,6 +277,7 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
     cbio = BIO_pop(acbio);
     *pcbio = cbio;
     if (cbio == NULL) {
+        /* Cannot call http_server_send_status(cbio, ...) */
         ret = -1;
         goto out;
     }
@@ -288,16 +291,26 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
 
     /* Read the request line. */
     len = BIO_gets(cbio, reqbuf, sizeof(reqbuf));
-    if (len <= 0)
+    if (len <= 0) {
+        log_message(prog, LOG_INFO,
+                    "Request line read error or empty request");
+        (void)http_server_send_status(cbio, 400, "Bad Request");
         goto out;
+    }
 
-    if (accept_get && strncmp(reqbuf, "GET ", 4) == 0) {
-        /* Expecting GET {sp} /URL {sp} HTTP/1.x */
-        for (url = reqbuf + 4; *url == ' '; ++url)
-            continue;
+    meth = reqbuf;
+    url = meth + 3;
+    if ((accept_get && strncmp(meth, "GET ", 4) == 0)
+            || (url++, strncmp(meth, "POST ", 5) == 0)) {
+        /* Expecting (GET|POST) {sp} /URL {sp} HTTP/1.x */
+        *(url++) = '\0';
+        while (*url == ' ')
+            url++;
         if (*url != '/') {
             log_message(prog, LOG_INFO,
-                        "Invalid GET -- URL does not begin with '/': %s", url);
+                        "Invalid %s -- URL does not begin with '/': %s",
+                        meth, url);
+            (void)http_server_send_status(cbio, 400, "Bad Request");
             goto out;
         }
         url++;
@@ -308,7 +321,9 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
                 break;
         if (strncmp(end, " HTTP/1.", 7) != 0) {
             log_message(prog, LOG_INFO,
-                        "Invalid GET -- bad HTTP/version string: %s", end + 1);
+                        "Invalid %s -- bad HTTP/version string: %s",
+                        meth, end + 1);
+            (void)http_server_send_status(cbio, 400, "Bad Request");
             goto out;
         }
         *end = '\0';
@@ -318,39 +333,52 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
          * 'url' was incremented above to point to the first byte *after*
          * the leading slash, so in case 'GET / ' it is now an empty string.
          */
-        if (url[0] == '\0')
+        if (strlen(meth) == 3 && url[0] == '\0') {
+            (void)http_server_send_status(cbio, 200, "OK");
             goto out;
+        }
 
         len = urldecode(url);
-        if (len <= 0) {
+        if (len < 0) {
             log_message(prog, LOG_INFO,
-                        "Invalid GET request -- bad URL encoding: %s", url);
+                        "Invalid %s request -- bad URL encoding: %s",
+                        meth, url);
+            (void)http_server_send_status(cbio, 400, "Bad Request");
             goto out;
         }
-        if ((getbio = BIO_new_mem_buf(url, len)) == NULL
-            || (b64 = BIO_new(BIO_f_base64())) == NULL) {
-            log_message(prog, LOG_ERR,
-                        "Could not allocate base64 bio with size = %d", len);
-            BIO_free_all(cbio);
-            *pcbio = NULL;
-            ret = -1;
-            goto out;
+        if (strlen(meth) == 3) { /* GET */
+            if ((getbio = BIO_new_mem_buf(url, len)) == NULL
+                || (b64 = BIO_new(BIO_f_base64())) == NULL) {
+                log_message(prog, LOG_ERR,
+                            "Could not allocate base64 bio with size = %d",
+                            len);
+                goto fatal;
+            }
+            BIO_set_flags(b64, BIO_FLAGS_BASE64_NO_NL);
+            getbio = BIO_push(b64, getbio);
         }
-        BIO_set_flags(b64, BIO_FLAGS_BASE64_NO_NL);
-        getbio = BIO_push(b64, getbio);
-    } else if (strncmp(reqbuf, "POST ", 5) != 0) {
+    } else {
         log_message(prog, LOG_INFO,
                     "HTTP request does not start with GET/POST: %s", reqbuf);
         /* TODO provide better diagnosis in case client tries TLS */
+        (void)http_server_send_status(cbio, 400, "Bad Request");
         goto out;
     }
 
+    /* chop any further/duplicate leading or trailing '/' */
+    while (*url == '/')
+        url++;
+    while (end >= url + 2 && end[-2] == '/' && end[-1] == '/')
+        end--;
+    *end = '\0';
+
     /* Read and skip past the headers. */
     for (;;) {
         len = BIO_gets(cbio, inbuf, sizeof(inbuf));
         if (len <= 0) {
             log_message(prog, LOG_ERR,
                         "Error skipping remaining HTTP headers");
+            (void)http_server_send_status(cbio, 400, "Bad Request");
             goto out;
         }
         if ((inbuf[0] == '\r') || (inbuf[0] == '\n'))
@@ -365,8 +393,14 @@ 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)
+    if (req == NULL) {
         log_message(prog, LOG_ERR, "Error parsing request");
+    } else if (ppath != NULL && (*ppath = OPENSSL_strdup(url)) == NULL) {
+        log_message(prog, LOG_ERR,
+                    "Out of memory allocating %d bytes", strlen(url) + 1);
+        ASN1_item_free(req, it);
+        goto fatal;
+    }
 
     *preq = req;
 
@@ -378,6 +412,17 @@ int http_server_get_asn1_req(const ASN1_ITEM *it, ASN1_VALUE **preq,
     acfd = (int)INVALID_SOCKET;
 # endif
     return ret;
+
+ fatal:
+    (void)http_server_send_status(cbio, 500, "Internal Server Error");
+    if (ppath != NULL) {
+        OPENSSL_free(*ppath);
+        *ppath = NULL;
+    }
+    BIO_free_all(cbio);
+    *pcbio = NULL;
+    ret = -1;
+    goto out;
 }
 
 /* assumes that cbio does not do an encoding that changes the output length */
@@ -392,4 +437,12 @@ int http_server_send_asn1_resp(BIO *cbio, const char *content_type,
     (void)BIO_flush(cbio);
     return ret;
 }
+
+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;
+
+    (void)BIO_flush(cbio);
+    return ret;
+}
 #endif
diff --git a/apps/ocsp.c b/apps/ocsp.c
index fd03611fe9..6095e6b2f6 100644
--- a/apps/ocsp.c
+++ b/apps/ocsp.c
@@ -234,7 +234,7 @@ int ocsp_main(int argc, char **argv)
     int noCAfile = 0, noCApath = 0, noCAstore = 0;
     int accept_count = -1, add_nonce = 1, noverify = 0, use_ssl = -1;
     int vpmtouched = 0, badsig = 0, i, ignore_err = 0, nmin = 0, ndays = -1;
-    int req_text = 0, resp_text = 0, ret = 1;
+    int req_text = 0, resp_text = 0, res, ret = 1;
     int req_timeout = -1;
     long nsec = MAX_VALIDITY_PERIOD, maxage = -1;
     unsigned long sign_flags = 0, verify_flags = 0, rflags = 0;
@@ -629,13 +629,17 @@ redo_accept:
 #endif
 
         req = NULL;
-        if (!do_responder(&req, &cbio, acbio, req_timeout))
+        res = do_responder(&req, &cbio, acbio, req_timeout);
+        if (res == 0)
             goto redo_accept;
 
         if (req == NULL) {
-            resp = OCSP_response_create(OCSP_RESPONSE_STATUS_MALFORMEDREQUEST,
-                                        NULL);
-            send_ocsp_response(cbio, resp);
+            if (res == 1) {
+                resp =
+                    OCSP_response_create(OCSP_RESPONSE_STATUS_MALFORMEDREQUEST,
+                                         NULL);
+                send_ocsp_response(cbio, resp);
+            }
             goto done_resp;
         }
     }
@@ -1151,7 +1155,7 @@ static int do_responder(OCSP_REQUEST **preq, BIO **pcbio, BIO *acbio,
 {
 #ifndef OPENSSL_NO_SOCK
     return http_server_get_asn1_req(ASN1_ITEM_rptr(OCSP_RESPONSE),
-                                    (ASN1_VALUE **)preq, pcbio, acbio,
+                                    (ASN1_VALUE **)preq, NULL, pcbio, acbio,
                                     prog, 1 /* accept_get */, timeout);
 #else
     BIO_printf(bio_err,
diff --git a/doc/man3/OSSL_CMP_SRV_CTX_new.pod b/doc/man3/OSSL_CMP_SRV_CTX_new.pod
index a2d47a45aa..27d4f6ca1e 100644
--- a/doc/man3/OSSL_CMP_SRV_CTX_new.pod
+++ b/doc/man3/OSSL_CMP_SRV_CTX_new.pod
@@ -85,32 +85,33 @@ OSSL_CMP_SRV_CTX_set_grant_implicit_confirm
 =head1 DESCRIPTION
 
 OSSL_CMP_SRV_process_request() implements the generic aspects of a CMP server.
-It does the typical generic checks on the given request message, calls
+Its arguments are the B<OSSL_CMP_SRV_CTX> I<srv_ctx> and the CMP request message
+I<req>. It does the typical generic checks on I<req>, calls
 the respective callback function (if present) for more specific processing,
 and then assembles a result message, which may be a CMP error message.
 
 OSSL_CMP_CTX_server_perform() is an interface to
 B<OSSL_CMP_SRV_process_request()> that can be used by a CMP client
 in the same way as B<OSSL_CMP_MSG_http_perform()>.
-The B<OSSL_CMP_SRV_CTX> must be set as B<transfer_cb_arg> of B<client_ctx>.
+The B<OSSL_CMP_SRV_CTX> must be set as I<transfer_cb_arg> of I<client_ctx>.
 
 OSSL_CMP_SRV_CTX_new() creates and initializes an OSSL_CMP_SRV_CTX structure
 and returns a pointer to it on success, NULL on error.
 
-OSSL_CMP_SRV_CTX_free() deletes the given B<srv_ctx>.
+OSSL_CMP_SRV_CTX_free() deletes the given I<srv_ctx>.
 
-OSSL_CMP_SRV_CTX_init() sets in the given B<srv_ctx> a custom server context
+OSSL_CMP_SRV_CTX_init() sets in the given I<srv_ctx> a custom server context
 pointer as well as callback functions performing the specific processing of CMP
 certificate requests, revocation requests, certificate confirmation requests,
 general messages, error messages, and poll requests.
-All arguments except B<srv_ctx> may be NULL.
+All arguments except I<srv_ctx> may be NULL.
 If a callback for some message type is not given this means that the respective
 type of CMP message is not supported by the server.
 
-OSSL_CMP_SRV_CTX_get0_cmp_ctx() returns the B<OSSL_CMP_CTX> from the B<srv_ctx>.
+OSSL_CMP_SRV_CTX_get0_cmp_ctx() returns the B<OSSL_CMP_CTX> from the I<srv_ctx>.
 
 OSSL_CMP_SRV_CTX_get0_custom_ctx() returns the custom server context from
-B<srv_ctx> that has been set using B<OSSL_CMP_SRV_CTX_init>.
+I<srv_ctx> that has been set using B<OSSL_CMP_SRV_CTX_init()>.
 
 OSSL_CMP_SRV_CTX_set_send_unprotected_errors() enables sending error messages
 and other forms of negative responses unprotected.
@@ -142,7 +143,7 @@ OSSL_CMP_SRV_CTX_get0_cmp_ctx() returns a B<OSSL_CMP_CTX> structure on success,
 NULL on error.
 
 OSSL_CMP_SRV_CTX_get0_custom_ctx() returns the custom server context
-that has been set using B<OSSL_CMP_SRV_CTX_init>.
+that has been set using B<OSSL_CMP_SRV_CTX_init()>.
 
 All other functions return 1 on success, 0 on error.