Add support for 2-way SSL when connecting with LdapServer

trinodb · Apr 5, 2022 · 9525d8f · 9525d8f
1 parent f36b9e7
commit 9525d8f
Show file tree

Hide file tree

Showing 8 changed files with 92 additions and 23 deletions.
diff --git a/docs/src/main/sphinx/security/ldap.rst b/docs/src/main/sphinx/security/ldap.rst
@@ -93,9 +93,10 @@ Property                           Description
                                    ``ldap.allow-insecure=true``.
 ``ldap.allow-insecure``            Allow using an LDAP connection that is not secured with
                                    TLS.
-``ldap.ssl-trust-certificate``     The path to the PEM encoded trust certificate for the
-                                   LDAP server. This file should contain the LDAP
-                                   server's certificate or its certificate authority.
+``ldap.ssl.keystore.path``         Path to the PEM or JKS key store.
+``ldap.ssl.keystore.password``     Password for the key store.
+``ldap.ssl.truststore.path``       Path to the PEM or JKS trust store.
+``ldap.ssl.truststore.password``   Password for the trust store.
 ``ldap.user-bind-pattern``         This property can be used to specify the LDAP user
                                    bind string for password authentication. This property
                                    must contain the pattern ``${USER}``, which is

diff --git a/...uthenticators/src/main/java/io/trino/plugin/password/ldap/JdkLdapAuthenticatorClient.java b/...uthenticators/src/main/java/io/trino/plugin/password/ldap/JdkLdapAuthenticatorClient.java
@@ -79,8 +79,11 @@ public JdkLdapAuthenticatorClient(LdapConfig ldapConfig)
 
         this.basicEnvironment = builder.buildOrThrow();
 
-        this.sslContext = Optional.ofNullable(ldapConfig.getTrustCertificate())
-                .map(JdkLdapAuthenticatorClient::createSslContext);
+        this.sslContext = createSslContext(
+                ldapConfig.getKeystorePath(),
+                ldapConfig.getKeystorePassword(),
+                ldapConfig.getTrustStorePath(),
+                ldapConfig.getTruststorePassword());
     }
 
     @Override
@@ -157,10 +160,17 @@ private Map<String, String> createEnvironment(String userDistinguishedName, Stri
         return environment.buildOrThrow();
     }
 
-    private static SSLContext createSslContext(File trustCertificate)
+    private static Optional<SSLContext> createSslContext(
+            Optional<File> keyStorePath,
+            Optional<String> keyStorePassword,
+            Optional<File> trustStorePath,
+            Optional<String> trustStorePassword)
     {
+        if (keyStorePath.isEmpty() && trustStorePath.isEmpty()) {
+            return Optional.empty();
+        }
         try {
-            return SslUtils.createSSLContext(Optional.empty(), Optional.empty(), Optional.of(trustCertificate), Optional.empty());
+            return Optional.of(SslUtils.createSSLContext(keyStorePath, keyStorePassword, trustStorePath, trustStorePassword));
         }
         catch (GeneralSecurityException | IOException e) {
             throw new RuntimeException(e);

diff --git a/...trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapConfig.java b/...trino-password-authenticators/src/main/java/io/trino/plugin/password/ldap/LdapConfig.java
@@ -18,6 +18,7 @@
 import io.airlift.configuration.Config;
 import io.airlift.configuration.ConfigDescription;
 import io.airlift.configuration.ConfigSecuritySensitive;
+import io.airlift.configuration.DefunctConfig;
 import io.airlift.configuration.validation.FileExists;
 import io.airlift.units.Duration;
 
@@ -33,11 +34,15 @@
 import static com.google.common.base.Strings.nullToEmpty;
 import static java.util.Objects.requireNonNull;
 
+@DefunctConfig("ldap.ssl-trust-certificate")
 public class LdapConfig
 {
     private String ldapUrl;
     private boolean allowInsecure;
-    private File trustCertificate;
+    private File keystorePath;
+    private String keystorePassword;
+    private File trustStorePath;
+    private String truststorePassword;
     private List<String> userBindSearchPatterns = ImmutableList.of();
     private String groupAuthorizationSearchPattern;
     private String userBaseDistinguishedName;
@@ -82,17 +87,57 @@ public boolean isUrlConfigurationValid()
         return nullToEmpty(ldapUrl).startsWith("ldaps://") || allowInsecure;
     }
 
-    @FileExists
-    public File getTrustCertificate()
+    public Optional<@FileExists File> getKeystorePath()
     {
-        return trustCertificate;
+        return Optional.ofNullable(keystorePath);
     }
 
-    @Config("ldap.ssl-trust-certificate")
-    @ConfigDescription("Path to the PEM trust certificate for the LDAP server")
-    public LdapConfig setTrustCertificate(File trustCertificate)
+    @Config("ldap.ssl.keystore.path")
+    @ConfigDescription("Path to the PEM or JKS key store")
+    public LdapConfig setKeystorePath(File path)
     {
-        this.trustCertificate = trustCertificate;
+        this.keystorePath = path;
+        return this;
+    }
+
+    public Optional<String> getKeystorePassword()
+    {
+        return Optional.ofNullable(keystorePassword);
+    }
+
+    @Config("ldap.ssl.keystore.password")
+    @ConfigSecuritySensitive
+    @ConfigDescription("Password for the key store")
+    public LdapConfig setKeystorePassword(String password)
+    {
+        this.keystorePassword = password;
+        return this;
+    }
+
+    public Optional<@FileExists File> getTrustStorePath()
+    {
+        return Optional.ofNullable(trustStorePath);
+    }
+
+    @Config("ldap.ssl.truststore.path")
+    @ConfigDescription("Path to the PEM or JKS trust store")
+    public LdapConfig setTrustStorePath(File path)
+    {
+        this.trustStorePath = path;
+        return this;
+    }
+
+    public Optional<String> getTruststorePassword()
+    {
+        return Optional.ofNullable(truststorePassword);
+    }
+
+    @Config("ldap.ssl.truststore.password")
+    @ConfigSecuritySensitive
+    @ConfigDescription("Password for the trust store")
+    public LdapConfig setTruststorePassword(String password)
+    {
+        this.truststorePassword = password;
         return this;
     }
 

diff --git a/...o-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapConfig.java b/...o-password-authenticators/src/test/java/io/trino/plugin/password/ldap/TestLdapConfig.java
@@ -43,7 +43,10 @@ public void testDefault()
         assertRecordedDefaults(recordDefaults(LdapConfig.class)
                 .setLdapUrl(null)
                 .setAllowInsecure(false)
-                .setTrustCertificate(null)
+                .setKeystorePath(null)
+                .setKeystorePassword(null)
+                .setTrustStorePath(null)
+                .setTruststorePassword(null)
                 .setUserBindSearchPatterns(" : ")
                 .setUserBaseDistinguishedName(null)
                 .setGroupAuthorizationSearchPattern(null)
@@ -59,12 +62,16 @@ public void testDefault()
     public void testExplicitConfig()
             throws IOException
     {
-        Path trustCertificateFile = Files.createTempFile(null, null);
+        Path trustStoreFile = Files.createTempFile(null, null);
+        Path keyStoreFile = Files.createTempFile(null, null);
 
         Map<String, String> properties = ImmutableMap.<String, String>builder()
                 .put("ldap.url", "ldaps://localhost:636")
                 .put("ldap.allow-insecure", "true")
-                .put("ldap.ssl-trust-certificate", trustCertificateFile.toString())
+                .put("ldap.ssl.keystore.path", keyStoreFile.toString())
+                .put("ldap.ssl.keystore.password", "12345")
+                .put("ldap.ssl.truststore.path", trustStoreFile.toString())
+                .put("ldap.ssl.truststore.password", "54321")
                 .put("ldap.user-bind-pattern", "uid=${USER},ou=org,dc=test,dc=com:uid=${USER},ou=alt")
                 .put("ldap.user-base-dn", "dc=test,dc=com")
                 .put("ldap.group-auth-pattern", "&(objectClass=user)(memberOf=cn=group)(user=username)")
@@ -79,7 +86,10 @@ public void testExplicitConfig()
         LdapConfig expected = new LdapConfig()
                 .setLdapUrl("ldaps://localhost:636")
                 .setAllowInsecure(true)
-                .setTrustCertificate(trustCertificateFile.toFile())
+                .setKeystorePath(keyStoreFile.toFile())
+                .setKeystorePassword("12345")
+                .setTrustStorePath(trustStoreFile.toFile())
+                .setTruststorePassword("54321")
                 .setUserBindSearchPatterns(ImmutableList.of("uid=${USER},ou=org,dc=test,dc=com", "uid=${USER},ou=alt"))
                 .setUserBaseDistinguishedName("dc=test,dc=com")
                 .setGroupAuthorizationSearchPattern("&(objectClass=user)(memberOf=cn=group)(user=username)")

diff --git a/pom.xml b/pom.xml
@@ -67,7 +67,7 @@
         <dep.coral.version>2.0.55</dep.coral.version>
         <dep.confluent.version>5.5.2</dep.confluent.version>
 
-        <dep.docker.images.version>53</dep.docker.images.version>
+        <dep.docker.images.version>54</dep.docker.images.version>
 
         <!--
           America/Bahia_Banderas has:

diff --git a/...-product-tests/conf/environment/singlenode-ldap-bind-dn/password-authenticator.properties b/...-product-tests/conf/environment/singlenode-ldap-bind-dn/password-authenticator.properties
@@ -1,6 +1,7 @@
 password-authenticator.name=ldap
 ldap.url=ldaps://ldapserver:636
-ldap.ssl-trust-certificate=/etc/openldap/certs/openldap-certificate.pem
+ldap.ssl.keystore.path=/etc/openldap/certs/trino-coordinator-for-ldap.pem
+ldap.ssl.truststore.path=/etc/openldap/certs/openldap-certificate.pem
 ldap.user-base-dn=dc=trino,dc=testldap,dc=com
 ldap.bind-dn=cn=admin,dc=trino,dc=testldap,dc=com
 ldap.bind-password=admin

diff --git a/...roduct-tests/conf/environment/singlenode-ldap-referrals/password-authenticator.properties b/...roduct-tests/conf/environment/singlenode-ldap-referrals/password-authenticator.properties
@@ -1,6 +1,7 @@
 password-authenticator.name=ldap
 ldap.url=ldaps://ldapserver:636
-ldap.ssl-trust-certificate=/etc/openldap/certs/openldap-certificate.pem
+ldap.ssl.keystore.path=/etc/openldap/certs/trino-coordinator-for-ldap.pem
+ldap.ssl.truststore.path=/etc/openldap/certs/openldap-certificate.pem
 ldap.user-bind-pattern=uid=${USER},ou=America,dc=trino,dc=testldap,dc=com:uid=${USER},ou=Asia,dc=trino,dc=testldap,dc=com
 ldap.user-base-dn=ou=World,dc=trino,dc=testldap,dc=com
 ldap.group-auth-pattern=(&(objectClass=inetOrgPerson)(uid=${USER})(memberof=cn=DefaultGroup,ou=America,dc=trino,dc=testldap,dc=com))
diff --git a/...r/presto-product-tests/conf/environment/singlenode-ldap/password-authenticator.properties b/...r/presto-product-tests/conf/environment/singlenode-ldap/password-authenticator.properties
@@ -1,6 +1,7 @@
 password-authenticator.name=ldap
 ldap.url=ldaps://ldapserver:636
-ldap.ssl-trust-certificate=/etc/openldap/certs/openldap-certificate.pem
+ldap.ssl.keystore.path=/etc/openldap/certs/trino-coordinator-for-ldap.pem
+ldap.ssl.truststore.path=/etc/openldap/certs/openldap-certificate.pem
 ldap.user-bind-pattern=uid=${USER},ou=America,dc=trino,dc=testldap,dc=com:uid=${USER},ou=Asia,dc=trino,dc=testldap,dc=com
 ldap.user-base-dn=dc=trino,dc=testldap,dc=com
 ldap.group-auth-pattern=(&(objectClass=inetOrgPerson)(uid=${USER})(memberof=cn=DefaultGroup,ou=America,dc=trino,dc=testldap,dc=com))