Safer defaults (#140)

* Safer defaults.

* Modify LoadSaltFilter and ValidateSaltFilter to validate against user ID.

* Docs improvements around safer defaults content

* Test fix from Michael Bien and a comment about running tests in IntelliJ.

app/src/main/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilter.java

@@ -23,6 +23,7 @@
 import javax.servlet.*;
 import javax.servlet.http.HttpServletRequest;
 import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.roller.weblogger.ui.core.RollerSession;
 import org.apache.roller.weblogger.ui.rendering.util.cache.SaltCache;
 
 /**
@@ -31,15 +32,17 @@
  */
 public class LoadSaltFilter implements Filter {
 
-    //@Override
     @Override
     public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
         throws IOException, ServletException {
+
         HttpServletRequest httpReq = (HttpServletRequest) request;
+        RollerSession rses = RollerSession.getRollerSession(httpReq);
+        String userId = rses != null && rses.getAuthenticatedUser() != null ? rses.getAuthenticatedUser().getId() : "";
 
-		SaltCache saltCache = SaltCache.getInstance();
+        SaltCache saltCache = SaltCache.getInstance();
         String salt = RandomStringUtils.random(20, 0, 0, true, true, null, new SecureRandom());
-        saltCache.put(salt, Boolean.TRUE);
+        saltCache.put(salt, userId);
         httpReq.setAttribute("salt", salt);
 
         chain.doFilter(request, response);

app/src/main/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilter.java

@@ -20,6 +20,7 @@
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.Objects;
 import java.util.Set;
 
 import javax.servlet.Filter;
@@ -35,39 +36,31 @@
 import org.apache.commons.logging.LogFactory;
 import org.apache.roller.weblogger.config.WebloggerConfig;
 import org.apache.roller.weblogger.ui.rendering.util.cache.SaltCache;
+import org.apache.roller.weblogger.ui.core.RollerSession;
 
 /**
- * Filter checks all POST request for presence of valid salt value and rejects
- * those without a salt value or with a salt value not generated by this Roller
- * instance.
+ * Filter checks all POST request for presence of valid salt value and rejects those without
+ * a salt value or with a salt value not generated by this Roller instance.
  */
 public class ValidateSaltFilter implements Filter {
-
     private static final Log log = LogFactory.getLog(ValidateSaltFilter.class);
-
     private Set<String> ignored = Collections.emptySet();
 
     @Override
     public void doFilter(ServletRequest request, ServletResponse response,
             FilterChain chain) throws IOException, ServletException {
         HttpServletRequest httpReq = (HttpServletRequest) request;
 
-        // note enctype="multipart/form-data" does not send parameters (see
-        // ROL-1956) requests of this type are stored in salt.ignored.urls in
-        // roller.properties
-        if (httpReq.getMethod().equals("POST")
-                && !isIgnoredURL(httpReq.getServletPath())) {
+        if ("POST".equals(httpReq.getMethod()) && !isIgnoredURL(httpReq.getServletPath())) {
+            RollerSession rses = RollerSession.getRollerSession(httpReq);
+            String userId = rses != null && rses.getAuthenticatedUser() != null ? rses.getAuthenticatedUser().getId() : "";
 
             String salt = httpReq.getParameter("salt");
             SaltCache saltCache = SaltCache.getInstance();
-            if (salt == null || saltCache.get(salt) == null
-                    || saltCache.get(salt).equals(false)) {
-
+            if (salt == null || !Objects.equals(saltCache.get(salt), userId)) {
                 if (log.isDebugEnabled()) {
-                    log.debug("Salt value not found on POST to URL : "
-                            + httpReq.getServletPath());
+                    log.debug("Valid salt value not found on POST to URL : " + httpReq.getServletPath());
                 }
-
                 throw new ServletException("Security Violation");
             }
         }
@@ -88,12 +81,8 @@ public void destroy() {
     }
 
     /**
-     * Checks if this is an ignored url defined in the salt.ignored.urls
-     * property
-     * 
-     * @param theUrl
-     *            the the url
-     * 
+     * Checks if this is an ignored url defined in the salt.ignored.urls property
+     * @param theUrl the the url
      * @return true, if is ignored resource
      */
     private boolean isIgnoredURL(String theUrl) {

app/src/main/java/org/apache/roller/weblogger/ui/rendering/util/cache/SaltCache.java

@@ -59,26 +59,22 @@ private SaltCache() {
 
             // we are only interested in props for this cache
             if(prop.startsWith(CACHE_ID+".")) {
-                cacheProps.put(prop.substring(CACHE_ID.length()+1), 
-                        WebloggerConfig.getProperty(prop));
+                cacheProps.put(prop.substring(CACHE_ID.length()+1), WebloggerConfig.getProperty(prop));
             }
         }
 
         log.info(cacheProps);
-
         contentCache = CacheManager.constructCache(null, cacheProps);
     }
 
-
     public static SaltCache getInstance() {
         return singletonInstance;
     }
 
 
-    public Object get(String key) {
-
+    public String get(String key) {
         Object entry = null;
-        
+
         ExpiringCacheEntry lazyEntry = (ExpiringCacheEntry) this.contentCache.get(key);
         if(lazyEntry != null) {
             entry = lazyEntry.getValue();
@@ -92,11 +88,11 @@ public Object get(String key) {
             log.debug("MISS "+key);
         }
 
-        return entry;
+        return entry != null ? entry.toString() : null;
     }
 
 
-    public void put(String key, Object value) {
+    public void put(String key, String value) {
 		// expire after 60 minutes
         contentCache.put(key, new ExpiringCacheEntry(value, RollerConstants.HOUR_IN_MS));
         log.debug("PUT "+key);

app/src/main/resources/org/apache/roller/weblogger/config/roller.properties

@@ -342,6 +342,9 @@ authentication.method=db
 # Enables HTTPS for login page only
 securelogin.enabled=false
 
+# With this settings, all users will have HTML posts sanitized.
+weblogAdminsUntrusted=true
+
 # Empty value used for passphrase in roller_user table when LDAP or CMA used;
 # openid presently generates a random (long) password string instead.
 users.passwords.externalAuthValue=<externalAuth>

app/src/main/resources/org/apache/roller/weblogger/config/runtimeConfigDefs.xml

@@ -199,7 +199,7 @@
 
       <property-def  name="uploads.enabled"  key="configForm.enableFileUploads">
          <type>boolean</type>
-         <default-value>true</default-value>
+         <default-value>false</default-value>
       </property-def>
       <property-def  name="uploads.types.allowed"  key="configForm.allowedExtensions">
          <type>string</type>
@@ -230,7 +230,7 @@
       </property-def>
       <property-def  name="themes.customtheme.allowed"  key="configForm.allowCustomTheme">
          <type>boolean</type>
-         <default-value>true</default-value>
+         <default-value>false</default-value>
       </property-def>
 
    </display-group>

app/src/test/java/org/apache/roller/weblogger/business/MediaFileTest.java

@@ -51,6 +51,9 @@ public MediaFileTest() {
     @BeforeEach
     public void setUp() throws Exception {
         TestUtils.setupWeblogger();
+        // allow media uploads for this test
+        Map<String, RuntimeConfigProperty> config = WebloggerFactory.getWeblogger().getPropertiesManager().getProperties();
+        config.get("uploads.enabled").setValue("true");
     }
 
     /**

app/src/test/java/org/apache/roller/weblogger/business/SQLScriptRunnerTest.java

@@ -30,8 +30,7 @@
 import java.sql.ResultSet;
 import java.sql.SQLException;
 
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 /**
  * Test parsing and running of SQL scripts
@@ -64,7 +63,7 @@ public void testParseOnly() throws Exception {
         String scriptPath = System.getProperty("project.build.directory")
                 + "/test-classes/WEB-INF/dbscripts/dummydb/createdb-"+dbname+".sql";
         SQLScriptRunner runner = new SQLScriptRunner(scriptPath);
-        assertTrue(runner.getCommandCount() == 5);        
+        assertEquals(5, runner.getCommandCount());
     }
 
     @Test
@@ -79,10 +78,12 @@ public void testSimpleRun() throws Exception {
             // but some folks test against MySQL
             dbname = "mysql";
         }
-
+
+        // if you are running the tests in IntelliJ, then you will need to add something like this to your VM options:
+        // -Dproject.build.directory=/Users/dave/src/apache-roller/app/target
+
         // run script to create tables
-        SQLScriptRunner create = 
-            new SQLScriptRunner(System.getProperty("project.build.directory")
+        SQLScriptRunner create = new SQLScriptRunner(System.getProperty("project.build.directory")
                     + "/test-classes/WEB-INF/dbscripts/dummydb/createdb-"+dbname+".sql");
         create.runScript(con, true);
 

docs/roller-install-guide.adoc

@@ -35,48 +35,46 @@ of the Apache Software Foundation.
 
 == Securing Roller
 
-Security should be top-of-mind when setting up any web site, even one
-that is on a private network and internal to your organization. Here are
-some recommendations for keeping your Roller installation secure:
-
-* *Perform Roller installation on a secure network*. When you are
-installing Roller it is possible for other users to interfere with your
-installation. If other users have access to the server, one of them
-could create the admin account before you do. So, when you install
-Roller, do so on a server that cannot be accessed by others.
-* *Do not allow open registration of new users*. Roller can offer a
-registration link so that new users can register themselves, but this
-feature is turned off because it is not safe to allow just anybody to
-register for an account on your blog server. If you want to turn it on,
-login as an administrative user, go to Roller’s Server Administration
-page and enable the *Allow New Users* option.
-* *Enable HTML Sanitization*. If you cannot trust the webloggers who
-will use your Roller site to author HTML, then you should configure
-Roller to sanitize all HTML published by the system. Do this by setting
-the _weblogAdminsUntrusted=true_ property in your
-_roller-custom.properties_ file.
-* *Do not allow File Uploads*. By default Roller allows users to upload
-files for display on their blogs. If don't trust your users, this is unsafe
-and you should disable File Uploads via the Server Administration page.
-* *Do not allow HTML in comments*. Roller can allow users to write
-comments in a safe-subset of HTML, but HTML use in comments is not
-allowed at all because of security concerns with even a so called
-safe-subset of HTML. If you want to turn it on, login as an
-administrative user, go to Roller’s Server Administration page, enable
-the *Allow html in comments* option and make sure the *HTML Subset
-Restriction* box is checked.
-* *Run Roller over SSL connection*. If you run Roller over a plain old
-HTTP connection, it is possible for others to snoop your password when
-you login, for example over an open WIFI network. To configure Roller to
-work over SSL (i.e., using https:// URLs), first modify the web.xml
-located in the Roller WAR (WEB-INF folder), uncommenting the
-<security-constraint/> element and following the instructions given in
-that file above that element. Next, follow your servlet container’s
-documentation for setting up SSL
-(http://tomcat.apache.org/tomcat-7.0-doc/ssl-howto.html for Tomcat, for
-example.) Then redeploy Roller and confirm that pages containing secure
-data such as the login page and new user registration page are available
-only via https:// URLs.
+Security is crucial when setting up any website, even on a private network.
+Here are some recommendations to keep your Roller installation secure:
+
+* *Install Roller on a secure network*. During installation, other users could
+interfere or access the Roller database or files. To prevent this, install Roller
+on a secure network or when the server is not in use by others.
+
+* *Disable new user registrations*. By default, Roller allows self-registration,
+which means anyone can create an account. To prevent this, disable the
+*Allow New Users* option on the Server Administration page.
+
+* *Use SSL for Roller*. Running Roller over HTTP can expose your password
+to snooping, especially on open WIFI networks. To configure SSL (https:// URLs),
+modify the web.xml in the Roller WAR (WEB-INF folder) by uncommenting
+the <security-constraint/> element and following the instructions.
+Then, follow your servlet container’s SSL setup documentation
+(e.g., http://tomcat.apache.org/tomcat-7.0-doc/ssl-howto.html for Tomcat).
+Redeploy Roller and ensure secure pages like the login and registration
+pages are accessible only via https:// URLs.
+
+Following these recommendations will help secure your Roller installation against
+common web vulnerabilities.
+
+=== Safer defaults
+
+As of Roller 6.1.4, several default settings have been updated to enhance security
+for multi-user weblog sites:
+
+* *HTML content sanitization*: Roller now sanitizes all HTML content by default
+to prevent malicious content. This is controlled by the _weblogAdminsUntrusted=true_
+property in your _roller-custom.properties_ file.
+
+* *Custom themes disabled*: By default, users cannot create custom themes.
+This can be enabled via the Server Admin page if you trust your users, as custom themes can pose security risks.
+
+* *File uploads disabled*: By default, file uploads are not allowed.
+If you trust your users, you can enable this feature via the Server Admin page.
+
+NOTE: If you are a solo blogger, you can safely enable un-sanitized HTML,
+file uploads, and custom themes by adjusting the above settings.
 
 == Ready to roll?
 

docs/roller-template-guide.adoc

@@ -41,10 +41,10 @@ Design -> Templates pages and you won’t be able to change or
 customize your theme. You need to have ADMIN permission within a weblog
 to be able to do the things described in this guide.
 
-NOTE: It is possible for a Roller site administrator to disable theme
-customization. So if you do have ADMIN permission in your weblog and you
-still don’t see the Design -> Templates page, perhaps your Roller
-site does not allow customization.
+NOTE: By default, theme customization is disabled in Roller.
+If you do have ADMIN permission in your weblog and you don’t see the
+Design -> Templates page, perhaps your Roller site does not allow customization.
+See your Roller Administrator about enabling custom themes on the Server Admin page.
 
 == The Roller template system