Skip to content

Commit

Permalink
[SECURITY-251] Have the possibility of not accepting all the certific…
Browse files Browse the repository at this point in the history
…ates by default
  • Loading branch information
fbelzunc committed Nov 30, 2016
1 parent 5fc2a72 commit 063c282
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import hudson.Extension;
import hudson.Functions;
import hudson.model.AbstractDescribableImpl;
import hudson.model.AdministrativeMonitor;
import hudson.model.Descriptor;
import hudson.model.Hudson;
import hudson.security.AbstractPasswordBasedSecurityRealm;
Expand Down Expand Up @@ -189,6 +190,18 @@ public class ActiveDirectorySecurityRealm extends AbstractPasswordBasedSecurityR
*/
protected List<EnvironmentProperty> environmentProperties;

/**
* Selects the SSL strategy to follow on the TLS connections
*
* <p>
* Even if we are not using any of the TLS ports (3269/636) the plugin will try to establish a TLS channel
* using startTLS. Because of this, we need to be able to specify the SSL strategy on the plugin
*
* <p>
* For the moment there are two possible values: trustAllCertificates and trustStore.
*/
protected String tlsConfiguration;

public transient String testDomain;

public transient String testDomainControllers;
Expand All @@ -210,13 +223,13 @@ public ActiveDirectorySecurityRealm(String domain, String site, String bindName,

public ActiveDirectorySecurityRealm(String domain, String site, String bindName,
String bindPassword, String server, GroupLookupStrategy groupLookupStrategy, boolean removeIrrelevantGroups, CacheConfiguration cache) {
this(domain, Lists.newArrayList(new ActiveDirectoryDomain(domain, server)), site, bindName, bindPassword, server, groupLookupStrategy, removeIrrelevantGroups, domain!=null, cache);
this(domain, Lists.newArrayList(new ActiveDirectoryDomain(domain, server)), site, bindName, bindPassword, server, groupLookupStrategy, removeIrrelevantGroups, domain!=null, cache, "trustAllCertificates");
}

@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) {
String bindPassword, String server, GroupLookupStrategy groupLookupStrategy, boolean removeIrrelevantGroups, Boolean customDomain, CacheConfiguration cache, String tlsConfiguration) {
if (customDomain!=null && !customDomain)
domains = null;
this.domain = fixEmpty(domain);
Expand All @@ -228,6 +241,9 @@ public ActiveDirectorySecurityRealm(String domain, List<ActiveDirectoryDomain> d
this.groupLookupStrategy = groupLookupStrategy;
this.removeIrrelevantGroups = removeIrrelevantGroups;
this.cache = cache;
this.tlsConfiguration = tlsConfiguration;
// On descriptor change we need to check if the secure TLS configuration is done
enableTlsConfigAdministrativeMonitor();
}

@DataBoundSetter
Expand Down Expand Up @@ -262,6 +278,12 @@ public GroupLookupStrategy getGroupLookupStrategy() {
return groupLookupStrategy;
}

// for jelly use only
@Restricted(NoExternalUse.class)
public String getTlsConfiguration() {
return tlsConfiguration;
}

public SecurityComponents createSecurityComponents() {
BeanBuilder builder = new BeanBuilder(getClass().getClassLoader());
Binding binding = new Binding();
Expand Down Expand Up @@ -338,6 +360,10 @@ public Object readResolve() throws ObjectStreamException {
activeDirectoryDomain.site = site;
}
}

// On startup we need to check if the TLS is correctly configured
enableTlsConfigAdministrativeMonitor();

return this;
}

Expand Down Expand Up @@ -397,6 +423,18 @@ public void doAuthTest(StaplerRequest req, StaplerResponse rsp, @QueryParameter
req.getView(this, "test.jelly").forward(req, rsp);
}

/**
* Enable the TLS AdministrativeMonitor in case TLS is still not configured
* or is configured as trustAllCertificates
*/
public void enableTlsConfigAdministrativeMonitor() {
if (tlsConfiguration == null || tlsConfiguration.equals("trustAllCertificates")) {
NOTICE.enableAdministrativeMonitor(true);
} else {
NOTICE.enableAdministrativeMonitor(false);
}
}

@Extension
public static final class DescriptorImpl extends Descriptor<SecurityRealm> {
public String getDisplayName() {
Expand Down Expand Up @@ -462,15 +500,32 @@ public ListBoxModel doFillGroupLookupStrategyItems() {
return model;
}

public ListBoxModel doFillTlsConfigurationItems() {
ListBoxModel listBoxModel = new ListBoxModel();
listBoxModel.add("TrustAllCertificates (Unsecure)", "trustAllCertificates");
listBoxModel.add("Use JDK trustStore", "trustStore");

return listBoxModel;
}

private boolean isTrustAllCertificatesEnabled(String tlsConfiguration) {
return (tlsConfiguration == null || !tlsConfiguration.equals("trustAllCertificates")) ? false : true;
}

private static boolean WARNED = false;

@Deprecated
public DirContext bind(String principalName, String password, List<SocketInfo> ldapServers, Hashtable<String, String> props) {
return bind(principalName, password, ldapServers, props, null);
}

/**
* 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) {
public DirContext bind(String principalName, String password, List<SocketInfo> ldapServers, Hashtable<String, String> props, String tlsConfiguration) {
// 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.
Expand All @@ -486,7 +541,9 @@ public DirContext bind(String principalName, String password, List<SocketInfo> l
}

newProps.put("java.naming.ldap.attributes.binary","tokenGroups objectSid");
newProps.put("java.naming.ldap.factory.socket",TrustAllSocketFactory.class.getName());
if (isTrustAllCertificatesEnabled(tlsConfiguration)) {
newProps.put("java.naming.ldap.factory.socket",TrustAllSocketFactory.class.getName());
}
newProps.putAll(props);
NamingException namingException = null;

Expand Down Expand Up @@ -540,9 +597,15 @@ private void customizeLdapProperties(Hashtable<String, String> props) {
customizeLdapProperty(props, "com.sun.jndi.ldap.connect.timeout");
customizeLdapProperty(props, "com.sun.jndi.ldap.read.timeout");
}

@IgnoreJRERequirement
@Deprecated
private LdapContext bind(String principalName, String password, SocketInfo server, Hashtable<String, String> props) throws NamingException {
return bind(principalName, password, server, props, null);
}

@IgnoreJRERequirement
private LdapContext bind(String principalName, String password, SocketInfo server, Hashtable<String, String> props, String tlsConfiguration) throws NamingException {
String ldapUrl = (FORCE_LDAPS?"ldaps://":"ldap://") + server + '/';
String oldName = Thread.currentThread().getName();
Thread.currentThread().setName("Connecting to "+ldapUrl+" : "+oldName);
Expand All @@ -560,7 +623,11 @@ private LdapContext bind(String principalName, String password, SocketInfo serve
// see http://download.oracle.com/javase/jndi/tutorial/ldap/ext/starttls.html
try {
StartTlsResponse rsp = (StartTlsResponse)context.extendedOperation(new StartTlsRequest());
rsp.negotiate((SSLSocketFactory)TrustAllSocketFactory.getDefault());
if (isTrustAllCertificatesEnabled(tlsConfiguration)) {
rsp.negotiate((SSLSocketFactory)TrustAllSocketFactory.getDefault());
} else {
rsp.negotiate();
}
LOGGER.fine("Connection upgraded to TLS");
} catch (NamingException e) {
LOGGER.log(Level.FINE, "Failed to start TLS. Authentication will be done via plain-text LDAP", e);
Expand Down Expand Up @@ -810,4 +877,44 @@ public String getDisplayName() {
}
}

@Extension
public final static TlsConfigurationAdministrativeMonitor NOTICE = new TlsConfigurationAdministrativeMonitor();

/**
* Administrative Monitor for changing TLS certificates management
*/
public static final class TlsConfigurationAdministrativeMonitor extends AdministrativeMonitor {
public boolean enableAdministrativeMonitor = false;

public String DEFAULT_SSL_MESSAGE = "TLS is not configured on Active Directory plugin correctly. " +
"Please, change the configuration to a secure option.";

public String getDefaultSslMessage() {
return DEFAULT_SSL_MESSAGE;
}

void enableAdministrativeMonitor(boolean enable) {
enableAdministrativeMonitor = enable;
}

public boolean isActivated() {
return enableAdministrativeMonitor;
}


/**
* Depending on whether the user said "dismiss" or "correct", send him to the right place.
*/
public void doAct(StaplerRequest req, StaplerResponse rsp) throws IOException {
if(req.hasParameter("correct")) {
rsp.sendRedirect(req.getRootPath()+"/configureSecurity");

}
}

public static TlsConfigurationAdministrativeMonitor get() {
return AdministrativeMonitor.all().get(TlsConfigurationAdministrativeMonitor.class);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,18 @@ public class ActiveDirectoryUnixAuthenticationProvider extends AbstractActiveDir
*/
private final static String LDAP_READ_TIMEOUT = "com.sun.jndi.ldap.read.timeout";

/**
* Selects the SSL strategy to follow on the TLS connections
*
* <p>
* Even if we are not using any of the TLS ports (3269/636) the plugin will try to establish a TLS channel
* using startTLS. Because of this, we need to be able to specify the SSL strategy on the plugin
*
* <p>
* For the moment there are two possible values: trustAllCertificates and trustStore.
*/
protected String tlsConfiguration;

public ActiveDirectoryUnixAuthenticationProvider(ActiveDirectorySecurityRealm realm) {
if (realm.domains==null) {
throw new IllegalArgumentException("An Active Directory domain name is required but it is not set");
Expand All @@ -147,6 +159,8 @@ public ActiveDirectoryUnixAuthenticationProvider(ActiveDirectorySecurityRealm re
this.userCache = cache.getUserCache();
this.groupCache = cache.getGroupCache();

this.tlsConfiguration =realm.tlsConfiguration;

Map<String, String> extraEnvVarsMap = ActiveDirectorySecurityRealm.EnvironmentProperty.toMap(realm.environmentProperties);
// TODO In JDK 8u65 I am facing JDK-8139721, JDK-8139942 which makes the plugin to break. Uncomment line once it is fixed.
//props.put(LDAP_CONNECT_TIMEOUT, System.getProperty(LDAP_CONNECT_TIMEOUT, DEFAULT_LDAP_CONNECTION_TIMEOUT));
Expand Down Expand Up @@ -277,7 +291,7 @@ public UserDetails call() throws AuthenticationException {
// two step approach. Use a special credential to obtain DN for the
// user trying to login, then authenticate.
try {
context = descriptor.bind(bindName, bindPassword, ldapServers, props);
context = descriptor.bind(bindName, bindPassword, ldapServers, props, tlsConfiguration);
anonymousBind = false;
} catch (BadCredentialsException e) {
throw new AuthenticationServiceException("Failed to bind to LDAP server with the bind name/password", e);
Expand Down Expand Up @@ -330,7 +344,7 @@ public UserDetails call() throws AuthenticationException {
// if we've used the credential specifically for the bind, we
// need to verify the provided password to do authentication
LOGGER.log(Level.FINE, "Attempting to validate password for DN={0}", dn);
DirContext test = descriptor.bind(dnFormatted, password, ldapServers, props);
DirContext test = descriptor.bind(dnFormatted, password, ldapServers, props, tlsConfiguration);
// Binding alone is not enough to test the credential. Need to actually perform some query operation.
// but if the authentication fails this throws an exception
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?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">
<div class="error">
<form method="post" action="${rootURL}/${it.url}/act" name="${it.id}">
<div style="float:right">
<f:submit name="correct" value="${%Correct}"/>
</div>
</form>
${it.defaultSslMessage}
</div>
</j:jelly>
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@
<f:entry field="removeIrrelevantGroups" title="${%Remove irrelevant groups}">
<f:checkbox />
</f:entry>
<j:choose>
<j:when test="${instance != null}">
<f:entry field="tlsConfiguration" title="${%TLS Configuration}">
<f:select />
</f:entry>
</j:when>
</j:choose>
<f:optionalBlock name="cache" title="${%Enable cache}" checked="${instance.cache != null}">
<f:entry field="size" title="${%Cache Size}">
<f:select default="256"/>
Expand Down

0 comments on commit 063c282

Please sign in to comment.