From: Dr. David von Oheimb Date: Mon, 25 May 2020 15:32:26 +0000 (+0200) Subject: Add request URL path checking and status responses to HTTP server X-Git-Tag: openssl-3.0.0-alpha4~121 X-Git-Url: https://git.openssl.org/?p=openssl.git;a=commitdiff_plain;h=5a2ba207ed94e79db606f80cf2873367e2a843bf Add request URL path checking and status responses to HTTP server Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/11998) --- 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 I and the CMP request message +I. It does the typical generic checks on I, 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 that can be used by a CMP client in the same way as B. -The B must be set as B of B. +The B must be set as I of I. 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. +OSSL_CMP_SRV_CTX_free() deletes the given I. -OSSL_CMP_SRV_CTX_init() sets in the given B a custom server context +OSSL_CMP_SRV_CTX_init() sets in the given I 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 may be NULL. +All arguments except I 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 from the B. +OSSL_CMP_SRV_CTX_get0_cmp_ctx() returns the B from the I. OSSL_CMP_SRV_CTX_get0_custom_ctx() returns the custom server context from -B that has been set using B. +I that has been set using B. 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 structure on success, NULL on error. OSSL_CMP_SRV_CTX_get0_custom_ctx() returns the custom server context -that has been set using B. +that has been set using B. All other functions return 1 on success, 0 on error.