From c6d0e3d3a7842b44eb7ed2c4946296908031ef03 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 6 Jul 2021 16:36:17 +0300 Subject: [PATCH] Return HTTP status code for small/malformed requests This commit makes BIND return HTTP status codes for malformed or too small requests. DNS request processing code would ignore such requests. Such an approach works well for other DNS transport but does not make much sense for HTTP, not allowing it to complete the request/response sequence. Suppose execution has reached the point where DNS message handling code has been called. In that case, it means that the HTTP request has been successfully processed, and, thus, we are expected to respond to it either with a message containing some DNS payload or at least to return an error status code. This commit ensures that BIND behaves this way. --- lib/isc/include/isc/netmgr.h | 12 ++++++++++++ lib/isc/netmgr/http.c | 15 +++++++++++++++ lib/isc/netmgr/netmgr-int.h | 10 ++++++++++ lib/isc/netmgr/netmgr.c | 32 ++++++++++++++++++++++++++++++++ lib/ns/client.c | 4 ++++ 5 files changed, 73 insertions(+) diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 25ccaa1bfa..23605e2928 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -509,6 +509,18 @@ isc_nm_http_endpoint(isc_nmsocket_t *sock, const char *uri, isc_nm_recv_cb_t cb, bool isc_nm_is_http_handle(isc_nmhandle_t *handle); +void +isc_nm_bad_request(isc_nmhandle_t *handle); +/*%< + * Perform a transport protocol specific action on the handle in case of a + * bad/malformed incoming DNS message. + * + * NOTE: The function currently is no-op for any protocol except HTTP/2. + * + * Requires: + * \li 'handle' is a valid netmgr handle object. + */ + void isc_nm_task_enqueue(isc_nm_t *mgr, isc_task_t *task, int threadid); /*%< diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index a21c882e86..af0739cbe4 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -2001,6 +2001,21 @@ server_call_cb(isc_nmsocket_t *socket, isc_nm_http_session_t *session, isc_nmhandle_detach(&handle); } +void +isc__nm_http_bad_request(isc_nmhandle_t *handle) { + isc_nmsocket_t *sock = NULL; + + REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(VALID_NMSOCK(handle->sock)); + sock = handle->sock; + REQUIRE(sock->type == isc_nm_httpsocket); + REQUIRE(!atomic_load(&sock->client)); + REQUIRE(VALID_HTTP2_SESSION(sock->h2.session)); + + (void)server_send_error_response(ISC_HTTP_ERROR_BAD_REQUEST, + sock->h2.session->ngsession, sock); +} + static int server_on_request_recv(nghttp2_session *ngsession, isc_nm_http_session_t *session, isc_nmsocket_t *socket) { diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index af341a344f..68891d425e 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -1654,6 +1654,16 @@ isc__nm_http_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg); void isc__nm_http_close(isc_nmsocket_t *sock); +void +isc__nm_http_bad_request(isc_nmhandle_t *handle); +/*%< + * Respond to the request with 400 "Bad Request" status. + * + * Requires: + * \li 'handle' is a valid HTTP netmgr handle object, referencing a server-side + * socket + */ + void isc__nm_async_httpsend(isc__networker_t *worker, isc__netievent_t *ev0); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 9c716423f1..877fd6d664 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -3301,6 +3301,38 @@ isc_nm_sequential(isc_nmhandle_t *handle) { atomic_store(&sock->sequential, true); } +void +isc_nm_bad_request(isc_nmhandle_t *handle) { + isc_nmsocket_t *sock; + + REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(VALID_NMSOCK(handle->sock)); + + sock = handle->sock; + switch (sock->type) { +#if HAVE_LIBNGHTTP2 + case isc_nm_httpsocket: + isc__nm_http_bad_request(handle); + break; +#endif /* HAVE_LIBNGHTTP2 */ + + case isc_nm_udpsocket: + case isc_nm_tcpdnssocket: + case isc_nm_tlsdnssocket: + return; + break; + + case isc_nm_tcpsocket: +#if HAVE_LIBNGHTTP2 + case isc_nm_tlssocket: +#endif /* HAVE_LIBNGHTTP2 */ + default: + INSIST(0); + ISC_UNREACHABLE(); + break; + } +} + #ifdef NETMGR_TRACE /* * Dump all active sockets in netmgr. We output to stderr diff --git a/lib/ns/client.c b/lib/ns/client.c index 9fa9d865ec..097733474f 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -1724,6 +1724,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, ns_client_log(client, DNS_LOGCATEGORY_SECURITY, NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(10), "dropped request: suspicious port"); + isc_nm_bad_request(handle); return; } #endif /* if NS_CLIENT_DROPPORT */ @@ -1737,6 +1738,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, ns_client_log(client, DNS_LOGCATEGORY_SECURITY, NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(10), "dropped request: blackholed peer"); + isc_nm_bad_request(handle); return; } @@ -1750,6 +1752,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, * There isn't enough header to determine whether * this was a request or a response. Drop it. */ + isc_nm_bad_request(handle); return; } @@ -1766,6 +1769,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult, */ if ((flags & DNS_MESSAGEFLAG_QR) != 0) { CTRACE("unexpected response"); + isc_nm_bad_request(handle); return; }