Author: markt
Date: Wed Mar  9 11:16:48 2011
New Revision: 1079752

URL: http://svn.apache.org/viewvc?rev=1079752&view=rev
Log:
CVE-2011-1088
Complete the fix for this issue. The optimisation not to configure an 
authenticator of there were no security constraints meant that in that case 
@ServletSecurity annotations had no effect. The unit tests did not pick this up 
since they added an authenticator directly.
Add an explicit unit test for this scenario.

Added:
    tomcat/trunk/test/webapp-3.0-servletsecurity/
    tomcat/trunk/test/webapp-3.0-servletsecurity/WEB-INF/
    tomcat/trunk/test/webapp-3.0-servletsecurity/WEB-INF/web.xml   (with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java
    tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java?rev=1079752&r1=1079751&r2=1079752&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java Wed Mar  9 
11:16:48 2011
@@ -366,10 +366,7 @@ public class ContextConfig
      */
     protected synchronized void authenticatorConfig() {
 
-        // Does this Context require an Authenticator?
-        SecurityConstraint constraints[] = context.findConstraints();
-        if ((constraints == null) || (constraints.length == 0))
-            return;
+        // Always need an authenticator to support @ServletSecurity annotations
         LoginConfig loginConfig = context.getLoginConfig();
         if (loginConfig == null) {
             loginConfig = DUMMY_LOGIN_CONFIG;

Modified: tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java?rev=1079752&r1=1079751&r2=1079752&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java 
(original)
+++ tomcat/trunk/test/org/apache/catalina/core/TestStandardWrapper.java Wed Mar 
 9 11:16:48 2011
@@ -125,6 +125,24 @@ public class TestStandardWrapper extends
         doTestSecurityAnnotationsAddServlet(true);
     }
     
+    public void testSecurityAnnotationsNoWebXmlConstraints() throws Exception {
+        // Setup Tomcat instance
+        Tomcat tomcat = getTomcatInstance();
+        
+        File appDir = new File("test/webapp-3.0-servletsecurity");
+        tomcat.addWebapp(null, "", appDir.getAbsolutePath());
+        
+        tomcat.start();
+        
+        ByteChunk bc = new ByteChunk();
+        int rc;
+        rc = getUrl("http://localhost:"; + getPort() + "/",
+                bc, null, null);
+        
+        assertNull(bc.toString());
+        assertEquals(403, rc);
+    }
+
     private void doTestSecurityAnnotationsAddServlet(boolean useCreateServlet)
             throws Exception {
 

Added: tomcat/trunk/test/webapp-3.0-servletsecurity/WEB-INF/web.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/webapp-3.0-servletsecurity/WEB-INF/web.xml?rev=1079752&view=auto
==============================================================================
--- tomcat/trunk/test/webapp-3.0-servletsecurity/WEB-INF/web.xml (added)
+++ tomcat/trunk/test/webapp-3.0-servletsecurity/WEB-INF/web.xml Wed Mar  9 
11:16:48 2011
@@ -0,0 +1,48 @@
+<?xml version="1.0" encoding="ISO-8859-1"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<web-app xmlns="http://java.sun.com/xml/ns/javaee";
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+  xsi:schemaLocation="http://java.sun.com/xml/ns/javaee
+                      http://java.sun.com/xml/ns/javaee/web-app_3_0.xsd";
+  version="3.0">  
+
+  <!--
+    WARNING:
+    For the unit tests to work correctly, no security constraints may be
+    configured in the web.xml. A login-config section is permitted but not
+    necessary for the tests. Adding a login-config would require changing the
+    return code checked in the unit tests.
+  -->
+
+  <display-name>Tomcat Test Application</display-name>
+  <description>
+     Used as part of the Tomcat unit tests when a full web application is
+     required.
+  </description>
+  
+  <servlet>
+    <servlet-name>RoleProtected</servlet-name>
+    
<servlet-class>org.apache.catalina.core.TestStandardWrapper$RoleAllowServlet</servlet-class>
+  </servlet>
+
+  <servlet-mapping>
+    <servlet-name>RoleProtected</servlet-name>
+    <url-pattern>/</url-pattern>
+  </servlet-mapping>
+
+</web-app>
\ No newline at end of file

Propchange: tomcat/trunk/test/webapp-3.0-servletsecurity/WEB-INF/web.xml
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1079752&r1=1079751&r2=1079752&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed Mar  9 11:16:48 2011
@@ -45,6 +45,10 @@
 <section name="Tomcat 7.0.11 (markt)">
   <subsection name="Catalina">
     <changelog>
+      <fix>
+        CVE-2011-1088: Completed fix. Don&apos;t ignore @ServletSecurity
+        annotations. (markt)
+      </fix>
       <add>
         <bug>25060</bug>: Close Apache Commons DBCP datasources when the
         associated JNDI naming context is stopped (e.g. for a non-global
@@ -88,6 +92,10 @@
   <subsection name="Catalina">
     <changelog>
       <fix>
+        CVE-2011-1088: Partial fix. Don&apos;t ignore @ServletSecurity
+        annotations. (markt)
+      </fix>
+      <fix>
         <bug>27988</bug>: Improve reporting of missing files. (markt)
       </fix>
       <fix>
@@ -103,10 +111,6 @@
         Improve shut down speed by not renewing threads during shut down when
         the <code>ThreadLocalLeakPreventionListener</code> is enabled. (markt) 
 
       </fix>
-      <fix>
-        CVE-2011-1088: Partial fix. Don&apos;t ignore @ServletSecurity
-        annotations. (markt)
-      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



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

Reply via email to