Support LDAP without TLS

trinodb · Apr 27, 2020 · 9c08506 · 9c08506
1 parent 6d71b83
commit 9c08506
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 4 deletions.
diff --git a/...ord-authenticators/src/main/java/io/prestosql/plugin/password/ldap/LdapAuthenticator.java b/...ord-authenticators/src/main/java/io/prestosql/plugin/password/ldap/LdapAuthenticator.java
@@ -91,6 +91,10 @@ public LdapAuthenticator(LdapConfig ldapConfig)
         checkState(bindDistinguishedName.isPresent() || userBindSearchPattern.isPresent(),
                 "Either user bind search pattern or bind distinguished name must be provided");
 
+        if (ldapConfig.getLdapUrl().startsWith("ldap://")) {
+            log.warn("Passwords will be sent in the clear to the LDAP server. Please consider using SSL to connect.");
+        }
+
         Map<String, String> environment = ImmutableMap.<String, String>builder()
                 .put(INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory")
                 .put(PROVIDER_URL, ldapUrl)

diff --git a/...o-password-authenticators/src/main/java/io/prestosql/plugin/password/ldap/LdapConfig.java b/...o-password-authenticators/src/main/java/io/prestosql/plugin/password/ldap/LdapConfig.java
@@ -16,16 +16,23 @@
 import io.airlift.configuration.Config;
 import io.airlift.configuration.ConfigDescription;
 import io.airlift.configuration.ConfigSecuritySensitive;
+import io.airlift.log.Logger;
 import io.airlift.units.Duration;
 
+import javax.validation.constraints.AssertTrue;
 import javax.validation.constraints.NotNull;
 import javax.validation.constraints.Pattern;
 
 import java.util.concurrent.TimeUnit;
 
+import static com.google.common.base.Strings.nullToEmpty;
+
 public class LdapConfig
 {
+    private static final Logger log = Logger.get(LdapConfig.class);
+
     private String ldapUrl;
+    private boolean allowInsecure;
     private String userBindSearchPattern;
     private String groupAuthorizationSearchPattern;
     private String userBaseDistinguishedName;
@@ -35,7 +42,7 @@ public class LdapConfig
     private Duration ldapCacheTtl = new Duration(1, TimeUnit.HOURS);
 
     @NotNull
-    @Pattern(regexp = "^ldaps://.*", message = "LDAP without SSL/TLS unsupported. Expected ldaps://")
+    @Pattern(regexp = "^ldaps?://.*", message = "Invalid LDAP server URL. Expected ldap:// or ldaps://")
     public String getLdapUrl()
     {
         return ldapUrl;
@@ -49,6 +56,25 @@ public LdapConfig setLdapUrl(String url)
         return this;
     }
 
+    public boolean isAllowInsecure()
+    {
+        return allowInsecure;
+    }
+
+    @Config("ldap.allow-insecure")
+    @ConfigDescription("Allow insecure connection to the LDAP server")
+    public LdapConfig setAllowInsecure(boolean allowInsecure)
+    {
+        this.allowInsecure = allowInsecure;
+        return this;
+    }
+
+    @AssertTrue(message = "Connecting to the LDAP server without SSL enabled requires `ldap.allow-insecure=true`")
+    public boolean isUrlConfigurationValid()
+    {
+        return nullToEmpty(ldapUrl).startsWith("ldaps://") || allowInsecure;
+    }
+
     public String getUserBindSearchPattern()
     {
         return userBindSearchPattern;

diff --git a/...ssword-authenticators/src/test/java/io/prestosql/plugin/password/ldap/TestLdapConfig.java b/...ssword-authenticators/src/test/java/io/prestosql/plugin/password/ldap/TestLdapConfig.java
@@ -18,6 +18,7 @@
 import io.airlift.units.Duration;
 import org.testng.annotations.Test;
 
+import javax.validation.constraints.AssertTrue;
 import javax.validation.constraints.NotNull;
 import javax.validation.constraints.Pattern;
 
@@ -37,6 +38,7 @@ public void testDefault()
     {
         assertRecordedDefaults(recordDefaults(LdapConfig.class)
                 .setLdapUrl(null)
+                .setAllowInsecure(false)
                 .setUserBindSearchPattern(null)
                 .setUserBaseDistinguishedName(null)
                 .setGroupAuthorizationSearchPattern(null)
@@ -51,6 +53,7 @@ public void testExplicitConfig()
     {
         Map<String, String> properties = new ImmutableMap.Builder<String, String>()
                 .put("ldap.url", "ldaps://localhost:636")
+                .put("ldap.allow-insecure", "true")
                 .put("ldap.user-bind-pattern", "uid=${USER},ou=org,dc=test,dc=com")
                 .put("ldap.user-base-dn", "dc=test,dc=com")
                 .put("ldap.group-auth-pattern", "&(objectClass=user)(memberOf=cn=group)(user=username)")
@@ -62,6 +65,7 @@ public void testExplicitConfig()
 
         LdapConfig expected = new LdapConfig()
                 .setLdapUrl("ldaps://localhost:636")
+                .setAllowInsecure(true)
                 .setUserBindSearchPattern("uid=${USER},ou=org,dc=test,dc=com")
                 .setUserBaseDistinguishedName("dc=test,dc=com")
                 .setGroupAuthorizationSearchPattern("&(objectClass=user)(memberOf=cn=group)(user=username)")
@@ -82,9 +86,23 @@ public void testValidation()
                 .setUserBaseDistinguishedName("dc=test,dc=com")
                 .setGroupAuthorizationSearchPattern("&(objectClass=user)(memberOf=cn=group)(user=username)"));
 
-        assertFailsValidation(new LdapConfig().setLdapUrl("ldap://"), "ldapUrl", "LDAP without SSL/TLS unsupported. Expected ldaps://", Pattern.class);
-        assertFailsValidation(new LdapConfig().setLdapUrl("localhost"), "ldapUrl", "LDAP without SSL/TLS unsupported. Expected ldaps://", Pattern.class);
-        assertFailsValidation(new LdapConfig().setLdapUrl("ldaps:/localhost"), "ldapUrl", "LDAP without SSL/TLS unsupported. Expected ldaps://", Pattern.class);
+        assertValidates(new LdapConfig()
+                .setLdapUrl("ldap://localhost")
+                .setAllowInsecure(true)
+                .setUserBindSearchPattern("uid=${USER},ou=org,dc=test,dc=com")
+                .setUserBaseDistinguishedName("dc=test,dc=com")
+                .setGroupAuthorizationSearchPattern("&(objectClass=user)(memberOf=cn=group)(user=username)"));
+
+        assertFailsValidation(
+                new LdapConfig()
+                        .setLdapUrl("ldap://")
+                        .setAllowInsecure(false),
+                "urlConfigurationValid",
+                "Connecting to the LDAP server without SSL enabled requires `ldap.allow-insecure=true`",
+                AssertTrue.class);
+
+        assertFailsValidation(new LdapConfig().setLdapUrl("localhost"), "ldapUrl", "Invalid LDAP server URL. Expected ldap:// or ldaps://", Pattern.class);
+        assertFailsValidation(new LdapConfig().setLdapUrl("ldaps:/localhost"), "ldapUrl", "Invalid LDAP server URL. Expected ldap:// or ldaps://", Pattern.class);
 
         assertFailsValidation(new LdapConfig(), "ldapUrl", "may not be null", NotNull.class);
     }

diff --git a/...main/java/io/prestosql/tests/product/launcher/env/environment/SinglenodeLdapInsecure.java b/...main/java/io/prestosql/tests/product/launcher/env/environment/SinglenodeLdapInsecure.java
@@ -0,0 +1,54 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.prestosql.tests.product.launcher.env.environment;
+
+import com.google.common.collect.ImmutableList;
+import io.prestosql.tests.product.launcher.docker.DockerFiles;
+import io.prestosql.tests.product.launcher.env.Environment;
+import io.prestosql.tests.product.launcher.env.EnvironmentOptions;
+import io.prestosql.tests.product.launcher.env.common.Hadoop;
+import io.prestosql.tests.product.launcher.env.common.Standard;
+import io.prestosql.tests.product.launcher.env.common.TestsEnvironment;
+import io.prestosql.tests.product.launcher.testcontainers.PortBinder;
+
+import javax.inject.Inject;
+
+import static java.util.Objects.requireNonNull;
+
+@TestsEnvironment
+public class SinglenodeLdapInsecure
+        extends AbstractSinglenodeLdap
+{
+    private final PortBinder portBinder;
+
+    @Inject
+    public SinglenodeLdapInsecure(Standard standard, Hadoop hadoop, DockerFiles dockerFiles, PortBinder portBinder, EnvironmentOptions environmentOptions)
+    {
+        super(ImmutableList.of(standard, hadoop), dockerFiles, portBinder, environmentOptions);
+        this.portBinder = requireNonNull(portBinder, "portBinder is null");
+    }
+
+    @Override
+    protected void extendEnvironment(Environment.Builder builder)
+    {
+        super.extendEnvironment(builder);
+        builder.configureContainer("ldapserver", container -> portBinder.exposePort(container, 389));
+    }
+
+    @Override
+    protected String getPasswordAuthenticatorConfigPath()
+    {
+        return "conf/environment/singlenode-ldap-without-ssl/password-authenticator.properties";
+    }
+}
diff --git a/...duct-tests/conf/environment/singlenode-ldap-without-ssl/password-authenticator.properties b/...duct-tests/conf/environment/singlenode-ldap-without-ssl/password-authenticator.properties
@@ -0,0 +1,6 @@
+password-authenticator.name=ldap
+ldap.url=ldap://ldapserver:389
+ldap.allow-insecure=true
+ldap.user-bind-pattern=uid=${USER},ou=Asia,dc=presto,dc=testldap,dc=com
+ldap.user-base-dn=ou=Asia,dc=presto,dc=testldap,dc=com
+ldap.group-auth-pattern=(&(objectClass=inetOrgPerson)(uid=${USER})(memberof=cn=DefaultGroup,ou=America,dc=presto,dc=testldap,dc=com))
diff --git a/presto-product-tests/bin/product-tests-suite-6-non-generic.sh b/presto-product-tests/bin/product-tests-suite-6-non-generic.sh
@@ -18,6 +18,11 @@ presto-product-tests-launcher/bin/run-launcher test run \
     -- -g ldap \
     || suite_exit_code=1
 
+presto-product-tests-launcher/bin/run-launcher test run \
+    --environment singlenode-ldap-insecure \
+    -- -g ldap \
+    || suite_exit_code=1
+
 presto-product-tests-launcher/bin/run-launcher test run \
     --environment singlenode-ldap-referrals \
     -- -g ldap \