From 0e3f7c2bbfa418ccae87c8d736692e9adb259c1a Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Fri, 20 Jan 2017 15:05:18 +0100 Subject: [PATCH 01/11] Added new DH groups --- README.adoc | 7 +- .../hierynomus/sshj/transport/kex/DHG.java | 49 +++++++ .../sshj/transport/kex/DHGroups.java | 92 ++++++++++++ .../sshj/transport/kex/ExtendedDHGroups.java | 59 ++++++++ .../java/net/schmizz/sshj/DefaultConfig.java | 29 +++- .../net/schmizz/sshj/transport/kex/DH.java | 2 +- .../schmizz/sshj/transport/kex/DHBase.java | 4 +- .../net/schmizz/sshj/transport/kex/DHG1.java | 1 + .../net/schmizz/sshj/transport/kex/DHG14.java | 2 + .../sshj/transport/kex/DHGroupData.java | 135 ++++++++++++++++-- .../net/schmizz/sshj/transport/kex/ECDH.java | 2 +- 11 files changed, 360 insertions(+), 22 deletions(-) create mode 100644 src/main/java/com/hierynomus/sshj/transport/kex/DHG.java create mode 100644 src/main/java/com/hierynomus/sshj/transport/kex/DHGroups.java create mode 100644 src/main/java/com/hierynomus/sshj/transport/kex/ExtendedDHGroups.java diff --git a/README.adoc b/README.adoc index 03571726..ad0b15ce 100644 --- a/README.adoc +++ b/README.adoc @@ -66,8 +66,13 @@ ciphers:: SSHJ also supports the following extended (non official) ciphers: `camellia{128,192,256}-{cbc,ctr}`, `camellia{128,192,256}-{cbc,ctr}@openssh.org` key exchange:: - `diffie-hellman-group1-sha1`, `diffie-hellman-group14-sha1`, `diffie-hellman-group-exchange-sha1`, `diffie-hellman-group-exchange-sha256`, + `diffie-hellman-group1-sha1`, `diffie-hellman-group14-sha1`, + `diffie-hellman-group14-sha256`, `diffie-hellman-group15-sha512`, `diffie-hellman-group16-sha512`, `diffie-hellman-group17-sha512`, `diffie-hellman-group18-sha512` + `diffie-hellman-group-exchange-sha1`, `diffie-hellman-group-exchange-sha256`, `ecdh-sha2-nistp256`, `ecdh-sha2-nistp384`, `ecdh-sha2-nistp521`, `curve25519-sha256@libssh.org` + SSHJ also supports the following extended (non official) key exchange algoriths: + `diffie-hellman-group14-sha256@ssh.com`, `diffie-hellman-group15-sha256`, `diffie-hellman-group15-sha256@ssh.com`, `diffie-hellman-group15-sha384@ssh.com`, + `diffie-hellman-group16-sha256`, `diffie-hellman-group16-sha384@ssh.com`, `diffie-hellman-group16-sha512@ssh.com`, `diffie-hellman-group18-sha512@ssh.com` signatures:: `ssh-rsa`, `ssh-dss`, `ecdsa-sha2-nistp256`, `ssh-ed25519` diff --git a/src/main/java/com/hierynomus/sshj/transport/kex/DHG.java b/src/main/java/com/hierynomus/sshj/transport/kex/DHG.java new file mode 100644 index 00000000..dc10d033 --- /dev/null +++ b/src/main/java/com/hierynomus/sshj/transport/kex/DHG.java @@ -0,0 +1,49 @@ +/* + * 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.kex; + +import net.schmizz.sshj.transport.digest.Digest; +import net.schmizz.sshj.transport.digest.SHA256; +import net.schmizz.sshj.transport.kex.AbstractDHG; +import net.schmizz.sshj.transport.kex.DH; +import net.schmizz.sshj.transport.kex.DHBase; +import net.schmizz.sshj.transport.kex.KeyExchange; + +import javax.crypto.spec.DHParameterSpec; +import java.math.BigInteger; +import java.security.GeneralSecurityException; + +import static net.schmizz.sshj.transport.kex.DHGroupData.G; +import static net.schmizz.sshj.transport.kex.DHGroupData.P14; + +/** + * + */ +public class DHG extends AbstractDHG { + private BigInteger group; + private BigInteger generator; + + public DHG(BigInteger group, BigInteger generator, Digest digest) { + super(new DH(), digest); + this.group = group; + this.generator = generator; + } + + @Override + protected void initDH(DHBase dh) throws GeneralSecurityException { + dh.init(new DHParameterSpec(group, generator), trans.getConfig().getRandomFactory()); + } +} diff --git a/src/main/java/com/hierynomus/sshj/transport/kex/DHGroups.java b/src/main/java/com/hierynomus/sshj/transport/kex/DHGroups.java new file mode 100644 index 00000000..1e08d7a4 --- /dev/null +++ b/src/main/java/com/hierynomus/sshj/transport/kex/DHGroups.java @@ -0,0 +1,92 @@ +/* + * 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.kex; + +import net.schmizz.sshj.transport.digest.*; +import net.schmizz.sshj.transport.kex.KeyExchange; + +import java.math.BigInteger; + +import static net.schmizz.sshj.transport.kex.DHGroupData.*; +import static net.schmizz.sshj.transport.kex.DHGroupData.P16; +import static net.schmizz.sshj.transport.kex.DHGroupData.P18; + +/** + * Factory methods for Diffie Hellmann KEX algorithms based on MODP groups / Oakley Groups + * + * - https://tools.ietf.org/html/rfc4253 + * - https://tools.ietf.org/html/draft-ietf-curdle-ssh-modp-dh-sha2-01 + */ +public class DHGroups { + + public static DHGroups.Factory Group1SHA1() { + return new DHGroups.Factory("diffie-hellman-group1-sha1", P1, G, new SHA1.Factory()); + } + + public static DHGroups.Factory Group14SHA1() { + return new DHGroups.Factory("diffie-hellman-group14-sha1", P14, G, new SHA1.Factory()); + } + + public static DHGroups.Factory Group14SHA256() { + return new DHGroups.Factory("diffie-hellman-group14-sha256", P14, G, new SHA256.Factory()); + } + + public static DHGroups.Factory Group15SHA512() { + return new DHGroups.Factory("diffie-hellman-group15-sha512", P15, G, new SHA512.Factory()); + } + + public static DHGroups.Factory Group16SHA512() { + return new DHGroups.Factory("diffie-hellman-group16-sha512", P16, G, new SHA512.Factory()); + } + + public static DHGroups.Factory Group17SHA512() { + return new DHGroups.Factory("diffie-hellman-group17-sha512", P17, G, new SHA512.Factory()); + } + + public static DHGroups.Factory Group18SHA512() { + return new DHGroups.Factory("diffie-hellman-group18-sha512", P18, G, new SHA512.Factory()); + } + + /** + * Named factory for DHG1 key exchange + */ + public static class Factory + implements net.schmizz.sshj.common.Factory.Named { + + private String name; + private BigInteger group; + private BigInteger generator; + private Factory.Named digestFactory; + + public Factory(String name, BigInteger group, BigInteger generator, Named digestFactory) { + this.name = name; + this.group = group; + this.generator = generator; + this.digestFactory = digestFactory; + } + + @Override + public KeyExchange create() { + return new DHG(group, generator, digestFactory.create()); + } + + @Override + public String getName() { + return name; + } + } + +} diff --git a/src/main/java/com/hierynomus/sshj/transport/kex/ExtendedDHGroups.java b/src/main/java/com/hierynomus/sshj/transport/kex/ExtendedDHGroups.java new file mode 100644 index 00000000..6d0153f3 --- /dev/null +++ b/src/main/java/com/hierynomus/sshj/transport/kex/ExtendedDHGroups.java @@ -0,0 +1,59 @@ +/* + * 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.kex; + +import net.schmizz.sshj.transport.digest.SHA256; +import net.schmizz.sshj.transport.digest.SHA384; +import net.schmizz.sshj.transport.digest.SHA512; + +import static net.schmizz.sshj.transport.kex.DHGroupData.*; + +/** + * Set of KEX methods that are not in official RFCs but are supported by some SSH servers. + */ +public class ExtendedDHGroups { + public static DHGroups.Factory Group14SHA256AtSSH() { + return new DHGroups.Factory("diffie-hellman-group14-sha256@ssh.com", P14, G, new SHA256.Factory()); + } + + public static DHGroups.Factory Group15SHA256() { + return new DHGroups.Factory("diffie-hellman-group15-sha256", P15, G, new SHA256.Factory()); + } + + public static DHGroups.Factory Group15SHA256AtSSH() { + return new DHGroups.Factory("diffie-hellman-group15-sha256@ssh.com", P15, G, new SHA256.Factory()); + } + + public static DHGroups.Factory Group15SHA384AtSSH() { + return new DHGroups.Factory("diffie-hellman-group15-sha384@ssh.com", P15, G, new SHA384.Factory()); + } + + public static DHGroups.Factory Group16SHA256() { + return new DHGroups.Factory("diffie-hellman-group16-sha256", P16, G, new SHA256.Factory()); + } + + public static DHGroups.Factory Group16SHA384AtSSH() { + return new DHGroups.Factory("diffie-hellman-group16-sha384@ssh.com", P16, G, new SHA384.Factory()); + } + + public static DHGroups.Factory Group16SHA512AtSSH() { + return new DHGroups.Factory("diffie-hellman-group16-sha512@ssh.com", P16, G, new SHA512.Factory()); + } + + public static DHGroups.Factory Group18SHA512AtSSH() { + return new DHGroups.Factory("diffie-hellman-group18-sha512@ssh.com", P18, G, new SHA512.Factory()); + } +} diff --git a/src/main/java/net/schmizz/sshj/DefaultConfig.java b/src/main/java/net/schmizz/sshj/DefaultConfig.java index 9275f72c..4a264d11 100644 --- a/src/main/java/net/schmizz/sshj/DefaultConfig.java +++ b/src/main/java/net/schmizz/sshj/DefaultConfig.java @@ -18,6 +18,9 @@ package net.schmizz.sshj; import com.hierynomus.sshj.signature.SignatureEdDSA; import com.hierynomus.sshj.transport.cipher.BlockCiphers; import com.hierynomus.sshj.transport.cipher.StreamCiphers; +import com.hierynomus.sshj.transport.kex.DHGroups; +import com.hierynomus.sshj.transport.kex.ExtendedDHGroups; +import com.hierynomus.sshj.userauth.keyprovider.OpenSSHKeyV1KeyFile; import net.schmizz.keepalive.KeepAliveProvider; import net.schmizz.sshj.common.Factory; import net.schmizz.sshj.common.LoggerFactory; @@ -27,14 +30,15 @@ import net.schmizz.sshj.signature.SignatureECDSA; import net.schmizz.sshj.signature.SignatureRSA; import net.schmizz.sshj.transport.cipher.*; import net.schmizz.sshj.transport.compression.NoneCompression; -import net.schmizz.sshj.transport.kex.*; +import net.schmizz.sshj.transport.kex.Curve25519SHA256; +import net.schmizz.sshj.transport.kex.DHGexSHA1; +import net.schmizz.sshj.transport.kex.DHGexSHA256; +import net.schmizz.sshj.transport.kex.ECDHNistP; import net.schmizz.sshj.transport.mac.*; import net.schmizz.sshj.transport.random.BouncyCastleRandom; import net.schmizz.sshj.transport.random.JCERandom; import net.schmizz.sshj.transport.random.SingletonRandomFactory; import net.schmizz.sshj.userauth.keyprovider.OpenSSHKeyFile; -import com.hierynomus.sshj.userauth.keyprovider.OpenSSHKeyV1KeyFile; - import net.schmizz.sshj.userauth.keyprovider.PKCS5KeyFile; import net.schmizz.sshj.userauth.keyprovider.PKCS8KeyFile; import net.schmizz.sshj.userauth.keyprovider.PuTTYKeyFile; @@ -110,10 +114,23 @@ public class DefaultConfig new ECDHNistP.Factory384(), new ECDHNistP.Factory256(), new DHGexSHA1.Factory(), - new DHG14.Factory(), - new DHG1.Factory()); + DHGroups.Group1SHA1(), + DHGroups.Group14SHA1(), + DHGroups.Group14SHA256(), + DHGroups.Group15SHA512(), + DHGroups.Group16SHA512(), + DHGroups.Group17SHA512(), + DHGroups.Group18SHA512(), + ExtendedDHGroups.Group14SHA256AtSSH(), + ExtendedDHGroups.Group15SHA256(), + ExtendedDHGroups.Group15SHA256AtSSH(), + ExtendedDHGroups.Group15SHA384AtSSH(), + ExtendedDHGroups.Group16SHA256(), + ExtendedDHGroups.Group16SHA384AtSSH(), + ExtendedDHGroups.Group16SHA512AtSSH(), + ExtendedDHGroups.Group18SHA512AtSSH()); } else { - setKeyExchangeFactories(new DHG1.Factory(), new DHGexSHA1.Factory()); + setKeyExchangeFactories(DHGroups.Group1SHA1(), new DHGexSHA1.Factory()); } } diff --git a/src/main/java/net/schmizz/sshj/transport/kex/DH.java b/src/main/java/net/schmizz/sshj/transport/kex/DH.java index 83f32bf8..6b5fe0d1 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/DH.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/DH.java @@ -40,7 +40,7 @@ public class DH extends DHBase { } @Override - protected void init(AlgorithmParameterSpec params, Factory randomFactory) throws GeneralSecurityException { + public void init(AlgorithmParameterSpec params, Factory randomFactory) throws GeneralSecurityException { if (!(params instanceof DHParameterSpec)) { throw new SSHRuntimeException("Wrong algorithm parameters for Diffie Hellman"); } diff --git a/src/main/java/net/schmizz/sshj/transport/kex/DHBase.java b/src/main/java/net/schmizz/sshj/transport/kex/DHBase.java index 9ad12fa6..61f46a4e 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/DHBase.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/DHBase.java @@ -26,7 +26,7 @@ import java.security.GeneralSecurityException; import java.security.KeyPairGenerator; import java.security.spec.AlgorithmParameterSpec; -abstract class DHBase { +public abstract class DHBase { protected final KeyPairGenerator generator; protected final KeyAgreement agreement; @@ -44,7 +44,7 @@ abstract class DHBase { abstract void computeK(byte[] f) throws GeneralSecurityException; - protected abstract void init(AlgorithmParameterSpec params, Factory randomFactory) throws GeneralSecurityException; + public abstract void init(AlgorithmParameterSpec params, Factory randomFactory) throws GeneralSecurityException; void setE(byte[] e) { this.e = e; diff --git a/src/main/java/net/schmizz/sshj/transport/kex/DHG1.java b/src/main/java/net/schmizz/sshj/transport/kex/DHG1.java index d8e79013..bbe8e67c 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/DHG1.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/DHG1.java @@ -26,6 +26,7 @@ import java.security.GeneralSecurityException; * @see RFC 4253 * * TODO refactor away the (unneeded) class + * @deprecated Replaced by {@link com.hierynomus.sshj.transport.kex.DHG} with {@link com.hierynomus.sshj.transport.kex.DHGroups} */ public class DHG1 extends AbstractDHG { diff --git a/src/main/java/net/schmizz/sshj/transport/kex/DHG14.java b/src/main/java/net/schmizz/sshj/transport/kex/DHG14.java index a6ab830a..9139bf62 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/DHG14.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/DHG14.java @@ -25,6 +25,8 @@ import java.security.GeneralSecurityException; *

* DHG14 does not work with the default JCE implementation provided by Sun because it does not support 2048 bits * encryption. It requires BouncyCastle to be used. + * + * @deprecated Replaced by {@link com.hierynomus.sshj.transport.kex.DHG} with {@link com.hierynomus.sshj.transport.kex.DHGroups} */ public class DHG14 extends AbstractDHG { diff --git a/src/main/java/net/schmizz/sshj/transport/kex/DHGroupData.java b/src/main/java/net/schmizz/sshj/transport/kex/DHGroupData.java index ebdd0b90..2f28389f 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/DHGroupData.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/DHGroupData.java @@ -17,26 +17,139 @@ package net.schmizz.sshj.transport.kex; import java.math.BigInteger; -/** Simple class holding the data for DH group key exchanges. */ +/** + * Simple class holding the data for DH group key exchanges. + */ public final class DHGroupData { public static final BigInteger G = new BigInteger("2"); + /** + * First Oakley Group (https://tools.ietf.org/html/rfc2409) - P1 + * prime: 2^768 - 2 ^704 - 1 + 2^64 * { [2^638 pi] + 149686 } + */ public static final BigInteger P1 = new BigInteger("1797693134862315907708391567937874531978602960487560117064444236841971802161585193" + - "6894783379586492554150218056548598050364644054819923910005079287700335581663922955" + - "3136239076508735759914822574862575007425302077447712589550957937778424442426617334" + - "727629299387668709205606050270810842907692932019128194467627007"); + "6894783379586492554150218056548598050364644054819923910005079287700335581663922955" + + "3136239076508735759914822574862575007425302077447712589550957937778424442426617334" + + "727629299387668709205606050270810842907692932019128194467627007"); + /** + * 2048-bit MODP Group - P14 (https://tools.ietf.org/html/rfc3526#section-3) + * prime: 2^2048 - 2^1984 - 1 + 2^64 * { [2^1918 pi] + 124476 } + */ public static final BigInteger P14 = new BigInteger("3231700607131100730033891392642382824881794124114023911284200975140074170663435422" + - "2619689417363569347117901737909704191754605873209195028853758986185622153212175412" + - "5149017745202702357960782362488842461894775876411059286460994117232454266225221932" + - "3054091903768052423551912567971587011700105805587765103886184728025797605490356973" + - "2561526167081339361799541336476559160368317896729073178384589680639671900977202194" + - "1686472258710314113364293195361934716365332097170774482279885885653692086452966360" + - "7725026895550592836275112117409697299806841055435958486658329164213621823107899099" + - "9448652468262416972035911852507045361090559"); + "2619689417363569347117901737909704191754605873209195028853758986185622153212175412" + + "5149017745202702357960782362488842461894775876411059286460994117232454266225221932" + + "3054091903768052423551912567971587011700105805587765103886184728025797605490356973" + + "2561526167081339361799541336476559160368317896729073178384589680639671900977202194" + + "1686472258710314113364293195361934716365332097170774482279885885653692086452966360" + + "7725026895550592836275112117409697299806841055435958486658329164213621823107899099" + + "9448652468262416972035911852507045361090559"); + /** + * 3072-bit MODP Group - P15 (https://tools.ietf.org/html/rfc3526#section-4) + * prime: 2^3072 - 2^3008 - 1 + 2^64 * { [2^2942 pi] + 1690314 } + */ + public static final BigInteger P15 = + new BigInteger("5809605995369958062791915965639201402176612226902900533702900882779736177890990861" + + "4720947744773395811473734101856463783280437298007504700982109244878669350591643715" + + "8816804754094398164451663275506750162643455639819318662899007124866081936120511979" + + "3693985433297036118232914410171876807536457391277857011849897410207519105333355801" + + "1211093568974594262718454713979526759594407934930716283941227805101246184882326024" + + "6464987685045886124578424092925842628769970531258450962541951346360515542801716571" + + "4465363094021609290561084025893662561222573202082865797821865270991145082200656978" + + "1771928270245389902399691755461907706456858934380117144304264093386763147435711545" + + "3714203157300427642870143303638180170530865983075119035294602548205993130657100472" + + "7362479688415574702596946457770284148435989129632853918392117997472632693078113129" + + "8864873993477969827727846158652326212896569442842168246113187097645351525073541163" + + "44703769998514148343807"); + + /** + * 4096-bit MODP Group - P16 (https://tools.ietf.org/html/rfc3526#section-5) + * prime: 2^4096 - 2^4032 - 1 + 2^64 * { [2^3966 pi] + 240904 } + */ + public static final BigInteger P16 = + new BigInteger("10443888814131525066796027198465295458312690609921350090225887564443381720223226907" + + "10444046669809783930111585737890362691860127079270495454517218673016928427459146001" + + "86688577976298222932119236830334623520436805101030915567415569746034717694639407653" + + "51572849948952848216337009218117167389724518349794558970103063334685907513583651387" + + "82250372269117968985194322444535687415522007151638638141456178420621277822674995027" + + "99027867345862954439173691976629900551150544617766815444623488266596168079657690319" + + "91160893476349471877789065280080047566925716669229641225661745827767073324523710012" + + "72163776841229318324903125740713574141005124561965913888899753461735347970011693256" + + "31675166067895083002751025580484610558346505544661509044430958305077580850929704003" + + "96800574353422539265662408981958636315888889363641299200593084556694540340103914782" + + "38784189888594672336242763795138176353222845524644040094258962433613354036104643881" + + "92523848922401019419308891166616558422942466816544168892779046060826486420423771700" + + "20547443379889419746612146996897065215430062626045358909981257522759426087721743761" + + "07314217749233048217904944409836238235772306749874396760463376480215133461333478395" + + "682746608242585133953883882226786118030184028136755970045385534758453247"); + + /** + * 6144-bit MODP Group - P17 (https://tools.ietf.org/html/rfc3526#section-6) + * prime: 2^6144 - 2^6080 - 1 + 2^64 * { [2^6014 pi] + 929484 } + */ + public static final BigInteger P17 = + new BigInteger("3375152182143856118451852315996741233006489780574184654817389047442942990132667244" + + "5203235101919165483964194359460994881062089387893762814044257438204432573941083014" + + "8270060902589258751610180963277323358005958319159760142088223040073278481327349332" + + "9788580321367526156496260334045722077682632250005809131096725397661997398803366366" + + "6385188155212656268079501726223369693427999804134467810120772356498596945532366527" + + "4005175754719693358549052745041195095923660137119541482588848792245999152034563158" + + "8103477655308367699571833559858639559116999957082451503501754353335269752528775333" + + "2500527176569576894926734950469293596134095086603716860086302051544539652689091299" + + "0997845889190523834630577894405654606814419024423999564190605216296046973478790246" + + "5431380018607831652696452928806274087901103517592005919217856147319900620589671943" + + "5014765345518490882366607110905303449152556221163232127426440691921134648766635695" + + "8502392313045917442156109850296368954067188807663082492273159842675422662594896843" + + "7222391644541101590050623941926790971632033120898897818086898743162371034761799235" + + "6201449023892203230133009421463914291201346063125219636964261683591541014344239275" + + "3407356909977322220697587739633908763605465157552805170421605254873028981223116697" + + "9967944753045360039934269703271445854959128593945394903498124811432232236723864504" + + "2515984447890788917823576330019151696568654314153058547592091366014550143819685170" + + "0683437001046776090411663697600809334136054989623820777788455998349074759534307874" + + "4620138456732853067527579296235488377080690082718368571835346957473168052062194454" + + "0947734619035177180057973022652571032196598229259194875709994709721793154158686515" + + "7485072742241813169487971046010682120152329216914824963468544136987197501906011027" + + "0527448105054323981513068607360107630451228454921845984604608225359676243382741906" + + "0089029417044871218316020923109988915707117567"); + + /** + * 8192-bit MODP Group - P18 (https://tools.ietf.org/html/rfc3526#section-7) + * prime: 2^8192 - 2^8128 - 1 + 2^64 * { [2^8062 pi] + 4743158 } + */ + public static final BigInteger P18 = + new BigInteger("10907481356194159294502949293597845003481551249531722117741011069661501689227856390" + + "28532473848836817769712164169076432969224698752674677662739994265785437233596157045" + + "97092233804069810050786103304731233182398243527947570019986097161273254052879655450" + + "28679197467769837593914759871425213158787195775191488118308799194269399584870875409" + + "65716419167467499326156226529675209172277001377591248147563782880558861083327174154" + + "01497513489312511601577631889029596069801161415772128252753946881651931933333750311" + + "47771923604122817210189558343776154804684792527488673203623853555966017951228067562" + + "17713579819870634321561907813255153703950795271232652404894983869492174481652303803" + + "49888136621050864726366837651413103110233683748899977574404673365182723939535354034" + + "84148728546397192946943234501868841898225445406472269872921606931847346549419069366" + + "46576130260972193280317171696418971553954161446191759093719524951116705577362073481" + + "31929604120128351615426904438925772770028968411946028348045230620413002491387998113" + + "59080269838682059693181678196808509986496944169079527129049624049377757896989172073" + + "56355227455066183815847669135530549755439819480321732925869069136146085326382334628" + + "74545639807160305805163420938670870330654590319960852382451372962513665912822110096" + + "77354505199524042481982628138310973742616503800172779169753241348465746813073370173" + + "80830353680623216336949471306191686438249305686413380231046096450953594089375540285" + + "03729247092939511402830554745258496207430943815182543790297601289174935519867842060" + + "37220349003113648930464957614043339386861400378480309162925432736845336400326376391" + + "00774502371542479302473698388692892420946478947733800387782741417786484770190108867" + + "87977899163321862864053398261932246615488301145229189025233648723608665439609385389" + + "86288058131775591620763631544364944775078712941198416378677017221666098312018454840" + + "78070518041336869808398454625586921201308185638888082699408686536045192649569198110" + + "35365994311180230063610650986502394366182943642656300791728205089442938884174888539" + + "82907077430529736053592775157496197308237732158947551217614678878653277071155738042" + + "64519206349215850195195364813387526811742474131549802130246506341207020335797706780" + + "70540694527543880626597851620970679570257924407538049023174103086261496878330620786" + + "96878681084236399719832090776247580804999882755913927872676271824428928096468742282" + + "63172435642368588260139161962836121481966092745325488641054238839295138992979335446" + + "110090325230955276870524611359124918392740353154294858383359"); } diff --git a/src/main/java/net/schmizz/sshj/transport/kex/ECDH.java b/src/main/java/net/schmizz/sshj/transport/kex/ECDH.java index ade185ba..0f35a83e 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/ECDH.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/ECDH.java @@ -41,7 +41,7 @@ public class ECDH extends DHBase { super("EC", "ECDH"); } - protected void init(AlgorithmParameterSpec params, Factory randomFactory) throws GeneralSecurityException { + public void init(AlgorithmParameterSpec params, Factory randomFactory) throws GeneralSecurityException { generator.initialize(params); KeyPair keyPair = generator.generateKeyPair(); agreement.init(keyPair.getPrivate()); From 9cb5bf4e101875584c252de5e4411bd55088a538 Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Fri, 20 Jan 2017 15:26:21 +0100 Subject: [PATCH 02/11] Organized imports --- src/main/java/com/hierynomus/sshj/transport/kex/DHG.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/main/java/com/hierynomus/sshj/transport/kex/DHG.java b/src/main/java/com/hierynomus/sshj/transport/kex/DHG.java index dc10d033..dbb97092 100644 --- a/src/main/java/com/hierynomus/sshj/transport/kex/DHG.java +++ b/src/main/java/com/hierynomus/sshj/transport/kex/DHG.java @@ -16,19 +16,14 @@ package com.hierynomus.sshj.transport.kex; import net.schmizz.sshj.transport.digest.Digest; -import net.schmizz.sshj.transport.digest.SHA256; import net.schmizz.sshj.transport.kex.AbstractDHG; import net.schmizz.sshj.transport.kex.DH; import net.schmizz.sshj.transport.kex.DHBase; -import net.schmizz.sshj.transport.kex.KeyExchange; import javax.crypto.spec.DHParameterSpec; import java.math.BigInteger; import java.security.GeneralSecurityException; -import static net.schmizz.sshj.transport.kex.DHGroupData.G; -import static net.schmizz.sshj.transport.kex.DHGroupData.P14; - /** * */ From 786734ce262184b38de2796edee358d9c831f91c Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Fri, 20 Jan 2017 15:43:36 +0100 Subject: [PATCH 03/11] Suppressing Method naming conventions for Factory classes --- .../java/com/hierynomus/sshj/transport/cipher/BlockCiphers.java | 1 + .../hierynomus/sshj/transport/cipher/ExtendedBlockCiphers.java | 1 + .../java/com/hierynomus/sshj/transport/cipher/StreamCiphers.java | 1 + src/main/java/com/hierynomus/sshj/transport/kex/DHGroups.java | 1 + .../java/com/hierynomus/sshj/transport/kex/ExtendedDHGroups.java | 1 + 5 files changed, 5 insertions(+) diff --git a/src/main/java/com/hierynomus/sshj/transport/cipher/BlockCiphers.java b/src/main/java/com/hierynomus/sshj/transport/cipher/BlockCiphers.java index ceb301e4..ea7818da 100644 --- a/src/main/java/com/hierynomus/sshj/transport/cipher/BlockCiphers.java +++ b/src/main/java/com/hierynomus/sshj/transport/cipher/BlockCiphers.java @@ -28,6 +28,7 @@ import net.schmizz.sshj.transport.cipher.Cipher; * * Some of the Ciphers are still implemented in net.schmizz.sshj.transport.cipher.*. These are scheduled to be migrated to here. */ +@SuppressWarnings("PMD.MethodNamingConventions") public class BlockCiphers { public static final String COUNTER_MODE = "CTR"; diff --git a/src/main/java/com/hierynomus/sshj/transport/cipher/ExtendedBlockCiphers.java b/src/main/java/com/hierynomus/sshj/transport/cipher/ExtendedBlockCiphers.java index 8a977daa..58534129 100644 --- a/src/main/java/com/hierynomus/sshj/transport/cipher/ExtendedBlockCiphers.java +++ b/src/main/java/com/hierynomus/sshj/transport/cipher/ExtendedBlockCiphers.java @@ -25,6 +25,7 @@ import static com.hierynomus.sshj.transport.cipher.BlockCiphers.COUNTER_MODE; * * - http://tools.ietf.org/id/draft-kanno-secsh-camellia-01.txt */ +@SuppressWarnings("PMD.MethodNamingConventions") public class ExtendedBlockCiphers { public static BlockCiphers.Factory Camellia128CTR() { return new BlockCiphers.Factory(16, 128, "camellia128-ctr", "Camellia", COUNTER_MODE); diff --git a/src/main/java/com/hierynomus/sshj/transport/cipher/StreamCiphers.java b/src/main/java/com/hierynomus/sshj/transport/cipher/StreamCiphers.java index e30f50c7..55d37f0f 100644 --- a/src/main/java/com/hierynomus/sshj/transport/cipher/StreamCiphers.java +++ b/src/main/java/com/hierynomus/sshj/transport/cipher/StreamCiphers.java @@ -23,6 +23,7 @@ import net.schmizz.sshj.transport.cipher.Cipher; * - https://tools.ietf.org/html/rfc4253#section-6.3 * - https://tools.ietf.org/html/rfc4345 */ +@SuppressWarnings("PMD.MethodNamingConventions") public class StreamCiphers { public static Factory Arcfour() { diff --git a/src/main/java/com/hierynomus/sshj/transport/kex/DHGroups.java b/src/main/java/com/hierynomus/sshj/transport/kex/DHGroups.java index 1e08d7a4..3efbddd8 100644 --- a/src/main/java/com/hierynomus/sshj/transport/kex/DHGroups.java +++ b/src/main/java/com/hierynomus/sshj/transport/kex/DHGroups.java @@ -30,6 +30,7 @@ import static net.schmizz.sshj.transport.kex.DHGroupData.P18; * - https://tools.ietf.org/html/rfc4253 * - https://tools.ietf.org/html/draft-ietf-curdle-ssh-modp-dh-sha2-01 */ +@SuppressWarnings("PMD.MethodNamingConventions") public class DHGroups { public static DHGroups.Factory Group1SHA1() { diff --git a/src/main/java/com/hierynomus/sshj/transport/kex/ExtendedDHGroups.java b/src/main/java/com/hierynomus/sshj/transport/kex/ExtendedDHGroups.java index 6d0153f3..9dd1fbc3 100644 --- a/src/main/java/com/hierynomus/sshj/transport/kex/ExtendedDHGroups.java +++ b/src/main/java/com/hierynomus/sshj/transport/kex/ExtendedDHGroups.java @@ -24,6 +24,7 @@ import static net.schmizz.sshj.transport.kex.DHGroupData.*; /** * Set of KEX methods that are not in official RFCs but are supported by some SSH servers. */ +@SuppressWarnings("PMD.MethodNamingConventions") public class ExtendedDHGroups { public static DHGroups.Factory Group14SHA256AtSSH() { return new DHGroups.Factory("diffie-hellman-group14-sha256@ssh.com", P14, G, new SHA256.Factory()); From aa47b0c5f7d0fe75da961cdad7a8cb1a0803f54f Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Fri, 20 Jan 2017 16:29:30 +0100 Subject: [PATCH 04/11] Cleaning up unused code --- .../java/net/schmizz/sshj/transport/Encoder.java | 13 ------------- .../sshj/transport/kex/Curve25519SHA256.java | 4 ---- .../channel/forwarded/RemotePortForwarderTest.java | 5 ----- .../java/com/hierynomus/sshj/test/HttpServer.java | 3 --- .../net/schmizz/sshj/sftp/PacketReaderTest.java | 6 ++---- 5 files changed, 2 insertions(+), 29 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/transport/Encoder.java b/src/main/java/net/schmizz/sshj/transport/Encoder.java index 58bd5800..c5831718 100644 --- a/src/main/java/net/schmizz/sshj/transport/Encoder.java +++ b/src/main/java/net/schmizz/sshj/transport/Encoder.java @@ -39,19 +39,6 @@ final class Encoder log = loggerFactory.getLogger(getClass()); } - private SSHPacket checkHeaderSpace(SSHPacket buffer) { - if (buffer.rpos() < 5) { - log.warn("Performance cost: when sending a packet, ensure that " - + "5 bytes are available in front of the buffer"); - SSHPacket nb = new SSHPacket(buffer.available() + 5); - nb.rpos(5); - nb.wpos(5); - nb.putBuffer(buffer); - buffer = nb; - } - return buffer; - } - private void compress(SSHPacket buffer) { compression.compress(buffer); } diff --git a/src/main/java/net/schmizz/sshj/transport/kex/Curve25519SHA256.java b/src/main/java/net/schmizz/sshj/transport/kex/Curve25519SHA256.java index 92f4461c..61bb42f7 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/Curve25519SHA256.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/Curve25519SHA256.java @@ -16,14 +16,10 @@ package net.schmizz.sshj.transport.kex; import net.schmizz.sshj.transport.digest.SHA256; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.security.GeneralSecurityException; public class Curve25519SHA256 extends AbstractDHG { - private static final Logger logger = LoggerFactory.getLogger(Curve25519SHA256.class); - /** Named factory for Curve25519SHA256 key exchange */ public static class Factory implements net.schmizz.sshj.common.Factory.Named { diff --git a/src/test/java/com/hierynomus/sshj/connection/channel/forwarded/RemotePortForwarderTest.java b/src/test/java/com/hierynomus/sshj/connection/channel/forwarded/RemotePortForwarderTest.java index c9bf1b8e..c6b77c5d 100644 --- a/src/test/java/com/hierynomus/sshj/connection/channel/forwarded/RemotePortForwarderTest.java +++ b/src/test/java/com/hierynomus/sshj/connection/channel/forwarded/RemotePortForwarderTest.java @@ -40,11 +40,6 @@ import static org.junit.Assert.assertThat; public class RemotePortForwarderTest { - // Credentials for an remote SSH Server to test against. - private static final String REMOTE_HOST = "x.x.x.x"; - private static final String USER = "xxxx"; - private static final String PASSWORD = "yyyy"; - private static final PortRange RANGE = new PortRange(9000, 9999); private static final InetSocketAddress HTTP_SERVER_SOCKET_ADDR = new InetSocketAddress("127.0.0.1", 8080); diff --git a/src/test/java/com/hierynomus/sshj/test/HttpServer.java b/src/test/java/com/hierynomus/sshj/test/HttpServer.java index 7bdd0101..76ec2333 100644 --- a/src/test/java/com/hierynomus/sshj/test/HttpServer.java +++ b/src/test/java/com/hierynomus/sshj/test/HttpServer.java @@ -27,9 +27,6 @@ public class HttpServer extends ExternalResource { private TemporaryFolder docRoot = new TemporaryFolder(); - public HttpServer() { - } - @Override protected void before() throws Throwable { docRoot.create(); diff --git a/src/test/java/net/schmizz/sshj/sftp/PacketReaderTest.java b/src/test/java/net/schmizz/sshj/sftp/PacketReaderTest.java index 234a520e..63b12819 100644 --- a/src/test/java/net/schmizz/sshj/sftp/PacketReaderTest.java +++ b/src/test/java/net/schmizz/sshj/sftp/PacketReaderTest.java @@ -33,8 +33,6 @@ public class PacketReaderTest { private DataOutputStream dataout; private PacketReader reader; - private SFTPEngine engine; - private Subsystem subsystem; @Before public void setUp() throws Exception { @@ -42,8 +40,8 @@ public class PacketReaderTest { PipedInputStream pipedin = new PipedInputStream(pipedout); dataout = new DataOutputStream(pipedout); - engine = Mockito.mock(SFTPEngine.class); - subsystem = Mockito.mock(Subsystem.class); + SFTPEngine engine = Mockito.mock(SFTPEngine.class); + Subsystem subsystem = Mockito.mock(Subsystem.class); Mockito.when(engine.getLoggerFactory()).thenReturn(LoggerFactory.DEFAULT); Mockito.when(engine.getSubsystem()).thenReturn(subsystem); Mockito.when(subsystem.getInputStream()).thenReturn(pipedin); From 356ec9ed08c33033e1fb56718d27c1af262715c8 Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Fri, 20 Jan 2017 16:30:01 +0100 Subject: [PATCH 05/11] Upgraded to mina sshd 1.2.0 --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index f6d61f34..b45de622 100644 --- a/build.gradle +++ b/build.gradle @@ -37,7 +37,7 @@ dependencies { testCompile "junit:junit:4.11" testCompile 'org.spockframework:spock-core:1.0-groovy-2.4' testCompile "org.mockito:mockito-core:1.9.5" - testCompile "org.apache.sshd:sshd-core:1.1.0" + testCompile "org.apache.sshd:sshd-core:1.2.0" testRuntime "ch.qos.logback:logback-classic:1.1.2" testCompile 'org.glassfish.grizzly:grizzly-http-server:2.3.17' testCompile 'org.apache.httpcomponents:httpclient:4.5.2' From 920537dac96c73462db0f0bbb6fa26afe01330cf Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Fri, 20 Jan 2017 16:41:16 +0100 Subject: [PATCH 06/11] More cleaning up --- .../com/hierynomus/sshj/transport/cipher/StreamCipher.java | 1 - src/main/java/net/schmizz/keepalive/KeepAlive.java | 1 - src/main/java/net/schmizz/sshj/common/IOUtils.java | 2 -- .../schmizz/sshj/connection/channel/ChannelInputStream.java | 1 - src/main/java/net/schmizz/sshj/sftp/PathHelper.java | 4 ++-- src/main/java/net/schmizz/sshj/sftp/RemoteResource.java | 1 - src/main/java/net/schmizz/sshj/transport/Decoder.java | 2 -- src/main/java/net/schmizz/sshj/transport/Reader.java | 1 - 8 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/hierynomus/sshj/transport/cipher/StreamCipher.java b/src/main/java/com/hierynomus/sshj/transport/cipher/StreamCipher.java index 4eeda899..9913abe9 100644 --- a/src/main/java/com/hierynomus/sshj/transport/cipher/StreamCipher.java +++ b/src/main/java/com/hierynomus/sshj/transport/cipher/StreamCipher.java @@ -19,7 +19,6 @@ import net.schmizz.sshj.transport.cipher.BaseCipher; import java.security.InvalidAlgorithmParameterException; import java.security.InvalidKeyException; -import java.security.SecureRandom; public class StreamCipher extends BaseCipher { diff --git a/src/main/java/net/schmizz/keepalive/KeepAlive.java b/src/main/java/net/schmizz/keepalive/KeepAlive.java index 8490c651..ea7958ee 100644 --- a/src/main/java/net/schmizz/keepalive/KeepAlive.java +++ b/src/main/java/net/schmizz/keepalive/KeepAlive.java @@ -19,7 +19,6 @@ import net.schmizz.sshj.connection.ConnectionException; import net.schmizz.sshj.connection.ConnectionImpl; import net.schmizz.sshj.transport.TransportException; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public abstract class KeepAlive extends Thread { protected final Logger log; diff --git a/src/main/java/net/schmizz/sshj/common/IOUtils.java b/src/main/java/net/schmizz/sshj/common/IOUtils.java index 9421f838..72963ad8 100644 --- a/src/main/java/net/schmizz/sshj/common/IOUtils.java +++ b/src/main/java/net/schmizz/sshj/common/IOUtils.java @@ -15,8 +15,6 @@ */ package net.schmizz.sshj.common; -import org.slf4j.Logger; - import java.io.ByteArrayOutputStream; import java.io.Closeable; import java.io.IOException; diff --git a/src/main/java/net/schmizz/sshj/connection/channel/ChannelInputStream.java b/src/main/java/net/schmizz/sshj/connection/channel/ChannelInputStream.java index c48f5db2..5c7f1e64 100644 --- a/src/main/java/net/schmizz/sshj/connection/channel/ChannelInputStream.java +++ b/src/main/java/net/schmizz/sshj/connection/channel/ChannelInputStream.java @@ -20,7 +20,6 @@ import net.schmizz.sshj.connection.ConnectionException; import net.schmizz.sshj.transport.Transport; import net.schmizz.sshj.transport.TransportException; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.InputStream; diff --git a/src/main/java/net/schmizz/sshj/sftp/PathHelper.java b/src/main/java/net/schmizz/sshj/sftp/PathHelper.java index b9fb9e03..b40fcd11 100644 --- a/src/main/java/net/schmizz/sshj/sftp/PathHelper.java +++ b/src/main/java/net/schmizz/sshj/sftp/PathHelper.java @@ -73,7 +73,7 @@ public class PathHelper { if (path.equals(pathSep)) return getComponents("", ""); - if (path.isEmpty() || path.equals(".") || path.equals("." + pathSep)) + if (path.isEmpty() || ".".equals(path) || ("." + pathSep).equals(path)) return getComponents(getDotDir()); final String withoutTrailSep = trimTrailingSeparator(path); @@ -81,7 +81,7 @@ public class PathHelper { final String parent = (lastSep == -1) ? "" : withoutTrailSep.substring(0, lastSep); final String name = (lastSep == -1) ? withoutTrailSep : withoutTrailSep.substring(lastSep + pathSep.length()); - if (name.equals(".") || name.equals("..")) { + if (".".equals(name) || "..".equals(name)) { return getComponents(canonicalizer.canonicalize(path)); } else { return getComponents(parent, name); diff --git a/src/main/java/net/schmizz/sshj/sftp/RemoteResource.java b/src/main/java/net/schmizz/sshj/sftp/RemoteResource.java index cdcbb8db..1a4a0690 100644 --- a/src/main/java/net/schmizz/sshj/sftp/RemoteResource.java +++ b/src/main/java/net/schmizz/sshj/sftp/RemoteResource.java @@ -16,7 +16,6 @@ package net.schmizz.sshj.sftp; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.Closeable; import java.io.IOException; diff --git a/src/main/java/net/schmizz/sshj/transport/Decoder.java b/src/main/java/net/schmizz/sshj/transport/Decoder.java index bfb48b48..98e015b8 100644 --- a/src/main/java/net/schmizz/sshj/transport/Decoder.java +++ b/src/main/java/net/schmizz/sshj/transport/Decoder.java @@ -16,12 +16,10 @@ package net.schmizz.sshj.transport; import net.schmizz.sshj.common.*; -import net.schmizz.sshj.transport.Transport; import net.schmizz.sshj.transport.cipher.Cipher; import net.schmizz.sshj.transport.compression.Compression; import net.schmizz.sshj.transport.mac.MAC; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** Decodes packets from the SSH binary protocol per the current algorithms. */ final class Decoder diff --git a/src/main/java/net/schmizz/sshj/transport/Reader.java b/src/main/java/net/schmizz/sshj/transport/Reader.java index 30b4e8b8..f39aa15d 100644 --- a/src/main/java/net/schmizz/sshj/transport/Reader.java +++ b/src/main/java/net/schmizz/sshj/transport/Reader.java @@ -16,7 +16,6 @@ package net.schmizz.sshj.transport; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.InputStream; import java.net.SocketTimeoutException; From c883c87963425a8b856f3490c8d571d23f8b4589 Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Mon, 23 Jan 2017 09:55:04 +0100 Subject: [PATCH 07/11] Fixed a few warnings --- .../forwarded/RemotePortForwarderTest.java | 20 +++++++------- .../sshj/test/BaseAlgorithmTest.java | 26 +++++++------------ .../userauth/method/AuthPasswordTest.java | 3 ++- .../net/schmizz/sshj/sftp/PathHelperTest.java | 4 +-- 4 files changed, 23 insertions(+), 30 deletions(-) diff --git a/src/test/java/com/hierynomus/sshj/connection/channel/forwarded/RemotePortForwarderTest.java b/src/test/java/com/hierynomus/sshj/connection/channel/forwarded/RemotePortForwarderTest.java index c6b77c5d..a99a2dde 100644 --- a/src/test/java/com/hierynomus/sshj/connection/channel/forwarded/RemotePortForwarderTest.java +++ b/src/test/java/com/hierynomus/sshj/connection/channel/forwarded/RemotePortForwarderTest.java @@ -50,7 +50,7 @@ public class RemotePortForwarderTest { public HttpServer httpServer = new HttpServer(); @Before - public void setup() throws IOException { + public void setUp() throws IOException { fixture.getServer().setTcpipForwardingFilter(new AcceptAllForwardingFilter()); File file = httpServer.getDocRoot().newFile("index.html"); FileUtil.writeToFile(file, "

Hi!

"); @@ -59,49 +59,49 @@ public class RemotePortForwarderTest { @Test public void shouldHaveWorkingHttpServer() throws IOException { // Just to check that we have a working http server... - httpGet("127.0.0.1", 8080); + assertThat(httpGet("127.0.0.1", 8080), equalTo(200)); } @Test public void shouldDynamicallyForwardPortForLocalhost() throws IOException { SSHClient sshClient = getFixtureClient(); RemotePortForwarder.Forward bind = forwardPort(sshClient, "127.0.0.1", new SinglePort(0)); - httpGet("127.0.0.1", bind.getPort()); + assertThat(httpGet("127.0.0.1", bind.getPort()), equalTo(200)); } @Test public void shouldDynamicallyForwardPortForAllIPv4() throws IOException { SSHClient sshClient = getFixtureClient(); RemotePortForwarder.Forward bind = forwardPort(sshClient, "0.0.0.0", new SinglePort(0)); - httpGet("127.0.0.1", bind.getPort()); + assertThat(httpGet("127.0.0.1", bind.getPort()), equalTo(200)); } @Test public void shouldDynamicallyForwardPortForAllProtocols() throws IOException { SSHClient sshClient = getFixtureClient(); RemotePortForwarder.Forward bind = forwardPort(sshClient, "", new SinglePort(0)); - httpGet("127.0.0.1", bind.getPort()); + assertThat(httpGet("127.0.0.1", bind.getPort()), equalTo(200)); } @Test public void shouldForwardPortForLocalhost() throws IOException { SSHClient sshClient = getFixtureClient(); RemotePortForwarder.Forward bind = forwardPort(sshClient, "127.0.0.1", RANGE); - httpGet("127.0.0.1", bind.getPort()); + assertThat(httpGet("127.0.0.1", bind.getPort()), equalTo(200)); } @Test public void shouldForwardPortForAllIPv4() throws IOException { SSHClient sshClient = getFixtureClient(); RemotePortForwarder.Forward bind = forwardPort(sshClient, "0.0.0.0", RANGE); - httpGet("127.0.0.1", bind.getPort()); + assertThat(httpGet("127.0.0.1", bind.getPort()), equalTo(200)); } @Test public void shouldForwardPortForAllProtocols() throws IOException { SSHClient sshClient = getFixtureClient(); RemotePortForwarder.Forward bind = forwardPort(sshClient, "", RANGE); - httpGet("127.0.0.1", bind.getPort()); + assertThat(httpGet("127.0.0.1", bind.getPort()), equalTo(200)); } private RemotePortForwarder.Forward forwardPort(SSHClient sshClient, String address, PortRange portRange) throws IOException { @@ -122,12 +122,12 @@ public class RemotePortForwarderTest { } } - private void httpGet(String server, int port) throws IOException { + private int httpGet(String server, int port) throws IOException { HttpClient client = HttpClientBuilder.create().build(); String urlString = "http://" + server + ":" + port; System.out.println("Trying: GET " + urlString); HttpResponse execute = client.execute(new HttpGet(urlString)); - assertThat(execute.getStatusLine().getStatusCode(), equalTo(200)); + return execute.getStatusLine().getStatusCode(); } private SSHClient getFixtureClient() throws IOException { diff --git a/src/test/java/com/hierynomus/sshj/test/BaseAlgorithmTest.java b/src/test/java/com/hierynomus/sshj/test/BaseAlgorithmTest.java index d2f6ac7c..7567ac24 100644 --- a/src/test/java/com/hierynomus/sshj/test/BaseAlgorithmTest.java +++ b/src/test/java/com/hierynomus/sshj/test/BaseAlgorithmTest.java @@ -42,25 +42,17 @@ public abstract class BaseAlgorithmTest { @Test public void shouldVerifyAlgorithm() throws IOException { - attempt(100); - } - - private void attempt(int times) throws IOException { - for (int i = 0; i < times; i++) { + for (int i = 0; i < 100; i++) { logger.info("--> Attempt {}", i); - verify(); - } - } - - private void verify() throws IOException { - configureServer(fixture.getServer()); - fixture.start(); - Config config = getClientConfig(new DefaultConfig()); - SSHClient sshClient = fixture.connectClient(fixture.setupClient(config)); - assertThat("should be connected", sshClient.isConnected()); - sshClient.disconnect(); + configureServer(fixture.getServer()); + fixture.start(); + Config config = getClientConfig(new DefaultConfig()); + SSHClient sshClient = fixture.connectClient(fixture.setupClient(config)); + assertThat("should be connected", sshClient.isConnected()); + sshClient.disconnect(); // fixture.stopServer(); - fixture.stopClient(); + fixture.stopClient(); + } } protected abstract Config getClientConfig(DefaultConfig defaultConfig); diff --git a/src/test/java/com/hierynomus/sshj/userauth/method/AuthPasswordTest.java b/src/test/java/com/hierynomus/sshj/userauth/method/AuthPasswordTest.java index 9277edf4..1e61e45a 100644 --- a/src/test/java/com/hierynomus/sshj/userauth/method/AuthPasswordTest.java +++ b/src/test/java/com/hierynomus/sshj/userauth/method/AuthPasswordTest.java @@ -69,7 +69,7 @@ public class AuthPasswordTest { fixture.getServer().setPasswordAuthenticator(new PasswordAuthenticator() { @Override public boolean authenticate(String username, String password, ServerSession session) { - if (password.equals("changeme")) { + if ("changeme".equals(password)) { throw new PasswordChangeRequiredException("Password was changeme", "Please provide your updated password", "en_US"); } else { return password.equals(username); @@ -84,6 +84,7 @@ public class AuthPasswordTest { SSHClient sshClient = fixture.setupConnectedDefaultClient(); expectedException.expect(UserAuthException.class); sshClient.authPassword("jeroen", "changeme"); + assertThat("Should not have authenticated", !sshClient.isAuthenticated()); } @Test diff --git a/src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java b/src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java index f5465978..5c4f129f 100644 --- a/src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java +++ b/src/test/java/net/schmizz/sshj/sftp/PathHelperTest.java @@ -30,9 +30,9 @@ public class PathHelperTest { @Override public String canonicalize(String path) throws IOException { - if (path.equals("") || path.equals(".") || path.equals("./")) + if ("".equals(path) || ".".equals(path) || "./".equals(path)) return "/home/me"; - if (path.equals("..") || path.equals("../")) + if ("..".equals(path) || "../".equals(path)) return "/home"; return path; } From 81341135109241562f043437260dc737c2706fbf Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Mon, 23 Jan 2017 10:46:01 +0100 Subject: [PATCH 08/11] Cleaned up more PMD/Codacy warnings --- .../net/schmizz/sshj/examples/KeepAlive.java | 3 - .../net/schmizz/sshj/common/StreamCopier.java | 7 +- .../AbstractForwardedChannelOpener.java | 1 - .../net/schmizz/sshj/sftp/PacketReader.java | 1 - .../java/net/schmizz/sshj/sftp/Response.java | 3 +- .../sshj/signature/AbstractSignature.java | 2 +- .../sshj/signature/SignatureECDSA.java | 2 +- .../schmizz/sshj/transport/KeyExchanger.java | 2 +- .../schmizz/sshj/transport/TransportImpl.java | 65 ++++++++++--------- .../schmizz/sshj/userauth/UserAuthImpl.java | 24 ++++--- 10 files changed, 54 insertions(+), 56 deletions(-) diff --git a/examples/src/main/java/net/schmizz/sshj/examples/KeepAlive.java b/examples/src/main/java/net/schmizz/sshj/examples/KeepAlive.java index 0049b4e4..6c843811 100644 --- a/examples/src/main/java/net/schmizz/sshj/examples/KeepAlive.java +++ b/examples/src/main/java/net/schmizz/sshj/examples/KeepAlive.java @@ -3,14 +3,11 @@ package net.schmizz.sshj.examples; import net.schmizz.keepalive.KeepAliveProvider; import net.schmizz.sshj.DefaultConfig; import net.schmizz.sshj.SSHClient; -import net.schmizz.sshj.common.IOUtils; import net.schmizz.sshj.connection.channel.direct.Session; -import net.schmizz.sshj.connection.channel.direct.Session.Command; import net.schmizz.sshj.transport.verification.PromiscuousVerifier; import java.io.IOException; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; /** This examples demonstrates how to setup keep-alive to detect connection dropping. */ public class KeepAlive { diff --git a/src/main/java/net/schmizz/sshj/common/StreamCopier.java b/src/main/java/net/schmizz/sshj/common/StreamCopier.java index 8260b92e..1fedc075 100644 --- a/src/main/java/net/schmizz/sshj/common/StreamCopier.java +++ b/src/main/java/net/schmizz/sshj/common/StreamCopier.java @@ -68,8 +68,11 @@ public class StreamCopier { } public StreamCopier listener(Listener listener) { - if (listener == null) listener = NULL_LISTENER; - this.listener = listener; + if (listener == null) { + this.listener = NULL_LISTENER; + } else { + this.listener = listener; + } return this; } diff --git a/src/main/java/net/schmizz/sshj/connection/channel/forwarded/AbstractForwardedChannelOpener.java b/src/main/java/net/schmizz/sshj/connection/channel/forwarded/AbstractForwardedChannelOpener.java index d79835ee..b1a59563 100644 --- a/src/main/java/net/schmizz/sshj/connection/channel/forwarded/AbstractForwardedChannelOpener.java +++ b/src/main/java/net/schmizz/sshj/connection/channel/forwarded/AbstractForwardedChannelOpener.java @@ -21,7 +21,6 @@ import net.schmizz.sshj.connection.channel.Channel; import net.schmizz.sshj.connection.channel.OpenFailException; import net.schmizz.sshj.transport.TransportException; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; diff --git a/src/main/java/net/schmizz/sshj/sftp/PacketReader.java b/src/main/java/net/schmizz/sshj/sftp/PacketReader.java index 5dc29a33..a2db39ab 100644 --- a/src/main/java/net/schmizz/sshj/sftp/PacketReader.java +++ b/src/main/java/net/schmizz/sshj/sftp/PacketReader.java @@ -18,7 +18,6 @@ package net.schmizz.sshj.sftp; import net.schmizz.concurrent.Promise; import net.schmizz.sshj.common.SSHException; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.InputStream; diff --git a/src/main/java/net/schmizz/sshj/sftp/Response.java b/src/main/java/net/schmizz/sshj/sftp/Response.java index a44fac9d..c269291a 100644 --- a/src/main/java/net/schmizz/sshj/sftp/Response.java +++ b/src/main/java/net/schmizz/sshj/sftp/Response.java @@ -20,7 +20,7 @@ import net.schmizz.sshj.common.Buffer; public final class Response extends SFTPPacket { - public static enum StatusCode { + public enum StatusCode { UNKNOWN(-1), OK(0), EOF(1), @@ -122,6 +122,7 @@ public final class Response return ensurePacketTypeIs(PacketType.STATUS).ensureStatusIs(StatusCode.OK); } + @SuppressWarnings("PMD.CompareObjectsWithEquals") public Response ensureStatusIs(StatusCode acceptable) throws SFTPException { final StatusCode sc = readStatusCode(); diff --git a/src/main/java/net/schmizz/sshj/signature/AbstractSignature.java b/src/main/java/net/schmizz/sshj/signature/AbstractSignature.java index 89b39917..9cd44a6f 100644 --- a/src/main/java/net/schmizz/sshj/signature/AbstractSignature.java +++ b/src/main/java/net/schmizz/sshj/signature/AbstractSignature.java @@ -84,7 +84,7 @@ public abstract class AbstractSignature | sig[i++] & 0x000000ff; byte[] newSig = new byte[j]; System.arraycopy(sig, i, newSig, 0, j); - sig = newSig; + return newSig; } return sig; } diff --git a/src/main/java/net/schmizz/sshj/signature/SignatureECDSA.java b/src/main/java/net/schmizz/sshj/signature/SignatureECDSA.java index 0abe8ca8..93e228a6 100644 --- a/src/main/java/net/schmizz/sshj/signature/SignatureECDSA.java +++ b/src/main/java/net/schmizz/sshj/signature/SignatureECDSA.java @@ -79,7 +79,7 @@ public class SignatureECDSA throw new SSHRuntimeException(String.format("Signature :: ecdsa-sha2-nistp256 expected, got %s", algo)); } final int rsLen = sigbuf.readUInt32AsInt(); - if (!(sigbuf.available() == rsLen)) { + if (sigbuf.available() != rsLen) { throw new SSHRuntimeException("Invalid key length"); } r = sigbuf.readBytes(); diff --git a/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java b/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java index 998b0c80..12432063 100644 --- a/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java +++ b/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java @@ -26,7 +26,6 @@ import net.schmizz.sshj.transport.mac.MAC; import net.schmizz.sshj.transport.verification.AlgorithmsVerifier; import net.schmizz.sshj.transport.verification.HostKeyVerifier; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.math.BigInteger; import java.security.GeneralSecurityException; @@ -158,6 +157,7 @@ final class KeyExchanger "Key exchange packet received when key exchange was not ongoing"); } + @SuppressWarnings("PMD.CompareObjectsWithEquals") private static void ensureReceivedMatchesExpected(Message got, Message expected) throws TransportException { if (got != expected) diff --git a/src/main/java/net/schmizz/sshj/transport/TransportImpl.java b/src/main/java/net/schmizz/sshj/transport/TransportImpl.java index 6d2073d7..8aba4a62 100644 --- a/src/main/java/net/schmizz/sshj/transport/TransportImpl.java +++ b/src/main/java/net/schmizz/sshj/transport/TransportImpl.java @@ -33,7 +33,9 @@ import java.io.OutputStream; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; -/** A thread-safe {@link Transport} implementation. */ +/** + * A thread-safe {@link Transport} implementation. + */ public final class TransportImpl implements Transport, DisconnectListener { @@ -49,11 +51,11 @@ public final class TransportImpl static final class ConnInfo { final String host; - final int port; final InputStream in; final OutputStream out; - public ConnInfo(String host, int port, InputStream in, OutputStream out) { + + ConnInfo(String host, int port, InputStream in, OutputStream out) { this.host = host; this.port = port; this.in = in; @@ -88,24 +90,32 @@ public final class TransportImpl private final Event close; - /** Client version identification string */ + /** + * Client version identification string + */ private final String clientID; private volatile int timeoutMs = 30 * 1000; // Crazy long, but it was the original default private volatile boolean authed = false; - /** Currently active service e.g. UserAuthService, ConnectionService */ + /** + * Currently active service e.g. UserAuthService, ConnectionService + */ private volatile Service service; private DisconnectListener disconnectListener; private ConnInfo connInfo; - /** Server version identification string */ + /** + * Server version identification string + */ private String serverID; - /** Message identifier of last packet received */ + /** + * Message identifier of last packet received + */ private Message msg; private final ReentrantLock writeLock = new ReentrantLock(); @@ -115,9 +125,9 @@ public final class TransportImpl this.loggerFactory = config.getLoggerFactory(); this.serviceAccept = new Event("service accept", TransportException.chainer, loggerFactory); this.close = new Event("transport close", TransportException.chainer, loggerFactory); - this.nullService = new NullService(this); + this.nullService = new NullService(this); this.service = nullService; - this.log = loggerFactory.getLogger(getClass()); + this.log = loggerFactory.getLogger(getClass()); this.disconnectListener = this; this.reader = new Reader(this); this.encoder = new Encoder(config.getRandomFactory().create(), writeLock, loggerFactory); @@ -138,7 +148,7 @@ public final class TransportImpl this.serviceAccept = new Event("service accept", TransportException.chainer, loggerFactory); this.close = new Event("transport close", TransportException.chainer, loggerFactory); this.log = loggerFactory.getLogger(getClass()); - this.nullService = new NullService(this); + this.nullService = new NullService(this); this.service = nullService; this.disconnectListener = this; this.reader = new Reader(this); @@ -194,6 +204,7 @@ public final class TransportImpl /** * Receive the server identification string. + * * @throws IOException If there was an error writing to the outputstream. */ private void sendClientIdent() throws IOException { @@ -212,9 +223,7 @@ public final class TransportImpl * This is not efficient but is only done once. * * @param buffer The buffer to read from. - * * @return empty string if full ident string has not yet been received - * * @throws IOException */ private String readIdentification(Buffer.PlainBuffer buffer) @@ -226,7 +235,7 @@ public final class TransportImpl if (!ident.startsWith("SSH-2.0-") && !ident.startsWith("SSH-1.99-")) throw new TransportException(DisconnectReason.PROTOCOL_VERSION_NOT_SUPPORTED, - "Server does not support SSHv2, identified as: " + ident); + "Server does not support SSHv2, identified as: " + ident); return ident; } @@ -337,7 +346,6 @@ public final class TransportImpl * Sends a service request for the specified service * * @param serviceName name of the service being requested - * * @throws TransportException if there is an error while sending the request */ private void sendServiceRequest(String serviceName) @@ -456,9 +464,9 @@ public final class TransportImpl log.debug("Sending SSH_MSG_DISCONNECT: reason=[{}], msg=[{}]", reason, message); try { write(new SSHPacket(Message.DISCONNECT) - .putUInt32(reason.toInt()) - .putString(message) - .putString("")); + .putUInt32(reason.toInt()) + .putString(message) + .putString("")); } catch (IOException worthless) { log.debug("Error writing packet: {}", worthless.toString()); } @@ -476,7 +484,6 @@ public final class TransportImpl * * @param msg the message identifer * @param buf buffer containg rest of the packet - * * @throws SSHException if an error occurs during handling (unrecoverable) */ @Override @@ -494,32 +501,27 @@ public final class TransportImpl else switch (msg) { - case DISCONNECT: { + case DISCONNECT: gotDisconnect(buf); break; - } - case IGNORE: { + case IGNORE: log.debug("Received SSH_MSG_IGNORE"); break; - } - case UNIMPLEMENTED: { + case UNIMPLEMENTED: gotUnimplemented(buf); break; - } - case DEBUG: { + case DEBUG: gotDebug(buf); break; - } - case SERVICE_ACCEPT: { + case SERVICE_ACCEPT: gotServiceAccept(); break; - } - case USERAUTH_BANNER: { + case USERAUTH_BANNER: log.debug("Received USERAUTH_BANNER"); break; - } default: sendUnimplemented(); + break; } } @@ -552,7 +554,7 @@ public final class TransportImpl try { if (!serviceAccept.hasWaiters()) throw new TransportException(DisconnectReason.PROTOCOL_ERROR, - "Got a service accept notification when none was awaited"); + "Got a service accept notification when none was awaited"); serviceAccept.set(); } finally { serviceAccept.unlock(); @@ -563,7 +565,6 @@ public final class TransportImpl * Got an SSH_MSG_UNIMPLEMENTED, so lets see where we're at and act accordingly. * * @param packet The 'unimplemented' packet received - * * @throws TransportException */ private void gotUnimplemented(SSHPacket packet) diff --git a/src/main/java/net/schmizz/sshj/userauth/UserAuthImpl.java b/src/main/java/net/schmizz/sshj/userauth/UserAuthImpl.java index 4171e49a..d6b7cc77 100644 --- a/src/main/java/net/schmizz/sshj/userauth/UserAuthImpl.java +++ b/src/main/java/net/schmizz/sshj/userauth/UserAuthImpl.java @@ -32,7 +32,9 @@ import java.util.LinkedList; import java.util.List; import java.util.concurrent.TimeUnit; -/** {@link UserAuth} implementation. */ +/** + * {@link UserAuth} implementation. + */ public class UserAuthImpl extends AbstractService implements UserAuth { @@ -111,22 +113,20 @@ public class UserAuthImpl try { switch (msg) { - case USERAUTH_BANNER: { + case USERAUTH_BANNER: banner = buf.readString(); - } - break; + break; - case USERAUTH_SUCCESS: { + case USERAUTH_SUCCESS: // In order to prevent race conditions, we immediately set the authenticated flag on the transport // And change the service before delivering the authenticated promise. // Should fix https://github.com/hierynomus/sshj/issues/237 trans.setAuthenticated(); // So it can put delayed compression into force if applicable trans.setService(nextService); // We aren't in charge anymore, next service is authenticated.deliver(true); - } - break; + break; - case USERAUTH_FAILURE: { + case USERAUTH_FAILURE: allowedMethods = Arrays.asList(buf.readString().split(",")); partialSuccess |= buf.readBoolean(); if (allowedMethods.contains(currentMethod.getName()) && currentMethod.shouldRetry()) { @@ -134,18 +134,16 @@ public class UserAuthImpl } else { authenticated.deliver(false); } - } - break; + break; - default: { + default: log.debug("Asking `{}` method to handle {} packet", currentMethod.getName(), msg); try { currentMethod.handle(msg, buf); } catch (UserAuthException e) { authenticated.deliverError(e); } - } - + break; } } finally { authenticated.unlock(); From ef3f7a2eaf02652a35516b397f4cd76564409f82 Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Mon, 23 Jan 2017 14:40:33 +0100 Subject: [PATCH 09/11] Fixed more warnings --- .../transport/IdentificationStringParser.java | 7 +++++-- .../net/schmizz/sshj/common/SecurityUtils.java | 2 +- .../connection/channel/AbstractChannel.java | 2 ++ .../net/schmizz/sshj/sftp/PacketReader.java | 11 ++++++----- .../java/net/schmizz/sshj/sftp/RemoteFile.java | 18 +++++++++--------- .../sshj/transport/cipher/NoneCipher.java | 2 ++ .../schmizz/sshj/util/gss/BogusGSSContext.java | 4 +++- .../sshj/util/gss/BogusGSSCredential.java | 4 +++- 8 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/hierynomus/sshj/transport/IdentificationStringParser.java b/src/main/java/com/hierynomus/sshj/transport/IdentificationStringParser.java index 7c23475e..cb55e5ce 100644 --- a/src/main/java/com/hierynomus/sshj/transport/IdentificationStringParser.java +++ b/src/main/java/com/hierynomus/sshj/transport/IdentificationStringParser.java @@ -62,8 +62,11 @@ public class IdentificationStringParser { } } - private void logHeaderLine(Buffer.PlainBuffer lineBuffer) { - + private void logHeaderLine(Buffer.PlainBuffer lineBuffer) throws Buffer.BufferException { + byte[] bytes = new byte[lineBuffer.available()]; + lineBuffer.readRawBytes(bytes); + String header = new String(bytes, 0, bytes.length - 1); + log.debug("Received header: {}", header); } private String readIdentification(Buffer.PlainBuffer lineBuffer) throws Buffer.BufferException, TransportException { diff --git a/src/main/java/net/schmizz/sshj/common/SecurityUtils.java b/src/main/java/net/schmizz/sshj/common/SecurityUtils.java index 67ff7669..a549b90c 100644 --- a/src/main/java/net/schmizz/sshj/common/SecurityUtils.java +++ b/src/main/java/net/schmizz/sshj/common/SecurityUtils.java @@ -254,7 +254,7 @@ public class SecurityUtils { throw new SSHRuntimeException("Failed to register BouncyCastle as the defaut JCE provider"); } } + registrationDone = true; } - registrationDone = true; } } diff --git a/src/main/java/net/schmizz/sshj/connection/channel/AbstractChannel.java b/src/main/java/net/schmizz/sshj/connection/channel/AbstractChannel.java index fa89f028..2698d251 100644 --- a/src/main/java/net/schmizz/sshj/connection/channel/AbstractChannel.java +++ b/src/main/java/net/schmizz/sshj/connection/channel/AbstractChannel.java @@ -329,6 +329,8 @@ public abstract class AbstractChannel protected void gotUnknown(Message msg, SSHPacket buf) throws ConnectionException, TransportException { + log.warn("Got unknown packet with type {}", msg); + } protected void handleRequest(String reqType, SSHPacket buf) diff --git a/src/main/java/net/schmizz/sshj/sftp/PacketReader.java b/src/main/java/net/schmizz/sshj/sftp/PacketReader.java index a2db39ab..30233ed4 100644 --- a/src/main/java/net/schmizz/sshj/sftp/PacketReader.java +++ b/src/main/java/net/schmizz/sshj/sftp/PacketReader.java @@ -24,10 +24,11 @@ import java.io.InputStream; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -public class PacketReader - extends Thread { +public class PacketReader extends Thread { - /** Logger */ + /** + * Logger + */ private final Logger log; private final InputStream in; @@ -63,7 +64,7 @@ public class PacketReader | lenBuf[3] & 0x000000ffL); if (len > SFTPPacket.MAX_SIZE) { - throw new SSHException(String.format("Indicated packet length %d too large", len)); + throw new SSHException(String.format("Indicated packet length %d too large", len)); } return (int) len; @@ -99,7 +100,7 @@ public class PacketReader log.debug("Received {} packet", resp.getType()); if (promise == null) throw new SFTPException("Received [" + resp.readType() + "] response for request-id " + resp.getRequestID() - + ", no such request was made"); + + ", no such request was made"); else promise.deliver(resp); } diff --git a/src/main/java/net/schmizz/sshj/sftp/RemoteFile.java b/src/main/java/net/schmizz/sshj/sftp/RemoteFile.java index 24043c21..14d054e1 100644 --- a/src/main/java/net/schmizz/sshj/sftp/RemoteFile.java +++ b/src/main/java/net/schmizz/sshj/sftp/RemoteFile.java @@ -81,10 +81,10 @@ public class RemoteFile protected Promise asyncWrite(long fileOffset, byte[] data, int off, int len) throws IOException { return requester.request(newRequest(PacketType.WRITE) - .putUInt64(fileOffset) - // TODO The SFTP spec claims this field is unneeded...? See #187 - .putUInt32(len) - .putRawBytes(data, off, len) + .putUInt64(fileOffset) + // TODO The SFTP spec claims this field is unneeded...? See #187 + .putUInt32(len) + .putRawBytes(data, off, len) ); } @@ -194,10 +194,10 @@ public class RemoteFile @Override public long skip(long n) throws IOException { - final long fileLength = length(); - final Long previousFileOffset = fileOffset; - fileOffset = Math.min(fileOffset + n, fileLength); - return fileOffset - previousFileOffset; + final long fileLength = length(); + final Long previousFileOffset = fileOffset; + fileOffset = Math.min(fileOffset + n, fileLength); + return fileOffset - previousFileOffset; } @Override @@ -341,7 +341,7 @@ public class RemoteFile public int available() throws IOException { boolean lastRead = true; while (!eof && (pending.available() <= 0) && lastRead) { - lastRead = retrieveUnconfirmedRead(false /*blocking*/); + lastRead = retrieveUnconfirmedRead(false /*blocking*/); } return pending.available(); } diff --git a/src/main/java/net/schmizz/sshj/transport/cipher/NoneCipher.java b/src/main/java/net/schmizz/sshj/transport/cipher/NoneCipher.java index e4f47fba..d576d3c6 100644 --- a/src/main/java/net/schmizz/sshj/transport/cipher/NoneCipher.java +++ b/src/main/java/net/schmizz/sshj/transport/cipher/NoneCipher.java @@ -46,10 +46,12 @@ public class NoneCipher @Override public void init(Mode mode, byte[] bytes, byte[] bytes1) { + // Nothing to do } @Override public void update(byte[] input, int inputOffset, int inputLen) { + // Nothing to do } } diff --git a/src/test/java/net/schmizz/sshj/util/gss/BogusGSSContext.java b/src/test/java/net/schmizz/sshj/util/gss/BogusGSSContext.java index 8f650756..892faac2 100644 --- a/src/test/java/net/schmizz/sshj/util/gss/BogusGSSContext.java +++ b/src/test/java/net/schmizz/sshj/util/gss/BogusGSSContext.java @@ -70,7 +70,9 @@ public class BogusGSSContext } @Override - public void dispose() throws GSSException {} + public void dispose() throws GSSException { + // Nothing to do + } @Override public int getWrapSizeLimit(int qop, boolean confReq, int maxTokenSize) throws GSSException { diff --git a/src/test/java/net/schmizz/sshj/util/gss/BogusGSSCredential.java b/src/test/java/net/schmizz/sshj/util/gss/BogusGSSCredential.java index a211b5ca..6a747e93 100644 --- a/src/test/java/net/schmizz/sshj/util/gss/BogusGSSCredential.java +++ b/src/test/java/net/schmizz/sshj/util/gss/BogusGSSCredential.java @@ -34,7 +34,9 @@ public class BogusGSSCredential } @Override - public void dispose() throws GSSException {} + public void dispose() throws GSSException { + // Nothing to do + } @Override public GSSName getName() throws GSSException { From 40f956b4b666f14291b2cf4880f9a2b7acde4522 Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Mon, 23 Jan 2017 15:33:22 +0100 Subject: [PATCH 10/11] More code cleanup --- .../java/net/schmizz/sshj/common/Buffer.java | 4 ++-- .../net/schmizz/sshj/common/StreamCopier.java | 20 ++++++++-------- .../sshj/connection/ConnectionImpl.java | 16 ++++++------- .../channel/ChannelOutputStream.java | 24 ++++++++----------- .../forwarded/RemotePortForwarder.java | 2 +- .../schmizz/sshj/sftp/RemoteDirectory.java | 3 ++- .../schmizz/sshj/sftp/SFTPFileTransfer.java | 13 ++++------ 7 files changed, 38 insertions(+), 44 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/common/Buffer.java b/src/main/java/net/schmizz/sshj/common/Buffer.java index 2c809774..d5887f2c 100644 --- a/src/main/java/net/schmizz/sshj/common/Buffer.java +++ b/src/main/java/net/schmizz/sshj/common/Buffer.java @@ -311,7 +311,7 @@ public class Buffer> { public T putUInt32(long uint32) { ensureCapacity(4); if (uint32 < 0 || uint32 > 0xffffffffL) - throw new RuntimeException("Invalid value: " + uint32); + throw new IllegalArgumentException("Invalid value: " + uint32); data[wpos++] = (byte) (uint32 >> 24); data[wpos++] = (byte) (uint32 >> 16); data[wpos++] = (byte) (uint32 >> 8); @@ -346,7 +346,7 @@ public class Buffer> { @SuppressWarnings("unchecked") public T putUInt64(long uint64) { if (uint64 < 0) - throw new RuntimeException("Invalid value: " + uint64); + throw new IllegalArgumentException("Invalid value: " + uint64); data[wpos++] = (byte) (uint64 >> 56); data[wpos++] = (byte) (uint64 >> 48); data[wpos++] = (byte) (uint64 >> 40); diff --git a/src/main/java/net/schmizz/sshj/common/StreamCopier.java b/src/main/java/net/schmizz/sshj/common/StreamCopier.java index 1fedc075..2cf0d0ac 100644 --- a/src/main/java/net/schmizz/sshj/common/StreamCopier.java +++ b/src/main/java/net/schmizz/sshj/common/StreamCopier.java @@ -15,7 +15,6 @@ */ package net.schmizz.sshj.common; -import net.schmizz.sshj.common.LoggerFactory; import net.schmizz.concurrent.Event; import net.schmizz.concurrent.ExceptionChainer; import org.slf4j.Logger; @@ -129,11 +128,13 @@ public class StreamCopier { final long startTime = System.currentTimeMillis(); if (length == -1) { - while ((read = in.read(buf)) != -1) - count = write(buf, count, read); + while ((read = in.read(buf)) != -1) { + count += write(buf, count, read); + } } else { - while (count < length && (read = in.read(buf, 0, (int) Math.min(bufSize, length - count))) != -1) - count = write(buf, count, read); + while (count < length && (read = in.read(buf, 0, (int) Math.min(bufSize, length - count))) != -1) { + count += write(buf, count, read); + } } if (!keepFlushing) @@ -149,14 +150,13 @@ public class StreamCopier { return count; } - private long write(byte[] buf, long count, int read) + private long write(byte[] buf, long curPos, int len) throws IOException { - out.write(buf, 0, read); - count += read; + out.write(buf, 0, len); if (keepFlushing) out.flush(); - listener.reportProgress(count); - return count; + listener.reportProgress(curPos + len); + return len; } } diff --git a/src/main/java/net/schmizz/sshj/connection/ConnectionImpl.java b/src/main/java/net/schmizz/sshj/connection/ConnectionImpl.java index 1a7b2ce5..5657d3a1 100644 --- a/src/main/java/net/schmizz/sshj/connection/ConnectionImpl.java +++ b/src/main/java/net/schmizz/sshj/connection/ConnectionImpl.java @@ -126,10 +126,9 @@ public class ConnectionImpl @Override public void handle(Message msg, SSHPacket buf) throws SSHException { - if (msg.in(91, 100)) + if (msg.in(91, 100)) { getChannel(buf).handle(msg, buf); - - else if (msg.in(80, 90)) + } else if (msg.in(80, 90)) { switch (msg) { case REQUEST_SUCCESS: gotGlobalReqResponse(buf); @@ -142,10 +141,11 @@ public class ConnectionImpl break; default: super.handle(msg, buf); + break; } - - else + } else { super.handle(msg, buf); + } } @Override @@ -174,11 +174,11 @@ public class ConnectionImpl } @Override - public void join() - throws InterruptedException { + public void join() throws InterruptedException { synchronized (internalSynchronizer) { - while (!channels.isEmpty()) + while (!channels.isEmpty()) { internalSynchronizer.wait(); + } } } diff --git a/src/main/java/net/schmizz/sshj/connection/channel/ChannelOutputStream.java b/src/main/java/net/schmizz/sshj/connection/channel/ChannelOutputStream.java index 184c2a55..9610dd39 100644 --- a/src/main/java/net/schmizz/sshj/connection/channel/ChannelOutputStream.java +++ b/src/main/java/net/schmizz/sshj/connection/channel/ChannelOutputStream.java @@ -27,9 +27,7 @@ import java.io.OutputStream; * {@link OutputStream} for channels. Buffers data upto the remote window's maximum packet size. Data can also be * flushed via {@link #flush()} and is also flushed on {@link #close()}. */ -public final class ChannelOutputStream - extends OutputStream - implements ErrorNotifiable { +public final class ChannelOutputStream extends OutputStream implements ErrorNotifiable { private final Channel chan; private final Transport trans; @@ -56,8 +54,7 @@ public final class ChannelOutputStream dataOffset = packet.wpos(); } - int write(byte[] data, int off, int len) - throws TransportException, ConnectionException { + int write(byte[] data, int off, int len) throws TransportException, ConnectionException { final int bufferSize = packet.wpos() - dataOffset; if (bufferSize >= win.getMaxPacketSize()) { flush(bufferSize, true); @@ -69,15 +66,12 @@ public final class ChannelOutputStream } } - boolean flush(boolean canAwaitExpansion) - throws TransportException, ConnectionException { + boolean flush(boolean canAwaitExpansion) throws TransportException, ConnectionException { return flush(packet.wpos() - dataOffset, canAwaitExpansion); } - boolean flush(int bufferSize, boolean canAwaitExpansion) - throws TransportException, ConnectionException { + boolean flush(int bufferSize, boolean canAwaitExpansion) throws TransportException, ConnectionException { while (bufferSize > 0) { - long remoteWindowSize = win.getSize(); if (remoteWindowSize == 0) { if (canAwaitExpansion) { @@ -140,10 +134,12 @@ public final class ChannelOutputStream public synchronized void write(final byte[] data, int off, int len) throws IOException { checkClose(); - while (len > 0) { - final int n = buffer.write(data, off, len); - off += n; - len -= n; + int length = len; + int offset = off; + while (length > 0) { + final int n = buffer.write(data, offset, len); + offset += n; + length -= n; } } diff --git a/src/main/java/net/schmizz/sshj/connection/channel/forwarded/RemotePortForwarder.java b/src/main/java/net/schmizz/sshj/connection/channel/forwarded/RemotePortForwarder.java index 90b673e4..01465763 100644 --- a/src/main/java/net/schmizz/sshj/connection/channel/forwarded/RemotePortForwarder.java +++ b/src/main/java/net/schmizz/sshj/connection/channel/forwarded/RemotePortForwarder.java @@ -130,7 +130,7 @@ public class RemotePortForwarder // Addresses match up return true; } - if ("localhost".equals(address) && (channelForward.address.equals("127.0.0.1") || channelForward.address.equals("::1"))) { + if ("localhost".equals(address) && ("127.0.0.1".equals(channelForward.address) || "::1".equals(channelForward.address))) { // Localhost special case. return true; } diff --git a/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java b/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java index 945918b7..f804a381 100644 --- a/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java +++ b/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java @@ -46,8 +46,9 @@ public class RemoteDirectory final FileAttributes attrs = res.readFileAttributes(); final PathComponents comps = requester.getPathHelper().getComponents(path, name); final RemoteResourceInfo inf = new RemoteResourceInfo(comps, attrs); - if (!(name.equals(".") || name.equals("..")) && (filter == null || filter.accept(inf))) + if (!(".".equals(name) || "..".equals(name)) && (filter == null || filter.accept(inf))) { rri.add(inf); + } } break; diff --git a/src/main/java/net/schmizz/sshj/sftp/SFTPFileTransfer.java b/src/main/java/net/schmizz/sshj/sftp/SFTPFileTransfer.java index e84094b5..6a260d2e 100644 --- a/src/main/java/net/schmizz/sshj/sftp/SFTPFileTransfer.java +++ b/src/main/java/net/schmizz/sshj/sftp/SFTPFileTransfer.java @@ -60,14 +60,12 @@ public class SFTPFileTransfer } @Override - public void upload(LocalSourceFile localFile, String remotePath) - throws IOException { + public void upload(LocalSourceFile localFile, String remotePath) throws IOException { new Uploader(localFile, remotePath).upload(getTransferListener()); } @Override - public void download(String source, LocalDestFile dest) - throws IOException { + public void download(String source, LocalDestFile dest) throws IOException { final PathComponents pathComponents = engine.getPathHelper().getComponents(source); final FileAttributes attributes = engine.stat(source); new Downloader().download(getTransferListener(), new RemoteResourceInfo(pathComponents, attributes), dest); @@ -91,10 +89,10 @@ public class SFTPFileTransfer private class Downloader { + @SuppressWarnings("PMD.MissingBreakInSwitch") private void download(final TransferListener listener, final RemoteResourceInfo remote, - final LocalDestFile local) - throws IOException { + final LocalDestFile local) throws IOException { final LocalDestFile adjustedFile; switch (remote.getAttributes().getType()) { case DIRECTORY: @@ -104,8 +102,7 @@ public class SFTPFileTransfer log.warn("Server did not supply information about the type of file at `{}` " + "-- assuming it is a regular file!", remote.getPath()); case REGULAR: - adjustedFile = downloadFile(listener.file(remote.getName(), remote.getAttributes().getSize()), - remote, local); + adjustedFile = downloadFile(listener.file(remote.getName(), remote.getAttributes().getSize()), remote, local); break; default: throw new IOException(remote + " is not a regular file or directory"); From 0b397bc3d7bf1be5cdef0c4262785d85b55f6e06 Mon Sep 17 00:00:00 2001 From: Jeroen van Erp Date: Mon, 23 Jan 2017 15:33:47 +0100 Subject: [PATCH 11/11] Added TODO --- src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java b/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java index f804a381..58507b5a 100644 --- a/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java +++ b/src/main/java/net/schmizz/sshj/sftp/RemoteDirectory.java @@ -32,6 +32,7 @@ public class RemoteDirectory public List scan(RemoteResourceFilter filter) throws IOException { List rri = new LinkedList(); + // TODO: Remove GOTO! loop: for (; ; ) { final Response res = requester.request(newRequest(PacketType.READDIR))