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

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


The following commit(s) were added to refs/heads/7.0.x by this push:
     new 40d5d93  Rename requiredSecret to secret and add secretRequired
40d5d93 is described below

commit 40d5d93bd284033cf4a1f77f5492444f83d803e2
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jan 21 14:24:33 2020 +0000

    Rename requiredSecret to secret and add secretRequired
    
    AJP Connector will not start if secretRequired="true" and secret is set
    to null or zero length String.
---
 .../apache/coyote/ajp/AbstractAjpProcessor.java    | 19 +++++---
 .../org/apache/coyote/ajp/AbstractAjpProtocol.java | 50 +++++++++++++++++++++-
 java/org/apache/coyote/ajp/AjpAprProtocol.java     |  2 +-
 java/org/apache/coyote/ajp/AjpNioProtocol.java     |  2 +-
 java/org/apache/coyote/ajp/AjpProtocol.java        |  2 +-
 java/org/apache/coyote/ajp/LocalStrings.properties |  1 +
 .../coyote/ajp/TestAbstractAjpProcessor.java       | 11 +++++
 webapps/docs/changelog.xml                         |  8 ++++
 webapps/docs/config/ajp.xml                        | 12 +++++-
 9 files changed, 96 insertions(+), 11 deletions(-)

diff --git a/java/org/apache/coyote/ajp/AbstractAjpProcessor.java 
b/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
index f3c84a9..e17b5a2 100644
--- a/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
+++ b/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
@@ -300,9 +300,16 @@ public abstract class AbstractAjpProcessor<S> extends 
AbstractProcessor<S> {
     /**
      * Required secret.
      */
+    @Deprecated
     protected String requiredSecret = null;
+    protected String secret = null;
+    public void setSecret(String secret) {
+        this.secret = secret;
+        this.requiredSecret = secret;
+    }
+    @Deprecated
     public void setRequiredSecret(String requiredSecret) {
-        this.requiredSecret = requiredSecret;
+        setSecret(requiredSecret);
     }
 
 
@@ -831,7 +838,7 @@ public abstract class AbstractAjpProcessor<S> extends 
AbstractProcessor<S> {
         }
 
         // Decode extra attributes
-        boolean secret = false;
+        boolean secretPresentInRequest = false;
         byte attributeCode;
         while ((attributeCode = requestHeaderMessage.getByte())
                 != Constants.SC_A_ARE_DONE) {
@@ -934,9 +941,9 @@ public abstract class AbstractAjpProcessor<S> extends 
AbstractProcessor<S> {
 
             case Constants.SC_A_SECRET:
                 requestHeaderMessage.getBytes(tmpMB);
-                if (requiredSecret != null) {
-                    secret = true;
-                    if (!tmpMB.equals(requiredSecret)) {
+                if (secret != null) {
+                    secretPresentInRequest = true;
+                    if (!tmpMB.equals(secret)) {
                         response.setStatus(403);
                         setErrorState(ErrorState.CLOSE_CLEAN, null);
                     }
@@ -952,7 +959,7 @@ public abstract class AbstractAjpProcessor<S> extends 
AbstractProcessor<S> {
         }
 
         // Check if secret was submitted if required
-        if ((requiredSecret != null) && !secret) {
+        if ((secret != null) && !secretPresentInRequest) {
             response.setStatus(403);
             setErrorState(ErrorState.CLOSE_CLEAN, null);
         }
diff --git a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java 
b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java
index 7c644db..c5e7335 100644
--- a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java
+++ b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java
@@ -95,13 +95,49 @@ public abstract class AbstractAjpProtocol<S> extends 
AbstractProtocol<S> {
 
 
     protected String requiredSecret = null;
+    private String secret = null;
+    /**
+     * Set the secret that must be included with every request.
+     *
+     * @param secret The required secret
+     */
+    public void setSecret(String secret) {
+        this.secret = secret;
+        this.requiredSecret = secret;
+    }
+    protected String getSecret() {
+        return secret;
+    }
     /**
      * Set the required secret that must be included with every request.
      *
      * @param requiredSecret The required secret
+     *
+     * @deprecated Replaced by {@link #setSecret(String)}.
+     *             Will be removed in Tomcat 11 onwards
      */
+    @Deprecated
     public void setRequiredSecret(String requiredSecret) {
-        this.requiredSecret = requiredSecret;
+        setSecret(requiredSecret);
+    }
+    /**
+     * @return The current secret
+     *
+     * @deprecated Replaced by {@link #getSecret()}.
+     *             Will be removed in Tomcat 11 onwards
+     */
+    @Deprecated
+    protected String getRequiredSecret() {
+        return getSecret();
+    }
+
+
+    private boolean secretRequired = true;
+    public void setSecretRequired(boolean secretRequired) {
+        this.secretRequired = secretRequired;
+    }
+    public boolean getSecretRequired() {
+        return secretRequired;
     }
 
 
@@ -151,4 +187,16 @@ public abstract class AbstractAjpProtocol<S> extends 
AbstractProtocol<S> {
             return null;
         }
     }
+
+
+    @Override
+    public void init() throws Exception {
+        if (getSecretRequired()) {
+            String secret = getSecret();
+            if (secret == null || secret.length() == 0) {
+                throw new 
IllegalArgumentException(sm.getString("ajpprotocol.nosecret"));
+            }
+        }
+        super.init();
+    }
 }
diff --git a/java/org/apache/coyote/ajp/AjpAprProtocol.java 
b/java/org/apache/coyote/ajp/AjpAprProtocol.java
index 418d2ca..887a343 100644
--- a/java/org/apache/coyote/ajp/AjpAprProtocol.java
+++ b/java/org/apache/coyote/ajp/AjpAprProtocol.java
@@ -150,7 +150,7 @@ public class AjpAprProtocol extends 
AbstractAjpProtocol<Long> {
             processor.setAjpFlush(proto.getAjpFlush());
             processor.setTomcatAuthentication(proto.tomcatAuthentication);
             processor.setTomcatAuthorization(proto.getTomcatAuthorization());
-            processor.setRequiredSecret(proto.requiredSecret);
+            processor.setSecret(proto.getSecret());
             processor.setKeepAliveTimeout(proto.getKeepAliveTimeout());
             processor.setClientCertProvider(proto.getClientCertProvider());
             processor.setMaxCookieCount(proto.getMaxCookieCount());
diff --git a/java/org/apache/coyote/ajp/AjpNioProtocol.java 
b/java/org/apache/coyote/ajp/AjpNioProtocol.java
index 8668323..c6f4843 100644
--- a/java/org/apache/coyote/ajp/AjpNioProtocol.java
+++ b/java/org/apache/coyote/ajp/AjpNioProtocol.java
@@ -180,7 +180,7 @@ public class AjpNioProtocol extends 
AbstractAjpProtocol<NioChannel> {
             processor.setAjpFlush(proto.getAjpFlush());
             processor.setTomcatAuthentication(proto.tomcatAuthentication);
             processor.setTomcatAuthorization(proto.getTomcatAuthorization());
-            processor.setRequiredSecret(proto.requiredSecret);
+            processor.setSecret(proto.getSecret());
             processor.setKeepAliveTimeout(proto.getKeepAliveTimeout());
             processor.setClientCertProvider(proto.getClientCertProvider());
             processor.setMaxCookieCount(proto.getMaxCookieCount());
diff --git a/java/org/apache/coyote/ajp/AjpProtocol.java 
b/java/org/apache/coyote/ajp/AjpProtocol.java
index 69c24eb..37a7e93 100644
--- a/java/org/apache/coyote/ajp/AjpProtocol.java
+++ b/java/org/apache/coyote/ajp/AjpProtocol.java
@@ -140,7 +140,7 @@ public class AjpProtocol extends 
AbstractAjpProtocol<Socket> {
             processor.setAjpFlush(proto.getAjpFlush());
             processor.setTomcatAuthentication(proto.tomcatAuthentication);
             processor.setTomcatAuthorization(proto.getTomcatAuthorization());
-            processor.setRequiredSecret(proto.requiredSecret);
+            processor.setSecret(proto.getSecret());
             processor.setKeepAliveTimeout(proto.getKeepAliveTimeout());
             processor.setClientCertProvider(proto.getClientCertProvider());
             processor.setMaxCookieCount(proto.getMaxCookieCount());
diff --git a/java/org/apache/coyote/ajp/LocalStrings.properties 
b/java/org/apache/coyote/ajp/LocalStrings.properties
index b725b79..dfd8fa3 100644
--- a/java/org/apache/coyote/ajp/LocalStrings.properties
+++ b/java/org/apache/coyote/ajp/LocalStrings.properties
@@ -37,5 +37,6 @@ ajpprocessor.ssl.notsupported=The SSL protocol is not 
supported by this connecto
 ajpprotocol.endpoint.starterror=Error starting endpoint
 ajpprotocol.failedwrite=Socket write failed
 ajpprotocol.init=Initializing Coyote AJP/1.3 on [{0}]
+ajpprotocol.nosecret=The AJP Connector is configured with 
secretRequired="true" but the secret attribute is either null or "". This 
combination is not valid.
 ajpprotocol.request.register=Error registering request processor in JMX
 ajpprotocol.start=Starting Coyote AJP/1.3 on [{0}]
diff --git a/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java 
b/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java
index 93221e7..08dc455 100644
--- a/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java
+++ b/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java
@@ -30,14 +30,25 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 
 import org.apache.catalina.Context;
+import org.apache.catalina.connector.Connector;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
 
 public class TestAbstractAjpProcessor extends TomcatBaseTest {
 
+    @Before
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+
+        Connector c = getTomcatInstance().getConnector();
+        c.setProperty("secretRequired", "false");
+    }
+
     @Override
     protected String getProtocol() {
         /*
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 6760df4..1aa5114 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -147,6 +147,14 @@
         Change the default bind address for the AJP/1.3 connector to be the
         loopback address. (markt)
       </update>
+      <add>
+        Rename the <code>requiredSecret</code> attribute of the AJP/1.3
+        Connector to <code>secret</code> and add a new attribute
+        <code>secretRequired</code> that defaults to <code>true</code>. When
+        <code>secretRequired</code> is <code>true</code> the AJP/1.3 Connector
+        will not start unless the <code>secret</code> attribute is configured 
to
+        a non-null, non-zero length String. (markt)
+      </add>
     </changelog>
   </subsection>
   <subsection name="Jasper">
diff --git a/webapps/docs/config/ajp.xml b/webapps/docs/config/ajp.xml
index 90ed2c5..615a0f3 100644
--- a/webapps/docs/config/ajp.xml
+++ b/webapps/docs/config/ajp.xml
@@ -431,8 +431,18 @@
       expected concurrent requests (synchronous and asynchronous).</p>
     </attribute>
 
-    <attribute name="requiredSecret" required="false">
+    <attribute name="secret" required="false">
       <p>Only requests from workers with this secret keyword will be accepted.
+      The default value is <code>null</code>. This attrbute must be specified
+      with a non-null, non-zero length value unless
+      <strong>secretRequired</strong> is explicitly configured to be
+      <code>false</code>.</p>
+    </attribute>
+
+    <attribute name="secretRequired" required="false">
+      <p>If this attribute is <code>true</code>, the AJP Connector will only
+      start if the <strong>secret</strong> attribute is configured with a
+      non-null, non-zero length value. The default value is <code>true</code>.
       </p>
     </attribute>
 


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

Reply via email to