From 8b61d968088fd5e279569c4eb0fe47f13878cd3b Mon Sep 17 00:00:00 2001 From: Shikhar Bhushan Date: Sat, 18 Jun 2011 20:16:56 +0100 Subject: [PATCH] changed some things around, lesser conversions / copying. still not found the bug. grr. --- .../java/net/schmizz/sshj/common/Buffer.java | 36 ++------------ .../net/schmizz/sshj/transport/Encoder.java | 4 +- .../schmizz/sshj/transport/KeyExchanger.java | 18 +++---- .../sshj/transport/kex/AbstractDHG.java | 43 ++++++++--------- .../net/schmizz/sshj/transport/kex/DH.java | 48 ++++++++----------- .../net/schmizz/sshj/transport/kex/DHG1.java | 8 ++-- .../net/schmizz/sshj/transport/kex/DHG14.java | 7 +-- .../sshj/transport/kex/KeyExchange.java | 5 +- 8 files changed, 66 insertions(+), 103 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/common/Buffer.java b/src/main/java/net/schmizz/sshj/common/Buffer.java index 7e3ccb0a..4386d29a 100644 --- a/src/main/java/net/schmizz/sshj/common/Buffer.java +++ b/src/main/java/net/schmizz/sshj/common/Buffer.java @@ -341,41 +341,13 @@ public class Buffer> { */ public BigInteger readMPInt() throws BufferException { - return new BigInteger(readMPIntAsBytes()); + return new BigInteger(readBytes()); } - /** - * Writes an SSH multiple-precision integer from a {@code BigInteger} - * - * @param bi {@code BigInteger} to write - * - * @return this - */ public T putMPInt(BigInteger bi) { - return putMPInt(bi.toByteArray()); - } - - /** - * Writes an SSH multiple-precision integer from a Java byte-array - * - * @param foo byte-array - * - * @return this - */ - public T putMPInt(byte[] foo) { - int i = foo.length; - if ((foo[0] & 0x80) != 0) { - i++; - putUInt32(i); - putByte((byte) 0); - } else - putUInt32(i); - return putRawBytes(foo); - } - - public byte[] readMPIntAsBytes() - throws BufferException { - return readBytes(); + final byte[] asBytes = bi.toByteArray(); + putUInt32(asBytes.length); + return putRawBytes(asBytes); } public long readUInt64() diff --git a/src/main/java/net/schmizz/sshj/transport/Encoder.java b/src/main/java/net/schmizz/sshj/transport/Encoder.java index e90a0788..b84e1df3 100644 --- a/src/main/java/net/schmizz/sshj/transport/Encoder.java +++ b/src/main/java/net/schmizz/sshj/transport/Encoder.java @@ -63,7 +63,7 @@ final class Encoder 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"); + + "5 bytes are available in front of the buffer"); SSHPacket nb = new SSHPacket(buffer.available() + 5); nb.rpos(5); nb.wpos(5); @@ -96,8 +96,6 @@ final class Encoder long encode(SSHPacket buffer) { encodeLock.lock(); try { - buffer = checkHeaderSpace(buffer); - if (log.isTraceEnabled()) log.trace("Encoding packet #{}: {}", seq, buffer.printHex()); diff --git a/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java b/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java index 95628611..3cfe86b8 100644 --- a/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java +++ b/src/main/java/net/schmizz/sshj/transport/KeyExchanger.java @@ -41,7 +41,6 @@ import net.schmizz.sshj.common.Buffer; import net.schmizz.sshj.common.DisconnectReason; import net.schmizz.sshj.common.ErrorNotifiable; import net.schmizz.sshj.common.Factory; -import net.schmizz.sshj.common.IOUtils; import net.schmizz.sshj.common.KeyType; import net.schmizz.sshj.common.Message; import net.schmizz.sshj.common.SSHException; @@ -57,6 +56,7 @@ import net.schmizz.sshj.transport.verification.HostKeyVerifier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.math.BigInteger; import java.security.GeneralSecurityException; import java.security.PublicKey; import java.util.Arrays; @@ -234,6 +234,7 @@ final class KeyExchanger private void gotKexInit(SSHPacket buf) throws TransportException { + buf.rpos(buf.rpos() - 1); final Proposal serverProposal = new Proposal(buf); negotiatedAlgs = clientProposal.negotiate(serverProposal); log.debug("Negotiated algorithms: {}", negotiatedAlgs); @@ -241,10 +242,8 @@ final class KeyExchanger negotiatedAlgs.getKeyExchangeAlgorithm()); try { kex.init(transport, - transport.getServerID().getBytes(IOUtils.UTF8), - transport.getClientID().getBytes(IOUtils.UTF8), - buf.getCompactData(), - clientProposal.getPacket().getCompactData()); + transport.getServerID(), transport.getClientID(), + serverProposal.getPacket().getCompactData(), clientProposal.getPacket().getCompactData()); } catch (GeneralSecurityException e) { throw new TransportException(DisconnectReason.KEY_EXCHANGE_FAILED, e); } @@ -262,7 +261,7 @@ final class KeyExchanger * * @return the resized key */ - private static byte[] resizedKey(byte[] E, int blockSize, Digest hash, byte[] K, byte[] H) { + private static byte[] resizedKey(byte[] E, int blockSize, Digest hash, BigInteger K, byte[] H) { while (blockSize > E.length) { Buffer.PlainBuffer buffer = new Buffer.PlainBuffer().putMPInt(K).putRawBytes(H).putRawBytes(E); hash.update(buffer.array(), 0, buffer.available()); @@ -280,13 +279,15 @@ final class KeyExchanger private void gotNewKeys() { final Digest hash = kex.getHash(); + final byte[] H = kex.getH(); + if (sessionID == null) // session id is 'H' from the first key exchange and does not change thereafter - sessionID = Arrays.copyOf(kex.getH(), kex.getH().length); + sessionID = H; final Buffer.PlainBuffer hashInput = new Buffer.PlainBuffer() .putMPInt(kex.getK()) - .putRawBytes(kex.getH()) + .putRawBytes(H) .putByte((byte) 0) // .putRawBytes(sessionID); final int pos = hashInput.available() - sessionID.length - 1; // Position of @@ -360,7 +361,6 @@ final class KeyExchanger * having sent the packet ourselves (would cause gotKexInit() to fail) */ kexInitSent.await(transport.getTimeout(), TimeUnit.SECONDS); - buf.rpos(buf.rpos() - 1); gotKexInit(buf); expected = Expected.FOLLOWUP; break; diff --git a/src/main/java/net/schmizz/sshj/transport/kex/AbstractDHG.java b/src/main/java/net/schmizz/sshj/transport/kex/AbstractDHG.java index 68f20dc4..9d61f341 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/AbstractDHG.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/AbstractDHG.java @@ -65,16 +65,14 @@ public abstract class AbstractDHG private Transport trans; - private final Digest sha = new SHA1(); + private final Digest sha1 = new SHA1(); private final DH dh = new DH(); - private byte[] V_S; - private byte[] V_C; + private String V_S; + private String V_C; private byte[] I_S; private byte[] I_C; - private byte[] e; - private byte[] K; private byte[] H; private PublicKey hostKey; @@ -84,13 +82,13 @@ public abstract class AbstractDHG } @Override - public byte[] getK() { - return Arrays.copyOf(K, K.length); + public BigInteger getK() { + return dh.getK(); } @Override public Digest getHash() { - return sha; + return sha1; } @Override @@ -99,19 +97,18 @@ public abstract class AbstractDHG } @Override - public void init(Transport trans, byte[] V_S, byte[] V_C, byte[] I_S, byte[] I_C) + public void init(Transport trans, String V_S, String V_C, byte[] I_S, byte[] I_C) throws GeneralSecurityException, TransportException { this.trans = trans; - this.V_S = Arrays.copyOf(V_S, V_S.length); - this.V_C = Arrays.copyOf(V_C, V_C.length); + this.V_S = V_S; + this.V_C = V_C; this.I_S = Arrays.copyOf(I_S, I_S.length); this.I_C = Arrays.copyOf(I_C, I_C.length); - sha.init(); + sha1.init(); initDH(dh); - e = dh.getE(); log.info("Sending SSH_MSG_KEXDH_INIT"); - trans.write(new SSHPacket(Message.KEXDH_INIT).putMPInt(e)); + trans.write(new SSHPacket(Message.KEXDH_INIT).putMPInt(dh.getE())); } @Override @@ -122,19 +119,18 @@ public abstract class AbstractDHG log.info("Received SSH_MSG_KEXDH_REPLY"); final byte[] K_S; - final byte[] f; + final BigInteger f; final byte[] sig; // signature sent by server try { K_S = packet.readBytes(); - f = packet.readMPIntAsBytes(); + f = packet.readMPInt(); sig = packet.readBytes(); hostKey = new Buffer.PlainBuffer(K_S).readPublicKey(); } catch (Buffer.BufferException be) { throw new TransportException(be); } - dh.setF(new BigInteger(f)); - K = dh.getK(); + dh.computeK(f); final Buffer.PlainBuffer buf = new Buffer.PlainBuffer() .putString(V_C) @@ -142,11 +138,11 @@ public abstract class AbstractDHG .putString(I_C) .putString(I_S) .putString(K_S) - .putMPInt(e) + .putMPInt(dh.getE()) .putMPInt(f) - .putMPInt(K); - sha.update(buf.array(), 0, buf.available()); - H = sha.digest(); + .putMPInt(dh.getK()); + sha1.update(buf.array(), buf.rpos(), buf.available()); + H = sha1.digest(); Signature signature = Factory.Named.Util.create(trans.getConfig().getSignatureFactories(), KeyType.fromKey(hostKey).toString()); @@ -158,6 +154,7 @@ public abstract class AbstractDHG return true; } - protected abstract void initDH(DH dh); + protected abstract void initDH(DH dh) + throws GeneralSecurityException; } 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 4c850096..abdf7011 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/DH.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/DH.java @@ -54,7 +54,6 @@ public class DH { private BigInteger p; private BigInteger g; private BigInteger e; // my public key - private BigInteger f; // your public key private BigInteger K; // shared secret key private final KeyPairGenerator generator; private final KeyAgreement agreement; @@ -68,39 +67,32 @@ public class DH { } } - public void setF(BigInteger f) { - this.f = f; - } - - public void setG(BigInteger g) { - this.g = g; - } - - public void setP(BigInteger p) { + public void init(BigInteger p, BigInteger g) + throws GeneralSecurityException { this.p = p; + this.g = g; + generator.initialize(new DHParameterSpec(p, g)); + final KeyPair kp = generator.generateKeyPair(); + agreement.init(kp.getPrivate()); + e = ((javax.crypto.interfaces.DHPublicKey) kp.getPublic()).getY(); + } - public byte[] getE() + public void computeK(BigInteger f) throws GeneralSecurityException { - if (e == null) { - generator.initialize(new DHParameterSpec(p, g)); - final KeyPair kp = generator.generateKeyPair(); - agreement.init(kp.getPrivate()); - e = ((javax.crypto.interfaces.DHPublicKey) kp.getPublic()).getY(); - } - return e.toByteArray(); + final KeyFactory keyFactory = SecurityUtils.getKeyFactory("DH"); + final PublicKey yourPubKey = keyFactory.generatePublic(new DHPublicKeySpec(f, p, g)); + agreement.doPhase(yourPubKey, true); + K = new BigInteger(agreement.generateSecret()); + } - public byte[] getK() - throws GeneralSecurityException { - if (K == null) { - final KeyFactory keyFactory = SecurityUtils.getKeyFactory("DH"); - final DHPublicKeySpec keySpec = new DHPublicKeySpec(f, p, g); - final PublicKey yourPubKey = keyFactory.generatePublic(keySpec); - agreement.doPhase(yourPubKey, true); - K = new BigInteger(agreement.generateSecret()); - } - return K.toByteArray(); + public BigInteger getE() { + return e; + } + + public BigInteger getK() { + return K; } } 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 7ccace79..f019b27b 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/DHG1.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/DHG1.java @@ -35,6 +35,8 @@ */ package net.schmizz.sshj.transport.kex; +import java.security.GeneralSecurityException; + /** * Diffie-Hellman key exchange with SHA-1 and Oakley Group 2 [RFC2409] (1024-bit MODP Group). * @@ -60,9 +62,9 @@ public class DHG1 } @Override - protected void initDH(DH dh) { - dh.setG(DHGroupData.G); - dh.setP(DHGroupData.P1); + protected void initDH(DH dh) + throws GeneralSecurityException { + dh.init(DHGroupData.P1, DHGroupData.G); } } 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 6324fc1a..5ddab369 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/DHG14.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/DHG14.java @@ -35,6 +35,8 @@ */ package net.schmizz.sshj.transport.kex; +import java.security.GeneralSecurityException; + /** * Diffie-Hellman key exchange with SHA-1 and Oakley Group 14 [RFC3526] (2048-bit MODP Group). *

@@ -61,9 +63,8 @@ public class DHG14 } @Override - protected void initDH(DH dh) { - dh.setG(DHGroupData.G); - dh.setP(DHGroupData.P14); + protected void initDH(DH dh) throws GeneralSecurityException { + dh.init(DHGroupData.P14, DHGroupData.G); } } diff --git a/src/main/java/net/schmizz/sshj/transport/kex/KeyExchange.java b/src/main/java/net/schmizz/sshj/transport/kex/KeyExchange.java index a962f598..9b52361e 100644 --- a/src/main/java/net/schmizz/sshj/transport/kex/KeyExchange.java +++ b/src/main/java/net/schmizz/sshj/transport/kex/KeyExchange.java @@ -41,6 +41,7 @@ import net.schmizz.sshj.transport.Transport; import net.schmizz.sshj.transport.TransportException; import net.schmizz.sshj.transport.digest.Digest; +import java.math.BigInteger; import java.security.GeneralSecurityException; import java.security.PublicKey; @@ -59,14 +60,14 @@ public interface KeyExchange { * @throws GeneralSecurityException * @throws TransportException if there is an error sending a packet */ - void init(Transport trans, byte[] V_S, byte[] V_C, byte[] I_S, byte[] I_C) + void init(Transport trans, String V_S, String V_C, byte[] I_S, byte[] I_C) throws GeneralSecurityException, TransportException; /** @return the computed H parameter */ byte[] getH(); /** @return the computed K parameter */ - byte[] getK(); + BigInteger getK(); /** * The message digest used by this key exchange algorithm.