Hello Richard,

Richard Levitte wrote:
In message <571f2941.4040...@openssl.org> on Tue, 26 Apr 2016 09:39:29 +0100, Matt 
Caswell <m...@openssl.org> said:

[SNIP]
I've seen no other opinion, so I went with it.  Would you mind having
a look at GH#995?  I did a bit of change in the docs, but could need
some help expressing it in a better manner.

Also, I'd like to hear from Douglas and Tomas if these changes found
in said pull request would fit your bill better...  basically, it
allows (or should allow, unless I've goofed something up) a call set
like this:

     RSA_set0_key(rsa, n, e, NULL);
     /* other stuff done, such as calculatig d */
     RSA_set0_key(rsa, NULL, NULL, d);
As methods allows user to set only public part I would like to propose to add new key method "...set0_privkey" to set just private key. This will allow to avoid duplicate of key public part between get0 and set0 key methods.


For protocol "0009-sshkey.c-opaque-DSA-structure.patch" is practical sample of an upgrade to 1.1 API. RSA is similar.


Cheers,
Richard


Roumen

>From 57d17bdf3ef9975b6f09a597557843943909b5b9 Mon Sep 17 00:00:00 2001
From: Roumen Petrov <open...@roumenpetrov.info>
Date: Sun, 3 Apr 2016 21:24:27 +0300
Subject: [PATCH 09/31] sshkey.c: opaque DSA structure

---
 sshkey.c | 180 +++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 140 insertions(+), 40 deletions(-)

diff --git a/sshkey.c b/sshkey.c
index 6d4a377..0bba185 100644
--- a/sshkey.c
+++ b/sshkey.c
@@ -4,7 +4,7 @@
  * Copyright (c) 2008 Alexander von Gernler.  All rights reserved.
  * Copyright (c) 2010,2011 Damien Miller.  All rights reserved.
  * X509 certificate support,
- * Copyright (c) 2002-2015 Roumen Petrov.  All rights reserved.
+ * Copyright (c) 2002-2016 Roumen Petrov.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -362,7 +362,11 @@ sshkey_size(const struct sshkey *k)
 		return BN_num_bits(k->rsa->n);
 	case KEY_DSA:
 	case KEY_DSA_CERT:
-		return BN_num_bits(k->dsa->p);
+		{
+		BIGNUM *p = NULL;
+		DSA_get0_pqg(k->dsa, &p, NULL, NULL);
+		return BN_num_bits(p);
+		}
 	case KEY_ECDSA:
 	case KEY_ECDSA_CERT:
 		return sshkey_curve_nid_to_bits(k->ecdsa_nid);
@@ -588,17 +592,27 @@ sshkey_new(int type)
 		break;
 	case KEY_DSA:
 	case KEY_DSA_CERT:
+		{
+		BIGNUM *p = NULL, *q = NULL, *g = NULL, *pub_key = NULL;
+
 		if ((dsa = DSA_new()) == NULL ||
-		    (dsa->p = BN_new()) == NULL ||
-		    (dsa->q = BN_new()) == NULL ||
-		    (dsa->g = BN_new()) == NULL ||
-		    (dsa->pub_key = BN_new()) == NULL) {
+		    (p = BN_new()) == NULL ||
+		    (q = BN_new()) == NULL ||
+		    (g = BN_new()) == NULL ||
+		    (pub_key = BN_new()) == NULL) {
+			BN_free(p);
+			BN_free(q);
+			BN_free(g);
+			BN_free(pub_key);
 			if (dsa != NULL)
 				DSA_free(dsa);
 			free(k);
 			return NULL;
 		}
+		DSA_set0_pqg(dsa, p, q, g);
+		DSA_set0_key(dsa, pub_key, NULL);
 		k->dsa = dsa;
+		}
 		break;
 	case KEY_ECDSA:
 	case KEY_ECDSA_CERT:
@@ -646,8 +660,19 @@ sshkey_add_private(struct sshkey *k)
 		break;
 	case KEY_DSA:
 	case KEY_DSA_CERT:
-		if (bn_maybe_alloc_failed(k->dsa->priv_key))
+		{
+		BIGNUM *pub_key = NULL, *priv_key = NULL;
+
+		if (bn_maybe_alloc_failed(priv_key))
+			return SSH_ERR_ALLOC_FAIL;
+		DSA_get0_key(k->dsa, &pub_key, NULL);
+		pub_key = BN_dup(pub_key);
+		if (pub_key == NULL) {
+			BN_free(priv_key);
 			return SSH_ERR_ALLOC_FAIL;
+		}
+		DSA_set0_key(k->dsa, pub_key, priv_key);
+		}
 		break;
 #undef bn_maybe_alloc_failed
 	case KEY_ECDSA:
@@ -914,14 +939,22 @@ to_blob_buf(const struct sshkey *key, struct sshbuf *b, int force_plain)
 		break;
 #ifdef WITH_OPENSSL
 	case KEY_DSA:
+		{
+		BIGNUM *p = NULL, *q = NULL, *g = NULL, *pub_key = NULL;
+
 		if (key->dsa == NULL)
 			return SSH_ERR_INVALID_ARGUMENT;
+
+		DSA_get0_pqg(key->dsa, &p, &q, &g);
+		DSA_get0_key(key->dsa, &pub_key, NULL);
+
 		if ((ret = sshbuf_put_cstring(b, typename)) != 0 ||
-		    (ret = sshbuf_put_bignum2(b, key->dsa->p)) != 0 ||
-		    (ret = sshbuf_put_bignum2(b, key->dsa->q)) != 0 ||
-		    (ret = sshbuf_put_bignum2(b, key->dsa->g)) != 0 ||
-		    (ret = sshbuf_put_bignum2(b, key->dsa->pub_key)) != 0)
+		    (ret = sshbuf_put_bignum2(b, p)) != 0 ||
+		    (ret = sshbuf_put_bignum2(b, q)) != 0 ||
+		    (ret = sshbuf_put_bignum2(b, g)) != 0 ||
+		    (ret = sshbuf_put_bignum2(b, pub_key)) != 0)
 			return ret;
+		}
 		break;
 # ifdef OPENSSL_HAS_ECC
 	case KEY_ECDSA:
@@ -1971,13 +2004,25 @@ sshkey_from_private(const struct sshkey *k, struct sshkey **pkp)
 	case KEY_DSA_CERT:
 		if ((n = sshkey_new(k->type)) == NULL)
 			return SSH_ERR_ALLOC_FAIL;
-		if ((BN_copy(n->dsa->p, k->dsa->p) == NULL) ||
-		    (BN_copy(n->dsa->q, k->dsa->q) == NULL) ||
-		    (BN_copy(n->dsa->g, k->dsa->g) == NULL) ||
-		    (BN_copy(n->dsa->pub_key, k->dsa->pub_key) == NULL)) {
+
+		{
+		BIGNUM *k_p = NULL, *k_q = NULL, *k_g = NULL, *k_pub_key = NULL;
+		BIGNUM *n_p = NULL, *n_q = NULL, *n_g = NULL, *n_pub_key = NULL;
+
+		DSA_get0_pqg(k->dsa, &k_p, &k_q, &k_g);
+		DSA_get0_key(k->dsa, &k_pub_key, NULL);
+
+		DSA_get0_pqg(n->dsa, &n_p, &n_q, &n_g);
+		DSA_get0_key(n->dsa, &n_pub_key, NULL);
+
+		if ((BN_copy(n_p, k_p) == NULL) ||
+		    (BN_copy(n_q, k_q) == NULL) ||
+		    (BN_copy(n_g, k_g) == NULL) ||
+		    (BN_copy(n_pub_key, k_pub_key) == NULL)) {
 			sshkey_free(n);
 			return SSH_ERR_ALLOC_FAIL;
 		}
+		}
 		break;
 # ifdef OPENSSL_HAS_ECC
 	case KEY_ECDSA:
@@ -2233,13 +2278,21 @@ sshkey_from_blob_internal(struct sshbuf *b, struct sshkey **keyp,
 			ret = SSH_ERR_ALLOC_FAIL;
 			goto out;
 		}
-		if (sshbuf_get_bignum2(b, key->dsa->p) != 0 ||
-		    sshbuf_get_bignum2(b, key->dsa->q) != 0 ||
-		    sshbuf_get_bignum2(b, key->dsa->g) != 0 ||
-		    sshbuf_get_bignum2(b, key->dsa->pub_key) != 0) {
+		{
+		BIGNUM *dsa_p = NULL, *dsa_q = NULL, *dsa_g = NULL;
+		BIGNUM *dsa_pub_key = NULL;
+
+		DSA_get0_pqg(key->dsa, &dsa_p, &dsa_q, &dsa_g);
+		DSA_get0_key(key->dsa, &dsa_pub_key, NULL);
+
+		if (sshbuf_get_bignum2(b, dsa_p) != 0 ||
+		    sshbuf_get_bignum2(b, dsa_q) != 0 ||
+		    sshbuf_get_bignum2(b, dsa_g) != 0 ||
+		    sshbuf_get_bignum2(b, dsa_pub_key) != 0) {
 			ret = SSH_ERR_INVALID_FORMAT;
 			goto out;
 		}
+		}
 #ifdef DEBUG_PK
 		DSA_print_fp(stderr, key->dsa, 8);
 #endif
@@ -2505,14 +2558,27 @@ sshkey_demote(const struct sshkey *k, struct sshkey **dkp)
 			goto fail;
 		/* FALLTHROUGH */
 	case KEY_DSA:
+		{
+		BIGNUM *p = NULL, *q = NULL, *g = NULL, *pub_key = NULL;
+
+		DSA_get0_pqg(k->dsa, &p, &q, &g);
+		DSA_get0_key(k->dsa, &pub_key, NULL);
+
 		if ((pk->dsa = DSA_new()) == NULL ||
-		    (pk->dsa->p = BN_dup(k->dsa->p)) == NULL ||
-		    (pk->dsa->q = BN_dup(k->dsa->q)) == NULL ||
-		    (pk->dsa->g = BN_dup(k->dsa->g)) == NULL ||
-		    (pk->dsa->pub_key = BN_dup(k->dsa->pub_key)) == NULL) {
+		    (p = BN_dup(p)) == NULL ||
+		    (q = BN_dup(q)) == NULL ||
+		    (g = BN_dup(g)) == NULL ||
+		    (pub_key = BN_dup(pub_key)) == NULL) {
+			BN_free(p);
+			BN_free(q);
+			BN_free(g);
+			BN_free(pub_key);
 			ret = SSH_ERR_ALLOC_FAIL;
 			goto fail;
 		}
+		DSA_set0_pqg(pk->dsa, p, q, g);
+		DSA_set0_key(pk->dsa, pub_key, NULL);
+		}
 		break;
 	case KEY_ECDSA_CERT:
 		if ((ret = sshkey_cert_copy(k, pk)) != 0)
@@ -2636,11 +2702,18 @@ sshkey_certify(struct sshkey *k, struct sshkey *ca)
 	switch (k->type) {
 #ifdef WITH_OPENSSL
 	case KEY_DSA_CERT:
-		if ((ret = sshbuf_put_bignum2(cert, k->dsa->p)) != 0 ||
-		    (ret = sshbuf_put_bignum2(cert, k->dsa->q)) != 0 ||
-		    (ret = sshbuf_put_bignum2(cert, k->dsa->g)) != 0 ||
-		    (ret = sshbuf_put_bignum2(cert, k->dsa->pub_key)) != 0)
+		{
+		BIGNUM *p = NULL, *q = NULL, *g = NULL, *pub_key = NULL;
+
+		DSA_get0_pqg(k->dsa, &p, &q, &g);
+		DSA_get0_key(k->dsa, &pub_key, NULL);
+
+		if ((ret = sshbuf_put_bignum2(cert, p)) != 0 ||
+		    (ret = sshbuf_put_bignum2(cert, q)) != 0 ||
+		    (ret = sshbuf_put_bignum2(cert, g)) != 0 ||
+		    (ret = sshbuf_put_bignum2(cert, pub_key)) != 0)
 			goto out;
+		}
 		break;
 # ifdef OPENSSL_HAS_ECC
 	case KEY_ECDSA_CERT:
@@ -2834,21 +2907,34 @@ sshkey_private_serialize(const struct sshkey *key, struct sshbuf *b)
 			goto out;
 		break;
 	case KEY_DSA:
-		if ((r = sshbuf_put_bignum2(b, key->dsa->p)) != 0 ||
-		    (r = sshbuf_put_bignum2(b, key->dsa->q)) != 0 ||
-		    (r = sshbuf_put_bignum2(b, key->dsa->g)) != 0 ||
-		    (r = sshbuf_put_bignum2(b, key->dsa->pub_key)) != 0 ||
-		    (r = sshbuf_put_bignum2(b, key->dsa->priv_key)) != 0)
+		{
+		BIGNUM *p = NULL, *q = NULL, *g = NULL;
+		BIGNUM *pub_key = NULL, *priv_key = NULL;
+
+		DSA_get0_pqg(key->dsa, &p, &q, &g);
+		DSA_get0_key(key->dsa, &pub_key, &priv_key);
+
+		if ((r = sshbuf_put_bignum2(b, p)) != 0 ||
+		    (r = sshbuf_put_bignum2(b, q)) != 0 ||
+		    (r = sshbuf_put_bignum2(b, g)) != 0 ||
+		    (r = sshbuf_put_bignum2(b, pub_key)) != 0 ||
+		    (r = sshbuf_put_bignum2(b, priv_key)) != 0)
 			goto out;
+		}
 		break;
 	case KEY_DSA_CERT:
+		{
+		BIGNUM *priv_key = NULL;
+
 		if (key->cert == NULL || sshbuf_len(key->cert->certblob) == 0) {
 			r = SSH_ERR_INVALID_ARGUMENT;
 			goto out;
 		}
+		DSA_get0_key(key->dsa, NULL, &priv_key);
 		if ((r = sshbuf_put_stringb(b, key->cert->certblob)) != 0 ||
-		    (r = sshbuf_put_bignum2(b, key->dsa->priv_key)) != 0)
+		    (r = sshbuf_put_bignum2(b, priv_key)) != 0)
 			goto out;
+		}
 		break;
 # ifdef OPENSSL_HAS_ECC
 	case KEY_ECDSA:
@@ -2929,18 +3015,32 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
 			r = SSH_ERR_ALLOC_FAIL;
 			goto out;
 		}
-		if ((r = sshbuf_get_bignum2(buf, k->dsa->p)) != 0 ||
-		    (r = sshbuf_get_bignum2(buf, k->dsa->q)) != 0 ||
-		    (r = sshbuf_get_bignum2(buf, k->dsa->g)) != 0 ||
-		    (r = sshbuf_get_bignum2(buf, k->dsa->pub_key)) != 0 ||
-		    (r = sshbuf_get_bignum2(buf, k->dsa->priv_key)) != 0)
+		{
+		BIGNUM *p = NULL, *q = NULL, *g = NULL;
+		BIGNUM *pub_key = NULL, *priv_key = NULL;
+
+		DSA_get0_pqg(k->dsa, &p, &q, &g);
+		DSA_get0_key(k->dsa, &pub_key, &priv_key);
+
+		if ((r = sshbuf_get_bignum2(buf, p)) != 0 ||
+		    (r = sshbuf_get_bignum2(buf, q)) != 0 ||
+		    (r = sshbuf_get_bignum2(buf, g)) != 0 ||
+		    (r = sshbuf_get_bignum2(buf, pub_key)) != 0 ||
+		    (r = sshbuf_get_bignum2(buf, priv_key)) != 0)
 			goto out;
+		}
 		break;
 	case KEY_DSA_CERT:
+		{
+		BIGNUM *priv_key = NULL;
+
 		if ((r = sshkey_froms(buf, &k)) != 0 ||
-		    (r = sshkey_add_private(k)) != 0 ||
-		    (r = sshbuf_get_bignum2(buf, k->dsa->priv_key)) != 0)
+		    (r = sshkey_add_private(k)) != 0)
 			goto out;
+		DSA_get0_key(k->dsa, NULL, &priv_key);
+		if ((r = sshbuf_get_bignum2(buf, priv_key)) != 0)
+			goto out;
+		}
 		break;
 # ifdef OPENSSL_HAS_ECC
 	case KEY_ECDSA:
-- 
1.8.4

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Reply via email to