CVE-2017-5929 Security vulnerability and redefine default log rotation policy.

Patch by Michael Shuler; Reviewed by Ariel Weisberg for CASSANDRA-14183

CHANGES.txt

@@ -1,4 +1,5 @@
 4.0
+ * CVE-2017-5929 Security vulnerability and redefine default log rotation policy (CASSANDRA-14183)
  * Use JVM default SSL validation algorithm instead of custom default (CASSANDRA-13259)
  * Better document in code InetAddressAndPort usage post 7544, incorporate port into UUIDGen node (CASSANDRA-14226)
  * Fix sstablemetadata date string for minLocalDeletionTime (CASSANDRA-14132)

build.xml

@@ -383,8 +383,8 @@
           <dependency groupId="org.slf4j" artifactId="slf4j-api" version="1.7.25"/>
           <dependency groupId="org.slf4j" artifactId="log4j-over-slf4j" version="1.7.25"/>
           <dependency groupId="org.slf4j" artifactId="jcl-over-slf4j" version="1.7.25" />
-          <dependency groupId="ch.qos.logback" artifactId="logback-core" version="1.1.3"/>
+          <dependency groupId="ch.qos.logback" artifactId="logback-core" version="1.2.3"/>
-          <dependency groupId="ch.qos.logback" artifactId="logback-classic" version="1.1.3"/>
+          <dependency groupId="ch.qos.logback" artifactId="logback-classic" version="1.2.3"/>
           <dependency groupId="org.codehaus.jackson" artifactId="jackson-core-asl" version="1.9.2"/>
           <dependency groupId="org.codehaus.jackson" artifactId="jackson-mapper-asl" version="1.9.2"/>
           <dependency groupId="com.googlecode.json-simple" artifactId="json-simple" version="1.1"/>

conf/logback.xml

@@ -22,7 +22,7 @@ In order to disable debug.log, comment-out the ASYNCDEBUGLOG
 appender reference in the root level section below.
 -->
 
-<configuration scan="true">
+<configuration scan="true" scanPeriod="60 seconds">
   <jmxConfigurator />
 
   <!-- No shutdown hook; we run it ourselves in StorageService after shutdown -->
@@ -34,14 +34,14 @@ appender reference in the root level section below.
       <level>INFO</level>
     </filter>
     <file>${cassandra.logdir}/system.log</file>
-    <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+    <rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
-      <fileNamePattern>${cassandra.logdir}/system.log.%i.zip</fileNamePattern>
+      <!-- rollover daily -->
-      <minIndex>1</minIndex>
+      <fileNamePattern>${cassandra.logdir}/system.log.%d{yyyy-MM-dd}.%i.zip</fileNamePattern>
-      <maxIndex>20</maxIndex>
+      <!-- each file should be at most 50MB, keep 7 days worth of history, but at most 5GB -->
+      <maxFileSize>50MB</maxFileSize>
+      <maxHistory>7</maxHistory>
+      <totalSizeCap>5GB</totalSizeCap>
     </rollingPolicy>
-    <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
-      <maxFileSize>20MB</maxFileSize>
-    </triggeringPolicy>
     <encoder>
       <pattern>%-5level [%thread] %date{ISO8601} %F:%L - %msg%n</pattern>
     </encoder>
@@ -51,14 +51,14 @@ appender reference in the root level section below.
 
   <appender name="DEBUGLOG" class="ch.qos.logback.core.rolling.RollingFileAppender">
     <file>${cassandra.logdir}/debug.log</file>
-    <rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
+    <rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
-      <fileNamePattern>${cassandra.logdir}/debug.log.%i.zip</fileNamePattern>
+      <!-- rollover daily -->
-      <minIndex>1</minIndex>
+      <fileNamePattern>${cassandra.logdir}/debug.log.%d{yyyy-MM-dd}.%i.zip</fileNamePattern>
-      <maxIndex>20</maxIndex>
+      <!-- each file should be at most 50MB, keep 7 days worth of history, but at most 5GB -->
+      <maxFileSize>50MB</maxFileSize>
+      <maxHistory>7</maxHistory>
+      <totalSizeCap>5GB</totalSizeCap>
     </rollingPolicy>
-    <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
-      <maxFileSize>20MB</maxFileSize>
-    </triggeringPolicy>
     <encoder>
       <pattern>%-5level [%thread] %date{ISO8601} %F:%L - %msg%n</pattern>
     </encoder>

src/java/org/apache/cassandra/cql3/functions/ThreadAwareSecurityManager.java

@@ -88,6 +88,8 @@ public final class ThreadAwareSecurityManager extends SecurityManager
         // To work around this, a custom ReconfigureOnChangeFilter is installed, that simply
         // prevents this configuration file check and possible reload of the configration,
         // while executing sandboxed UDF code.
+        //
+        // NOTE: this is obsolte with logback versions (at least since 1.2.3)
         Logger l = LoggerFactory.getLogger(ThreadAwareSecurityManager.class);
         ch.qos.logback.classic.Logger logbackLogger = (ch.qos.logback.classic.Logger) l;
         LoggerContext ctx = logbackLogger.getLoggerContext();
@@ -110,6 +112,9 @@ public final class ThreadAwareSecurityManager extends SecurityManager
     /**
      * The purpose of this class is to prevent logback from checking for config file change,
      * if the current thread is executing a sandboxed thread to avoid {@link AccessControlException}s.
+     *
+     * This is obsolete with logback versions that replaced {@link ReconfigureOnChangeFilter}
+     * with {@link ch.qos.logback.classic.joran.ReconfigureOnChangeTask} (at least logback since 1.2.3).
      */
     private static class SMAwareReconfigureOnChangeFilter extends ReconfigureOnChangeFilter
     {

test/conf/logback-test.xml

@@ -17,7 +17,7 @@
  under the License.
 -->
 
-<configuration debug="false" scan="true">
+<configuration debug="false" scan="true" scanPeriod="60 seconds">
   <!-- Shutdown hook ensures that async appender flushes -->
   <shutdownHook class="ch.qos.logback.core.hook.DelayingShutdownHook"/>
 

test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java

@@ -218,10 +218,10 @@ public class DatabaseDescriptorRefTest
             method.invoke(null);
 
             if ("clientInitialization".equals(methodName) &&
-                threadCount + 1 == threads.getThreadCount())
+                threadCount + 2 == threads.getThreadCount())
             {
-                // ignore the "AsyncAppender-Worker-ASYNC" thread
+                // ignore the "AsyncAppender-Worker-ASYNC" and "logback-1" threads
-                threadCount++;
+                threadCount = threadCount + 2;
             }
 
             if (threadCount != threads.getThreadCount())

test/unit/org/apache/cassandra/cql3/validation/operations/AggregationTest.java

@@ -38,6 +38,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.joran.ReconfigureOnChangeTask;
 import ch.qos.logback.classic.spi.TurboFilterList;
 import ch.qos.logback.classic.turbo.ReconfigureOnChangeFilter;
 import ch.qos.logback.classic.turbo.TurboFilter;
@@ -58,6 +59,7 @@ import org.apache.cassandra.transport.Event;
 import org.apache.cassandra.transport.ProtocolVersion;
 import org.apache.cassandra.transport.messages.ResultMessage;
 
+import static ch.qos.logback.core.CoreConstants.RECONFIGURE_ON_CHANGE_TASK;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -1849,6 +1851,16 @@ public class AggregationTest extends CQLTester
                 break;
             }
         }
+
+        ReconfigureOnChangeTask roct = (ReconfigureOnChangeTask) ctx.getObject(RECONFIGURE_ON_CHANGE_TASK);
+        if (roct != null)
+        {
+            // New functionality in logback - they replaced ReconfigureOnChangeFilter (which runs in the logging code)
+            // with an async ReconfigureOnChangeTask - i.e. in a thread that does not become sandboxed.
+            // Let the test run anyway, just we cannot reconfigure it (and it is pointless to reconfigure).
+            return;
+        }
+
         assertTrue("ReconfigureOnChangeFilter not in logback's turbo-filter list - do that by adding scan=\"true\" to logback-test.xml's configuration element", done);
     }