From e91d5a99ee8d460152bfdae4607ec62f8bc80370 Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Wed, 10 May 2023 23:26:05 +0000 Subject: [PATCH 1/2] GUACAMOLE-926: Add configuration option for enabling self-signed SQL Server certs for local testing. --- .../SQLServerAuthenticationProviderModule.java | 4 ++++ .../sqlserver/conf/SQLServerEnvironment.java | 17 +++++++++++++++++ .../conf/SQLServerGuacamoleProperties.java | 13 +++++++++++++ 3 files changed, 34 insertions(+) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/SQLServerAuthenticationProviderModule.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/SQLServerAuthenticationProviderModule.java index 74d3c950f..5a3d4002b 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/SQLServerAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/SQLServerAuthenticationProviderModule.java @@ -76,6 +76,10 @@ public class SQLServerAuthenticationProviderModule implements Module { // Use UTF-8 in database driverProperties.setProperty("characterEncoding", "UTF-8"); + + // Trust unknown server certificates if configured to do so + if (environment.trustAllServerCertificates()) + driverProperties.setProperty("trustServerCertificate", "true"); // Retrieve instance name and set it String instance = environment.getSQLServerInstance(); diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java index 5aedec784..612fa16d8 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java @@ -295,4 +295,21 @@ public class SQLServerEnvironment extends JDBCEnvironment { true); } + /** + * Returns true if all server certificates should be trusted, including + * those signed by an unknown certificate authority, such as self-signed + * certificates, or false otherwise. + * + * @throws GuacamoleException + * If an error occurs while retrieving the property value, or if the + * value was not set, as this property is required. + */ + public boolean trustAllServerCertificates() throws GuacamoleException { + + // Do not trust unknown certificates unless explicitly enabled + return getProperty( + SQLServerGuacamoleProperties.SQLSERVER_TRUST_ALL_SERVER_CERTIFICATES, + false); + } + } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerGuacamoleProperties.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerGuacamoleProperties.java index 721b3345a..c4df81381 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerGuacamoleProperties.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerGuacamoleProperties.java @@ -245,4 +245,17 @@ public class SQLServerGuacamoleProperties { }; + /** + * Whether or not all server certificates should be trusted, including those + * signed by an unknown certificate authority, such as self-signed + * certificates. + */ + public static final BooleanGuacamoleProperty SQLSERVER_TRUST_ALL_SERVER_CERTIFICATES = + new BooleanGuacamoleProperty() { + + @Override + public String getName() { return "sqlserver-trust-all-server-certificates"; } + + }; + } From c4e6b046aeff38cec847a480bef2a47c61ee1b8b Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Thu, 11 May 2023 00:12:43 +0000 Subject: [PATCH 2/2] GUACAMOLE-926: Disable batch executor for SQL Server JDBC extension - it doesn't work. --- .../jdbc/JDBCAuthenticationProviderModule.java | 9 +++++---- .../guacamole/auth/jdbc/JDBCEnvironment.java | 18 ++++++++++++++++++ .../sqlserver/conf/SQLServerEnvironment.java | 15 +++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java index 7b735bdf4..1b361e561 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCAuthenticationProviderModule.java @@ -127,10 +127,11 @@ public class JDBCAuthenticationProviderModule extends MyBatisModule { // Transaction factory bindTransactionFactoryType(JdbcTransactionFactory.class); - // Set the JDBC Auth provider to use batch execution when possible - bindConfigurationSetting(configuration -> { - configuration.setDefaultExecutorType(ExecutorType.BATCH); - }); + // Set the JDBC Auth provider to use batch execution if enabled + if (environment.shouldUseBatchExecutor()) + bindConfigurationSetting(configuration -> { + configuration.setDefaultExecutorType(ExecutorType.BATCH); + }); // Add MyBatis mappers addMapperClass(ConnectionMapper.class); diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCEnvironment.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCEnvironment.java index f7a890e49..27f3d0563 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCEnvironment.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/JDBCEnvironment.java @@ -254,4 +254,22 @@ public abstract class JDBCEnvironment extends DelegatingEnvironment { */ public abstract boolean enforceAccessWindowsForActiveSessions() throws GuacamoleException; + /** + * Returns true if the JDBC batch executor should be used by default, false + * otherwise. The batch executor allows repeated updates to be batched + * together for improved performance. + * See https://mybatis.org/mybatis-3/java-api.html#sqlSessions + * + * @return + * true if the batch executor should be used by default, false otherwise. + */ + public boolean shouldUseBatchExecutor() { + + // Unless otherwise overwritten due to implementation-specific problems, + // all JDBC extensions should use the batch executor if possible to + // ensure the best performance for repetitive queries + return true; + + } + } diff --git a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java index 612fa16d8..3f3133743 100644 --- a/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java +++ b/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-sqlserver/src/main/java/org/apache/guacamole/auth/sqlserver/conf/SQLServerEnvironment.java @@ -295,6 +295,21 @@ public class SQLServerEnvironment extends JDBCEnvironment { true); } + @Override + public boolean shouldUseBatchExecutor() { + + // The SQL Server driver does not work when batch execution is enabled. + // Specifically, inserts fail with com.microsoft.sqlserver.jdbc.SQLServerException: + // The statement must be executed before any results can be obtained. + // See https://github.com/microsoft/mssql-jdbc/issues/358 for more. + logger.warn( + "JDBC batch executor is disabled for SQL Server Connections. " + + "Large batched updates may run slower." + ); + return false; + + } + /** * Returns true if all server certificates should be trusted, including * those signed by an unknown certificate authority, such as self-signed