QPID-8667: [Broker-J] Database connection with client certificate aut…

…hentication exposes keystore / truststore passwords (#236)
apache · Jan 29, 2024 · cb51d8d · cb51d8d
1 parent c881886
commit cb51d8d
Show file tree

Hide file tree

Showing 24 changed files with 974 additions and 104 deletions.
diff --git a/...g-logback/src/main/java/org/apache/qpid/server/logging/logback/jdbc/JDBCBrokerLogger.java b/...g-logback/src/main/java/org/apache/qpid/server/logging/logback/jdbc/JDBCBrokerLogger.java
@@ -22,6 +22,8 @@
 import org.apache.qpid.server.model.BrokerLogger;
 import org.apache.qpid.server.model.ManagedAttribute;
 import org.apache.qpid.server.model.ManagedContextDefault;
+import org.apache.qpid.server.security.FileKeyStore;
+import org.apache.qpid.server.security.FileTrustStore;
 import org.apache.qpid.server.store.jdbc.DefaultConnectionProviderFactory;
 import org.apache.qpid.server.store.jdbc.JDBCSettings;
 
@@ -52,4 +54,32 @@ public interface JDBCBrokerLogger<X extends JDBCBrokerLogger<X>> extends BrokerL
     @Override
     @ManagedAttribute
     String getTableNamePrefix();
+
+    @Override
+    @ManagedAttribute(description = "Optional keystore holding the key for secure database connection")
+    FileKeyStore<?> getKeyStore();
+
+    @Override
+    @ManagedAttribute(description = "Name of the database vendor specific keystore path property, " +
+            "property value is taken from the keystore")
+    String getKeyStorePathPropertyName();
+
+    @Override
+    @ManagedAttribute(description = "Name of the database vendor specific keystore password property, " +
+            "property value is taken from the keystore")
+    String getKeyStorePasswordPropertyName();
+
+    @Override
+    @ManagedAttribute(description = "Optional truststore holding the certificate for secure database connection")
+    FileTrustStore<?> getTrustStore();
+
+    @Override
+    @ManagedAttribute(description = "Name of the database vendor specific truststore path property, " +
+            "property value is taken from the truststore")
+    String getTrustStorePathPropertyName();
+
+    @Override
+    @ManagedAttribute(description = "Name of the database vendor specific truststore password property, " +
+            "property value is taken from the truststore")
+    String getTrustStorePasswordPropertyName();
 }
diff --git a/...gback/src/main/java/org/apache/qpid/server/logging/logback/jdbc/JDBCBrokerLoggerImpl.java b/...gback/src/main/java/org/apache/qpid/server/logging/logback/jdbc/JDBCBrokerLoggerImpl.java
@@ -37,6 +37,8 @@
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
 import org.apache.qpid.server.model.SystemConfig;
+import org.apache.qpid.server.security.FileKeyStore;
+import org.apache.qpid.server.security.FileTrustStore;
 import org.apache.qpid.server.store.jdbc.JDBCSettings;
 
 @SuppressWarnings("unused")
@@ -66,6 +68,24 @@ public class JDBCBrokerLoggerImpl extends AbstractBrokerLogger<JDBCBrokerLoggerI
     @ManagedAttributeField(afterSet = "restartAppenderIfExists")
     private String _tableNamePrefix;
 
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private FileKeyStore<?> _keyStore;
+
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private String _keyStorePathPropertyName;
+
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private String _keyStorePasswordPropertyName;
+
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private FileTrustStore<?> _trustStore;
+
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private String _trustStorePathPropertyName;
+
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private String _trustStorePasswordPropertyName;
+
     @ManagedObjectFactoryConstructor
     protected JDBCBrokerLoggerImpl(final Map<String, Object> attributes, Broker<?> broker)
     {
@@ -103,6 +123,42 @@ public String getTableNamePrefix()
         return _tableNamePrefix;
     }
 
+    @Override
+    public FileKeyStore<?> getKeyStore()
+    {
+        return _keyStore;
+    }
+
+    @Override
+    public String getKeyStorePathPropertyName()
+    {
+        return _keyStorePathPropertyName;
+    }
+
+    @Override
+    public String getKeyStorePasswordPropertyName()
+    {
+        return _keyStorePasswordPropertyName;
+    }
+
+    @Override
+    public FileTrustStore<?> getTrustStore()
+    {
+        return _trustStore;
+    }
+
+    @Override
+    public String getTrustStorePathPropertyName()
+    {
+        return _trustStorePathPropertyName;
+    }
+
+    @Override
+    public String getTrustStorePasswordPropertyName()
+    {
+        return _trustStorePasswordPropertyName;
+    }
+
     @Override
     protected ListenableFuture<Void> onClose()
     {

diff --git a/...back/src/main/java/org/apache/qpid/server/logging/logback/jdbc/JDBCVirtualHostLogger.java b/...back/src/main/java/org/apache/qpid/server/logging/logback/jdbc/JDBCVirtualHostLogger.java
@@ -21,6 +21,8 @@
 
 import org.apache.qpid.server.model.ManagedAttribute;
 import org.apache.qpid.server.model.VirtualHostLogger;
+import org.apache.qpid.server.security.FileKeyStore;
+import org.apache.qpid.server.security.FileTrustStore;
 import org.apache.qpid.server.store.jdbc.DefaultConnectionProviderFactory;
 import org.apache.qpid.server.store.jdbc.JDBCSettings;
 
@@ -46,4 +48,32 @@ public interface JDBCVirtualHostLogger<X extends JDBCVirtualHostLogger<X>> exten
     @Override
     @ManagedAttribute
     String getTableNamePrefix();
+
+    @Override
+    @ManagedAttribute(description = "Optional keystore holding the key for secure database connection")
+    FileKeyStore<?> getKeyStore();
+
+    @Override
+    @ManagedAttribute(description = "Name of the database vendor specific keystore path property, " +
+            "property value is taken from the keystore")
+    String getKeyStorePathPropertyName();
+
+    @Override
+    @ManagedAttribute(description = "Name of the database vendor specific keystore password property, " +
+            "property value is taken from the keystore")
+    String getKeyStorePasswordPropertyName();
+
+    @Override
+    @ManagedAttribute(description = "Optional truststore holding the certificate for secure database connection")
+    FileTrustStore<?> getTrustStore();
+
+    @Override
+    @ManagedAttribute(description = "Name of the database vendor specific truststore path property, " +
+            "property value is taken from the truststore")
+    String getTrustStorePathPropertyName();
+
+    @Override
+    @ManagedAttribute(description = "Name of the database vendor specific truststore password property, " +
+            "property value is taken from the truststore")
+    String getTrustStorePasswordPropertyName();
 }
diff --git a/.../src/main/java/org/apache/qpid/server/logging/logback/jdbc/JDBCVirtualHostLoggerImpl.java b/.../src/main/java/org/apache/qpid/server/logging/logback/jdbc/JDBCVirtualHostLoggerImpl.java
@@ -33,6 +33,8 @@
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
 import org.apache.qpid.server.model.VirtualHost;
+import org.apache.qpid.server.security.FileKeyStore;
+import org.apache.qpid.server.security.FileTrustStore;
 import org.apache.qpid.server.store.jdbc.JDBCSettings;
 
 @SuppressWarnings("unused")
@@ -62,6 +64,24 @@ public class JDBCVirtualHostLoggerImpl extends AbstractVirtualHostLogger<JDBCVir
     @ManagedAttributeField(afterSet = "restartAppenderIfExists")
     private String _tableNamePrefix;
 
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private FileKeyStore<?> _keyStore;
+
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private String _keyStorePathPropertyName;
+
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private String _keyStorePasswordPropertyName;
+
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private FileTrustStore<?> _trustStore;
+
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private String _trustStorePathPropertyName;
+
+    @ManagedAttributeField(afterSet = "restartConnectionSourceIfExists")
+    private String _trustStorePasswordPropertyName;
+
     @ManagedObjectFactoryConstructor
     protected JDBCVirtualHostLoggerImpl(final Map<String, Object> attributes, VirtualHost<?> virtualHost)
     {
@@ -99,6 +119,42 @@ public String getTableNamePrefix()
         return _tableNamePrefix;
     }
 
+    @Override
+    public FileKeyStore<?> getKeyStore()
+    {
+        return _keyStore;
+    }
+
+    @Override
+    public String getKeyStorePathPropertyName()
+    {
+        return _keyStorePathPropertyName;
+    }
+
+    @Override
+    public String getKeyStorePasswordPropertyName()
+    {
+        return _keyStorePasswordPropertyName;
+    }
+
+    @Override
+    public FileTrustStore<?> getTrustStore()
+    {
+        return _trustStore;
+    }
+
+    @Override
+    public String getTrustStorePathPropertyName()
+    {
+        return _trustStorePathPropertyName;
+    }
+
+    @Override
+    public String getTrustStorePasswordPropertyName()
+    {
+        return _trustStorePasswordPropertyName;
+    }
+
     @Override
     protected void validateChange(ConfiguredObject<?> proxyForValidation, Set<String> changedAttributes)
     {

diff --git a/.../src/main/java/org/apache/qpid/server/store/jdbc/hikaricp/HikariCPConnectionProvider.java b/.../src/main/java/org/apache/qpid/server/store/jdbc/hikaricp/HikariCPConnectionProvider.java
@@ -33,12 +33,21 @@
 
 import com.zaxxer.hikari.HikariConfig;
 import com.zaxxer.hikari.HikariDataSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.model.KeyStore;
+import org.apache.qpid.server.model.TrustStore;
+import org.apache.qpid.server.security.FileKeyStore;
+import org.apache.qpid.server.security.FileTrustStore;
 import org.apache.qpid.server.store.jdbc.ConnectionProvider;
 
 public class HikariCPConnectionProvider implements ConnectionProvider
 {
+    private static final Logger LOGGER = LoggerFactory.getLogger(HikariCPConnectionProvider.class);
+    private static final String ADDING_DATASOURCE_PROPERTY = "Adding dataSource property '{}' with value '{}'";
+
     public static final int DEFAULT_MIN_IDLE = 20;
     public static final int DEFAULT_MAX_POOLSIZE = 40;
 
@@ -47,15 +56,28 @@ public class HikariCPConnectionProvider implements ConnectionProvider
     public HikariCPConnectionProvider(final String connectionUrl,
                                       final String username,
                                       final String password,
+                                      final KeyStore<?> keyStore,
+                                      final String keyStorePathPropertyName,
+                                      final String keyStorePasswordPropertyName,
+                                      final TrustStore<?> trustStore,
+                                      final String trustStorePathPropertyName,
+                                      final String trustStorePasswordPropertyName,
                                       final Map<String, String> providerAttributes)
     {
-        final HikariConfig config = createHikariCPConfig(connectionUrl, username, password, providerAttributes);
+        final HikariConfig config = createHikariCPConfig(connectionUrl, username, password, keyStore, keyStorePathPropertyName,
+                keyStorePasswordPropertyName, trustStore, trustStorePathPropertyName, trustStorePasswordPropertyName, providerAttributes);
         _dataSource = new HikariDataSource(config);
     }
 
     static HikariConfig createHikariCPConfig(final String connectionUrl,
                                              final String username,
                                              final String password,
+                                             final KeyStore<?> keyStore,
+                                             final String keyStorePathPropertyName,
+                                             final String keyStorePasswordPropertyName,
+                                             final TrustStore<?> trustStore,
+                                             final String trustStorePathPropertyName,
+                                             final String trustStorePasswordPropertyName,
                                              final Map<String, String> providerAttributes)
     {
         final Map<String, String> attributes = new HashMap<>(providerAttributes);
@@ -77,8 +99,41 @@ static HikariConfig createHikariCPConfig(final String connectionUrl,
             if (username != null)
             {
                 config.setUsername(username);
+            }
+            if (password != null)
+            {
                 config.setPassword(password);
             }
+            if (keyStore instanceof FileKeyStore)
+            {
+                if (keyStorePathPropertyName != null)
+                {
+                    final String path = ((FileKeyStore) keyStore).getPath();
+                    LOGGER.debug(ADDING_DATASOURCE_PROPERTY, keyStorePathPropertyName, path);
+                    config.addDataSourceProperty(keyStorePathPropertyName, path);
+                }
+                if (keyStorePasswordPropertyName != null)
+                {
+                    final String pwd = ((FileKeyStore) keyStore).getPassword() == null ? "null" : "******";
+                    LOGGER.debug(ADDING_DATASOURCE_PROPERTY, keyStorePasswordPropertyName, pwd);
+                    config.addDataSourceProperty(keyStorePasswordPropertyName, ((FileKeyStore) keyStore).getPassword());
+                }
+            }
+            if (trustStore instanceof FileTrustStore)
+            {
+                if (trustStorePathPropertyName != null)
+                {
+                    final String path = ((FileTrustStore) trustStore).getPath();
+                    LOGGER.debug(ADDING_DATASOURCE_PROPERTY, trustStorePathPropertyName, path);
+                    config.addDataSourceProperty(trustStorePathPropertyName, ((FileTrustStore) trustStore).getPath());
+                }
+                if (trustStorePasswordPropertyName != null)
+                {
+                    final String pwd = ((FileTrustStore) trustStore).getPassword() == null ? "null" : "******";
+                    LOGGER.debug(ADDING_DATASOURCE_PROPERTY, trustStorePasswordPropertyName, pwd);
+                    config.addDataSourceProperty(trustStorePasswordPropertyName, ((FileTrustStore) trustStore).getPassword());
+                }
+            }
             return config;
         }
         catch (Exception e)

diff --git a/...in/java/org/apache/qpid/server/store/jdbc/hikaricp/HikariCPConnectionProviderFactory.java b/...in/java/org/apache/qpid/server/store/jdbc/hikaricp/HikariCPConnectionProviderFactory.java
@@ -29,6 +29,8 @@
 
 import com.zaxxer.hikari.HikariConfig;
 
+import org.apache.qpid.server.model.KeyStore;
+import org.apache.qpid.server.model.TrustStore;
 import org.apache.qpid.server.plugin.PluggableService;
 import org.apache.qpid.server.store.jdbc.ConnectionProvider;
 import org.apache.qpid.server.store.jdbc.JDBCConnectionProviderFactory;
@@ -64,9 +66,20 @@ public String getType()
     }
 
     @Override
-    public ConnectionProvider getConnectionProvider(String connectionUrl, String username, String password, Map<String, String> providerAttributes)
+    public ConnectionProvider getConnectionProvider(
+            final String connectionUrl,
+            final String username,
+            final String password,
+            final KeyStore<?> keyStore,
+            final String keyStorePathPropertyName,
+            final String keyStorePasswordPropertyName,
+            final TrustStore<?> trustStore,
+            final String trustStorePathPropertyName,
+            final String trustStorePasswordPropertyName,
+            final Map<String, String> providerAttributes)
     {
-        return new HikariCPConnectionProvider(connectionUrl, username, password, providerAttributes);
+        return new HikariCPConnectionProvider(connectionUrl, username, password, keyStore, keyStorePathPropertyName,
+                keyStorePasswordPropertyName, trustStore, trustStorePathPropertyName, trustStorePasswordPropertyName, providerAttributes);
     }
 
     @Override