[openssl-commits] [openssl] master update

[bernd.edlinger at hotmail.de]

The branch master has been updated
       via  7966101e20abdcf1da4815dc41e47f52cce02f0f (commit)
      from  b50951d36c329b07e1c92d41cc02c9085fb567d7 (commit)


- Log -----------------------------------------------------------------
commit 7966101e20abdcf1da4815dc41e47f52cce02f0f
Author: David Benjamin <davidben at google.com>
Date:   Mon Sep 18 11:58:24 2017 -0400

    Allow DH_set0_key with only private key.
    
    The pub_key field for DH isn't actually used in DH_compute_key at all.
    (Note the peer public key is passed in as as BIGNUM.) It's mostly there
    so the caller may extract it from DH_generate_key. It doesn't
    particularly need to be present if filling in a DH from external
    parameters.
    
    The check in DH_set0_key conflicts with adding OpenSSL 1.1.0 to Node.
    Their public API is a thin wrapper over the old OpenSSL one:
    https://nodejs.org/api/crypto.html#crypto_class_diffiehellman
    
    They have separate setPrivateKey and setPublicKey methods, so the public
    key may be set last or not at all. In 1.0.2, either worked fine since
    operations on DH objects generally didn't use the public key.  (Like
    with OpenSSL, Node's setPublicKey method is also largely a no-op, but so
    it goes.) In 1.1.0, DH_set0_key prevents create a private-key-only DH
    object.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/4384)

-----------------------------------------------------------------------

Summary of changes:
 crypto/dh/dh_lib.c       |  7 -------
 doc/man3/DH_get0_pqg.pod | 12 +++++-------
 test/dhtest.c            | 28 +++++++++++++++++++++++-----
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/crypto/dh/dh_lib.c b/crypto/dh/dh_lib.c
index 4530ce7..746a1b4 100644
--- a/crypto/dh/dh_lib.c
+++ b/crypto/dh/dh_lib.c
@@ -231,13 +231,6 @@ void DH_get0_key(const DH *dh, const BIGNUM **pub_key, const BIGNUM **priv_key)
 
 int DH_set0_key(DH *dh, BIGNUM *pub_key, BIGNUM *priv_key)
 {
-    /* If the field pub_key in dh is NULL, the corresponding input
-     * parameters MUST be non-NULL.  The priv_key field may
-     * be left NULL.
-     */
-    if (dh->pub_key == NULL && pub_key == NULL)
-        return 0;
-
     if (pub_key != NULL) {
         BN_free(dh->pub_key);
         dh->pub_key = pub_key;
diff --git a/doc/man3/DH_get0_pqg.pod b/doc/man3/DH_get0_pqg.pod
index 7dd875b..ec476a7 100644
--- a/doc/man3/DH_get0_pqg.pod
+++ b/doc/man3/DH_get0_pqg.pod
@@ -48,13 +48,11 @@ been set yet, although if the private key has been set then the public key must
 be. The values point to the internal representation of the public key and
 private key values. This memory should not be freed directly.
 
-The public and private key values can be set using DH_set0_key(). The public
-key must be non-NULL the first time this function is called on a given DH
-object. The private key may be NULL.  On subsequent calls, either may be NULL,
-which means the corresponding DH field is left untouched. As for DH_set0_pqg()
-this function transfers the memory management of the key values to the DH
-object, and therefore they should not be freed directly after this function has
-been called.
+The public and private key values can be set using DH_set0_key(). Either
+parameter may be NULL, which means the corresponding DH field is left
+untouched. As with DH_set0_pqg() this function transfers the memory management
+of the key values to the DH object, and therefore they should not be freed
+directly after this function has been called.
 
 DH_set_flags() sets the flags in the B<flags> parameter on the DH object.
 Multiple flags can be passed in one go (bitwise ORed together). Any flags that
diff --git a/test/dhtest.c b/test/dhtest.c
index 7500f37..96d7027 100644
--- a/test/dhtest.c
+++ b/test/dhtest.c
@@ -29,12 +29,14 @@ static int dh_test(void)
     BN_GENCB *_cb = NULL;
     DH *a = NULL;
     DH *b = NULL;
+    DH *c = NULL;
     const BIGNUM *ap = NULL, *ag = NULL, *apub_key = NULL;
-    const BIGNUM *bpub_key = NULL;
-    BIGNUM *bp = NULL, *bg = NULL;
+    const BIGNUM *bpub_key = NULL, *bpriv_key = NULL;
+    BIGNUM *bp = NULL, *bg = NULL, *cpriv_key = NULL;
     unsigned char *abuf = NULL;
     unsigned char *bbuf = NULL;
-    int i, alen, blen, aout, bout;
+    unsigned char *cbuf = NULL;
+    int i, alen, blen, clen, aout, bout, cout;
     int ret = 0;
 
     if (!TEST_ptr(_cb = BN_GENCB_new()))
@@ -70,7 +72,14 @@ static int dh_test(void)
 
     if (!DH_generate_key(b))
         goto err;
-    DH_get0_key(b, &bpub_key, NULL);
+    DH_get0_key(b, &bpub_key, &bpriv_key);
+
+    /* Also test with a private-key-only copy of |b|. */
+    if (!TEST_ptr(c = DHparams_dup(b))
+            || !TEST_ptr(cpriv_key = BN_dup(bpriv_key))
+            || !TEST_true(DH_set0_key(c, NULL, cpriv_key)))
+        goto err;
+    cpriv_key = NULL;
 
     alen = DH_size(a);
     if (!TEST_ptr(abuf = OPENSSL_malloc(alen))
@@ -82,8 +91,14 @@ static int dh_test(void)
             || !TEST_true((bout = DH_compute_key(bbuf, apub_key, b)) != -1))
         goto err;
 
+    clen = DH_size(c);
+    if (!TEST_ptr(cbuf = OPENSSL_malloc(clen))
+            || !TEST_true((cout = DH_compute_key(cbuf, apub_key, c)) != -1))
+        goto err;
+
     if (!TEST_true(aout >= 4)
-            || !TEST_mem_eq(abuf, aout, bbuf, bout))
+            || !TEST_mem_eq(abuf, aout, bbuf, bout)
+            || !TEST_mem_eq(abuf, aout, cbuf, cout))
         goto err;
 
     ret = 1;
@@ -91,10 +106,13 @@ static int dh_test(void)
  err:
     OPENSSL_free(abuf);
     OPENSSL_free(bbuf);
+    OPENSSL_free(cbuf);
     DH_free(b);
     DH_free(a);
+    DH_free(c);
     BN_free(bp);
     BN_free(bg);
+    BN_free(cpriv_key);
     BN_GENCB_free(_cb);
     return ret;
 }