refactor using ssh_session structure

* move computeKey to C code
* Use `mpint_e`, `secret` instead of bn_e, bn_K, drop bn_f
* compute DH image and secret in DH related code
* drop {get,put}{BN2,SSH2BN}

9 files changed, 148 insertions, 158 deletions

diff --git a/src/protocol/internal/fqterm_ssh2_kex.cpp b/src/protocol/internal/fqterm_ssh2_kex.cpp
index f51f459..8566e92 100644
--- a/src/protocol/internal/fqterm_ssh2_kex.cpp
+++ b/src/protocol/internal/fqterm_ssh2_kex.cpp
@@ -44,14 +44,7 @@ FQTermSSH2Kex::FQTermSSH2Kex(const char *V_C, const char *V_S)
   I_S_ = NULL;
   K_S_ = NULL;
 
-  bn_x_ = BN_new();
-  bn_e_ = BN_new();
-  ctx_ = BN_CTX_new();
-
-  bn_K_ = BN_new();
-  bn_f_ = BN_new();
-
-  session_id_ = NULL;
+  sess.session_id = NULL;
 }
 
 FQTermSSH2Kex::~FQTermSSH2Kex() {
@@ -60,14 +53,8 @@ FQTermSSH2Kex::~FQTermSSH2Kex() {
   if (K_S_)
     delete [] K_S_;
 
-  BN_clear_free(bn_x_);
-  BN_clear_free(bn_e_);
-  BN_CTX_free(ctx_);
-
-  BN_clear_free(bn_K_);
-  BN_clear_free(bn_f_);
-
-  delete[] session_id_;
+  if (sess.session_id != NULL)
+    delete[] sess.session_id;
 }
 
 void FQTermSSH2Kex::initKex(FQTermSSHPacketReceiver *packetReceiver, 
@@ -156,7 +143,7 @@ bool FQTermSSH2Kex::negotiateAlgorithms() {
 	  emit kexError(tr("No matching KEX algorithms!"));
 	  return false;
   }
-  this->dh = new_dh();
+  sess.dh = new_dh();
 
   // TODO: host key algorithms
   size_t hk_algo_len = packet_receiver_->getInt();
@@ -258,12 +245,23 @@ bool FQTermSSH2Kex::negotiateAlgorithms() {
   return true;
 }
 
-void FQTermSSH2Kex::exchangeKey() {
-  BN_pseudo_rand_range(bn_x_, dh->p);
-  BN_mod_exp(bn_e_, dh->g, bn_x_, dh->p, ctx_);
+/* RFC 4253 section 8:
+ * client generate x and compute e=g^x
+ * server generate y and compute f=g^y
+ *
+ * client sends:
+ * byte      SSH_MSG_KEXDH_INIT
+ * mpint     e
+ * server sends:
+ * byte      SSH_MSG_KEXDH_REPLY
+ * string    server public host key and certificates (K_S)
+ * mpint     f
+ * string    signature of H
+ */
 
+void FQTermSSH2Kex::exchangeKey() {
   packet_sender_->startPacket(SSH2_MSG_KEXDH_INIT);
-  packet_sender_->putBN2(bn_e_);
+  packet_sender_->putRawData((const char*)sess.dh->mpint_e, sess.dh->e_len);
   packet_sender_->write();
 }
 
@@ -275,19 +273,22 @@ bool FQTermSSH2Kex::verifyKey() {
     return false;
   }
 
-  // Extract data
 
   if (K_S_)
     delete [] K_S_;
   K_S_ = (char*)packet_receiver_->getString(&K_S_len_);
 
-  packet_receiver_->getBN2(bn_f_);
+  int mpint_f_len;
+  unsigned char *mpint_f = (unsigned char *)packet_receiver_->getString(&mpint_f_len);
+  if (ssh_dh_compute_secret(sess.dh, mpint_f, mpint_f_len) < 0) {
+    emit kexError(tr("Error when computing shared secret"));
+    delete mpint_f;
+    return false;
+  }
 
   int s_len = -1;
   unsigned char *s = (unsigned char *)packet_receiver_->getString(&s_len);
 
-  BN_mod_exp(bn_K_, bn_f_, bn_x_, dh->p, ctx_);
-
   FQTermSSHBuffer *buffer = packet_sender_->output_buffer_;
 
   buffer->clear();
@@ -296,16 +297,16 @@ bool FQTermSSH2Kex::verifyKey() {
   buffer->putString(I_C_, I_C_len_);
   buffer->putString(I_S_, I_S_len_);
   buffer->putString(K_S_, K_S_len_);
-  buffer->putSSH2BN(bn_e_);
-  buffer->putSSH2BN(bn_f_);
-  buffer->putSSH2BN(bn_K_);
+  buffer->putRawData((const char*)sess.dh->mpint_e, sess.dh->e_len);
+  buffer->putString((const char*)mpint_f, mpint_f_len);
+  buffer->putRawData((const char*)sess.dh->secret, sess.dh->secret_len);
 
-  ssh_dh_hash(dh, buffer->data(), H_, buffer->len());
+  ssh_dh_hash(sess.dh, buffer->data(), sess.H, buffer->len());
 
   // Start verify
   // ssh-rsa specifies SHA-1 hashing
   unsigned char s_H[SHA_DIGEST_LENGTH];
-  SHA1(H_, dh->digest.hashlen, s_H);
+  SHA1(sess.H, sess.dh->digest.hashlen, s_H);
 
   // Ignore the first 15 bytes of the signature of H sent from server:
   // algorithm_name_length[4], algorithm_name[7]("ssh-rsa") and signature_length[4].
@@ -317,6 +318,7 @@ bool FQTermSSH2Kex::verifyKey() {
 
   RSA_free(rsactx);
 
+  delete [] mpint_f;
   delete [] s;
 
   return res == 1;
@@ -365,14 +367,16 @@ void FQTermSSH2Kex::sendNewKeys(){
 }
 
 bool FQTermSSH2Kex::changeKeyAlg() {
+	unsigned char IV_c2s[128], IV_s2c[128], key_c2s[128], key_s2c[128], mac_c2s[128], mac_s2c[128];
+
   if (packet_receiver_->packetType() != SSH2_MSG_NEWKEYS) {
     emit kexError(tr("Expect a SSH_MSG_NEWKEYS packet"));
     return false;
   }
 
-  if (session_id_ == NULL) {
-	  session_id_ = new unsigned char[dh->digest.hashlen];
-	  memcpy(session_id_, H_, dh->digest.hashlen);
+  if (sess.session_id == NULL) {
+	  sess.session_id = new unsigned char[sess.dh->digest.hashlen];
+	  memcpy(sess.session_id, sess.H, sess.dh->digest.hashlen);
   }
 
   // From RFC 4253 section 7.2:
@@ -391,75 +395,35 @@ bool FQTermSSH2Kex::changeKeyAlg() {
   // Integrity key server to client: HASH(K || H || "F" || session_id)
 
   int IV_c2s_len = packet_sender_->getIVSize();
-  unsigned char *IV_c2s = computeKey(IV_c2s_len, 'A');
+  computeKey(&sess, IV_c2s_len, 'A', IV_c2s);
 
   int IV_s2c_len = packet_receiver_->getIVSize();
-  unsigned char *IV_s2c = computeKey(IV_s2c_len, 'B');
+  computeKey(&sess, IV_s2c_len, 'B', IV_s2c);
 
   int key_c2s_len = packet_sender_->getKeySize();
-  unsigned char *key_c2s = computeKey(key_c2s_len, 'C');
+  computeKey(&sess, key_c2s_len, 'C', key_c2s);
 
   int key_s2c_len = packet_receiver_->getKeySize();
-  unsigned char *key_s2c = computeKey(key_s2c_len, 'D');
+  computeKey(&sess, key_s2c_len, 'D', key_s2c);
 
   int mac_key_c2s_len = packet_sender_->getMacKeySize();
-  unsigned char *mac_key_c2s = computeKey(mac_key_c2s_len, 'E');
+  computeKey(&sess, mac_key_c2s_len, 'E', mac_c2s);
 
   int mac_key_s2c_len = packet_receiver_->getMacKeySize();
-  unsigned char *mac_key_s2c = computeKey(mac_key_s2c_len, 'F');
-
+  computeKey(&sess, mac_key_s2c_len, 'F', mac_s2c);
 
   packet_sender_->startEncryption(key_c2s, IV_c2s);
-  packet_sender_->startMac(mac_key_c2s);
+  packet_sender_->startMac(mac_c2s);
 
   packet_receiver_->startEncryption(key_s2c, IV_s2c);
-  packet_receiver_->startMac(mac_key_s2c);
+  packet_receiver_->startMac(mac_s2c);
 
   /* now key exchange ends */
-  ssh_dh_free(dh);
-
-  delete[] IV_c2s;
-  delete[] IV_s2c;
-  delete[] key_c2s;
-  delete[] key_s2c;
-  delete[] mac_key_c2s;
-  delete[] mac_key_s2c;
+  ssh_dh_free(sess.dh);
 
   return true;
 }
 
-unsigned char *FQTermSSH2Kex::computeKey(int expected_len, char flag)
-{
-	unsigned char *key = new unsigned char[expected_len + SHA_DIGEST_LENGTH];
-
-	int len = 0;
-
-	EVP_MD_CTX *mdctx = dh->digest.mdctx;
-	const EVP_MD *md = dh->digest.md;
-	int hashlen = dh->digest.hashlen;
-
-	FQTermSSHBuffer K(BN_num_bytes(bn_K_) + 5);
-	K.putSSH2BN(bn_K_);
-
-	while (len < expected_len) {
-		EVP_DigestInit_ex(mdctx, md, NULL);
-		EVP_DigestUpdate(mdctx, K.data(), K.len());
-		EVP_DigestUpdate(mdctx, H_, hashlen);
-
-		if (len == 0) {
-			EVP_DigestUpdate(mdctx, &flag, 1);
-			EVP_DigestUpdate(mdctx, session_id_, hashlen);
-		} else {
-			EVP_DigestUpdate(mdctx, key, len);
-		}
-
-		EVP_DigestFinal_ex(mdctx, key+len, NULL);
-		len += SHA_DIGEST_LENGTH;
-	}
-
-	return key;
-}
-
 }  // namespace FQTerm
 
 #include "fqterm_ssh2_kex.moc"
diff --git a/src/protocol/internal/fqterm_ssh2_kex.h b/src/protocol/internal/fqterm_ssh2_kex.h
index 80da76d..978cb17 100644
--- a/src/protocol/internal/fqterm_ssh2_kex.h
+++ b/src/protocol/internal/fqterm_ssh2_kex.h
@@ -24,7 +24,7 @@
 #include <openssl/sha.h>
 
 #include "fqterm_ssh_kex.h"
-#include "ssh_diffie-hellman.h"
+#include "ssh_session.h"
 
 namespace FQTerm {
 
@@ -58,17 +58,7 @@ private:
   int K_S_len_;
   char *K_S_;
 
-  SSH_DH *dh;
-  BIGNUM *bn_x_;
-  BIGNUM *bn_e_;
-  BN_CTX *ctx_;
-
-  BIGNUM *bn_K_;
-  BIGNUM *bn_f_;
-
-  unsigned char H_[SHA512_DIGEST_LENGTH];
-
-  unsigned char *session_id_;
+  ssh_session sess;
 
   bool is_first_kex_;
 
@@ -83,8 +73,6 @@ private:
   void sendNewKeys();
   bool changeKeyAlg();
 
-  unsigned char *computeKey(int len, char flag);
-
 public:
   FQTermSSH2Kex(const char *V_C, const char *V_S);
   ~FQTermSSH2Kex();
diff --git a/src/protocol/internal/fqterm_ssh_buffer.cpp b/src/protocol/internal/fqterm_ssh_buffer.cpp
index d307c78..9775891 100644
--- a/src/protocol/internal/fqterm_ssh_buffer.cpp
+++ b/src/protocol/internal/fqterm_ssh_buffer.cpp
@@ -127,34 +127,6 @@ void FQTermSSHBuffer::putSSH1BN(BIGNUM *bignum) {
   delete [] buf;
 }
 
-
-
-void FQTermSSHBuffer::putSSH2BN(BIGNUM *bignum) {
-  // FIXME: support negative number and add error handling.
-
-  FQ_VERIFY(!BN_is_negative(bignum));  // currently we don't support negative big number.
-
-  if (BN_is_zero(bignum)) {
-    this->putInt(0);
-  } else {
-    u_int bytes = BN_num_bytes(bignum) + 1;
-
-    FQ_VERIFY(bytes >= 2); // currently we don't support big numbers so small
-
-    std::vector<u_char> buf(bytes);
-    buf[0] = 0;
-
-    int bin_len = BN_bn2bin(bignum, &buf[0] + 1);
-
-    FQ_VERIFY(bin_len == (int)bytes - 1);
-      
-    u_int no_high_bit = (buf[1] & 0x80) ? 0 : 1;
-
-    this->putInt(bytes - no_high_bit);
-    this->putRawData((const char *)&buf[0] + no_high_bit, bytes - no_high_bit);
-  }
-}
-
 //==============================================================================
 // Retrieves a BIGNUM from the buffer.
 //==============================================================================
@@ -182,22 +154,6 @@ void FQTermSSHBuffer::getSSH1BN(BIGNUM *bignum) {
   consume(bytes);
 }
 
-void FQTermSSHBuffer::getSSH2BN(BIGNUM *bignum) {
-  // FIXME: support negative numbers and error handling
-  int len;
-  unsigned char *hex_data = (unsigned char *)getString(&len);
-
-  FQ_VERIFY(!(len > 0 && (hex_data[0] & 0x80)));  // don't support negative numbers.
-
-  FQ_VERIFY(len < 10240);  // don't support so large numbers.
-
-  BIGNUM *res = BN_bin2bn(hex_data, len, bignum);
-
-  FQ_VERIFY(res != NULL);
-
-  delete [] hex_data;
-}
-
 u_short FQTermSSHBuffer::getWord() {
   u_char buf[2];
   u_short data;
diff --git a/src/protocol/internal/fqterm_ssh_buffer.h b/src/protocol/internal/fqterm_ssh_buffer.h
index e06872f..0525557 100644
--- a/src/protocol/internal/fqterm_ssh_buffer.h
+++ b/src/protocol/internal/fqterm_ssh_buffer.h
@@ -62,9 +62,6 @@ class FQTermSSHBuffer {
   void putSSH1BN(BIGNUM *bignum);
   void getSSH1BN(BIGNUM *bignum);
 
-  void putSSH2BN(BIGNUM *bignum);
-  void getSSH2BN(BIGNUM *bignum);
-  
   void putInt(u_int data);
   u_int getInt();
   
diff --git a/src/protocol/internal/fqterm_ssh_packet.cpp b/src/protocol/internal/fqterm_ssh_packet.cpp
index d9a8f89..6e40de8 100644
--- a/src/protocol/internal/fqterm_ssh_packet.cpp
+++ b/src/protocol/internal/fqterm_ssh_packet.cpp
@@ -77,10 +77,6 @@ void FQTermSSHPacketSender::putBN(BIGNUM *bn) {
   buffer_->putSSH1BN(bn);
 }
 
-void FQTermSSHPacketSender::putBN2(BIGNUM *bn) {
-  buffer_->putSSH2BN(bn);
-}
-
 void FQTermSSHPacketSender::startPacket(int pkt_type) {
   buffer_->clear();
   buffer_->putByte(pkt_type);
@@ -162,10 +158,6 @@ void FQTermSSHPacketReceiver::getBN(BIGNUM *bignum) {
   buffer_->getSSH1BN(bignum);
 }
 
-void FQTermSSHPacketReceiver::getBN2(BIGNUM *bignum) {
-  buffer_->getSSH2BN(bignum);
-}
-
 void FQTermSSHPacketReceiver::consume(int len) {
   buffer_->consume(len);
 }
diff --git a/src/protocol/internal/fqterm_ssh_packet.h b/src/protocol/internal/fqterm_ssh_packet.h
index 7caa663..83dcac7 100644
--- a/src/protocol/internal/fqterm_ssh_packet.h
+++ b/src/protocol/internal/fqterm_ssh_packet.h
@@ -50,7 +50,6 @@ class FQTermSSHPacketSender: public QObject {
   void putString(const char *string, int len = -1);
   void putRawData(const char *data, int length);
   void putBN(BIGNUM *bignum);
-  void putBN2(BIGNUM *bignum);
   void write();
 
   virtual int getIVSize() const { return cipher->IVSize;}
@@ -101,7 +100,6 @@ class FQTermSSHPacketReceiver: public QObject {
   void *getString(int *length = NULL);
   void getRawData(char *data, int length);
   void getBN(BIGNUM *bignum);
-  void getBN2(BIGNUM *bignum);
   void consume(int len);
 
   virtual int packetDataLen() const { return real_data_len_;}
diff --git a/src/protocol/internal/ssh_diffie-hellman.c b/src/protocol/internal/ssh_diffie-hellman.c
index 72bd253..4b340b4 100644
--- a/src/protocol/internal/ssh_diffie-hellman.c
+++ b/src/protocol/internal/ssh_diffie-hellman.c
@@ -1,4 +1,5 @@
 #include "ssh_diffie-hellman.h"
+#include "ssh_session.h"
 #include "ssh_crypto_common.h"
 #include <stdlib.h>
 
@@ -64,15 +65,39 @@ ssh_dh_free(SSH_DH *dh)
 	BN_free(dh->g);
 	BN_free(dh->p);
 	ssh_md_ctx_free(dh->digest.mdctx);
+	BN_clear_free(dh->bn_x);
+	BN_CTX_free(dh->ctx);
+	free(dh->mpint_e);
+	free(dh->secret);
 	free(dh);
 }
 
-SSH_DH *
-ssh_dh_group1_sha1(void)
+static void dh_compute(SSH_DH *dh)
+{
+	dh->bn_x = BN_new();
+	BN_pseudo_rand_range(dh->bn_x, dh->p);
+
+	BIGNUM *bn_e = BN_new();
+	BN_mod_exp(bn_e, dh->g, dh->bn_x, dh->p, dh->ctx);
+	dh->e_len = BN_bn2mpi(bn_e, NULL);
+	dh->mpint_e = (unsigned char*)malloc(dh->e_len);
+	BN_bn2mpi(bn_e, dh->mpint_e);
+	BN_free(bn_e);
+}
+
+static SSH_DH *create_dh()
 {
 	SSH_DH *dh = (SSH_DH*)malloc(sizeof(SSH_DH));
+	dh->ctx = BN_CTX_new();
 	dh->g = BN_new();
 	dh->p = BN_new();
+	return dh;
+}
+
+SSH_DH *
+ssh_dh_group1_sha1(void)
+{
+	SSH_DH *dh = create_dh();
 	dh->digest = (evp_md_t) {
 		.mdctx = ssh_md_ctx_new(),
 		.md = EVP_sha1(),
@@ -80,15 +105,14 @@ ssh_dh_group1_sha1(void)
 	};
 	BN_set_word(dh->g, g);
 	BN_bin2bn(prime_group1, 128, dh->p);
+	dh_compute(dh);
 	return dh;
 }
 
 SSH_DH *
 ssh_dh_group14_sha1(void)
 {
-	SSH_DH *dh = (SSH_DH*)malloc(sizeof(SSH_DH));
-	dh->g = BN_new();
-	dh->p = BN_new();
+	SSH_DH *dh = create_dh();
 	dh->digest = (evp_md_t) {
 		.mdctx = ssh_md_ctx_new(),
 		.md = EVP_sha1(),
@@ -96,6 +120,7 @@ ssh_dh_group14_sha1(void)
 	};
 	BN_set_word(dh->g, g);
 	BN_bin2bn(prime_group14, 256, dh->p);
+	dh_compute(dh);
 	return dh;
 }
 
@@ -107,6 +132,19 @@ ssh_dh_hash(SSH_DH *dh, const unsigned char *in, unsigned char *out, size_t n)
 	EVP_DigestFinal_ex(dh->digest.mdctx, out, NULL);
 }
 
+int ssh_dh_compute_secret(SSH_DH *dh, const unsigned char *f_bin, int f_len)
+{
+	BIGNUM *bn_f = BN_new();
+	if (bn_f == NULL || BN_bin2bn(f_bin, f_len, bn_f) == NULL)
+		return -1;
+	BIGNUM *bn_k = BN_new();
+	BN_mod_exp(bn_k, bn_f, dh->bn_x, dh->p, dh->ctx);
+	dh->secret_len = BN_bn2mpi(bn_k, NULL);
+	dh->secret = (unsigned char*)malloc(dh->secret_len);
+	BN_bn2mpi(bn_k, dh->secret);
+	return 0;
+}
+
 struct
 {
 	const char *name;
@@ -128,3 +166,30 @@ search_dh(const char *s)
 }
 
 const char all_dh_list[] = "diffie-hellman-group14-sha1,diffie-hellman-group1-sha1";
+
+void computeKey(ssh_session *sess, int expected_len, char flag, unsigned char key[])
+{
+	SSH_DH *dh = sess->dh;
+
+	int len = 0;
+
+	EVP_MD_CTX *mdctx = dh->digest.mdctx;
+	const EVP_MD *md = dh->digest.md;
+	int hashlen = dh->digest.hashlen;
+
+	while (len < expected_len) {
+		EVP_DigestInit_ex(mdctx, md, NULL);
+		EVP_DigestUpdate(mdctx, dh->secret, dh->secret_len);
+		EVP_DigestUpdate(mdctx, sess->H, hashlen);
+
+		if (len == 0) {
+			EVP_DigestUpdate(mdctx, &flag, 1);
+			EVP_DigestUpdate(mdctx, sess->session_id, hashlen);
+		} else {
+			EVP_DigestUpdate(mdctx, key, len);
+		}
+
+		EVP_DigestFinal_ex(mdctx, key+len, NULL);
+		len += hashlen;
+	}
+}
diff --git a/src/protocol/internal/ssh_diffie-hellman.h b/src/protocol/internal/ssh_diffie-hellman.h
index 0716d48..f94e1fa 100644
--- a/src/protocol/internal/ssh_diffie-hellman.h
+++ b/src/protocol/internal/ssh_diffie-hellman.h
@@ -31,6 +31,12 @@ extern "C" {
 	{
 		BIGNUM *g; // generator
 		BIGNUM *p; // prime
+		BIGNUM *bn_x; /* random number */
+		int e_len;
+		unsigned char *mpint_e; /* g^x mod p */
+		int secret_len;
+		unsigned char *secret;
+		BN_CTX *ctx;
 		evp_md_t digest;
 	} SSH_DH;
 
@@ -38,6 +44,7 @@ extern "C" {
 	SSH_DH *ssh_dh_group1_sha1(void);
 	SSH_DH *ssh_dh_group14_sha1(void);
 	void ssh_dh_hash(SSH_DH*, const unsigned char* data, unsigned char*, size_t len);
+	int ssh_dh_compute_secret(SSH_DH*, const unsigned char *, int);
 
 	typedef SSH_DH*(*NEW_DH)(void);
 	extern const char all_dh_list[];
diff --git a/src/protocol/internal/ssh_session.h b/src/protocol/internal/ssh_session.h
new file mode 100644
index 0000000..d7df1c9
--- /dev/null
+++ b/src/protocol/internal/ssh_session.h
@@ -0,0 +1,23 @@
+#ifndef SSH_SESSION_H
+#define SSH_SESSION_H
+
+#include "ssh_diffie-hellman.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif /* } for better indentation in Vim */
+
+typedef struct
+{
+	unsigned char *session_id;
+	unsigned char H[SHA512_DIGEST_LENGTH];
+	SSH_DH *dh;
+} ssh_session;
+
+void computeKey(ssh_session *, int, char, unsigned char []);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif