Be safer with how we handle ltc_ecc_sets[] (particularly with

system libtomcrypt)

A bit of progress with ecdsa code

5 files changed, 100 insertions, 17 deletions

diff --git a/crypto_desc.c b/crypto_desc.c
index 403e085..ed4de30 100644
--- a/crypto_desc.c
+++ b/crypto_desc.c
@@ -2,6 +2,7 @@
 #include "dbutil.h"
 #include "crypto_desc.h"
 #include "ltc_prng.h"
+#include "ecc.h"
 
 #ifdef DROPBEAR_LTC_PRNG
 	int dropbear_ltc_prng = -1;
@@ -68,6 +69,7 @@ void crypto_init() {
 
 #ifdef DROPBEAR_ECC
 	ltc_mp = ltm_desc;
+	dropbear_ecc_fill_dp();
 #endif
 }
 
diff --git a/debug.h b/debug.h
index 02c100f..200c004 100644
--- a/debug.h
+++ b/debug.h
@@ -39,7 +39,7 @@
  * Caution: Don't use this in an unfriendly environment (ie unfirewalled),
  * since the printing may not sanitise strings etc. This will add a reasonable
  * amount to your executable size. */
-#define DEBUG_TRACE
+/*#define DEBUG_TRACE*/
 
 /* All functions writing to the cleartext payload buffer call
  * CHECKCLEARTOWRITE() before writing. This is only really useful if you're
@@ -69,7 +69,7 @@
 
 /* To debug with GDB it is easier to run with no forking of child processes.
    You will need to pass "-F" as well. */
-#define DEBUG_NOFORK
+/*#define DEBUG_NOFORK*/
 
 
 /* For testing as non-root on shadowed systems, include the crypt of a password
diff --git a/ecc.c b/ecc.c
index 10ae322..9e013bf 100644
--- a/ecc.c
+++ b/ecc.c
@@ -6,30 +6,60 @@
 
 #ifdef DROPBEAR_ECC
 
-// TODO: use raw bytes for the dp rather than the hex strings in libtomcrypt's ecc.c
+// .dp members are filled out by dropbear_ecc_fill_dp() at startup
 #ifdef DROPBEAR_ECC_256
-const struct dropbear_ecc_curve ecc_curve_nistp256 = {
-	.dp = &ltc_ecc_sets[0],
+struct dropbear_ecc_curve ecc_curve_nistp256 = {
+	.ltc_size = 32,
 	.hashdesc = &sha256_desc,
 	.name = "nistp256"
 };
 #endif
 #ifdef DROPBEAR_ECC_384
-const struct dropbear_ecc_curve ecc_curve_nistp384 = {
-	.dp = &ltc_ecc_sets[1],
+struct dropbear_ecc_curve ecc_curve_nistp384 = {
+	.ltc_size = 48,
 	.hashdesc = &sha384_desc,
 	.name = "nistp384"
 };
 #endif
 #ifdef DROPBEAR_ECC_521
-const struct dropbear_ecc_curve ecc_curve_nistp521 = {
-	.dp = &ltc_ecc_sets[2],
+struct dropbear_ecc_curve ecc_curve_nistp521 = {
+	.ltc_size = 66,
 	.hashdesc = &sha512_desc,
 	.name = "nistp521"
 };
 #endif
 
-static ecc_key * new_ecc_key(void) {
+struct dropbear_ecc_curve *dropbear_ecc_curves[] = {
+#ifdef DROPBEAR_ECC_256
+	&ecc_curve_nistp256,
+#endif
+#ifdef DROPBEAR_ECC_384
+	&ecc_curve_nistp384,
+#endif
+#ifdef DROPBEAR_ECC_521
+	&ecc_curve_nistp521,
+#endif
+	NULL
+};
+
+void dropbear_ecc_fill_dp() {
+	struct dropbear_ecc_curve **curve;
+	// libtomcrypt guarantees they're ordered by size
+	const ltc_ecc_set_type *dp = ltc_ecc_sets;
+	for (curve = dropbear_ecc_curves; *curve; curve++) {
+		for (;dp->size > 0; dp++) {
+			if (dp->size == (*curve)->ltc_size) {
+				(*curve)->dp = dp;
+				break;
+			}
+		}
+		if (!(*curve)->dp) {
+			dropbear_exit("Missing ECC params %s", (*curve)->name);
+		}
+	}
+}
+
+ecc_key * new_ecc_key(void) {
 	ecc_key *key = m_malloc(sizeof(*key));
 	key->pubkey.x = m_malloc(sizeof(mp_int));
 	key->pubkey.y = m_malloc(sizeof(mp_int));
diff --git a/ecc.h b/ecc.h
index 35775d8..7791a70 100644
--- a/ecc.h
+++ b/ecc.h
@@ -9,14 +9,18 @@
 #ifdef DROPBEAR_ECC
 
 struct dropbear_ecc_curve {
+	int ltc_size; // to match the byte sizes in ltc_ecc_sets[]
 	const ltc_ecc_set_type *dp; // curve domain parameters
 	const struct ltc_hash_descriptor *hashdesc;
 	const char *name;
 };
 
-extern const struct dropbear_ecc_curve ecc_curve_nistp256;
-extern const struct dropbear_ecc_curve ecc_curve_nistp384;
-extern const struct dropbear_ecc_curve ecc_curve_nistp521;
+extern struct dropbear_ecc_curve ecc_curve_nistp256;
+extern struct dropbear_ecc_curve ecc_curve_nistp384;
+extern struct dropbear_ecc_curve ecc_curve_nistp521;
+extern struct dropbear_ecc_curve *dropbear_ecc_curves[];
+
+void dropbear_ecc_fill_dp();
 
 // "pubkey" refers to a point, but LTC uses ecc_key structure for both public
 // and private keys
diff --git a/ecdsa.c b/ecdsa.c
index b29ef1f..9784c97 100644
--- a/ecdsa.c
+++ b/ecdsa.c
@@ -1,6 +1,7 @@
 #include "includes.h"
 #include "dbutil.h"
 #include "crypto_desc.h"
+#include "ecc.h"
 
 #ifdef DROPBEAR_ECDSA
 
@@ -10,17 +11,17 @@ ecc_key *gen_ecdsa_priv_key(unsigned int bit_size) {
 	switch (bit_size) {
 #ifdef DROPBEAR_ECC_256
 		case 256:
-			dp = &ltc_ecc_sets[0];
+			dp = ecc_curve_nistp256.dp;
 			break;
 #endif
 #ifdef DROPBEAR_ECC_384
 		case 384:
-			dp = &ltc_ecc_sets[0];
+			dp = ecc_curve_nistp384.dp;
 			break;
 #endif
 #ifdef DROPBEAR_ECC_521
 		case 521:
-			dp = &ltc_ecc_sets[0];
+			dp = ecc_curve_nistp521.dp;
 			break;
 #endif
 	}
@@ -45,8 +46,54 @@ ecc_key *gen_ecdsa_priv_key(unsigned int bit_size) {
 	return new_key;
 }
 
-int buf_get_ecdsa_pub_key(buffer* buf, ecc_key *key) {
+ecc_key *buf_get_ecdsa_pub_key(buffer* buf) {
+	unsigned char *key_ident = NULL, *identifier = NULL;
+	unsigned int key_ident_len, identifier_len;
+	buffer *q_buf = NULL;
+	struct dropbear_ecc_curve **curve;
+	ecc_key *new_key = NULL;
 
+	// string   "ecdsa-sha2-[identifier]"
+	key_ident = buf_getstring(buf, &key_ident_len);
+	// string   "ecdsa-sha2-[identifier]"
+	identifier = buf_getstring(buf, &identifier_len);
+
+	if (key_ident_len != identifier_len + strlen("ecdsa-sha2-")) {
+		TRACE(("Bad identifier lengths"))
+		goto out;
+	}
+	if (memcmp(&key_ident[strlen("ecdsa-sha2-")], identifier, identifier_len) != 0) {
+		TRACE(("mismatching identifiers"))
+		goto out;
+	}
+
+	for (curve = dropbear_ecc_curves; *curve; curve++) {
+		if (memcmp(identifier, (*curve)->name, strlen((*curve)->name)) == 0) {
+			break;
+		}
+	}
+	if (!*curve) {
+		TRACE(("couldn't match ecc curve"))
+		goto out;
+	}
+
+	// string Q
+	q_buf = buf_getstringbuf(buf);
+	new_key = buf_get_ecc_raw_pubkey(q_buf, *curve);
+
+out:
+	if (key_ident) {
+		m_free(key_ident);
+	}
+	if (identifier) {
+		m_free(identifier);
+	}
+	if (q_buf) {
+		buf_free(q_buf);
+		q_buf = NULL;
+	}
+	TRACE(("leave buf_get_ecdsa_pub_key"))	
+	return new_key;
 }