Fix: if the client knows CA key, it should send host key algo proposal for certificates (#733)

* Fix: if the client knows CA key, it should send host key algo proposal for certificates

* Run specific SSH server in KeyWithCertificateSpec

Required to verify the case with wrong host key algorithm proposals. See #733

* Split KeyWithCertificateSpec into HostKeyWithCertificateSpec and PublicKeyAuthWithCertificateSpec

Prevents from starting unnecessary SSHD containers, making the tests run a bit faster when they are launched separately.

src/itest/docker-image/test-container/sshd_config

@@ -1,158 +0,0 @@
-#	$OpenBSD: sshd_config,v 1.101 2017/03/14 07:19:07 djm Exp $
-
-# This is the sshd server system-wide configuration file.  See
-# sshd_config(5) for more information.
-
-# This sshd was compiled with PATH=/bin:/usr/bin:/sbin:/usr/sbin
-
-# The strategy used for options in the default sshd_config shipped with
-# OpenSSH is to specify options with their default value where
-# possible, but leave them commented.  Uncommented options override the
-# default value.
-
-#Port 22
-#AddressFamily any
-#ListenAddress 0.0.0.0
-#ListenAddress ::
-
-#HostKey /etc/ssh/ssh_host_rsa_key
-#HostKey /etc/ssh/ssh_host_dsa_key
-#HostKey /etc/ssh/ssh_host_ecdsa_key
-#HostKey /etc/ssh/ssh_host_ed25519_key
-
-# Ciphers and keying
-#RekeyLimit default none
-
-# Logging
-#SyslogFacility AUTH
-#LogLevel INFO
-
-# Authentication:
-
-#LoginGraceTime 2m
-PermitRootLogin yes
-#StrictModes yes
-#MaxAuthTries 6
-#MaxSessions 10
-
-#PubkeyAuthentication yes
-
-# The default is to check both .ssh/authorized_keys and .ssh/authorized_keys2
-# but this is overridden so installations will only check .ssh/authorized_keys
-AuthorizedKeysFile	.ssh/authorized_keys
-
-#AuthorizedPrincipalsFile none
-
-#AuthorizedKeysCommand none
-#AuthorizedKeysCommandUser nobody
-
-# For this to work you will also need host keys in /etc/ssh/ssh_known_hosts
-#HostbasedAuthentication no
-# Change to yes if you don't trust ~/.ssh/known_hosts for
-# HostbasedAuthentication
-#IgnoreUserKnownHosts no
-# Don't read the user's ~/.rhosts and ~/.shosts files
-#IgnoreRhosts yes
-
-# To disable tunneled clear text passwords, change to no here!
-#PasswordAuthentication yes
-#PermitEmptyPasswords no
-
-# Change to no to disable s/key passwords
-#ChallengeResponseAuthentication yes
-
-# Kerberos options
-#KerberosAuthentication no
-#KerberosOrLocalPasswd yes
-#KerberosTicketCleanup yes
-#KerberosGetAFSToken no
-
-# GSSAPI options
-#GSSAPIAuthentication no
-#GSSAPICleanupCredentials yes
-
-# Set this to 'yes' to enable PAM authentication, account processing,
-# and session processing. If this is enabled, PAM authentication will
-# be allowed through the ChallengeResponseAuthentication and
-# PasswordAuthentication.  Depending on your PAM configuration,
-# PAM authentication via ChallengeResponseAuthentication may bypass
-# the setting of "PermitRootLogin without-password".
-# If you just want the PAM account and session checks to run without
-# PAM authentication, then enable this but set PasswordAuthentication
-# and ChallengeResponseAuthentication to 'no'.
-#UsePAM no
-
-#AllowAgentForwarding yes
-#AllowTcpForwarding yes
-#GatewayPorts no
-#X11Forwarding no
-#X11DisplayOffset 10
-#X11UseLocalhost yes
-#PermitTTY yes
-#PrintMotd yes
-#PrintLastLog yes
-#TCPKeepAlive yes
-#UseLogin no
-#PermitUserEnvironment no
-#Compression delayed
-#ClientAliveInterval 0
-#ClientAliveCountMax 3
-#UseDNS no
-#PidFile /run/sshd.pid
-#MaxStartups 10:30:100
-#PermitTunnel no
-#ChrootDirectory none
-#VersionAddendum none
-
-# no default banner path
-#Banner none
-
-# override default of no subsystems
-Subsystem	sftp	/usr/lib/ssh/sftp-server
-
-# the following are HPN related configuration options
-# tcp receive buffer polling. disable in non autotuning kernels
-#TcpRcvBufPoll yes
-
-# disable hpn performance boosts
-#HPNDisabled no
-
-# buffer size for hpn to non-hpn connections
-#HPNBufferSize 2048
-
-
-# Example of overriding settings on a per-user basis
-#Match User anoncvs
-#	X11Forwarding no
-#	AllowTcpForwarding no
-#	PermitTTY no
-#	ForceCommand cvs server
-
-KexAlgorithms curve25519-sha256,curve25519-sha256@libssh.org,ecdh-sha2-nistp256,ecdh-sha2-nistp384,ecdh-sha2-nistp521,diffie-hellman-group-exchange-sha256,diffie-hellman-group16-sha512,diffie-hellman-group18-sha512,diffie-hellman-group14-sha256,diffie-hellman-group14-sha1,diffie-hellman-group1-sha1,diffie-hellman-group-exchange-sha1
-macs umac-64-etm@openssh.com,umac-128-etm@openssh.com,hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-ripemd160-etm@openssh.com,umac-64@openssh.com,umac-128@openssh.com,hmac-sha2-256,hmac-sha2-512,hmac-ripemd160,hmac-ripemd160@openssh.com
-
-TrustedUserCAKeys /etc/ssh/trusted_ca_keys
-
-Ciphers 3des-cbc,blowfish-cbc,aes128-cbc,aes192-cbc,aes256-cbc,aes128-ctr,aes192-ctr,aes256-ctr,aes128-gcm@openssh.com,aes256-gcm@openssh.com,chacha20-poly1305@openssh.com
-
-HostKey /etc/ssh/ssh_host_rsa_key
-HostKey /etc/ssh/ssh_host_dsa_key
-HostKey /etc/ssh/ssh_host_ecdsa_key
-HostKey /etc/ssh/ssh_host_ed25519_key
-
-HostKey /etc/ssh/ssh_host_ecdsa_256_key
-HostCertificate /etc/ssh/ssh_host_ecdsa_256_key-cert.pub
-
-HostKey /etc/ssh/ssh_host_ecdsa_384_key
-HostCertificate /etc/ssh/ssh_host_ecdsa_384_key-cert.pub
-
-HostKey /etc/ssh/ssh_host_ecdsa_521_key
-HostCertificate /etc/ssh/ssh_host_ecdsa_521_key-cert.pub
-
-HostKey /etc/ssh/ssh_host_ed25519_384_key
-HostCertificate /etc/ssh/ssh_host_ed25519_384_key-cert.pub
-
-HostKey /etc/ssh/ssh_host_rsa_2048_key
-HostCertificate /etc/ssh/ssh_host_rsa_2048_key-cert.pub
-
-LogLevel DEBUG2

src/itest/groovy/com/hierynomus/sshj/SshdContainer.java

@@ -32,18 +32,30 @@ import java.util.concurrent.Future;
  * A JUnit4 rule for launching a generic SSH server container.
  */
 public class SshdContainer extends GenericContainer<SshdContainer> {
-    @SuppressWarnings("unused")  // Used dynamically by Spock
+    public static class Builder {
-    public SshdContainer() {
+        public static final String DEFAULT_SSHD_CONFIG = "" +
-        this(new ImageFromDockerfile()
+                "PermitRootLogin yes\n" +
-                .withDockerfileFromBuilder(SshdContainer::defaultDockerfileBuilder)
+                "AuthorizedKeysFile .ssh/authorized_keys\n" +
-                .withFileFromPath(".", Paths.get("src/itest/docker-image")));
+                "Subsystem sftp /usr/lib/ssh/sftp-server\n" +
-    }
+                "KexAlgorithms curve25519-sha256,curve25519-sha256@libssh.org,ecdh-sha2-nistp256,ecdh-sha2-nistp384,ecdh-sha2-nistp521,diffie-hellman-group-exchange-sha256,diffie-hellman-group16-sha512,diffie-hellman-group18-sha512,diffie-hellman-group14-sha256,diffie-hellman-group14-sha1,diffie-hellman-group1-sha1,diffie-hellman-group-exchange-sha1\n" +
-
+                "macs umac-64-etm@openssh.com,umac-128-etm@openssh.com,hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-ripemd160-etm@openssh.com,umac-64@openssh.com,umac-128@openssh.com,hmac-sha2-256,hmac-sha2-512,hmac-ripemd160,hmac-ripemd160@openssh.com\n" +
-    public SshdContainer(@NotNull Future<String> future) {
+                "TrustedUserCAKeys /etc/ssh/trusted_ca_keys\n" +
-        super(future);
+                "Ciphers 3des-cbc,blowfish-cbc,aes128-cbc,aes192-cbc,aes256-cbc,aes128-ctr,aes192-ctr,aes256-ctr,aes128-gcm@openssh.com,aes256-gcm@openssh.com,chacha20-poly1305@openssh.com\n" +
-        withExposedPorts(22);
+                "HostKey /etc/ssh/ssh_host_rsa_key\n" +
-        setWaitStrategy(new SshServerWaitStrategy());
+                "HostKey /etc/ssh/ssh_host_dsa_key\n" +
-    }
+                "HostKey /etc/ssh/ssh_host_ecdsa_key\n" +
+                "HostKey /etc/ssh/ssh_host_ed25519_key\n" +
+                "HostKey /etc/ssh/ssh_host_ecdsa_256_key\n" +
+                "HostCertificate /etc/ssh/ssh_host_ecdsa_256_key-cert.pub\n" +
+                "HostKey /etc/ssh/ssh_host_ecdsa_384_key\n" +
+                "HostCertificate /etc/ssh/ssh_host_ecdsa_384_key-cert.pub\n" +
+                "HostKey /etc/ssh/ssh_host_ecdsa_521_key\n" +
+                "HostCertificate /etc/ssh/ssh_host_ecdsa_521_key-cert.pub\n" +
+                "HostKey /etc/ssh/ssh_host_ed25519_384_key\n" +
+                "HostCertificate /etc/ssh/ssh_host_ed25519_384_key-cert.pub\n" +
+                "HostKey /etc/ssh/ssh_host_rsa_2048_key\n" +
+                "HostCertificate /etc/ssh/ssh_host_rsa_2048_key-cert.pub\n" +
+                "LogLevel DEBUG2\n";
 
         public static void defaultDockerfileBuilder(@NotNull DockerfileBuilder builder) {
             builder.from("sickp/alpine-sshd:7.5-r2");
@@ -54,7 +66,6 @@ public class SshdContainer extends GenericContainer<SshdContainer> {
             builder.add("test-container/ssh_host_ecdsa_key.pub", "/etc/ssh/ssh_host_ecdsa_key.pub");
             builder.add("test-container/ssh_host_ed25519_key", "/etc/ssh/ssh_host_ed25519_key");
             builder.add("test-container/ssh_host_ed25519_key.pub", "/etc/ssh/ssh_host_ed25519_key.pub");
-        builder.add("test-container/sshd_config", "/etc/ssh/sshd_config");
             builder.copy("test-container/trusted_ca_keys", "/etc/ssh/trusted_ca_keys");
             builder.copy("test-container/host_keys/*", "/etc/ssh/");
 
@@ -68,6 +79,38 @@ public class SshdContainer extends GenericContainer<SshdContainer> {
                     + " && chmod 644 /etc/ssh/*.pub"
                     + " && chown -R sshj:sshj /home/sshj");
             builder.entryPoint("/sbin/tini", "/entrypoint.sh", "-o", "LogLevel=DEBUG2");
+
+            builder.add("sshd_config", "/etc/ssh/sshd_config");
+        }
+
+        private @NotNull String sshdConfig = DEFAULT_SSHD_CONFIG;
+
+        public @NotNull Builder withSshdConfig(@NotNull String sshdConfig) {
+            this.sshdConfig = sshdConfig;
+            return this;
+        }
+
+        public @NotNull SshdContainer build() {
+            return new SshdContainer(buildInner());
+        }
+
+        private @NotNull Future<String> buildInner() {
+            return new ImageFromDockerfile()
+                    .withDockerfileFromBuilder(Builder::defaultDockerfileBuilder)
+                    .withFileFromPath(".", Paths.get("src/itest/docker-image"))
+                    .withFileFromString("sshd_config", sshdConfig);
+        }
+    }
+
+    @SuppressWarnings("unused")  // Used dynamically by Spock
+    public SshdContainer() {
+        this(new SshdContainer.Builder().buildInner());
+    }
+
+    public SshdContainer(@NotNull Future<String> future) {
+        super(future);
+        withExposedPorts(22);
+        setWaitStrategy(new SshServerWaitStrategy());
     }
 
     public SSHClient getConnectedClient(Config config) throws IOException {

src/itest/groovy/com/hierynomus/sshj/signature/HostKeyWithCertificateSpec.groovy

@@ -0,0 +1,83 @@
+/*
+ * Copyright (C)2009 - SSHJ Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.hierynomus.sshj.signature
+
+import com.hierynomus.sshj.SshdContainer
+import net.schmizz.sshj.DefaultConfig
+import net.schmizz.sshj.SSHClient
+import net.schmizz.sshj.transport.verification.OpenSSHKnownHosts
+import spock.lang.Specification
+import spock.lang.Unroll
+
+import java.nio.file.Files
+
+/**
+ * This is a brief test for verifying connection to a server using keys with certificates.
+ *
+ * Also, take a look at the unit test {@link net.schmizz.sshj.transport.verification.KeyWithCertificateUnitSpec}.
+ */
+class HostKeyWithCertificateSpec extends Specification {
+    @Unroll
+    def "accepting a signed host public key #hostKey"() {
+        given:
+        SshdContainer sshd = new SshdContainer.Builder()
+            .withSshdConfig("""
+                PasswordAuthentication yes
+                HostKey /etc/ssh/$hostKey
+                HostCertificate /etc/ssh/${hostKey}-cert.pub
+                """.stripMargin())
+            .build()
+        sshd.start()
+
+        and:
+        File knownHosts = Files.createTempFile("known_hosts", "").toFile()
+        knownHosts.deleteOnExit()
+
+        and:
+        File caPubKey = new File("src/itest/resources/keyfiles/certificates/CA_rsa.pem.pub")
+        def address = "127.0.0.1"
+        String knownHostsFileContents = "" +
+                "@cert-authority ${ address} ${caPubKey.text}" +
+                "\n@cert-authority [${address}]:${sshd.firstMappedPort} ${caPubKey.text}"
+        knownHosts.write(knownHostsFileContents)
+
+        and:
+        SSHClient sshClient = new SSHClient(new DefaultConfig())
+        sshClient.addHostKeyVerifier(new OpenSSHKnownHosts(knownHosts))
+        sshClient.connect(address, sshd.firstMappedPort)
+
+        when:
+        sshClient.authPassword("sshj", "ultrapassword")
+
+        then:
+        sshClient.authenticated
+
+        and:
+        knownHosts.getText() == knownHostsFileContents
+
+        cleanup:
+        sshd.stop()
+
+        where:
+        hostKey << [
+                "ssh_host_ecdsa_256_key",
+                "ssh_host_ecdsa_384_key",
+                "ssh_host_ecdsa_521_key",
+                "ssh_host_ed25519_384_key",
+                "ssh_host_rsa_2048_key",
+        ]
+    }
+}

src/itest/groovy/com/hierynomus/sshj/signature/PublicKeyAuthWithCertificateSpec.groovy

@@ -18,22 +18,18 @@ package com.hierynomus.sshj.signature
 import com.hierynomus.sshj.SshdContainer
 import net.schmizz.sshj.DefaultConfig
 import net.schmizz.sshj.SSHClient
-import net.schmizz.sshj.transport.verification.OpenSSHKnownHosts
 import net.schmizz.sshj.transport.verification.PromiscuousVerifier
 import org.junit.ClassRule
 import spock.lang.Shared
 import spock.lang.Specification
 import spock.lang.Unroll
 
-import java.nio.file.Files
-import java.util.stream.Collectors
-
 /**
  * This is a brief test for verifying connection to a server using keys with certificates.
  *
  * Also, take a look at the unit test {@link net.schmizz.sshj.transport.verification.KeyWithCertificateUnitSpec}.
  */
-class KeyWithCertificateSpec extends Specification {
+class PublicKeyAuthWithCertificateSpec extends Specification {
     @Shared
     @ClassRule
     SshdContainer sshd
@@ -82,44 +78,4 @@ class KeyWithCertificateSpec extends Specification {
                 "id_ed25519_384_rfc4716_signed_by_rsa",
         ]
     }
-
-    @Unroll
-    def "accepting a signed host public key with type #hostKeyAlgo"() {
-        given:
-        File knownHosts = Files.createTempFile("known_hosts", "").toFile()
-        knownHosts.deleteOnExit()
-
-        and:
-        File caPubKey = new File("src/itest/resources/keyfiles/certificates/CA_rsa.pem.pub")
-        def address = "127.0.0.1"
-        String knownHostsFileContents = "" +
-                "@cert-authority ${ address} ${caPubKey.text}" +
-                "\n@cert-authority [${address}]:${sshd.firstMappedPort} ${caPubKey.text}"
-        knownHosts.write(knownHostsFileContents)
-
-        and:
-        def config = new DefaultConfig()
-        config.keyAlgorithms = config.keyAlgorithms.stream()
-                .filter { it.name == hostKeyAlgo }
-                .collect(Collectors.toList())
-        SSHClient sshClient = new SSHClient(config)
-        sshClient.addHostKeyVerifier(new OpenSSHKnownHosts(knownHosts))
-        sshClient.connect(address, sshd.firstMappedPort)
-
-        when:
-        sshClient.authPassword("sshj", "ultrapassword")
-
-        then:
-        sshClient.authenticated
-
-        and:
-        knownHosts.getText() == knownHostsFileContents
-
-        where:
-        hostKeyAlgo << [
-                "ecdsa-sha2-nistp256-cert-v01@openssh.com",
-                "ssh-ed25519-cert-v01@openssh.com",
-                "ssh-rsa-cert-v01@openssh.com",
-        ]
-    }
 }

src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java

@@ -138,7 +138,19 @@ public class OpenSSHKnownHosts
         for (KnownHostEntry e : entries) {
             try {
                 if (e.appliesTo(adjustedHostname)) {
-                    knownHostAlgorithms.add(e.getType().toString());
+                    final KeyType type = e.getType();
+                    if (e instanceof HostEntry && ((HostEntry) e).marker == Marker.CA_CERT) {
+                        // Only the CA key type is known, but the type of the host key is not.
+                        // Adding all supported types for keys with certificates.
+                        for (final KeyType candidate : KeyType.values()) {
+                            if (candidate.getParent() != null) {
+                                knownHostAlgorithms.add(candidate.toString());
+                            }
+                        }
+                    }
+                    else {
+                        knownHostAlgorithms.add(type.toString());
+                    }
                 }
             } catch (IOException ioe) {
             }