From e68feb1fe0b90c8ee2c3a41943c3c80a022e85f5 Mon Sep 17 00:00:00 2001 From: hht8 Date: Tue, 19 May 2020 15:36:49 +0800 Subject: [PATCH] fix cves --- CVE-2019-12418.patch | 112 +++++++++++ CVE-2019-17563.patch | 137 +++++++++++++ CVE-2020-1935.patch | 443 ++++++++++++++++++++++++++++++++++++++++++ CVE-2020-1938-1.patch | 50 +++++ CVE-2020-1938-2.patch | 160 +++++++++++++++ CVE-2020-1938-3.patch | 142 ++++++++++++++ CVE-2020-1938-4.patch | 86 ++++++++ CVE-2020-1938-5.patch | 32 +++ tomcat.spec | 16 +- 9 files changed, 1177 insertions(+), 1 deletion(-) create mode 100644 CVE-2019-12418.patch create mode 100644 CVE-2019-17563.patch create mode 100644 CVE-2020-1935.patch create mode 100644 CVE-2020-1938-1.patch create mode 100644 CVE-2020-1938-2.patch create mode 100644 CVE-2020-1938-3.patch create mode 100644 CVE-2020-1938-4.patch create mode 100644 CVE-2020-1938-5.patch diff --git a/CVE-2019-12418.patch b/CVE-2019-12418.patch new file mode 100644 index 0000000..c373c43 --- /dev/null +++ b/CVE-2019-12418.patch @@ -0,0 +1,112 @@ +From 1fc9f589dbdd8295cf313b2667ab041c425f99c3 Mon Sep 17 00:00:00 2001 +From: remm +Date: Thu, 14 Nov 2019 13:39:31 +0100 +Subject: [PATCH] Refactor JMX remote RMI registry creation + +--- + .../mbeans/JmxRemoteLifecycleListener.java | 65 ++++++++++++++----- + 1 file changed, 49 insertions(+), 16 deletions(-) + +diff --git a/java/org/apache/catalina/mbeans/JmxRemoteLifecycleListener.java b/java/org/apache/catalina/mbeans/JmxRemoteLifecycleListener.java +index ae04294..e832935 100644 +--- a/java/org/apache/catalina/mbeans/JmxRemoteLifecycleListener.java ++++ b/java/org/apache/catalina/mbeans/JmxRemoteLifecycleListener.java +@@ -25,10 +25,11 @@ import java.net.MalformedURLException; + import java.net.ServerSocket; + import java.net.Socket; + import java.net.UnknownHostException; ++import java.rmi.AccessException; + import java.rmi.AlreadyBoundException; ++import java.rmi.NotBoundException; ++import java.rmi.Remote; + import java.rmi.RemoteException; +-import java.rmi.registry.LocateRegistry; +-import java.rmi.registry.Registry; + import java.rmi.server.RMIClientSocketFactory; + import java.rmi.server.RMIServerSocketFactory; + import java.security.NoSuchAlgorithmException; +@@ -301,18 +302,6 @@ public class JmxRemoteLifecycleListener implements LifecycleListener { + RMIClientSocketFactory registryCsf, RMIServerSocketFactory registrySsf, + RMIClientSocketFactory serverCsf, RMIServerSocketFactory serverSsf) { + +- // Create the RMI registry +- Registry registry; +- try { +- registry = LocateRegistry.createRegistry( +- theRmiRegistryPort, registryCsf, registrySsf); +- } catch (RemoteException e) { +- log.error(sm.getString( +- "jmxRemoteLifecycleListener.createRegistryFailed", +- serverName, Integer.toString(theRmiRegistryPort)), e); +- return null; +- } +- + if (bindAddress == null) { + bindAddress = "localhost"; + } +@@ -333,11 +322,20 @@ public class JmxRemoteLifecycleListener implements LifecycleListener { + cs = new RMIConnectorServer(serviceUrl, theEnv, server, + ManagementFactory.getPlatformMBeanServer()); + cs.start(); +- registry.bind("jmxrmi", server.toStub()); ++ Remote jmxServer = server.toStub(); ++ // Create the RMI registry ++ try { ++ new JmxRegistry(theRmiRegistryPort, registryCsf, registrySsf, "jmxrmi", jmxServer); ++ } catch (RemoteException e) { ++ log.error(sm.getString( ++ "jmxRemoteLifecycleListener.createRegistryFailed", ++ serverName, Integer.toString(theRmiRegistryPort)), e); ++ return null; ++ } + log.info(sm.getString("jmxRemoteLifecycleListener.start", + Integer.toString(theRmiRegistryPort), + Integer.toString(theRmiServerPort), serverName)); +- } catch (IOException | AlreadyBoundException e) { ++ } catch (IOException e) { + log.error(sm.getString( + "jmxRemoteLifecycleListener.createServerFailed", + serverName), e); +@@ -493,4 +491,39 @@ public class JmxRemoteLifecycleListener implements LifecycleListener { + return true; + } + } ++ ++ ++ private static class JmxRegistry extends sun.rmi.registry.RegistryImpl { ++ private static final long serialVersionUID = -3772054804656428217L; ++ private final String jmxName; ++ private final Remote jmxServer; ++ public JmxRegistry(int port, RMIClientSocketFactory csf, ++ RMIServerSocketFactory ssf, String jmxName, Remote jmxServer) throws RemoteException { ++ super(port, csf, ssf); ++ this.jmxName = jmxName; ++ this.jmxServer = jmxServer; ++ } ++ @Override ++ public Remote lookup(String name) ++ throws RemoteException, NotBoundException { ++ return (jmxName.equals(name)) ? jmxServer : null; ++ } ++ @Override ++ public void bind(String name, Remote obj) ++ throws RemoteException, AlreadyBoundException, AccessException { ++ } ++ @Override ++ public void unbind(String name) ++ throws RemoteException, NotBoundException, AccessException { ++ } ++ @Override ++ public void rebind(String name, Remote obj) ++ throws RemoteException, AccessException { ++ } ++ @Override ++ public String[] list() throws RemoteException { ++ return new String[] { jmxName }; ++ } ++ } ++ + } +-- +2.23.0 + diff --git a/CVE-2019-17563.patch b/CVE-2019-17563.patch new file mode 100644 index 0000000..97f569d --- /dev/null +++ b/CVE-2019-17563.patch @@ -0,0 +1,137 @@ +From fabfa49abf917e126dbcf299fed40a1ab96d6f7a Mon Sep 17 00:00:00 2001 +From: wang_yue111 +Date: Fri, 15 May 2020 17:17:57 +0800 +Subject: [PATCH] 2 + +--- + .../authenticator/AuthenticatorBase.java | 7 ++-- + .../catalina/authenticator/Constants.java | 3 ++ + .../authenticator/FormAuthenticator.java | 36 +++++-------------- + 3 files changed, 16 insertions(+), 30 deletions(-) + +diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java +index 880ebde..47d562b 100644 +--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java ++++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java +@@ -1021,10 +1021,11 @@ public abstract class AuthenticatorBase extends ValveBase + } + + // Cache the authentication information in our session, if any +- if (cache) { +- if (session != null) { ++ if (session != null) { ++ if (cache) { + session.setAuthType(authType); + session.setPrincipal(principal); ++ } else { + if (username != null) { + session.setNote(Constants.SESS_USERNAME_NOTE, username); + } else { +diff --git a/java/org/apache/catalina/authenticator/Constants.java b/java/org/apache/catalina/authenticator/Constants.java +index 452a4f0..c9580d6 100644 +--- a/java/org/apache/catalina/authenticator/Constants.java ++++ b/java/org/apache/catalina/authenticator/Constants.java +@@ -93,7 +93,10 @@ public class Constants { + + /** + * The previously authenticated principal (if caching is disabled). ++ * ++ * @deprecated Unused. Will be removed in Tomcat 10. + */ ++ @Deprecated + public static final String FORM_PRINCIPAL_NOTE = + "org.apache.catalina.authenticator.PRINCIPAL"; + +diff --git a/java/org/apache/catalina/authenticator/FormAuthenticator.java b/java/org/apache/catalina/authenticator/FormAuthenticator.java +index 1b54ddd..44c783e 100644 +--- a/java/org/apache/catalina/authenticator/FormAuthenticator.java ++++ b/java/org/apache/catalina/authenticator/FormAuthenticator.java +@@ -133,10 +133,6 @@ public class FormAuthenticator + protected boolean doAuthenticate(Request request, HttpServletResponse response) + throws IOException { + +- if (checkForCachedAuthentication(request, response, true)) { +- return true; +- } +- + // References to objects we will need later + Session session = null; + Principal principal = null; +@@ -158,11 +154,8 @@ public class FormAuthenticator + principal = + context.getRealm().authenticate(username, password); + if (principal != null) { +- session.setNote(Constants.FORM_PRINCIPAL_NOTE, principal); ++ register(request, response, principal, HttpServletRequest.FORM_AUTH, username, password); + if (!matchRequest(request)) { +- register(request, response, principal, +- HttpServletRequest.FORM_AUTH, +- username, password); + return true; + } + } +@@ -181,17 +174,6 @@ public class FormAuthenticator + + session.getIdInternal() + + "'"); + } +- principal = (Principal) +- session.getNote(Constants.FORM_PRINCIPAL_NOTE); +- register(request, response, principal, HttpServletRequest.FORM_AUTH, +- (String) session.getNote(Constants.SESS_USERNAME_NOTE), +- (String) session.getNote(Constants.SESS_PASSWORD_NOTE)); +- // If we're caching principals we no longer need the username +- // and password in the session, so remove them +- if (cache) { +- session.removeNote(Constants.SESS_USERNAME_NOTE); +- session.removeNote(Constants.SESS_PASSWORD_NOTE); +- } + if (restoreRequest(request, session)) { + if (log.isDebugEnabled()) { + log.debug("Proceed to restored request"); +@@ -206,6 +188,12 @@ public class FormAuthenticator + } + } + ++ // This check has to be after the previous check for a matching request ++ // because that matching request may also include a cached Principal. ++ if (checkForCachedAuthentication(request, response, true)) { ++ return true; ++ } ++ + // Acquire references to objects we will need to evaluate + String contextPath = request.getContextPath(); + String requestURI = request.getDecodedRequestURI(); +@@ -297,12 +285,7 @@ public class FormAuthenticator + return false; + } + +- // Save the authenticated Principal in our session +- session.setNote(Constants.FORM_PRINCIPAL_NOTE, principal); +- +- // Save the username and password as well +- session.setNote(Constants.SESS_USERNAME_NOTE, username); +- session.setNote(Constants.SESS_PASSWORD_NOTE, password); ++ register(request, response, principal, HttpServletRequest.FORM_AUTH, username, password); + + // Redirect the user to the original request URI (which will cause + // the original request to be restored) +@@ -510,7 +493,7 @@ public class FormAuthenticator + } + + // Is there a saved principal? +- if (session.getNote(Constants.FORM_PRINCIPAL_NOTE) == null) { ++ if (cache && session.getPrincipal() == null || !cache && request.getPrincipal() == null) { + return false; + } + +@@ -541,7 +524,6 @@ public class FormAuthenticator + SavedRequest saved = (SavedRequest) + session.getNote(Constants.FORM_REQUEST_NOTE); + session.removeNote(Constants.FORM_REQUEST_NOTE); +- session.removeNote(Constants.FORM_PRINCIPAL_NOTE); + if (saved == null) { + return false; + } +-- +2.23.0 + diff --git a/CVE-2020-1935.patch b/CVE-2020-1935.patch new file mode 100644 index 0000000..033c655 --- /dev/null +++ b/CVE-2020-1935.patch @@ -0,0 +1,443 @@ +From 8bfb0ff7f25fe7555a5eb2f7984f73546c11aa26 Mon Sep 17 00:00:00 2001 +From: Mark Thomas +Date: Mon, 6 Jan 2020 20:53:25 +0000 +Subject: [PATCH] Stricter header value parsing + +--- + .../coyote/http11/AbstractHttp11Protocol.java | 51 ++++++++++--- + .../coyote/http11/Http11InputBuffer.java | 51 +++++++++---- + .../apache/coyote/http11/Http11Processor.java | 2 +- + .../apache/tomcat/util/http/MimeHeaders.java | 2 +- + .../tomcat/util/http/parser/HttpParser.java | 11 +++ + .../coyote/http11/TestHttp11InputBuffer.java | 74 +++++++++++++++---- + webapps/docs/config/http.xml | 11 ++- + 8 files changed, 164 insertions(+), 43 deletions(-) + +diff --git a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java +index e5ab885..9d10cbf 100644 +--- a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java ++++ b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java +@@ -136,27 +136,56 @@ public abstract class AbstractHttp11Protocol extends AbstractProtocol { + } + + +- private boolean rejectIllegalHeaderName = true; ++ private boolean rejectIllegalHeader = true; + /** +- * If an HTTP request is received that contains an illegal header name (i.e. +- * the header name is not a token) will the request be rejected (with a 400 +- * response) or will the illegal header be ignored. ++ * If an HTTP request is received that contains an illegal header name or ++ * value (e.g. the header name is not a token) will the request be rejected ++ * (with a 400 response) or will the illegal header be ignored? + * + * @return {@code true} if the request will be rejected or {@code false} if + * the header will be ignored + */ +- public boolean getRejectIllegalHeaderName() { return rejectIllegalHeaderName; } ++ public boolean getRejectIllegalHeader() { return rejectIllegalHeader; } + /** +- * If an HTTP request is received that contains an illegal header name (i.e. +- * the header name is not a token) should the request be rejected (with a +- * 400 response) or should the illegal header be ignored. ++ * If an HTTP request is received that contains an illegal header name or ++ * value (e.g. the header name is not a token) should the request be ++ * rejected (with a 400 response) or should the illegal header be ignored? ++ * ++ * @param rejectIllegalHeader {@code true} to reject requests with illegal ++ * header names or values, {@code false} to ++ * ignore the header ++ */ ++ public void setRejectIllegalHeader(boolean rejectIllegalHeader) { ++ this.rejectIllegalHeader = rejectIllegalHeader; ++ } ++ /** ++ * If an HTTP request is received that contains an illegal header name or ++ * value (e.g. the header name is not a token) will the request be rejected ++ * (with a 400 response) or will the illegal header be ignored? ++ * ++ * @return {@code true} if the request will be rejected or {@code false} if ++ * the header will be ignored ++ * ++ * @deprecated Now an alias for {@link #getRejectIllegalHeader()}. Will be ++ * removed in Tomcat 10 onwards. ++ */ ++ @Deprecated ++ public boolean getRejectIllegalHeaderName() { return rejectIllegalHeader; } ++ /** ++ * If an HTTP request is received that contains an illegal header name or ++ * value (e.g. the header name is not a token) should the request be ++ * rejected (with a 400 response) or should the illegal header be ignored? + * + * @param rejectIllegalHeaderName {@code true} to reject requests with +- * illegal header names, {@code false} to +- * ignore the header ++ * illegal header names or values, ++ * {@code false} to ignore the header ++ * ++ * @deprecated Now an alias for {@link #setRejectIllegalHeader(boolean)}. ++ * Will be removed in Tomcat 10 onwards. + */ ++ @Deprecated + public void setRejectIllegalHeaderName(boolean rejectIllegalHeaderName) { +- this.rejectIllegalHeaderName = rejectIllegalHeaderName; ++ this.rejectIllegalHeader = rejectIllegalHeaderName; + } + + +diff --git a/java/org/apache/coyote/http11/Http11InputBuffer.java b/java/org/apache/coyote/http11/Http11InputBuffer.java +index 2dc7c17..57c670e 100644 +--- a/java/org/apache/coyote/http11/Http11InputBuffer.java ++++ b/java/org/apache/coyote/http11/Http11InputBuffer.java +@@ -64,7 +64,7 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler + private final MimeHeaders headers; + + +- private final boolean rejectIllegalHeaderName; ++ private final boolean rejectIllegalHeader; + + /** + * State. +@@ -150,13 +150,13 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler + // ----------------------------------------------------------- Constructors + + public Http11InputBuffer(Request request, int headerBufferSize, +- boolean rejectIllegalHeaderName, HttpParser httpParser) { ++ boolean rejectIllegalHeader, HttpParser httpParser) { + + this.request = request; + headers = request.getMimeHeaders(); + + this.headerBufferSize = headerBufferSize; +- this.rejectIllegalHeaderName = rejectIllegalHeaderName; ++ this.rejectIllegalHeader = rejectIllegalHeader; + this.httpParser = httpParser; + + filterLibrary = new InputFilter[0]; +@@ -752,6 +752,8 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler + // + + byte chr = 0; ++ byte prevChr = 0; ++ + while (headerParsePos == HeaderParsePosition.HEADER_START) { + + // Read new bytes if needed +@@ -762,17 +764,23 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler + } + } + ++ prevChr = chr; + chr = byteBuffer.get(); + +- if (chr == Constants.CR) { +- // Skip +- } else if (chr == Constants.LF) { ++ if (chr == Constants.CR && prevChr != Constants.CR) { ++ // Possible start of CRLF - process the next byte. ++ } else if (prevChr == Constants.CR && chr == Constants.LF) { + return HeaderParseStatus.DONE; + } else { +- byteBuffer.position(byteBuffer.position() - 1); ++ if (prevChr == 0) { ++ // Must have only read one byte ++ byteBuffer.position(byteBuffer.position() - 1); ++ } else { ++ // Must have read two bytes (first was CR, second was not LF) ++ byteBuffer.position(byteBuffer.position() - 2); ++ } + break; + } +- + } + + if (headerParsePos == HeaderParsePosition.HEADER_START) { +@@ -868,11 +876,22 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler + } + } + ++ prevChr = chr; + chr = byteBuffer.get(); + if (chr == Constants.CR) { +- // Skip +- } else if (chr == Constants.LF) { ++ // Possible start of CRLF - process the next byte. ++ } else if (prevChr == Constants.CR && chr == Constants.LF) { + eol = true; ++ } else if (prevChr == Constants.CR) { ++ // Invalid value ++ // Delete the header (it will be the most recent one) ++ headers.removeHeader(headers.size() - 1); ++ return skipLine(); ++ } else if (chr != Constants.HT && HttpParser.isControl(chr)) { ++ // Invalid value ++ // Delete the header (it will be the most recent one) ++ headers.removeHeader(headers.size() - 1); ++ return skipLine(); + } else if (chr == Constants.SP || chr == Constants.HT) { + byteBuffer.put(headerData.realPos, chr); + headerData.realPos++; +@@ -924,6 +943,9 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler + headerParsePos = HeaderParsePosition.HEADER_SKIPLINE; + boolean eol = false; + ++ byte chr = 0; ++ byte prevChr = 0; ++ + // Reading bytes until the end of the line + while (!eol) { + +@@ -935,21 +957,22 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler + } + + int pos = byteBuffer.position(); +- byte chr = byteBuffer.get(); ++ prevChr = chr; ++ chr = byteBuffer.get(); + if (chr == Constants.CR) { + // Skip +- } else if (chr == Constants.LF) { ++ } else if (prevChr == Constants.CR && chr == Constants.LF) { + eol = true; + } else { + headerData.lastSignificantChar = pos; + } + } +- if (rejectIllegalHeaderName || log.isDebugEnabled()) { ++ if (rejectIllegalHeader || log.isDebugEnabled()) { + String message = sm.getString("iib.invalidheader", + new String(byteBuffer.array(), headerData.start, + headerData.lastSignificantChar - headerData.start + 1, + StandardCharsets.ISO_8859_1)); +- if (rejectIllegalHeaderName) { ++ if (rejectIllegalHeader) { + throw new IllegalArgumentException(message); + } + log.debug(message); +diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java +index df002e0..86556ec 100644 +--- a/java/org/apache/coyote/http11/Http11Processor.java ++++ b/java/org/apache/coyote/http11/Http11Processor.java +@@ -153,7 +153,7 @@ public class Http11Processor extends AbstractProcessor { + protocol.getRelaxedQueryChars()); + + inputBuffer = new Http11InputBuffer(request, protocol.getMaxHttpHeaderSize(), +- protocol.getRejectIllegalHeaderName(), httpParser); ++ protocol.getRejectIllegalHeader(), httpParser); + request.setInputBuffer(inputBuffer); + + outputBuffer = new Http11OutputBuffer(response, protocol.getMaxHttpHeaderSize()); +diff --git a/java/org/apache/tomcat/util/http/MimeHeaders.java b/java/org/apache/tomcat/util/http/MimeHeaders.java +index 59504ee..b76b274 100644 +--- a/java/org/apache/tomcat/util/http/MimeHeaders.java ++++ b/java/org/apache/tomcat/util/http/MimeHeaders.java +@@ -396,7 +396,7 @@ public class MimeHeaders { + * reset and swap with last header + * @param idx the index of the header to remove. + */ +- private void removeHeader(int idx) { ++ public void removeHeader(int idx) { + MimeHeaderField mh = headers[idx]; + + mh.recycle(); +diff --git a/java/org/apache/tomcat/util/http/parser/HttpParser.java b/java/org/apache/tomcat/util/http/parser/HttpParser.java +index 827f2c5..644b1d1 100644 +--- a/java/org/apache/tomcat/util/http/parser/HttpParser.java ++++ b/java/org/apache/tomcat/util/http/parser/HttpParser.java +@@ -317,6 +317,17 @@ public class HttpParser { + } + + ++ public static boolean isControl(int c) { ++ // Fast for valid control characters, slower for some incorrect ++ // ones ++ try { ++ return IS_CONTROL[c]; ++ } catch (ArrayIndexOutOfBoundsException ex) { ++ return false; ++ } ++ } ++ ++ + // Skip any LWS and position to read the next character. The next character + // is returned as being able to 'peek()' it allows a small optimisation in + // some cases. +diff --git a/test/org/apache/coyote/http11/TestHttp11InputBuffer.java b/test/org/apache/coyote/http11/TestHttp11InputBuffer.java +index 131fa21..e60a474 100644 +--- a/test/org/apache/coyote/http11/TestHttp11InputBuffer.java ++++ b/test/org/apache/coyote/http11/TestHttp11InputBuffer.java +@@ -163,13 +163,13 @@ public class TestHttp11InputBuffer extends TomcatBaseTest { + + + @Test +- public void testBug51557Separators() throws Exception { ++ public void testBug51557SeparatorsInName() throws Exception { + char httpSeparators[] = new char[] { + '\t', ' ', '\"', '(', ')', ',', '/', ':', ';', '<', + '=', '>', '?', '@', '[', '\\', ']', '{', '}' }; + + for (char s : httpSeparators) { +- doTestBug51557Char(s); ++ doTestBug51557CharInName(s); + tearDown(); + setUp(); + } +@@ -177,13 +177,38 @@ public class TestHttp11InputBuffer extends TomcatBaseTest { + + + @Test +- public void testBug51557Ctl() throws Exception { ++ public void testBug51557CtlInName() throws Exception { + for (int i = 0; i < 31; i++) { +- doTestBug51557Char((char) i); ++ doTestBug51557CharInName((char) i); ++ tearDown(); ++ setUp(); ++ } ++ doTestBug51557CharInName((char) 127); ++ } ++ ++ ++ @Test ++ public void testBug51557CtlInValue() throws Exception { ++ for (int i = 0; i < 31; i++) { ++ if (i == '\t') { ++ // TAB is allowed ++ continue; ++ } ++ doTestBug51557InvalidCharInValue((char) i); ++ tearDown(); ++ setUp(); ++ } ++ doTestBug51557InvalidCharInValue((char) 127); ++ } ++ ++ ++ @Test ++ public void testBug51557ObsTextInValue() throws Exception { ++ for (int i = 128; i < 255; i++) { ++ doTestBug51557ValidCharInValue((char) i); + tearDown(); + setUp(); + } +- doTestBug51557Char((char) 127); + } + + +@@ -226,7 +251,7 @@ public class TestHttp11InputBuffer extends TomcatBaseTest { + } + + +- private void doTestBug51557Char(char s) { ++ private void doTestBug51557CharInName(char s) { + Bug51557Client client = + new Bug51557Client("X-Bug" + s + "51557", "invalid"); + +@@ -236,6 +261,29 @@ public class TestHttp11InputBuffer extends TomcatBaseTest { + Assert.assertTrue(client.isResponseBodyOK()); + } + ++ ++ private void doTestBug51557InvalidCharInValue(char s) { ++ Bug51557Client client = ++ new Bug51557Client("X-Bug51557-Invalid", "invalid" + s + "invalid"); ++ ++ client.doRequest(); ++ Assert.assertTrue("Testing [" + (int) s + "]", client.isResponse200()); ++ Assert.assertEquals("Testing [" + (int) s + "]", "abcd", client.getResponseBody()); ++ Assert.assertTrue(client.isResponseBodyOK()); ++ } ++ ++ ++ private void doTestBug51557ValidCharInValue(char s) { ++ Bug51557Client client = ++ new Bug51557Client("X-Bug51557-Valid", "valid" + s + "valid"); ++ ++ client.doRequest(); ++ Assert.assertTrue("Testing [" + (int) s + "]", client.isResponse200()); ++ Assert.assertEquals("Testing [" + (int) s + "]", "valid" + s + "validabcd", client.getResponseBody()); ++ Assert.assertTrue(client.isResponseBodyOK()); ++ } ++ ++ + /** + * Bug 51557 test client. + */ +@@ -243,12 +291,12 @@ public class TestHttp11InputBuffer extends TomcatBaseTest { + + private final String headerName; + private final String headerLine; +- private final boolean rejectIllegalHeaderName; ++ private final boolean rejectIllegalHeader; + + public Bug51557Client(String headerName) { + this.headerName = headerName; + this.headerLine = headerName; +- this.rejectIllegalHeaderName = false; ++ this.rejectIllegalHeader = false; + } + + public Bug51557Client(String headerName, String headerValue) { +@@ -256,10 +304,10 @@ public class TestHttp11InputBuffer extends TomcatBaseTest { + } + + public Bug51557Client(String headerName, String headerValue, +- boolean rejectIllegalHeaderName) { ++ boolean rejectIllegalHeader) { + this.headerName = headerName; + this.headerLine = headerName + ": " + headerValue; +- this.rejectIllegalHeaderName = rejectIllegalHeaderName; ++ this.rejectIllegalHeader = rejectIllegalHeader; + } + + private Exception doRequest() { +@@ -273,8 +321,8 @@ public class TestHttp11InputBuffer extends TomcatBaseTest { + + try { + Connector connector = tomcat.getConnector(); +- connector.setProperty("rejectIllegalHeaderName", +- Boolean.toString(rejectIllegalHeaderName)); ++ connector.setProperty("rejectIllegalHeader", ++ Boolean.toString(rejectIllegalHeader)); + tomcat.start(); + setPort(connector.getLocalPort()); + +@@ -548,7 +596,7 @@ public class TestHttp11InputBuffer extends TomcatBaseTest { + + try { + Connector connector = tomcat.getConnector(); +- connector.setProperty("rejectIllegalHeaderName", "false"); ++ connector.setProperty("rejectIllegalHeader", "false"); + tomcat.start(); + setPort(connector.getLocalPort()); + +diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml +index ebb277d..3902c9a 100644 +--- a/webapps/docs/config/http.xml ++++ b/webapps/docs/config/http.xml +@@ -551,14 +551,19 @@ + expected concurrent requests (synchronous and asynchronous).

+ + +- +-

If an HTTP request is received that contains an illegal header name +- (i.e. the header name is not a token) this setting determines if the ++ ++

If an HTTP request is received that contains an illegal header name or ++ value (e.g. the header name is not a token) this setting determines if the + request will be rejected with a 400 response (true) or if the + illegal header be ignored (false). The default value is + true which will cause the request to be rejected.

+ + ++ ++

This attribute is deprecated. It will be removed in Tomcat 10 onwards. ++ It is now an alias for rejectIllegalHeader.

++ ++ + +

The HTTP/1.1 + specification requires that certain characters are %nn encoded when +-- +2.23.0 + diff --git a/CVE-2020-1938-1.patch b/CVE-2020-1938-1.patch new file mode 100644 index 0000000..c2feb27 --- /dev/null +++ b/CVE-2020-1938-1.patch @@ -0,0 +1,50 @@ +From 0e8a50f0a5958744bea1fd6768c862e04d3b7e75 Mon Sep 17 00:00:00 2001 +From: Mark Thomas +Date: Tue, 21 Jan 2020 13:02:13 +0000 +Subject: [PATCH] Change the default bind address for AJP to the loopback + address + +--- + java/org/apache/coyote/ajp/AbstractAjpProtocol.java | 4 ++++ + webapps/docs/changelog.xml | 4 ++++ + webapps/docs/config/ajp.xml | 5 +---- + 3 files changed, 9 insertions(+), 4 deletions(-) + +diff --git a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java +index 2500abd7ad..8e0593b771 100644 +--- a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java ++++ b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java +@@ -16,6 +16,8 @@ + */ + package org.apache.coyote.ajp; + ++import java.net.InetAddress; ++ + import org.apache.coyote.AbstractProtocol; + import org.apache.coyote.Processor; + import org.apache.coyote.UpgradeProtocol; +@@ -46,6 +48,8 @@ public AbstractAjpProtocol(AbstractEndpoint endpoint) { + setConnectionTimeout(Constants.DEFAULT_CONNECTION_TIMEOUT); + // AJP does not use Send File + getEndpoint().setUseSendfile(false); ++ // AJP listens on loopback by default ++ getEndpoint().setAddress(InetAddress.getLoopbackAddress()); + ConnectionHandler cHandler = new ConnectionHandler<>(this); + setHandler(cHandler); + getEndpoint().setHandler(cHandler); +diff --git a/webapps/docs/config/ajp.xml b/webapps/docs/config/ajp.xml +index c70af91eae..5535a062e7 100644 +--- a/webapps/docs/config/ajp.xml ++++ b/webapps/docs/config/ajp.xml +@@ -308,10 +308,7 @@ + +

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 127.0.0.1 +- indicates that the Connector will only listen on the loopback +- interface.

++ port. By default, the loopback address will be used.

+ + + diff --git a/CVE-2020-1938-2.patch b/CVE-2020-1938-2.patch new file mode 100644 index 0000000..cb57232 --- /dev/null +++ b/CVE-2020-1938-2.patch @@ -0,0 +1,160 @@ +From 9ac90532e9a7d239f90952edb229b07c80a9a3eb Mon Sep 17 00:00:00 2001 +From: Mark Thomas +Date: Tue, 21 Jan 2020 14:24:33 +0000 +Subject: [PATCH] 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. +--- + .../coyote/ajp/AbstractAjpProtocol.java | 49 +++++++++++++++++-- + java/org/apache/coyote/ajp/AjpProcessor.java | 12 ++--- + .../apache/coyote/ajp/LocalStrings.properties | 1 + + webapps/docs/config/ajp.xml | 12 ++++- + 5 files changed, 72 insertions(+), 10 deletions(-) + +diff --git a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java +index 8e0593b771..81da7da6d1 100644 +--- 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 +index a3e628d2eb..d466de2c09 100644 +--- 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 +index 9b569bbc6e..01de92a424 100644 +--- a/java/org/apache/coyote/ajp/LocalStrings.properties ++++ b/java/org/apache/coyote/ajp/LocalStrings.properties +@@ -28,6 +28,7 @@ ajpprocessor.request.prepare=Error preparing request + # limitations under the License. + + 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/config/ajp.xml b/webapps/docs/config/ajp.xml +index 5535a062e7..3999a13e66 100644 +--- a/webapps/docs/config/ajp.xml ++++ b/webapps/docs/config/ajp.xml +@@ -428,8 +428,18 @@ + expected concurrent requests (synchronous and asynchronous).

+ + +- ++ +

Only requests from workers with this secret keyword will be accepted. ++ The default value is null. This attrbute must be specified ++ with a non-null, non-zero length value unless ++ secretRequired is explicitly configured to be ++ false.

++ ++ ++ ++

If this attribute is true, the AJP Connector will only ++ start if the secret attribute is configured with a ++ non-null, non-zero length value. The default value is true. +

+ + diff --git a/CVE-2020-1938-3.patch b/CVE-2020-1938-3.patch new file mode 100644 index 0000000..dc152a7 --- /dev/null +++ b/CVE-2020-1938-3.patch @@ -0,0 +1,142 @@ +From 64fa5b99442589ef0bf2a7fcd71ad2bc68b35fad Mon Sep 17 00:00:00 2001 +From: Mark Thomas +Date: Tue, 21 Jan 2020 15:04:12 +0000 +Subject: [PATCH] Add new AJP attribute allowedArbitraryRequestAttributes + +Requests with unrecognised attributes will be blocked with a 403 +--- + .../coyote/ajp/AbstractAjpProtocol.java | 13 +++++++ + java/org/apache/coyote/ajp/AjpProcessor.java | 36 ++++++++++++++++++- + webapps/docs/config/ajp.xml | 19 ++++++++++ + 4 files changed, 72 insertions(+), 1 deletion(-) + +diff --git a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java +index 81da7da6d1..a2f5e2844a 100644 +--- a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java ++++ b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java +@@ -17,6 +17,7 @@ + package org.apache.coyote.ajp; + + import java.net.InetAddress; ++import java.util.regex.Pattern; + + import org.apache.coyote.AbstractProtocol; + import org.apache.coyote.Processor; +@@ -188,6 +189,18 @@ public boolean getSecretRequired() { + } + + ++ private Pattern allowedArbitraryRequestAttributesPattern; ++ public void setAllowedArbitraryRequestAttributes(String allowedArbitraryRequestAttributes) { ++ this.allowedArbitraryRequestAttributesPattern = Pattern.compile(allowedArbitraryRequestAttributes); ++ } ++ public String getAllowedArbitraryRequestAttributes() { ++ return allowedArbitraryRequestAttributesPattern.pattern(); ++ } ++ protected Pattern getAllowedArbitraryRequestAttributesPattern() { ++ return allowedArbitraryRequestAttributesPattern; ++ } ++ ++ + /** + * AJP packet size. + */ +diff --git a/java/org/apache/coyote/ajp/AjpProcessor.java b/java/org/apache/coyote/ajp/AjpProcessor.java +index d466de2c09..f3d783f546 100644 +--- a/java/org/apache/coyote/ajp/AjpProcessor.java ++++ b/java/org/apache/coyote/ajp/AjpProcessor.java +@@ -25,6 +25,11 @@ + import java.security.NoSuchProviderException; + import java.security.cert.CertificateFactory; + import java.security.cert.X509Certificate; ++import java.util.Collections; ++import java.util.HashSet; ++import java.util.Set; ++import java.util.regex.Matcher; ++import java.util.regex.Pattern; + + import javax.servlet.http.HttpServletResponse; + +@@ -78,6 +83,9 @@ + private static final byte[] pongMessageArray; + + ++ private static final Set javaxAttributes; ++ ++ + static { + // Allocate the end message array + AjpMessage endMessage = new AjpMessage(16); +@@ -118,6 +126,14 @@ + pongMessageArray = new byte[pongMessage.getLen()]; + System.arraycopy(pongMessage.getBuffer(), 0, pongMessageArray, + 0, pongMessage.getLen()); ++ ++ // Build the Set of javax attributes ++ Set s = new HashSet<>(); ++ 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); + } + + +@@ -728,8 +744,26 @@ private void prepareRequest() { + } + } 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. ++ Pattern pattern = protocol.getAllowedArbitraryRequestAttributesPattern(); ++ if (pattern == null) { ++ response.setStatus(403); ++ setErrorState(ErrorState.CLOSE_CLEAN, null); ++ } else { ++ Matcher m = pattern.matcher(n); ++ if (m.matches()) { ++ request.setAttribute(n, v); ++ } else { ++ response.setStatus(403); ++ setErrorState(ErrorState.CLOSE_CLEAN, null); ++ } ++ } + } + break; + +diff --git a/webapps/docs/config/ajp.xml b/webapps/docs/config/ajp.xml +index 3999a13e66..69348a18e2 100644 +--- a/webapps/docs/config/ajp.xml ++++ b/webapps/docs/config/ajp.xml +@@ -311,6 +311,25 @@ + port. By default, the loopback address will be used.

+ + ++ ++

The AJP protocol passes some information from the reverse proxy to the ++ AJP connector using request attributes. These attributes are:

++
    ++
  • javax.servlet.request.cipher_suite
    • ++
  • javax.servlet.request.key_size
    • ++
  • javax.servlet.request.ssl_session
    • ++
  • javax.servlet.request.X509Certificate
    • ++
  • AJP_LOCAL_ADDR
    • ++
  • AJP_REMOTE_PORT
    • ++
  • AJP_SSL_PROTOCOL
    • ++
  • JK_LB_ACTIVATION
    • ++
++

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 null.

++ ++ + +

Controls when the socket used by the connector is bound. By default it + is bound when the connector is initiated and unbound when the connector is diff --git a/CVE-2020-1938-4.patch b/CVE-2020-1938-4.patch new file mode 100644 index 0000000..e5e6108 --- /dev/null +++ b/CVE-2020-1938-4.patch @@ -0,0 +1,86 @@ +From 5716044b61cb5b638d8f0de848ac64df03184bc7 Mon Sep 17 00:00:00 2001 +From: wang_yue111 +Date: Mon, 18 May 2020 15:23:19 +0800 +Subject: [PATCH] 3 + +--- + conf/server.xml | 5 ++++- + .../apache/coyote/ajp/AbstractAjpProtocol.java | 18 +++++++++--------- + java/org/apache/coyote/ajp/AjpProcessor.java | 2 +- + webapps/docs/config/ajp.xml | 2 +- + 4 files changed, 15 insertions(+), 12 deletions(-) + +diff --git a/conf/server.xml b/conf/server.xml +index fce8922..81a4e16 100644 +--- a/conf/server.xml ++++ b/conf/server.xml +@@ -113,7 +113,10 @@ + --> + + +- ++ + + +