[SECURITY-3059]

jenkinsci · Jul 4, 2023 · 549dde6 · 549dde6
1 parent 1f99c08
commit 549dde6
Show file tree

Hide file tree

Showing 12 changed files with 82 additions and 45 deletions.
diff --git a/pom.xml b/pom.xml
@@ -34,7 +34,7 @@
   </developers>
 
   <properties>
-    <revision>2.31</revision>
+    <revision>2.30.1</revision>
     <changelist>-SNAPSHOT</changelist>
     <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
     <jenkins.version>2.332.4</jenkins.version>

diff --git a/src/main/java/hudson/plugins/active_directory/ActiveDirectoryDomain.java b/src/main/java/hudson/plugins/active_directory/ActiveDirectoryDomain.java
@@ -268,14 +268,15 @@ public ListBoxModel doFillTlsConfigurationItems() {
 
         @RequirePOST
         public FormValidation doValidateTest(@QueryParameter(fixEmpty = true) String name, @QueryParameter(fixEmpty = true) String servers, @QueryParameter(fixEmpty = true) String site, @QueryParameter(fixEmpty = true) String bindName,
-                                             @QueryParameter(fixEmpty = true) String bindPassword, @QueryParameter(fixEmpty = true) TlsConfiguration tlsConfiguration, @QueryParameter(fixEmpty = true) boolean requireTLS) throws IOException, ServletException, NamingException {
+                                             @QueryParameter(fixEmpty = true) String bindPassword, @QueryParameter(fixEmpty = true) TlsConfiguration tlsConfiguration, @QueryParameter GroupLookupStrategy groupLookupStrategy,
+                                             @QueryParameter(fixEmpty = false) boolean removeIrrelevantGroups, @QueryParameter(fixEmpty = true) boolean startTls, @QueryParameter(fixEmpty = true) boolean requireTLS) throws NamingException {
             Jenkins.get().checkPermission(Jenkins.ADMINISTER);
             ActiveDirectoryDomain domain = new ActiveDirectoryDomain(name, servers, site, bindName, bindPassword, tlsConfiguration);
             List<ActiveDirectoryDomain> domains = new ArrayList<>(1);
             domains.add(domain);
 
-            ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, domains, site, bindName,
-                    bindPassword, null, GroupLookupStrategy.AUTO, false, true, null, false, (ActiveDirectoryInternalUsersDatabase) null, requireTLS);
+            ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(name, domains, site, bindName,
+                    bindPassword, null, groupLookupStrategy, removeIrrelevantGroups, true, null, startTls, null, requireTLS);
 
             ClassLoader ccl = Thread.currentThread().getContextClassLoader();
             Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
@@ -325,7 +326,7 @@ public FormValidation doValidateTest(@QueryParameter(fixEmpty = true) String nam
                     // Make sure the bind actually works
                     try {
                         Hashtable<String, String> props = new Hashtable<>(0);
-                        DirContext context = activeDirectorySecurityRealm.getDescriptor().bind(bindName, Secret.toString(password), obtainerServers, props, tlsConfiguration, requireTLS);
+                        DirContext context = activeDirectorySecurityRealm.getDescriptor().bind(bindName, Secret.toString(password), obtainerServers, props, tlsConfiguration, requireTLS, startTls);
                         try {
                             // Actually do a search to make sure the credential is valid
                             Attributes userAttributes = new LDAPSearchBuilder(context, toDC(name)).subTreeScope().searchOne("(objectClass=user)");

diff --git a/src/main/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealm.java b/src/main/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealm.java
@@ -567,13 +567,18 @@ public DirContext bind(String principalName, String password, List<SocketInfo> l
             return bind(principalName, password, ldapServers, props, tlsConfiguration, isRequireTLS());
         }
 
-        /**
-         * Binds to the server using the specified username/password.
-         * <p>
-         * In a real deployment, often there are servers that don't respond or
-         * otherwise broken, so try all the servers.
-         */
+        @Deprecated
         public DirContext bind(String principalName, String password, List<SocketInfo> ldapServers, Hashtable<String, String> props, TlsConfiguration tlsConfiguration, boolean requireTLS) throws NamingException {
+            return bind(principalName, password, ldapServers, props, tlsConfiguration, requireTLS, isStartTLS());
+        }
+
+            /**
+             * Binds to the server using the specified username/password.
+             * <p>
+             * In a real deployment, often there are servers that don't respond or
+             * otherwise broken, so try all the servers.
+             */
+        public DirContext bind(String principalName, String password, List<SocketInfo> ldapServers, Hashtable<String, String> props, TlsConfiguration tlsConfiguration, boolean requireTLS, boolean startTls) throws NamingException {
             // in a AD forest, it'd be mighty nice to be able to login as "joe"
             // as opposed to "joe@europe",
             // but the bind operation doesn't appear to allow me to do so.
@@ -599,7 +604,7 @@ public DirContext bind(String principalName, String password, List<SocketInfo> l
 
             for (SocketInfo ldapServer : ldapServers) {
                 try {
-                    LdapContext context = bind(principalName, password, ldapServer, newProps, tlsConfiguration, requireTLS);
+                    LdapContext context = bind(principalName, password, ldapServer, newProps, tlsConfiguration, requireTLS, startTls);
                     LOGGER.fine("Bound to " + ldapServer);
                     return context;
                 } catch (javax.naming.AuthenticationException e) {
@@ -656,8 +661,15 @@ private LdapContext bind(String principalName, String password, SocketInfo serve
             return bind(principalName, password, server, props, null, isRequireTLS());
         }
 
+        @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD", justification = "Deprecated method.It will removed at some point")
         @IgnoreJRERequirement
+        @Deprecated
         private LdapContext bind(String principalName, String password, SocketInfo server, Hashtable<String, String> props, TlsConfiguration tlsConfiguration, boolean requireTLS) throws NamingException {
+            return bind(principalName, password, server, props, tlsConfiguration, requireTLS, isStartTLS());
+        }
+
+        @IgnoreJRERequirement
+        private LdapContext bind(String principalName, String password, SocketInfo server, Hashtable<String, String> props, TlsConfiguration tlsConfiguration, boolean requireTLS, boolean startTLS) throws NamingException {
             String ldapUrl = (requireTLS?"ldaps://":"ldap://") + server + '/';
             String oldName = Thread.currentThread().getName();
             Thread.currentThread().setName("Connecting to "+ldapUrl+" : "+oldName);
@@ -670,14 +682,7 @@ private LdapContext bind(String principalName, String password, SocketInfo serve
 
                 LdapContext context = new InitialLdapContext(props, null);
 
-                boolean isStartTls = true;
-                SecurityRealm securityRealm = Jenkins.getActiveInstance().getSecurityRealm();
-                if (securityRealm instanceof ActiveDirectorySecurityRealm) {
-                    ActiveDirectorySecurityRealm activeDirectorySecurityRealm = (ActiveDirectorySecurityRealm) securityRealm;
-                     isStartTls= activeDirectorySecurityRealm.isStartTls();
-                }
-
-                if (!requireTLS && isStartTls) {
+                if (!requireTLS && startTLS) {
                     // try to upgrade to TLS if we can, but failing to do so isn't fatal
                     // see http://download.oracle.com/javase/jndi/tutorial/ldap/ext/starttls.html
                     StartTlsResponse rsp = null;
@@ -858,6 +863,16 @@ private boolean isRequireTLS() {
             }
             return requireTLS;
         }
+
+        @Deprecated // this is here purely to ease access to the variable from the descriptor.
+        private boolean isStartTLS() {
+            boolean startTLS = true; // secure by default
+            SecurityRealm securityRealm = Jenkins.get().getSecurityRealm();
+            if (securityRealm instanceof ActiveDirectorySecurityRealm) {
+                startTLS = Boolean.TRUE.equals(((ActiveDirectorySecurityRealm)securityRealm).isStartTls());
+            }
+            return startTLS;
+        }
     }
 
     @Override

diff --git a/src/main/java/hudson/plugins/active_directory/TlsConfiguration.java b/src/main/java/hudson/plugins/active_directory/TlsConfiguration.java
@@ -6,7 +6,7 @@
  * Classification of all possible TLS configurations
  *
  */
-enum TlsConfiguration {
+public enum TlsConfiguration {
     TRUST_ALL_CERTIFICATES  (Messages._TlsConfiguration_TrustAllCertificates()),
     JDK_TRUSTSTORE          (Messages._TlsConfiguration_JdkTrustStore())
     ;

diff --git a/src/main/resources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/config.jelly b/src/main/resources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/config.jelly
@@ -42,6 +42,9 @@
     </j:when>
     <j:otherwise>
       <!-- code path for ActiveDirectoryUnixAuthenticationProvider: running on Unix -->
+      <st:include page="configAdvanced.jelly" class="${descriptor.clazz}"/>
+      <st:include page="configCache.jelly" class="${descriptor.clazz}"/>
+      <st:include page="requireTLS.jelly" class="${descriptor.clazz}"/>
       <f:entry field="domains" title="${%Domains}">
         <f:repeatable field="domains" add="${%Add Domain}">
           <a:blockWrapper>
@@ -65,17 +68,14 @@
               <f:select />
             </f:entry>
             <f:nested>
-              <f:validateButton with="name,servers,site,bindName,bindPassword,tlsConfiguration" title="${%Test Domain}" method="validateTest"/>
+              <f:validateButton with="name,servers,site,bindName,bindPassword,tlsConfiguration,groupLookupStrategy,removeIrrelevantGroups,startTls,requireTLS" title="${%Test Domain}" method="validateTest"/>
             </f:nested>
           </a:blockWrapper>
           <div align="right">
             <input type="button" value="Delete Domain" class="repeatable-delete" style="margin-left: 1em;" />
           </div>
         </f:repeatable>
       </f:entry>
-      <st:include page="configAdvanced.jelly" class="${descriptor.clazz}"/>
-      <st:include page="configCache.jelly" class="${descriptor.clazz}"/>
-      <st:include page="requireTLS.jelly" class="${descriptor.clazz}"/>
     </j:otherwise>
   </j:choose>
 </j:jelly>
diff --git a/...sources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/configAdvanced.jelly b/...sources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/configAdvanced.jelly
@@ -1,12 +1,12 @@
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
   <f:advanced>
-    <f:entry field="startTls" title="${%Enable StartTls}">
-      <f:checkbox name="startTls" default="true" />
-    </f:entry>
     <f:entry field="groupLookupStrategy" title="${%Group Membership Lookup Strategy}">
       <f:select default="TOKENGROUPS"/>
     </f:entry>
+    <f:entry field="startTls" title="${%Enable StartTls}">
+      <f:checkbox name="startTls" default="true" />
+    </f:entry>
     <f:optionalBlock field="internalUsersDatabase" title="${%Use Jenkins Internal Database}" checked="${instance.internalUsersDatabase != null}">
       <f:entry field="jenkinsInternalUser" title="${%Jenkins Internal Database User}">
         <f:textbox />

diff --git a/...resources/hudson/plugins/active_directory/Security1389AdministrativeMonitor/message.jelly b/...resources/hudson/plugins/active_directory/Security1389AdministrativeMonitor/message.jelly
@@ -3,7 +3,7 @@
 
   <div class="alert alert-danger">
     <form method="post" action="${rootURL}/${it.url}/disable">
-      <f:submit primary="false" clazz="jenkins-!-destructive-color" value="${%Dismiss}"/>
+      <f:submit value="${%Dismiss}"/>
     </form>
     <p>${%blurb(rootURL + "/configureSecurity/")}</p>
   </div>

diff --git a/...son/plugins/active_directory/Security1389AdministrativeMonitorLegacySysProp/message.jelly b/...son/plugins/active_directory/Security1389AdministrativeMonitorLegacySysProp/message.jelly
@@ -3,7 +3,7 @@
 
   <div class="alert alert-warning">
     <form method="post" action="${rootURL}/${it.url}/disable">
-      <f:submit primary="false" clazz="jenkins-!-destructive-color" value="${%Dismiss}"/>
+      <f:submit value="${%Dismiss}" />
     </form>
     <p>${%blurb}</p>
   </div>

diff --git a/src/test/java/hudson/plugins/active_directory/WindowsAdsiModeUserCacheDisabledIT.java b/src/test/java/hudson/plugins/active_directory/WindowsAdsiModeUserCacheDisabledIT.java
@@ -38,14 +38,14 @@ public static void setUp() {
     public void dynamicCacheEnableSetUp() throws Exception {
         CacheConfiguration cacheConfiguration = new CacheConfiguration(500,30);
         ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, null, null, null,
-                null, null, null, false, null, cacheConfiguration, null, (ActiveDirectoryInternalUsersDatabase) null);
+                null, null, null, false, null, cacheConfiguration, null, (ActiveDirectoryInternalUsersDatabase) null, false);
         j.jenkins.setSecurityRealm(activeDirectorySecurityRealm);
     }
 
 
     public void dynamicCacheDisabledSetUp() throws Exception {
         ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, null, null, null,
-                null, null, null, false, null, null, null, (ActiveDirectoryInternalUsersDatabase) null);
+                null, null, null, false, null, null, null, (ActiveDirectoryInternalUsersDatabase) null, false);
         j.jenkins.setSecurityRealm(activeDirectorySecurityRealm);
     }
 

diff --git a/src/test/java/hudson/plugins/active_directory/WindowsAdsiModeUserCacheEnabledIT.java b/src/test/java/hudson/plugins/active_directory/WindowsAdsiModeUserCacheEnabledIT.java
@@ -58,14 +58,14 @@ public static void disableHealthMetrics() {
     public void dynamicCacheEnableSetUp() throws Exception {
         CacheConfiguration cacheConfiguration = new CacheConfiguration(500,30);
         ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, null, null, null,
-                null, null, null, false, null, cacheConfiguration, null, (ActiveDirectoryInternalUsersDatabase) null);
+                null, null, null, false, null, cacheConfiguration, null, (ActiveDirectoryInternalUsersDatabase) null, false);
         j.jenkins.setSecurityRealm(activeDirectorySecurityRealm);
     }
 
 
     public void dynamicCacheDisabledSetUp() throws Exception {
         ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, null, null, null,
-                null, null, null, false, null, null, null, (ActiveDirectoryInternalUsersDatabase) null);
+                null, null, null, false, null, null, null, (ActiveDirectoryInternalUsersDatabase) null, false);
         j.jenkins.setSecurityRealm(activeDirectorySecurityRealm);
     }
 

diff --git a/src/test/java/hudson/plugins/active_directory/docker/ActiveDirectoryGenericContainer.java b/src/test/java/hudson/plugins/active_directory/docker/ActiveDirectoryGenericContainer.java
@@ -34,6 +34,7 @@ public ActiveDirectoryGenericContainer() {
      */
     public ActiveDirectoryGenericContainer<SELF> withStaticPorts() {
         addFixedExposedPort(3268, 3268); // global catalog
+        addFixedExposedPort(3269, 3269); // global catalog over tls
         addFixedExposedPort(53, 53, InternetProtocol.TCP); // DNS over TCP
         addFixedExposedPort(53, 53, InternetProtocol.UDP); // DNS over UDP
         return this;