[Security] Prevent invalid client hostnames from appearing in

the log file. If a double-reverse lookup was performed (e.g.,
for an "Allow from .my.domain" directive) but failed, then
a spoofed dns-reverse-address could appear in the logs. Now
the numeric address is logged instead. Note that
reverse-address-spoofing did NOT actually allow access
to any protected resource! It was only possible to cause apache to
log arbitrary names (for resources protected thusly) if you had
control over the reverse dns zone.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/1.3.x@93426 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/CHANGES b/src/CHANGES
index 632f79439f5f86eabb6653dc9bee3692e4d03000..c342e0f87dc07a52a99d0312d73a7589c7d0b6e4 100644 (file)
--- a/src/CHANGES
+++ b/src/CHANGES
@@ -1,5 +1,13 @@
 Changes with Apache 1.3.24
 
+  *) [Security] Prevent invalid client hostnames from appearing in
+     the log file. If a double-reverse lookup was performed (e.g.,
+     for an "Allow from .my.domain" directive) but failed, then
+     a spoofed dns-reverse-address could appear in the logs. Now
+     the numeric address is logged instead. Note that
+     reverse-address-spoofing did NOT actually allow access
+     to any protected resource!  [Martin Kraemer]
+
   *) Some browsers ignore cookies that have been merged into a
      single Set-Cookie header. Set-Cookie and Set-Cookie2 headers
      are now unmerged in the http proxy before being sent to the

diff --git a/src/main/http_core.c b/src/main/http_core.c
index 3cd27156c610610287c95adbacba58cbeb0b1fb1..46e8c35d56d4b861768dfdc954db972d11b7187d 100644 (file)
--- a/src/main/http_core.c
+++ b/src/main/http_core.c
@@ -619,6 +619,19 @@ API_EXPORT(char *) ap_response_code_string(request_rec *r, int error_index)
 
 
 /* Code from Harald Hanche-Olsen <hanche@imf.unit.no> */
+/* Note: the function returns its result in conn->double_reverse:
+ *       +1: forward lookup of the previously reverse-looked-up
+ *           hostname in conn->remote_host succeeded, and at
+ *           least one of its IP addresses matches the client.
+ *       -1: forward lookup of conn->remote_host failed, or
+ *           none of the addresses found matches the client connection
+ *           (possible DNS spoof in the reverse zone!)
+ *       If do_double_reverse() returns -1, then it also invalidates
+ *       conn->remote_host to prevent an invalid name from appearing
+ *       in the log files. Conn->remote_host is set to "", because
+ *       a setting of NULL would allow another reverse lookup,
+ *       depending on the flags given to ap_get_remote_host().
+ */
 static ap_inline void do_double_reverse (conn_rec *conn)
 {
     struct hostent *hptr;
@@ -630,6 +643,7 @@ static ap_inline void do_double_reverse (conn_rec *conn)
     if (conn->remote_host == NULL || conn->remote_host[0] == '\0') {
        /* single reverse failed, so don't bother */
        conn->double_reverse = -1;
+        conn->remote_host = ""; /* prevent another lookup */
        return;
     }
     hptr = gethostbyname(conn->remote_host);
@@ -645,6 +659,8 @@ static ap_inline void do_double_reverse (conn_rec *conn)
        }
     }
     conn->double_reverse = -1;
+    /* invalidate possible reverse-resolved hostname if forward lookup fails */
+    conn->remote_host = "";
 }
 
 API_EXPORT(const char *) ap_get_remote_host(conn_rec *conn, void *dir_config,
@@ -683,9 +699,6 @@ API_EXPORT(const char *) ap_get_remote_host(conn_rec *conn, void *dir_config,
           
            if (hostname_lookups == HOSTNAME_LOOKUP_DOUBLE) {
                do_double_reverse(conn);
-               if (conn->double_reverse != 1) {
-                   conn->remote_host = NULL;
-               }
            }
        }
        /* if failed, set it to the NULL string to indicate error */