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

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


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

commit abb3b595a2cd668d96fb23411568a57837d74306
Author: Rainer Jung <rainer.j...@kippdata.de>
AuthorDate: Thu Dec 10 15:40:13 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    |   9 +-
 .../apache/catalina/valves/RemoteCIDRValve.java    |   9 +-
 .../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(+), 147 deletions(-)

diff --git a/java/org/apache/catalina/connector/Request.java 
b/java/org/apache/catalina/connector/Request.java
index cd6e343..3172a19 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -411,6 +411,12 @@ public class Request implements HttpServletRequest {
 
 
     /**
+     * Connection peer address.
+     */
+    protected String peerAddr = null;
+
+
+    /**
      * Remote host.
      */
     protected String remoteHost = null;
@@ -487,6 +493,7 @@ public class Request implements HttpServletRequest {
         localesParsed = false;
         secure = false;
         remoteAddr = null;
+        peerAddr = null;
         remoteHost = null;
         remotePort = -1;
         localPort = -1;
@@ -1301,6 +1308,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 b204001..2f6d635 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);
@@ -865,19 +872,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) {
@@ -889,7 +927,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();
+                }
             }
         }
     }
@@ -1732,6 +1774,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 7dfd92e..fe373fd 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 f340029..cea229c 100644
--- a/java/org/apache/catalina/valves/RemoteAddrValve.java
+++ b/java/org/apache/catalina/valves/RemoteAddrValve.java
@@ -44,12 +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().getPortWithOffset();
+        if (getUsePeerAddress()) {
+            property = request.getPeerAddr();
         } else {
             property = request.getRequest().getRemoteAddr();
         }
+        if (getAddConnectorPort()) {
+            property = property + ";" +
+                request.getConnector().getPortWithOffset();
+        }
         process(property, request, response);
     }
 
diff --git a/java/org/apache/catalina/valves/RemoteCIDRValve.java 
b/java/org/apache/catalina/valves/RemoteCIDRValve.java
index 120564d..8d55757 100644
--- a/java/org/apache/catalina/valves/RemoteCIDRValve.java
+++ b/java/org/apache/catalina/valves/RemoteCIDRValve.java
@@ -129,12 +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().getPortWithOffset();
+        if (getUsePeerAddress()) {
+            property = request.getPeerAddr();
         } else {
             property = request.getRequest().getRemoteAddr();
         }
+        if (getAddConnectorPort()) {
+            property = property + ";" +
+                request.getConnector().getPortWithOffset();
+        }
         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 b214cad..520bea0 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -451,6 +451,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 5c5af4f..c33fc20 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 1475ba7..a431968 100644
--- a/java/org/apache/coyote/Constants.java
+++ b/java/org/apache/coyote/Constants.java
@@ -96,4 +96,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 c5b0117..eea1487 100644
--- a/java/org/apache/coyote/Request.java
+++ b/java/org/apache/coyote/Request.java
@@ -94,6 +94,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();
@@ -267,6 +268,10 @@ public final class Request {
         return remoteAddrMB;
     }
 
+    public MessageBytes peerAddr() {
+        return peerAddrMB;
+    }
+
     public MessageBytes remoteHost() {
         return remoteHostMB;
     }
@@ -624,6 +629,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 7094133..c99ad9e 100644
--- a/java/org/apache/coyote/ajp/AjpProcessor.java
+++ b/java/org/apache/coyote/ajp/AjpProcessor.java
@@ -665,6 +665,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 179b7cb..730b913 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();
@@ -108,19 +122,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);
@@ -185,156 +218,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 0e485c5..1cf63f4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -122,6 +122,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 011586a..5f05189 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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to