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