diff --git a/src/main/java/hudson/remoting/Engine.java b/src/main/java/hudson/remoting/Engine.java index 649ac3bde..08ff6dfad 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 disableHttpsCertValidation; + private boolean noReconnect; /** @@ -153,9 +150,6 @@ public void run() { * @since 2.62.1 */ private boolean keepAlive = true; - - - @CheckForNull private JarCache jarCache = null; @@ -327,6 +321,26 @@ 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. * @param agentLog Path to the agent log. @@ -482,6 +496,7 @@ private void innerRun(IOHub hub, SSLContext context, ExecutorService service) { resolver.setTunnel(tunnel); try { resolver.setSslSocketFactory(getSSLSocketFactory()); + resolver.setDisableHttpsCertValidation(disableHttpsCertValidation); } catch (Exception e) { events.error(e); } @@ -809,8 +824,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/Launcher.java b/src/main/java/hudson/remoting/Launcher.java index c0109d0a4..120a419c6 100644 --- a/src/main/java/hudson/remoting/Launcher.java +++ b/src/main/java/hudson/remoting/Launcher.java @@ -40,6 +40,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.jenkinsci.remoting.util.PathUtils; import org.w3c.dom.Document; import org.w3c.dom.NodeList; @@ -189,6 +191,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") @@ -210,15 +218,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") @@ -345,6 +351,11 @@ public void run() throws Exception { jnlpArgs.add(c); } } + if (noCertificateCheck) { + // 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 { hudson.remoting.jnlp.Main._main(jnlpArgs.toArray(new String[jnlpArgs.size()])); } catch (CmdLineException e) { @@ -766,21 +777,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 f8d50f2e1..382fa0fe1 100644 --- a/src/main/java/hudson/remoting/jnlp/Main.java +++ b/src/main/java/hudson/remoting/jnlp/Main.java @@ -27,7 +27,6 @@ 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; @@ -45,6 +44,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; @@ -52,7 +53,6 @@ import hudson.remoting.Engine; import hudson.remoting.EngineListener; -import java.nio.file.Path; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; @@ -103,6 +103,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,6 +253,12 @@ public Engine createEngine() { engine.setJarCache(new FileSystemJarCache(jarCache,true)); engine.setNoReconnect(noReconnect); engine.setKeepAlive(!noKeepAlive); + + 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 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 0c7c0ddb5..8175ef97c 100644 --- a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java +++ b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java @@ -24,6 +24,9 @@ package org.jenkinsci.remoting.engine; import hudson.remoting.Base64; +import org.jenkinsci.remoting.util.https.NoCheckHostnameVerifier; +import org.jenkinsci.remoting.util.https.NoCheckTrustManager; + import java.io.IOException; import java.net.ConnectException; import java.net.HttpURLConnection; @@ -38,7 +41,9 @@ 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.interfaces.RSAPublicKey; import java.security.spec.InvalidKeySpecException; import java.security.spec.X509EncodedKeySpec; @@ -58,6 +63,9 @@ import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.TrustManager; + import static java.util.logging.Level.INFO; import static org.jenkinsci.remoting.util.ThrowableUtils.chain; @@ -80,6 +88,8 @@ public class JnlpAgentEndpointResolver { private String tunnel; + private boolean disableHttpsCertValidation; + /** * 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 +147,27 @@ public void setTunnel(String tunnel) { this.tunnel = tunnel; } + /** + * Determine if certificate checking should be ignored for JNLP endpoint + * + * @return {@code true} if the HTTPs certificate is disabled, endpoint check is ignored + */ + + public boolean isDisableHttpsCertValidation() { + return disableHttpsCertValidation; + } + + /** + * Sets if the HTTPs certificate check should be disabled. + * + * This behavior is no recommended. + * @param disableHttpsCertValidation + * @since TODO + */ + public void setDisableHttpsCertValidation(boolean disableHttpsCertValidation) { + this.disableHttpsCertValidation = disableHttpsCertValidation; + } + @CheckForNull public JnlpAgentEndpoint resolve() throws IOException { IOException firstError = null; @@ -157,7 +188,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, disableHttpsCertValidation); try { try { con.setConnectTimeout(30000); @@ -314,7 +345,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, disableHttpsCertValidation); con.setConnectTimeout(5000); con.setReadTimeout(5000); con.connect(); @@ -335,7 +366,6 @@ public void waitForReady() throws InterruptedException { } finally { t.setName(oldName); } - } @CheckForNull @@ -382,7 +412,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 disableHttpsCertValidation) throws IOException { String httpProxy = null; // If http.proxyHost property exists, openConnection() uses it. if (System.getProperty("http.proxyHost") == null) { @@ -411,8 +441,30 @@ 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) { - ((HttpsURLConnection) con).setSSLSocketFactory(sslSocketFactory); + + if (con instanceof HttpsURLConnection) { + final HttpsURLConnection httpsConnection = (HttpsURLConnection) con; + if (disableHttpsCertValidation) { + System.err.println("Warning: HTTPs certificate check is disabled for the endpoint"); + + 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) { + // 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) { + httpsConnection.setSSLSocketFactory(sslSocketFactory); + //FIXME: Is it really required in this path? Seems like a bug + httpsConnection.setHostnameVerifier(new NoCheckHostnameVerifier()); + } } 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]; + } +}