avoid writing the keystore if no changes were made

Author: rogin

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

@@ -53,7 +53,6 @@
 import javax.net.ssl.SSLSocketFactory;
 import javax.xml.parsers.DocumentBuilderFactory;
 
-import com.mirth.connect.client.core.BrandingConstants;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.collections4.MapUtils;
@@ -63,6 +62,7 @@
 import org.apache.commons.configuration2.builder.FileBasedConfigurationBuilder;
 import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.apache.commons.dbutils.DbUtils;
+import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.ArrayUtils;
 import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.StringUtils;
@@ -75,7 +75,6 @@
 import org.apache.ibatis.session.SqlSessionManager;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.bouncycastle.asn1.ASN1Sequence;
 import org.bouncycastle.asn1.x500.X500Name;
 import org.bouncycastle.asn1.x509.AuthorityKeyIdentifier;
 import org.bouncycastle.asn1.x509.BasicConstraints;
@@ -94,6 +93,7 @@
 import com.mirth.commons.encryption.Encryptor;
 import com.mirth.commons.encryption.KeyEncryptor;
 import com.mirth.commons.encryption.Output;
+import com.mirth.connect.client.core.BrandingConstants;
 import com.mirth.connect.client.core.ControllerException;
 import com.mirth.connect.client.core.PropertiesConfigurationUtil;
 import com.mirth.connect.donkey.model.DatabaseConstants;
@@ -463,7 +463,7 @@ public void initialize() {
         } catch (Exception e) {
             logger.error("Failed to initialize configuration controller", e);
         } finally {
-            ResourceUtil.closeResourceQuietly(versionPropertiesStream);
+            IOUtils.closeQuietly(versionPropertiesStream);
         }
     }
 
@@ -1234,6 +1234,8 @@ public void initializeSecuritySettings() {
                 keyStore = KeyStore.getInstance(mirthConfig.getString("keystore.type", "JCEKS"));
             }
 
+            boolean dirtiedKeystore = false;
+
             if (keyStoreFile.exists()) {
                 keyStoreFileIs = new FileInputStream(keyStoreFile);
                 keyStore.load(keyStoreFileIs, keyStorePassword);
@@ -1256,20 +1258,23 @@ public void initializeSecuritySettings() {
                 }
 
                 keyStore.load(null, keyStorePassword);
+                dirtiedKeystore = true;
                 logger.debug("keystore file not found, created new one");
             }
 
-            configureEncryption(provider, keyStore, keyPassword);
-            generateDefaultCertificate(provider, keyStore, keyPassword);
+            dirtiedKeystore |= configureEncryption(provider, keyStore, keyPassword);
+            dirtiedKeystore |= generateDefaultCertificate(provider, keyStore, keyPassword);
 
-            // write the keystore back to the file
-            fos = new FileOutputStream(keyStoreFile);
-            keyStore.store(fos, keyStorePassword);
+            // only re-write the keystore if it was changed
+            if(dirtiedKeystore) {
+                fos = new FileOutputStream(keyStoreFile);
+                keyStore.store(fos, keyStorePassword);
+            }
         } catch (Exception e) {
             logger.error("Could not initialize security settings.", e);
         } finally {
-            ResourceUtil.closeResourceQuietly(keyStoreFileIs);
-            ResourceUtil.closeResourceQuietly(fos);
+            IOUtils.closeQuietly(keyStoreFileIs);
+            IOUtils.closeQuietly(fos);
         }
     }
 
@@ -1352,7 +1357,7 @@ private void saveMirthConfig() throws FileNotFoundException, ConfigurationExcept
         try {
             PropertiesConfigurationUtil.saveTo(mirthConfig, os);
         } finally {
-            ResourceUtil.closeResourceQuietly(os);
+            IOUtils.closeQuietly(os);
         }
     }
 
@@ -1433,8 +1438,8 @@ public void migrateKeystore() {
         } catch (Exception e) {
             logger.error("Error migrating encryption key from database to keystore.", e);
         } finally {
-            ResourceUtil.closeResourceQuietly(mirthPropsIs);
-            ResourceUtil.closeResourceQuietly(keyStoreOuputStream);
+            IOUtils.closeQuietly(mirthPropsIs);
+            IOUtils.closeQuietly(keyStoreOuputStream);
         }
     }
 
@@ -1446,17 +1451,15 @@ public void updatePropertiesConfiguration(PropertiesConfiguration config) {
     /**
      * Instantiates the encryptor and digester using the configuration properties. If the properties
      * are not found, reasonable defaults are used.
-     * 
-     * @param provider
-     *            The provider to use (ex. BC)
-     * @param keyStore
-     *            The keystore from which to load the secret encryption key
-     * @param keyPassword
-     *            The secret key password
+     *
+     * @param provider    The provider to use (ex. BC)
+     * @param keyStore    The keystore from which to load the secret encryption key
+     * @param keyPassword The secret key password
      * @throws Exception
      */
-    private void configureEncryption(Provider provider, KeyStore keyStore, char[] keyPassword) throws Exception {
+    private boolean configureEncryption(Provider provider, KeyStore keyStore, char[] keyPassword) throws Exception {
         SecretKey secretKey = null;
+        boolean dirtiedKeystore = false;
 
         if (!keyStore.containsAlias(SECRET_KEY_ALIAS)) {
             logger.debug("encryption key not found, generating new one");
@@ -1465,6 +1468,7 @@ private void configureEncryption(Provider provider, KeyStore keyStore, char[] ke
             secretKey = keyGenerator.generateKey();
             KeyStore.SecretKeyEntry entry = new KeyStore.SecretKeyEntry(secretKey);
             keyStore.setEntry(SECRET_KEY_ALIAS, entry, new KeyStore.PasswordProtection(keyPassword));
+            dirtiedKeystore = true;
         } else {
             logger.debug("found encryption key in keystore");
             secretKey = (SecretKey) keyStore.getKey(SECRET_KEY_ALIAS, keyPassword);
@@ -1509,15 +1513,18 @@ private void configureEncryption(Provider provider, KeyStore keyStore, char[] ke
             );
             // @formatter:on
         }
+
+        return dirtiedKeystore;
     }
 
     /**
      * Checks for an existing certificate to use for secure communication between the server and
      * client. If no certficate exists, this will generate a new one.
      * 
      */
-    private void generateDefaultCertificate(Provider provider, KeyStore keyStore, char[] keyPassword) throws Exception {
+    private boolean generateDefaultCertificate(Provider provider, KeyStore keyStore, char[] keyPassword) throws Exception {
         final String certificateAlias = "mirthconnect";
+        boolean dirtiedKeystore = false;
 
         if (!keyStore.containsAlias(certificateAlias)) {
             // Common CA and SSL cert attributes
@@ -1555,9 +1562,12 @@ private void generateDefaultCertificate(Provider provider, KeyStore keyStore, ch
             // add the generated SSL cert to the keystore using the key password
             keyStore.setKeyEntry(certificateAlias, sslKeyPair.getPrivate(), keyPassword, new Certificate[] {
                     sslCert });
+            dirtiedKeystore = true;
         } else {
             logger.debug("found certificate in keystore");
         }
+
+        return dirtiedKeystore;
     }
 
     private boolean isDatabaseRunning() {

server/src/com/mirth/connect/server/util/ResourceUtil.java

@@ -44,6 +44,13 @@ public static InputStream getResourceStream(Class<?> clazz, String resourceName)
         return is;
     }
 
+    /**
+     * Close a Closeable resource quietly. Trap and ignore any exceptions thrown when calling close()
+     *
+     * @deprecated import org.apache.commons.io.IOUtils does this and is already on the classpath. Further deprecated by try-with-resources since Java 7
+     * @param resource the Closeable object to try and close
+     */
+    @Deprecated
     public static void closeResourceQuietly(Closeable resource) {
         if (resource != null) {
             try {