Comments from code review

3 files changed, 73 insertions, 7 deletions

diff --git a/src/lib/ffi/ffi.h b/src/lib/ffi/ffi.h
index cfb7b853e..296d64ef2 100644
--- a/src/lib/ffi/ffi.h
+++ b/src/lib/ffi/ffi.h
@@ -702,8 +702,56 @@ BOTAN_PUBLIC_API(2,0) int botan_privkey_create_ecdsa(botan_privkey_t* key, botan
 BOTAN_PUBLIC_API(2,0) int botan_privkey_create_ecdh(botan_privkey_t* key, botan_rng_t rng, const char* params);
 BOTAN_PUBLIC_API(2,0) int botan_privkey_create_mceliece(botan_privkey_t* key, botan_rng_t rng, size_t n, size_t t);
 BOTAN_PUBLIC_API(2,0) int botan_privkey_create_dh(botan_privkey_t* key, botan_rng_t rng, const char* param);
-BOTAN_PUBLIC_API(2,0) int botan_privkey_create_dsa(botan_privkey_t* key, botan_rng_t rng, size_t pbits, size_t qbits);
-BOTAN_PUBLIC_API(2,0) int botan_privkey_create_elgamal(botan_privkey_t* key, botan_rng_t rng, size_t pbits);
+
+
+/*
+ * Generates DSA key pair. Gives to a caller control over key length
+ * and order of a subgroup 'q'.
+ *
+ * @param   key   handler to the resulting key
+ * @param   rng   initialized PRNG
+ * @param   pbits length of the key in bits. Must be between in range (1024, 3072)
+ *          and multiple of 64. Bit size of the prime 'p'
+ * @param   qbits order of the subgroup. Must be in range (160, 256) and multiple
+ *          of 8
+ *
+ * @returns BOTAN_FFI_SUCCESS Success, `key' initialized with DSA key
+ * @returns BOTAN_FFI_ERROR_NULL_POINTER  either `key' or `rng' is NULL
+ * @returns BOTAN_FFI_ERROR_BAD_PARAMETER unexpected value for either `pbits' or
+ *          `qbits'
+ * @returns BOTAN_FFI_ERROR_NOT_IMPLEMENTED functionality not implemented
+ *
+-------------------------------------------------------------------------------- */
+BOTAN_PUBLIC_API(2,5) int botan_privkey_create_dsa(
+                                botan_privkey_t* key,
+                                botan_rng_t rng,
+                                size_t pbits,
+                                size_t qbits);
+
+/*
+ * Generates ElGamal key pair. Caller has a control over key length
+ * and order of a subgroup 'q'. Function is able to use two types of
+ * primes:
+ *    * if pbits-1 == qbits then safe primes are used for key generation
+ *    * otherwise generation uses group of prime order
+ *
+ * @param   key   handler to the resulting key
+ * @param   rng   initialized PRNG
+ * @param   pbits length of the key in bits. Must be at least 1024
+ * @param   qbits order of the subgroup. Must be at least 160
+ *
+ * @returns BOTAN_FFI_SUCCESS Success, `key' initialized with DSA key
+ * @returns BOTAN_FFI_ERROR_NULL_POINTER  either `key' or `rng' is NULL
+ * @returns BOTAN_FFI_ERROR_BAD_PARAMETER unexpected value for either `pbits' or
+ *          `qbits'
+ * @returns BOTAN_FFI_ERROR_NOT_IMPLEMENTED functionality not implemented
+ *
+-------------------------------------------------------------------------------- */
+BOTAN_PUBLIC_API(2,5) int botan_privkey_create_elgamal(
+                            botan_privkey_t* key,
+                            botan_rng_t rng,
+                            size_t pbits,
+                            size_t qbits);
 
 /*
 * Input currently assumed to be PKCS #8 structure;
diff --git a/src/lib/ffi/ffi_pkey_algs.cpp b/src/lib/ffi/ffi_pkey_algs.cpp
index 3b4bde7eb..a20d7de40 100644
--- a/src/lib/ffi/ffi_pkey_algs.cpp
+++ b/src/lib/ffi/ffi_pkey_algs.cpp
@@ -337,9 +337,15 @@ int botan_privkey_create_dsa(botan_privkey_t* key, botan_rng_t rng_obj, size_t p
    {
 #if defined(BOTAN_HAS_DSA)
 
-    if(rng_obj == nullptr)
+    if ((rng_obj == nullptr) || (key == nullptr))
       return BOTAN_FFI_ERROR_NULL_POINTER;
 
+    if ((pbits % 64) || (qbits % 8) ||
+        (pbits < 1024) || (pbits > 3072) ||
+        (qbits < 160) || (qbits > 256)) {
+      return BOTAN_FFI_ERROR_BAD_PARAMETER;
+    }
+
     return ffi_guard_thunk(BOTAN_CURRENT_FUNCTION, [=]() -> int {
       Botan::RandomNumberGenerator& rng = safe_get(rng_obj);
       Botan::DL_Group group(rng, Botan::DL_Group::Prime_Subgroup, pbits, qbits);
@@ -459,16 +465,27 @@ int botan_privkey_load_ecdsa(botan_privkey_t* key,
    }
 
 /* ElGamal specific operations */
-int botan_privkey_create_elgamal(botan_privkey_t* key, botan_rng_t rng_obj, size_t pbits)
+int botan_privkey_create_elgamal(botan_privkey_t* key,
+                                 botan_rng_t rng_obj,
+                                 size_t pbits,
+                                 size_t qbits)
    {
 #if defined(BOTAN_HAS_ELGAMAL)
 
-    if(rng_obj == nullptr)
+    if ((rng_obj == nullptr) || (key == nullptr))
       return BOTAN_FFI_ERROR_NULL_POINTER;
 
+    if ((pbits < 1024) || (qbits<160)) {
+      return BOTAN_FFI_ERROR_BAD_PARAMETER;
+    }
+
+    Botan::DL_Group::PrimeType prime_type = ((pbits-1) == qbits)
+      ? Botan::DL_Group::Strong
+      : Botan::DL_Group::Prime_Subgroup;
+
     return ffi_guard_thunk(BOTAN_CURRENT_FUNCTION, [=]() -> int {
       Botan::RandomNumberGenerator& rng = safe_get(rng_obj);
-      Botan::DL_Group group(rng, Botan::DL_Group::Strong, pbits);
+      Botan::DL_Group group(rng, prime_type, pbits, qbits);
       *key = new botan_privkey_struct(new Botan::ElGamal_PrivateKey(rng, group));
       return BOTAN_FFI_SUCCESS;
     });
diff --git a/src/tests/test_ffi.cpp b/src/tests/test_ffi.cpp
index af6f57b80..00deabb06 100644
--- a/src/tests/test_ffi.cpp
+++ b/src/tests/test_ffi.cpp
@@ -1,6 +1,7 @@
 /*
 * (C) 2015 Jack Lloyd
 * (C) 2016 René Korthaus
+* (C) 2018 Ribose Inc, Krzysztof Kwiatkowski
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -1956,7 +1957,7 @@ class FFI_Unit_Tests final : public Test
                do_elgamal_test(priv, rng, result);
             }
 
-         if(TEST_FFI_OK(botan_privkey_create_elgamal, (&priv, rng, 2048)))
+         if(TEST_FFI_OK(botan_privkey_create_elgamal, (&priv, rng, 1024, 160)))
             {
                do_elgamal_test(priv, rng, result);
             }