From cf7309c866ce413e2ee6453acddd3a620932c170 Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Tue, 30 Mar 2021 12:00:34 +0200 Subject: [PATCH 1/2] Add extra logging to troubleshoot issue #670 --- .../net/schmizz/sshj/transport/Encoder.java | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/transport/Encoder.java b/src/main/java/net/schmizz/sshj/transport/Encoder.java index f9cc17499..29a84a63c 100644 --- a/src/main/java/net/schmizz/sshj/transport/Encoder.java +++ b/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; @@ -83,18 +84,38 @@ long encode(SSHPacket buffer) { } // Compute padding length - int padLen = cipherSize - (lengthWithoutPadding % cipherSize); - if (padLen < 4 || (authMode && padLen < cipherSize)) { - padLen += cipherSize; + // random padding - Arbitrary-length padding, such that the total length of + // (packet_length || padding_length || payload || random padding) + // 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. + int mod = 8; + if (cipherSize > mod) { + mod = cipherSize; + } + + 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 + // The minimum size of a packet is 16 (or the cipher block size, + // 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,10 +151,16 @@ long encode(SSHPacket buffer) { 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(); From 8f3d751431a985f821e05b67129c8237f4230cfe Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Tue, 20 Apr 2021 11:29:52 +0200 Subject: [PATCH 2/2] Some cleanup of integration container and add integration test --- src/itest/docker-image/Dockerfile | 4 +++- .../test-container/ssh_host_dsa_key | 21 +++++++++++++++++ .../test-container/ssh_host_dsa_key.pub | 1 + .../docker-image/test-container/sshd_config | 10 +++++--- .../hierynomus/sshj/IntegrationSpec.groovy | 23 +++++++++++++++++++ .../net/schmizz/sshj/transport/Encoder.java | 22 ++++++++---------- 6 files changed, 65 insertions(+), 16 deletions(-) create mode 100644 src/itest/docker-image/test-container/ssh_host_dsa_key create mode 100644 src/itest/docker-image/test-container/ssh_host_dsa_key.pub diff --git a/src/itest/docker-image/Dockerfile b/src/itest/docker-image/Dockerfile index db913c9f2..28c19e3ba 100644 --- a/src/itest/docker-image/Dockerfile +++ b/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"] \ No newline at end of file +ENTRYPOINT ["/sbin/tini", "/entrypoint.sh", "-o", "LogLevel=DEBUG2"] diff --git a/src/itest/docker-image/test-container/ssh_host_dsa_key b/src/itest/docker-image/test-container/ssh_host_dsa_key new file mode 100644 index 000000000..fd09af2c8 --- /dev/null +++ b/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----- diff --git a/src/itest/docker-image/test-container/ssh_host_dsa_key.pub b/src/itest/docker-image/test-container/ssh_host_dsa_key.pub new file mode 100644 index 000000000..1b3070bc1 --- /dev/null +++ b/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 diff --git a/src/itest/docker-image/test-container/sshd_config b/src/itest/docker-image/test-container/sshd_config index 48a513319..66255e32f 100644 --- a/src/itest/docker-image/test-container/sshd_config +++ b/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 \ No newline at end of file +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 diff --git a/src/itest/groovy/com/hierynomus/sshj/IntegrationSpec.groovy b/src/itest/groovy/com/hierynomus/sshj/IntegrationSpec.groovy index 060384f08..ddd80d5f3 100644 --- a/src/itest/groovy/com/hierynomus/sshj/IntegrationSpec.groovy +++ b/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() + + } } diff --git a/src/main/java/net/schmizz/sshj/transport/Encoder.java b/src/main/java/net/schmizz/sshj/transport/Encoder.java index 29a84a63c..5d5a4b6f1 100644 --- a/src/main/java/net/schmizz/sshj/transport/Encoder.java +++ b/src/main/java/net/schmizz/sshj/transport/Encoder.java @@ -74,7 +74,7 @@ long encode(SSHPacket buffer) { } 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 @@ -90,11 +90,9 @@ long encode(SSHPacket buffer) { // 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. - int mod = 8; - if (cipherSize > mod) { - mod = cipherSize; - } + 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; @@ -107,14 +105,14 @@ long encode(SSHPacket buffer) { // 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 + // 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) @@ -167,7 +165,7 @@ long encode(SSHPacket buffer) { } } - 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"); }