- markus@cvs.openbsd.org 2013/12/06 13:30:08

     [authfd.c key.c key.h ssh-agent.c]
     move private key (de)serialization to key.c; ok djm

5 files changed, 214 insertions, 179 deletions

diff --git a/ChangeLog b/ChangeLog
index f5e06c9e..0db8ab44 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,6 +9,9 @@
      remove duplicated character ('g') in getopt() string;
      document the (few) remaining option characters so we don't have to
      rummage next time.
+   - markus@cvs.openbsd.org 2013/12/06 13:30:08
+     [authfd.c key.c key.h ssh-agent.c]
+     move private key (de)serialization to key.c; ok djm
 
 20131205
  - (djm) OpenBSD CVS Sync
diff --git a/authfd.c b/authfd.c
index 775786be..2cda8b30 100644
--- a/authfd.c
+++ b/authfd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: authfd.c,v 1.87 2013/05/17 00:13:13 djm Exp $ */
+/* $OpenBSD: authfd.c,v 1.89 2013/12/06 13:30:08 markus Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -42,8 +42,8 @@
 #include <sys/socket.h>
 
 #include <openssl/evp.h>
-
 #include <openssl/crypto.h>
+
 #include <fcntl.h>
 #include <stdlib.h>
 #include <signal.h>
@@ -206,7 +206,7 @@ ssh_get_authentication_connection(void)
 	if (sock < 0)
 		return NULL;
 
-	auth = xmalloc(sizeof(*auth));
+	auth = xcalloc(1, sizeof(*auth));
 	auth->fd = sock;
 	buffer_init(&auth->identities);
 	auth->howmany = 0;
@@ -474,58 +474,7 @@ ssh_encode_identity_rsa1(Buffer *b, RSA *key, const char *comment)
 static void
 ssh_encode_identity_ssh2(Buffer *b, Key *key, const char *comment)
 {
-	buffer_put_cstring(b, key_ssh_name(key));
-	switch (key->type) {
-	case KEY_RSA:
-		buffer_put_bignum2(b, key->rsa->n);
-		buffer_put_bignum2(b, key->rsa->e);
-		buffer_put_bignum2(b, key->rsa->d);
-		buffer_put_bignum2(b, key->rsa->iqmp);
-		buffer_put_bignum2(b, key->rsa->p);
-		buffer_put_bignum2(b, key->rsa->q);
-		break;
-	case KEY_RSA_CERT_V00:
-	case KEY_RSA_CERT:
-		if (key->cert == NULL || buffer_len(&key->cert->certblob) == 0)
-			fatal("%s: no cert/certblob", __func__);
-		buffer_put_string(b, buffer_ptr(&key->cert->certblob),
-		    buffer_len(&key->cert->certblob));
-		buffer_put_bignum2(b, key->rsa->d);
-		buffer_put_bignum2(b, key->rsa->iqmp);
-		buffer_put_bignum2(b, key->rsa->p);
-		buffer_put_bignum2(b, key->rsa->q);
-		break;
-	case KEY_DSA:
-		buffer_put_bignum2(b, key->dsa->p);
-		buffer_put_bignum2(b, key->dsa->q);
-		buffer_put_bignum2(b, key->dsa->g);
-		buffer_put_bignum2(b, key->dsa->pub_key);
-		buffer_put_bignum2(b, key->dsa->priv_key);
-		break;
-	case KEY_DSA_CERT_V00:
-	case KEY_DSA_CERT:
-		if (key->cert == NULL || buffer_len(&key->cert->certblob) == 0)
-			fatal("%s: no cert/certblob", __func__);
-		buffer_put_string(b, buffer_ptr(&key->cert->certblob),
-		    buffer_len(&key->cert->certblob));
-		buffer_put_bignum2(b, key->dsa->priv_key);
-		break;
-#ifdef OPENSSL_HAS_ECC
-	case KEY_ECDSA:
-		buffer_put_cstring(b, key_curve_nid_to_name(key->ecdsa_nid));
-		buffer_put_ecpoint(b, EC_KEY_get0_group(key->ecdsa),
-		    EC_KEY_get0_public_key(key->ecdsa));
-		buffer_put_bignum2(b, EC_KEY_get0_private_key(key->ecdsa));
-		break;
-	case KEY_ECDSA_CERT:
-		if (key->cert == NULL || buffer_len(&key->cert->certblob) == 0)
-			fatal("%s: no cert/certblob", __func__);
-		buffer_put_string(b, buffer_ptr(&key->cert->certblob),
-		    buffer_len(&key->cert->certblob));
-		buffer_put_bignum2(b, EC_KEY_get0_private_key(key->ecdsa));
-		break;
-#endif
-	}
+	key_private_serialize(key, b);
 	buffer_put_cstring(b, comment);
 }
 
diff --git a/key.c b/key.c
index 3ee2d61a..0282e082 100644
--- a/key.c
+++ b/key.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: key.c,v 1.106 2013/12/02 03:09:22 djm Exp $ */
+/* $OpenBSD: key.c,v 1.107 2013/12/06 13:30:08 markus Exp $ */
 /*
  * read_bignum():
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -2276,3 +2276,194 @@ key_dump_ec_key(const EC_KEY *key)
 }
 #endif /* defined(DEBUG_KEXECDH) || defined(DEBUG_PK) */
 #endif /* OPENSSL_HAS_ECC */
+
+void
+key_private_serialize(const Key *key, Buffer *b)
+{
+	buffer_put_cstring(b, key_ssh_name(key));
+	switch (key->type) {
+	case KEY_RSA:
+		buffer_put_bignum2(b, key->rsa->n);
+		buffer_put_bignum2(b, key->rsa->e);
+		buffer_put_bignum2(b, key->rsa->d);
+		buffer_put_bignum2(b, key->rsa->iqmp);
+		buffer_put_bignum2(b, key->rsa->p);
+		buffer_put_bignum2(b, key->rsa->q);
+		break;
+	case KEY_RSA_CERT_V00:
+	case KEY_RSA_CERT:
+		if (key->cert == NULL || buffer_len(&key->cert->certblob) == 0)
+			fatal("%s: no cert/certblob", __func__);
+		buffer_put_string(b, buffer_ptr(&key->cert->certblob),
+		    buffer_len(&key->cert->certblob));
+		buffer_put_bignum2(b, key->rsa->d);
+		buffer_put_bignum2(b, key->rsa->iqmp);
+		buffer_put_bignum2(b, key->rsa->p);
+		buffer_put_bignum2(b, key->rsa->q);
+		break;
+	case KEY_DSA:
+		buffer_put_bignum2(b, key->dsa->p);
+		buffer_put_bignum2(b, key->dsa->q);
+		buffer_put_bignum2(b, key->dsa->g);
+		buffer_put_bignum2(b, key->dsa->pub_key);
+		buffer_put_bignum2(b, key->dsa->priv_key);
+		break;
+	case KEY_DSA_CERT_V00:
+	case KEY_DSA_CERT:
+		if (key->cert == NULL || buffer_len(&key->cert->certblob) == 0)
+			fatal("%s: no cert/certblob", __func__);
+		buffer_put_string(b, buffer_ptr(&key->cert->certblob),
+		    buffer_len(&key->cert->certblob));
+		buffer_put_bignum2(b, key->dsa->priv_key);
+		break;
+#ifdef OPENSSL_HAS_ECC
+	case KEY_ECDSA:
+		buffer_put_cstring(b, key_curve_nid_to_name(key->ecdsa_nid));
+		buffer_put_ecpoint(b, EC_KEY_get0_group(key->ecdsa),
+		    EC_KEY_get0_public_key(key->ecdsa));
+		buffer_put_bignum2(b, EC_KEY_get0_private_key(key->ecdsa));
+		break;
+	case KEY_ECDSA_CERT:
+		if (key->cert == NULL || buffer_len(&key->cert->certblob) == 0)
+			fatal("%s: no cert/certblob", __func__);
+		buffer_put_string(b, buffer_ptr(&key->cert->certblob),
+		    buffer_len(&key->cert->certblob));
+		buffer_put_bignum2(b, EC_KEY_get0_private_key(key->ecdsa));
+		break;
+#endif /* OPENSSL_HAS_ECC */
+	}
+}
+
+Key *
+key_private_deserialize(Buffer *blob)
+{
+	char *type_name;
+	Key *k = NULL;
+	u_char *cert;
+	u_int len;
+	int type;
+#ifdef OPENSSL_HAS_ECC
+	char *curve;
+	BIGNUM *exponent;
+	EC_POINT *q;
+#endif
+
+	type_name = buffer_get_string(blob, NULL);
+	type = key_type_from_name(type_name);
+	switch (type) {
+	case KEY_DSA:
+		k = key_new_private(type);
+		buffer_get_bignum2(blob, k->dsa->p);
+		buffer_get_bignum2(blob, k->dsa->q);
+		buffer_get_bignum2(blob, k->dsa->g);
+		buffer_get_bignum2(blob, k->dsa->pub_key);
+		buffer_get_bignum2(blob, k->dsa->priv_key);
+		break;
+	case KEY_DSA_CERT_V00:
+	case KEY_DSA_CERT:
+		cert = buffer_get_string(blob, &len);
+		if ((k = key_from_blob(cert, len)) == NULL)
+			fatal("Certificate parse failed");
+		free(cert);
+		key_add_private(k);
+		buffer_get_bignum2(blob, k->dsa->priv_key);
+		break;
+#ifdef OPENSSL_HAS_ECC
+	case KEY_ECDSA:
+		k = key_new_private(type);
+		k->ecdsa_nid = key_ecdsa_nid_from_name(type_name);
+		curve = buffer_get_string(blob, NULL);
+		if (k->ecdsa_nid != key_curve_name_to_nid(curve))
+			fatal("%s: curve names mismatch", __func__);
+		free(curve);
+		k->ecdsa = EC_KEY_new_by_curve_name(k->ecdsa_nid);
+		if (k->ecdsa == NULL)
+			fatal("%s: EC_KEY_new_by_curve_name failed",
+			    __func__);
+		q = EC_POINT_new(EC_KEY_get0_group(k->ecdsa));
+		if (q == NULL)
+			fatal("%s: BN_new failed", __func__);
+		if ((exponent = BN_new()) == NULL)
+			fatal("%s: BN_new failed", __func__);
+		buffer_get_ecpoint(blob,
+			EC_KEY_get0_group(k->ecdsa), q);
+		buffer_get_bignum2(blob, exponent);
+		if (EC_KEY_set_public_key(k->ecdsa, q) != 1)
+			fatal("%s: EC_KEY_set_public_key failed",
+			    __func__);
+		if (EC_KEY_set_private_key(k->ecdsa, exponent) != 1)
+			fatal("%s: EC_KEY_set_private_key failed",
+			    __func__);
+		if (key_ec_validate_public(EC_KEY_get0_group(k->ecdsa),
+		    EC_KEY_get0_public_key(k->ecdsa)) != 0)
+			fatal("%s: bad ECDSA public key", __func__);
+		if (key_ec_validate_private(k->ecdsa) != 0)
+			fatal("%s: bad ECDSA private key", __func__);
+		BN_clear_free(exponent);
+		EC_POINT_free(q);
+		break;
+	case KEY_ECDSA_CERT:
+		cert = buffer_get_string(blob, &len);
+		if ((k = key_from_blob(cert, len)) == NULL)
+			fatal("Certificate parse failed");
+		free(cert);
+		key_add_private(k);
+		if ((exponent = BN_new()) == NULL)
+			fatal("%s: BN_new failed", __func__);
+		buffer_get_bignum2(blob, exponent);
+		if (EC_KEY_set_private_key(k->ecdsa, exponent) != 1)
+			fatal("%s: EC_KEY_set_private_key failed",
+			    __func__);
+		if (key_ec_validate_public(EC_KEY_get0_group(k->ecdsa),
+		    EC_KEY_get0_public_key(k->ecdsa)) != 0 ||
+		    key_ec_validate_private(k->ecdsa) != 0)
+			fatal("%s: bad ECDSA key", __func__);
+		BN_clear_free(exponent);
+		break;
+#endif
+	case KEY_RSA:
+		k = key_new_private(type);
+		buffer_get_bignum2(blob, k->rsa->n);
+		buffer_get_bignum2(blob, k->rsa->e);
+		buffer_get_bignum2(blob, k->rsa->d);
+		buffer_get_bignum2(blob, k->rsa->iqmp);
+		buffer_get_bignum2(blob, k->rsa->p);
+		buffer_get_bignum2(blob, k->rsa->q);
+
+		/* Generate additional parameters */
+		rsa_generate_additional_parameters(k->rsa);
+		break;
+	case KEY_RSA_CERT_V00:
+	case KEY_RSA_CERT:
+		cert = buffer_get_string(blob, &len);
+		if ((k = key_from_blob(cert, len)) == NULL)
+			fatal("Certificate parse failed");
+		free(cert);
+		key_add_private(k);
+		buffer_get_bignum2(blob, k->rsa->d);
+		buffer_get_bignum2(blob, k->rsa->iqmp);
+		buffer_get_bignum2(blob, k->rsa->p);
+		buffer_get_bignum2(blob, k->rsa->q);
+		break;
+	default:
+		free(type_name);
+		buffer_clear(blob);
+		return NULL;
+	}
+	free(type_name);
+
+	/* enable blinding */
+	switch (k->type) {
+	case KEY_RSA:
+	case KEY_RSA_CERT_V00:
+	case KEY_RSA_CERT:
+	case KEY_RSA1:
+		if (RSA_blinding_on(k->rsa, NULL) != 1) {
+			error("%s: RSA_blinding_on failed", __func__);
+			key_free(k);
+			return NULL;
+		}
+		break;
+	}
+	return k;
+}
diff --git a/key.h b/key.h
index 8a78a828..efa71176 100644
--- a/key.h
+++ b/key.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: key.h,v 1.38 2013/10/29 09:42:11 djm Exp $ */
+/* $OpenBSD: key.h,v 1.39 2013/12/06 13:30:08 markus Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -151,4 +151,7 @@ void	key_dump_ec_point(const EC_GROUP *, const EC_POINT *);
 void	key_dump_ec_key(const EC_KEY *);
 #endif
 
+void     key_private_serialize(const Key *, Buffer *);
+Key	*key_private_deserialize(Buffer *);
+
 #endif
diff --git a/ssh-agent.c b/ssh-agent.c
index c3b11729..0196f8f6 100644
--- a/ssh-agent.c
+++ b/ssh-agent.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-agent.c,v 1.177 2013/07/20 01:50:20 djm Exp $ */
+/* $OpenBSD: ssh-agent.c,v 1.178 2013/12/06 13:30:08 markus Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -464,16 +464,9 @@ process_add_identity(SocketEntry *e, int version)
 	Idtab *tab = idtab_lookup(version);
 	Identity *id;
 	int type, success = 0, confirm = 0;
-	char *type_name, *comment;
+	char *comment;
 	time_t death = 0;
 	Key *k = NULL;
-#ifdef OPENSSL_HAS_ECC
-	BIGNUM *exponent;
-	EC_POINT *q;
-	char *curve;
-#endif
-	u_char *cert;
-	u_int len;
 
 	switch (version) {
 	case 1:
@@ -490,125 +483,21 @@ process_add_identity(SocketEntry *e, int version)
 
 		/* Generate additional parameters */
 		rsa_generate_additional_parameters(k->rsa);
-		break;
-	case 2:
-		type_name = buffer_get_string(&e->request, NULL);
-		type = key_type_from_name(type_name);
-		switch (type) {
-		case KEY_DSA:
-			k = key_new_private(type);
-			buffer_get_bignum2(&e->request, k->dsa->p);
-			buffer_get_bignum2(&e->request, k->dsa->q);
-			buffer_get_bignum2(&e->request, k->dsa->g);
-			buffer_get_bignum2(&e->request, k->dsa->pub_key);
-			buffer_get_bignum2(&e->request, k->dsa->priv_key);
-			break;
-		case KEY_DSA_CERT_V00:
-		case KEY_DSA_CERT:
-			cert = buffer_get_string(&e->request, &len);
-			if ((k = key_from_blob(cert, len)) == NULL)
-				fatal("Certificate parse failed");
-			free(cert);
-			key_add_private(k);
-			buffer_get_bignum2(&e->request, k->dsa->priv_key);
-			break;
-#ifdef OPENSSL_HAS_ECC
-		case KEY_ECDSA:
-			k = key_new_private(type);
-			k->ecdsa_nid = key_ecdsa_nid_from_name(type_name);
-			curve = buffer_get_string(&e->request, NULL);
-			if (k->ecdsa_nid != key_curve_name_to_nid(curve))
-				fatal("%s: curve names mismatch", __func__);
-			free(curve);
-			k->ecdsa = EC_KEY_new_by_curve_name(k->ecdsa_nid);
-			if (k->ecdsa == NULL)
-				fatal("%s: EC_KEY_new_by_curve_name failed",
-				    __func__);
-			q = EC_POINT_new(EC_KEY_get0_group(k->ecdsa));
-			if (q == NULL)
-				fatal("%s: BN_new failed", __func__);
-			if ((exponent = BN_new()) == NULL)
-				fatal("%s: BN_new failed", __func__);
-			buffer_get_ecpoint(&e->request,
-				EC_KEY_get0_group(k->ecdsa), q);
-			buffer_get_bignum2(&e->request, exponent);
-			if (EC_KEY_set_public_key(k->ecdsa, q) != 1)
-				fatal("%s: EC_KEY_set_public_key failed",
-				    __func__);
-			if (EC_KEY_set_private_key(k->ecdsa, exponent) != 1)
-				fatal("%s: EC_KEY_set_private_key failed",
-				    __func__);
-			if (key_ec_validate_public(EC_KEY_get0_group(k->ecdsa),
-			    EC_KEY_get0_public_key(k->ecdsa)) != 0)
-				fatal("%s: bad ECDSA public key", __func__);
-			if (key_ec_validate_private(k->ecdsa) != 0)
-				fatal("%s: bad ECDSA private key", __func__);
-			BN_clear_free(exponent);
-			EC_POINT_free(q);
-			break;
-		case KEY_ECDSA_CERT:
-			cert = buffer_get_string(&e->request, &len);
-			if ((k = key_from_blob(cert, len)) == NULL)
-				fatal("Certificate parse failed");
-			free(cert);
-			key_add_private(k);
-			if ((exponent = BN_new()) == NULL)
-				fatal("%s: BN_new failed", __func__);
-			buffer_get_bignum2(&e->request, exponent);
-			if (EC_KEY_set_private_key(k->ecdsa, exponent) != 1)
-				fatal("%s: EC_KEY_set_private_key failed",
-				    __func__);
-			if (key_ec_validate_public(EC_KEY_get0_group(k->ecdsa),
-			    EC_KEY_get0_public_key(k->ecdsa)) != 0 ||
-			    key_ec_validate_private(k->ecdsa) != 0)
-				fatal("%s: bad ECDSA key", __func__);
-			BN_clear_free(exponent);
-			break;
-#endif /* OPENSSL_HAS_ECC */
-		case KEY_RSA:
-			k = key_new_private(type);
-			buffer_get_bignum2(&e->request, k->rsa->n);
-			buffer_get_bignum2(&e->request, k->rsa->e);
-			buffer_get_bignum2(&e->request, k->rsa->d);
-			buffer_get_bignum2(&e->request, k->rsa->iqmp);
-			buffer_get_bignum2(&e->request, k->rsa->p);
-			buffer_get_bignum2(&e->request, k->rsa->q);
-
-			/* Generate additional parameters */
-			rsa_generate_additional_parameters(k->rsa);
-			break;
-		case KEY_RSA_CERT_V00:
-		case KEY_RSA_CERT:
-			cert = buffer_get_string(&e->request, &len);
-			if ((k = key_from_blob(cert, len)) == NULL)
-				fatal("Certificate parse failed");
-			free(cert);
-			key_add_private(k);
-			buffer_get_bignum2(&e->request, k->rsa->d);
-			buffer_get_bignum2(&e->request, k->rsa->iqmp);
-			buffer_get_bignum2(&e->request, k->rsa->p);
-			buffer_get_bignum2(&e->request, k->rsa->q);
-			break;
-		default:
-			free(type_name);
-			buffer_clear(&e->request);
-			goto send;
-		}
-		free(type_name);
-		break;
-	}
-	/* enable blinding */
-	switch (k->type) {
-	case KEY_RSA:
-	case KEY_RSA_CERT_V00:
-	case KEY_RSA_CERT:
-	case KEY_RSA1:
+
+		/* enable blinding */
 		if (RSA_blinding_on(k->rsa, NULL) != 1) {
 			error("process_add_identity: RSA_blinding_on failed");
 			key_free(k);
 			goto send;
 		}
 		break;
+	case 2:
+		k = key_private_deserialize(&e->request);
+		if (k == NULL) {
+			buffer_clear(&e->request);
+			goto send;
+		}
+		break;
 	}
 	comment = buffer_get_string(&e->request, NULL);
 	if (k == NULL) {