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