diff --git a/README.adoc b/README.adoc index fa000eab..b6ac70f3 100644 --- a/README.adoc +++ b/README.adoc @@ -107,6 +107,9 @@ Google Group: http://groups.google.com/group/sshj-users Fork away! == Release history +SSHJ 0.24.0 (2018-??-??):: +* Added support for hmac-ripemd160 + SSHJ 0.23.0 (2017-10-13):: * Merged https://github.com/hierynomus/sshj/pulls/372[#372]: Upgrade to 'net.i2p.crypto:eddsa:0.2.0' * Fixed https://github.com/hierynomus/sshj/issues/355[#355] and https://github.com/hierynomus/sshj/issues/354[#354]: Correctly decode signature bytes diff --git a/src/main/java/net/schmizz/sshj/common/Buffer.java b/src/main/java/net/schmizz/sshj/common/Buffer.java index fe9805f6..7c420fcb 100644 --- a/src/main/java/net/schmizz/sshj/common/Buffer.java +++ b/src/main/java/net/schmizz/sshj/common/Buffer.java @@ -460,10 +460,13 @@ public class Buffer> { public PublicKey readPublicKey() throws BufferException { + KeyType keyType = KeyType.fromString(readString()); try { - return KeyType.fromString(readString()).readPubKeyFromBuffer(this); + return keyType.readPubKeyFromBuffer(this); } catch (GeneralSecurityException e) { throw new SSHRuntimeException(e); + } catch (UnsupportedOperationException uoe) { + throw new BufferException("Could not decode keytype " + keyType); } } diff --git a/src/main/java/net/schmizz/sshj/transport/verification/ConsoleKnownHostsVerifier.java b/src/main/java/net/schmizz/sshj/transport/verification/ConsoleKnownHostsVerifier.java index f8aad9e7..4fbe3b2f 100644 --- a/src/main/java/net/schmizz/sshj/transport/verification/ConsoleKnownHostsVerifier.java +++ b/src/main/java/net/schmizz/sshj/transport/verification/ConsoleKnownHostsVerifier.java @@ -41,7 +41,7 @@ public class ConsoleKnownHostsVerifier protected boolean hostKeyUnverifiableAction(String hostname, PublicKey key) { final KeyType type = KeyType.fromKey(key); console.printf("The authenticity of host '%s' can't be established.\n" + - "%s key fingerprint is %s.\n", hostname, type, SecurityUtils.getFingerprint(key)); + "%s key fingerprint is %s.\n", hostname, type, SecurityUtils.getFingerprint(key)); String response = console.readLine("Are you sure you want to continue connecting (yes/no)? "); while (!(response.equalsIgnoreCase(YES) || response.equalsIgnoreCase(NO))) { response = console.readLine("Please explicitly enter yes/no: "); @@ -60,7 +60,7 @@ public class ConsoleKnownHostsVerifier } @Override - protected boolean hostKeyChangedAction(KnownHostEntry entry, String hostname, PublicKey key) { + protected boolean hostKeyChangedAction(String hostname, PublicKey key) { final KeyType type = KeyType.fromKey(key); final String fp = SecurityUtils.getFingerprint(key); final String path = getFile().getAbsolutePath(); diff --git a/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java b/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java index 473d4832..18355e1c 100644 --- a/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java +++ b/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java @@ -87,14 +87,23 @@ public class OpenSSHKnownHosts final String adjustedHostname = (port != 22) ? "[" + hostname + "]:" + port : hostname; + boolean foundApplicableHostEntry = false; for (KnownHostEntry e : entries) { try { - if (e.appliesTo(type, adjustedHostname)) - return e.verify(key) || hostKeyChangedAction(e, adjustedHostname, key); + if (e.appliesTo(type, adjustedHostname)) { + foundApplicableHostEntry = true; + if (e.verify(key)) { + return true; + } + } } catch (IOException ioe) { log.error("Error with {}: {}", e, ioe); return false; } + + } + if (foundApplicableHostEntry) { + return hostKeyChangedAction(adjustedHostname, key); } return hostKeyUnverifiableAction(adjustedHostname, key); @@ -104,7 +113,7 @@ public class OpenSSHKnownHosts return false; } - protected boolean hostKeyChangedAction(KnownHostEntry entry, String hostname, PublicKey key) { + protected boolean hostKeyChangedAction(String hostname, PublicKey key) { log.warn("Host key for `{}` has changed!", hostname); return false; } @@ -199,7 +208,7 @@ public class OpenSSHKnownHosts } if(split.length < 3) { log.error("Error reading entry `{}`", line); - return null; + return new BadHostEntry(line); } final String hostnames = split[i++]; final String sType = split[i++]; @@ -209,7 +218,13 @@ public class OpenSSHKnownHosts if (type != KeyType.UNKNOWN) { final String sKey = split[i++]; - key = new Buffer.PlainBuffer(Base64.decode(sKey)).readPublicKey(); + try { + byte[] keyBytes = Base64.decode(sKey); + key = new Buffer.PlainBuffer(keyBytes).readPublicKey(); + } catch (IOException ioe) { + log.warn("Error decoding Base64 key bytes", ioe); + return new BadHostEntry(line); + } } else if (isBits(sType)) { type = KeyType.RSA; // int bits = Integer.valueOf(sType); @@ -220,11 +235,11 @@ public class OpenSSHKnownHosts key = keyFactory.generatePublic(new RSAPublicKeySpec(n, e)); } catch (Exception ex) { log.error("Error reading entry `{}`, could not create key", line, ex); - return null; + return new BadHostEntry(line); } } else { log.error("Error reading entry `{}`, could not determine type", line); - return null; + return new BadHostEntry(line); } return new HostEntry(marker, hostnames, type, key); @@ -364,6 +379,44 @@ public class OpenSSHKnownHosts } } + public static class BadHostEntry implements KnownHostEntry { + private String line; + + public BadHostEntry(String line) { + this.line = line; + } + + @Override + public KeyType getType() { + return KeyType.UNKNOWN; + } + + @Override + public String getFingerprint() { + return null; + } + + @Override + public boolean appliesTo(String host) throws IOException { + return false; + } + + @Override + public boolean appliesTo(KeyType type, String host) throws IOException { + return false; + } + + @Override + public boolean verify(PublicKey key) throws IOException { + return false; + } + + @Override + public String getLine() { + return line; + } + } + public enum Marker { CA_CERT("@cert-authority"), REVOKED("@revoked"); diff --git a/src/test/groovy/com/hierynomus/sshj/transport/verification/OpenSSHKnownHostsSpec.groovy b/src/test/groovy/com/hierynomus/sshj/transport/verification/OpenSSHKnownHostsSpec.groovy new file mode 100644 index 00000000..bf745f30 --- /dev/null +++ b/src/test/groovy/com/hierynomus/sshj/transport/verification/OpenSSHKnownHostsSpec.groovy @@ -0,0 +1,114 @@ +/* + * 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.transport.verification + +import net.schmizz.sshj.common.Base64 +import net.schmizz.sshj.common.Buffer +import net.schmizz.sshj.transport.verification.OpenSSHKnownHosts +import org.junit.Rule +import org.junit.rules.TemporaryFolder +import spock.lang.Specification +import spock.lang.Unroll + +import static org.hamcrest.CoreMatchers.equalTo +import static org.hamcrest.CoreMatchers.instanceOf +import static org.junit.Assert.assertThat +import static org.junit.Assert.assertThat + +class OpenSSHKnownHostsSpec extends Specification { + + @Rule + TemporaryFolder folder + + def "should check all host entries for key"() { + given: + def f = knownHosts(""" +host1 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBCiYp2IDgzDFhl8T4TRLIhEljvEixz1YN0XWh4dYh0REGK9T4QKiyb28EztPMdcOtz1uyX5rUGYXX9hj99S4SiU= +host1 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBLTjA7hduYGmvV9smEEsIdGLdghSPD7kL8QarIIOkeXmBh+LTtT/T1K+Ot/rmXCZsP8hoUXxbvN+Tks440Ci0ck= +""") + def pk = new Buffer.PlainBuffer(Base64.decode("AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBLTjA7hduYGmvV9smEEsIdGLdghSPD7kL8QarIIOkeXmBh+LTtT/T1K+Ot/rmXCZsP8hoUXxbvN+Tks440Ci0ck=")).readPublicKey() + when: + def knownhosts = new OpenSSHKnownHosts(f) + + then: + knownhosts.verify("host1", 22, pk) + } + + def "should not fail on bad base64 entry"() { + given: + def f = knownHosts(""" +host1 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTIDgzDFhl8T4TRLIhEljvEixz1YN0XWh4dYh0REGK9T4QKiyb28EztPMdcOtz1uyX5rUGYXX9hj99S4SiU= +host1 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBLTjA7hduYGmvV9smEEsIdGLdghSPD7kL8QarIIOkeXmBh+LTtT/T1K+Ot/rmXCZsP8hoUXxbvN+Tks440Ci0ck= +""") + def pk = new Buffer.PlainBuffer(Base64.decode("AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBLTjA7hduYGmvV9smEEsIdGLdghSPD7kL8QarIIOkeXmBh+LTtT/T1K+Ot/rmXCZsP8hoUXxbvN+Tks440Ci0ck=")).readPublicKey() + when: + def knownhosts = new OpenSSHKnownHosts(f) + + then: + knownhosts.verify("host1", 22, pk) + } + + def "should mark bad line and not fail"() { + given: + def f = knownHosts("M36Lo+Ik5ukNugvvoNFlpnyiHMmtKxt3FpyEfYuryXjNqMNWHn/ARVnpUIl5jRLTB7WBzyLYMG7X5nuoFL9zYqKGtHxChbDunxMVbspw5WXI9VN+qxcLwmITmpEvI9ApyS/Ox2ZyN7zw==\n") + + when: + def knownhosts = new OpenSSHKnownHosts(f) + + then: + knownhosts.entries().size() == 1 + knownhosts.entries().get(0) instanceof OpenSSHKnownHosts.BadHostEntry + } + + @Unroll + def "should add comment for #type line"() { + given: + def f = knownHosts(s) + + when: + def knownHosts = new OpenSSHKnownHosts(f) + + then: + knownHosts.entries().size() == 1 + knownHosts.entries().get(0) instanceof OpenSSHKnownHosts.CommentEntry + + where: + type << ["empty", "comment"] + s << ["\n", "#comment\n"] + } + + @Unroll + def "should match any host name from multi-host line"() { + given: + def f = knownHosts("schmizz.net,69.163.155.180 ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA6P9Hlwdahh250jGZYKg2snRq2j2lFJVdKSHyxqbJiVy9VX9gTkN3K2MD48qyrYLYOyGs3vTttyUk+cK++JMzURWsrP4piby7LpeOT+3Iq8CQNj4gXZdcH9w15Vuk2qS11at6IsQPVHpKD9HGg9//EFUccI/4w06k4XXLm/IxOGUwj6I2AeWmEOL3aDi+fe07TTosSdLUD6INtR0cyKsg0zC7Da24ixoShT8Oy3x2MpR7CY3PQ1pUVmvPkr79VeA+4qV9F1JM09WdboAMZgWQZ+XrbtuBlGsyhpUHSCQOya+kOJ+bYryS+U7A+6nmTW3C9FX4FgFqTF89UHOC7V0zZQ==") + def pk = new Buffer.PlainBuffer(Base64.decode("AAAAB3NzaC1yc2EAAAABIwAAAQEA6P9Hlwdahh250jGZYKg2snRq2j2lFJVdKSHyxqbJiVy9VX9gTkN3K2MD48qyrYLYOyGs3vTttyUk+cK++JMzURWsrP4piby7LpeOT+3Iq8CQNj4gXZdcH9w15Vuk2qS11at6IsQPVHpKD9HGg9//EFUccI/4w06k4XXLm/IxOGUwj6I2AeWmEOL3aDi+fe07TTosSdLUD6INtR0cyKsg0zC7Da24ixoShT8Oy3x2MpR7CY3PQ1pUVmvPkr79VeA+4qV9F1JM09WdboAMZgWQZ+XrbtuBlGsyhpUHSCQOya+kOJ+bYryS+U7A+6nmTW3C9FX4FgFqTF89UHOC7V0zZQ==")).readPublicKey() + + when: + def knownHosts = new OpenSSHKnownHosts(f) + + then: + knownHosts.verify(h, 22, pk) + + where: + h << ["schmizz.net", "69.163.155.180"] + } + + def knownHosts(String s) { + def f = folder.newFile("known_hosts") + f.write(s) + return f + } +} diff --git a/src/test/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHostsTest.java b/src/test/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHostsTest.java deleted file mode 100644 index 7bd4a7b4..00000000 --- a/src/test/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHostsTest.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * 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 net.schmizz.sshj.transport.verification; - -import net.schmizz.sshj.util.KeyUtil; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; - -import java.io.BufferedWriter; -import java.io.File; -import java.io.FileWriter; -import java.io.IOException; -import java.security.GeneralSecurityException; -import java.security.PublicKey; - -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.instanceOf; -import static org.junit.Assert.*; - -public class OpenSSHKnownHostsTest { - - @Rule - public TemporaryFolder temp = new TemporaryFolder(); - - public File writeKnownHosts(String line) - throws IOException { - File known_hosts = temp.newFile("known_hosts"); - FileWriter fileWriter = new FileWriter(known_hosts); - BufferedWriter writer = new BufferedWriter(fileWriter); - writer.write(line); - writer.write("\r\n"); - writer.flush(); - writer.close(); - return known_hosts; - } - - @Test - public void shouldAddCommentForEmptyLine() - throws IOException { - File file = writeKnownHosts(""); - OpenSSHKnownHosts openSSHKnownHosts = new OpenSSHKnownHosts(file); - assertThat(openSSHKnownHosts.entries().size(), equalTo(1)); - assertThat(openSSHKnownHosts.entries().get(0), instanceOf(OpenSSHKnownHosts.CommentEntry.class)); - } - - @Test - public void shouldAddCommentForCommentLine() - throws IOException { - File file = writeKnownHosts("# this is a comment"); - OpenSSHKnownHosts openSSHKnownHosts = new OpenSSHKnownHosts(file); - assertThat(openSSHKnownHosts.entries().size(), equalTo(1)); - assertThat(openSSHKnownHosts.entries().get(0), instanceOf(OpenSSHKnownHosts.CommentEntry.class)); - } - - @Test - public void testSchmizzEntry() - throws IOException, GeneralSecurityException { - OpenSSHKnownHosts kh = new OpenSSHKnownHosts(new File("src/test/resources/known_hosts")); - final PublicKey key = KeyUtil - .newRSAPublicKey( - "e8ff4797075a861db9d2319960a836b2746ada3da514955d2921f2c6a6c9895cbd557f604e43772b6303e3cab2ad82d83b21acdef4edb72524f9c2bef893335115acacfe2989bcbb2e978e4fedc8abc090363e205d975c1fdc35e55ba4daa4b5d5ab7a22c40f547a4a0fd1c683dfff10551c708ff8c34ea4e175cb9bf2313865308fa23601e5a610e2f76838be7ded3b4d3a2c49d2d40fa20db51d1cc8ab20d330bb0dadb88b1a12853f0ecb7c7632947b098dcf435a54566bcf92befd55e03ee2a57d17524cd3d59d6e800c66059067e5eb6edb81946b3286950748240ec9afa4389f9b62bc92f94ec0fba9e64d6dc2f455f816016a4c5f3d507382ed5d3365", - "23"); - - assertTrue(kh.verify("schmizz.net", 22, key)); - assertTrue(kh.verify("69.163.155.180", 22, key)); - assertFalse(kh.verify("69.163.155.18", 22, key)); - } - - @Test - public void testVerifyIndexError() throws Exception { - final OpenSSHKnownHosts v = new OpenSSHKnownHosts(new File("src/test/resources/known_hosts.invalid")); - assertTrue(v.entries().isEmpty()); - } -} diff --git a/src/test/resources/known_hosts b/src/test/resources/known_hosts index 32a1df0e..7b345379 100644 --- a/src/test/resources/known_hosts +++ b/src/test/resources/known_hosts @@ -1,4 +1,11 @@ +# Line 1: bogus line, with invalid base64 string +# Line 2: syntactically correct line, but incorrect base64 string +# Line 3: correct line (that should match) Above we have a plain line +# Line 4: hashed line +# Line 5: v1 line +# Comment lines like these should be ignored +schmizz.net,69.163.155.180 ssh-rsa #BOGUS#aC1yc2EAAAABIwAAAQEA6P9Hlwdahh250jGZYKg2snRq2j2lFJVdKSHyxqbJiVy9VX9gTkN3K2MD48qyrYLYOyGs3vTttyUk+cK++JMzURWsrP4piby7LpeOT+3Iq8CQNj4gXZdcH9w15Vuk2qS11at6IsQPVHpKD9HGg9//EFUccI/4w06k4XXLm/IxOGUwj6I2AeWmEOL3aDi+fe07TTosSdLUD6INtR0cyKsg0zC7Da24ixoShT8Oy3x2MpR7CY3PQ1pUVmvPkr79VeA+4qV9F1JM09WdboAMZgWQZ+XrbtuBlGsyhpUHSCQOya+kOJ+bYryS+U7A+6nmTW3C9FX4FgFqTF89UHOC7V0zZQ== +schmizz.net,69.163.155.180 ssh-rsa AAAAAAAAAC1yc2EAAAABIwAAAQEA6P9Hlwdahh250jGZYKg2snRq2j2lFJVdKSHyxqbJiVy9VX9gTkN3K2MD48qyrYLYOyGs3vTttyUk+cK++JMzURWsrP4piby7LpeOT+3Iq8CQNj4gXZdcH9w15Vuk2qS11at6IsQPVHpKD9HGg9//EFUccI/4w06k4XXLm/IxOGUwj6I2AeWmEOL3aDi+fe07TTosSdLUD6INtR0cyKsg0zC7Da24ixoShT8Oy3x2MpR7CY3PQ1pUVmvPkr79VeA+4qV9F1JM09WdboAMZgWQZ+XrbtuBlGsyhpUHSCQOya+kOJ+bYryS+U7A+6nmTW3C9FX4FgFqTF89UHOC7V0zZQ== schmizz.net,69.163.155.180 ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA6P9Hlwdahh250jGZYKg2snRq2j2lFJVdKSHyxqbJiVy9VX9gTkN3K2MD48qyrYLYOyGs3vTttyUk+cK++JMzURWsrP4piby7LpeOT+3Iq8CQNj4gXZdcH9w15Vuk2qS11at6IsQPVHpKD9HGg9//EFUccI/4w06k4XXLm/IxOGUwj6I2AeWmEOL3aDi+fe07TTosSdLUD6INtR0cyKsg0zC7Da24ixoShT8Oy3x2MpR7CY3PQ1pUVmvPkr79VeA+4qV9F1JM09WdboAMZgWQZ+XrbtuBlGsyhpUHSCQOya+kOJ+bYryS+U7A+6nmTW3C9FX4FgFqTF89UHOC7V0zZQ== -# Above we have a plain line, Below we have a hashed line, Last is a v1 line, This is a garbage line. |1|dy7xSefq6NmJms6AzANG3w45W28=|SSCTlHs4pZbc2uaRoPvjyEAHE1g= ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAu64GJcCkdtckPGt8uKTyhG1ShT1Np1kh10eE49imQ4Nh9Y/IrSPzDtYUAazQ88ABc2NffuOKkdn2qtUwZ1ulfcdNfN3oTim3BiVHqa041pKG0L+onQe8Bo+CaG5KBLy/C24eNGM9EcfQvDQOnq1eD3lnR/l8fFckldzjfxZgar0yT9Bb3pwp50oN+1wSEINJEHOgMIW8kZBQmyNr/B+b7yX+Y1s1vuYIP/i4WimCVmkdi9G87Ga8w7GxKalRD2QOG6Xms2YWRQDN6M/MOn4tda3EKolbWkctEWcQf/PcVJffTH4Wv5f0RjVyrQv4ha4FZcNAv6RkRd9WkiCsiTKioQ== test.com,1.1.1.1 2048 35 22017496617994656680820635966392838863613340434802393112245951008866692373218840197754553998457793202561151141246686162285550121243768846314646395880632789308110750881198697743542374668273149584280424505890648953477691795864456749782348425425954366277600319096366690719901119774784695056100331902394094537054256611668966698242432417382422091372756244612839068092471592121759862971414741954991375710930168229171638843329213652899594987626853020377726482288618521941129157643483558764875338089684351824791983007780922947554898825663693324944982594850256042689880090306493029526546183035567296830604572253312294059766327 diff --git a/src/test/resources/known_hosts.invalid b/src/test/resources/known_hosts.invalid deleted file mode 100644 index edd846e6..00000000 --- a/src/test/resources/known_hosts.invalid +++ /dev/null @@ -1 +0,0 @@ -M36Lo+Ik5ukNugvvoNFlpnyiHMmtKxt3FpyEfYuryXjNqMNWHn/ARVnpUIl5jRLTB7WBzyLYMG7X5nuoFL9zYqKGtHxChbDunxMVbspw5WXI9VN+qxcLwmITmpEvI9ApyS/Ox2ZyN7zw==