rtmpdh: Don't use the OpenSSL DH struct

Message ID 20161022110031.21458-1-martin@martin.st
State Committed
Headers show

Commit Message

Martin Storsjö Oct. 22, 2016, 11 a.m.
Instead use our own struct, which we already use when using
gcrypt and gnutls.

In OpenSSL 1.1, the DH struct has been made opaque.
---
Changed bn_modexp to a static function, for more proper error handling.
---
 libavformat/rtmpdh.c | 95 +++++++++++++++++++++++++---------------------------
 libavformat/rtmpdh.h | 14 ++++----
 2 files changed, 52 insertions(+), 57 deletions(-)

Comments

Luca Barbato Oct. 22, 2016, 1:11 p.m. | #1
On 22/10/2016 13:00, Martin Storsjö wrote:
> Changed bn_modexp to a static function, for more proper error handling.

Fine for me, thank you =)
Diego Biurrun Oct. 22, 2016, 1:50 p.m. | #2
On Sat, Oct 22, 2016 at 02:00:31PM +0300, Martin Storsjö wrote:
> --- a/libavformat/rtmpdh.c
> +++ b/libavformat/rtmpdh.c
> @@ -116,13 +119,42 @@
> +#define bn_hex2bn(bn, buf, ret)     ret = BN_hex2bn(&bn, buf)
> +#define bn_random(bn, num_bits)     BN_rand(bn, num_bits, 0, 0)
> +static int bn_modexp(FFBigNum bn, FFBigNum y, FFBigNum q, FFBigNum p)
> +{
> +    BN_CTX *ctx = BN_CTX_new();
> +    if (!ctx)
> +        return AVERROR(ENOMEM);
> +    if (!BN_mod_exp(bn, y, q, p, ctx)) {
> +            BN_CTX_free(ctx);
> +            return AVERROR(EINVAL);
> +    }

Indentation is off.

Diego
Martin Storsjö Oct. 22, 2016, 6:25 p.m. | #3
On Sat, 22 Oct 2016, Diego Biurrun wrote:

> On Sat, Oct 22, 2016 at 02:00:31PM +0300, Martin Storsjö wrote:
>> --- a/libavformat/rtmpdh.c
>> +++ b/libavformat/rtmpdh.c
>> @@ -116,13 +119,42 @@
>> +#define bn_hex2bn(bn, buf, ret)     ret = BN_hex2bn(&bn, buf)
>> +#define bn_random(bn, num_bits)     BN_rand(bn, num_bits, 0, 0)
>> +static int bn_modexp(FFBigNum bn, FFBigNum y, FFBigNum q, FFBigNum p)
>> +{
>> +    BN_CTX *ctx = BN_CTX_new();
>> +    if (!ctx)
>> +        return AVERROR(ENOMEM);
>> +    if (!BN_mod_exp(bn, y, q, p, ctx)) {
>> +            BN_CTX_free(ctx);
>> +            return AVERROR(EINVAL);
>> +    }
>
> Indentation is off.

Fixed locally, will push tomorrow or so, unless there are further 
comments.

// Martin

Patch

diff --git a/libavformat/rtmpdh.c b/libavformat/rtmpdh.c
index e7a83e1..644cbf2 100644
--- a/libavformat/rtmpdh.c
+++ b/libavformat/rtmpdh.c
@@ -54,7 +54,6 @@ 
     "F71C35FDAD44CFD2D74F9208BE258FF324943328F67329C0" \
     "FFFFFFFFFFFFFFFF"
 
-#if CONFIG_GMP || CONFIG_GCRYPT
 #if CONFIG_GMP
 #define bn_new(bn)                      \
     do {                                \
@@ -93,7 +92,6 @@ 
         else                                        \
             ret = 1;                                \
     } while (0)
-#define bn_modexp(bn, y, q, p)      mpz_powm(bn, y, q, p)
 #define bn_random(bn, num_bits)                       \
     do {                                              \
         int bits = num_bits;                          \
@@ -104,6 +102,11 @@ 
         }                                             \
         mpz_fdiv_r_2exp(bn, bn, num_bits);            \
     } while (0)
+static int bn_modexp(FFBigNum bn, FFBigNum y, FFBigNum q, FFBigNum p)
+{
+    mpz_powm(bn, y, q, p);
+    return 0;
+}
 #elif CONFIG_GCRYPT
 #define bn_new(bn)                  bn = gcry_mpi_new(1)
 #define bn_free(bn)                 gcry_mpi_release(bn)
@@ -116,13 +119,42 @@ 
 #define bn_bn2bin(bn, buf, len)     gcry_mpi_print(GCRYMPI_FMT_USG, buf, len, NULL, bn)
 #define bn_bin2bn(bn, buf, len)     gcry_mpi_scan(&bn, GCRYMPI_FMT_USG, buf, len, NULL)
 #define bn_hex2bn(bn, buf, ret)     ret = (gcry_mpi_scan(&bn, GCRYMPI_FMT_HEX, buf, 0, 0) == 0)
-#define bn_modexp(bn, y, q, p)      gcry_mpi_powm(bn, y, q, p)
 #define bn_random(bn, num_bits)     gcry_mpi_randomize(bn, num_bits, GCRY_WEAK_RANDOM)
+static int bn_modexp(FFBigNum bn, FFBigNum y, FFBigNum q, FFBigNum p)
+{
+    gcry_mpi_powm(bn, y, q, p);
+    return 0;
+}
+#elif CONFIG_OPENSSL
+#define bn_new(bn)                  bn = BN_new()
+#define bn_free(bn)                 BN_free(bn)
+#define bn_set_word(bn, w)          BN_set_word(bn, w)
+#define bn_cmp(a, b)                BN_cmp(a, b)
+#define bn_copy(to, from)           BN_copy(to, from)
+#define bn_sub_word(bn, w)          BN_sub_word(bn, w)
+#define bn_cmp_1(bn)                BN_cmp(bn, BN_value_one())
+#define bn_num_bytes(bn)            BN_num_bytes(bn)
+#define bn_bn2bin(bn, buf, len)     BN_bn2bin(bn, buf)
+#define bn_bin2bn(bn, buf, len)     bn = BN_bin2bn(buf, len, 0)
+#define bn_hex2bn(bn, buf, ret)     ret = BN_hex2bn(&bn, buf)
+#define bn_random(bn, num_bits)     BN_rand(bn, num_bits, 0, 0)
+static int bn_modexp(FFBigNum bn, FFBigNum y, FFBigNum q, FFBigNum p)
+{
+    BN_CTX *ctx = BN_CTX_new();
+    if (!ctx)
+        return AVERROR(ENOMEM);
+    if (!BN_mod_exp(bn, y, q, p, ctx)) {
+            BN_CTX_free(ctx);
+            return AVERROR(EINVAL);
+    }
+    BN_CTX_free(ctx);
+    return 0;
+}
 #endif
 
 #define MAX_BYTES 18000
 
-#define dh_new()                    av_malloc(sizeof(FF_DH))
+#define dh_new()                    av_mallocz(sizeof(FF_DH))
 
 static FFBigNum dh_generate_key(FF_DH *dh)
 {
@@ -143,7 +175,8 @@  static FFBigNum dh_generate_key(FF_DH *dh)
         return NULL;
     }
 
-    bn_modexp(dh->pub_key, dh->g, dh->priv_key, dh->p);
+    if (bn_modexp(dh->pub_key, dh->g, dh->priv_key, dh->p) < 0)
+        return NULL;
 
     return dh->pub_key;
 }
@@ -152,12 +185,16 @@  static int dh_compute_key(FF_DH *dh, FFBigNum pub_key_bn,
                           uint32_t secret_key_len, uint8_t *secret_key)
 {
     FFBigNum k;
+    int ret;
 
     bn_new(k);
     if (!k)
         return -1;
 
-    bn_modexp(k, pub_key_bn, dh->priv_key, dh->p);
+    if ((ret = bn_modexp(k, pub_key_bn, dh->priv_key, dh->p)) < 0) {
+        bn_free(k);
+        return ret;
+    }
     bn_bn2bin(k, secret_key, secret_key_len);
     bn_free(k);
 
@@ -175,48 +212,6 @@  void ff_dh_free(FF_DH *dh)
     bn_free(dh->priv_key);
     av_free(dh);
 }
-#elif CONFIG_OPENSSL
-#define bn_new(bn)                  bn = BN_new()
-#define bn_free(bn)                 BN_free(bn)
-#define bn_set_word(bn, w)          BN_set_word(bn, w)
-#define bn_cmp(a, b)                BN_cmp(a, b)
-#define bn_copy(to, from)           BN_copy(to, from)
-#define bn_sub_word(bn, w)          BN_sub_word(bn, w)
-#define bn_cmp_1(bn)                BN_cmp(bn, BN_value_one())
-#define bn_num_bytes(bn)            BN_num_bytes(bn)
-#define bn_bn2bin(bn, buf, len)     BN_bn2bin(bn, buf)
-#define bn_bin2bn(bn, buf, len)     bn = BN_bin2bn(buf, len, 0)
-#define bn_hex2bn(bn, buf, ret)     ret = BN_hex2bn(&bn, buf)
-#define bn_modexp(bn, y, q, p)               \
-    do {                                     \
-        BN_CTX *ctx = BN_CTX_new();          \
-        if (!ctx)                            \
-            return AVERROR(ENOMEM);          \
-        if (!BN_mod_exp(bn, y, q, p, ctx)) { \
-            BN_CTX_free(ctx);                \
-            return AVERROR(EINVAL);          \
-        }                                    \
-        BN_CTX_free(ctx);                    \
-    } while (0)
-
-#define dh_new()                                DH_new()
-#define dh_generate_key(dh)                     DH_generate_key(dh)
-
-static int dh_compute_key(FF_DH *dh, FFBigNum pub_key_bn,
-                          uint32_t secret_key_len, uint8_t *secret_key)
-{
-    if (secret_key_len < DH_size(dh))
-        return AVERROR(EINVAL);
-    return DH_compute_key(secret_key, pub_key_bn, dh);
-}
-
-void ff_dh_free(FF_DH *dh)
-{
-    if (!dh)
-        return;
-    DH_free(dh);
-}
-#endif
 
 static int dh_is_valid_public_key(FFBigNum y, FFBigNum p, FFBigNum q)
 {
@@ -245,8 +240,10 @@  static int dh_is_valid_public_key(FFBigNum y, FFBigNum p, FFBigNum q)
      * random data.
      */
     /* y must fulfill y^q mod p = 1 */
-    bn_modexp(bn, y, q, p);
+    if ((ret = bn_modexp(bn, y, q, p)) < 0)
+        goto fail;
 
+    ret = AVERROR(EINVAL);
     if (bn_cmp_1(bn))
         goto fail;
 
diff --git a/libavformat/rtmpdh.h b/libavformat/rtmpdh.h
index eb742dd..5233de0 100644
--- a/libavformat/rtmpdh.h
+++ b/libavformat/rtmpdh.h
@@ -26,7 +26,6 @@ 
 
 #include "config.h"
 
-#if CONFIG_GMP || CONFIG_GCRYPT
 #if CONFIG_GMP
 #include <gmp.h>
 
@@ -35,6 +34,12 @@  typedef mpz_ptr FFBigNum;
 #include <gcrypt.h>
 
 typedef gcry_mpi_t FFBigNum;
+
+#elif CONFIG_OPENSSL
+#include <openssl/bn.h>
+#include <openssl/dh.h>
+
+typedef BIGNUM *FFBigNum;
 #endif
 
 typedef struct FF_DH {
@@ -45,13 +50,6 @@  typedef struct FF_DH {
     long length;
 } FF_DH;
 
-#elif CONFIG_OPENSSL
-#include <openssl/bn.h>
-#include <openssl/dh.h>
-
-typedef BIGNUM *FFBigNum;
-typedef DH FF_DH;
-#endif
 
 /**
  * Initialize a Diffie-Hellmann context.