From 833838ad6ae1d5a25c9f22f1ff247719d938e74b Mon Sep 17 00:00:00 2001 From: Matthew Ludlum Date: Fri, 20 Oct 2017 17:26:25 -0500 Subject: [PATCH 1/4] Add insecure flag to allow bypassing SSL hostname verification and cert checks --- src/main/java/hudson/remoting/Engine.java | 29 +++++-- src/main/java/hudson/remoting/jnlp/Main.java | 10 ++- .../engine/JnlpAgentEndpointResolver.java | 85 +++++++++++++++++-- 3 files changed, 110 insertions(+), 14 deletions(-) diff --git a/src/main/java/hudson/remoting/Engine.java b/src/main/java/hudson/remoting/Engine.java index 649ac3bde..dbde54be8 100644 --- a/src/main/java/hudson/remoting/Engine.java +++ b/src/main/java/hudson/remoting/Engine.java @@ -23,15 +23,11 @@ */ package hudson.remoting; -import edu.umd.cs.findbugs.annotations.Nullable; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.remoting.Channel.Mode; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; -import java.io.FileOutputStream; import java.io.IOException; -import java.io.PrintStream; import java.net.Socket; import java.net.URL; import java.nio.file.Path; @@ -134,7 +130,6 @@ public void run() { */ @CheckForNull private URL hudsonUrl; - private final String secretKey; public final String slaveName; private String credentials; @@ -145,6 +140,8 @@ public void run() { */ private String tunnel; + private boolean insecure; + private boolean noReconnect; /** @@ -156,6 +153,23 @@ public void run() { + /** + * Determines if JNLPAgentEndpointResolver will not perform certificate validation + * @return + */ + public boolean isInsecure() { + return insecure; + } + + /** + * Sets if JNLPAgentEndpointResolver will not perform certificate validation + * + * @param insecure + */ + + public void setInsecure(boolean insecure) { + this.insecure = insecure; + } @CheckForNull private JarCache jarCache = null; @@ -327,6 +341,8 @@ public void setNoReconnect(boolean noReconnect) { this.noReconnect = noReconnect; } + + /** * Sets the destination for agent logs. * @param agentLog Path to the agent log. @@ -482,6 +498,7 @@ private void innerRun(IOHub hub, SSLContext context, ExecutorService service) { resolver.setTunnel(tunnel); try { resolver.setSslSocketFactory(getSSLSocketFactory()); + resolver.setInsecure(insecure); } catch (Exception e) { events.error(e); } @@ -809,8 +826,8 @@ private SSLSocketFactory getSSLSocketFactory() trustManagerFactory.init(keyStore); // prepare the SSL context SSLContext ctx = SSLContext.getInstance("TLS"); - ctx.init(null, trustManagerFactory.getTrustManagers(), null); // now we have our custom socket factory + ctx.init(null, trustManagerFactory.getTrustManagers(), null); sslSocketFactory = ctx.getSocketFactory(); } return sslSocketFactory; diff --git a/src/main/java/hudson/remoting/jnlp/Main.java b/src/main/java/hudson/remoting/jnlp/Main.java index 747224bf4..a04a2b7d7 100644 --- a/src/main/java/hudson/remoting/jnlp/Main.java +++ b/src/main/java/hudson/remoting/jnlp/Main.java @@ -27,14 +27,12 @@ import hudson.remoting.FileSystemJarCache; import java.io.ByteArrayInputStream; import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.io.UnsupportedEncodingException; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import org.jenkinsci.remoting.engine.WorkDirManager; -import org.jenkinsci.remoting.util.IOUtils; import org.kohsuke.args4j.Option; import org.kohsuke.args4j.CmdLineParser; import org.kohsuke.args4j.Argument; @@ -51,7 +49,6 @@ import hudson.remoting.Engine; import hudson.remoting.EngineListener; -import java.nio.file.Path; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; @@ -92,6 +89,10 @@ public class Main { usage="If the connection ends, don't retry and just exit.") public boolean noReconnect = false; + @Option(name="-insecure", + usage="Ignore SSL validation errors - use as a last resort only.") + public boolean insecure = false; + @Option(name="-noKeepAlive", usage="Disable TCP socket keep alive on connection to the master.") public boolean noKeepAlive = false; @@ -242,6 +243,9 @@ public Engine createEngine() { engine.setJarCache(new FileSystemJarCache(jarCache,true)); engine.setNoReconnect(noReconnect); engine.setKeepAlive(!noKeepAlive); + LOGGER.log(INFO, "Insecure Status: {0}", insecure); + engine.setInsecure(insecure); + // TODO: ideally logging should be initialized before the "Setting up slave" entry if (agentLog != null) { diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java index 4dfbef50b..bad55a8ec 100644 --- a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java +++ b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java @@ -38,7 +38,10 @@ import java.net.URL; import java.net.URLConnection; import java.security.KeyFactory; +import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; +import java.security.cert.X509Certificate; import java.security.interfaces.RSAPublicKey; import java.security.spec.InvalidKeySpecException; import java.security.spec.X509EncodedKeySpec; @@ -58,6 +61,12 @@ import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSession; +import javax.net.ssl.TrustManager; +import javax.net.ssl.X509TrustManager; + import static java.util.logging.Level.INFO; import static org.jenkinsci.remoting.util.ThrowableUtils.chain; @@ -80,6 +89,8 @@ public class JnlpAgentEndpointResolver { private String tunnel; + private boolean insecure; + /** * If specified, only the protocols from the list will be tried during the connection. * The option provides protocol names, but the order of the check is defined internally and cannot be changed. @@ -137,6 +148,25 @@ public void setTunnel(String tunnel) { this.tunnel = tunnel; } + /** + * Determine if certificate checking should be ignored for JNLP endpoint + * + * @return if insecure, endpoint check is ignored + */ + + public boolean isInsecure() { + return insecure; + } + + /** + * Sets if insecure mode of endpoint should be used. + * + * @param insecure + */ + public void setInsecure(boolean insecure) { + this.insecure = insecure; + } + @CheckForNull public JnlpAgentEndpoint resolve() throws IOException { IOException firstError = null; @@ -154,7 +184,7 @@ public JnlpAgentEndpoint resolve() throws IOException { // find out the TCP port HttpURLConnection con = - (HttpURLConnection) openURLConnection(salURL, credentials, proxyCredentials, sslSocketFactory); + (HttpURLConnection) openURLConnection(salURL, credentials, proxyCredentials, sslSocketFactory, insecure); try { try { con.setConnectTimeout(30000); @@ -310,7 +340,7 @@ public void waitForReady() throws InterruptedException { t.setName(oldName + ": trying " + url + " for " + retries + " times"); HttpURLConnection con = - (HttpURLConnection) openURLConnection(url, credentials, proxyCredentials, sslSocketFactory); + (HttpURLConnection) openURLConnection(url, credentials, proxyCredentials, sslSocketFactory, insecure); con.setConnectTimeout(5000); con.setReadTimeout(5000); con.connect(); @@ -331,7 +361,6 @@ public void waitForReady() throws InterruptedException { } finally { t.setName(oldName); } - } @CheckForNull @@ -378,7 +407,7 @@ static InetSocketAddress getResolvedHttpProxyAddress(@Nonnull String host, int p * Credentials can be passed e.g. to support running Jenkins behind a (reverse) proxy requiring authorization */ static URLConnection openURLConnection(URL url, String credentials, String proxyCredentials, - SSLSocketFactory sslSocketFactory) throws IOException { + SSLSocketFactory sslSocketFactory, boolean insecure) throws IOException { String httpProxy = null; // If http.proxyHost property exists, openConnection() uses it. if (System.getProperty("http.proxyHost") == null) { @@ -407,8 +436,54 @@ static URLConnection openURLConnection(URL url, String credentials, String proxy String encoding = Base64.encode(proxyCredentials.getBytes("UTF-8")); con.setRequestProperty("Proxy-Authorization", "Basic " + encoding); } - if (con instanceof HttpsURLConnection && sslSocketFactory != null) { + + if (insecure && con instanceof HttpsURLConnection) { + System.out.println(String.format("Insecure Status: %s", insecure)); + try { + SSLContext ctx = SSLContext.getInstance("TLS"); + + ctx.init(null, new TrustManager[]{new X509TrustManager() { + @Override + public X509Certificate[] getAcceptedIssuers() { + return null; + } + + @Override + public void checkClientTrusted(X509Certificate[] certs, + String authType) { + } + + @Override + public void checkServerTrusted(X509Certificate[] certs, + String authType) { + } + + }}, new SecureRandom()); + sslSocketFactory = ctx.getSocketFactory(); + + HostnameVerifier allHostsValid = new HostnameVerifier() { + @Override + public boolean verify(String hostname, SSLSession session) { + return true; + } + }; + + ((HttpsURLConnection) con).setHostnameVerifier(allHostsValid); + ((HttpsURLConnection) con).setSSLSocketFactory(sslSocketFactory); + } catch (KeyManagementException | NoSuchAlgorithmException ex) { + System.err.println(String.format("Error setting insecure; %s", ex.getMessage())); + } + + } + else if (con instanceof HttpsURLConnection && sslSocketFactory != null) { ((HttpsURLConnection) con).setSSLSocketFactory(sslSocketFactory); + HostnameVerifier allHostsValid = new HostnameVerifier() { + @Override + public boolean verify(String hostname, SSLSession session) { + return true; + } + }; + ((HttpsURLConnection) con).setHostnameVerifier(allHostsValid); } return con; } From 328a7666d923fd1d7453057c6da36b349c11d1d0 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Fri, 3 Nov 2017 14:14:45 +0100 Subject: [PATCH 2/4] Refactor the implementation and reuse old code created by @stephenc --- src/main/java/hudson/remoting/Engine.java | 42 ++++++----- src/main/java/hudson/remoting/Launcher.java | 35 ++++------ src/main/java/hudson/remoting/jnlp/Main.java | 23 ++++-- .../engine/JnlpAgentEndpointResolver.java | 70 +++++++------------ .../util/https/NoCheckHostnameVerifier.java | 45 ++++++++++++ .../util/https/NoCheckTrustManager.java | 50 +++++++++++++ 6 files changed, 171 insertions(+), 94 deletions(-) create mode 100644 src/main/java/org/jenkinsci/remoting/util/https/NoCheckHostnameVerifier.java create mode 100644 src/main/java/org/jenkinsci/remoting/util/https/NoCheckTrustManager.java diff --git a/src/main/java/hudson/remoting/Engine.java b/src/main/java/hudson/remoting/Engine.java index dbde54be8..96d1b393d 100644 --- a/src/main/java/hudson/remoting/Engine.java +++ b/src/main/java/hudson/remoting/Engine.java @@ -140,7 +140,7 @@ public void run() { */ private String tunnel; - private boolean insecure; + private boolean disableHttpsCertValidation; private boolean noReconnect; @@ -150,26 +150,6 @@ public void run() { * @since 2.62.1 */ private boolean keepAlive = true; - - - - /** - * Determines if JNLPAgentEndpointResolver will not perform certificate validation - * @return - */ - public boolean isInsecure() { - return insecure; - } - - /** - * Sets if JNLPAgentEndpointResolver will not perform certificate validation - * - * @param insecure - */ - - public void setInsecure(boolean insecure) { - this.insecure = insecure; - } @CheckForNull private JarCache jarCache = null; @@ -341,7 +321,25 @@ public void setNoReconnect(boolean noReconnect) { this.noReconnect = noReconnect; } + /** + * Determines if JNLPAgentEndpointResolver will not perform certificate validation in the HTTPs mode. + * + * @return {@code true} if the certificate validation is disabled. + * @since TODO + */ + public boolean isDisableHttpsCertValidation() { + return disableHttpsCertValidation; + } + /** + * Sets if JNLPAgentEndpointResolver will not perform certificate validation in the HTTPs mode. + * + * @param disableHttpsCertValidation {@code true} if the certificate validation is disabled. + * @since TODO + */ + public void setDisableHttpsCertValidation(boolean disableHttpsCertValidation) { + this.disableHttpsCertValidation = disableHttpsCertValidation; + } /** * Sets the destination for agent logs. @@ -498,7 +496,7 @@ private void innerRun(IOHub hub, SSLContext context, ExecutorService service) { resolver.setTunnel(tunnel); try { resolver.setSslSocketFactory(getSSLSocketFactory()); - resolver.setInsecure(insecure); + resolver.setInsecure(disableHttpsCertValidation); } catch (Exception e) { events.error(e); } diff --git a/src/main/java/hudson/remoting/Launcher.java b/src/main/java/hudson/remoting/Launcher.java index e55a81cbb..10f6af948 100644 --- a/src/main/java/hudson/remoting/Launcher.java +++ b/src/main/java/hudson/remoting/Launcher.java @@ -41,6 +41,8 @@ import org.jenkinsci.remoting.engine.WorkDirManager; import org.jenkinsci.remoting.util.IOUtils; +import org.jenkinsci.remoting.util.https.NoCheckHostnameVerifier; +import org.jenkinsci.remoting.util.https.NoCheckTrustManager; import org.w3c.dom.Document; import org.w3c.dom.NodeList; import org.xml.sax.SAXException; @@ -191,6 +193,12 @@ public void addClasspath(String pathList) throws Exception { "certificate file to read.", forbids = "-noCertificateCheck") public List candidateCertificates; + /** + * Disables HTTPs Certificate validation of the server when using {@link org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver}. + * This option is managed by the {@code -noCertificateCheck} option. + */ + private boolean noCertificateCheck = false; + public InetSocketAddress connectionTarget = null; @Option(name="-connectTo",usage="make a TCP connection to the given host and port, then start communication.",metaVar="HOST:PORT") @@ -212,15 +220,13 @@ public void setConnectTo(String target) { @Option(name="-noCertificateCheck", forbids = "-cert") public void setNoCertificateCheck(boolean ignored) throws NoSuchAlgorithmException, KeyManagementException { System.out.println("Skipping HTTPS certificate checks altogether. Note that this is not secure at all."); + + this.noCertificateCheck = true; SSLContext context = SSLContext.getInstance("TLS"); context.init(null, new TrustManager[]{new NoCheckTrustManager()}, new java.security.SecureRandom()); HttpsURLConnection.setDefaultSSLSocketFactory(context.getSocketFactory()); // bypass host name check, too. - HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() { - public boolean verify(String s, SSLSession sslSession) { - return true; - } - }); + HttpsURLConnection.setDefaultHostnameVerifier(new NoCheckHostnameVerifier()); } @Option(name="-noReconnect",usage="Doesn't try to reconnect when a communication fail, and exit instead") @@ -347,6 +353,10 @@ public void run() throws Exception { jnlpArgs.add(c); } } + if (noCertificateCheck) { + //TODO: Rename option? + jnlpArgs.add("-disableHttpsCertValidation"); + } try { hudson.remoting.jnlp.Main._main(jnlpArgs.toArray(new String[jnlpArgs.size()])); } catch (CmdLineException e) { @@ -768,21 +778,6 @@ protected void onDead(Throwable cause) { System.err.println("channel stopped"); } - /** - * {@link X509TrustManager} that performs no check at all. - */ - private static class NoCheckTrustManager implements X509TrustManager { - public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { - } - - public void checkServerTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { - } - - public X509Certificate[] getAcceptedIssuers() { - return new X509Certificate[0]; - } - } - public static boolean isWindows() { return File.pathSeparatorChar==';'; } diff --git a/src/main/java/hudson/remoting/jnlp/Main.java b/src/main/java/hudson/remoting/jnlp/Main.java index a04a2b7d7..5363af69e 100644 --- a/src/main/java/hudson/remoting/jnlp/Main.java +++ b/src/main/java/hudson/remoting/jnlp/Main.java @@ -42,6 +42,8 @@ import java.util.logging.Logger; import java.util.logging.Level; import static java.util.logging.Level.INFO; +import static java.util.logging.Level.WARNING; + import java.util.List; import java.util.ArrayList; import java.net.URL; @@ -89,10 +91,6 @@ public class Main { usage="If the connection ends, don't retry and just exit.") public boolean noReconnect = false; - @Option(name="-insecure", - usage="Ignore SSL validation errors - use as a last resort only.") - public boolean insecure = false; - @Option(name="-noKeepAlive", usage="Disable TCP socket keep alive on connection to the master.") public boolean noKeepAlive = false; @@ -103,6 +101,16 @@ public class Main { "certificate file to read.") public List candidateCertificates; + /** + * Disables HTTPs Certificate validation of the server when using {@link org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver}. + * + * This option is not recommended for production use. + * @since TODO + */ + @Option(name="-disableHttpsCertValidation", + usage="Ignore SSL validation errors - use as a last resort only.") + public boolean disableHttpsCertValidation = false; + /** * Specifies a destination for error logs. * If specified, this option overrides the default destination within {@link #workDir}. @@ -243,8 +251,11 @@ public Engine createEngine() { engine.setJarCache(new FileSystemJarCache(jarCache,true)); engine.setNoReconnect(noReconnect); engine.setKeepAlive(!noKeepAlive); - LOGGER.log(INFO, "Insecure Status: {0}", insecure); - engine.setInsecure(insecure); + + if (disableHttpsCertValidation) { + LOGGER.log(WARNING, "Certificate validation for HTTPs endpoints is disabled"); + } + engine.setDisableHttpsCertValidation(disableHttpsCertValidation); // TODO: ideally logging should be initialized before the "Setting up slave" entry diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java index bad55a8ec..b0412f63f 100644 --- a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java +++ b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java @@ -24,6 +24,10 @@ package org.jenkinsci.remoting.engine; import hudson.remoting.Base64; +import org.jenkinsci.remoting.util.https.NoCheckHostnameVerifier; +import org.jenkinsci.remoting.util.https.NoCheckTrustManager; +import sun.net.www.protocol.https.HttpsURLConnectionImpl; + import java.io.IOException; import java.net.ConnectException; import java.net.HttpURLConnection; @@ -151,7 +155,7 @@ public void setTunnel(String tunnel) { /** * Determine if certificate checking should be ignored for JNLP endpoint * - * @return if insecure, endpoint check is ignored + * @return if disableHttpsCertValidation, endpoint check is ignored */ public boolean isInsecure() { @@ -159,7 +163,7 @@ public boolean isInsecure() { } /** - * Sets if insecure mode of endpoint should be used. + * Sets if disableHttpsCertValidation mode of endpoint should be used. * * @param insecure */ @@ -437,53 +441,27 @@ static URLConnection openURLConnection(URL url, String credentials, String proxy con.setRequestProperty("Proxy-Authorization", "Basic " + encoding); } - if (insecure && con instanceof HttpsURLConnection) { - System.out.println(String.format("Insecure Status: %s", insecure)); - try { - SSLContext ctx = SSLContext.getInstance("TLS"); - - ctx.init(null, new TrustManager[]{new X509TrustManager() { - @Override - public X509Certificate[] getAcceptedIssuers() { - return null; - } - - @Override - public void checkClientTrusted(X509Certificate[] certs, - String authType) { - } + if (con instanceof HttpsURLConnection) { + final HttpsURLConnection httpsConnection = (HttpsURLConnection) con; + if (insecure) { + System.out.println(String.format("Insecure Status: %s", insecure)); - @Override - public void checkServerTrusted(X509Certificate[] certs, - String authType) { - } - - }}, new SecureRandom()); - sslSocketFactory = ctx.getSocketFactory(); - - HostnameVerifier allHostsValid = new HostnameVerifier() { - @Override - public boolean verify(String hostname, SSLSession session) { - return true; - } - }; + try { + SSLContext ctx = SSLContext.getInstance("TLS"); + ctx.init(null, new TrustManager[]{new NoCheckTrustManager()}, new SecureRandom()); + sslSocketFactory = ctx.getSocketFactory(); + + httpsConnection.setHostnameVerifier(new NoCheckHostnameVerifier()); + httpsConnection.setSSLSocketFactory(sslSocketFactory); + } catch (KeyManagementException | NoSuchAlgorithmException ex) { + System.err.println(String.format("Error setting disableHttpsCertValidation; %s", ex.getMessage())); + } - ((HttpsURLConnection) con).setHostnameVerifier(allHostsValid); - ((HttpsURLConnection) con).setSSLSocketFactory(sslSocketFactory); - } catch (KeyManagementException | NoSuchAlgorithmException ex) { - System.err.println(String.format("Error setting insecure; %s", ex.getMessage())); + } else if (sslSocketFactory != null) { + httpsConnection.setSSLSocketFactory(sslSocketFactory); + //FIXME: Is it really required in this path? Seems like a bug + httpsConnection.setHostnameVerifier(new NoCheckHostnameVerifier()); } - - } - else if (con instanceof HttpsURLConnection && sslSocketFactory != null) { - ((HttpsURLConnection) con).setSSLSocketFactory(sslSocketFactory); - HostnameVerifier allHostsValid = new HostnameVerifier() { - @Override - public boolean verify(String hostname, SSLSession session) { - return true; - } - }; - ((HttpsURLConnection) con).setHostnameVerifier(allHostsValid); } return con; } diff --git a/src/main/java/org/jenkinsci/remoting/util/https/NoCheckHostnameVerifier.java b/src/main/java/org/jenkinsci/remoting/util/https/NoCheckHostnameVerifier.java new file mode 100644 index 000000000..943bb0274 --- /dev/null +++ b/src/main/java/org/jenkinsci/remoting/util/https/NoCheckHostnameVerifier.java @@ -0,0 +1,45 @@ +/* + * + * The MIT License + * + * Copyright (c) 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 org.jenkinsci.remoting.util.https; + +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; + +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLSession; + +/** + * Hostname verifier, which accepts any hostname. + */ +@Restricted(NoExternalUse.class) +public class NoCheckHostnameVerifier implements HostnameVerifier { + + @Override + public boolean verify(String s, SSLSession sslSession) { + return true; + } +} diff --git a/src/main/java/org/jenkinsci/remoting/util/https/NoCheckTrustManager.java b/src/main/java/org/jenkinsci/remoting/util/https/NoCheckTrustManager.java new file mode 100644 index 000000000..18f9909fa --- /dev/null +++ b/src/main/java/org/jenkinsci/remoting/util/https/NoCheckTrustManager.java @@ -0,0 +1,50 @@ +/* + * + * The MIT License + * + * Copyright (c) 2016- 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 org.jenkinsci.remoting.util.https; + +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; + +import javax.net.ssl.X509TrustManager; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; + +/** + * {@link X509TrustManager} that performs no check at all. + */ +@Restricted(NoExternalUse.class) +public class NoCheckTrustManager implements X509TrustManager { + public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { + } + + public void checkServerTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { + } + + public X509Certificate[] getAcceptedIssuers() { + return new X509Certificate[0]; + } +} From 9e2e1a9b24901145346ddbdf784b55c80ae7ca3c Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Fri, 10 Nov 2017 15:36:39 +0100 Subject: [PATCH 3/4] Refactor the HTTPs check kill switch in JnlpAgentEndpointResolver --- src/main/java/hudson/remoting/Engine.java | 2 +- .../engine/JnlpAgentEndpointResolver.java | 37 +++++++++---------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/main/java/hudson/remoting/Engine.java b/src/main/java/hudson/remoting/Engine.java index 96d1b393d..08ff6dfad 100644 --- a/src/main/java/hudson/remoting/Engine.java +++ b/src/main/java/hudson/remoting/Engine.java @@ -496,7 +496,7 @@ private void innerRun(IOHub hub, SSLContext context, ExecutorService service) { resolver.setTunnel(tunnel); try { resolver.setSslSocketFactory(getSSLSocketFactory()); - resolver.setInsecure(disableHttpsCertValidation); + resolver.setDisableHttpsCertValidation(disableHttpsCertValidation); } catch (Exception e) { events.error(e); } diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java index b0412f63f..9fa45f486 100644 --- a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java +++ b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java @@ -26,7 +26,6 @@ import hudson.remoting.Base64; import org.jenkinsci.remoting.util.https.NoCheckHostnameVerifier; import org.jenkinsci.remoting.util.https.NoCheckTrustManager; -import sun.net.www.protocol.https.HttpsURLConnectionImpl; import java.io.IOException; import java.net.ConnectException; @@ -45,7 +44,6 @@ import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; -import java.security.cert.X509Certificate; import java.security.interfaces.RSAPublicKey; import java.security.spec.InvalidKeySpecException; import java.security.spec.X509EncodedKeySpec; @@ -65,11 +63,8 @@ import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLSocketFactory; -import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLSession; import javax.net.ssl.TrustManager; -import javax.net.ssl.X509TrustManager; import static java.util.logging.Level.INFO; import static org.jenkinsci.remoting.util.ThrowableUtils.chain; @@ -93,7 +88,7 @@ public class JnlpAgentEndpointResolver { private String tunnel; - private boolean insecure; + private boolean disableHttpsCertValidation; /** * If specified, only the protocols from the list will be tried during the connection. @@ -155,20 +150,22 @@ public void setTunnel(String tunnel) { /** * Determine if certificate checking should be ignored for JNLP endpoint * - * @return if disableHttpsCertValidation, endpoint check is ignored + * @return {@code true} if the HTTPs certificate is disabled, endpoint check is ignored */ - public boolean isInsecure() { - return insecure; + public boolean isDisableHttpsCertValidation() { + return disableHttpsCertValidation; } /** - * Sets if disableHttpsCertValidation mode of endpoint should be used. + * Sets if the HTTPs certificate check should be disabled. * - * @param insecure + * This behavior is no recommended. + * @param disableHttpsCertValidation + * @since TODO */ - public void setInsecure(boolean insecure) { - this.insecure = insecure; + public void setDisableHttpsCertValidation(boolean disableHttpsCertValidation) { + this.disableHttpsCertValidation = disableHttpsCertValidation; } @CheckForNull @@ -188,7 +185,7 @@ public JnlpAgentEndpoint resolve() throws IOException { // find out the TCP port HttpURLConnection con = - (HttpURLConnection) openURLConnection(salURL, credentials, proxyCredentials, sslSocketFactory, insecure); + (HttpURLConnection) openURLConnection(salURL, credentials, proxyCredentials, sslSocketFactory, disableHttpsCertValidation); try { try { con.setConnectTimeout(30000); @@ -344,7 +341,7 @@ public void waitForReady() throws InterruptedException { t.setName(oldName + ": trying " + url + " for " + retries + " times"); HttpURLConnection con = - (HttpURLConnection) openURLConnection(url, credentials, proxyCredentials, sslSocketFactory, insecure); + (HttpURLConnection) openURLConnection(url, credentials, proxyCredentials, sslSocketFactory, disableHttpsCertValidation); con.setConnectTimeout(5000); con.setReadTimeout(5000); con.connect(); @@ -411,7 +408,7 @@ static InetSocketAddress getResolvedHttpProxyAddress(@Nonnull String host, int p * Credentials can be passed e.g. to support running Jenkins behind a (reverse) proxy requiring authorization */ static URLConnection openURLConnection(URL url, String credentials, String proxyCredentials, - SSLSocketFactory sslSocketFactory, boolean insecure) throws IOException { + SSLSocketFactory sslSocketFactory, boolean disableHttpsCertValidation) throws IOException { String httpProxy = null; // If http.proxyHost property exists, openConnection() uses it. if (System.getProperty("http.proxyHost") == null) { @@ -443,8 +440,8 @@ static URLConnection openURLConnection(URL url, String credentials, String proxy if (con instanceof HttpsURLConnection) { final HttpsURLConnection httpsConnection = (HttpsURLConnection) con; - if (insecure) { - System.out.println(String.format("Insecure Status: %s", insecure)); + if (disableHttpsCertValidation) { + System.err.println("Warning: HTTPs certificate check is disabled for the endpoint"); try { SSLContext ctx = SSLContext.getInstance("TLS"); @@ -454,7 +451,9 @@ static URLConnection openURLConnection(URL url, String credentials, String proxy httpsConnection.setHostnameVerifier(new NoCheckHostnameVerifier()); httpsConnection.setSSLSocketFactory(sslSocketFactory); } catch (KeyManagementException | NoSuchAlgorithmException ex) { - System.err.println(String.format("Error setting disableHttpsCertValidation; %s", ex.getMessage())); + // We could just suppress it, but the exception will unlikely happen. + // So let's just propagate the error and fail the resolution + throw new IOException("Cannot initialize the insecure HTTPs mode", ex); } } else if (sslSocketFactory != null) { From 77621ec883cdcd8a2c222cf63312949dbea6e1d4 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Mon, 20 Nov 2017 05:50:52 +0100 Subject: [PATCH 4/4] Launcher: Modify comment about -noCertificateCheck usage for JNLP arguments --- src/main/java/hudson/remoting/Launcher.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/remoting/Launcher.java b/src/main/java/hudson/remoting/Launcher.java index 557bfecf1..120a419c6 100644 --- a/src/main/java/hudson/remoting/Launcher.java +++ b/src/main/java/hudson/remoting/Launcher.java @@ -352,7 +352,8 @@ public void run() throws Exception { } } if (noCertificateCheck) { - //TODO: Rename option? + // Generally it is not required since the default settings have been changed anyway. + // But we set it up just in case there are overrides somewhere in the logic jnlpArgs.add("-disableHttpsCertValidation"); } try {