mwiede / jsch

fork of the popular jsch library
Other
671 stars 124 forks source link

InvalidAlgorithmParameterException when using DHE bitsize larger than 1024 (non-BC) #418

Open mvegter opened 8 months ago

mvegter commented 8 months ago
kex: algorithm: diffie-hellman-group-exchange-sha256
kex: host key algorithm: ssh-rsa
kex: server->client cipher: aes128-ctr MAC: hmac-sha2-256 compression: none
kex: client->server cipher: aes128-ctr

SSH_MSG_KEX_DH_GEX_REQUEST(2048<8192<8192)
> DH key size must be multiple of 64, and can only range from 512 to 8192 (inclusive). 
  The specific key size 4095 is not supported

SSH_MSG_KEX_DH_GEX_REQUEST(2048<3072<8192)
> DH key size must be multiple of 64, and can only range from 512 to 8192 (inclusive).
  The specific key size 3191 is not supported

SSH_MSG_KEX_DH_GEX_REQUEST(1024<2048<8192)
> DH key size must be multiple of 64, and can only range from 512 to 8192 (inclusive).
  The specific key size 3191 is not supported

SSH_MSG_KEX_DH_GEX_REQUEST(1024<2048<2048)
> DH key size must be multiple of 64, and can only range from 512 to 8192 (inclusive).
  The specific key size 2047 is not supported

SSH_MSG_KEX_DH_GEX_REQUEST(1024<1024<2048)
> Success
Caused by: java.security.InvalidAlgorithmParameterException: DH key size...
    at com.sun.crypto.provider.DHKeyPairGenerator.initialize(DHKeyPairGenerator.java:140) ~[?:?]
    at java.security.KeyPairGenerator$Delegate.initialize(KeyPairGenerator.java:698) ~[?:?]
    at java.security.KeyPairGenerator.initialize(KeyPairGenerator.java:436) ~[?:?]
    at com.jcraft.jsch.jce.DH.getE(DH.java:58) ~[jsch-0.2.11.jar:0.2.11]
    at com.jcraft.jsch.DHGEX.next(DHGEX.java:137) ~[jsch-0.2.11.jar:0.2.11]
    at com.jcraft.jsch.Session.connect(Session.java:326) ~[jsch-0.2.11.jar:0.2.11]
-Djsch.dhgex_min=1024 -Djsch.dhgex_preferred=2048 -Djsch.dhgex_max=8192

Played around with the dhgex properties, and only dhgex_preferred = 1024 seemed to work without having BouncyCastle . I would not expect this given https://bugs.openjdk.org/browse/JDK-8072452 Support DHE sizes up to 8192-bits and DSA sizes up to 3072-bits as I'm running on 17

mvegter commented 8 months ago
SSH_MSG_KEX_DH_GEX_REQUEST(2048<3072<8192)

        dh.setP(p); // p.length is 399

Which translates nicely into 3192 , which should be valid . Perhaps this is related https://bugs.openjdk.org/browse/JDK-8255283 / Leading zeros in Diffie-Hellman keys are ignored . Seen on two different remote servers, both having remote version SSH-2.0-Sun_SSH_1.1.9

norrisjeremy commented 7 months ago

Hi @mvegter,

I am unable to reproduce the behavior you are reporting using our built-in integration tests and Bouncy Castle excluded from the classpath via the following:

mvn failsafe:integration-test -DskipITs=false -Dit.test=AlgorithmsIT#testDHGEXSizes -Dmaven.test.dependency.excludes=org.bouncycastle:bcprov-jdk18on

Reading through the notes at JDK-8255283, it seems this behavior may be extremely specific to quirks to the server being connected. I'm also not sure how JSch could workaround this issue, since it seems the issue is with the JDK's built-in security provider. You may simply have to either use Bouncy Castle or simply restrict the DH group sizes as you have found.

Thanks, Jeremy

mvegter commented 7 months ago

Hey @norrisjeremy , yeah I intercepted the dh.setP(p); and could confirm it was the correct number of elements in the array, but after providing that to the BigInteger we lost one (leading zero). So I'm afraid that there is nothing to do about this. On our end we indeed lowered the min/preferred DH group size to work around this, which is a shame.

norrisjeremy commented 7 months ago

Hi @mvegter,

Yes, I think the code that is failing the check is here. It extracts the bitLength() of the P BigInteger, which according to the Javadocs:

Returns the number of bits in the minimal two's-complement representation of this BigInteger, excluding a sign bit.

So if the P value from the server includes a zero in it's most significant bit, then that won't be counted in the bitLength(). For example:

jshell> new BigInteger(1, new byte[]{(byte)0x7f, (byte)0xff}).bitLength();
$1 ==> 15

So even though we pass a two-element byte array (which should be 16 bits in length) to the BigInteger constructor, bitLength() returns a value of 15, since the most significant bit is zero.

I suspect that is why you are getting errors where the key size is values like 4095, 3191, 2047, etc.

I'm not sure if there's anything we can do, since the internal DHKeyPairGenerator class operates strictly on BigInteger values.

Does this server (SSH-2.0-Sun_SSH_1.1.9) always fail every time using > 1024, or is it intermittent?

Thanks, Jeremy

mvegter commented 6 months ago

Does this server (SSH-2.0-Sun_SSH_1.1.9) always fail every time using > 1024, or is it intermittent?

Hey @norrisjeremy , sorry for the late reply but yes it fails every time.

austinarbor commented 3 months ago

@mvegter can you describe your workaround more? Currently seeing this with an sftp server running on RHEL 6 (OpenSSH 5.3) when connecting using java 8. I'm also unable to reproduce myself.

A user has sent logs of them successfully connecting to the server using the old jsch version 0.1.54. This is all pretty outside my wheelhouse, but could it possibly be related to https://github.com/mwiede/jsch/commit/db4bf8bb ? That seems like the only commit that made changes of significance to the DH code since 0.1.54

mvegter commented 3 months ago

@austinarbor the defaults have changed in 7d450c7e7ca628180022c38fb47b6fb963fd918a , for the given connections we simply resorted to using the old defaults of 1024 for both the jsch.dhgex_min and jsch.dhgex_preferred

austinarbor commented 3 months ago

@mvegter thanks! I actually ended up already having BC on my classpath and was in a bit of a time crunch to get things into a working state so I went with the BC approach

JackieJK commented 2 months ago

encountered this problem while using jsch0.2.17 with Sun_SSH_1.1