SQL: the SSL default configuration shouldn't override the https protocol if used (#34635)

* The default SSL option shouldn't override the https protocol if specified. Fixes https://github.com/elastic/elasticsearch/issues/33817

x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationTests.java

@@ -73,15 +73,40 @@ public class JdbcConfigurationTests extends ESTestCase {
         assertThat(ci.debugOut(), is("jdbc.out"));
     }
 
-    public void testHttpWithSSLEnabled() throws Exception {
+    public void testHttpWithSSLEnabledFromProperty() throws Exception {
         JdbcConfiguration ci = ci("jdbc:es://test?ssl=true");
         assertThat(ci.baseUri().toString(), is("https://test:9200/"));
     }
+    
+    public void testHttpWithSSLEnabledFromPropertyAndDisabledFromProtocol() throws Exception {
+        JdbcConfiguration ci = ci("jdbc:es://http://test?ssl=true");
+        assertThat(ci.baseUri().toString(), is("https://test:9200/"));
+    }
+    
+    public void testHttpWithSSLEnabledFromProtocol() throws Exception {
+        JdbcConfiguration ci = ci("jdbc:es://https://test:9200");
+        assertThat(ci.baseUri().toString(), is("https://test:9200/"));
+    }
+    
+    public void testHttpWithSSLEnabledFromProtocolAndProperty() throws Exception {
+        JdbcConfiguration ci = ci("jdbc:es://https://test:9200?ssl=true");
+        assertThat(ci.baseUri().toString(), is("https://test:9200/"));
+    }
 
-    public void testHttpWithSSLDisabled() throws Exception {
+    public void testHttpWithSSLDisabledFromProperty() throws Exception {
         JdbcConfiguration ci = ci("jdbc:es://test?ssl=false");
         assertThat(ci.baseUri().toString(), is("http://test:9200/"));
     }
+    
+    public void testHttpWithSSLDisabledFromPropertyAndProtocol() throws Exception {
+        JdbcConfiguration ci = ci("jdbc:es://http://test?ssl=false");
+        assertThat(ci.baseUri().toString(), is("http://test:9200/"));
+    }
+    
+    public void testHttpWithSSLDisabledFromPropertyAndEnabledFromProtocol() throws Exception {
+        Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://https://test?ssl=false"));
+        assertEquals("Cannot enable SSL: HTTPS protocol being used in the URL and SSL disabled in properties", e.getMessage());
+    }
 
     public void testTimoutOverride() throws Exception {
         Properties properties  = new Properties();

x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/ConnectionConfiguration.java

@@ -98,7 +98,7 @@ public class ConnectionConfiguration {
         user = settings.getProperty(AUTH_USER);
         pass = settings.getProperty(AUTH_PASS);
 
-        sslConfig = new SslConfig(settings);
+        sslConfig = new SslConfig(settings, baseURI);
         proxyConfig = new ProxyConfig(settings);
 
         this.baseURI = normalizeSchema(baseURI, connectionString, sslConfig.isEnabled());
@@ -126,20 +126,9 @@ public class ConnectionConfiguration {
 
 
     private static URI normalizeSchema(URI uri, String connectionString, boolean isSSLEnabled)  {
-        // Make sure the protocol is correct
-        final String scheme;
-        if (isSSLEnabled) {
-            // It's ok to upgrade from http to https
-            scheme = "https";
-        } else {
-            // Silently downgrading from https to http can cause security issues
-            if ("https".equals(uri.getScheme())) {
-                throw new ClientException("SSL is disabled");
-            }
-            scheme = "http";
-        }
         try {
-            return new URI(scheme, null, uri.getHost(), uri.getPort(), uri.getPath(), uri.getQuery(), uri.getFragment());
+            return new URI(isSSLEnabled ? "https" : "http", null, uri.getHost(), uri.getPort(), uri.getPath(), uri.getQuery(),
+                    uri.getFragment());
         } catch (URISyntaxException ex) {
             throw new ClientException("Cannot parse process baseURI [" + connectionString + "] " + ex.getMessage());
         }

x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/SslConfig.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.client;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -62,8 +63,19 @@ public class SslConfig {
 
     private final SSLContext sslContext;
 
-    SslConfig(Properties settings) {
-        enabled = StringUtils.parseBoolean(settings.getProperty(SSL, SSL_DEFAULT));
+    SslConfig(Properties settings, URI baseURI) {
+        boolean isSchemaPresent = baseURI.getScheme() != null;
+        boolean isSSLPropertyPresent = settings.getProperty(SSL) != null;
+        boolean isHttpsScheme = "https".equals(baseURI.getScheme());
+        
+        if (!isSSLPropertyPresent && !isSchemaPresent) {
+            enabled = StringUtils.parseBoolean(SSL_DEFAULT);
+        } else {
+            if (isSSLPropertyPresent && isHttpsScheme && !StringUtils.parseBoolean(settings.getProperty(SSL))) {
+                throw new ClientException("Cannot enable SSL: HTTPS protocol being used in the URL and SSL disabled in properties");
+            }
+            enabled = isHttpsScheme || StringUtils.parseBoolean(settings.getProperty(SSL, SSL_DEFAULT));
+        }
         protocol = settings.getProperty(SSL_PROTOCOL, SSL_PROTOCOL_DEFAULT);
         keystoreLocation = settings.getProperty(SSL_KEYSTORE_LOCATION, SSL_KEYSTORE_LOCATION_DEFAULT);
         keystorePass = settings.getProperty(SSL_KEYSTORE_PASS, SSL_KEYSTORE_PASS_DEFAULT);