]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
dnsdist: Only call getsockname() once per incoming DoH connection 11851/head
authorRemi Gacogne <remi.gacogne@powerdns.com>
Thu, 11 Aug 2022 15:58:29 +0000 (17:58 +0200)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Thu, 11 Aug 2022 15:58:29 +0000 (17:58 +0200)
The current code is calling h2o_socket_getpeername() and
h2o_socket_getsockname() once per DoH _query_, and while the former
is cheap because h2o caches the result for us, the latter is actually
expensive, so this code caches both values so that we only retrieve
them once per DoH connection.

pdns/dnsdistdist/doh.cc

index ec0e068f912f0af86a6baf5782af3789dca5cb5a..4a7f498df6be66fd1704ece29d0265933270673b 100644 (file)
@@ -274,6 +274,8 @@ void handleDOHTimeout(DOHUnitUniquePtr&& oldDU)
 struct DOHConnection
 {
   std::shared_ptr<DOHAcceptContext> d_acceptCtx{nullptr};
+  ComboAddress d_remote;
+  ComboAddress d_local;
   struct timeval d_connectionStartTime{0, 0};
   size_t d_nbQueries{0};
   int d_desc{-1};
@@ -950,46 +952,46 @@ static int doh_handler(h2o_handler_t *self, h2o_req_t *req)
     if (!req->conn->ctx->storage.size) {
       return 0; // although we might was well crash on this
     }
+    DOHServerConfig* dsc = reinterpret_cast<DOHServerConfig*>(req->conn->ctx->storage.entries[0].data);
     h2o_socket_t* sock = req->conn->callbacks->get_socket(req->conn);
-    ComboAddress remote;
-    ComboAddress local;
 
-    if (h2o_socket_getpeername(sock, reinterpret_cast<struct sockaddr*>(&remote)) == 0) {
-      /* getpeername failed, likely because the connection has already been closed,
-         but anyway that means we can't get the remote address, which could allow an ACL bypass */
-      h2o_send_error_500(req, getReasonFromStatusCode(500).c_str(), "Internal Server Error - Unable to get remote address", 0);
+    const int descriptor = h2o_socket_get_fd(sock);
+    if (descriptor == -1) {
       return 0;
     }
 
-    h2o_socket_getsockname(sock, reinterpret_cast<struct sockaddr*>(&local));
-    DOHServerConfig* dsc = reinterpret_cast<DOHServerConfig*>(req->conn->ctx->storage.entries[0].data);
+    auto& conn = t_conns.at(descriptor);
+    ++conn.d_nbQueries;
+    if (conn.d_nbQueries == 1) {
+      if (h2o_socket_get_ssl_session_reused(sock) == 0) {
+        ++dsc->cs->tlsNewSessions;
+      }
+      else {
+        ++dsc->cs->tlsResumptions;
+      }
+
+      if (h2o_socket_getpeername(sock, reinterpret_cast<struct sockaddr*>(&conn.d_remote)) == 0) {
+        /* getpeername failed, likely because the connection has already been closed,
+           but anyway that means we can't get the remote address, which could allow an ACL bypass */
+        h2o_send_error_500(req, getReasonFromStatusCode(500).c_str(), "Internal Server Error - Unable to get remote address", 0);
+        return 0;
+      }
+
+      h2o_socket_getsockname(sock, reinterpret_cast<struct sockaddr*>(&conn.d_local));
+    }
 
     if (dsc->df->d_trustForwardedForHeader) {
-      processForwardedForHeader(req, remote);
+      processForwardedForHeader(req, conn.d_remote);
     }
 
     auto& holders = dsc->holders;
-    if (!holders.acl->match(remote)) {
+    if (!holders.acl->match(conn.d_remote)) {
       ++g_stats.aclDrops;
-      vinfolog("Query from %s (DoH) dropped because of ACL", remote.toStringWithPort());
+      vinfolog("Query from %s (DoH) dropped because of ACL", conn.d_remote.toStringWithPort());
       h2o_send_error_403(req, "Forbidden", "dns query not allowed because of ACL", 0);
       return 0;
     }
 
-    const int descriptor = h2o_socket_get_fd(sock);
-    if (descriptor != -1) {
-      auto& conn = t_conns.at(descriptor);
-      ++conn.d_nbQueries;
-      if (conn.d_nbQueries == 1) {
-        if (h2o_socket_get_ssl_session_reused(sock) == 0) {
-          ++dsc->cs->tlsNewSessions;
-        }
-        else {
-          ++dsc->cs->tlsResumptions;
-        }
-      }
-    }
-
     if (auto tlsversion = h2o_socket_get_ssl_protocol_version(sock)) {
       if(!strcmp(tlsversion, "TLSv1.0"))
         ++dsc->cs->tls10queries;
@@ -1044,7 +1046,7 @@ static int doh_handler(h2o_handler_t *self, h2o_req_t *req)
       query.reserve(req->entity.len + maxAdditionalSizeForEDNS);
       query.resize(req->entity.len);
       memcpy(query.data(), req->entity.base, req->entity.len);
-      doh_dispatch_query(dsc, self, req, std::move(query), local, remote, std::move(path));
+      doh_dispatch_query(dsc, self, req, std::move(query), conn.d_local, conn.d_remote, std::move(path));
     }
     else if(req->query_at != SIZE_MAX && (req->path.len - req->query_at > 5)) {
       auto pos = path.find("?dns=");
@@ -1083,7 +1085,7 @@ static int doh_handler(h2o_handler_t *self, h2o_req_t *req)
           else
             ++dsc->df->d_http1Stats.d_nbQueries;
 
-          doh_dispatch_query(dsc, self, req, std::move(decoded), local, remote, std::move(path));
+          doh_dispatch_query(dsc, self, req, std::move(decoded), conn.d_local, conn.d_remote, std::move(path));
         }
       }
       else