You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by rj...@apache.org on 2020/12/10 15:05:39 UTC

[tomcat] branch 8.5.x updated: Add peerAddress to coyote request

This is an automated email from the ASF dual-hosted git repository.

rjung pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
     new 088b8c1  Add peerAddress to coyote request
088b8c1 is described below

commit 088b8c198d84b6bfe495e1661ca192bc8d5f59e0
Author: Rainer Jung <ra...@kippdata.de>
AuthorDate: Thu Dec 10 16:05:03 2020 +0100

    Add peerAddress to coyote request
    
    It contains the IP address of the direct connection peer.
    If a reverse proxy sits in front of Tomcat and the protocol
    used is AJP or HTTP in combination with the RemoteIp(Valve|Filter),
    the peer address might differ from the remoteAddress.
    The latter then contains the address of the client in front of the
    reverse proxy, not the address of the proxy itself.
    
    Support for the peer address has been added to the
    RemoteAddrValve and RemoteCIDRValve with the new attribute
    "usePeerAddress". This can be used to restrict access
    to Tomcat based on the reverse proxy IP address, which is especially
    useful to harden access to AJP connectors.
    
    The peer address can also be logged in the access log
    using the new %{peer}a syntax.
---
 java/org/apache/catalina/connector/Request.java    |  19 ++
 .../catalina/valves/AbstractAccessLogValve.java    |  60 ++++-
 .../apache/catalina/valves/LocalStrings.properties |   1 +
 .../apache/catalina/valves/RemoteAddrValve.java    |   8 +-
 .../apache/catalina/valves/RemoteCIDRValve.java    |   8 +-
 .../apache/catalina/valves/RequestFilterValve.java |  30 +++
 java/org/apache/coyote/AbstractProcessor.java      |   6 +
 java/org/apache/coyote/ActionCode.java             |   5 +
 java/org/apache/coyote/Constants.java              |   7 +
 java/org/apache/coyote/Request.java                |   6 +
 java/org/apache/coyote/RequestInfo.java            |   5 +
 java/org/apache/coyote/ajp/AjpProcessor.java       |   4 +
 .../catalina/valves/TestRequestFilterValve.java    | 297 ++++++++++++---------
 webapps/docs/changelog.xml                         |  16 ++
 webapps/docs/config/valve.xml                      |  31 ++-
 15 files changed, 358 insertions(+), 145 deletions(-)

diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java
index dcf6c7f..3ffaabf 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -403,6 +403,12 @@ public class Request implements HttpServletRequest {
 
 
     /**
+     * Connection peer address.
+     */
+    protected String peerAddr = null;
+
+
+    /**
      * Remote host.
      */
     protected String remoteHost = null;
@@ -479,6 +485,7 @@ public class Request implements HttpServletRequest {
         localesParsed = false;
         secure = false;
         remoteAddr = null;
+        peerAddr = null;
         remoteHost = null;
         remotePort = -1;
         localPort = -1;
@@ -1309,6 +1316,18 @@ public class Request implements HttpServletRequest {
 
 
     /**
+     * @return the connection peer IP address making this Request.
+     */
+    public String getPeerAddr() {
+        if (peerAddr == null) {
+            coyoteRequest.action(ActionCode.REQ_PEER_ADDR_ATTRIBUTE, coyoteRequest);
+            peerAddr = coyoteRequest.peerAddr().toString();
+        }
+        return peerAddr;
+    }
+
+
+    /**
      * @return the remote host name making this Request.
      */
     @Override
diff --git a/java/org/apache/catalina/valves/AbstractAccessLogValve.java b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
index 41c799c..10c0798 100644
--- a/java/org/apache/catalina/valves/AbstractAccessLogValve.java
+++ b/java/org/apache/catalina/valves/AbstractAccessLogValve.java
@@ -163,6 +163,13 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
         LOCAL, REMOTE
     }
 
+    /**
+     * The list of our ip address types.
+     */
+    private enum RemoteAddressType {
+        REMOTE, PEER
+    }
+
     //------------------------------------------------------ Constructor
     public AbstractAccessLogValve() {
         super(true);
@@ -876,19 +883,50 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
      * write remote IP address - %a
      */
     protected class RemoteAddrElement implements AccessLogElement, CachedElement {
+        /**
+         * Type of address to log
+         */
+        private static final String remoteAddress = "remote";
+        private static final String peerAddress = "peer";
+
+        private final RemoteAddressType remoteAddressType;
+
+        public RemoteAddrElement() {
+            remoteAddressType = RemoteAddressType.REMOTE;
+        }
+
+        public RemoteAddrElement(String type) {
+            switch (type) {
+            case remoteAddress:
+                remoteAddressType = RemoteAddressType.REMOTE;
+                break;
+            case peerAddress:
+                remoteAddressType = RemoteAddressType.PEER;
+                break;
+            default:
+                log.error(sm.getString("accessLogValve.invalidRemoteAddressType", type));
+                remoteAddressType = RemoteAddressType.REMOTE;
+                break;
+            }
+        }
+
         @Override
         public void addElement(CharArrayWriter buf, Date date, Request request,
                 Response response, long time) {
             String value = null;
-            if (requestAttributesEnabled) {
-                Object addr = request.getAttribute(REMOTE_ADDR_ATTRIBUTE);
-                if (addr == null) {
-                    value = request.getRemoteAddr();
+            if (remoteAddressType == RemoteAddressType.PEER) {
+                value = request.getPeerAddr();
+            } else {
+                if (requestAttributesEnabled) {
+                    Object addr = request.getAttribute(REMOTE_ADDR_ATTRIBUTE);
+                    if (addr == null) {
+                        value = request.getRemoteAddr();
+                    } else {
+                        value = addr.toString();
+                    }
                 } else {
-                    value = addr.toString();
+                    value = request.getRemoteAddr();
                 }
-            } else {
-                value = request.getRemoteAddr();
             }
 
             if (ipv6Canonical) {
@@ -900,7 +938,11 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
         @Override
         public void cache(Request request) {
             if (!requestAttributesEnabled) {
-                request.getRemoteAddr();
+                if (remoteAddressType == RemoteAddressType.PEER) {
+                    request.getPeerAddr();
+                } else {
+                    request.getRemoteAddr();
+                }
             }
         }
     }
@@ -1743,6 +1785,8 @@ public abstract class AbstractAccessLogValve extends ValveBase implements Access
             return new CookieElement(name);
         case 'o':
             return new ResponseHeaderElement(name);
+        case 'a':
+            return new RemoteAddrElement(name);
         case 'p':
             return new PortElement(name);
         case 'r':
diff --git a/java/org/apache/catalina/valves/LocalStrings.properties b/java/org/apache/catalina/valves/LocalStrings.properties
index 2ec1abd..83b8aa1 100644
--- a/java/org/apache/catalina/valves/LocalStrings.properties
+++ b/java/org/apache/catalina/valves/LocalStrings.properties
@@ -18,6 +18,7 @@ accessLogValve.closeFail=Failed to close access log file
 accessLogValve.deleteFail=Failed to delete old access log [{0}]
 accessLogValve.invalidLocale=Failed to set locale to [{0}]
 accessLogValve.invalidPortType=Invalid port type [{0}], using server (local) port
+accessLogValve.invalidRemoteAddressType=Invalid remote address type [{0}], using remote (non-peer) address
 accessLogValve.openDirFail=Failed to create directory [{0}] for access logs
 accessLogValve.openFail=Failed to open access log file [{0}]
 accessLogValve.renameFail=Failed to rename access log from [{0}] to [{1}]
diff --git a/java/org/apache/catalina/valves/RemoteAddrValve.java b/java/org/apache/catalina/valves/RemoteAddrValve.java
index d8dc2a3..77183ac 100644
--- a/java/org/apache/catalina/valves/RemoteAddrValve.java
+++ b/java/org/apache/catalina/valves/RemoteAddrValve.java
@@ -44,11 +44,15 @@ public final class RemoteAddrValve extends RequestFilterValve {
     @Override
     public void invoke(Request request, Response response) throws IOException, ServletException {
         String property;
-        if (getAddConnectorPort()) {
-            property = request.getRequest().getRemoteAddr() + ";" + request.getConnector().getPort();
+        if (getUsePeerAddress()) {
+            property = request.getPeerAddr();
         } else {
             property = request.getRequest().getRemoteAddr();
         }
+        if (getAddConnectorPort()) {
+            property = property + ";" +
+                request.getConnector().getPort();
+        }
         process(property, request, response);
     }
 
diff --git a/java/org/apache/catalina/valves/RemoteCIDRValve.java b/java/org/apache/catalina/valves/RemoteCIDRValve.java
index 0bdf450..da207ba 100644
--- a/java/org/apache/catalina/valves/RemoteCIDRValve.java
+++ b/java/org/apache/catalina/valves/RemoteCIDRValve.java
@@ -129,11 +129,15 @@ public final class RemoteCIDRValve extends RequestFilterValve {
     @Override
     public void invoke(final Request request, final Response response) throws IOException, ServletException {
         String property;
-        if (getAddConnectorPort()) {
-            property = request.getRequest().getRemoteAddr() + ";" + request.getConnector().getPort();
+        if (getUsePeerAddress()) {
+            property = request.getPeerAddr();
         } else {
             property = request.getRequest().getRemoteAddr();
         }
+        if (getAddConnectorPort()) {
+            property = property + ";" +
+                request.getConnector().getPort();
+        }
         process(property, request, response);
     }
 
diff --git a/java/org/apache/catalina/valves/RequestFilterValve.java b/java/org/apache/catalina/valves/RequestFilterValve.java
index 010d1de..6ed5940 100644
--- a/java/org/apache/catalina/valves/RequestFilterValve.java
+++ b/java/org/apache/catalina/valves/RequestFilterValve.java
@@ -139,6 +139,13 @@ public abstract class RequestFilterValve extends ValveBase {
      */
     private volatile boolean addConnectorPort = false;
 
+    /**
+     * Flag deciding whether we use the connection peer address
+     * or the remote address. This makes a dfifference when
+     * using AJP or the RemoteIpValve.
+     */
+    private volatile boolean usePeerAddress = false;
+
     // ------------------------------------------------------------- Properties
 
 
@@ -288,6 +295,29 @@ public abstract class RequestFilterValve extends ValveBase {
         this.addConnectorPort = addConnectorPort;
     }
 
+
+    /**
+     * Get the flag deciding whether we use the connection peer address
+     * or the remote address. This makes a dfifference when
+     * using AJP or the RemoteIpValve.
+     * @return <code>true</code> if we use the connection peer address
+     */
+    public boolean getUsePeerAddress() {
+        return usePeerAddress;
+    }
+
+
+    /**
+     * Set the flag deciding whether we use the connection peer address
+     * or the remote address. This makes a dfifference when
+     * using AJP or the RemoteIpValve.
+     *
+     * @param usePeerAddress The new flag
+     */
+    public void setUsePeerAddress(boolean usePeerAddress) {
+        this.usePeerAddress = usePeerAddress;
+    }
+
     // --------------------------------------------------------- Public Methods
 
     /**
diff --git a/java/org/apache/coyote/AbstractProcessor.java b/java/org/apache/coyote/AbstractProcessor.java
index 6140839..4b10ef5 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -456,6 +456,12 @@ public abstract class AbstractProcessor extends AbstractProcessorLight implement
             }
             break;
         }
+        case REQ_PEER_ADDR_ATTRIBUTE: {
+            if (getPopulateRequestAttributesFromSocket() && socketWrapper != null) {
+                request.peerAddr().setString(socketWrapper.getRemoteAddr());
+            }
+            break;
+        }
         case REQ_HOST_ATTRIBUTE: {
             populateRequestAttributeRemoteHost();
             break;
diff --git a/java/org/apache/coyote/ActionCode.java b/java/org/apache/coyote/ActionCode.java
index 2795a81..a59c92e 100644
--- a/java/org/apache/coyote/ActionCode.java
+++ b/java/org/apache/coyote/ActionCode.java
@@ -77,6 +77,11 @@ public enum ActionCode {
     REQ_HOST_ADDR_ATTRIBUTE,
 
     /**
+     * Callback for lazy evaluation - extract the connection peer address.
+     */
+    REQ_PEER_ADDR_ATTRIBUTE,
+
+    /**
      * Callback for lazy evaluation - extract the SSL-related attributes
      * including the client certificate if present.
      */
diff --git a/java/org/apache/coyote/Constants.java b/java/org/apache/coyote/Constants.java
index 9de194d..f7126dc 100644
--- a/java/org/apache/coyote/Constants.java
+++ b/java/org/apache/coyote/Constants.java
@@ -111,4 +111,11 @@ public final class Constants {
      * the X-Forwarded-For HTTP header.
      */
     public static final String REMOTE_ADDR_ATTRIBUTE = "org.apache.tomcat.remoteAddr";
+
+    /**
+     * The request attribute set by the RemoteIpFilter, RemoteIpValve (and may
+     * be set by other similar components) that identifies for the connector the
+     * connection peer IP address.
+     */
+    public static final String PEER_ADDR_ATTRIBUTE = "org.apache.tomcat.peerAddr";
 }
diff --git a/java/org/apache/coyote/Request.java b/java/org/apache/coyote/Request.java
index e932694..e7c943b 100644
--- a/java/org/apache/coyote/Request.java
+++ b/java/org/apache/coyote/Request.java
@@ -95,6 +95,7 @@ public final class Request {
 
     // remote address/host
     private final MessageBytes remoteAddrMB = MessageBytes.newInstance();
+    private final MessageBytes peerAddrMB = MessageBytes.newInstance();
     private final MessageBytes localNameMB = MessageBytes.newInstance();
     private final MessageBytes remoteHostMB = MessageBytes.newInstance();
     private final MessageBytes localAddrMB = MessageBytes.newInstance();
@@ -255,6 +256,10 @@ public final class Request {
         return remoteAddrMB;
     }
 
+    public MessageBytes peerAddr() {
+        return peerAddrMB;
+    }
+
     public MessageBytes remoteHost() {
         return remoteHostMB;
     }
@@ -653,6 +658,7 @@ public final class Request {
         localAddrMB.recycle();
         localNameMB.recycle();
         localPort = -1;
+        peerAddrMB.recycle();
         remoteAddrMB.recycle();
         remoteHostMB.recycle();
         remotePort = -1;
diff --git a/java/org/apache/coyote/RequestInfo.java b/java/org/apache/coyote/RequestInfo.java
index 30216b7..629ca0f 100644
--- a/java/org/apache/coyote/RequestInfo.java
+++ b/java/org/apache/coyote/RequestInfo.java
@@ -96,6 +96,11 @@ public class RequestInfo  {
         return req.remoteAddr().toString();
     }
 
+    public String getPeerAddr() {
+        req.action(ActionCode.REQ_PEER_ADDR_ATTRIBUTE, null);
+        return req.peerAddr().toString();
+    }
+
     /**
      * Obtain the remote address for this connection as reported by an
      * intermediate proxy (if any).
diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java b/java/org/apache/coyote/ajp/AjpProcessor.java
index d1c3e10..1256815 100644
--- a/java/org/apache/coyote/ajp/AjpProcessor.java
+++ b/java/org/apache/coyote/ajp/AjpProcessor.java
@@ -759,6 +759,10 @@ public class AjpProcessor extends AbstractProcessor {
         requestHeaderMessage.getBytes(request.localName());
         request.setLocalPort(requestHeaderMessage.getInt());
 
+        if (socketWrapper != null) {
+            request.peerAddr().setString(socketWrapper.getRemoteAddr());
+        }
+
         boolean isSSL = requestHeaderMessage.getByte() != 0;
         if (isSSL) {
             request.scheme().setString("https");
diff --git a/test/org/apache/catalina/valves/TestRequestFilterValve.java b/test/org/apache/catalina/valves/TestRequestFilterValve.java
index 3f076c8..aae1cc6 100644
--- a/test/org/apache/catalina/valves/TestRequestFilterValve.java
+++ b/test/org/apache/catalina/valves/TestRequestFilterValve.java
@@ -67,6 +67,7 @@ public class TestRequestFilterValve {
     private static final String CIDR6_NO_ALLOW_NO_DENY = "1:0:0:0:0:0:0:1";
 
     private static final int PORT = 8080;
+    private static final String ADDR_OTHER = "1.2.3.4";
     private static final String PORT_MATCH_PATTERN    = ";\\d*";
     private static final String PORT_NO_MATCH_PATTERN = ";8081";
 
@@ -91,9 +92,22 @@ public class TestRequestFilterValve {
         }
     }
 
+    private void twoTests(String allow, String deny, boolean denyStatus,
+                         boolean addConnectorPort,
+                         boolean auth, String property, String type,
+                         boolean allowed) {
+        oneTest(allow, deny, denyStatus, addConnectorPort, false,
+                auth, property, type, allowed);
+        if (!type.equals("Host")) {
+            oneTest(allow, deny, denyStatus, addConnectorPort, true,
+                    auth, property, type, allowed);
+        }
+    }
+
     private void oneTest(String allow, String deny, boolean denyStatus,
-                         boolean addConnectorPort, boolean auth,
-                         String property, String type, boolean allowed) {
+                         boolean addConnectorPort, boolean usePeerAddress,
+                         boolean auth, String property, String type,
+                         boolean allowed) {
         // PREPARE
         RequestFilterValve valve = null;
         Connector connector = new Connector();
@@ -109,19 +123,38 @@ public class TestRequestFilterValve {
         request.setCoyoteRequest(new org.apache.coyote.Request());
 
         Assert.assertNotNull("Invalid test with null type", type);
+
+        request.setCoyoteRequest(new org.apache.coyote.Request());
+
         if (property != null) {
             if (type.equals("Addr")) {
                 valve = new RemoteAddrValve();
-                request.setRemoteAddr(property);
-                msg.append(" ip='" + property + "'");
+                if (usePeerAddress) {
+                    request.setRemoteAddr(ADDR_OTHER);
+                    request.getCoyoteRequest().peerAddr().setString(property);
+                    ((RemoteAddrValve)valve).setUsePeerAddress(true);
+                    msg.append(" peer='" + property + "'");
+                } else {
+                    request.setRemoteAddr(property);
+                    request.getCoyoteRequest().peerAddr().setString(ADDR_OTHER);
+                    msg.append(" ip='" + property + "'");
+                }
             } else if (type.equals("Host")) {
                 valve = new RemoteHostValve();
                 request.setRemoteHost(property);
                 msg.append(" host='" + property + "'");
             } else if (type.equals("CIDR")) {
                 valve = new RemoteCIDRValve();
-                request.setRemoteAddr(property);
-                msg.append(" ip='" + property + "'");
+                if (usePeerAddress) {
+                    request.setRemoteAddr(ADDR_OTHER);
+                    request.getCoyoteRequest().peerAddr().setString(property);
+                    ((RemoteCIDRValve)valve).setUsePeerAddress(true);
+                    msg.append(" peer='" + property + "'");
+                } else {
+                    request.setRemoteAddr(property);
+                    request.getCoyoteRequest().peerAddr().setString(ADDR_OTHER);
+                    msg.append(" ip='" + property + "'");
+                }
             }
         }
         Assert.assertNotNull("Invalid test type" + type, valve);
@@ -186,156 +219,156 @@ public class TestRequestFilterValve {
         // Test without ports
         apat = allow_pat;
         dpat = deny_pat;
-        oneTest(null, null, false, false, auth, AllowAndDeny,  type, false);
-        oneTest(null, null, true,  false, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, false, auth, AllowAndDeny,  type, true);
-        oneTest(apat, null, false, false, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, null, true,  false, auth, AllowAndDeny,  type, true);
-        oneTest(apat, null, true,  false, auth, NoAllowNoDeny, type, false);
-        oneTest(null, dpat, false, false, auth, AllowAndDeny,  type, false);
-        oneTest(null, dpat, false, false, auth, NoAllowNoDeny, type, true);
-        oneTest(null, dpat, true,  false, auth, AllowAndDeny,  type, false);
-        oneTest(null, dpat, true,  false, auth, NoAllowNoDeny, type, true);
-        oneTest(apat, dpat, false, false, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, false, false, auth, OnlyAllow,     type, true);
-        oneTest(apat, dpat, false, false, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, false, false, auth, AllowAndDeny,  type, false);
-        oneTest(apat, dpat, true,  false, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, true,  false, auth, OnlyAllow,     type, true);
-        oneTest(apat, dpat, true,  false, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, true,  false, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, false, false, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, true,  false, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, false, auth, AllowAndDeny,  type, true);
+        twoTests(apat, null, false, false, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, null, true,  false, auth, AllowAndDeny,  type, true);
+        twoTests(apat, null, true,  false, auth, NoAllowNoDeny, type, false);
+        twoTests(null, dpat, false, false, auth, AllowAndDeny,  type, false);
+        twoTests(null, dpat, false, false, auth, NoAllowNoDeny, type, true);
+        twoTests(null, dpat, true,  false, auth, AllowAndDeny,  type, false);
+        twoTests(null, dpat, true,  false, auth, NoAllowNoDeny, type, true);
+        twoTests(apat, dpat, false, false, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, false, false, auth, OnlyAllow,     type, true);
+        twoTests(apat, dpat, false, false, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, false, false, auth, AllowAndDeny,  type, false);
+        twoTests(apat, dpat, true,  false, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, true,  false, auth, OnlyAllow,     type, true);
+        twoTests(apat, dpat, true,  false, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, true,  false, auth, AllowAndDeny,  type, false);
 
         // Test with port in pattern but forgotten "addConnectorPort"
         apat = allow_pat + PORT_MATCH_PATTERN;
         dpat = deny_pat + PORT_MATCH_PATTERN;
-        oneTest(null, null, false, false, auth, AllowAndDeny,  type, false);
-        oneTest(null, null, true,  false, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, false, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, false, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, null, true,  false, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, true,  false, auth, NoAllowNoDeny, type, false);
-        oneTest(null, dpat, false, false, auth, AllowAndDeny,  type, true);
-        oneTest(null, dpat, false, false, auth, NoAllowNoDeny, type, true);
-        oneTest(null, dpat, true,  false, auth, AllowAndDeny,  type, true);
-        oneTest(null, dpat, true,  false, auth, NoAllowNoDeny, type, true);
-        oneTest(apat, dpat, false, false, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, false, false, auth, OnlyAllow,     type, false);
-        oneTest(apat, dpat, false, false, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, false, false, auth, AllowAndDeny,  type, false);
-        oneTest(apat, dpat, true,  false, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, true,  false, auth, OnlyAllow,     type, false);
-        oneTest(apat, dpat, true,  false, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, true,  false, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, false, false, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, true,  false, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, false, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, false, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, null, true,  false, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, true,  false, auth, NoAllowNoDeny, type, false);
+        twoTests(null, dpat, false, false, auth, AllowAndDeny,  type, true);
+        twoTests(null, dpat, false, false, auth, NoAllowNoDeny, type, true);
+        twoTests(null, dpat, true,  false, auth, AllowAndDeny,  type, true);
+        twoTests(null, dpat, true,  false, auth, NoAllowNoDeny, type, true);
+        twoTests(apat, dpat, false, false, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, false, false, auth, OnlyAllow,     type, false);
+        twoTests(apat, dpat, false, false, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, false, false, auth, AllowAndDeny,  type, false);
+        twoTests(apat, dpat, true,  false, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, true,  false, auth, OnlyAllow,     type, false);
+        twoTests(apat, dpat, true,  false, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, true,  false, auth, AllowAndDeny,  type, false);
 
         // Test with "addConnectorPort" but port not in pattern
         apat = allow_pat;
         dpat = deny_pat;
-        oneTest(null, null, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(null, null, true,  true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, null, true,  true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, true,  true, auth, NoAllowNoDeny, type, false);
-        oneTest(null, dpat, false, true, auth, AllowAndDeny,  type, true);
-        oneTest(null, dpat, false, true, auth, NoAllowNoDeny, type, true);
-        oneTest(null, dpat, true,  true, auth, AllowAndDeny,  type, true);
-        oneTest(null, dpat, true,  true, auth, NoAllowNoDeny, type, true);
-        oneTest(apat, dpat, false, true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, false, true, auth, OnlyAllow,     type, false);
-        oneTest(apat, dpat, false, true, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, dpat, true,  true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, true,  true, auth, OnlyAllow,     type, false);
-        oneTest(apat, dpat, true,  true, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, null, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, true,  true, auth, NoAllowNoDeny, type, false);
+        twoTests(null, dpat, false, true, auth, AllowAndDeny,  type, true);
+        twoTests(null, dpat, false, true, auth, NoAllowNoDeny, type, true);
+        twoTests(null, dpat, true,  true, auth, AllowAndDeny,  type, true);
+        twoTests(null, dpat, true,  true, auth, NoAllowNoDeny, type, true);
+        twoTests(apat, dpat, false, true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, false, true, auth, OnlyAllow,     type, false);
+        twoTests(apat, dpat, false, true, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, dpat, true,  true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, true,  true, auth, OnlyAllow,     type, false);
+        twoTests(apat, dpat, true,  true, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, true,  true, auth, AllowAndDeny,  type, false);
 
         // Test "addConnectorPort" and with port matching in both patterns
         apat = allow_pat + PORT_MATCH_PATTERN;
         dpat = deny_pat + PORT_MATCH_PATTERN;
-        oneTest(null, null, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(null, null, true,  true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, true, auth, AllowAndDeny,  type, true);
-        oneTest(apat, null, false, true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, null, true,  true, auth, AllowAndDeny,  type, true);
-        oneTest(apat, null, true,  true, auth, NoAllowNoDeny, type, false);
-        oneTest(null, dpat, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(null, dpat, false, true, auth, NoAllowNoDeny, type, true);
-        oneTest(null, dpat, true,  true, auth, AllowAndDeny,  type, false);
-        oneTest(null, dpat, true,  true, auth, NoAllowNoDeny, type, true);
-        oneTest(apat, dpat, false, true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, false, true, auth, OnlyAllow,     type, true);
-        oneTest(apat, dpat, false, true, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, dpat, true,  true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, true,  true, auth, OnlyAllow,     type, true);
-        oneTest(apat, dpat, true,  true, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, true, auth, AllowAndDeny,  type, true);
+        twoTests(apat, null, false, true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, null, true,  true, auth, AllowAndDeny,  type, true);
+        twoTests(apat, null, true,  true, auth, NoAllowNoDeny, type, false);
+        twoTests(null, dpat, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(null, dpat, false, true, auth, NoAllowNoDeny, type, true);
+        twoTests(null, dpat, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(null, dpat, true,  true, auth, NoAllowNoDeny, type, true);
+        twoTests(apat, dpat, false, true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, false, true, auth, OnlyAllow,     type, true);
+        twoTests(apat, dpat, false, true, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, dpat, true,  true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, true,  true, auth, OnlyAllow,     type, true);
+        twoTests(apat, dpat, true,  true, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, true,  true, auth, AllowAndDeny,  type, false);
 
         // Test "addConnectorPort" and with port not matching in both patterns
         apat = allow_pat + PORT_NO_MATCH_PATTERN;
         dpat = deny_pat + PORT_NO_MATCH_PATTERN;
-        oneTest(null, null, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(null, null, true,  true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, null, true,  true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, true,  true, auth, NoAllowNoDeny, type, false);
-        oneTest(null, dpat, false, true, auth, AllowAndDeny,  type, true);
-        oneTest(null, dpat, false, true, auth, NoAllowNoDeny, type, true);
-        oneTest(null, dpat, true,  true, auth, AllowAndDeny,  type, true);
-        oneTest(null, dpat, true,  true, auth, NoAllowNoDeny, type, true);
-        oneTest(apat, dpat, false, true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, false, true, auth, OnlyAllow,     type, false);
-        oneTest(apat, dpat, false, true, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, dpat, true,  true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, true,  true, auth, OnlyAllow,     type, false);
-        oneTest(apat, dpat, true,  true, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, null, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, true,  true, auth, NoAllowNoDeny, type, false);
+        twoTests(null, dpat, false, true, auth, AllowAndDeny,  type, true);
+        twoTests(null, dpat, false, true, auth, NoAllowNoDeny, type, true);
+        twoTests(null, dpat, true,  true, auth, AllowAndDeny,  type, true);
+        twoTests(null, dpat, true,  true, auth, NoAllowNoDeny, type, true);
+        twoTests(apat, dpat, false, true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, false, true, auth, OnlyAllow,     type, false);
+        twoTests(apat, dpat, false, true, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, dpat, true,  true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, true,  true, auth, OnlyAllow,     type, false);
+        twoTests(apat, dpat, true,  true, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, true,  true, auth, AllowAndDeny,  type, false);
 
         // Test "addConnectorPort" and with port matching only in allow
         apat = allow_pat + PORT_MATCH_PATTERN;
         dpat = deny_pat + PORT_NO_MATCH_PATTERN;
-        oneTest(null, null, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(null, null, true,  true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, true, auth, AllowAndDeny,  type, true);
-        oneTest(apat, null, false, true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, null, true,  true, auth, AllowAndDeny,  type, true);
-        oneTest(apat, null, true,  true, auth, NoAllowNoDeny, type, false);
-        oneTest(null, dpat, false, true, auth, AllowAndDeny,  type, true);
-        oneTest(null, dpat, false, true, auth, NoAllowNoDeny, type, true);
-        oneTest(null, dpat, true,  true, auth, AllowAndDeny,  type, true);
-        oneTest(null, dpat, true,  true, auth, NoAllowNoDeny, type, true);
-        oneTest(apat, dpat, false, true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, false, true, auth, OnlyAllow,     type, true);
-        oneTest(apat, dpat, false, true, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, false, true, auth, AllowAndDeny,  type, true);
-        oneTest(apat, dpat, true,  true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, true,  true, auth, OnlyAllow,     type, true);
-        oneTest(apat, dpat, true,  true, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, true,  true, auth, AllowAndDeny,  type, true);
+        twoTests(null, null, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, true, auth, AllowAndDeny,  type, true);
+        twoTests(apat, null, false, true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, null, true,  true, auth, AllowAndDeny,  type, true);
+        twoTests(apat, null, true,  true, auth, NoAllowNoDeny, type, false);
+        twoTests(null, dpat, false, true, auth, AllowAndDeny,  type, true);
+        twoTests(null, dpat, false, true, auth, NoAllowNoDeny, type, true);
+        twoTests(null, dpat, true,  true, auth, AllowAndDeny,  type, true);
+        twoTests(null, dpat, true,  true, auth, NoAllowNoDeny, type, true);
+        twoTests(apat, dpat, false, true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, false, true, auth, OnlyAllow,     type, true);
+        twoTests(apat, dpat, false, true, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, false, true, auth, AllowAndDeny,  type, true);
+        twoTests(apat, dpat, true,  true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, true,  true, auth, OnlyAllow,     type, true);
+        twoTests(apat, dpat, true,  true, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, true,  true, auth, AllowAndDeny,  type, true);
 
         // Test "addConnectorPort" and with port matching only in deny
         apat = allow_pat + PORT_NO_MATCH_PATTERN;
         dpat = deny_pat + PORT_MATCH_PATTERN;
-        oneTest(null, null, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(null, null, true,  true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, false, true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, null, true,  true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, null, true,  true, auth, NoAllowNoDeny, type, false);
-        oneTest(null, dpat, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(null, dpat, false, true, auth, NoAllowNoDeny, type, true);
-        oneTest(null, dpat, true,  true, auth, AllowAndDeny,  type, false);
-        oneTest(null, dpat, true,  true, auth, NoAllowNoDeny, type, true);
-        oneTest(apat, dpat, false, true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, false, true, auth, OnlyAllow,     type, false);
-        oneTest(apat, dpat, false, true, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, false, true, auth, AllowAndDeny,  type, false);
-        oneTest(apat, dpat, true,  true, auth, NoAllowNoDeny, type, false);
-        oneTest(apat, dpat, true,  true, auth, OnlyAllow,     type, false);
-        oneTest(apat, dpat, true,  true, auth, OnlyDeny,      type, false);
-        oneTest(apat, dpat, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(null, null, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, false, true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, null, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, null, true,  true, auth, NoAllowNoDeny, type, false);
+        twoTests(null, dpat, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(null, dpat, false, true, auth, NoAllowNoDeny, type, true);
+        twoTests(null, dpat, true,  true, auth, AllowAndDeny,  type, false);
+        twoTests(null, dpat, true,  true, auth, NoAllowNoDeny, type, true);
+        twoTests(apat, dpat, false, true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, false, true, auth, OnlyAllow,     type, false);
+        twoTests(apat, dpat, false, true, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, false, true, auth, AllowAndDeny,  type, false);
+        twoTests(apat, dpat, true,  true, auth, NoAllowNoDeny, type, false);
+        twoTests(apat, dpat, true,  true, auth, OnlyAllow,     type, false);
+        twoTests(apat, dpat, true,  true, auth, OnlyDeny,      type, false);
+        twoTests(apat, dpat, true,  true, auth, AllowAndDeny,  type, false);
     }
 
     @Test
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 96be4a9..645b070 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -129,6 +129,22 @@
         Especially add support for connector specific configuration
         using <code>addConnectorPort</code>. (rjung)
       </add>
+      <add>
+        Add <code>peerAddress</code> to coyote request, which contains
+        the IP address of the direct connection peer. If a reverse proxy
+        sits in front of Tomcat and the protocol used is AJP or HTTP
+        in combination with the <code>RemoteIp(Valve|Filter)</code>,
+        the peer address might differ from the <code>remoteAddress</code>.
+        The latter then contains the address of the client in front of the
+        reverse proxy, not the address of the proxy itself.
+        Support for the peer address has been added to the
+        RemoteAddrValve and RemoteCIDRValve with the new attribute
+        <code>usePeerAddress</code>. This can be used to restrict access
+        to Tomcat based on the reverse proxy IP address, which is especially
+        useful to harden access to AJP connectors. The peer address can also
+        be logged in the access log using the new <code>%{peer}a</code>
+        syntax. (rjung)
+      </add>
     </changelog>
   </subsection>
   <subsection name="Jasper">
diff --git a/webapps/docs/config/valve.xml b/webapps/docs/config/valve.xml
index 35c71a9..4c04caf 100644
--- a/webapps/docs/config/valve.xml
+++ b/webapps/docs/config/valve.xml
@@ -284,7 +284,8 @@
     the current request and response.  The following pattern codes are
     supported:</p>
     <ul>
-    <li><b>%a</b> - Remote IP address</li>
+    <li><b>%a</b> - Remote IP address.
+        See also <code>%{xxx}a</code> below.</li>
     <li><b>%A</b> - Local IP address</li>
     <li><b>%b</b> - Bytes sent, excluding HTTP headers, or '-' if zero</li>
     <li><b>%B</b> - Bytes sent, excluding HTTP headers</li>
@@ -331,6 +332,8 @@
     syntax. Each of them can be used multiple times with different <code>xxx</code> keys:
     </p>
     <ul>
+    <li><b><code>%{xxx}a</code></b> write remote address (client) (<code>xxx==remote</code>) or
+        connection peer address (<code>xxx=peer</code>)</li>
     <li><b><code>%{xxx}i</code></b> write value of incoming header with name <code>xxx</code> (escaped if required)</li>
     <li><b><code>%{xxx}o</code></b> write value of outgoing header with name <code>xxx</code> (escaped if required)</li>
     <li><b><code>%{xxx}c</code></b> write value of cookie with name <code>xxx</code> (escaped if required)</li>
@@ -530,6 +533,12 @@
     <code>true</code>, one can append the server connector port separated with a
     semicolon (";") to allow different expressions for each connector.</p>
 
+    <p>By setting the attribute <code>usePeerAddress</code> to
+    <code>true</code>, the valve will use the connection peer address in its
+    checks. This will differ from the client IP, if a reverse proxy is used
+    in front of Tomcat in combination with either the AJP protocol, or the
+    HTTP protocol plus the <code>RemoteIp(Valve|Filter)</code>.</p>
+
     <p>A refused request will be answered a response with status code
     <code>403</code>. This status code can be overwritten using the attribute
     <code>denyStatus</code>.</p>
@@ -610,6 +619,13 @@
         depending on the client and the connector that is used to access an application.</p>
       </attribute>
 
+      <attribute name="usePeerAddress" required="false">
+        <p>Use the connection peer address instead of the client IP address.
+        They will differ, if a reverse proxy is used in front of Tomcat in
+        combination with either the AJP protocol, or the HTTP protocol plus
+        the <code>RemoteIp(Valve|Filter)</code>.</p>
+      </attribute>
+
     </attributes>
 
   </subsection>
@@ -789,6 +805,12 @@
     <code>true</code>, one can append the server connector port separated with a
     semicolon (";") to allow different expressions for each connector.</p>
 
+    <p>By setting the attribute <code>usePeerAddress</code> to
+    <code>true</code>, the valve will use the connection peer address in its
+    checks. This will differ from the client IP, if a reverse proxy is used
+    in front of Tomcat in combination with either the AJP protocol, or the
+    HTTP protocol plus the <code>RemoteIp(Valve|Filter)</code>.</p>
+
     <p>A refused request will be answered a response with status code
     <code>403</code>. This status code can be overwritten using the attribute
     <code>denyStatus</code>.</p>
@@ -875,6 +897,13 @@
         depending on the client and the connector that is used to access an application.</p>
       </attribute>
 
+      <attribute name="usePeerAddress" required="false">
+        <p>Use the connection peer address instead of the client IP address.
+        They will differ, if a reverse proxy is used in front of Tomcat in
+        combination with either the AJP protocol, or the HTTP protocol plus
+        the <code>RemoteIp(Valve|Filter)</code>.</p>
+      </attribute>
+
     </attributes>
 
   </subsection>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org