CVE-2022-23105

@@ -34,6 +34,7 @@
 import com4j.typelibs.activeDirectory.IADsOpenDSObject;
 import com4j.typelibs.activeDirectory.IADsUser;
 import com4j.typelibs.ado20.ClassFactory;
+import com4j.typelibs.ado20.Property;
 import com4j.typelibs.ado20._Command;
 import com4j.typelibs.ado20._Connection;
 import com4j.typelibs.ado20._Recordset;
@@ -55,11 +56,15 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Locale;
 import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import org.apache.commons.lang.StringUtils;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.springframework.dao.DataAccessException;
 import org.springframework.dao.DataAccessResourceFailureException;
 
@@ -72,6 +77,12 @@ public class ActiveDirectoryAuthenticationProvider extends AbstractActiveDirecto
     @SuppressFBWarnings("MS_SHOULD_BE_FINAL")
     private static /* non-final for Groovy */ boolean ALLOW_EMPTY_PASSWORD = Boolean.getBoolean(ActiveDirectoryAuthenticationProvider.class.getName() + ".ALLOW_EMPTY_PASSWORD");
 
+    @Restricted(NoExternalUse.class)
+    static final String ADSI_FLAGS_SYSTEM_PROPERTY_NAME = ActiveDirectoryAuthenticationProvider.class.getName() + ".ADSI_FLAGS_OVERRIDE";
+
+    @Restricted(NoExternalUse.class)
+    static final String ADSI_PASSWORDLESS_FLAGS_SYSTEM_PROPERTY_NAME = ActiveDirectoryAuthenticationProvider.class.getName() + ".ADSI_PASSWORDLESS_FLAGS_OVERRIDE";
+
     private final String defaultNamingContext;
     /**
      * ADO connection for searching Active Directory.
@@ -93,19 +104,58 @@ public class ActiveDirectoryAuthenticationProvider extends AbstractActiveDirecto
      */
     private final Cache<String, ActiveDirectoryGroupDetails> groupCache;
 
+    /** Flags used for ADSI when we have a username/password */
+    private final int ADSI_FLAGS;
+
+    /** Flags used for ADSI when we are using null userneam/password */
+    private final int ADSI_PASSWORDLESS_FLAGS;
+
+    @Deprecated
+    @Restricted(DoNotUse.class)
     public ActiveDirectoryAuthenticationProvider() throws IOException {
         this(null);
     }
 
     public ActiveDirectoryAuthenticationProvider(ActiveDirectorySecurityRealm realm) throws DataAccessException {
-        try {
-            IADs rootDSE = COM4J.getObject(IADs.class, "LDAP://RootDSE", null);
+        final Integer adsi_override_flags = Integer.getInteger(ADSI_FLAGS_SYSTEM_PROPERTY_NAME);
+        if (adsi_override_flags != null) {
+            LOGGER.log(Level.INFO, () -> String.format(Locale.ROOT, "ADSI_FLAGS_OVERRIDE set, use the following as the flags for ADSI: 0x%1$04X", adsi_override_flags));
+            LOGGER.log(Level.INFO, "See https://www.jenkins.io/redirect/plugin/active-directory/iads-ads_authentication_enum for full flag details.");
+            ADSI_FLAGS = adsi_override_flags.intValue();
+        } else {
+            if (realm == null) {
+                // backwards compatibility in case anyone is actually calling ActiveDirectoryAuthenticationProvider.
+                // just be secure by default.
+                ADSI_FLAGS = DEFAULT_TLS_FLAGS;
+            } else {
+                ADSI_FLAGS = realm.getRequireTLS() ? DEFAULT_TLS_FLAGS : DEFAULT_NON_TLS_FLAGS;
+            }
+        }
+        final Integer adsi_passwordless_override_flags = Integer.getInteger(ADSI_PASSWORDLESS_FLAGS_SYSTEM_PROPERTY_NAME);
+        if (adsi_passwordless_override_flags != null) {
+            LOGGER.log(Level.INFO, () -> String.format(Locale.ROOT, "ADSI_PASSWORDLESS_FLAGS_OVERRIDE set, use the following as the flags for passwordless ADSI: 0x%1$04X", adsi_passwordless_override_flags));
+            LOGGER.log(Level.INFO, "See https://www.jenkins.io/redirect/plugin/active-directory/iads-ads_authentication_enum for full flag details.");
+            ADSI_PASSWORDLESS_FLAGS = adsi_passwordless_override_flags.intValue();
+        } else {
+            // use ADS_SECURE_AUTHENTICATION to use the process' credentials.
+            ADSI_PASSWORDLESS_FLAGS = ADSI_FLAGS | ADS_SECURE_AUTHENTICATION;
+        }
 
+        try {
+            // do this in 2 stages so we can set the ADSI flags :)
+            // we add ADS_SECURE_AUTHENTICATION here as this is using username/password less auth
+            // and want the auth of the running process not an anonymous bind
+            // without this on server 2019 I observed that if the first user to attempt to login failed then no users could login at all :-o
+            IADsOpenDSObject dso = COM4J.getObject(IADsOpenDSObject.class, "LDAP:", null);
+            IADs rootDSE = dso.openDSObject("LDAP://RootDSE", null, null, ADSI_PASSWORDLESS_FLAGS).queryInterface(IADs.class);
             defaultNamingContext = (String)rootDSE.get("defaultNamingContext");
-            LOGGER.info("Active Directory domain is "+defaultNamingContext);
+            LOGGER.info("Active Directory domain is " + defaultNamingContext);
 
             con = ClassFactory.createConnection();
             con.provider("ADsDSOObject");
+            Property property = con.properties("ADSI Flag");
+            property.value(ADSI_PASSWORDLESS_FLAGS);
+
             con.open("Active Directory Provider",""/*default*/,""/*default*/,-1/*default*/);
 
             if (realm != null) {
@@ -177,8 +227,8 @@ protected UserDetails retrieveUser(final String username,final  UsernamePassword
                         IADsUser usr;
                         try {
                             usr = (authentication==null
-                                    ? dso.openDSObject(dnToLdapUrl(dn), null, null, ADS_READONLY_SERVER)
-                                    : dso.openDSObject(dnToLdapUrl(dn), dn, ((UserPassword)password).getPassword(), ADS_READONLY_SERVER))
+                                    ? dso.openDSObject(dnToLdapUrl(dn), null, null, ADSI_PASSWORDLESS_FLAGS)
+                                    : dso.openDSObject(dnToLdapUrl(dn), dn, ((UserPassword)password).getPassword(), ADSI_FLAGS))
                                     .queryInterface(IADsUser.class);
                         } catch (ComException e) {
                             // this is failing
@@ -275,16 +325,20 @@ private boolean isAccountDisabled(IADsUser usr) {
     }
 
     private String getDnOfUserOrGroup(String userOrGroupname) throws UsernameNotFoundException {
-		_Command cmd = ClassFactory.createCommand();
+        /* This would be easier with IDirectorySearch
+         * https://docs.microsoft.com/en-us/windows/win32/adsi/searching-with-idirectorysearch
+         * however that is not possible with com4j so we use the VB documented way
+         * https://docs.microsoft.com/en-us/windows/win32/ad/example-code-for-searching-for-users
+         */
+        _Command cmd = ClassFactory.createCommand();
         cmd.activeConnection(con);
-
-        cmd.commandText("<LDAP://"+defaultNamingContext+">;(sAMAccountName="+userOrGroupname+");distinguishedName;subTree");
+        cmd.commandText("<LDAP://" +defaultNamingContext+ ">;(sAMAccountName="+userOrGroupname+");distinguishedName;subTree");
         _Recordset rs = cmd.execute(null, Variant.getMissing(), -1/*default*/);
-        if(rs.eof())
-            throw new UsernameNotFoundException("No such user or group: "+userOrGroupname);
-
+        if(rs.eof()) {
+            throw new UsernameNotFoundException("No such user or group: " + userOrGroupname);
+        }
         String dn = rs.fields().item("distinguishedName").value().toString();
-		return dn;
+        return dn;
 	}
 
 	public GroupDetails loadGroupByGroupname(final String groupname) {
@@ -296,7 +350,7 @@ public GroupDetails loadGroupByGroupname(final String groupname) {
                     // First get the distinguishedName
                     String dn = getDnOfUserOrGroup(groupname);
                     IADsOpenDSObject dso = COM4J.getObject(IADsOpenDSObject.class, "LDAP:", null);
-                    IADsGroup group = dso.openDSObject(dnToLdapUrl(dn), null, null, ADS_READONLY_SERVER)
+                    IADsGroup group = dso.openDSObject(dnToLdapUrl(dn), null, null, ADSI_PASSWORDLESS_FLAGS)
                             .queryInterface(IADsGroup.class);
 
                     // If not a group will throw UserMayOrMayNotExistException
@@ -330,13 +384,44 @@ public GroupDetails loadGroupByGroupname(final String groupname) {
         }
     }
 
-    private static final Logger LOGGER = Logger.getLogger(ActiveDirectoryAuthenticationProvider.class.getName());
+	private static final Logger LOGGER = Logger.getLogger(ActiveDirectoryAuthenticationProvider.class.getName());
+
+    /*
+     * ADS flags from https://docs.microsoft.com/en-gb/windows/win32/api/iads/ne-iads-ads_authentication_enum
+     */
+
+    /**
+     * Requests secure authentication.
+     */
+    // only use this for anonymous binds (seems to break at least default server 2019 setups)
+    private static final int ADS_SECURE_AUTHENTICATION = 0x1;
+
+    /**
+     * The channel is encrypted using Secure Sockets Layer (SSL).
+     * Not supported in all deployments as it requires the Certificate Server be deployed.
+     * This is identical to {@code ADS_USE_ENCRYPTION}.
+     */
+    private static final int ADS_USE_SSL = 0x2;
 
     /**
      * Signify that we can connect to a read-only mirror.
-     *
-     * See http://msdn.microsoft.com/en-us/library/windows/desktop/aa772247(v=vs.85).aspx
      */
     private static final int ADS_READONLY_SERVER = 0x4;
 
+    /**
+     * Verifies data integrity.
+     */
+    private static final int ADS_USE_SIGNING = 0x40;
+
+    /**
+     * Encrypts data using Kerberos.
+     */
+    private static final int ADS_USE_SEALING= 0x80;
+
+    /** ADSI flags to use when not in TLS MODE */
+    private static final int DEFAULT_NON_TLS_FLAGS = ADS_READONLY_SERVER | ADS_USE_SIGNING | ADS_USE_SEALING;
+
+    /** ADSI flags to use when in TLS MODE */
+    private static final int DEFAULT_TLS_FLAGS = ADS_READONLY_SERVER | ADS_USE_SSL;
+
 }

@@ -294,7 +294,7 @@ public FormValidation doValidateTest(@QueryParameter(fixEmpty = true) String nam
                     // this check must be identical to that of ActiveDirectory.groovy
                     try {
                         // make sure we can connect via ADSI
-                        new ActiveDirectoryAuthenticationProvider();
+                        new ActiveDirectoryAuthenticationProvider(activeDirectorySecurityRealm);
                         return FormValidation.ok("Success");
                     } catch (Exception e) {
                         return FormValidation.error(e, "Failed to contact Active Directory");

@@ -25,6 +25,8 @@
 
 import com.sun.jndi.ldap.LdapCtxFactory;
 import com4j.typelibs.ado20.ClassFactory;
+
+import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.Extension;
 import hudson.Functions;
@@ -34,6 +36,7 @@
 import hudson.security.AuthorizationStrategy;
 import hudson.security.GroupDetails;
 import hudson.security.SecurityRealm;
+import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
 import hudson.util.Secret;
 import jenkins.model.Jenkins;
@@ -51,6 +54,8 @@
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.interceptor.RequirePOST;
+import org.kohsuke.stapler.verb.GET;
+import org.kohsuke.stapler.verb.POST;
 import org.springframework.dao.DataAccessException;
 
 import javax.naming.Context;
@@ -88,6 +93,10 @@
  * @author Kohsuke Kawaguchi
  */
 public class ActiveDirectorySecurityRealm extends AbstractPasswordBasedSecurityRealm {
+
+    @Restricted(NoExternalUse.class)
+    static final String LEGACY_FORCE_LDAPS_PROPERTY = ActiveDirectorySecurityRealm.class.getName()+".forceLdaps";
+
     /**
      * Represent the old Active Directory Domain
      *
@@ -159,9 +168,17 @@ public class ActiveDirectorySecurityRealm extends AbstractPasswordBasedSecurityR
     /**
      * If true enable startTls in case plain communication is used. In case the plugin
      * is configured to use TLS then this option will not have any impact.
+     * @see #getRequireTLS()
      */
     public Boolean startTls;
 
+    /**
+     * If true uses ensures that the ldap connection is encrypted with TLS (or ssl).
+     * Takes precedence over requireTLS when set to {@code Boolean.TRUE}
+     * is configured to use TLS then this option will not have any impact.
+     */
+    private Boolean requireTLS = Boolean.TRUE;
+
     private GroupLookupStrategy groupLookupStrategy;
 
     /**
@@ -236,11 +253,16 @@ public ActiveDirectorySecurityRealm(String domain, List<ActiveDirectoryDomain> d
         this(domain, domains, site, bindName, bindPassword, server, groupLookupStrategy, removeIrrelevantGroups, customDomain, cache, startTls, (ActiveDirectoryInternalUsersDatabase) null);
     }
 
+    @Deprecated
+    public ActiveDirectorySecurityRealm(String domain, List<ActiveDirectoryDomain> domains, String site, String bindName,
+                                        String bindPassword, String server, GroupLookupStrategy groupLookupStrategy, boolean removeIrrelevantGroups, Boolean customDomain, CacheConfiguration cache, Boolean startTls, ActiveDirectoryInternalUsersDatabase internalUsersDatabase) {
+        this(domain, domains, site, bindName,bindPassword, server, groupLookupStrategy, removeIrrelevantGroups, customDomain, cache, startTls, internalUsersDatabase, true);
+    }
 
     @DataBoundConstructor
     // as Java signature, this binding doesn't make sense, so please don't use this constructor
     public ActiveDirectorySecurityRealm(String domain, List<ActiveDirectoryDomain> domains, String site, String bindName,
-                                        String bindPassword, String server, GroupLookupStrategy groupLookupStrategy, boolean removeIrrelevantGroups, Boolean customDomain, CacheConfiguration cache, Boolean startTls, ActiveDirectoryInternalUsersDatabase internalUsersDatabase) {
+                                        String bindPassword, String server, GroupLookupStrategy groupLookupStrategy, boolean removeIrrelevantGroups, Boolean customDomain, CacheConfiguration cache, Boolean startTls, ActiveDirectoryInternalUsersDatabase internalUsersDatabase, boolean requireTLS) {
         if (customDomain!=null && !customDomain)
             domains = null;
         this.domain = fixEmpty(domain);
@@ -254,6 +276,7 @@ public ActiveDirectorySecurityRealm(String domain, List<ActiveDirectoryDomain> d
         this.cache = cache;
         this.startTls = startTls;
         this.internalUsersDatabase = internalUsersDatabase;
+        this.requireTLS = Boolean.valueOf(requireTLS);
     }
 
     @DataBoundSetter
@@ -284,6 +307,26 @@ public Boolean isStartTls() {
         return startTls;
     }
 
+    @Restricted(NoExternalUse.class)
+    @NonNull
+    /**
+     * Obtain the Boolean flag showing if the AD Connection shall enforce the use of TLS (plain text connections will not be allowed).
+     * Despite returning a {@code Boolean} this will never return {@code null}, it is a Boolean only to match the underlying {@code requireTLS} field, and if that is {@code null} the legacy {@code forceLdaps} property is used.
+     * @return {@code Boolean.TRUE} iff the connection to the server requires TLS.
+     */
+    public Boolean getRequireTLS() {
+        if (requireTLS != null) {
+            return requireTLS;
+        }
+        // legacy was forced by a system property prior to SECURITY-1389
+        return Boolean.getBoolean(LEGACY_FORCE_LDAPS_PROPERTY);
+    }
+
+    @Restricted(NoExternalUse.class)
+    public boolean isRequireTLSPersisted() {
+        return requireTLS != null;
+    }
+
     public Integer getSize() {
         return cache == null ? null : cache.getSize();
     }
@@ -374,7 +417,6 @@ public Object readResolve() throws ObjectStreamException {
         if (startTls == null) {
             this.startTls = true;
         }
-
         return this;
     }
 
@@ -500,6 +542,14 @@ public ListBoxModel doFillGroupLookupStrategyItems() {
             return model;
         }
 
+        public FormValidation doCheckRequireTLS() {
+            Jenkins.get().checkPermission(Jenkins.ADMINISTER);
+            if (System.getProperty(ActiveDirectoryAuthenticationProvider.ADSI_FLAGS_SYSTEM_PROPERTY_NAME) != null) {
+                return FormValidation.warning("This setting is overridden by the ADSI mode system property");
+            }
+            return FormValidation.ok();
+        }
+
         private boolean isTrustAllCertificatesEnabled(TlsConfiguration tlsConfiguration) {
             return (tlsConfiguration == null || TlsConfiguration.TRUST_ALL_CERTIFICATES.equals(tlsConfiguration));
         }
@@ -511,13 +561,18 @@ public DirContext bind(String principalName, String password, List<SocketInfo> l
             return bind(principalName, password, ldapServers, props, TlsConfiguration.TRUST_ALL_CERTIFICATES);
         }
 
+        @Deprecated
+        public DirContext bind(String principalName, String password, List<SocketInfo> ldapServers, Hashtable<String, String> props, TlsConfiguration tlsConfiguration) throws NamingException {
+            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.
          */
-        public DirContext bind(String principalName, String password, List<SocketInfo> ldapServers, Hashtable<String, String> props, TlsConfiguration tlsConfiguration) throws NamingException {
+        public DirContext bind(String principalName, String password, List<SocketInfo> ldapServers, Hashtable<String, String> props, TlsConfiguration tlsConfiguration, boolean requireTLS) 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.
@@ -534,7 +589,7 @@ public DirContext bind(String principalName, String password, List<SocketInfo> l
 
             newProps.put("java.naming.ldap.attributes.binary","tokenGroups objectSid");
 
-            if (FORCE_LDAPS && isTrustAllCertificatesEnabled(tlsConfiguration)) {
+            if (requireTLS && isTrustAllCertificatesEnabled(tlsConfiguration)) {
                 newProps.put("java.naming.ldap.factory.socket", TrustAllSocketFactory.class.getName());
             }
 
@@ -543,7 +598,7 @@ public DirContext bind(String principalName, String password, List<SocketInfo> l
 
             for (SocketInfo ldapServer : ldapServers) {
                 try {
-                    LdapContext context = bind(principalName, password, ldapServer, newProps, tlsConfiguration);
+                    LdapContext context = bind(principalName, password, ldapServer, newProps, tlsConfiguration, requireTLS);
                     LOGGER.fine("Bound to " + ldapServer);
                     return context;
                 } catch (javax.naming.AuthenticationException e) {
@@ -597,12 +652,12 @@ private void customizeLdapProperties(Hashtable<String, String> props) {
         @IgnoreJRERequirement
         @Deprecated
         private LdapContext bind(String principalName, String password, SocketInfo server, Hashtable<String, String> props) throws NamingException {
-            return bind(principalName, password, server, props, null);
+            return bind(principalName, password, server, props, null, isRequireTLS());
         }
 
         @IgnoreJRERequirement
-        private LdapContext bind(String principalName, String password, SocketInfo server, Hashtable<String, String> props, TlsConfiguration tlsConfiguration) throws NamingException {
-            String ldapUrl = (FORCE_LDAPS?"ldaps://":"ldap://") + server + '/';
+        private LdapContext bind(String principalName, String password, SocketInfo server, Hashtable<String, String> props, TlsConfiguration tlsConfiguration, boolean requireTLS) throws NamingException {
+            String ldapUrl = (requireTLS?"ldaps://":"ldap://") + server + '/';
             String oldName = Thread.currentThread().getName();
             Thread.currentThread().setName("Connecting to "+ldapUrl+" : "+oldName);
             LOGGER.fine("Connecting to " + ldapUrl);
@@ -621,7 +676,7 @@ private LdapContext bind(String principalName, String password, SocketInfo serve
                      isStartTls= activeDirectorySecurityRealm.isStartTls();
                 }
 
-                if (!FORCE_LDAPS && isStartTls) {
+                if (!requireTLS && isStartTls) {
                     // 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;
@@ -692,6 +747,14 @@ public List<SocketInfo> obtainLDAPServer(ActiveDirectoryDomain activeDirectoryDo
             return obtainLDAPServer(createDNSLookupContext(), activeDirectoryDomain.getName(), activeDirectoryDomain.getSite(), activeDirectoryDomain.getServers());
         }
 
+        /**
+         * @deprecated see obtainLDAPServer(DirContext, String, String, String, boolean)
+         */
+        @Deprecated
+        public List<SocketInfo> obtainLDAPServer(DirContext ictx, String domainName, String site, String preferredServers) throws NamingException {
+            return obtainLDAPServer(ictx, domainName, site, preferredServers, isRequireTLS());
+        }
+
         /**
          * Use DNS and obtains the LDAP servers that we should try.
          *
@@ -702,10 +765,11 @@ public List<SocketInfo> obtainLDAPServer(ActiveDirectoryDomain activeDirectoryDo
          *      an authentication attempt with every listed server, which can lock the user out!) This also
          *      puts this feature in alignment with {@link #DOMAIN_CONTROLLERS}, which seems to indicate that
          *      there are users who prefer this behaviour.
-         *
+         * 
+         * @param useTLS {@code true} if we should use ldaps.
          * @return A list with at least one item.
          */
-        public List<SocketInfo> obtainLDAPServer(DirContext ictx, String domainName, String site, String preferredServers) throws NamingException {
+        public List<SocketInfo> obtainLDAPServer(DirContext ictx, String domainName, String site, String preferredServers, boolean useTLS) throws NamingException {
             List<SocketInfo> result = new ArrayList<>();
             if (preferredServers==null || preferredServers.isEmpty())
                 preferredServers = DOMAIN_CONTROLLERS;
@@ -769,7 +833,7 @@ public int compareTo(PrioritizedSocketInfo that) {
                     if (hostName.endsWith("."))
                         hostName = hostName.substring(0, hostName.length()-1);
                     int port = Integer.parseInt(fields[2]);
-                    if (FORCE_LDAPS) {
+                    if (isRequireTLS()) {
                         // map to LDAPS ports. I don't think there's any SRV records specifically for LDAPS.
                         // I think Microsoft considers LDAP+TLS the way to go, or else there should have been
                         // separate SRV entries.
@@ -793,6 +857,16 @@ public int compareTo(PrioritizedSocketInfo that) {
             LOGGER.fine(ldapServer + " resolved to " + result);
             return result;
         }
+
+        @Deprecated // this is here purely to ease access to the variable from the descriptor.
+        private boolean isRequireTLS() {
+            boolean requireTLS = true; // secure by default
+            SecurityRealm securityRealm = Jenkins.get().getSecurityRealm();
+            if (securityRealm instanceof ActiveDirectorySecurityRealm) {
+                requireTLS = Boolean.TRUE.equals(((ActiveDirectorySecurityRealm)securityRealm).getRequireTLS());
+            }
+            return requireTLS;
+        }
     }
 
     @Override
@@ -843,17 +917,6 @@ protected UserDetails authenticate(String username, String password) throws Auth
     @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Diagnostic fields are left mutable so that groovy console can be used to dynamically turn/off probes.")
     public static String DOMAIN_CONTROLLERS = System.getProperty(ActiveDirectorySecurityRealm.class.getName()+".domainControllers");
 
-    /**
-     * Instead of LDAP+TLS upgrade, start right away with LDAPS.
-     * For the time being I'm trying not to expose this to users. I don't see why any AD shouldn't support
-     * TLS upgrade if it's got the certificate.
-     *
-     * One legitimate use case is when the domain controller is Windows 2000, which doesn't support TLS
-     * (according to http://support.microsoft.com/kb/321051).
-     */
-    @SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Diagnostic fields are left mutable so that groovy console can be used to dynamically turn/off probes.")
-    public static boolean FORCE_LDAPS = Boolean.getBoolean(ActiveDirectorySecurityRealm.class.getName()+".forceLdaps");
-
     /**
      * Store all the extra environment variable to be used on the LDAP Context
      */

@@ -0,0 +1,22 @@
+package hudson.plugins.active_directory;
+
+import hudson.Extension;
+import hudson.model.AdministrativeMonitor;
+import hudson.security.SecurityRealm;
+
+import jenkins.model.Jenkins;
+
+@Extension
+public class Security1389AdministrativeMonitor extends AdministrativeMonitor {
+
+    @Override
+    public boolean isActivated() {
+        SecurityRealm securityRealm = Jenkins.get().getSecurityRealm();
+        if (securityRealm instanceof ActiveDirectorySecurityRealm) {
+            ActiveDirectorySecurityRealm adRealm = (ActiveDirectorySecurityRealm) securityRealm;
+            return !adRealm.isRequireTLSPersisted();
+        }
+        return false;
+    }
+
+}

@@ -0,0 +1,24 @@
+package hudson.plugins.active_directory;
+
+import hudson.Extension;
+import hudson.model.AdministrativeMonitor;
+import hudson.security.SecurityRealm;
+
+import jenkins.model.Jenkins;
+
+@Extension
+public class Security1389AdministrativeMonitorLegacySysProp extends AdministrativeMonitor {
+
+    private final boolean SYSTEM_PROPERTY_SET = System.getProperty(ActiveDirectorySecurityRealm.LEGACY_FORCE_LDAPS_PROPERTY) != null;
+
+    @Override
+    public boolean isActivated() {
+        SecurityRealm securityRealm = Jenkins.get().getSecurityRealm();
+        if (securityRealm instanceof ActiveDirectorySecurityRealm) {
+            ActiveDirectorySecurityRealm adRealm = (ActiveDirectorySecurityRealm) securityRealm;
+            return SYSTEM_PROPERTY_SET && adRealm.isRequireTLSPersisted();
+        }
+        return false;
+    }
+
+}

@@ -37,6 +37,7 @@
           </f:optionalBlock>
         </a:blockWrapper>
         <st:include page="configCache.jelly" class="${descriptor.clazz}"/>
+        <st:include page="requireTLS.jelly" class="${descriptor.clazz}"/>
       </f:entry>
     </j:when>
     <j:otherwise>
@@ -74,6 +75,7 @@
       </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>

@@ -0,0 +1,9 @@
+<div>
+    <p>Requires the connection to Active Directory to be TLS encrypted.
+    </p><p>
+    If you are using non ADSI mode (you have specified a domain name in the configuration) then this will use LDAP over TLS (port 636 or port 3269) and will take precedence over <code>startTLS</code> which upgrades a connection to TLS in place.
+    </p><p>
+    For ADSI mode this changes ADSI from negotiating encryption for the *authentication* part only to using TLS encryption for the entire connection.
+    </p>
+    <P><strong>Note</strong>: in either operating mode enabling this option requires that your domain(s) have the <a rel="noopener noreferrer" target="_blank" href="https://www.jenkins.io/redirect/plugin/active-directory/certificate-services">Certificate Services</a> installed, or other <a rel="noopener noreferrer" target="_blank" href="https://www.jenkins.io/redirect/plugin/active-directory/3rd-party-cert-auth">manual setup</a> has been performed.
+</div>

@@ -1,5 +1,5 @@
 <div>
     This property allows you to enable/disable StartTLS. In case the Active Directory plugin
-    is set-up to use TLS, then StartTLS will not try to start.
-    StartTLS will only tries to start in case the communication is started on plain.
+    is set-up to require TLS, then StartTLS will not try to start.
+    StartTLS will only tries to start in case the communication is started without encryption.
 </div>
\ No newline at end of file

@@ -0,0 +1,7 @@
+<?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:entry field="requireTLS" title="${%Require TLS}">
+        <f:checkbox default="true" />
+    </f:entry>
+</j:jelly>

@@ -0,0 +1,10 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:f="/lib/form" xmlns:l="/lib/layout">
+
+  <div class="alert alert-warning">
+    <form method="post" action="${rootURL}/${it.url}/disable">
+      <f:submit value="${%Dismiss}" />
+    </form>
+    <p>${%blurb}</p>
+  </div>
+</j:jelly>

@@ -0,0 +1,3 @@
+blurb=The System Property <code>hudson.plugins.active_directory.ActiveDirectorySecurityRealm.forceLdaps</code> \
+      is configured, however this is no longer controlling any behavior in the Active Directory plug-in and can \
+      be removed.

@@ -0,0 +1,10 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:f="/lib/form" xmlns:l="/lib/layout">
+
+  <div class="alert alert-danger">
+    <form method="post" action="${rootURL}/${it.url}/disable">
+      <f:submit value="${%Dismiss}"/>
+    </form>
+    <p>${%blurb(rootURL + "/configureSecurity/")}</p>
+  </div>
+</j:jelly>

@@ -0,0 +1,2 @@
+blurb=The Active Directory <a href="{0}">configuration</a> has not been updated to \
+     specify a value for <tt>Require TLS</tt> since the plug-in was upgraded for SECURITY-1389.

@@ -55,7 +55,7 @@ public void customSingleADSetup(ActiveDirectoryDomain activeDirectoryDomain, Str
         List<ActiveDirectoryDomain> domains = new ArrayList<>(1);
         domains.add(activeDirectoryDomain);
 
-        ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, domains, site, bindName, bindPassword, null, groupLookupStrategy, removeIrrelevantGroups, customDomain, cache, startTls, internalUsersDatabase);
+        ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, domains, site, bindName, bindPassword, null, groupLookupStrategy, removeIrrelevantGroups, customDomain, cache, startTls, internalUsersDatabase, false);
         j.getInstance().setSecurityRealm(activeDirectorySecurityRealm);
         while(!FileUtils.readFileToString(d.getLogfile()).contains("custom (exit status 0; expected)")) {
             Thread.sleep(1000);

@@ -76,7 +76,7 @@ public void customSingleADSetup(ActiveDirectoryDomain activeDirectoryDomain, Str
         List<ActiveDirectoryDomain> domains = new ArrayList<>(1);
         domains.add(activeDirectoryDomain);
 
-        ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, domains, site, bindName, bindPassword, null, groupLookupStrategy, removeIrrelevantGroups, customDomain, cache, startTls, internalUsersDatabase);
+        ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, domains, site, bindName, bindPassword, null, groupLookupStrategy, removeIrrelevantGroups, customDomain, cache, startTls, internalUsersDatabase, false);
         j.getInstance().setSecurityRealm(activeDirectorySecurityRealm);
         while(!FileUtils.readFileToString(d.getLogfile()).contains("custom (exit status 0; expected)")) {
             Thread.sleep(1000);

@@ -98,7 +98,7 @@ public void dynamicSetUp() throws Exception {
         ActiveDirectoryDomain activeDirectoryDomain = new ActiveDirectoryDomain(AD_DOMAIN, dockerIp + ":" +  dockerPort , null, AD_MANAGER_DN, AD_MANAGER_DN_PASSWORD);
         List<ActiveDirectoryDomain> domains = new ArrayList<>(1);
         domains.add(activeDirectoryDomain);
-        ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, domains, null, null, null, null, GroupLookupStrategy.RECURSIVE, false, true, null, false, null, null);
+        ActiveDirectorySecurityRealm activeDirectorySecurityRealm = new ActiveDirectorySecurityRealm(null, domains, null, null, null, null, GroupLookupStrategy.RECURSIVE, false, true, null, false, null, false);
         j.getInstance().setSecurityRealm(activeDirectorySecurityRealm);
         while(!FileUtils.readFileToString(d.getLogfile()).contains("custom (exit status 0; expected)")) {
             Thread.sleep(1000);

@@ -0,0 +1,29 @@
+package hudson.plugins.active_directory;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.recipes.LocalData;
+import hudson.ExtensionList;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+
+public class Security1389AdministrativeMonitorTest {
+
+    @Rule
+    public JenkinsRule jr = new JenkinsRule();
+
+    @Test
+    @LocalData
+    public void testMonitorIsShownForExistingInstalls() throws Exception {
+        assertThat(jr.jenkins.getSecurityRealm().getClass(), is(ActiveDirectorySecurityRealm.class));
+        Security1389AdministrativeMonitor adminMontor = ExtensionList.lookupSingleton(Security1389AdministrativeMonitor.class);
+        assertTrue("Admin monitor should be activated", adminMontor.isActivated());
+        jr.submit(jr.createWebClient().goTo("configureSecurity").getFormByName("config"));
+        assertFalse("Admin monitor should be activated", adminMontor.isActivated());
+    }
+
+}

@@ -0,0 +1,41 @@
+<?xml version='1.1' encoding='UTF-8'?>
+<hudson>
+  <disabledAdministrativeMonitors/>
+  <version>2.235.5</version>
+  <numExecutors>0</numExecutors>
+  <mode>NORMAL</mode>
+  <useSecurity>true</useSecurity>
+  <authorizationStrategy class="hudson.security.AuthorizationStrategy$Unsecured"/>
+  <securityRealm class="hudson.plugins.active_directory.ActiveDirectorySecurityRealm">
+    <startTls>true</startTls>
+    <groupLookupStrategy>AUTO</groupLookupStrategy>
+    <removeIrrelevantGroups>false</removeIrrelevantGroups>
+  </securityRealm>
+  <disableRememberMe>false</disableRememberMe>
+  <projectNamingStrategy class="jenkins.model.ProjectNamingStrategy$DefaultProjectNamingStrategy"/>
+  <workspaceDir>${JENKINS_HOME}/workspace/${ITEM_FULL_NAME}</workspaceDir>
+  <buildsDir>${ITEM_ROOTDIR}/builds</buildsDir>
+  <markupFormatter class="hudson.markup.EscapedMarkupFormatter"/>
+  <jdks/>
+  <viewsTabBar class="hudson.views.DefaultViewsTabBar"/>
+  <myViewsTabBar class="hudson.views.DefaultMyViewsTabBar"/>
+  <clouds/>
+  <scmCheckoutRetryCount>0</scmCheckoutRetryCount>
+  <views>
+    <hudson.model.AllView>
+      <owner class="hudson" reference="../../.."/>
+      <name>all</name>
+      <filterExecutors>false</filterExecutors>
+      <filterQueue>false</filterQueue>
+      <properties class="hudson.model.View$PropertyList"/>
+    </hudson.model.AllView>
+  </views>
+  <primaryView>all</primaryView>
+  <slaveAgentPort>0</slaveAgentPort>
+  <label></label>
+  <crumbIssuer class="hudson.security.csrf.DefaultCrumbIssuer">
+    <excludeClientIPFromCrumb>false</excludeClientIPFromCrumb>
+  </crumbIssuer>
+  <nodeProperties/>
+  <globalNodeProperties/>
+</hudson>
\ No newline at end of file