Behavior fixes for API compatability (#86)

* Explicitly set outl for zero inl

* Return null check to internal function

* Restore null checks for internal block ciphers and correct SAL annotations

* Support larger/smaller buffers for key exchange

* Return sufficient buffer size for ECDSA size

* Fix returns for ECDH size request and bump version

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.13.0)
 
 project(SymCrypt-OpenSSL
-    VERSION 1.5.0
+    VERSION 1.5.1
     DESCRIPTION "The SymCrypt engine for OpenSSL (SCOSSL)"
     HOMEPAGE_URL "https://github.com/microsoft/SymCrypt-OpenSSL")
 

ScosslCommon/src/scossl_ecc.c

@@ -515,11 +515,11 @@ cleanup:
 }
 
 // Return the max length of the DER encoded signature
-// 2 * (private key length) + 7 DER encoding header bytes
+// 2 * (private key length) + DER encoding header bytes
 _Use_decl_annotations_
 SIZE_T scossl_ecdsa_size(PCSYMCRYPT_ECURVE curve)
 {
-    return 2*SymCryptEcurveSizeofScalarMultiplier(curve) + 7;
+    return 2*SymCryptEcurveSizeofScalarMultiplier(curve) + 8;
 }
 
 _Use_decl_annotations_

SymCryptProvider/src/ciphers/p_scossl_aes.c

@@ -428,6 +428,7 @@ static SCOSSL_STATUS p_scossl_aes_generic_stream_update(_Inout_ SCOSSL_AES_CTX *
 {
     if (inl == 0)
     {
+        *outl = 0;
         return SCOSSL_SUCCESS;
     }
 
@@ -760,7 +761,7 @@ static SCOSSL_STATUS p_scossl_aes_generic_set_ctx_params(_Inout_ SCOSSL_AES_CTX
 }
 
 static SCOSSL_STATUS scossl_aes_cbc_cipher(_Inout_ SCOSSL_AES_CTX *ctx,
-                                           _Out_writes_bytes_opt_(*outl) unsigned char *out, _Out_ size_t *outl, size_t outsize,
+                                           _Out_writes_bytes_opt_(*outl) unsigned char *out, _Out_opt_ size_t *outl, size_t outsize,
                                            _In_reads_bytes_(inl) const unsigned char *in, size_t inl)
 {
     if (outsize < inl)
@@ -787,7 +788,7 @@ static SCOSSL_STATUS scossl_aes_cbc_cipher(_Inout_ SCOSSL_AES_CTX *ctx,
 }
 
 static SCOSSL_STATUS scossl_aes_ecb_cipher(_Inout_ SCOSSL_AES_CTX *ctx,
-                                           _Out_writes_bytes_opt_(*outl) unsigned char *out, _Out_ size_t *outl, size_t outsize,
+                                           _Out_writes_bytes_opt_(*outl) unsigned char *out, _Out_opt_ size_t *outl, size_t outsize,
                                            _In_reads_bytes_(inl) const unsigned char *in, size_t inl)
 {
     if (outsize < inl)
@@ -851,7 +852,7 @@ static SCOSSL_STATUS p_scossl_aes_cfb_cipher_internal(_Inout_ SCOSSL_AES_CTX *ct
 // previous call, the remaining space in the buffer is filled and
 // encrypted/decrypted with the previous chaining value before continuing.
 static SCOSSL_STATUS scossl_aes_cfb_cipher(_Inout_ SCOSSL_AES_CTX *ctx,
-                                           _Out_writes_bytes_opt_(*outl) unsigned char *out, _Out_ size_t *outl, size_t outsize,
+                                           _Out_writes_bytes_opt_(*outl) unsigned char *out, _Out_opt_ size_t *outl, size_t outsize,
                                            _In_reads_bytes_(inl) const unsigned char *in, size_t inl)
 {
     BYTE pbPartialBufOut[SYMCRYPT_AES_BLOCK_SIZE];

SymCryptProvider/src/ciphers/p_scossl_aes_xts.c

@@ -123,7 +123,7 @@ static SCOSSL_STATUS p_scossl_aes_xts_cipher(SCOSSL_AES_XTS_CTX *ctx,
                                              _Out_writes_bytes_(*outl) unsigned char *out, _Out_ size_t *outl, size_t outsize,
                                              _In_reads_bytes_(inl) const unsigned char *in, size_t inl)
 {
-    if( inl < SYMCRYPT_AES_BLOCK_SIZE )
+    if (inl < SYMCRYPT_AES_BLOCK_SIZE)
     {
         ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_INPUT_LENGTH);
         return SCOSSL_FAILURE;

SymCryptProvider/src/keyexch/p_scossl_dh.c

@@ -252,13 +252,19 @@ static SCOSSL_STATUS p_scossl_dh_plain_derive(_In_ SCOSSL_DH_CTX *ctx,
 
     if (secret != NULL)
     {
+        if (outlen < cbAgreedSecret)
+        {
+            ERR_raise(ERR_LIB_PROV, PROV_R_OUTPUT_BUFFER_TOO_SMALL);
+            return SCOSSL_FAILURE;
+        }
+
         scError = SymCryptDhSecretAgreement(
             ctx->provKey->keyCtx->dlkey,
             ctx->peerProvKey->keyCtx->dlkey,
             SYMCRYPT_NUMBER_FORMAT_MSB_FIRST,
             0,
             secret,
-            outlen);
+            cbAgreedSecret);
         if (scError != SYMCRYPT_NO_ERROR)
         {
             ERR_raise(ERR_LIB_PROV, ERR_R_INTERNAL_ERROR);

SymCryptProvider/src/keyexch/p_scossl_ecdh.c

@@ -97,6 +97,10 @@ static SCOSSL_STATUS p_scossl_ecdh_derive(_In_ SCOSSL_ECDH_CTX *ctx,
                                           _Out_writes_bytes_opt_(*secretlen) unsigned char *secret, _Out_ size_t *secretlen,
                                           size_t outlen)
 {
+    PBYTE pbSecret = secret;
+    PBYTE pbSecretBuf = NULL;
+    SIZE_T cbSecretBuf = 0;
+    SCOSSL_STATUS ret = SCOSSL_FAILURE;
     SYMCRYPT_ERROR scError = SYMCRYPT_NO_ERROR;
     SYMCRYPT_NUMBER_FORMAT numberFormat = ctx->keyCtx->isX25519 ? SYMCRYPT_NUMBER_FORMAT_LSB_FIRST : SYMCRYPT_NUMBER_FORMAT_MSB_FIRST;
 
@@ -111,26 +115,53 @@ static SCOSSL_STATUS p_scossl_ecdh_derive(_In_ SCOSSL_ECDH_CTX *ctx,
         return SCOSSL_FAILURE;
     }
 
-    *secretlen = SymCryptEckeySizeofPublicKey(ctx->keyCtx->key, SYMCRYPT_ECPOINT_FORMAT_X);
+    cbSecretBuf = SymCryptEckeySizeofPublicKey(ctx->keyCtx->key, SYMCRYPT_ECPOINT_FORMAT_X);
     if (secret == NULL)
     {
+        *secretlen = cbSecretBuf;
         return SCOSSL_SUCCESS;
     }
 
+    if (outlen < cbSecretBuf)
+    {
+       if ((pbSecretBuf = OPENSSL_secure_malloc(cbSecretBuf)) == NULL)
+        {
+            ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
+            goto cleanup;
+        }
+
+        pbSecret = pbSecretBuf;
+    }
+
     scError = SymCryptEcDhSecretAgreement(
         ctx->keyCtx->key,
         ctx->peerKeyCtx->key,
         numberFormat,
         0,
-        secret,
-        outlen);
+        pbSecret,
+        cbSecretBuf);
     if (scError != SYMCRYPT_NO_ERROR)
     {
         ERR_raise(ERR_LIB_PROV, ERR_R_INTERNAL_ERROR);
-        return SCOSSL_FAILURE;
+        goto cleanup;
     }
 
-    return SCOSSL_SUCCESS;
+    if (outlen < cbSecretBuf)
+    {
+        memcpy(secret, pbSecretBuf, outlen);
+        *secretlen = outlen;
+    }
+    else
+    {
+        *secretlen = cbSecretBuf;
+    }
+
+    ret = SCOSSL_SUCCESS;
+
+cleanup:
+    OPENSSL_secure_clear_free(pbSecretBuf, cbSecretBuf);
+
+    return ret;
 }
 
 // This implementation currently does not accept any parameters