[JENKINS-73825] ldap allows insecure configurations

src/main/java/jenkins/security/plugins/ldap/LDAPConfiguration.java

@@ -36,6 +36,7 @@
 import hudson.util.Secret;
 import jenkins.model.Jenkins;
 import java.nio.charset.StandardCharsets;
+import jenkins.security.FIPS140;
 import org.apache.commons.lang.StringUtils;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
@@ -147,7 +148,15 @@
     private transient String id;
 
     @DataBoundConstructor
-    public LDAPConfiguration(@NonNull String server, String rootDN, boolean inhibitInferRootDN, String managerDN, Secret managerPasswordSecret) {
+    public LDAPConfiguration(@NonNull String server, String rootDN, boolean inhibitInferRootDN, String managerDN, Secret managerPasswordSecret){
+        if(FIPS140.useCompliantAlgorithms() && !validateServerUrlIsSecure(server)){
+            throw new IllegalArgumentException(Messages.LDAPConfiguration_InsecureServer(server));
+        }
+        String managerPassword = Secret.toString(managerPasswordSecret);
+        if(StringUtils.isNotBlank(managerPassword) && !"undefined".equals(managerPassword) &&
+                FIPS140.useCompliantAlgorithms() && StringUtils.length(managerPassword) < 14) {
+            throw new IllegalArgumentException(Messages.LDAPConfiguration_passwordTooShortFIPS());
+        }
         this.server = server.trim();
         this.managerDN = fixEmpty(managerDN);
         this.managerPasswordSecret = managerPasswordSecret;
@@ -390,6 +399,20 @@
         return getId().equals(id);
     }
 
+    /**
+     * Validates if the LDAP server URL is secure (uses ldaps).
+     * Returns FALSE if the server URL is not secure.
+     */
+    public static boolean validateServerUrlIsSecure(String server) {
+        String[] servers = server.split("\\s+");
+        for (String s : servers) {
+            if (!s.startsWith("ldaps://")) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     @Extension
     public static final class LDAPConfigurationDescriptor extends Descriptor<LDAPConfiguration> {
         //For jelly usage
@@ -407,7 +430,9 @@
         public FormValidation doCheckServer(@QueryParameter String value, @QueryParameter String managerDN, @QueryParameter Secret managerPasswordSecret,@QueryParameter String rootDN) {
             String server = value;
             String managerPassword = Secret.toString(managerPasswordSecret);
-
+            if(FIPS140.useCompliantAlgorithms() && !validateServerUrlIsSecure(server)){
+                return  FormValidation.error(Messages.LDAPConfiguration_InsecureServer(server));
+            }
             if(!Jenkins.get().hasPermission(Jenkins.ADMINISTER))
                 return FormValidation.ok();
 
@@ -461,6 +486,14 @@
             }
         }
 
+        @POST
+        public FormValidation doCheckManagerPasswordSecret(@QueryParameter String managerPasswordSecret) {
+            if(FIPS140.useCompliantAlgorithms() && StringUtils.length(managerPasswordSecret) < 14) {
+                return FormValidation.error(Messages.LDAPConfiguration_passwordTooShortFIPS());
+            }
+            return FormValidation.ok();
+        }
+
         private void forceClose(Context ctx){
             if(ctx==null){
                 return;

src/main/resources/jenkins/security/plugins/ldap/Messages.properties

@@ -66,3 +66,5 @@ UserDetails.Inactive=The user "{0}" is inactive until {1}.
 UserDetails.Expired=The user "{0}" is expired since {1}.
 UserDetails.CredentialsExpired=The user "{0}" has expired credentials.
 UserDetails.Locked=The user "{0}" is locked and must be unlocked by an administrator.
+LDAPConfiguration.InsecureServer=LDAP server URL is not secure: {0}.
+LDAPConfiguration.passwordTooShortFIPS=Password is too short (< 14 characters)

src/test/java/hudson/security/LDAPEmbeddedFIPSTest.java

@@ -0,0 +1,158 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2017 CloudBees,Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package hudson.security;
+
+import hudson.util.FormValidation;
+import io.jenkins.plugins.casc.misc.ConfiguredWithCode;
+import jenkins.model.Jenkins;
+import hudson.tasks.MailAddressResolver;
+import hudson.util.Secret;
+
+import java.security.cert.X509Certificate;
+
+import jenkins.model.IdStrategy;
+import jenkins.security.plugins.ldap.*;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.rules.ExpectedException;
+import org.jvnet.hudson.test.FlagRule;
+import org.springframework.security.ldap.userdetails.LdapUserDetails;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.RuleChain;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
+
+import javax.net.ssl.*;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+
+@LDAPTestConfiguration(ldapsProtocol = true)
+public class LDAPEmbeddedFIPSTest {
+    public LDAPRule ads = new LDAPRule();
+    public JenkinsRule r = new JenkinsRule();
+    @Rule
+    public RuleChain chain = RuleChain.outerRule(ads).around(r);
+    @Rule
+    public LoggerRule log = new LoggerRule();
+    @Rule
+    public ExpectedException thrown = ExpectedException.none();
+
+    @ClassRule
+    public static FlagRule<String> fipsSystemPropertyRule =
+            FlagRule.systemProperty("jenkins.security.FIPS140.COMPLIANCE", "true");
+
+
+    @BeforeClass
+    public static void setUp(){
+        disableSSLVerification();
+    }
+
+    //@Test
+    @LDAPSchema(ldif = "planetexpress", id = "planetexpress", dn = "dc=planetexpress,dc=com")
+    public void login() throws Exception {
+       // disableSSLVerification();
+        System.out.println(ads.getUrl());;
+        String secureUrl = "ldaps://insecure.example.com";
+        LDAPSecurityRealm realm =
+                new LDAPSecurityRealm(ads.getUrlTls(), "dc=planetexpress,dc=com", null, null, null, null, null,
+                        "uid=admin,ou=system", Secret.fromString("pass"), false, false, null,
+                        null, "cn", "mail", null, null);
+        r.jenkins.setSecurityRealm(realm);
+        r.configRoundtrip();
+        String content = r.createWebClient().login("fry", "fry").goTo("whoAmI").getBody().getTextContent();
+        assertThat(content, containsString("Philip J. Fry"));
+
+
+        LdapUserDetails zoidberg = (LdapUserDetails) r.jenkins.getSecurityRealm().loadUserByUsername2("zoidberg");
+        assertThat(zoidberg.getDn(), is("cn=John A. Zoidberg,ou=people,dc=planetexpress,dc=com"));
+
+        String leelaEmail = MailAddressResolver.resolve(r.jenkins.getUser("leela"));
+        assertThat(leelaEmail, is("[email protected]"));
+    }
+
+    public static void disableSSLVerification() {
+        try {
+            TrustManager[] trustAllCerts = new TrustManager[]{
+                    new X509TrustManager() {
+                        public X509Certificate[] getAcceptedIssuers() {
+                            return null;
+                        }
+                        public void checkClientTrusted(X509Certificate[] certs, String authType) {
+                        }
+                        public void checkServerTrusted(X509Certificate[] certs, String authType) {
+                        }
+                    }
+            };
+
+            SSLContext sc = SSLContext.getInstance("SSL");
+            sc.init(null, trustAllCerts, new java.security.SecureRandom());
+            HttpsURLConnection.setDefaultSSLSocketFactory(sc.getSocketFactory());
+
+            HostnameVerifier allHostsValid = (hostname, session) -> true;
+            HttpsURLConnection.setDefaultHostnameVerifier(allHostsValid);
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+    }
+
+    @Test
+    public void testPasswordCheck() {
+        //Test when password is null
+        LDAPConfiguration configuration = new LDAPConfiguration("ldaps://ldap.example.com", "dc=example,dc=com", true, null, null);
+        assertNotNull(configuration);
+
+        // Test with a short password
+        thrown.expect(IllegalArgumentException.class);
+        thrown.expectMessage("Password is too short");
+        configuration = new LDAPConfiguration("ldaps://ldap.example.com", "dc=example,dc=com", true, null, Secret.fromString("shortString"));
+
+        //Test with a strong password
+        configuration = new LDAPConfiguration("ldaps://ldap.example.com", "dc=example,dc=com", true, null, Secret.fromString("ThisIsVeryStrongPassword"));
+        assertNotNull(configuration);
+    }
+
+    @Test
+    public void testPasswordCheckOnCheckServer(){
+        // Test with a short password
+        FormValidation shortPasswordValidation = new LDAPConfiguration.LDAPConfigurationDescriptor().doCheckManagerPasswordSecret("short");
+        assertEquals(FormValidation.Kind.ERROR, shortPasswordValidation.kind);
+        assertThat(shortPasswordValidation.getMessage(), containsString("Password is too short"));
+
+        // Test with a strong password but server validation fails hence checking for 'Unknown host'
+        FormValidation strongPasswordValidation = new LDAPConfiguration.LDAPConfigurationDescriptor().doCheckManagerPasswordSecret("ThisIsVeryStrongPassword");
+        assertEquals(FormValidation.Kind.OK, strongPasswordValidation.kind);
+    }
+}

src/test/java/jenkins/security/plugins/ldap/CasCFIPSTest.java

@@ -0,0 +1,59 @@
+package jenkins.security.plugins.ldap;
+
+import hudson.security.LDAPSecurityRealm;
+import io.jenkins.plugins.casc.ConfiguratorException;
+import io.jenkins.plugins.casc.misc.ConfiguredWithCode;
+import io.jenkins.plugins.casc.misc.JenkinsConfiguredWithCodeRule;
+import jenkins.model.IdStrategy;
+import jenkins.model.Jenkins;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.contrib.java.lang.system.EnvironmentVariables;
+import org.junit.rules.RuleChain;
+import org.jvnet.hudson.test.FlagRule;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Based on {@link jenkins.security.plugins.ldap.CasCTest}
+ */
+public class CasCFIPSTest {
+
+    @ClassRule
+    public static FlagRule<String> fipsSystemPropertyRule =
+            FlagRule.systemProperty("jenkins.security.FIPS140.COMPLIANCE", "true");
+
+    @Rule
+    public RuleChain chain = RuleChain.outerRule(new EnvironmentVariables()
+                    .set("LDAP_PASSWORD", "SECRET_Password_123"))
+            .around(new JenkinsConfiguredWithCodeRule());
+
+    @Test
+    @ConfiguredWithCode("casc_ldap_secure.yml")
+    public void configure_ldap() {
+        final LDAPSecurityRealm securityRealm = (LDAPSecurityRealm) Jenkins.get().getSecurityRealm();
+        assertEquals(1, securityRealm.getConfigurations().size());
+        assertTrue(securityRealm.getUserIdStrategy() instanceof IdStrategy.CaseInsensitive);
+        assertTrue(securityRealm.getGroupIdStrategy() instanceof IdStrategy.CaseSensitive);
+        final LDAPConfiguration configuration = securityRealm.getConfigurations().get(0);
+        assertEquals("ldaps://ldap.acme.com", configuration.getServer());
+        assertEquals("SECRET_Password_123", configuration.getManagerPassword());
+        assertEquals("manager", configuration.getManagerDN());
+        assertEquals("(&(objectCategory=User)(sAMAccountName={0}))", configuration.getUserSearch());
+        assertEquals("(&(cn={0})(objectclass=group))", configuration.getGroupSearchFilter());
+        final FromGroupSearchLDAPGroupMembershipStrategy strategy = ((FromGroupSearchLDAPGroupMembershipStrategy) configuration.getGroupMembershipStrategy());
+        assertEquals("(&(objectClass=group)(|(cn=GROUP_1)(cn=GROUP_2)))", strategy.getFilter());
+    }
+
+    /**
+     * Expect an exception when LDAP url is not secure & FIPS is enabled
+     */
+    @Test
+    @ConfiguredWithCode(value = "casc.yml", expected = ConfiguratorException.class)
+    public void configure_ldap_for_invalid() {
+        // This test is expected to throw an ConfiguratorException while loading the configuration itself
+        // because the LDAP URL is not secure and FIPS is enabled. Hence, the code block is empty.
+    }
+}

src/test/java/jenkins/security/plugins/ldap/LDAPConfigurationTest.java

@@ -29,8 +29,8 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.collection.IsArrayWithSize.arrayWithSize;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.*;
+import static org.junit.Assert.assertFalse;
 
 /**
  * Tests {@link LDAPConfiguration}.
@@ -142,4 +142,39 @@ public void normalizeServerSameButDifferentOrder() {
         assertThat(n1.split("\\s+"), arrayWithSize(s1.split("\\s+").length));
     }
 
+    @Test
+    public void testValidateServerUrlIsSecure_SecureUrl() {
+        String secureUrl = "ldaps://secure.example.com";
+        assertTrue(LDAPConfiguration.validateServerUrlIsSecure(secureUrl));
+    }
+
+    @Test
+    public void testValidateServerUrlIsSecure_InsecureUrl() {
+        String insecureUrl = "ldap://insecure.example.com";
+        assertFalse(LDAPConfiguration.validateServerUrlIsSecure(insecureUrl));
+    }
+
+    @Test
+    public void testValidateServerUrlIsSecure_MixedUrls() {
+        String mixedUrls = "ldaps://secure.example.com ldap://insecure.example.com";
+        assertFalse(LDAPConfiguration.validateServerUrlIsSecure(mixedUrls));
+    }
+
+    @Test
+    public void testValidateServerUrlIsSecure_AllSecureUrls() {
+        String allSecureUrls = "ldaps://secure1.example.com ldaps://secure2.example.com";
+        assertTrue(LDAPConfiguration.validateServerUrlIsSecure(allSecureUrls));
+    }
+
+    @Test
+    public void testValidateServerUrlIsSecure_AllInsecureUrls() {
+        String allInsecureUrls = "ldap://insecure1.example.com ldap://insecure2.example.com";
+        assertFalse(LDAPConfiguration.validateServerUrlIsSecure(allInsecureUrls));
+    }
+
+    @Test
+    public void testValidateServerUrlIsSecure_plainUrl() {
+        String allInsecureUrls = "insecure1.example.com ";
+        assertFalse(LDAPConfiguration.validateServerUrlIsSecure(allInsecureUrls));
+    }
 }

src/test/resources/jenkins/security/plugins/ldap/casc_ldap_secure.yml

@@ -0,0 +1,18 @@
+jenkins:
+  securityRealm:
+    ldap:
+      configurations:
+        - server: ldaps://ldap.acme.com
+          rootDN: dc=acme,dc=fr
+          managerDN: "manager"
+          managerPasswordSecret: ${LDAP_PASSWORD}
+          userSearch: "(&(objectCategory=User)(sAMAccountName={0}))"
+          groupSearchFilter: "(&(cn={0})(objectclass=group))"
+          groupMembershipStrategy:
+            fromGroupSearch:
+              filter: "(&(objectClass=group)(|(cn=GROUP_1)(cn=GROUP_2)))"
+      cache:
+        size: 100
+        ttl: 10
+      userIdStrategy: CaseInsensitive
+      groupIdStrategy: CaseSensitive