Merge branch 'artem/doh-ignore-accept-header' into 'main'

DoH: Improve compatiblity by ignoring an "Accept" HTTP header value

See merge request isc-projects/bind9!5246

CHANGES

@@ -1,3 +1,6 @@
+5675.	[bug]		Improve BIND's compatibility with DoH clients by
+			ignoring an "Accept" HTTP header value. [GL !5246]
+
 5674.	[bug]		Fix BIND hanging when HTTP/2 streams are aborted
 			prematurely by web browsers. [GL !5245]
 

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);
 /*%<

lib/isc/netmgr/http.c

@@ -1805,23 +1805,6 @@ server_handle_content_type_header(isc_nmsocket_t *socket, const uint8_t *value,
 	return (resp);
 }
 
-static isc_http_error_responses_t
-server_handle_accept_header(isc_nmsocket_t *socket, const uint8_t *value,
-			    const size_t valuelen) {
-	const char type_accept_all[] = "*/*";
-	const char type_dns_message[] = DNS_MEDIA_TYPE;
-	isc_http_error_responses_t resp = ISC_HTTP_ERROR_SUCCESS;
-
-	UNUSED(socket);
-
-	if (!(HEADER_MATCH(type_dns_message, value, valuelen) ||
-	      HEADER_MATCH(type_accept_all, value, valuelen)))
-	{
-		resp = ISC_HTTP_ERROR_UNSUPPORTED_MEDIA_TYPE;
-	}
-	return (resp);
-}
-
 static isc_http_error_responses_t
 server_handle_header(isc_nmsocket_t *socket, const uint8_t *name,
 		     size_t namelen, const uint8_t *value,
@@ -1831,7 +1814,6 @@ server_handle_header(isc_nmsocket_t *socket, const uint8_t *name,
 	const char path[] = ":path";
 	const char method[] = ":method";
 	const char scheme[] = ":scheme";
-	const char accept[] = "accept";
 	const char content_length[] = "Content-Length";
 	const char content_type[] = "Content-Type";
 
@@ -1852,9 +1834,6 @@ server_handle_header(isc_nmsocket_t *socket, const uint8_t *name,
 	} else if (!was_error && HEADER_MATCH(content_type, name, namelen)) {
 		code = server_handle_content_type_header(socket, value,
 							 valuelen);
-	} else if (!was_error &&
-		   HEADER_MATCH(accept, (const char *)name, namelen)) {
-		code = server_handle_accept_header(socket, value, valuelen);
 	}
 
 	return (code);
@@ -2022,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) {
@@ -2057,7 +2051,7 @@ server_on_request_recv(nghttp2_session *ngsession,
 		if (isc_base64_decodestring(socket->h2.query_data,
 					    &decoded_buf) != ISC_R_SUCCESS)
 		{
-			code = ISC_HTTP_ERROR_GENERIC;
+			code = ISC_HTTP_ERROR_BAD_REQUEST;
 			goto error;
 		}
 		isc__buffer_usedregion(&decoded_buf, &data);
@@ -2076,7 +2070,7 @@ server_on_request_recv(nghttp2_session *ngsession,
 error:
 	result = server_send_error_response(code, ngsession, socket);
 	if (result != ISC_R_SUCCESS) {
-		return (NGHTTP2_ERR_CALLBACK_FAILURE);
+		return (NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE);
 	}
 	return (0);
 }

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);
 

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

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;
 	}