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 · Feb 4, 2020 · 9ac9053 · 9ac9053
1 parent 0e8a50f
commit 9ac9053
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 10 deletions.
diff --git a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java
@@ -143,17 +143,48 @@ public void setTomcatAuthorization(boolean tomcatAuthorization) {
     }
 
 
-    private 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;
+    }
+    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 requiredSecret;
+        return getSecret();
+    }
+
+
+    private boolean secretRequired = true;
+    public void setSecretRequired(boolean secretRequired) {
+        this.secretRequired = secretRequired;
+    }
+    public boolean getSecretRequired() {
+        return secretRequired;
     }
 
 
@@ -210,4 +241,16 @@ protected Processor createUpgradeProcessor(SocketWrapperBase<?> socket,
         throw new IllegalStateException(sm.getString("ajpprotocol.noUpgradeHandler",
                 upgradeToken.getHttpUpgradeHandler().getClass().getName()));
     }
+
+
+    @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/AjpProcessor.java b/java/org/apache/coyote/ajp/AjpProcessor.java
@@ -698,8 +698,8 @@ private void prepareRequest() {
         }
 
         // Decode extra attributes
-        String requiredSecret = protocol.getRequiredSecret();
-        boolean secret = false;
+        String secret = protocol.getSecret();
+        boolean secretPresentInRequest = false;
         byte attributeCode;
         while ((attributeCode = requestHeaderMessage.getByte())
                 != Constants.SC_A_ARE_DONE) {
@@ -801,9 +801,9 @@ private void prepareRequest() {
 
             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);
                     }
@@ -819,7 +819,7 @@ private void prepareRequest() {
         }
 
         // 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/LocalStrings.properties b/java/org/apache/coyote/ajp/LocalStrings.properties
@@ -28,5 +28,6 @@ ajpprocessor.request.prepare=Error preparing request
 ajpprocessor.request.process=Error processing request
 
 ajpprotocol.noSSL=SSL is not supported with AJP. The SSL host configuration for [{0}] was ignored
+ajpprotocol.nosecret=The AJP Connector is configured with secretRequired="true" but the secret attribute is either null or "". This combination is not valid.
 ajpprotocol.noUpgrade=Upgrade is not supported with AJP. The UpgradeProtocol configuration for [{0}] was ignored
 ajpprotocol.noUpgradeHandler=Upgrade is not supported with AJP. The HttpUpgradeHandler [{0}] can not be processed
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -182,6 +182,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
@@ -428,8 +428,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>