Hi all,

I am currently working on bugzilla #39673. The issue here is that NTLM authentication as used in Microsoft products don't work. The reason is that the brilliant engineers at Microsoft who invented this authentication scheme assumed that subsequent client requests sent over a single connection towards the server (i.e. using keep alive) also reach the server on a single connection. But as Rüdiger Plüm noted some years ago in bugzilla #39673:

  "the current 2.2.x proxy implementation does NOT support
   NTLM, because there is no guarantee that the same backend
   connection is used for the next request on a keepalive
   frontend connection. Each request from a frontend connection
   leases a backend connection from a connection pool for the
   request and returns it back to the pool immediately after
   the request has been processed. If the next request on
   this keepalive frontend connection is processed it may
   lease a different backend connection from the pool. As far
   as I understand NTLM this approach is not compatible with
   NTLM."

Looking at how mod_proxy_ftp.c solves a similar problem, I tried to solve that issue with the attached patch (also attached to bugzilla). If a NTLM request is detected, the cleanup of the currently leased backend connection is skipped. Instead the backend connection is registered with the client connection pool, so that it is closed and cleaned up as soon as the client disconnects. Additionally the backend connection is registered as a config record in the client connection, so that it can get re-used for subsequent requests on the same client connection.

I would like to solicit some feedback about the approach. What would I need to change for the patch being accepted in trunk?

Regards,
Micha
diff -aur httpd-2.2.22/modules/proxy/mod_proxy_http.c httpd-2.2.22.patched/modules/proxy/mod_proxy_http.c
--- httpd-2.2.22/modules/proxy/mod_proxy_http.c	2011-02-14 09:04:57.000000000 +0100
+++ httpd-2.2.22.patched/modules/proxy/mod_proxy_http.c	2013-02-26 18:21:07.000000000 +0100
@@ -1911,6 +1911,26 @@
 }
 
 /*
+ * This is a hook to be registered with the client connection pool that should
+ * ensure the backend connection is closed as soon as the client connection is
+ * closed. As NTLM appears to authenticate TCP connections instead of HTTP
+ * requests (which is broken by design), we want to really close the backend
+ * connection and not let other clients re-use it. Otherwise the other clients
+ * may get authenticated with foreign auth credentials they don't own.
+ */
+typedef struct {
+    const char *proxy_function;
+    proxy_conn_rec *backend;
+    server_rec *server;
+} http_backend_cleanup_t;
+static apr_status_t proxy_http_ntlm_disconnect_backend(void *data) {
+    http_backend_cleanup_t *d = (http_backend_cleanup_t *)data;
+    d->backend->close = 1;
+    ap_proxy_release_connection(d->proxy_function, d->backend, d->server);
+    return APR_SUCCESS;
+};
+
+/*
  * This handles http:// URLs, and other URLs using a remote proxy over http
  * If proxyhost is NULL, then contact the server directly, otherwise
  * go via the proxy.
@@ -1929,6 +1949,9 @@
     char *scheme;
     const char *proxy_function;
     const char *u;
+    const char *auth_hdr;
+    int is_ntlm_request = 0;
+    http_backend_cleanup_t *cleanup_data;
     proxy_conn_rec *backend = NULL;
     int is_ssl = 0;
     conn_rec *c = r->connection;
@@ -1973,6 +1996,12 @@
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
              "proxy: HTTP: serving URL %s", url);
 
+    /* check whether we need to re-use an already established backend connection */
+    backend = (proxy_conn_rec *)ap_get_module_config(c->conn_config, &proxy_http_module);
+    if (backend) {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+                     "proxy: re-using already established connection to backend server");
+    } else {
 
     /* create space for state information */
     if ((status = ap_proxy_acquire_connection(proxy_function, &backend,
@@ -2027,6 +2056,32 @@
         }
     }
 
+        /*
+         * check whether we have an NTLM request that requires
+         * client connections mapping to exactly one backend connection
+         * and no shared usage of backend connections by multiple clients.
+         */
+        auth_hdr = apr_table_get(r->headers_in, "WWW-Authenticate");
+        if (auth_hdr && (!strcasecmp(auth_hdr, "NTLM") ||
+                         !strncasecmp(auth_hdr, "NTLM ", 5))) {
+            is_ntlm_request = 1;
+
+            /* enforce that this backend connection is not shared with other
+             * clients. */
+            backend->close = 1;
+
+            /* Register backend connection with client connection. */
+            cleanup_data = apr_pcalloc(c->pool, sizeof(cleanup_data));
+            cleanup_data->proxy_function = proxy_function;
+            cleanup_data->backend = backend;
+            cleanup_data->server = r->server;
+            apr_pool_cleanup_register(c->pool, cleanup_data,
+                                      proxy_http_ntlm_disconnect_backend,
+                                      apr_pool_cleanup_null);
+            ap_set_module_config(c->conn_config, &proxy_http_module, backend);
+        }
+    }
+
     /* Step Four: Send the Request */
     if ((status = ap_proxy_http_request(p, r, backend, backend->connection,
                                         conf, uri, url, server_portstr)) != OK)
@@ -2042,6 +2097,12 @@
 
 cleanup:
     if (backend) {
+        if (is_ntlm_request) {
+            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+                         "proxy: keeping backend connection open until client disconnects");
+            return status;
+        }
+
         if (status != OK)
             backend->close = 1;
         ap_proxy_http_cleanup(proxy_function, r, backend);

Reply via email to