Added -strictHostKey option to CLI in -ssh mode.

[FIXED JENKINS-33595] Picks up jenkinsci/sshd-plugin#11 to turn off SSHD by default, but expose it to tests which wish to enable it.
jenkinsci · Mar 24, 2017 · 60632c0 · 60632c0
1 parent e69ba0f
commit 60632c0
Show file tree

Hide file tree

Showing 9 changed files with 131 additions and 18 deletions.
diff --git a/cli/pom.xml b/cli/pom.xml
@@ -60,7 +60,7 @@
     </dependency>
     <dependency>
       <groupId>org.slf4j</groupId>
-      <artifactId>slf4j-nop</artifactId>
+      <artifactId>slf4j-jdk14</artifactId>
       <optional>true</optional> <!-- ditto -->
     </dependency>
     <dependency> <!-- TODO remove and replace PrivateKeyProvider with SecurityUtils.createFileKeyPairProvider() as in SshClient -->

diff --git a/cli/src/main/java/hudson/cli/CLI.java b/cli/src/main/java/hudson/cli/CLI.java
@@ -445,6 +445,7 @@ public static int _main(String[] _args) throws Exception {
 
         String user = null;
         String auth = null;
+        boolean strictHostKey = false;
 
         while(!args.isEmpty()) {
             String head = args.get(0);
@@ -516,6 +517,11 @@ public boolean verify(String s, SSLSession sslSession) {
                 sshAuthRequestedExplicitly = true;
                 continue;
             }
+            if (head.equals("-strictHostKey")) {
+                strictHostKey = true;
+                args = args.subList(1, args.size());
+                continue;
+            }
             if (head.equals("-user") && args.size() >= 2) {
                 user = args.get(1);
                 args = args.subList(2, args.size());
@@ -572,7 +578,11 @@ public boolean verify(String s, SSLSession sslSession) {
                 LOGGER.warning("-user required when using -ssh");
                 return -1;
             }
-            return sshConnection(url, user, args, provider);
+            return sshConnection(url, user, args, provider, strictHostKey);
+        }
+
+        if (strictHostKey) {
+            LOGGER.warning("-strictHostKey meaningful only with -ssh");
         }
 
         if (user != null) {
@@ -626,7 +636,7 @@ public boolean verify(String s, SSLSession sslSession) {
         }
     }
 
-    private static int sshConnection(String jenkinsUrl, String user, List<String> args, PrivateKeyProvider provider) throws IOException {
+    private static int sshConnection(String jenkinsUrl, String user, List<String> args, PrivateKeyProvider provider, final boolean strictHostKey) throws IOException {
         URL url = new URL(jenkinsUrl + "/login");
         URLConnection conn = url.openConnection();
         String endpointDescription = conn.getHeaderField("X-SSH-Endpoint");
@@ -653,10 +663,8 @@ private static int sshConnection(String jenkinsUrl, String user, List<String> ar
             KnownHostsServerKeyVerifier verifier = new DefaultKnownHostsServerKeyVerifier(new ServerKeyVerifier() {
                 @Override
                 public boolean verifyServerKey(ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey) {
-                    /** unknown key is okay, but log */
                     LOGGER.log(Level.WARNING, "Unknown host key for {0}", remoteAddress.toString());
-                    // TODO should not trust unknown hosts by default; this should be opt-in
-                    return true;
+                    return !strictHostKey;
                 }
             }, true);
 

diff --git a/cli/src/main/resources/hudson/cli/client/Messages.properties b/cli/src/main/resources/hudson/cli/client/Messages.properties
@@ -10,6 +10,7 @@ CLI.Usage=Jenkins CLI\n\
    -noCertificateCheck : bypass HTTPS certificate check entirely. Use with caution\n\
    -noKeyAuth   : don't try to load the SSH authentication private key. Conflicts with -i\n\
    -user        : specify user (for use with -ssh)\n\
+   -strictHostKey : request strict host key checking (for use with -ssh)\n\
    -logger FINE : enable detailed logging from the client\n\
    -auth [ USER:SECRET | @FILE ] : specify username and either password or API token (or load from them both from a file);\n\
                                    for use with -http, or -remoting but only when the JNLP agent port is disabled\n\

diff --git a/pom.xml b/pom.xml
@@ -191,11 +191,6 @@ THE SOFTWARE.
         <artifactId>slf4j-api</artifactId>
         <version>${slf4jVersion}</version>
       </dependency>
-      <dependency>
-        <groupId>org.slf4j</groupId>
-        <artifactId>slf4j-nop</artifactId>
-        <version>${slf4jVersion}</version>
-      </dependency>
       <dependency>
         <groupId>org.slf4j</groupId>
         <artifactId>slf4j-jdk14</artifactId>

diff --git a/test/pom.xml b/test/pom.xml
@@ -52,12 +52,6 @@ THE SOFTWARE.
       <artifactId>jenkins-war</artifactId>
       <version>${project.version}</version>
       <classifier>war-for-test</classifier>
-      <exclusions>
-        <exclusion>
-          <groupId>org.jenkins-ci.modules</groupId>
-          <artifactId>sshd</artifactId>
-        </exclusion>
-      </exclusions>
     </dependency>
     <dependency>
       <groupId>${project.groupId}</groupId>

diff --git a/test/src/test/java/hudson/cli/CLITest.java b/test/src/test/java/hudson/cli/CLITest.java
@@ -0,0 +1,87 @@
+/*
+ * The MIT License
+ *
+ * Copyright 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 hudson.cli;
+
+import hudson.Launcher;
+import hudson.model.User;
+import hudson.util.StreamTaskListener;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import jenkins.model.Jenkins;
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
+import static org.hamcrest.Matchers.containsString;
+import org.jenkinsci.main.modules.cli.auth.ssh.UserPropertyImpl;
+import org.jenkinsci.main.modules.sshd.SSHD;
+import static org.junit.Assert.*;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+
+public class CLITest {
+
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+    @Rule
+    public TemporaryFolder tmp = new TemporaryFolder();
+
+    @Issue("JENKINS-41745")
+    @Test
+    public void strictHostKey() throws Exception {
+        File home = tmp.newFolder();
+        // Seems it gets created automatically but with inappropriate permissions:
+        File known_hosts = new File(new File(home, ".ssh"), "known_hosts");
+        assertTrue(known_hosts.getParentFile().mkdir());
+        assertTrue(known_hosts.createNewFile());
+        assertTrue(known_hosts.setWritable(false, false));
+        assertTrue(known_hosts.setWritable(true, true));
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.ADMINISTER).everywhere().to("admin"));
+        SSHD.get().setPort(0);
+        File jar = tmp.newFile("jenkins-cli.jar");
+        FileUtils.copyURLToFile(r.jenkins.getJnlpJars("jenkins-cli.jar").getURL(), jar);
+        File privkey = tmp.newFile("id_rsa");
+        FileUtils.copyURLToFile(CLITest.class.getResource("id_rsa"), privkey);
+        User.get("admin").addProperty(new UserPropertyImpl(IOUtils.toString(CLITest.class.getResource("id_rsa.pub"))));
+        assertNotEquals(0, new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(
+            "java", "-Duser.home=" + home, "-jar", jar.getAbsolutePath(), "-s", r.getURL().toString(), "-ssh", "-user", "admin", "-i", privkey.getAbsolutePath(), "-strictHostKey", "who-am-i"
+        ).stdout(System.out).stderr(System.err).join());
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        assertEquals(0, new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(
+            "java", "-Duser.home=" + home, "-jar", jar.getAbsolutePath(), "-s", r.getURL().toString(), "-ssh", "-user", "admin", "-i", privkey.getAbsolutePath(), "who-am-i"
+        ).stdout(baos).stderr(System.err).join());
+        assertThat(baos.toString(), containsString("Authenticated as: admin"));
+        baos = new ByteArrayOutputStream();
+        assertEquals(0, new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(
+            "java", "-Duser.home=" + home, "-jar", jar.getAbsolutePath(), "-s", r.getURL().toString(), "-ssh", "-user", "admin", "-i", privkey.getAbsolutePath(), "-strictHostKey", "who-am-i"
+        ).stdout(baos).stderr(System.err).join());
+        assertThat(baos.toString(), containsString("Authenticated as: admin"));
+    }
+
+}
diff --git a/test/src/test/resources/hudson/cli/id_rsa b/test/src/test/resources/hudson/cli/id_rsa
@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEpAIBAAKCAQEAyTqwFqp5Ww2Tr/52D7hhdOwgzYGBUqxrOFopa+kjNEL1Yqwb
++mApUWZ+D3zN9PurhUcVUfeYVXiYWFJ0kG72HIJawL/0BR5oYxRJfumK8Z/sAzAL
+xdhc5O5twETrr9gU3cxtvF5oJNP0I9HickAOeC+ZNpiDIIblrhvxXl/QwqrR+/Gv
+Nb8TApj+rxXEfNp+N69iGnnxzWn1FeKeOAWpwoBAxZNoqBQAFacF7xfQnoygyekC
+xk+ts2O5Zzv8iJ10sVf+x2Q79rxAtsc0xOGhZbBAzbmFTz0PE4iWuo/Vo1c6mM7u
+/dam+FxB2NqPNw7W+4eiCnEVkiQZlrxmuGvK7wIDAQABAoIBACml1+QZDFzoBnUa
+eVzvkFwesvtVnmp5/QcAwinvarXaVedCL9g2JtcOG3EhJ49YtzsyZxs7329xMja1
+eiKalJ157UaPc/XLQVegT0XRGEzCCJrwSr979F39awGsQgt28XqmYN/nui5FH/Z5
+7iAvWc9OKqu+DQWiZc8PQXmC4zYmvhGQ8vKx44RSqlWCjd9IqBVhpE5gxpI/SmCx
+umUNNtoH0hBWr+MsVHzr6UUrC3a99+7bB4We8XMXXFLzbTUSgiYFmK+NxPs/Fux/
+IAyXAMbDw2HeqZ7g4kTaf4cvmVOwhh4zlvB4p7j301LdO1jmvs9z0fn/QJcTpVM7
+ISMKwAECgYEA/uKVdmOKTk3dKzKRFXtWJjqypOXakoX+25lUcVv2PXYRr8Sln9jC
+A13fbhvwq+FqbdnNlB23ag5niCVLfUpB1DYYP5jd4lU8D6HZQiHlmokB6nLT9NIW
+iTcG88E58Bta/l1Ue5Yn+LqluBC4i289wFbH1kZyxQ565s5dJEv9uAECgYEAyhwF
+ZOqTK2lZe5uuN4owVLQaYFj9fsdFHULzlK/UAtkG1gCJhjBmwSEpZFFMH6WgwHk5
+SHJEom0uB4qRv8gQcxl9OSiDsp56ymr0NBhlPVXWr6IzLotLy5XBC1muqvYYlj7E
+kHgSet/h8RUM/FeEiwOFHDU2DkMb8Qx1hfMdAu8CgYBSEsYL9CuB4WK5WTQMlcV8
+0+PYY0dJbSpOrgXZ5sHYsp8pWQn3+cUnbl/WxdpujkxGCR9AdX0tAmxmE5RGSNX/
+rleKiv/PtKB9bCFYQS/83ecnBkioCcpF7tknPm4YmcZoJ8dfcE94sSlRpti11WEu
+AQOiRNcKCwqaLZMib/HIAQKBgQCdiOffeERMYypfgcJzAiCX9WZV0SeOCS7jFwub
+ys17hsSgS/zl/pYpVXrY+dFXHZfGTvcKdB7xaB6nvCfND9lajfSgd+bndEYLvwAo
+Fxfajizv64LvdZ4XytuUyEuwcHBLtBMs9Jqa8iU/8AOWMXVbkdvQV92RkleWNPrp
+9MyZOwKBgQD9x8MnX5LVBfQKuL9qX6l9Da06EyMkzfz3obKn9AAJ3Xj9+45TNPJu
+HnZyvJWesl1vDjXQTm+PVkdyE0WQgoiVX+wxno0hsoly5Uqb5EYHtTUrZzRpkyLK
+1VmtDxT5D8gorUgn6crzk4PKaxRkPfAimZdlkQm6iOtuR3kqn5BtIQ==
+-----END RSA PRIVATE KEY-----
diff --git a/test/src/test/resources/hudson/cli/id_rsa.pub b/test/src/test/resources/hudson/cli/id_rsa.pub
@@ -0,0 +1 @@
+ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDJOrAWqnlbDZOv/nYPuGF07CDNgYFSrGs4Wilr6SM0QvVirBv6YClRZn4PfM30+6uFRxVR95hVeJhYUnSQbvYcglrAv/QFHmhjFEl+6Yrxn+wDMAvF2Fzk7m3AROuv2BTdzG28Xmgk0/Qj0eJyQA54L5k2mIMghuWuG/FeX9DCqtH78a81vxMCmP6vFcR82n43r2IaefHNafUV4p44BanCgEDFk2ioFAAVpwXvF9CejKDJ6QLGT62zY7lnO/yInXSxV/7HZDv2vEC2xzTE4aFlsEDNuYVPPQ8TiJa6j9WjVzqYzu791qb4XEHY2o83Dtb7h6IKcRWSJBmWvGa4a8rv your_email@example.com
diff --git a/war/pom.xml b/war/pom.xml
@@ -134,7 +134,7 @@ THE SOFTWARE.
     <dependency>
       <groupId>org.jenkins-ci.modules</groupId>
       <artifactId>sshd</artifactId>
-      <version>1.11-20170315.153852-1</version> <!-- TODO https://github.com/jenkinsci/sshd-module/pull/10 -->
+      <version>1.11-20170324.200647-2</version> <!-- TODO https://github.com/jenkinsci/sshd-module/pull/11 -->
     </dependency>
     <dependency>
       <groupId>org.jenkins-ci.ui</groupId>