Merge pull request #10 from microsoft/spnofzin/dhdsa

Code quality: DH and DSA

SymCryptEngine/src/sc_ossl_dh.c

@@ -16,8 +16,7 @@ typedef int (*PFN_DH_meth_bn_mod_exp)(const DH* dh, BIGNUM* r,
 typedef int (*PFN_DH_meth_init)(DH* dh);
 typedef int (*PFN_DH_meth_finish)(DH* dh);
 
-
-int sc_ossl_dh_generate_key(DH* dh)
+SCOSSL_STATUS sc_ossl_dh_generate_key(_Inout_ DH* dh)
 {
     const DH_METHOD* ossl_dh_meth = DH_OpenSSL();
 
@@ -29,7 +28,7 @@ int sc_ossl_dh_generate_key(DH* dh)
     return pfn_dh_meth_generate_key(dh);
 }
 
-int sc_ossl_dh_compute_key(unsigned char* key, const BIGNUM* pub_key, DH* dh)
+SCOSSL_RETURNLENGTH sc_ossl_dh_compute_key(_Out_writes_bytes_(DH_size(dh)) unsigned char* key, _In_ const BIGNUM* pub_key, _In_ DH* dh)
 {
     const DH_METHOD* ossl_dh_meth = DH_OpenSSL();
 
@@ -41,10 +40,9 @@ int sc_ossl_dh_compute_key(unsigned char* key, const BIGNUM* pub_key, DH* dh)
     return pfn_dh_meth_compute_key(key, pub_key, dh);
 }
 
-
-int sc_ossl_dh_bn_mod_exp(const DH* dh, BIGNUM* r,
-    const BIGNUM* a, const BIGNUM* p,
-    const BIGNUM* m, BN_CTX* ctx, BN_MONT_CTX* m_ctx)
+SCOSSL_STATUS sc_ossl_dh_bn_mod_exp(_In_ const DH* dh, _Out_ BIGNUM* r,
+    _In_ const BIGNUM* a, _In_ const BIGNUM* p,
+    _In_ const BIGNUM* m, _In_ BN_CTX* ctx, _In_ BN_MONT_CTX* m_ctx)
 {
     const DH_METHOD* ossl_dh_meth = DH_OpenSSL();
 
@@ -56,8 +54,7 @@ int sc_ossl_dh_bn_mod_exp(const DH* dh, BIGNUM* r,
     return pfn_dh_meth_bm_mod_exp(dh, r, a, p, m, ctx, m_ctx);
 }
 
-
-int sc_ossl_dh_init(DH* dh)
+SCOSSL_STATUS sc_ossl_dh_init(_Inout_ DH* dh)
 {
     const DH_METHOD* ossl_dh_meth = DH_OpenSSL();
 
@@ -69,8 +66,7 @@ int sc_ossl_dh_init(DH* dh)
     return pfn_dh_meth_init(dh);
 }
 
-
-int sc_ossl_dh_finish(DH* dh)
+SCOSSL_STATUS sc_ossl_dh_finish(_Inout_ DH* dh)
 {
     const DH_METHOD* ossl_dh_meth = DH_OpenSSL();
 

SymCryptEngine/src/sc_ossl_dh.h

@@ -3,23 +3,38 @@
 //
 
 #include "sc_ossl.h"
+#include "sc_ossl_helpers.h"
 #include <openssl/dh.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-int sc_ossl_dh_generate_key(DH* dh);
+// Generates public and private DH values.
+// Expects shared parameters dh->p and dh->g to be set.
+// Generates a random private DH key unless dh->priv_key set, and computes corresponding
+// public value dh->pub_key.
+// Returns 1 on success, 0 otherwise
+SCOSSL_STATUS sc_ossl_dh_generate_key(_Inout_ DH* dh);
 
-int sc_ossl_dh_compute_key(unsigned char* key, const BIGNUM* pub_key, DH* dh);
+// Computes the shared secret from the private DH value in dh and the other party's public
+// value in pub_key and stores it in key. key must point to DH_size(dh) bytes of memory.
+// Returns size of shared secret on success, or -1 on error.
+SCOSSL_RETURNLENGTH sc_ossl_dh_compute_key(_Out_writes_bytes_(DH_size(dh)) unsigned char* key, _In_ const BIGNUM* pub_key, _In_ DH* dh);
 
-int sc_ossl_dh_bn_mod_exp(const DH* dh, BIGNUM* r,
-    const BIGNUM* a, const BIGNUM* p,
-    const BIGNUM* m, BN_CTX* ctx, BN_MONT_CTX* m_ctx);
+// Computes r = a ^ p mod m
+// Returns 1 on success, or 0 on error
+SCOSSL_STATUS sc_ossl_dh_bn_mod_exp(_In_ const DH* dh, _Out_ BIGNUM* r,
+    _In_ const BIGNUM* a, _In_ const BIGNUM* p,
+    _In_ const BIGNUM* m, _In_ BN_CTX* ctx, _In_ BN_MONT_CTX* m_ctx);
 
-int sc_ossl_dh_init(DH* dh);
+// Initializes a new DH instance.
+// Returns 1 on success, or 0 on error
+SCOSSL_STATUS sc_ossl_dh_init(_Inout_ DH* dh);
 
-int sc_ossl_dh_finish(DH* dh);
+// Destroys instance of DH object. The memory for dh is not freed by this function.
+// Returns 1 on success, or 0 on error
+SCOSSL_STATUS sc_ossl_dh_finish(_Inout_ DH* dh);
 
 #ifdef __cplusplus
 }

SymCryptEngine/src/sc_ossl_dsa.c

@@ -14,7 +14,8 @@ typedef int (*PFN_DSA_meth_verify) (const unsigned char* dgst, int dgst_len, DSA
 typedef int (*PFN_DSA_meth_init)(DSA* dsa);
 typedef int (*PFN_DSA_meth_finish)(DSA* dsa);
 
-DSA_SIG* sc_ossl_dsa_sign(const unsigned char* dgst, int dlen, DSA* dsa)
+_Success_(return != NULL)
+DSA_SIG* sc_ossl_dsa_sign(_In_reads_bytes_(dlen) const unsigned char* dgst, int dlen, _In_ DSA* dsa)
 {
     const DSA_METHOD* ossl_dsa_meth = DSA_OpenSSL();
     PFN_DSA_meth_sign pfn_dsa_sign = DSA_meth_get_sign(ossl_dsa_meth);
@@ -25,8 +26,8 @@ DSA_SIG* sc_ossl_dsa_sign(const unsigned char* dgst, int dlen, DSA* dsa)
     return pfn_dsa_sign(dgst, dlen, dsa);
 }
 
-int sc_ossl_dsa_sign_setup(DSA* dsa, BN_CTX* ctx_in,
-    BIGNUM** kinvp, BIGNUM** rp)
+SCOSSL_STATUS sc_ossl_dsa_sign_setup(_In_ DSA* dsa, _In_ BN_CTX* ctx_in,
+    _Out_ BIGNUM** kinvp, _Out_ BIGNUM** rp)
 {
     const DSA_METHOD* ossl_dsa_meth = DSA_OpenSSL();
     PFN_DSA_meth_sign_setup pfn_dsa_sign_setup = DSA_meth_get_sign_setup(ossl_dsa_meth);
@@ -37,8 +38,8 @@ int sc_ossl_dsa_sign_setup(DSA* dsa, BN_CTX* ctx_in,
     return pfn_dsa_sign_setup(dsa, ctx_in, kinvp, rp);
 }
 
-int sc_ossl_dsa_verify(const unsigned char* dgst, int dgst_len,
-    DSA_SIG* sig, DSA* dsa)
+SCOSSL_STATUS sc_ossl_dsa_verify(_In_reads_bytes_(dgst_len) const unsigned char* dgst, int dgst_len,
+    _In_ DSA_SIG* sig, _In_ DSA* dsa)
 {
     const DSA_METHOD* ossl_dsa_meth = DSA_OpenSSL();
     PFN_DSA_meth_verify pfn_dsa_verify = DSA_meth_get_verify(ossl_dsa_meth);
@@ -49,7 +50,7 @@ int sc_ossl_dsa_verify(const unsigned char* dgst, int dgst_len,
     return pfn_dsa_verify(dgst, dgst_len, sig, dsa);
 }
 
-int sc_ossl_dsa_init(DSA* dsa)
+SCOSSL_STATUS sc_ossl_dsa_init(_Inout_ DSA* dsa)
 {
     const DSA_METHOD* ossl_dsa_meth = DSA_OpenSSL();
     PFN_DSA_meth_init pfn_dsa_init = DSA_meth_get_init(ossl_dsa_meth);
@@ -60,8 +61,7 @@ int sc_ossl_dsa_init(DSA* dsa)
     return pfn_dsa_init(dsa);
 }
 
-
-int sc_ossl_dsa_finish(DSA* dsa)
+SCOSSL_STATUS sc_ossl_dsa_finish(_Inout_ DSA* dsa)
 {
     const DSA_METHOD* ossl_dsa_meth = DSA_OpenSSL();
     PFN_DSA_meth_finish pfn_dsa_finish = DSA_meth_get_finish(ossl_dsa_meth);

SymCryptEngine/src/sc_ossl_dsa.h

@@ -3,6 +3,7 @@
 //
 
 #include "sc_ossl.h"
+#include "sc_ossl_helpers.h"
 #include <openssl/dsa.h>
 
 #ifdef __cplusplus
@@ -10,15 +11,28 @@ extern "C" {
 #endif
 
 
-DSA_SIG* sc_ossl_dsa_sign(const unsigned char* dgst, int dlen, DSA* dsa);
+// Computes a digital signature on the dlen byte message digest dgst using the private key dsa
+// and returns it in a newly allocated DSA_SIG structure.
+// Returns the signature on success, or NULL on error.
+_Success_(return != NULL)
+DSA_SIG* sc_ossl_dsa_sign(_In_reads_bytes_(dlen) const unsigned char* dgst, int dlen, _In_ DSA* dsa);
 
-int sc_ossl_dsa_sign_setup(DSA* dsa, BN_CTX* ctx_in, BIGNUM** kinvp, BIGNUM** rp);
+// Precalculates the DSA signature values k^-1 and r.
+// Returns 1 on success, or 0 on error.
+SCOSSL_STATUS sc_ossl_dsa_sign_setup(_In_ DSA* dsa, _In_ BN_CTX* ctx_in, _Out_ BIGNUM** kinvp, _Out_ BIGNUM** rp);
 
-int sc_ossl_dsa_verify(const unsigned char* dgst, int dgst_len, DSA_SIG* sig, DSA* dsa);
+// Verifies that the signature sig matches a given message digest dgst of size dgst_len.
+// dsa is the signer's public key.
+// Returns 1 for a valid signature, 0 for an incorrect signature, and -1 on error.
+SCOSSL_STATUS sc_ossl_dsa_verify(_In_reads_bytes_(dgst_len) const unsigned char* dgst, int dgst_len, _In_ DSA_SIG* sig, _In_ DSA* dsa);
 
-int sc_ossl_dsa_init(DSA* dsa);
+// Initializes a new DSA instance.
+// Returns 1 on success, or 0 on error
+SCOSSL_STATUS sc_ossl_dsa_init(_Inout_ DSA* dsa);
 
-int sc_ossl_dsa_finish(DSA* dsa);
+// Destroys instance of DSA object. The memory for dsa is not freed by this function.
+// Returns 1 on success, or 0 on error
+SCOSSL_STATUS sc_ossl_dsa_finish(_Inout_ DSA* dsa);
 
 #ifdef __cplusplus
 }