Some cleanup of integration container and add integration test

- Added an override for copy method, allowing the user to specify whether preserve flag is used in the SCP command.
- Propagated the preserveTime boolean to process method to skip preserveTimeIfPossible when it's not desired

examples/pom.xml

@@ -55,7 +55,7 @@
         <dependency>
             <groupId>com.hierynomus</groupId>
             <artifactId>sshj</artifactId>
-            <version>0.24.0</version>
+            <version>0.31.0</version>
         </dependency>
     </dependencies>
 

examples/src/main/java/net/schmizz/sshj/examples/InMemoryKnownHosts.java

@@ -18,7 +18,7 @@ public class InMemoryKnownHosts {
     public static void main(String[] args) throws IOException {
         InputStream entry = new ByteArrayInputStream("localhost ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIPmhSBtMctNa4hsZt8QGlsYSE5/gMkjeand69Vj4ir13".getBytes(Charset.defaultCharset()));
         SSHClient ssh = new SSHClient();
-        ssh.addHostKeyVerifier(new InMemoryHostKeyVerifier(entry, Charset.defaultCharset()));
+        ssh.addHostKeyVerifier(new OpenSSHKnownHosts(new InputStreamReader(entry, Charset.defaultCharset())));
         ssh.connect("localhost");
         try {
             ssh.authPublickey(System.getProperty("user.name"));
@@ -28,44 +28,4 @@ public class InMemoryKnownHosts {
         }
     }
 
-    public static class InMemoryHostKeyVerifier implements HostKeyVerifier {
-
-        private final List<OpenSSHKnownHosts.KnownHostEntry> entries = new ArrayList<OpenSSHKnownHosts.KnownHostEntry>();
-
-        public InMemoryHostKeyVerifier(InputStream inputStream, Charset charset) throws IOException {
-            final OpenSSHKnownHosts.EntryFactory entryFactory = new OpenSSHKnownHosts.EntryFactory();
-            BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream, charset));
-            while(reader.ready()) {
-                String line = reader.readLine();
-                try {
-                    OpenSSHKnownHosts.KnownHostEntry entry = entryFactory.parseEntry(line);
-                    if (entry != null) {
-                        entries.add(entry);
-                    }
-                } catch (Exception e) {
-                    //log error
-                }
-            }
-        }
-
-        @Override
-        public boolean verify(String hostname, int port, PublicKey key) {
-            final KeyType type = KeyType.fromKey(key);
-            if (type == KeyType.UNKNOWN) {
-                return false;
-            }
-
-            for (OpenSSHKnownHosts.KnownHostEntry e : entries) {
-                try {
-                    if (e.appliesTo(type, hostname) && e.verify(key)) {
-                        return true;
-                    }
-                } catch (IOException ioe) {
-                    //log error
-                }
-            }
-            return false;
-        }
-    }
-
 }

examples/src/main/java/net/schmizz/sshj/examples/LocalPF.java

@@ -2,6 +2,7 @@ package net.schmizz.sshj.examples;
 
 import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.connection.channel.direct.LocalPortForwarder;
+import net.schmizz.sshj.connection.channel.direct.Parameters;
 
 import java.io.IOException;
 import java.net.InetSocketAddress;
@@ -28,8 +29,8 @@ public class LocalPF {
             * _We_ listen on localhost:8080 and forward all connections on to server, which then forwards it to
             * google.com:80
             */
-            final LocalPortForwarder.Parameters params
+            final Parameters params
-                    = new LocalPortForwarder.Parameters("0.0.0.0", 8080, "google.com", 80);
+                    = new Parameters("0.0.0.0", 8080, "google.com", 80);
             final ServerSocket ss = new ServerSocket();
             ss.setReuseAddress(true);
             ss.bind(new InetSocketAddress(params.getLocalHost(), params.getLocalPort()));

src/itest/docker-image/Dockerfile

@@ -2,6 +2,8 @@ FROM sickp/alpine-sshd:7.5-r2
 
 ADD authorized_keys /home/sshj/.ssh/authorized_keys
 
+ADD test-container/ssh_host_dsa_key     /etc/ssh/ssh_host_dsa_key
+ADD test-container/ssh_host_dsa_key.pub /etc/ssh/ssh_host_dsa_key.pub
 ADD test-container/ssh_host_ecdsa_key     /etc/ssh/ssh_host_ecdsa_key
 ADD test-container/ssh_host_ecdsa_key.pub /etc/ssh/ssh_host_ecdsa_key.pub
 ADD test-container/ssh_host_ed25519_key   /etc/ssh/ssh_host_ed25519_key
@@ -21,4 +23,4 @@ RUN \
   chmod 644 /etc/ssh/ssh_host_ed25519_key.pub && \
   chown -R sshj:sshj /home/sshj
 
-ENTRYPOINT ["/sbin/tini", "/entrypoint.sh", "-o", "LogLevel=DEBUG2"]
+ENTRYPOINT ["/sbin/tini", "/entrypoint.sh", "-o", "LogLevel=DEBUG2"]

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

@@ -0,0 +1,21 @@
+-----BEGIN OPENSSH PRIVATE KEY-----
+b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABsgAAAAdzc2gtZH
+NzAAAAgQDGUD3XnEQf3S4iB8LbVAw9diSaTTbaKGRt+D89d6QWHJ75I684F6iehXzr7iKq
+PmmVy2Bp3BfSM1m6P4wJftHbVW6dbJSeVW1Ov6SIRA5XEG/hrY1DmlC9aXhksqYKUKffYx
+WOEg4p2eCtvGyu/j9EucGrC76BraEboAHpXsojNwAAABUAuEexd+FsMSbNSsJz99HcNO7M
+hy8AAACBAL++FvcyX8PZ7VeQj+gu0CkDhZM14IpNGKKOBDFvZTUILHHG7TnkjBR1HJw/bR
+lwkvrvYC+155sT/BC2OKpVkiUyCw1LKGVtJBas7UlqmTa4m1fEGjaWX0b1Op2hpwpxCkmj
+qS07HlC/nEHsEEMdCa9HxCfbEdMmUdLNSwEVJrPAAAAAgAGb6J24Ra4fz55vlK1vt34u39
+0YdMg1RLiCa4dFubc8khEMYKpwBIoyECglJyDdLXnuJQiwVzNgX1azx10G84d5QCoAZPQK
+WK1oWjj8rB+4gtEjE1sB7fZkxSXF/BaRJQPiglm0kN9gJU6RZSz4vKZIE8Bk/+htXgcXvv
+8R0eShAAAB6EMk1g1DJNYNAAAAB3NzaC1kc3MAAACBAMZQPdecRB/dLiIHwttUDD12JJpN
+NtooZG34Pz13pBYcnvkjrzgXqJ6FfOvuIqo+aZXLYGncF9IzWbo/jAl+0dtVbp1slJ5VbU
+6/pIhEDlcQb+GtjUOaUL1peGSypgpQp99jFY4SDinZ4K28bK7+P0S5wasLvoGtoRugAele
+yiM3AAAAFQC4R7F34WwxJs1KwnP30dw07syHLwAAAIEAv74W9zJfw9ntV5CP6C7QKQOFkz
+Xgik0Yoo4EMW9lNQgsccbtOeSMFHUcnD9tGXCS+u9gL7XnmxP8ELY4qlWSJTILDUsoZW0k
+FqztSWqZNribV8QaNpZfRvU6naGnCnEKSaOpLTseUL+cQewQQx0Jr0fEJ9sR0yZR0s1LAR
+Ums8AAAACAAZvonbhFrh/Pnm+UrW+3fi7f3Rh0yDVEuIJrh0W5tzySEQxgqnAEijIQKCUn
+IN0tee4lCLBXM2BfVrPHXQbzh3lAKgBk9ApYrWhaOPysH7iC0SMTWwHt9mTFJcX8FpElA+
+KCWbSQ32AlTpFlLPi8pkgTwGT/6G1eBxe+/xHR5KEAAAAUCbVJM+EkJ5K/JWd08304hz+O
+Cq8AAAANYWp2YW5lcnBAdGhvcgECAwQF
+-----END OPENSSH PRIVATE KEY-----

src/itest/docker-image/test-container/ssh_host_dsa_key.pub

@@ -0,0 +1 @@
+ssh-dss AAAAB3NzaC1kc3MAAACBAMZQPdecRB/dLiIHwttUDD12JJpNNtooZG34Pz13pBYcnvkjrzgXqJ6FfOvuIqo+aZXLYGncF9IzWbo/jAl+0dtVbp1slJ5VbU6/pIhEDlcQb+GtjUOaUL1peGSypgpQp99jFY4SDinZ4K28bK7+P0S5wasLvoGtoRugAeleyiM3AAAAFQC4R7F34WwxJs1KwnP30dw07syHLwAAAIEAv74W9zJfw9ntV5CP6C7QKQOFkzXgik0Yoo4EMW9lNQgsccbtOeSMFHUcnD9tGXCS+u9gL7XnmxP8ELY4qlWSJTILDUsoZW0kFqztSWqZNribV8QaNpZfRvU6naGnCnEKSaOpLTseUL+cQewQQx0Jr0fEJ9sR0yZR0s1LARUms8AAAACAAZvonbhFrh/Pnm+UrW+3fi7f3Rh0yDVEuIJrh0W5tzySEQxgqnAEijIQKCUnIN0tee4lCLBXM2BfVrPHXQbzh3lAKgBk9ApYrWhaOPysH7iC0SMTWwHt9mTFJcX8FpElA+KCWbSQ32AlTpFlLPi8pkgTwGT/6G1eBxe+/xHR5KE= ajvanerp@thor

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

@@ -128,9 +128,13 @@ Subsystem	sftp	/usr/lib/ssh/sftp-server
 #	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/users_rsa_ca.pub
 
-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
+KexAlgorithms +diffie-hellman-group1-sha1,diffie-hellman-group-exchange-sha1
+macs +hmac-ripemd160-etm@openssh.com,hmac-ripemd160,hmac-ripemd160@openssh.com
+Ciphers +3des-cbc,blowfish-cbc,aes128-cbc,aes192-cbc,aes256-cbc
+PubKeyAcceptedKeyTypes +ssh-dss
+HostKeyAlgorithms +ssh-dss
+HostbasedAcceptedKeyTypes +ssh-dss

src/itest/groovy/com/hierynomus/sshj/IntegrationSpec.groovy

@@ -16,6 +16,11 @@
 package com.hierynomus.sshj
 
 import com.hierynomus.sshj.key.KeyAlgorithms
+import com.hierynomus.sshj.transport.kex.DHGroups
+import com.hierynomus.sshj.transport.cipher.BlockCiphers
+import com.hierynomus.sshj.transport.mac.Macs
+import net.schmizz.sshj.transport.verification.PromiscuousVerifier
+
 import net.schmizz.sshj.DefaultConfig
 import net.schmizz.sshj.SSHClient
 import net.schmizz.sshj.transport.TransportException
@@ -92,4 +97,22 @@ class IntegrationSpec extends IntegrationBaseSpec {
         thrown(UserAuthException.class)
         !client.isAuthenticated()
     }
+
+    def "should correctly connect using parameters in #670"() {
+        given:
+        def config = new DefaultConfig()
+        // config.setKeyAlgorithms(Collections.singletonList(KeyAlgorithms.SSHDSA())) // Can't get openssh to offer ssh-dss
+        config.setKeyExchangeFactories(DHGroups.Group1SHA1())
+        config.setCipherFactories(BlockCiphers.TripleDESCBC())
+        config.setMACFactories(Macs.HMACSHA1())
+        SSHClient sshClient = new SSHClient(config)
+        sshClient.addHostKeyVerifier(new PromiscuousVerifier())
+
+        when:
+        sshClient.connect(SERVER_IP, DOCKER_PORT)
+
+        then:
+        sshClient.isConnected()
+
+    }
 }

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

@@ -16,6 +16,7 @@
 package net.schmizz.sshj.transport;
 
 import net.schmizz.sshj.common.Buffer;
+import net.schmizz.sshj.common.ByteArrayUtils;
 import net.schmizz.sshj.common.LoggerFactory;
 import net.schmizz.sshj.common.SSHPacket;
 import net.schmizz.sshj.transport.cipher.Cipher;
@@ -73,7 +74,7 @@ final class Encoder
             }
 
             final int payloadSize = buffer.available();
-            int lengthWithoutPadding;
+            final int lengthWithoutPadding;
             if (etm) {
                 // in Encrypt-Then-Mac mode, the length field is not encrypted, so we should keep it out of the
                 // padding length calculation
@@ -83,18 +84,36 @@ final class Encoder
             }
 
             // Compute padding length
-            int padLen = cipherSize - (lengthWithoutPadding % cipherSize);
+            // random padding - Arbitrary-length padding, such that the total length of
-            if (padLen < 4 || (authMode && padLen < cipherSize)) {
+            // (packet_length || padding_length || payload || random padding)
-                padLen += cipherSize;
+            // is a multiple of the cipher block size or 8, whichever is
+            // larger.  There MUST be at least four bytes of padding.  The
+            // padding SHOULD consist of random bytes.  The maximum amount of
+            // padding is 255 bytes.
+            final int mod = cipherSize > 8 ? cipherSize : 8;
+
+            // (lengthWithoutPadding + padLen) % mod == 0 where 4 <= padLen < 256
+            int padLen = mod - (lengthWithoutPadding % mod);
+            if (padLen < 4 || (authMode && padLen < mod)) {
+                padLen += mod;
             }
 
             final int startOfPacket = buffer.rpos() - 5;
             int packetLen = 1 + payloadSize + padLen; // packetLength = padLen (1 byte) + payload + padding
 
-            if (packetLen < 16) {
+            // The minimum size of a packet is 16 (or the cipher block size,
-                padLen += cipherSize;
+            // whichever is larger) bytes (plus 'mac').  Implementations SHOULD
+            // decrypt the length after receiving the first 8 (or cipher block size,
+            // whichever is larger) bytes of a packet.
+            // if (packetLen < 16) {
+            //     padLen += cipherSize;
+            //     packetLen = 1 + payloadSize + padLen;
+            // }
+            while (packetLen < Math.max(16, cipherSize)) {
+                padLen += mod; // Increment with the mod so that multiple holds
                 packetLen = 1 + payloadSize + padLen;
             }
+
             /*
              * In AES-GCM ciphers, they require packets must be a multiple of 16 bytes (which is also block size of AES)
              * as mentioned in RFC5647 Section 7.2. So we are calculating the extra padding as necessary here
@@ -130,17 +149,23 @@ final class Encoder
                 if (mac != null) {
                     putMAC(buffer, startOfPacket, endOfPadding);
                 }
+
+                if (log.isTraceEnabled()) {
+                    log.trace("Encrypting packet #{}: {}", seq, ByteArrayUtils.printHex(buffer.array(), startOfPacket, 4 + packetLen));
+                }
+
                 cipher.update(buffer.array(), startOfPacket, 4 + packetLen);
             }
             buffer.rpos(startOfPacket); // Make ready-to-read
 
+
             return seq;
         } finally {
             encodeLock.unlock();
         }
     }
 
-    protected void aeadOutgoingBuffer(Buffer buf, int offset, int len) {
+    protected void aeadOutgoingBuffer(Buffer<?> buf, int offset, int len) {
         if (cipher == null || cipher.getAuthenticationTagSize() == 0) {
             throw new IllegalArgumentException("AEAD mode requires an AEAD cipher");
         }

src/main/java/net/schmizz/sshj/xfer/scp/SCPUploadClient.java

@@ -43,11 +43,15 @@ public class SCPUploadClient extends AbstractSCPClient {
         return copy(sourceFile, remotePath, ScpCommandLine.EscapeMode.SingleQuote);
     }
 
-    public synchronized int copy(LocalSourceFile sourceFile, String remotePath, ScpCommandLine.EscapeMode escapeMode)
+    public synchronized int copy (LocalSourceFile sourceFile, String remotePath, ScpCommandLine.EscapeMode escapeMode) throws IOException {
-            throws IOException {
+        return copy(sourceFile, remotePath, escapeMode, true);
+    }
+
+    public synchronized int copy(LocalSourceFile sourceFile, String remotePath, ScpCommandLine.EscapeMode escapeMode, boolean preserveTimes)
+        throws IOException {
         engine.cleanSlate();
         try {
-            startCopy(sourceFile, remotePath, escapeMode);
+            startCopy(sourceFile, remotePath, escapeMode, preserveTimes);
         } finally {
             engine.exit();
         }
@@ -58,40 +62,44 @@ public class SCPUploadClient extends AbstractSCPClient {
         this.uploadFilter = uploadFilter;
     }
 
-    private void startCopy(LocalSourceFile sourceFile, String targetPath, ScpCommandLine.EscapeMode escapeMode)
+    private void startCopy(LocalSourceFile sourceFile, String targetPath, ScpCommandLine.EscapeMode escapeMode, boolean preserveTimes)
-            throws IOException {
+        throws IOException {
         ScpCommandLine commandLine = ScpCommandLine.with(ScpCommandLine.Arg.SINK)
-                .and(ScpCommandLine.Arg.RECURSIVE)
+            .and(ScpCommandLine.Arg.RECURSIVE)
-                .and(ScpCommandLine.Arg.PRESERVE_TIMES, sourceFile.providesAtimeMtime())
+            .and(ScpCommandLine.Arg.LIMIT, String.valueOf(bandwidthLimit), (bandwidthLimit > 0));
-                .and(ScpCommandLine.Arg.LIMIT, String.valueOf(bandwidthLimit), (bandwidthLimit > 0));
+        if (preserveTimes) {
+            commandLine.and(ScpCommandLine.Arg.PRESERVE_TIMES, sourceFile.providesAtimeMtime());
+        }
         commandLine.withPath(targetPath, escapeMode);
         engine.execSCPWith(commandLine);
         engine.check("Start status OK");
-        process(engine.getTransferListener(), sourceFile);
+        process(engine.getTransferListener(), sourceFile, preserveTimes);
     }
 
-    private void process(TransferListener listener, LocalSourceFile f)
+    private void process(TransferListener listener, LocalSourceFile f, boolean preserveTimes)
-            throws IOException {
+        throws IOException {
         if (f.isDirectory()) {
-            sendDirectory(listener.directory(f.getName()), f);
+            sendDirectory(listener.directory(f.getName()), f, preserveTimes);
         } else if (f.isFile()) {
-            sendFile(listener.file(f.getName(), f.getLength()), f);
+            sendFile(listener.file(f.getName(), f.getLength()), f, preserveTimes);
         } else
             throw new IOException(f + " is not a regular file or directory");
     }
 
-    private void sendDirectory(TransferListener listener, LocalSourceFile f)
+    private void sendDirectory(TransferListener listener, LocalSourceFile f, boolean preserveTimes)
-            throws IOException {
+        throws IOException {
         preserveTimeIfPossible(f);
         engine.sendMessage("D0" + getPermString(f) + " 0 " + f.getName());
         for (LocalSourceFile child : f.getChildren(uploadFilter))
-            process(listener, child);
+            process(listener, child, preserveTimes);
         engine.sendMessage("E");
     }
 
-    private void sendFile(StreamCopier.Listener listener, LocalSourceFile f)
+    private void sendFile(StreamCopier.Listener listener, LocalSourceFile f, boolean preserveTimes)
-            throws IOException {
+        throws IOException {
-        preserveTimeIfPossible(f);
+        if(preserveTimes) {
+            preserveTimeIfPossible(f);
+        }
         final InputStream src = f.getInputStream();
         try {
             engine.sendMessage("C0" + getPermString(f) + " " + f.getLength() + " " + f.getName());