File tomcat-8.0.53-CVE-2020-1938.patch of Package tomcat.28070

Overview Repositories Revisions Requests Users Attributes Meta

File tomcat-8.0.53-CVE-2020-1938.patch of Package tomcat.28070

From 81928922795ad1df74f08db9c5139bfa0acae3b8 Mon Sep 17 00:00:00 2001
From: Matei Albu <malbu@suse.de>
Date: Tue, 3 Mar 2020 17:36:21 +0100
Subject: [PATCH] CVE-2020-1938

---
 .../apache/coyote/ajp/AbstractAjpProcessor.java    | 62 ++++++++++++++++---
 .../org/apache/coyote/ajp/AbstractAjpProtocol.java | 72 +++++++++++++++++++++-
 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 |  2 +-
 java/org/apache/tomcat/util/compat/JreCompat.java  | 27 ++++++++
 .../coyote/ajp/TestAbstractAjpProcessor.java       | 12 ++++
 webapps/docs/config/ajp.xml                        | 36 +++++++++--
 9 files changed, 201 insertions(+), 16 deletions(-)

Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
@@ -26,7 +26,12 @@ import java.security.cert.CertificateFac
 import java.security.cert.X509Certificate;
 import java.util.Iterator;
 import java.util.concurrent.LinkedBlockingDeque;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import javax.servlet.RequestDispatcher;
 import javax.servlet.http.HttpServletResponse;
@@ -86,6 +91,9 @@ public abstract class AbstractAjpProcess
 protected static final byte[] pongMessageArray;
 
 
+ private static final Set<String> javaxAttributes;
+
+
 static {
 // Allocate the end message array
 AjpMessage endMessage = new AjpMessage(16);
@@ -126,6 +134,14 @@ public abstract class AbstractAjpProcess
 pongMessageArray = new byte[pongMessage.getLen()];
 System.arraycopy(pongMessage.getBuffer(), 0, pongMessageArray,
 0, pongMessage.getLen());
+
+ // Build the Set of javax attributes
+ Set<String> s = new HashSet<String>();
+ s.add("javax.servlet.request.cipher_suite");
+ s.add("javax.servlet.request.key_size");
+ s.add("javax.servlet.request.ssl_session");
+ s.add("javax.servlet.request.X509Certificate");
+ javaxAttributes= Collections.unmodifiableSet(s);
 }
 
 
@@ -341,9 +357,16 @@ public abstract class AbstractAjpProcess
 /**
 * 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);
 }
 
 
@@ -360,9 +383,15 @@ public abstract class AbstractAjpProcess
 public String getClientCertProvider() { return clientCertProvider; }
 public void setClientCertProvider(String s) { this.clientCertProvider = s; }
 
- // --------------------------------------------------------- Public Methods
+
+ private Pattern allowedRequestAttributesPatternPattern;
+ public void setAllowedRequestAttributesPatternPattern(Pattern allowedRequestAttributesPatternPattern) {
+ this.allowedRequestAttributesPatternPattern = allowedRequestAttributesPatternPattern;
+ }
 
 
+ // --------------------------------------------------------- Public Methods
+
 /**
 * Send an action to the connector.
 *
@@ -1263,7 +1292,7 @@ public abstract class AbstractAjpProcess
 }
 
 // Decode extra attributes
- boolean secret = false;
+ boolean secretPresentInRequest = false;
 byte attributeCode;
 while ((attributeCode = requestHeaderMessage.getByte())
 != Constants.SC_A_ARE_DONE) {
@@ -1292,8 +1321,25 @@ public abstract class AbstractAjpProcess
 }
 } else if(n.equals(Constants.SC_A_SSL_PROTOCOL)) {
 request.setAttribute(SSLSupport.PROTOCOL_VERSION_KEY, v);
+ } else if (n.equals("JK_LB_ACTIVATION")) {
+ request.setAttribute(n, v);
+ } else if (javaxAttributes.contains(n)) {
+ request.setAttribute(n, v);
 } else {
- request.setAttribute(n, v );
+ // All 'known' attributes will be processed by the previous
+ // blocks. Any remaining attribute is an 'arbitrary' one.
+ if (allowedRequestAttributesPatternPattern == null) {
+ response.setStatus(403);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
+ } else {
+ Matcher m = allowedRequestAttributesPatternPattern.matcher(n);
+ if (m.matches()) {
+ request.setAttribute(n, v);
+ } else {
+ response.setStatus(403);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
+ }
+ }
 }
 break;
 
@@ -1363,9 +1409,9 @@ public abstract class AbstractAjpProcess
 
 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);
 }
@@ -1381,7 +1427,7 @@ public abstract class AbstractAjpProcess
 }
 
 // Check if secret was submitted if required
- if ((requiredSecret != null) && !secret) {
+ if ((secret != null) && !secretPresentInRequest) {
 response.setStatus(403);
 setErrorState(ErrorState.CLOSE_CLEAN, null);
 }
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/AbstractAjpProtocol.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/ajp/AbstractAjpProtocol.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/AbstractAjpProtocol.java
@@ -18,6 +18,8 @@ package org.apache.coyote.ajp;
 
 import java.nio.ByteBuffer;
 
+import java.util.regex.Pattern;
+
 import org.apache.coyote.AbstractProtocol;
 import org.apache.coyote.Processor;
 import org.apache.coyote.UpgradeToken;
@@ -87,8 +89,61 @@ public abstract class AbstractAjpProtoco
 * Required secret.
 */
 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 = false;
+ public void setSecretRequired(boolean secretRequired) {
+ this.secretRequired = secretRequired;
+ }
+ public boolean getSecretRequired() {
+ return secretRequired;
+ }
+
+
+ private Pattern allowedRequestAttributesPatternPattern;
+ public void setAllowedRequestAttributesPattern(String allowedRequestAttributesPattern) {
+ this.allowedRequestAttributesPatternPattern = Pattern.compile(allowedRequestAttributesPattern);
+ }
+ public String getAllowedRequestAttributesPattern() {
+ return allowedRequestAttributesPatternPattern.pattern();
+ }
+ protected Pattern getAllowedRequestAttributesPatternPattern() {
+ return allowedRequestAttributesPatternPattern;
 }
 
 
@@ -110,10 +165,11 @@ public abstract class AbstractAjpProtoco
 processor.setAjpFlush(getAjpFlush());
 processor.setTomcatAuthentication(getTomcatAuthentication());
 processor.setTomcatAuthorization(getTomcatAuthorization());
- processor.setRequiredSecret(requiredSecret);
+ processor.setSecret(getSecret());
 processor.setKeepAliveTimeout(getKeepAliveTimeout());
 processor.setClientCertProvider(getClientCertProvider());
 processor.setMaxCookieCount(getMaxCookieCount());
+ processor.setAllowedRequestAttributesPatternPattern(getAllowedRequestAttributesPatternPattern());
 }
 
 protected abstract static class AbstractAjpConnectionHandler<S,P extends AbstractAjpProcessor<S>>
@@ -138,4 +194,16 @@ public abstract class AbstractAjpProtoco
 return null;
 }
 }
+
+
+ @Override
+ public void start() throws Exception {
+ if (getSecretRequired()) {
+ String secret = getSecret();
+ if (secret == null || secret.length() == 0) {
+ throw new IllegalArgumentException(sm.getString("ajpprotocol.nosecret"));
+ }
+ }
+ super.start();
+ }
 }
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/AjpAprProtocol.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/ajp/AjpAprProtocol.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/AjpAprProtocol.java
@@ -20,6 +20,7 @@ import org.apache.coyote.AbstractProtoco
 import org.apache.coyote.Processor;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.compat.JreCompat;
 import org.apache.tomcat.util.net.AbstractEndpoint;
 import org.apache.tomcat.util.net.AprEndpoint;
 import org.apache.tomcat.util.net.AprEndpoint.Handler;
@@ -61,6 +62,7 @@ public class AjpAprProtocol extends Abst
 
 public AjpAprProtocol() {
 endpoint = new AprEndpoint();
+ endpoint.setAddress(JreCompat.getInstance().getLoopbackAddress());
 cHandler = new AjpConnectionHandler(this);
 ((AprEndpoint) endpoint).setHandler(cHandler);
 setSoLinger(Constants.DEFAULT_CONNECTION_LINGER);
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/AjpNioProtocol.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/ajp/AjpNioProtocol.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/AjpNioProtocol.java
@@ -23,6 +23,7 @@ import org.apache.coyote.AbstractProtoco
 import org.apache.coyote.Processor;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.compat.JreCompat;
 import org.apache.tomcat.util.net.AbstractEndpoint;
 import org.apache.tomcat.util.net.NioChannel;
 import org.apache.tomcat.util.net.NioEndpoint;
@@ -56,6 +57,7 @@ public class AjpNioProtocol extends Abst
 
 public AjpNioProtocol() {
 endpoint = new NioEndpoint();
+ endpoint.setAddress(JreCompat.getInstance().getLoopbackAddress());
 cHandler = new AjpConnectionHandler(this);
 ((NioEndpoint) endpoint).setHandler(cHandler);
 setSoLinger(Constants.DEFAULT_CONNECTION_LINGER);
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/AjpProtocol.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/ajp/AjpProtocol.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/AjpProtocol.java
@@ -22,6 +22,7 @@ import org.apache.coyote.AbstractProtoco
 import org.apache.coyote.Processor;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.compat.JreCompat;
 import org.apache.tomcat.util.net.AbstractEndpoint;
 import org.apache.tomcat.util.net.JIoEndpoint;
 import org.apache.tomcat.util.net.JIoEndpoint.Handler;
@@ -57,6 +58,7 @@ public class AjpProtocol extends Abstrac
 
 public AjpProtocol() {
 endpoint = new JIoEndpoint();
+ endpoint.setAddress(JreCompat.getInstance().getLoopbackAddress());
 cHandler = new AjpConnectionHandler(this);
 ((JIoEndpoint) endpoint).setHandler(cHandler);
 setSoLinger(Constants.DEFAULT_CONNECTION_LINGER);
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/LocalStrings.properties
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/ajp/LocalStrings.properties
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/ajp/LocalStrings.properties
@@ -25,7 +25,6 @@ ajpprocessor.readtimeout=Timeout attempt
 ajpprocessor.request.prepare=Error preparing request
 ajpprocessor.request.process=Error processing request
 ajpprocessor.certs.fail=Certificate conversion failed
-ajpprocessor.comet.notsupported=The Comet protocol is not supported by this connector
 ajpprocessor.ssl.notsupported=The SSL protocol is not supported by this connector
 ajpprocessor.httpupgrade.notsupported=HTTP upgrade is not supported by the AJP protocol
 
@@ -35,3 +34,4 @@ ajpmessage.invalid=Invalid message recei
 ajpmessage.invalidLength=Invalid message received with length {0}
 ajpMessage.invalidPos=Requested read of bytes at position [{0}] which is beyond the end of the AJP message
 
+ajpprotocol.nosecret=The AJP Connector is configured with secretRequired="true" but the secret attribute is either null or "". This combination is not valid.
Index: apache-tomcat-8.0.53-src/java/org/apache/tomcat/util/compat/JreCompat.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/tomcat/util/compat/JreCompat.java
+++ apache-tomcat-8.0.53-src/java/org/apache/tomcat/util/compat/JreCompat.java
@@ -18,8 +18,10 @@ package org.apache.tomcat.util.compat;
 
 import java.io.File;
 import java.io.IOException;
+import java.net.InetAddress;
 import java.net.URL;
 import java.net.URLConnection;
+import java.net.UnknownHostException;
 import java.util.Deque;
 import java.util.jar.JarFile;
 
@@ -69,6 +71,31 @@ public class JreCompat {
 return instance;
 }
 
+ public InetAddress getLoopbackAddress() {
+ // Javadoc for getByName() states that calling with null will return one
+ // of the loopback addresses
+ InetAddress result = null;
+ try {
+ result = InetAddress.getByName(null);
+ } catch (UnknownHostException e) {
+ // This would be unusual but ignore it in this case.
+ }
+ if (result == null) {
+ // Fallback to default IPv4 loopback address.
+ // Not perfect but good enough and if the address is not valid the
+ // bind will fail later with an appropriate error message
+ try {
+ result = InetAddress.getByName("127.0.0.1");
+ } catch (UnknownHostException e) {
+ // Unreachable.
+ // For text representations of IP addresses only the format is
+ // checked.
+ }
+ }
+
+ return result;
+ }
+
 
 // Java 7 implementation of Java 8 methods
 
Index: apache-tomcat-8.0.53-src/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java
+++ apache-tomcat-8.0.53-src/test/org/apache/coyote/ajp/TestAbstractAjpProcessor.java
@@ -33,14 +33,26 @@ import javax.servlet.http.HttpServletReq
 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");
+ c.setProperty("allowedRequestAttributesPattern", "MYATTRIBUTE.*");
+ }
+
 @Override
 protected String getProtocol() {
 /*
Index: apache-tomcat-8.0.53-src/webapps/docs/config/ajp.xml
===================================================================
--- apache-tomcat-8.0.53-src.orig/webapps/docs/config/ajp.xml
+++ apache-tomcat-8.0.53-src/webapps/docs/config/ajp.xml
@@ -310,10 +310,26 @@
 <attribute name="address" required="false">
 For servers with more than one IP address, this attribute
 specifies which address will be used for listening on the specified
- port. By default, this port will be used on all IP addresses
- associated with the server. A value of <code>127.0.0.1</code>
- indicates that the Connector will only listen on the loopback
- interface.
+ port. By default, the loopback address will be used.
+ </attribute>
+
+ <attribute name="allowedRequestAttributesPattern" required="false">
+ The AJP protocol passes some information from the reverse proxy to the
+ AJP connector using request attributes. These attributes are:
+ <ul>
+ <li>javax.servlet.request.cipher_suite</li>
+ <li>javax.servlet.request.key_size</li>
+ <li>javax.servlet.request.ssl_session</li>
+ <li>javax.servlet.request.X509Certificate</li>
+ <li>AJP_LOCAL_ADDR</li>
+ <li>AJP_REMOTE_PORT</li>
+ <li>AJP_SSL_PROTOCOL</li>
+ <li>JK_LB_ACTIVATION</li>
+ </ul>
+ The AJP protocol supports the passing of arbitrary request attributes.
+ Requests containing arbitrary request attributes will be rejected with a
+ 403 response unless the entire attribute name matches this regular
+ expression. If not specified, the default value is <code>null</code>.
 </attribute>
 
 <attribute name="bindOnInit" required="false">
@@ -441,8 +457,18 @@
 expected concurrent requests (synchronous and asynchronous).
 </attribute>
 
- <attribute name="requiredSecret" required="false">
+ <attribute name="secret" required="false">
 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
+ secretRequired is explicitly configured to be
+ <code>false</code>.
+ </attribute>
+
+ <attribute name="secretRequired" required="false">
+ If this attribute is <code>true</code>, the AJP Connector will only
+ start if the secret attribute is configured with a
+ non-null, non-zero length value. The default value is <code>true</code>.
 
 </attribute>
 
Index: apache-tomcat-8.0.53-src/conf/server.xml
===================================================================
--- apache-tomcat-8.0.53-src.orig/conf/server.xml
+++ apache-tomcat-8.0.53-src/conf/server.xml
@@ -88,8 +88,11 @@
 -->
 
 
- <Connector port="8009" protocol="AJP/1.3" redirectPort="8443" />
-
+ 
 
 <!-- An Engine represents the entry point (within Catalina) that processes
 every request. The Engine implementation for Tomcat stand alone

Places

File tomcat-8.0.53-CVE-2020-1938.patch of Package tomcat.28070

Places