Merge pull request #797 from GregDThomas/OF-1159

Fix OF-1159 - add database column to identify encryption status

src/database/openfire_db2.sql

@@ -123,6 +123,7 @@ CREATE TABLE ofID (
 CREATE TABLE ofProperty (
   name        VARCHAR(100) NOT NULL,
   propValue   VARCHAR(3000) NOT NULL,
+  encrypted   INTEGER,
   CONSTRAINT ofProperty_pk PRIMARY KEY (name)
 );
 
@@ -392,7 +393,7 @@ INSERT INTO ofID (idType, id) VALUES (19, 1);
 INSERT INTO ofID (idType, id) VALUES (23, 1);
 INSERT INTO ofID (idType, id) VALUES (26, 2);
 
-INSERT INTO ofVersion (name, version) VALUES ('openfire', 25);
+INSERT INTO ofVersion (name, version) VALUES ('openfire', 26);
 
 -- Entry for admin user
 INSERT INTO ofUser (username, plainPassword, name, email, creationDate, modificationDate)

src/database/openfire_hsqldb.sql

@@ -123,6 +123,7 @@ CREATE TABLE ofID (
 CREATE TABLE ofProperty (
   name        VARCHAR(100)  NOT NULL,
   propValue   VARCHAR(4000) NOT NULL,
+  encrypted   INTEGER,
   CONSTRAINT ofProperty_pk PRIMARY KEY (name)
 );
 
@@ -378,7 +379,7 @@ INSERT INTO ofID (idType, id) VALUES (19, 1);
 INSERT INTO ofID (idType, id) VALUES (23, 1);
 INSERT INTO ofID (idType, id) VALUES (26, 2);
 
-INSERT INTO ofVersion (name, version) VALUES ('openfire', 25);
+INSERT INTO ofVersion (name, version) VALUES ('openfire', 26);
 
 // Entry for admin user
 INSERT INTO ofUser (username, plainPassword, name, email, creationDate, modificationDate)

src/database/openfire_mysql.sql

@@ -112,6 +112,7 @@ CREATE TABLE ofID (
 CREATE TABLE ofProperty (
   name        VARCHAR(100)              NOT NULL,
   propValue   TEXT                      NOT NULL,
+  encrypted   INTEGER,
   PRIMARY KEY (name)
 );
 
@@ -367,7 +368,7 @@ INSERT INTO ofID (idType, id) VALUES (19, 1);
 INSERT INTO ofID (idType, id) VALUES (23, 1);
 INSERT INTO ofID (idType, id) VALUES (26, 2);
 
-INSERT INTO ofVersion (name, version) VALUES ('openfire', 25);
+INSERT INTO ofVersion (name, version) VALUES ('openfire', 26);
 
 # Entry for admin user
 INSERT INTO ofUser (username, plainPassword, name, email, creationDate, modificationDate)

src/database/openfire_oracle.sql

@@ -121,6 +121,7 @@ CREATE TABLE ofID (
 CREATE TABLE ofProperty (
   name        VARCHAR2(100) NOT NULL,
   propValue   VARCHAR2(4000) NOT NULL,
+  encrypted   INTEGER,
   CONSTRAINT ofProperty_pk PRIMARY KEY (name)
 );
 
@@ -376,7 +377,7 @@ INSERT INTO ofID (idType, id) VALUES (19, 1);
 INSERT INTO ofID (idType, id) VALUES (23, 1);
 INSERT INTO ofID (idType, id) VALUES (26, 2);
 
-INSERT INTO ofVersion (name, version) VALUES ('openfire', 25);
+INSERT INTO ofVersion (name, version) VALUES ('openfire', 26);
 
 -- Entry for admin user
 INSERT INTO ofUser (username, plainPassword, name, email, creationDate, modificationDate)

src/database/openfire_postgresql.sql

@@ -128,6 +128,7 @@ CREATE TABLE ofID (
 CREATE TABLE ofProperty (
   name        VARCHAR(100) NOT NULL,
   propValue   VARCHAR(4000) NOT NULL,
+  encrypted   INTEGER,
   CONSTRAINT ofProperty_pk PRIMARY KEY (name)
 );
 
@@ -384,7 +385,7 @@ INSERT INTO ofID (idType, id) VALUES (19, 1);
 INSERT INTO ofID (idType, id) VALUES (23, 1);
 INSERT INTO ofID (idType, id) VALUES (26, 2);
 
-INSERT INTO ofVersion (name, version) VALUES ('openfire', 25);
+INSERT INTO ofVersion (name, version) VALUES ('openfire', 26);
 
 -- Entry for admin user
 INSERT INTO ofUser (username, plainPassword, name, email, creationDate, modificationDate)

src/database/openfire_sqlserver.sql

@@ -126,6 +126,7 @@ CREATE TABLE ofID (
 CREATE TABLE ofProperty (
   name        NVARCHAR(100) NOT NULL,
   propValue   NTEXT NOT NULL,
+  encrypted   INTEGER,
   CONSTRAINT ofProperty_pk PRIMARY KEY (name)
 );
 
@@ -381,7 +382,7 @@ INSERT INTO ofID (idType, id) VALUES (19, 1);
 INSERT INTO ofID (idType, id) VALUES (23, 1);
 INSERT INTO ofID (idType, id) VALUES (26, 2);
 
-INSERT INTO ofVersion (name, version) VALUES ('openfire', 25);
+INSERT INTO ofVersion (name, version) VALUES ('openfire', 26);
 
 /* Entry for admin user */
 INSERT INTO ofUser (username, plainPassword, name, email, creationDate, modificationDate)

src/database/openfire_sybase.sql

@@ -126,6 +126,7 @@ CREATE TABLE ofID (
 CREATE TABLE ofProperty (
   name         NVARCHAR(100) NOT NULL,
   propValue    TEXT NOT NULL,
+  encrypted    INTEGER,
   CONSTRAINT ofProperty_pk PRIMARY KEY (name)
 );
 
@@ -382,7 +383,7 @@ INSERT INTO ofID (idType, id) VALUES (19, 1);
 INSERT INTO ofID (idType, id) VALUES (23, 1);
 INSERT INTO ofID (idType, id) VALUES (26, 2);
 
-INSERT INTO ofVersion (name, version) VALUES ('openfire', 25);
+INSERT INTO ofVersion (name, version) VALUES ('openfire', 26);
 
 /* Entry for admin user */
 INSERT INTO ofUser (username, plainPassword, name, email, creationDate, modificationDate)

src/database/upgrade/26/openfire_db2.sql

+ALTER TABLE ofProperty
+  ADD encrypted   INTEGER;
+
+UPDATE ofVersion SET version = 26 WHERE name = 'openfire';
\ No newline at end of file

src/database/upgrade/26/openfire_hsqldb.sql

+ALTER TABLE ofProperty
+  ADD encrypted   INTEGER;
+
+UPDATE ofVersion SET version = 26 WHERE name = 'openfire';
\ No newline at end of file

src/database/upgrade/26/openfire_mysql.sql

+ALTER TABLE ofProperty
+  ADD encrypted   INTEGER;
+
+UPDATE ofVersion SET version = 26 WHERE name = 'openfire';
\ No newline at end of file

src/database/upgrade/26/openfire_oracle.sql

+ALTER TABLE ofProperty
+  ADD encrypted   INTEGER;
+
+UPDATE ofVersion SET version = 26 WHERE name = 'openfire';
\ No newline at end of file

src/database/upgrade/26/openfire_postgresql.sql

+ALTER TABLE ofProperty
+  ADD encrypted   INTEGER;
+
+UPDATE ofVersion SET version = 26 WHERE name = 'openfire';
\ No newline at end of file

src/database/upgrade/26/openfire_sqlserver.sql

+ALTER TABLE ofProperty
+  ADD encrypted   INTEGER;
+
+UPDATE ofVersion SET version = 26 WHERE name = 'openfire';
\ No newline at end of file

src/database/upgrade/26/openfire_sybase.sql

+ALTER TABLE ofProperty
+  ADD encrypted   INTEGER;
+
+UPDATE ofVersion SET version = 26 WHERE name = 'openfire';
\ No newline at end of file

src/java/org/jivesoftware/database/SchemaManager.java

@@ -66,7 +66,7 @@ public class SchemaManager {
     /**
      * Current Openfire database schema version.
      */
-    private static final int DATABASE_VERSION = 25;
+    private static final int DATABASE_VERSION = 26;
 
     /**
      * Checks the Openfire database schema to ensure that it's installed and up to date.

src/java/org/jivesoftware/util/JiveGlobals.java

@@ -777,13 +777,25 @@ public class JiveGlobals {
      * @param value the value of the property being set.
      */
     public static void setProperty(String name, String value) {
+        setProperty(name, value, false);
+    }
+
+    /**
+     * Sets a Jive property. If the property doesn't already exists, a new
+     * one will be created.
+     *
+     * @param name the name of the property being set.
+     * @param value the value of the property being set.
+     * @param encrypt {@code true} to encrypt the property in the database, other {@code false}
+     */
+    public static void setProperty(String name, String value, boolean encrypt) {
         if (properties == null) {
             if (isSetupMode()) {
                 return;
             }
             properties = JiveProperties.getInstance();
         }
-        properties.put(name, value);
+        properties.put(name, value, encrypt);
     }
 
     /**
@@ -883,7 +895,23 @@ public class JiveGlobals {
             properties = JiveProperties.getInstance();
         }
         properties.remove(name);
-        setPropertyEncrypted(name, false);
+        clearXMLPropertyEncryptionEntry(name);
+    }
+
+    static void clearXMLPropertyEncryptionEntry(String name) {
+        if (isSetupMode()) {
+            return;
+        }
+        if (securityProperties == null) {
+            loadSecurityProperties();
+        }
+        if (openfireProperties == null) {
+            loadOpenfireProperties();
+        }
+        // Note; only remove the encryption indicator from XML file if the (encrypted) property is not also defined in the XML file
+        if (JiveGlobals.isXMLPropertyEncrypted(name) && openfireProperties.getProperty(name) == null) {
+            securityProperties.removeFromList(ENCRYPTED_PROPERTY_NAMES, name);
+        }
     }
 
     /**
@@ -920,19 +948,38 @@ public class JiveGlobals {
     }
 
 
+    /**
+     * Determines whether an XML property is configured for encryption.
+     *
+     * @param name
+     *            The name of the property
+     * @return {@code true} if the property is stored using encryption, otherwise {@code false}
+     */
+    static boolean isXMLPropertyEncrypted(final String name) {
+        if (securityProperties == null) {
+            loadSecurityProperties();
+        }
+        return name != null &&
+                !name.startsWith(JiveGlobals.ENCRYPTED_PROPERTY_NAME_PREFIX) &&
+                securityProperties.getProperties(JiveGlobals.ENCRYPTED_PROPERTY_NAMES, true).contains(name);
+
+    }
+
     /**
      * Determines whether a property is configured for encryption.
      * 
-     * @param name The name of the property
-     * @return True if the property is stored using encryption, otherwise false
+     * @param name
+     *            The name of the property
+     * @return {@code true} if the property is stored using encryption, otherwise {@code false}
      */
     public static boolean isPropertyEncrypted(String name) {
-    	if (securityProperties == null) {
-    		loadSecurityProperties();
-    	}
-    	return	name != null && 
-    			!name.startsWith(ENCRYPTED_PROPERTY_NAME_PREFIX) &&
-    			securityProperties.getProperties(ENCRYPTED_PROPERTY_NAMES, true).contains(name);
+        if (properties == null) {
+            if (isSetupMode()) {
+                return false;
+            }
+            properties = JiveProperties.getInstance();
+        }
+        return properties.isEncrypted(name);
     }
 
     /**
@@ -943,19 +990,13 @@ public class JiveGlobals {
      * @return True if the property's encryption status changed, otherwise false
      */
     public static boolean setPropertyEncrypted(String name, boolean encrypt) {
-    	if (securityProperties == null) {
-    		loadSecurityProperties();
-    	}
-    	boolean propertyWasChanged;
-    	if (encrypt) {
-    		propertyWasChanged = securityProperties.addToList(ENCRYPTED_PROPERTY_NAMES, name);
-    	} else {
-    		propertyWasChanged = securityProperties.removeFromList(ENCRYPTED_PROPERTY_NAMES, name);
-    	}
-    	if (propertyWasChanged) {
-    		resetProperty(name);
-    	}
-    	return propertyWasChanged;
+        if (properties == null) {
+            if (isSetupMode()) {
+                return false;
+            }
+            properties = JiveProperties.getInstance();
+        }
+        return properties.setPropertyEncrypted(name, encrypt);
     }
 
     /**

src/java/org/jivesoftware/util/JiveProperties.java

@@ -42,14 +42,17 @@ public class JiveProperties implements Map<String, String> {
 
 	private static final Logger Log = LoggerFactory.getLogger(JiveProperties.class);
 
-    private static final String LOAD_PROPERTIES = "SELECT name, propValue FROM ofProperty";
-    private static final String INSERT_PROPERTY = "INSERT INTO ofProperty(name, propValue) VALUES(?,?)";
-    private static final String UPDATE_PROPERTY = "UPDATE ofProperty SET propValue=? WHERE name=?";
+    private static final String LOAD_PROPERTIES = "SELECT name, propValue, encrypted FROM ofProperty";
+    private static final String INSERT_PROPERTY = "INSERT INTO ofProperty(name, propValue, encrypted) VALUES(?,?,?)";
+    private static final String UPDATE_PROPERTY = "UPDATE ofProperty SET propValue=?, encrypted=? WHERE name=?";
     private static final String DELETE_PROPERTY = "DELETE FROM ofProperty WHERE name LIKE ?";
 
     private static JiveProperties instance = null;
 
+    // The map of property keys to their values
     private Map<String, String> properties;
+    // The map of property keys to a boolean indicating if they are encrypted or not
+    private Map<String, Boolean> encrypted;
 
     /**
      * Returns a singleton instance of JiveProperties.
@@ -76,9 +79,11 @@ public class JiveProperties implements Map<String, String> {
     public void init() {
         if (properties == null) {
             properties = new ConcurrentHashMap<>();
+            encrypted = new ConcurrentHashMap<>();
         }
         else {
             properties.clear();
+            encrypted.clear();
         }
 
         loadProperties();
@@ -136,6 +141,39 @@ public class JiveProperties implements Map<String, String> {
         return properties.get(key);
     }
 
+    /**
+     * Indicates the encryption status for the given property.
+     * 
+     * @param name
+     *            The name of the property
+     * @return {@code true} if the property exists and is encrypted, otherwise {@code false}
+     */
+    boolean isEncrypted(final String name) {
+        if (name == null) {
+            return false;
+        }
+        final Boolean isEncrypted = encrypted.get(name);
+        return isEncrypted != null && isEncrypted;
+    }
+
+    /**
+     * Set the encryption status for the given property.
+     *
+     * @param name
+     *            The name of the property
+     * @param encrypt
+     *            True to encrypt the property, false to decrypt
+     * @return {@code true} if the property's encryption status changed, otherwise {@code false}
+     */
+    boolean setPropertyEncrypted(String name, boolean encrypt) {
+        final boolean encryptionWasChanged = name != null && properties.containsKey(name) && isEncrypted(name) != encrypt;
+        if (encryptionWasChanged) {
+            final String value = get(name);
+            put(name, value, encrypt);
+        }
+        return encryptionWasChanged;
+    }
+    
     /**
      * Return all children property names of a parent property as a Collection
      * of String objects. For example, given the properties <tt>X.Y.A</tt>,
@@ -217,8 +255,19 @@ public class JiveProperties implements Map<String, String> {
         PropertyEventDispatcher.dispatchEvent(key, PropertyEventDispatcher.EventType.property_deleted, params);
     }
 
-    @Override
-    public String put(String key, String value) {
+    /**
+     * Saves a property, optionally encrypting it
+     * 
+     * @param key
+     *            The name of the property
+     * @param value
+     *            The value of the property
+     * @param isEncrypted
+     *            {@code true} to encrypt the property, {@code true} to leave in plain text
+     * @return The previous value associated with {@code key}, or {@code null} if there was no mapping for
+     *         {@code key}.
+     */
+    public String put(String key, String value, boolean isEncrypted) {
         if (value == null) {
             // This is the same as deleting, so remove it.
             return remove(key);
@@ -234,15 +283,16 @@ public class JiveProperties implements Map<String, String> {
         String result;
         synchronized (this) {
             if (properties.containsKey(key)) {
-                if (!properties.get(key).equals(value)) {
-                    updateProperty(key, value);
-                }
+                updateProperty(key, value, isEncrypted);
             }
             else {
-                insertProperty(key, value);
+                insertProperty(key, value, isEncrypted);
             }
 
             result = properties.put(key, value);
+            encrypted.put(key, isEncrypted);
+            // We now know the database is correct - so we can remove the entry from security.conf
+            JiveGlobals.clearXMLPropertyEncryptionEntry(key);
         }
 
         // Generate event.
@@ -251,13 +301,21 @@ public class JiveProperties implements Map<String, String> {
         PropertyEventDispatcher.dispatchEvent(key, PropertyEventDispatcher.EventType.property_set, params);
 
         // Send update to other cluster members.
-        CacheFactory.doClusterTask(PropertyClusterEventTask.createPutTask(key, value));
+        CacheFactory.doClusterTask(PropertyClusterEventTask.createPutTask(key, value, isEncrypted));
 
         return result;
     }
 
-    void localPut(String key, String value) {
+    @Override
+    public String put(String key, String value) {
+        return put(key, value, isEncrypted(key));
+    }
+
+    void localPut(String key, String value, boolean isEncrypted) {
         properties.put(key, value);
+        encrypted.put(key, isEncrypted);
+        // We now know the database is correct - so we can remove the entry from security.conf
+        JiveGlobals.clearXMLPropertyEncryptionEntry(key);
 
         // Generate event.
         Map<String, Object> params = new HashMap<>();
@@ -289,7 +347,7 @@ public class JiveProperties implements Map<String, String> {
         }
     }
 
-    private void insertProperty(String name, String value) {
+    private void insertProperty(String name, String value, boolean isEncrypted) {
     	Encryptor encryptor = getEncryptor();
         Connection con = null;
         PreparedStatement pstmt = null;
@@ -297,7 +355,8 @@ public class JiveProperties implements Map<String, String> {
             con = DbConnectionManager.getConnection();
             pstmt = con.prepareStatement(INSERT_PROPERTY);
             pstmt.setString(1, name);
-            pstmt.setString(2, JiveGlobals.isPropertyEncrypted(name) ? encryptor.encrypt(value) : value);
+            pstmt.setString(2, isEncrypted ? encryptor.encrypt(value) : value);
+            pstmt.setInt(3, isEncrypted ? 1 : 0);
             pstmt.executeUpdate();
         }
         catch (SQLException e) {
@@ -308,15 +367,16 @@ public class JiveProperties implements Map<String, String> {
         }
     }
 
-    private void updateProperty(String name, String value) {
+    private void updateProperty(String name, String value, boolean isEncrypted) {
     	Encryptor encryptor = getEncryptor();
         Connection con = null;
         PreparedStatement pstmt = null;
         try {
             con = DbConnectionManager.getConnection();
             pstmt = con.prepareStatement(UPDATE_PROPERTY);
-            pstmt.setString(1, JiveGlobals.isPropertyEncrypted(name) ? encryptor.encrypt(value) : value);
-            pstmt.setString(2, name);
+            pstmt.setString(1, isEncrypted ? encryptor.encrypt(value) : value);
+            pstmt.setInt(2, isEncrypted ? 1 : 0);
+            pstmt.setString(3, name);
             pstmt.executeUpdate();
         }
         catch (SQLException e) {
@@ -356,7 +416,8 @@ public class JiveProperties implements Map<String, String> {
             while (rs.next()) {
                 String name = rs.getString(1);
                 String value = rs.getString(2);
-                if (JiveGlobals.isPropertyEncrypted(name)) {
+                boolean isEncrypted = rs.getInt(3) == 1 || JiveGlobals.isXMLPropertyEncrypted(name);
+                if (isEncrypted) {
                 	try { 
                 		value = encryptor.decrypt(value); 
                 	} catch (Exception ex) {
@@ -365,7 +426,8 @@ public class JiveProperties implements Map<String, String> {
                 	}
                 }
                 if (value != null) { 
-                	properties.put(name, value); 
+                	properties.put(name, value);
+                	encrypted.put(name, isEncrypted);
                 }
             }
         }

src/java/org/jivesoftware/util/PropertyClusterEventTask.java

@@ -33,12 +33,14 @@ public class PropertyClusterEventTask implements ClusterTask<Void> {
     private Type event;
     private String key;
     private String value;
+    private boolean isEncrypted;
 
-    public static PropertyClusterEventTask createPutTask(String key, String value) {
+    public static PropertyClusterEventTask createPutTask(String key, String value, boolean isEncrypted) {
         PropertyClusterEventTask task = new PropertyClusterEventTask();
         task.event = Type.put;
         task.key = key;
         task.value = value;
+        task.isEncrypted = isEncrypted;
         return task;
     }
 
@@ -57,7 +59,7 @@ public class PropertyClusterEventTask implements ClusterTask<Void> {
     @Override
     public void run() {
         if (Type.put == event) {
-            JiveProperties.getInstance().localPut(key, value);
+            JiveProperties.getInstance().localPut(key, value, isEncrypted);
         }
         else if (Type.deleted == event) {
             JiveProperties.getInstance().localRemove(key);
@@ -71,6 +73,7 @@ public class PropertyClusterEventTask implements ClusterTask<Void> {
         ExternalizableUtil.getInstance().writeBoolean(out, value != null);
         if (value != null) {
             ExternalizableUtil.getInstance().writeSafeUTF(out, value);
+            ExternalizableUtil.getInstance().writeBoolean(out, isEncrypted);
         }
     }
 
@@ -80,6 +83,7 @@ public class PropertyClusterEventTask implements ClusterTask<Void> {
         key = ExternalizableUtil.getInstance().readSafeUTF(in);
         if (ExternalizableUtil.getInstance().readBoolean(in)) {
             value = ExternalizableUtil.getInstance().readSafeUTF(in);
+            isEncrypted = ExternalizableUtil.getInstance().readBoolean(in);
         }
     }
 

src/java/org/jivesoftware/util/XMLProperties.java

@@ -207,7 +207,7 @@ public class XMLProperties {
         }
         else {
         	// check to see if the property is marked as encrypted
-        	if (JiveGlobals.isPropertyEncrypted(name)) {
+        	if (JiveGlobals.isXMLPropertyEncrypted(name)) {
         		Attribute encrypted = element.attribute(ENCRYPTED_ATTRIBUTE);
         		if (encrypted != null) {
             		value = JiveGlobals.getPropertyEncryptor().decrypt(value);
@@ -269,7 +269,7 @@ public class XMLProperties {
             value = prop.getTextTrim();
             if (!"".equals(value)) {
             	// check to see if the property is marked as encrypted
-            	if (JiveGlobals.isPropertyEncrypted(name)) {
+            	if (JiveGlobals.isXMLPropertyEncrypted(name)) {
             		Attribute encrypted = prop.attribute(ENCRYPTED_ATTRIBUTE);
             		if (encrypted != null) {
                 		value = JiveGlobals.getPropertyEncryptor().decrypt(value);
@@ -649,7 +649,7 @@ public class XMLProperties {
         else {
         	String propValue = StringEscapeUtils.escapeXml(value);
         	// check to see if the property is marked as encrypted
-        	if (JiveGlobals.isPropertyEncrypted(name)) {
+        	if (JiveGlobals.isXMLPropertyEncrypted(name)) {
         		propValue = JiveGlobals.getPropertyEncryptor().encrypt(value);
         		element.addAttribute(ENCRYPTED_ATTRIBUTE, "true");
         	}

src/web/server-properties.jsp

@@ -106,10 +106,9 @@
             errors.put("propValueLength","");
         }
         if (errors.size() == 0) {
-        	JiveGlobals.setPropertyEncrypted(propName, encrypt);
-            JiveGlobals.setProperty(propName, propValue);
+            JiveGlobals.setProperty(propName, propValue, encrypt);
             // Log the event
-            webManager.logEvent("set server property "+propName, propName+" = "+propValue);
+            webManager.logEvent("set server property "+propName, propName+" = " + (encrypt ? "********" : propValue));
             response.sendRedirect("server-properties.jsp?success=true");
             return;
         }