Prepped for v0.17.2 release

This ensures that any header lines sent before the identification
string do not break the identification parsing if they are longer
than the identification string should be.

README.adoc

@@ -1,7 +1,7 @@
 = sshj - SSHv2 library for Java
 Jeroen van Erp
 :sshj_groupid: com.hierynomus
-:sshj_version: 0.16.0
+:sshj_version: 0.17.2
 :source-highlighter: pygments
 
 image:https://travis-ci.org/hierynomus/sshj.svg?branch=master[link="https://travis-ci.org/hierynomus/sshj"]
@@ -98,6 +98,8 @@ Google Group: http://groups.google.com/group/sshj-users
 Fork away!
 
 == Release history
+SSHJ 0.17.1 (2016-07-06)::
+* Improved parsing of the SSH Server identification. Too long header lines now no longer break the protocol.
 SSHJ 0.17.0 (2016-07-05)::
 * *Introduced breaking change in SFTP copy behaviour*: Previously an SFTP copy operation would behave differently if both source and target were folders with different names.
   In this case instead of copying the contents of the source into the target directory, the directory itself was copied as a sub directory of the target directory.

build.gradle

@@ -44,6 +44,7 @@ test {
         exceptionFormat = 'full'
     }
     include "**/*Test.*"
+    include "**/*Spec.*"
     if (!project.hasProperty("allTests")) {
         useJUnit {
             excludeCategories 'com.hierynomus.sshj.test.SlowTests'

src/main/java/com/hierynomus/sshj/transport/IdentificationStringParser.java

@@ -0,0 +1,95 @@
+/*
+ * 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;
+
+import net.schmizz.sshj.common.Buffer;
+import net.schmizz.sshj.common.ByteArrayUtils;
+import net.schmizz.sshj.transport.TransportException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Arrays;
+
+public class IdentificationStringParser {
+    private static final Logger logger = LoggerFactory.getLogger(IdentificationStringParser.class);
+    private final Buffer.PlainBuffer buffer;
+
+    private byte[] EXPECTED_START_BYTES = new byte[] {'S', 'S', 'H', '-'};
+
+    public IdentificationStringParser(Buffer.PlainBuffer buffer) {
+        this.buffer = buffer;
+    }
+
+    public String parseIdentificationString() throws IOException {
+        for (;;) {
+            Buffer.PlainBuffer lineBuffer = new Buffer.PlainBuffer();
+            int lineStartPos = buffer.rpos();
+            for (;;) {
+                if (buffer.available() == 0) {
+                    buffer.rpos(lineStartPos);
+                    return "";
+                }
+                byte b = buffer.readByte();
+                lineBuffer.putByte(b);
+                if (b == '\n') {
+                    if (checkForIdentification(lineBuffer)) {
+                        return readIdentification(lineBuffer);
+                    } else {
+                        logHeaderLine(lineBuffer);
+                    }
+                    break;
+                }
+            }
+        }
+    }
+
+    private void logHeaderLine(Buffer.PlainBuffer lineBuffer) {
+
+    }
+
+    private String readIdentification(Buffer.PlainBuffer lineBuffer) throws Buffer.BufferException, TransportException {
+        byte[] bytes = new byte[lineBuffer.available()];
+        lineBuffer.readRawBytes(bytes);
+        if (bytes.length > 255) {
+            logger.error("Incorrect identification String received, line was longer than expected: {}", new String(bytes));
+            logger.error("Just for good measure, bytes were: {}", ByteArrayUtils.printHex(bytes, 0, bytes.length));
+            throw new TransportException("Incorrect identification: line too long: " + ByteArrayUtils.printHex(bytes, 0, bytes.length));
+        }
+        if (bytes[bytes.length - 2] != '\r') {
+            String ident = new String(bytes, 0, bytes.length - 1);
+            logger.warn("Server identification has bad line ending, was expecting a '\\r\\n' however got: '{}' (hex: {})", (char) (bytes[bytes.length - 2] & 0xFF), Integer.toHexString(bytes[bytes.length - 2] & 0xFF));
+            logger.warn("Will treat the identification of this server '{}' leniently", ident);
+            return ident;
+            // logger.error("Data received up til here was: {}", new String(bytes));
+            // throw new TransportException("Incorrect identification: bad line ending: " + ByteArrayUtils.toHex(bytes, 0, bytes.length));
+        }
+
+        // Strip off the \r\n
+        return new String(bytes, 0, bytes.length - 2);
+    }
+
+    private boolean checkForIdentification(Buffer.PlainBuffer lineBuffer) throws Buffer.BufferException {
+        if (lineBuffer.available() < 4) {
+            return false;
+        }
+        byte[] buf = new byte[4];
+        lineBuffer.readRawBytes(buf);
+        // Reset
+        lineBuffer.rpos(0);
+        return Arrays.equals(EXPECTED_START_BYTES, buf);
+    }
+}

src/main/java/net/schmizz/sshj/DefaultConfig.java

@@ -84,7 +84,7 @@ public class DefaultConfig
 
     private final Logger log = LoggerFactory.getLogger(getClass());
 
-    private static final String VERSION = "SSHJ_0_16_0";
+    private static final String VERSION = "SSHJ_0_17_2";
 
     public DefaultConfig() {
         setVersion(VERSION);

src/main/java/net/schmizz/sshj/transport/TransportImpl.java

@@ -15,18 +15,14 @@
  */
 package net.schmizz.sshj.transport;
 
+import com.hierynomus.sshj.transport.IdentificationStringParser;
 import net.schmizz.concurrent.ErrorDeliveryUtil;
 import net.schmizz.concurrent.Event;
 import net.schmizz.sshj.AbstractService;
 import net.schmizz.sshj.Config;
 import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.Service;
-import net.schmizz.sshj.common.Buffer;
-import net.schmizz.sshj.common.DisconnectReason;
-import net.schmizz.sshj.common.IOUtils;
-import net.schmizz.sshj.common.Message;
-import net.schmizz.sshj.common.SSHException;
-import net.schmizz.sshj.common.SSHPacket;
+import net.schmizz.sshj.common.*;
 import net.schmizz.sshj.transport.verification.AlgorithmsVerifier;
 import net.schmizz.sshj.transport.verification.HostKeyVerifier;
 import org.slf4j.Logger;
@@ -207,38 +203,47 @@ public final class TransportImpl
      */
     private String readIdentification(Buffer.PlainBuffer buffer)
             throws IOException {
-        String ident;
-
-        byte[] data = new byte[256];
-        for (; ; ) {
-            int savedBufPos = buffer.rpos();
-            int pos = 0;
-            boolean needLF = false;
-            for (; ; ) {
-                if (buffer.available() == 0) {
-                    // Need more data, so undo reading and return null
-                    buffer.rpos(savedBufPos);
-                    return "";
-                }
-                byte b = buffer.readByte();
-                if (b == '\r') {
-                    needLF = true;
-                    continue;
-                }
-                if (b == '\n')
-                    break;
-                if (needLF)
-                    throw new TransportException("Incorrect identification: bad line ending");
-                if (pos >= data.length)
-                    throw new TransportException("Incorrect identification: line too long");
-                data[pos++] = b;
-            }
-            ident = new String(data, 0, pos);
-            if (ident.startsWith("SSH-"))
-                break;
-            if (buffer.rpos() > 16 * 1024)
-                throw new TransportException("Incorrect identification: too many header lines");
+        String ident = new IdentificationStringParser(buffer).parseIdentificationString();
+        if (ident.isEmpty()) {
+            return ident;
         }
+//
+//        byte[] data = new byte[256];
+//        for (; ; ) {
+//            int savedBufPos = buffer.rpos();
+//            int pos = 0;
+//            boolean needLF = false;
+//            for (; ; ) {
+//                if (buffer.available() == 0) {
+//                    // Need more data, so undo reading and return null
+//                    buffer.rpos(savedBufPos);
+//                    return "";
+//                }
+//                byte b = buffer.readByte();
+//                if (b == '\r') {
+//                    needLF = true;
+//                    continue;
+//                }
+//                if (b == '\n')
+//                    break;
+//                if (needLF) {
+//                    log.error("Incorrect identification, was expecting a '\n' after the '\r', got: '{}' (hex: {})", b, Integer.toHexString(b & 0xFF));
+//                    log.error("Data received up til here was: {}", new String(data, 0, pos));
+//                    throw new TransportException("Incorrect identification: bad line ending: " + ByteArrayUtils.toHex(data, 0, pos));
+//                }
+//                if (pos >= data.length) {
+//                    log.error("Incorrect identification String received, line was longer than expected: {}", new String(data, 0, pos));
+//                    log.error("Just for good measure, bytes were: {}", ByteArrayUtils.printHex(data, 0, pos));
+//                    throw new TransportException("Incorrect identification: line too long: " + ByteArrayUtils.printHex(data, 0, pos));
+//                }
+//                data[pos++] = b;
+//            }
+//            ident = new String(data, 0, pos);
+//            if (ident.startsWith("SSH-"))
+//                break;
+//            if (buffer.rpos() > 16 * 1024)
+//                throw new TransportException("Incorrect identification: too many header lines");
+//        }
 
         if (!ident.startsWith("SSH-2.0-") && !ident.startsWith("SSH-1.99-"))
             throw new TransportException(DisconnectReason.PROTOCOL_VERSION_NOT_SUPPORTED,

src/test/groovy/com/hierynomus/sshj/sftp/SFTPClientSpec.groovy

@@ -26,7 +26,7 @@ import spock.lang.Unroll
 
 import static org.codehaus.groovy.runtime.IOGroovyMethods.withCloseable
 
-class SFTPClientTest extends Specification {
+class SFTPClientSpec extends Specification {
 
     @Rule
     public SshFixture fixture = new SshFixture()

src/test/groovy/com/hierynomus/sshj/transport/IdentificationStringParserSpec.groovy

@@ -0,0 +1,103 @@
+/*
+ * 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
+
+import net.schmizz.sshj.common.Buffer
+import net.schmizz.sshj.transport.TransportException
+import spock.lang.Specification
+
+class IdentificationStringParserSpec extends Specification {
+
+    def "should parse simple identification string"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("SSH-2.0-OpenSSH-6.13\r\n".bytes)
+
+        when:
+        def ident = new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        ident == "SSH-2.0-OpenSSH-6.13"
+    }
+
+    def "should leniently parse identification string without carriage return"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("SSH-2.0-OpenSSH-6.13\n".bytes)
+
+        when:
+        def ident = new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        ident == "SSH-2.0-OpenSSH-6.13"
+    }
+
+    def "should not parse header lines as part of ident"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("header1\nheader2\r\nSSH-2.0-OpenSSH-6.13\r\n".bytes)
+
+        when:
+        def ident = new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        ident == "SSH-2.0-OpenSSH-6.13"
+    }
+
+    def "should fail on too long ident string"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("SSH-2.0-OpenSSH-6.13 ".bytes)
+        byte[] bs = new byte[255 - buffer.wpos()]
+        Arrays.fill(bs, 'a'.bytes[0])
+        buffer.putRawBytes(bs).putRawBytes("\r\n".bytes)
+
+        when:
+        new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        thrown(TransportException.class)
+    }
+
+    def "should not fail on too long header line"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("header1 ".bytes)
+        byte[] bs = new byte[255 - buffer.wpos()]
+        new Random().nextBytes(bs)
+        buffer.putRawBytes(bs).putRawBytes("\r\n".bytes)
+        buffer.putRawBytes("SSH-2.0-OpenSSH-6.13\r\n".bytes)
+
+        when:
+        def ident = new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        ident == "SSH-2.0-OpenSSH-6.13"
+    }
+
+    def "should not fail on very short header line"() {
+        given:
+        def buffer = new Buffer.PlainBuffer()
+        buffer.putRawBytes("h1\n".bytes)
+        buffer.putRawBytes("SSH-2.0-OpenSSH-6.13\r\n".bytes)
+
+        when:
+        def ident = new IdentificationStringParser(buffer).parseIdentificationString()
+
+        then:
+        ident == "SSH-2.0-OpenSSH-6.13"
+    }
+}