Secure keystores with filesystem acl, use pkcs12

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

Author: mgaffigan

server/basedir-includes/configure-from-env

@@ -16,10 +16,16 @@ if [ $custom_extension_count != 0 ]; then
 	done
 fi
 
-# set storepass and keypass to 'changeme' so they aren't overwritten later
-KEYSTORE_PASS=changeme
-sed -i "s/^keystore\.storepass\s*=\s*.*\$/keystore.storepass = ${KEYSTORE_PASS//\//\\/}/" "$APP_DIR/conf/mirth.properties"
-sed -i "s/^keystore\.keypass\s*=\s*.*\$/keystore.keypass = ${KEYSTORE_PASS//\//\\/}/" "$APP_DIR/conf/mirth.properties"
+# The keystore is created with no passphrase and protected by file permissions instead, so there is
+# no longer a password to pin here. Keystores created by earlier versions are JCEKS and were pinned
+# to 'changeme', so keep those settings when one is already present in appdata.
+KEYSTORE_FILE="$APP_DIR/appdata/keystore.jks"
+if [ -f "$KEYSTORE_FILE" ] && [ "$(head -c 4 "$KEYSTORE_FILE" | od -An -tx1 | tr -d '[:space:]')" = "cececece" ]; then
+	echo "Found an existing JCEKS keystore, keeping the previous keystore settings."
+	sed -i "s/^keystore\.storepass\s*=\s*.*\$/keystore.storepass = changeme/" "$APP_DIR/conf/mirth.properties"
+	sed -i "s/^keystore\.keypass\s*=\s*.*\$/keystore.keypass = changeme/" "$APP_DIR/conf/mirth.properties"
+	sed -i "s/^keystore\.type\s*=\s*.*\$/keystore.type = JCEKS/" "$APP_DIR/conf/mirth.properties"
+fi
 
 # merge the environment variables into /opt/engine/conf/mirth.properties
 # db type

server/conf/mirth.properties

@@ -25,10 +25,13 @@ password.reuselimit = 0
 version = 4.6.0
 
 # keystore
+# The keystore is created on first boot with no passphrase and is instead protected by file
+# permissions that restrict it to the user the server runs as. Set storepass/keypass if you would
+# rather protect it with a passphrase, and set type to JCEKS for keystores created before 4.6.0.
 keystore.path = ${dir.appdata}/keystore.jks
-keystore.storepass = 81uWxplDtB
-keystore.keypass = 81uWxplDtB
-keystore.type = JCEKS
+keystore.storepass =
+keystore.keypass =
+keystore.type = PKCS12
 
 # server
 http.contextpath = /

server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java

@@ -13,6 +13,7 @@
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.OutputStream;
@@ -121,6 +122,7 @@
 import com.mirth.connect.server.mybatis.KeyValuePair;
 import com.mirth.connect.server.tools.ClassPathResource;
 import com.mirth.connect.server.util.DatabaseUtil;
+import com.mirth.connect.server.util.FilePermissionUtil;
 import com.mirth.connect.server.util.PasswordRequirementsChecker;
 import com.mirth.connect.server.util.ResourceUtil;
 import com.mirth.connect.server.util.SqlConfig;
@@ -195,8 +197,6 @@ public class DefaultConfigurationController extends ConfigurationController {
     private static final String XSTREAM_ALLOW_TYPES = "xstream.allowtypes";
     private static final String XSTREAM_ALLOW_TYPE_HIERARCHIES = "xstream.allowtypehierarchies";
 
-    private static final String DEFAULT_STOREPASS = "81uWxplDtB";
-
     // singleton pattern
     private static ConfigurationController instance = null;
 
@@ -1239,22 +1239,6 @@ public void initializeSecuritySettings() {
                 keyStore.load(keyStoreFileIs, keyStorePassword);
                 logger.debug("found and loaded keystore: " + keyStoreFile.getAbsolutePath());
             } else {
-                /*
-                 * If a new keystore is being created, and the passwords are the defaults, then
-                 * create new passwords.
-                 */
-                if (Arrays.equals(keyStorePassword, DEFAULT_STOREPASS.toCharArray()) && Arrays.equals(keyPassword, DEFAULT_STOREPASS.toCharArray())) {
-                    String keyStorePasswordStr = generateNewPassword();
-                    mirthConfig.setProperty("keystore.storepass", keyStorePasswordStr);
-                    keyStorePassword = keyStorePasswordStr.toCharArray();
-
-                    String keyPasswordStr = generateNewPassword();
-                    mirthConfig.setProperty("keystore.keypass", keyPasswordStr);
-                    keyPassword = keyPasswordStr.toCharArray();
-
-                    saveMirthConfig();
-                }
-
                 keyStore.load(null, keyStorePassword);
                 logger.debug("keystore file not found, created new one");
             }
@@ -1263,6 +1247,7 @@ public void initializeSecuritySettings() {
             generateDefaultCertificate(provider, keyStore, keyPassword);
 
             // write the keystore back to the file
+            FilePermissionUtil.createOwnerOnlyFile(keyStoreFile);
             fos = new FileOutputStream(keyStoreFile);
             keyStore.store(fos, keyStorePassword);
         } catch (Exception e) {
@@ -1273,19 +1258,6 @@ public void initializeSecuritySettings() {
         }
     }
 
-    /**
-     * Creates a random 12-character alphanumeric password.
-     */
-    private String generateNewPassword() {
-        String characters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
-        SecureRandom random = new SecureRandom();
-        StringBuilder builder = new StringBuilder();
-        for (int i = 1; i <= 12; i++) {
-            builder.append(characters.charAt(random.nextInt(characters.length())));
-        }
-        return builder.toString();
-    }
-
     @Override
     public void initializeDatabaseSettings() {
         try {

server/src/main/java/com/mirth/connect/server/util/FilePermissionUtil.java

@@ -0,0 +1,81 @@
+package com.mirth.connect.server.util;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.AclEntry;
+import java.nio.file.attribute.AclEntryPermission;
+import java.nio.file.attribute.AclEntryType;
+import java.nio.file.attribute.AclFileAttributeView;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
+import java.nio.file.attribute.UserPrincipal;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.Set;
+
+public class FilePermissionUtil {
+
+    private static final Set<PosixFilePermission> OWNER_ONLY = EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE);
+
+    private FilePermissionUtil() {}
+
+    /*
+     * Creates the file if it does not already exist, and restricts it so that only the user the
+     * server runs as may read or write it. This is what protects files that hold key material in
+     * place of a passphrase, so the file must never be created readable and then locked down
+     * afterwards; on POSIX the permissions are applied as part of the create itself.
+     */
+    public static void createOwnerOnlyFile(File file) throws IOException {
+        Path path = file.toPath();
+
+        if (!Files.exists(path)) {
+            File parent = file.getParentFile();
+
+            if (parent != null) {
+                Files.createDirectories(parent.toPath());
+            }
+
+            if (isPosix(path)) {
+                Files.createFile(path, PosixFilePermissions.asFileAttribute(OWNER_ONLY));
+                return;
+            }
+
+            Files.createFile(path);
+        }
+
+        restrictToOwner(path);
+    }
+
+    /*
+     * Replaces the permissions on an existing file with owner read/write only. On Windows the
+     * entire ACL is replaced with a single entry for the file's owner, which also detaches the file
+     * from any permissions inherited from its parent directory.
+     */
+    private static void restrictToOwner(Path path) throws IOException {
+        if (isPosix(path)) {
+            Files.setPosixFilePermissions(path, OWNER_ONLY);
+            return;
+        }
+
+        AclFileAttributeView aclView = Files.getFileAttributeView(path, AclFileAttributeView.class);
+
+        if (aclView != null) {
+            UserPrincipal owner = aclView.getOwner();
+            // @formatter:off
+            AclEntry entry = AclEntry.newBuilder()
+                    .setType(AclEntryType.ALLOW)
+                    .setPrincipal(owner)
+                    .setPermissions(EnumSet.allOf(AclEntryPermission.class))
+                    .build();
+            // @formatter:on
+            aclView.setAcl(Collections.singletonList(entry));
+        }
+    }
+
+    private static boolean isPosix(Path path) {
+        return Files.getFileAttributeView(path, PosixFileAttributeView.class) != null;
+    }
+}

server/src/test/java/com/mirth/connect/server/util/FilePermissionUtilTest.java

@@ -0,0 +1,61 @@
+package com.mirth.connect.server.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.PosixFileAttributeView;
+import java.nio.file.attribute.PosixFilePermissions;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class FilePermissionUtilTest {
+
+    @Rule
+    public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+    @Test
+    public void testCreatesMissingFileOwnerOnly() throws Exception {
+        File file = new File(temporaryFolder.getRoot(), "nested/keystore.p12");
+
+        FilePermissionUtil.createOwnerOnlyFile(file);
+
+        assertTrue(file.exists());
+        assertPermissions(file);
+    }
+
+    @Test
+    public void testRestrictsExistingFile() throws Exception {
+        File file = temporaryFolder.newFile("keystore.p12");
+        assumeTrue(file.setReadable(true, false));
+
+        FilePermissionUtil.createOwnerOnlyFile(file);
+
+        assertPermissions(file);
+    }
+
+    @Test
+    public void testPreservesExistingContent() throws Exception {
+        File file = temporaryFolder.newFile("keystore.p12");
+
+        try (FileOutputStream fos = new FileOutputStream(file)) {
+            fos.write(new byte[] { 1, 2, 3 });
+        }
+
+        FilePermissionUtil.createOwnerOnlyFile(file);
+
+        assertEquals(3, file.length());
+    }
+
+    private void assertPermissions(File file) throws Exception {
+        Path path = file.toPath();
+        assumeTrue(Files.getFileAttributeView(path, PosixFileAttributeView.class) != null);
+        assertEquals("rw-------", PosixFilePermissions.toString(Files.getPosixFilePermissions(path)));
+    }
+}

server/src/test/resources/mirth.properties

@@ -25,10 +25,13 @@ password.reuselimit = 0
 version = 4.6.0
 
 # keystore
+# Created if missing with no passphrase and secured by chmod 600.
 keystore.path = ${dir.appdata}/keystore.jks
-keystore.storepass = 81uWxplDtB
-keystore.keypass = 81uWxplDtB
-keystore.type = JCEKS
+keystore.storepass =
+keystore.keypass =
+# Set to JCEKS for compatibility with older versions of the engine.
+# a PKCS12 is a standard pfx file.  JCEKS is a the older java-specific format.
+keystore.type = PKCS12
 
 # server
 http.contextpath = /