[1/6] mina-sshd git commit: [SSHD-846] Nullify AbstractSession KEX instance before marking state as DONE to avoid race condition and possible NPE

[l***@apache.org]

Repository: mina-sshd
Updated Branches:
refs/heads/master 28c52e91c -> 8ec4b9c36


[SSHD-846] Nullify AbstractSession KEX instance before marking state as DONE to avoid race condition and possible NPE


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/8ec4b9c3
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/8ec4b9c3
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/8ec4b9c3

Branch: refs/heads/master
Commit: 8ec4b9c36cd28e40cc81cf9f13f30706459295c9
Parents: 29d7712
Author: Goldstein Lyor <***@cb4.com>
Authored: Tue Oct 2 18:01:14 2018 +0300
Committer: Lyor Goldstein <***@gmail.com>
Committed: Wed Oct 3 20:05:17 2018 +0300

----------------------------------------------------------------------
.../sshd/common/session/helpers/AbstractSession.java | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/8ec4b9c3/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
index 287c8cd..ce4c3fe 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
@@ -849,9 +849,11 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen
Map<KexProposalOption, String> result = negotiate();
String kexAlgorithm = result.get(KexProposalOption.ALGORITHMS);
Collection<? extends NamedFactory<KeyExchange>> kexFactories = getKeyExchangeFactories();
- kex = ValidateUtils.checkNotNull(NamedFactory.create(kexFactories, kexAlgorithm),
- "Unknown negotiated KEX algorithm: %s",
- kexAlgorithm);
+ synchronized (pendingPackets) {
+ kex = ValidateUtils.checkNotNull(NamedFactory.create(kexFactories, kexAlgorithm),
+ "Unknown negotiated KEX algorithm: %s",
+ kexAlgorithm);
+ }

byte[] v_s = serverVersion.getBytes(StandardCharsets.UTF_8);
byte[] v_c = clientVersion.getBytes(StandardCharsets.UTF_8);
@@ -889,8 +891,8 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen
Collection<? extends Map.Entry<? extends SshFutureListener<IoWriteFuture>, IoWriteFuture>> pendingWrites;
synchronized (pendingPackets) {
pendingWrites = sendPendingPackets(pendingPackets);
- kexState.set(KexState.DONE);
kex = null; // discard and GC since KEX is completed
+ kexState.set(KexState.DONE);
}

int pendingCount = pendingWrites.size();

[l***@apache.org]

[SSHD-846] Validate non-null 'f' value during DH KEX


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/dd542ce3
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/dd542ce3
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/dd542ce3

Branch: refs/heads/master
Commit: dd542ce316e4eb4846698d885f9a52ceb2625367
Parents: 14ef05a
Author: Goldstein Lyor <***@cb4.com>
Authored: Tue Oct 2 09:18:09 2018 +0300
Committer: Lyor Goldstein <***@gmail.com>
Committed: Wed Oct 3 20:05:17 2018 +0300

----------------------------------------------------------------------
.../src/main/java/org/apache/sshd/client/kex/DHGClient.java | 2 +-
sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java | 8 ++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/dd542ce3/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java b/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
index be18517..337c956 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/kex/DHGClient.java
@@ -106,7 +106,7 @@ public class DHGClient extends AbstractDHClientKeyExchange {
}
if (cmd != SshConstants.SSH_MSG_KEXDH_REPLY) {
throw new SshException(SshConstants.SSH2_DISCONNECT_KEY_EXCHANGE_FAILED,
- "Protocol error: expected packet SSH_MSG_KEXDH_REPLY, got " + KeyExchange.getSimpleKexOpcodeName(cmd));
+ "Protocol error: expected packet SSH_MSG_KEXDH_REPLY, got " + KeyExchange.getSimpleKexOpcodeName(cmd));
}

byte[] k_s = buffer.getBytes();

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/dd542ce3/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
index 6cc1cb8..97f69be 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
@@ -23,6 +23,7 @@ import java.security.KeyFactory;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.PublicKey;
+import java.util.Objects;

import javax.crypto.interfaces.DHPublicKey;
import javax.crypto.spec.DHParameterSpec;
@@ -38,6 +39,8 @@ import org.apache.sshd.common.util.security.SecurityUtils;
* @author <a href="mailto:***@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DHG extends AbstractDH {
+ public static final String KEX_TYPE = "DH";
+
private BigInteger p;
private BigInteger g;
private BigInteger f; // your public key
@@ -48,7 +51,7 @@ public class DHG extends AbstractDH {
}

public DHG(Factory<? extends Digest> digestFactory, BigInteger pValue, BigInteger gValue) throws Exception {
- myKeyAgree = SecurityUtils.getKeyAgreement("DH");
+ myKeyAgree = SecurityUtils.getKeyAgreement(KEX_TYPE);
factory = digestFactory;
p = pValue; // do not check for null-ity since in some cases it can be
g = gValue; // do not check for null-ity since in some cases it can be
@@ -69,6 +72,7 @@ public class DHG extends AbstractDH {

@Override
protected byte[] calculateK() throws Exception {
+ Objects.requireNonNull(f, "Missing 'f' value");
DHPublicKeySpec keySpec = new DHPublicKeySpec(f, p, g);
KeyFactory myKeyFac = SecurityUtils.getKeyFactory("DH");
PublicKey yourPubKey = myKeyFac.generatePublic(keySpec);
@@ -106,7 +110,7 @@ public class DHG extends AbstractDH {
}

public void setF(BigInteger f) {
- this.f = f;
+ this.f = Objects.requireNonNull(f, "No 'f' value specified");
}

@Override

[l***@apache.org]

[SSHD-846] Added AbstractDH derived classes 'toString' implementation


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/29d77123
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/29d77123
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/29d77123

Branch: refs/heads/master
Commit: 29d77123269047ada7e317ff6f1c9da0592399b7
Parents: dd542ce
Author: Goldstein Lyor <***@cb4.com>
Authored: Tue Oct 2 09:23:40 2018 +0300
Committer: Lyor Goldstein <***@gmail.com>
Committed: Wed Oct 3 20:05:17 2018 +0300

----------------------------------------------------------------------
.../java/org/apache/sshd/common/kex/AbstractDH.java | 16 ++++++++++++++++
.../main/java/org/apache/sshd/common/kex/DHG.java | 11 +++++++++++
.../main/java/org/apache/sshd/common/kex/ECDH.java | 9 +++++++++
3 files changed, 36 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/29d77123/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
index 77b0a85..0bba382 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
@@ -38,6 +38,10 @@ public abstract class AbstractDH {

public abstract void setF(byte[] f);

+ public boolean isPublicDataAvailable() {
+ return e_array != null;
+ }
+
/**
* Lazy-called by {@link #getE()} if the public key data has not
* been generated yet.
@@ -60,6 +64,10 @@ public abstract class AbstractDH {
return e_array;
}

+ public boolean isSharedSecretAvailable() {
+ return k_array != null;
+ }
+
/**
* Lazy-called by {@link #getK()} if the shared secret data has
* not been calculated yet
@@ -102,6 +110,14 @@ public abstract class AbstractDH {

public abstract Digest getHash() throws Exception;

+ @Override
+ public String toString() {
+ return getClass().getSimpleName()
+ + "[publicDataAvailable=" + isPublicDataAvailable()
+ + ", sharedSecretAvailable=" + isSharedSecretAvailable()
+ + "]";
+ }
+
/**
* The shared secret returned by {@link javax.crypto.KeyAgreement#generateSecret()}
* is a byte array, which can (by chance, roughly 1 out of 256 times) begin

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/29d77123/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
index 97f69be..a67929f 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
@@ -65,6 +65,7 @@ public class DHG extends AbstractDH {

KeyPair myKpair = myKpairGen.generateKeyPair();
myKeyAgree.init(myKpair.getPrivate());
+
DHPublicKey pubKey = (DHPublicKey) myKpair.getPublic();
BigInteger e = pubKey.getY();
return e.toByteArray();
@@ -117,4 +118,14 @@ public class DHG extends AbstractDH {
public Digest getHash() throws Exception {
return factory.create();
}
+
+ @Override
+ public String toString() {
+ return super.toString()
+ + "[p=" + p
+ + ", g=" + g
+ + ", f=" + f
+ + ", digest=" + factory
+ + "]";
+ }
}

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/29d77123/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
index 16f9b25..f30924a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
@@ -69,6 +69,7 @@ public class ECDH extends AbstractDH {
Objects.requireNonNull(params, "No ECParameterSpec(s)");
KeyPairGenerator myKpairGen = SecurityUtils.getKeyPairGenerator(KeyUtils.EC_ALGORITHM);
myKpairGen.initialize(params);
+
KeyPair myKpair = myKpairGen.generateKeyPair();
myKeyAgree.init(myKpair.getPrivate());

@@ -108,4 +109,12 @@ public class ECDH extends AbstractDH {

return curve.getDigestForParams();
}
+
+ @Override
+ public String toString() {
+ return super.toString()
+ + "[curve=" + curve
+ + ", f=" + f
+ + "]";
+ }
}

[l***@apache.org]

[SSHD-846] Use an ephemeral KeyPairGenerator when creating DH KEX key-pair


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/36853fd0
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/36853fd0
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/36853fd0

Branch: refs/heads/master
Commit: 36853fd00d771db1bce180f10a82941ec9f61f86
Parents: 28c52e9
Author: Goldstein Lyor <***@cb4.com>
Authored: Tue Oct 2 08:49:46 2018 +0300
Committer: Lyor Goldstein <***@gmail.com>
Committed: Wed Oct 3 20:05:17 2018 +0300

----------------------------------------------------------------------
.../org/apache/sshd/common/kex/AbstractDH.java | 21 ++++++++----
.../java/org/apache/sshd/common/kex/DHG.java | 34 ++++++++------------
.../java/org/apache/sshd/common/kex/ECDH.java | 30 +++++++----------
3 files changed, 40 insertions(+), 45 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/36853fd0/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
index 251de67..d5beccb 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
@@ -18,7 +18,7 @@
*/
package org.apache.sshd.common.kex;

-import java.math.BigInteger;
+import javax.crypto.KeyAgreement;

import org.apache.sshd.common.digest.Digest;
import org.apache.sshd.common.util.NumberUtils;
@@ -28,8 +28,10 @@ import org.apache.sshd.common.util.NumberUtils;
*/
public abstract class AbstractDH {

- protected BigInteger k; // shared secret key
- private byte[] k_array;
+ protected KeyAgreement myKeyAgree;
+
+ private byte[] k_array; // shared secret key
+ private byte[] e_array; // public key used in the exchange

protected AbstractDH() {
super();
@@ -37,14 +39,21 @@ public abstract class AbstractDH {

public abstract void setF(byte[] e);

- public abstract byte[] getE() throws Exception;
+ protected abstract byte[] calculateE() throws Exception;
+
+ public byte[] getE() throws Exception {
+ if (e_array == null) {
+ e_array = calculateE();
+ }
+
+ return e_array;
+ }

protected abstract byte[] calculateK() throws Exception;

public byte[] getK() throws Exception {
- if (k == null) {
+ if (k_array == null) {
k_array = calculateK();
- k = new BigInteger(k_array);
}
return k_array;
}

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/36853fd0/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
index 44fa725..6cc1cb8 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/DHG.java
@@ -24,7 +24,7 @@ import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.PublicKey;

-import javax.crypto.KeyAgreement;
+import javax.crypto.interfaces.DHPublicKey;
import javax.crypto.spec.DHParameterSpec;
import javax.crypto.spec.DHPublicKeySpec;

@@ -38,14 +38,9 @@ import org.apache.sshd.common.util.security.SecurityUtils;
* @author <a href="mailto:***@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class DHG extends AbstractDH {
-
private BigInteger p;
private BigInteger g;
- private BigInteger e; // my public key
- private byte[] e_array;
private BigInteger f; // your public key
- private KeyPairGenerator myKpairGen;
- private KeyAgreement myKeyAgree;
private Factory<? extends Digest> factory;

public DHG(Factory<? extends Digest> digestFactory) throws Exception {
@@ -53,30 +48,29 @@ public class DHG extends AbstractDH {
}

public DHG(Factory<? extends Digest> digestFactory, BigInteger pValue, BigInteger gValue) throws Exception {
- myKpairGen = SecurityUtils.getKeyPairGenerator("DH");
myKeyAgree = SecurityUtils.getKeyAgreement("DH");
factory = digestFactory;
- p = pValue;
- g = gValue;
+ p = pValue; // do not check for null-ity since in some cases it can be
+ g = gValue; // do not check for null-ity since in some cases it can be
}

@Override
- public byte[] getE() throws Exception {
- if (e == null) {
- DHParameterSpec dhSkipParamSpec = new DHParameterSpec(p, g);
- myKpairGen.initialize(dhSkipParamSpec);
- KeyPair myKpair = myKpairGen.generateKeyPair();
- myKeyAgree.init(myKpair.getPrivate());
- e = ((javax.crypto.interfaces.DHPublicKey) (myKpair.getPublic())).getY();
- e_array = e.toByteArray();
- }
- return e_array;
+ protected byte[] calculateE() throws Exception {
+ DHParameterSpec dhSkipParamSpec = new DHParameterSpec(p, g);
+ KeyPairGenerator myKpairGen = SecurityUtils.getKeyPairGenerator("DH");
+ myKpairGen.initialize(dhSkipParamSpec);
+
+ KeyPair myKpair = myKpairGen.generateKeyPair();
+ myKeyAgree.init(myKpair.getPrivate());
+ DHPublicKey pubKey = (DHPublicKey) myKpair.getPublic();
+ BigInteger e = pubKey.getY();
+ return e.toByteArray();
}

@Override
protected byte[] calculateK() throws Exception {
- KeyFactory myKeyFac = SecurityUtils.getKeyFactory("DH");
DHPublicKeySpec keySpec = new DHPublicKeySpec(f, p, g);
+ KeyFactory myKeyFac = SecurityUtils.getKeyFactory("DH");
PublicKey yourPubKey = myKeyFac.generatePublic(keySpec);
myKeyAgree.doPhase(yourPubKey, true);
return stripLeadingZeroes(myKeyAgree.generateSecret());

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/36853fd0/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
index 0fc178e..f5b5070 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
@@ -28,8 +28,6 @@ import java.security.spec.ECPoint;
import java.security.spec.ECPublicKeySpec;
import java.util.Objects;

-import javax.crypto.KeyAgreement;
-
import org.apache.sshd.common.cipher.ECCurves;
import org.apache.sshd.common.config.keys.KeyUtils;
import org.apache.sshd.common.digest.Digest;
@@ -42,13 +40,8 @@ import org.apache.sshd.common.util.security.SecurityUtils;
* @author <a href="mailto:***@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class ECDH extends AbstractDH {
-
private ECParameterSpec params;
- private ECPoint e;
- private byte[] e_array;
private ECPoint f;
- private KeyPairGenerator myKpairGen;
- private KeyAgreement myKeyAgree;

public ECDH() throws Exception {
this((ECParameterSpec) null);
@@ -63,22 +56,21 @@ public class ECDH extends AbstractDH {
}

public ECDH(ECParameterSpec paramSpec) throws Exception {
- myKpairGen = SecurityUtils.getKeyPairGenerator(KeyUtils.EC_ALGORITHM);
myKeyAgree = SecurityUtils.getKeyAgreement("ECDH");
- params = paramSpec;
+ params = paramSpec; // do not check for null-ity since in some cases it can be
}

@Override
- public byte[] getE() throws Exception {
- if (e == null) {
- Objects.requireNonNull(params, "No ECParameterSpec(s)");
- myKpairGen.initialize(params);
- KeyPair myKpair = myKpairGen.generateKeyPair();
- myKeyAgree.init(myKpair.getPrivate());
- e = ((ECPublicKey) myKpair.getPublic()).getW();
- e_array = ECCurves.encodeECPoint(e, params);
- }
- return e_array;
+ protected byte[] calculateE() throws Exception {
+ Objects.requireNonNull(params, "No ECParameterSpec(s)");
+ KeyPairGenerator myKpairGen = SecurityUtils.getKeyPairGenerator(KeyUtils.EC_ALGORITHM);
+ myKpairGen.initialize(params);
+ KeyPair myKpair = myKpairGen.generateKeyPair();
+ myKeyAgree.init(myKpair.getPrivate());
+
+ ECPublicKey pubKey = (ECPublicKey) myKpair.getPublic();
+ ECPoint e = pubKey.getW();
+ return ECCurves.encodeECPoint(e, params);
}

@Override

[l***@apache.org]

[SSHD-846] Re-use original curve (if provided) when returning ECDH used digest


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/14ef05ad
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/14ef05ad
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/14ef05ad

Branch: refs/heads/master
Commit: 14ef05adfd4f8c3262aa7b8560025048af68351a
Parents: 9335f22
Author: Goldstein Lyor <***@cb4.com>
Authored: Tue Oct 2 09:17:28 2018 +0300
Committer: Lyor Goldstein <***@gmail.com>
Committed: Wed Oct 3 20:05:17 2018 +0300

----------------------------------------------------------------------
.../main/java/org/apache/sshd/common/kex/ECDH.java | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/14ef05ad/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
index f5b5070..16f9b25 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/ECDH.java
@@ -40,6 +40,9 @@ import org.apache.sshd.common.util.security.SecurityUtils;
* @author <a href="mailto:***@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class ECDH extends AbstractDH {
+ public static final String KEX_TYPE = "ECDH";
+
+ private ECCurves curve;
private ECParameterSpec params;
private ECPoint f;

@@ -53,10 +56,11 @@ public class ECDH extends AbstractDH {

public ECDH(ECCurves curve) throws Exception {
this(Objects.requireNonNull(curve, "No known curve instance provided").getParameters());
+ this.curve = curve;
}

public ECDH(ECParameterSpec paramSpec) throws Exception {
- myKeyAgree = SecurityUtils.getKeyAgreement("ECDH");
+ myKeyAgree = SecurityUtils.getKeyAgreement(KEX_TYPE);
params = paramSpec; // do not check for null-ity since in some cases it can be
}

@@ -76,8 +80,9 @@ public class ECDH extends AbstractDH {
@Override
protected byte[] calculateK() throws Exception {
Objects.requireNonNull(params, "No ECParameterSpec(s)");
- KeyFactory myKeyFac = SecurityUtils.getKeyFactory(KeyUtils.EC_ALGORITHM);
+ Objects.requireNonNull(f, "Missing 'f' value");
ECPublicKeySpec keySpec = new ECPublicKeySpec(f, params);
+ KeyFactory myKeyFac = SecurityUtils.getKeyFactory(KeyUtils.EC_ALGORITHM);
PublicKey yourPubKey = myKeyFac.generatePublic(keySpec);
myKeyAgree.doPhase(yourPubKey, true);
return stripLeadingZeroes(myKeyAgree.generateSecret());
@@ -90,13 +95,17 @@ public class ECDH extends AbstractDH {
@Override
public void setF(byte[] f) {
Objects.requireNonNull(params, "No ECParameterSpec(s)");
+ Objects.requireNonNull(f, "No 'f' value specified");
this.f = ECCurves.octetStringToEcPoint(f);
}

@Override
public Digest getHash() throws Exception {
- Objects.requireNonNull(params, "No ECParameterSpec(s)");
- ECCurves curve = Objects.requireNonNull(ECCurves.fromCurveParameters(params), "Unknown curve parameters");
+ if (curve == null) {
+ Objects.requireNonNull(params, "No ECParameterSpec(s)");
+ curve = Objects.requireNonNull(ECCurves.fromCurveParameters(params), "Unknown curve parameters");
+ }
+
return curve.getDigestForParams();
}
}

[l***@apache.org]

[SSHD-846] Garbage-collect the KeyAgreement instance used in the DH KEX once public & private part have been calculated


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/9335f22c
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/9335f22c
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/9335f22c

Branch: refs/heads/master
Commit: 9335f22c6fd1003a610fcf4f469697a398d80225
Parents: 36853fd
Author: Goldstein Lyor <***@cb4.com>
Authored: Tue Oct 2 09:03:43 2018 +0300
Committer: Lyor Goldstein <***@gmail.com>
Committed: Wed Oct 3 20:05:17 2018 +0300

----------------------------------------------------------------------
.../org/apache/sshd/common/kex/AbstractDH.java | 46 +++++++++++++++++++-
1 file changed, 44 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/9335f22c/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java b/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
index d5beccb..77b0a85 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/kex/AbstractDH.java
@@ -27,7 +27,6 @@ import org.apache.sshd.common.util.NumberUtils;
* Base class for the Diffie-Hellman key agreement.
*/
public abstract class AbstractDH {
-
protected KeyAgreement myKeyAgree;

private byte[] k_array; // shared secret key
@@ -37,27 +36,70 @@ public abstract class AbstractDH {
super();
}

- public abstract void setF(byte[] e);
+ public abstract void setF(byte[] f);

+ /**
+ * Lazy-called by {@link #getE()} if the public key data has not
+ * been generated yet.
+ *
+ * @return The calculated public key data
+ * @throws Exception If failed to generate the relevant data
+ */
protected abstract byte[] calculateE() throws Exception;

+ /**
+ * @return The local public key data
+ * @throws Exception If failed to calculate it
+ */
public byte[] getE() throws Exception {
if (e_array == null) {
e_array = calculateE();
+ checkKeyAgreementNecessity();
}

return e_array;
}

+ /**
+ * Lazy-called by {@link #getK()} if the shared secret data has
+ * not been calculated yet
+ *
+ * @return The shared secret data
+ * @throws Exception If failed to calculate it
+ */
protected abstract byte[] calculateK() throws Exception;

+ /**
+ * @return The shared secret key
+ * @throws Exception If failed to calculate it
+ */
public byte[] getK() throws Exception {
if (k_array == null) {
k_array = calculateK();
+ checkKeyAgreementNecessity();
}
return k_array;
}

+ /**
+ * Called after either public or private parts have been calculated
+ * in order to check if the key-agreement mediator is still required.
+ * By default, if both public and private parts have been calculated
+ * then key-agreement mediator is null-ified to enable GC for it.
+ *
+ * @see #getE()
+ * @see #getK()
+ */
+ protected void checkKeyAgreementNecessity() {
+ if ((e_array == null) || (k_array == null)) {
+ return;
+ }
+
+ if (myKeyAgree != null) {
+ myKeyAgree = null; // allow GC for key agreement object
+ }
+ }
+
public abstract Digest getHash() throws Exception;

/**