On Tue, Jul 24, 2012 at 08:42:34AM +0000, Plüm, Rüdiger, Vodafone Group wrote:
> So after this rant I come to the conclusion that your proposed approach is 
> the best:
> 
> Only compare the names and not the IP's in the proxy case.

Attached does this - any comments?  I suppose this requires a major MMN 
bump?  Can backport it in a backwards-compat way for 2.4.x, though.
Index: mod_proxy_ftp.c
===================================================================
--- mod_proxy_ftp.c     (revision 1357684)
+++ mod_proxy_ftp.c     (working copy)
@@ -1143,7 +1143,7 @@
     }
 
     /* check if ProxyBlock directive on this host */
-    if (OK != ap_proxy_checkproxyblock(r, conf, connect_addr)) {
+    if (OK != ap_proxy_checkproxyblock(r, conf, connectname, connect_addr)) {
         return ap_proxyerror(r, HTTP_FORBIDDEN,
                              "Connect to remote machine blocked");
     }
Index: proxy_util.c
===================================================================
--- proxy_util.c        (revision 1357684)
+++ proxy_util.c        (working copy)
@@ -759,29 +759,29 @@
     return host != NULL && ap_strstr_c(host, This->name) != NULL;
 }
 
-/* checks whether a host in uri_addr matches proxyblock */
 PROXY_DECLARE(int) ap_proxy_checkproxyblock(request_rec *r, proxy_server_conf 
*conf,
-                             apr_sockaddr_t *uri_addr)
+                                            const char *hostname, 
apr_sockaddr_t *addr)
 {
     int j;
-    apr_sockaddr_t * src_uri_addr = uri_addr;
+
     /* XXX FIXME: conf->noproxies->elts is part of an opaque structure */
     for (j = 0; j < conf->noproxies->nelts; j++) {
         struct noproxy_entry *npent = (struct noproxy_entry *) 
conf->noproxies->elts;
         struct apr_sockaddr_t *conf_addr = npent[j].addr;
-        uri_addr = src_uri_addr;
+
         ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
                       "checking remote machine [%s] against [%s]",
-                      uri_addr->hostname, npent[j].name);
-        if (ap_strstr_c(uri_addr->hostname, npent[j].name)
+                      hostname, npent[j].name);
+        if (ap_strstr_c(hostname, npent[j].name)
             || npent[j].name[0] == '*') {
             ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(00916)
                           "connect to remote machine %s blocked: name %s "
-                          "matched", uri_addr->hostname, npent[j].name);
+                          "matched", hostname, npent[j].name);
             return HTTP_FORBIDDEN;
         }
         while (conf_addr) {
-            uri_addr = src_uri_addr;
+            apr_sockaddr_t *uri_addr = addr;
+
             while (uri_addr) {
                 char *conf_ip;
                 char *uri_ip;
@@ -792,8 +792,8 @@
                               uri_ip);
                 if (!apr_strnatcasecmp(conf_ip, uri_ip)) {
                     ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, 
APLOGNO(00917)
-                                 "connect to remote machine %s blocked: "
-                                 "IP %s matched", uri_addr->hostname, conf_ip);
+                                  "connect to remote machine %s blocked: "
+                                  "IP %s matched", hostname, conf_ip);
                     return HTTP_FORBIDDEN;
                 }
                 uri_addr = uri_addr->next;
@@ -2128,7 +2128,8 @@
         }
     }
     /* check if ProxyBlock directive on this host */
-    if (OK != ap_proxy_checkproxyblock(r, conf, conn->addr)) {
+    if (OK != ap_proxy_checkproxyblock(r, conf, uri->hostname, 
+                                       proxyname ? NULL : conn->addr)) {
         return ap_proxyerror(r, HTTP_FORBIDDEN,
                              "Connect to remote machine blocked");
     }
Index: mod_proxy.h
===================================================================
--- mod_proxy.h (revision 1357684)
+++ mod_proxy.h (working copy)
@@ -533,7 +533,19 @@
 PROXY_DECLARE(char *)ap_proxy_canon_netloc(apr_pool_t *p, char **const urlp, 
char **userp,
                                            char **passwordp, char **hostp, 
apr_port_t *port);
 PROXY_DECLARE(int) ap_proxyerror(request_rec *r, int statuscode, const char 
*message);
-PROXY_DECLARE(int) ap_proxy_checkproxyblock(request_rec *r, proxy_server_conf 
*conf, apr_sockaddr_t *uri_addr);
+
+/** Test whether the hostname/address of the request are blocked by the 
ProxyBlock
+ * configuration.
+ * @param r         request
+ * @param conf      server configuration
+ * @param hostname  hostname from request URI
+ * @param addr      resolved address of hostname, or NULL if not known
+ * @return OK on success, or else an errro
+ */
+PROXY_DECLARE(int) ap_proxy_checkproxyblock(request_rec *r, proxy_server_conf 
*conf, 
+                                            const char *hostname, 
apr_sockaddr_t *addr);
+
+
 PROXY_DECLARE(int) ap_proxy_pre_http_request(conn_rec *c, request_rec *r);
 /* DEPRECATED (will be replaced with ap_proxy_connect_backend */
 PROXY_DECLARE(int) ap_proxy_connect_to_backend(apr_socket_t **, const char *, 
apr_sockaddr_t *, const char *, proxy_server_conf *, request_rec *);
Index: mod_proxy_connect.c
===================================================================
--- mod_proxy_connect.c (revision 1357684)
+++ mod_proxy_connect.c (working copy)
@@ -205,7 +205,7 @@
     conn_rec *backconn;
 
     apr_bucket_brigade *bb = apr_brigade_create(p, c->bucket_alloc);
-    apr_status_t err, rv;
+    apr_status_t rv;
     apr_size_t nbytes;
     char buffer[HUGE_STRING_LEN];
     apr_socket_t *client_socket = ap_get_conn_socket(c);
@@ -216,7 +216,7 @@
     const apr_pollfd_t *signalled;
     apr_int32_t pollcnt, pi;
     apr_int16_t pollevent;
-    apr_sockaddr_t *uri_addr, *connect_addr;
+    apr_sockaddr_t *nexthop;
 
     apr_uri_t uri;
     const char *connectname;
@@ -246,37 +246,32 @@
     ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01019)
                   "connecting %s to %s:%d", url, uri.hostname, uri.port);
 
-    /* do a DNS lookup for the destination host */
-    err = apr_sockaddr_info_get(&uri_addr, uri.hostname, APR_UNSPEC, uri.port,
-                                0, p);
-    if (APR_SUCCESS != err) {
+    /* Determine host/port of next hop; from request URI or of a proxy. */
+    connectname = proxyname ? proxyname : uri.hostname;
+    connectport = proxyname ? proxyport : uri.port;
+
+    /* Do a DNS lookup for the next hop */
+    rv = apr_sockaddr_info_get(&nexthop, connectname, APR_UNSPEC, 
+                               connectport, 0, p);
+    if (rv != APR_SUCCESS) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO()
+                      "failed to resolve hostname '%s'", connectname);
         return ap_proxyerror(r, HTTP_BAD_GATEWAY,
                              apr_pstrcat(p, "DNS lookup failure for: ",
-                                         uri.hostname, NULL));
+                                         connectname, NULL));
     }
 
-    /* are we connecting directly, or via a proxy? */
-    if (proxyname) {
-        connectname = proxyname;
-        connectport = proxyport;
-        err = apr_sockaddr_info_get(&connect_addr, proxyname, APR_UNSPEC,
-                                    proxyport, 0, p);
+    /* Check ProxyBlock directive on the hostname/address.  */
+    if (ap_proxy_checkproxyblock(r, conf, uri.hostname, 
+                                 proxyname ? NULL : nexthop) != OK) {
+        return ap_proxyerror(r, HTTP_FORBIDDEN,
+                             "Connect to remote machine blocked");
     }
-    else {
-        connectname = uri.hostname;
-        connectport = uri.port;
-        connect_addr = uri_addr;
-    }
+
     ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r,
                   "connecting to remote proxy %s on port %d",
                   connectname, connectport);
 
-    /* check if ProxyBlock directive on this host */
-    if (OK != ap_proxy_checkproxyblock(r, conf, uri_addr)) {
-        return ap_proxyerror(r, HTTP_FORBIDDEN,
-                             "Connect to remote machine blocked");
-    }
-
     /* Check if it is an allowed port */
     if(!allowed_port(c_conf, uri.port)) {
               return ap_proxyerror(r, HTTP_FORBIDDEN,
@@ -289,15 +284,6 @@
      * We have determined who to connect to. Now make the connection.
      */
 
-    /* get all the possible IP addresses for the destname and loop through them
-     * until we get a successful connection
-     */
-    if (APR_SUCCESS != err) {
-        return ap_proxyerror(r, HTTP_BAD_GATEWAY,
-                             apr_pstrcat(p, "DNS lookup failure for: ",
-                                         connectname, NULL));
-    }
-
     /*
      * At this point we have a list of one or more IP addresses of
      * the machine to connect to. If configured, reorder this
@@ -308,7 +294,7 @@
      * For now we do nothing, ie we get DNS round robin.
      * XXX FIXME
      */
-    failed = ap_proxy_connect_to_backend(&sock, "CONNECT", connect_addr,
+    failed = ap_proxy_connect_to_backend(&sock, "CONNECT", nexthop,
                                          connectname, conf, r);
 
     /* handle a permanent error from the above loop */
@@ -355,7 +341,7 @@
         /* peer reset */
         ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(01021)
                       "an error occurred creating a new connection "
-                      "to %pI (%s)", connect_addr, connectname);
+                      "to %pI (%s)", nexthop, connectname);
         apr_socket_close(sock);
         return HTTP_INTERNAL_SERVER_ERROR;
     }
@@ -370,7 +356,7 @@
 
     ap_log_rerror(APLOG_MARK, APLOG_TRACE3, 0, r,
                   "connection complete to %pI (%s)",
-                  connect_addr, connectname);
+                  nexthop, connectname);
     apr_table_setn(r->notes, "proxy-source-port", apr_psprintf(r->pool, "%hu",
                    backconn->local_addr->port));
 

Reply via email to