From 822fd1c9ec0a7072afb571862039982b732b924b Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Mon, 18 Jan 2016 13:50:18 +0100 Subject: [PATCH 1/4] OF-1049: Improve Certificate Management The admin console should existing-but-invalid configuration. Configuration should be modifiable through admin console. --- src/i18n/openfire_i18n_en.properties | 4 + .../keystore/CertificateStoreManager.java | 8 +- .../security-certificate-store-management.jsp | 150 +++++++++++++++--- 3 files changed, 136 insertions(+), 26 deletions(-) diff --git a/src/i18n/openfire_i18n_en.properties b/src/i18n/openfire_i18n_en.properties index 6123f3433f..3fb352ed47 100644 --- a/src/i18n/openfire_i18n_en.properties +++ b/src/i18n/openfire_i18n_en.properties @@ -2262,6 +2262,10 @@ ssl.certificates.store-management.component-stores.info=These stores are used to ssl.certificates.store-management.connection-manager-stores.title=Connection Manager Stores ssl.certificates.store-management.connection-manager-stores.info=These stores are used to establish connections with Openfire Connection Managers.ssl.certificates.store-management.socket-s2s-stores.title=Server Federation Stores ssl.certificates.store-management.manage=Manage Store Contents +ssl.certificates.store-management.file_label=File +ssl.certificates.store-management.password_label=Password +ssl.certificates.store-management.error.cannot-access=Configuration problem: Unable to access the store. +ssl.certificates.store-management.saved_successfully=Settings updated successfully. # Openfire Certificates Page diff --git a/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java b/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java index ece594efcf..3faaa7a492 100644 --- a/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java +++ b/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java @@ -95,7 +95,7 @@ public TrustStore getTrustStore( ConnectionType type ) return trustStores.get( configuration ); } - public void replaceIdentityStore( ConnectionType type, CertificateStoreConfiguration configuration ) throws CertificateStoreConfigException + public void replaceIdentityStore( ConnectionType type, CertificateStoreConfiguration configuration, boolean createIfAbsent ) throws CertificateStoreConfigException { if ( type == null) { @@ -114,7 +114,7 @@ public void replaceIdentityStore( ConnectionType type, CertificateStoreConfigura if ( !identityStores.containsKey( configuration ) ) { // This constructor can throw an exception. If it does, the state of the manager should not have already changed. - final IdentityStore store = new IdentityStore( configuration, true ); + final IdentityStore store = new IdentityStore( configuration, createIfAbsent ); identityStores.put( configuration, store ); } @@ -143,7 +143,7 @@ public void replaceIdentityStore( ConnectionType type, CertificateStoreConfigura JiveGlobals.setProperty( type.getPrefix() + "keypass", new String( configuration.getPassword() ) ); } - public void replaceTrustStore( ConnectionType type, CertificateStoreConfiguration configuration ) throws CertificateStoreConfigException + public void replaceTrustStore( ConnectionType type, CertificateStoreConfiguration configuration, boolean createIfAbsent ) throws CertificateStoreConfigException { if ( type == null) { @@ -162,7 +162,7 @@ public void replaceTrustStore( ConnectionType type, CertificateStoreConfiguratio if ( !trustStores.containsKey( configuration ) ) { // This constructor can throw an exception. If it does, the state of the manager should not have already changed. - final TrustStore store = new TrustStore( configuration, true ); + final TrustStore store = new TrustStore( configuration, createIfAbsent ); trustStores.put( configuration, store ); } diff --git a/src/web/security-certificate-store-management.jsp b/src/web/security-certificate-store-management.jsp index 1c3948a1ef..405ad411d5 100644 --- a/src/web/security-certificate-store-management.jsp +++ b/src/web/security-certificate-store-management.jsp @@ -3,6 +3,10 @@ <%@ page import="java.util.HashMap" %> <%@ page import="org.jivesoftware.openfire.spi.ConnectionType" %> <%@ page import="org.jivesoftware.openfire.XMPPServer" %> +<%@ page import="org.jivesoftware.openfire.keystore.CertificateStoreManager" %> +<%@ page import="org.jivesoftware.openfire.keystore.CertificateStoreConfiguration" %> +<%@ page import="java.io.File" %> +<%@ page import="org.jivesoftware.util.Log" %> <%@ taglib uri="admin" prefix="admin" %> <%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %> <%@ taglib uri="http://java.sun.com/jsp/jstl/fmt" prefix="fmt" %> @@ -12,10 +16,55 @@ <% webManager.init(request, response, session, application, out ); + final CertificateStoreManager certificateStoreManager = XMPPServer.getInstance().getCertificateStoreManager(); + final Map errors = new HashMap<>(); pageContext.setAttribute( "errors", errors ); pageContext.setAttribute( "connectionTypes", ConnectionType.values() ); - pageContext.setAttribute( "certificateStoreManager", XMPPServer.getInstance().getCertificateStoreManager() ); + pageContext.setAttribute( "certificateStoreManager", certificateStoreManager ); + + final boolean update = request.getParameter("update") != null; + if ( update ) { + ConnectionType connectionType = null; + try { + connectionType = ConnectionType.valueOf( request.getParameter( "connectionType" ) ); + } catch ( IllegalArgumentException ex ) { + Log.warn( ex ); + errors.put( "connectionType", ex.getMessage() ); + } + + final String locKey = request.getParameter( "loc-key" ); + final String pwdKey = request.getParameter( "pwd-key" ); + final String locTrust = request.getParameter( "loc-trust" ); + final String pwdTrust = request.getParameter( "pwd-trust" ); + + if ( locKey == null || locKey.isEmpty() ) { + errors.put( "locKey", "Identity Store location must be defined." ); + } + if ( pwdKey == null || pwdKey.isEmpty() ) { + errors.put( "pwdKey", "Identity Store password must be defined." ); + } + if ( locTrust == null || locTrust.isEmpty() ) { + errors.put( "locTrust", "Trust Store location must be defined." ); + } + if ( pwdTrust == null || pwdTrust.isEmpty() ) { + errors.put( "pwdTrust", "Trust Store password must be defined." ); + } + + if ( errors.isEmpty() ) { + try + { + final CertificateStoreConfiguration configKey = new CertificateStoreConfiguration( "jks", new File( locKey ), pwdKey.toCharArray() ); + final CertificateStoreConfiguration configTrust = new CertificateStoreConfiguration( "jks", new File( locTrust ), pwdTrust.toCharArray() ); + certificateStoreManager.replaceIdentityStore( connectionType, configKey, false ); + certificateStoreManager.replaceTrustStore( connectionType, configTrust, false ); + pageContext.setAttribute( "updated", true ); + } catch ( Exception ex ) { + Log.warn( ex ); + errors.put( "update", ex.getMessage() ); + } + } + } %> @@ -40,6 +89,13 @@ + + + + + + +

@@ -73,27 +129,77 @@ - -

- -

- - - - - - - - - - - - - - -
- - +
+ + + +

+ +

+ +

+ + + + + + + + + + + + + + + + + + + + + + + + + +
 
+ +
+

+ + + + + + + + + + + + + + + + + + + + + + + + + +
 
+ +
+ "> + + + +
From 03d1cce5a10153f362e24c5ad2469598e2a1bb97 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 19 Jan 2016 11:07:15 +0100 Subject: [PATCH 2/4] OF-1049: Admin console secure port Don't initialize the admin console TLS connector when no identity store is available (that's guaranteed to fail). Don't log admin console startup success when there's an error. --- .../openfire/container/AdminConsolePlugin.java | 10 +++++----- .../openfire/keystore/CertificateStoreManager.java | 6 ++++++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/java/org/jivesoftware/openfire/container/AdminConsolePlugin.java b/src/java/org/jivesoftware/openfire/container/AdminConsolePlugin.java index f8bbb427de..6ec1f547a7 100644 --- a/src/java/org/jivesoftware/openfire/container/AdminConsolePlugin.java +++ b/src/java/org/jivesoftware/openfire/container/AdminConsolePlugin.java @@ -142,7 +142,7 @@ public void startup() { sslEnabled = false; try { final IdentityStore identityStore = XMPPServer.getInstance().getCertificateStoreManager().getIdentityStore( ConnectionType.WEBADMIN ); - if (adminSecurePort > 0 ) + if (identityStore != null && adminSecurePort > 0 ) { if ( identityStore.getAllCertificates().isEmpty() ) { @@ -189,7 +189,7 @@ public void startup() { } catch ( Exception e ) { - Log.error( "An exception occured while trying to make available the admin console via HTTPS.", e ); + Log.error( "An exception occurred while trying to make available the admin console via HTTPS.", e ); } // Make sure that at least one connector was registered. @@ -206,13 +206,13 @@ public void startup() { try { adminServer.start(); + + // Log the ports that the admin server is listening on. + logAdminConsolePorts(); } catch (Exception e) { Log.error("Could not start admin console server", e); } - - // Log the ports that the admin server is listening on. - logAdminConsolePorts(); } /** diff --git a/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java b/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java index 3faaa7a492..e056c8cd58 100644 --- a/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java +++ b/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java @@ -86,12 +86,18 @@ public synchronized void destroy() public IdentityStore getIdentityStore( ConnectionType type ) { final CertificateStoreConfiguration configuration = typeToIdentityStore.get( type ); + if (configuration == null) { + return null; + } return identityStores.get( configuration ); } public TrustStore getTrustStore( ConnectionType type ) { final CertificateStoreConfiguration configuration = typeToTrustStore.get( type ); + if (configuration == null) { + return null; + } return trustStores.get( configuration ); } From 9b0d89101958e564f17c1a32df9ca66dc315be44 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 19 Jan 2016 11:13:12 +0100 Subject: [PATCH 3/4] OF-1049 Test identity store when initializing Check if the keys in the identity store can be accessed when the identity store is being created. --- .../openfire/keystore/IdentityStore.java | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/java/org/jivesoftware/openfire/keystore/IdentityStore.java b/src/java/org/jivesoftware/openfire/keystore/IdentityStore.java index 241bce9df9..154e9f363e 100644 --- a/src/java/org/jivesoftware/openfire/keystore/IdentityStore.java +++ b/src/java/org/jivesoftware/openfire/keystore/IdentityStore.java @@ -7,7 +7,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.net.ssl.KeyManager; import javax.net.ssl.KeyManagerFactory; import java.io.IOException; import java.security.*; @@ -40,26 +39,21 @@ public class IdentityStore extends CertificateStore { private static final Logger Log = LoggerFactory.getLogger( IdentityStore.class ); -// protected final KeyManagerFactory keyFactory; - public IdentityStore( CertificateStoreConfiguration configuration, boolean createIfAbsent ) throws CertificateStoreConfigException { super( configuration, createIfAbsent ); -// try -// { -// keyFactory = KeyManagerFactory.getInstance( KeyManagerFactory.getDefaultAlgorithm() ); -// keyFactory.init( store, configuration.getPassword() ); -// } -// catch ( UnrecoverableKeyException | NoSuchAlgorithmException | KeyStoreException ex ) -// { -// throw new CertificateStoreConfigException( "Unable to load store of type '" + configuration.getType() + "' from location '" + configuration.getFile() + "'", ex ); -// } - } -// public KeyManager[] getKeyManagers() -// { -// return keyFactory.getKeyManagers(); -// } + try + { + final KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance( KeyManagerFactory.getDefaultAlgorithm() ); + keyManagerFactory.init( this.getStore(), configuration.getPassword() ); + } + catch ( NoSuchAlgorithmException | UnrecoverableKeyException | KeyStoreException ex ) + { + throw new CertificateStoreConfigException( "Unable to initialize identity store (a common cause: the password for a key is different from the password of the entire store).", ex ); + } + + } /** * Creates a Certificate Signing Request based on the private key and certificate identified by the provided alias. From 128847a923a207df383e92c20bd2d0f3ffdeadb9 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 19 Jan 2016 11:14:11 +0100 Subject: [PATCH 4/4] OF-1049: Prevent partially initialized objects By reversing the order in which objects are added to the internal state, a failure during store construction won't put the manager in a half-initialized state. --- .../openfire/keystore/CertificateStoreManager.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java b/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java index e056c8cd58..fbc89cdce0 100644 --- a/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java +++ b/src/java/org/jivesoftware/openfire/keystore/CertificateStoreManager.java @@ -43,32 +43,34 @@ public synchronized void initialize( XMPPServer server ) { try { + Log.debug( "(identity store for connection type '{}') Initializing store...", type ); final CertificateStoreConfiguration identityStoreConfiguration = getIdentityStoreConfiguration( type ); - typeToIdentityStore.put( type, identityStoreConfiguration ); if ( !identityStores.containsKey( identityStoreConfiguration ) ) { final IdentityStore store = new IdentityStore( identityStoreConfiguration, false ); identityStores.put( identityStoreConfiguration, store ); } + typeToIdentityStore.put( type, identityStoreConfiguration ); } catch ( CertificateStoreConfigException | IOException e ) { - Log.warn( "Unable to instantiate identity store for type '" + type + "'", e ); + Log.warn( "(identity store for connection type '{}') Unable to instantiate store ", type, e ); } try { + Log.debug( "(trust store for connection type '{}') Initializing store...", type ); final CertificateStoreConfiguration trustStoreConfiguration = getTrustStoreConfiguration( type ); - typeToTrustStore.put( type, trustStoreConfiguration ); if ( !trustStores.containsKey( trustStoreConfiguration ) ) { final TrustStore store = new TrustStore( trustStoreConfiguration, false ); trustStores.put( trustStoreConfiguration, store ); } + typeToTrustStore.put( type, trustStoreConfiguration ); } catch ( CertificateStoreConfigException | IOException e ) { - Log.warn( "Unable to instantiate trust store for type '" + type + "'", e ); + Log.warn( "(trust store for connection type '{}') Unable to instantiate store ", type, e ); } } }