From efc888d18c49a8a3e0a1085d2d052660c9d1ff95 Mon Sep 17 00:00:00 2001 From: Marc Hoersken Date: Sat, 21 Jun 2014 15:14:07 +0200 Subject: [PATCH] wincng: Added explicit clear memory feature to WinCNG backend This re-introduces the original feature proposed during the development of the WinCNG crypto backend. It still needs to be added to libssh2 itself and probably other backends. Memory is cleared using the function SecureZeroMemory which is available on Windows systems, just like the WinCNG backend. --- configure.ac | 23 ++++++++++ src/wincng.c | 146 +++++++++++++++++++++++++++++++++-------------------------- 2 files changed, 104 insertions(+), 65 deletions(-) diff --git a/configure.ac b/configure.ac index ba4dd7a..798def9 100644 --- a/configure.ac +++ b/configure.ac @@ -97,6 +97,7 @@ AC_ARG_WITH(libz, use_libz=$withval,use_libz=auto) found_crypto=none +support_clear_memory=no # Look for OpenSSL if test "$found_crypto" = "none" && test "$use_openssl" != "no"; then @@ -150,6 +151,7 @@ if test "$ac_cv_libbcrypt" = "yes"; then LIBS="$LIBS -lcrypt32" fi found_crypto="Windows Cryptography API: Next Generation" + support_clear_memory=yes fi AM_CONDITIONAL(WINCNG, test "$ac_cv_libbcrypt" = "yes") @@ -197,6 +199,26 @@ if test "$GEX_NEW" != "no"; then AC_DEFINE(LIBSSH2_DH_GEX_NEW, 1, [Enable newer diffie-hellman-group-exchange-sha1 syntax]) fi +AC_ARG_ENABLE(clear-memory, + AC_HELP_STRING([--disable-clear-memory],[Disable clearing of memory before being freed]), + [CLEAR_MEMORY=$enableval]) +if test "$CLEAR_MEMORY" != "no"; then + if test "$support_clear_memory" = "yes"; then + AC_DEFINE(LIBSSH2_CLEAR_MEMORY, 1, [Enable clearing of memory before being freed]) + enable_clear_memory=yes + else + AC_MSG_ERROR([secure clearing/zeroing of memory is not supported by the selected crypto backend]) + enable_clear_memory=unsupported + fi +else + if test "$support_clear_memory" = "yes"; then + enable_clear_memory=no + else + AC_MSG_WARN([secure clearing/zeroing of memory is not supported by the selected crypto backend]) + enable_clear_memory=unsupported + fi +fi + dnl ************************************************************ dnl option to switch on compiler debug options dnl @@ -362,6 +384,7 @@ AC_MSG_NOTICE([summary of build options: Compiler flags: ${CFLAGS} Library types: Shared=${enable_shared}, Static=${enable_static} Crypto library: ${found_crypto} + Clear memory: $enable_clear_memory Debug build: $enable_debug Build examples: $build_examples Path to sshd: $ac_cv_path_SSHD (only for self-tests) diff --git a/src/wincng.c b/src/wincng.c index b575350..5dfca52 100644 --- a/src/wincng.c +++ b/src/wincng.c @@ -280,6 +280,20 @@ _libssh2_wincng_random(void *buf, int len) return BCRYPT_SUCCESS(ret) ? 0 : -1; } +static void +_libssh2_wincng_mfree(void *buf, int len) +{ + if (!buf) + return; + +#ifdef LIBSSH2_CLEAR_MEMORY + if (len > 0) + SecureZeroMemory(buf, len); +#endif + + free(buf); +} + /*******************************************************************/ /* @@ -322,7 +336,7 @@ _libssh2_wincng_hash_init(_libssh2_wincng_hash_ctx *ctx, pbHashObject, dwHashObject, key, keylen, 0); if (!BCRYPT_SUCCESS(ret)) { - free(pbHashObject); + _libssh2_wincng_mfree(pbHashObject, dwHashObject); return -1; } @@ -355,11 +369,11 @@ _libssh2_wincng_hash_final(_libssh2_wincng_hash_ctx *ctx, ret = BCryptFinishHash(ctx->hHash, hash, ctx->cbHash, 0); BCryptDestroyHash(ctx->hHash); + ctx->hHash = NULL; - if (ctx->pbHashObject) - free(ctx->pbHashObject); - - memset(ctx, 0, sizeof(_libssh2_wincng_hash_ctx)); + _libssh2_wincng_mfree(ctx->pbHashObject, ctx->dwHashObject); + ctx->pbHashObject = NULL; + ctx->dwHashObject = 0; return ret; } @@ -403,11 +417,11 @@ void _libssh2_wincng_hmac_cleanup(_libssh2_wincng_hash_ctx *ctx) { BCryptDestroyHash(ctx->hHash); + ctx->hHash = NULL; - if (ctx->pbHashObject) - free(ctx->pbHashObject); - - memset(ctx, 0, sizeof(_libssh2_wincng_hash_ctx)); + _libssh2_wincng_mfree(ctx->pbHashObject, ctx->dwHashObject); + ctx->pbHashObject = NULL; + ctx->dwHashObject = 0; } @@ -449,17 +463,17 @@ _libssh2_wincng_key_sha1_verify(_libssh2_wincng_key_ctx *ctx, _libssh2_wincng.hAlgHashSHA1, hash, hashlen); - free(data); + _libssh2_wincng_mfree(data, datalen); if (ret) { - free(hash); + _libssh2_wincng_mfree(hash, hashlen); return -1; } datalen = sig_len; data = malloc(datalen); if (!data) { - free(hash); + _libssh2_wincng_mfree(hash, hashlen); return -1; } @@ -474,8 +488,8 @@ _libssh2_wincng_key_sha1_verify(_libssh2_wincng_key_ctx *ctx, ret = BCryptVerifySignature(ctx->hKey, pPaddingInfo, hash, hashlen, data, datalen, flags); - free(hash); - free(data); + _libssh2_wincng_mfree(hash, hashlen); + _libssh2_wincng_mfree(data, datalen); return BCRYPT_SUCCESS(ret) ? 0 : -1; } @@ -568,7 +582,7 @@ _libssh2_wincng_asn_decode(unsigned char *pbEncoded, pbEncoded, cbEncoded, 0, NULL, pbDecoded, &cbDecoded); if (!ret) { - free(pbDecoded); + _libssh2_wincng_mfree(pbDecoded, cbDecoded); return -1; } @@ -638,7 +652,7 @@ _libssh2_wincng_asn_decode_bn(unsigned char *pbEncoded, *ppbDecoded = pbDecoded; *pcbDecoded = cbDecoded; } - free(pbInteger); + _libssh2_wincng_mfree(pbInteger, cbInteger); } return ret; @@ -683,10 +697,10 @@ _libssh2_wincng_asn_decode_bns(unsigned char *pbEncoded, *pcbCount = length; } else { for (length = 0; length < index; length++) { - if (rpbDecoded[length]) { - free(rpbDecoded[length]); - rpbDecoded[length] = NULL; - } + _libssh2_wincng_mfree(rpbDecoded[length], + rcbDecoded[length]); + rpbDecoded[length] = NULL; + rcbDecoded[length] = 0; } free(rpbDecoded); free(rcbDecoded); @@ -699,7 +713,7 @@ _libssh2_wincng_asn_decode_bns(unsigned char *pbEncoded, ret = -1; } - free(pbDecoded); + _libssh2_wincng_mfree(pbDecoded, cbDecoded); } return ret; @@ -845,7 +859,7 @@ _libssh2_wincng_rsa_new(libssh2_rsa_ctx **rsa, ret = BCryptImportKeyPair(_libssh2_wincng.hAlgRSA, NULL, lpszBlobType, &hKey, key, keylen, 0); if (!BCRYPT_SUCCESS(ret)) { - free(key); + _libssh2_wincng_mfree(key, keylen); return -1; } @@ -853,7 +867,7 @@ _libssh2_wincng_rsa_new(libssh2_rsa_ctx **rsa, *rsa = malloc(sizeof(libssh2_rsa_ctx)); if (!(*rsa)) { BCryptDestroyKey(hKey); - free(key); + _libssh2_wincng_mfree(key, keylen); return -1; } @@ -889,7 +903,7 @@ _libssh2_wincng_rsa_new_private(libssh2_rsa_ctx **rsa, PKCS_RSA_PRIVATE_KEY, &pbStructInfo, &cbStructInfo); - free(pbEncoded); + _libssh2_wincng_mfree(pbEncoded, cbEncoded); if (ret) { return -1; @@ -900,7 +914,7 @@ _libssh2_wincng_rsa_new_private(libssh2_rsa_ctx **rsa, LEGACY_RSAPRIVATE_BLOB, &hKey, pbStructInfo, cbStructInfo, 0); if (!BCRYPT_SUCCESS(ret)) { - free(pbStructInfo); + _libssh2_wincng_mfree(pbStructInfo, cbStructInfo); return -1; } @@ -908,7 +922,7 @@ _libssh2_wincng_rsa_new_private(libssh2_rsa_ctx **rsa, *rsa = malloc(sizeof(libssh2_rsa_ctx)); if (!(*rsa)) { BCryptDestroyKey(hKey); - free(pbStructInfo); + _libssh2_wincng_mfree(pbStructInfo, cbStructInfo); return -1; } @@ -982,7 +996,7 @@ _libssh2_wincng_rsa_sha1_sign(LIBSSH2_SESSION *session, ret = STATUS_NO_MEMORY; } - free(data); + _libssh2_wincng_mfree(data, datalen); return BCRYPT_SUCCESS(ret) ? 0 : -1; } @@ -994,12 +1008,10 @@ _libssh2_wincng_rsa_free(libssh2_rsa_ctx *rsa) return; BCryptDestroyKey(rsa->hKey); + rsa->hKey = NULL; - if (rsa->pbKeyObject) - free(rsa->pbKeyObject); - - memset(rsa, 0, sizeof(libssh2_rsa_ctx)); - free(rsa); + _libssh2_wincng_mfree(rsa->pbKeyObject, rsa->cbKeyObject); + _libssh2_wincng_mfree(rsa, sizeof(libssh2_rsa_ctx)); } @@ -1093,7 +1105,7 @@ _libssh2_wincng_dsa_new(libssh2_dsa_ctx **dsa, ret = BCryptImportKeyPair(_libssh2_wincng.hAlgDSA, NULL, lpszBlobType, &hKey, key, keylen, 0); if (!BCRYPT_SUCCESS(ret)) { - free(key); + _libssh2_wincng_mfree(key, keylen); return -1; } @@ -1101,7 +1113,7 @@ _libssh2_wincng_dsa_new(libssh2_dsa_ctx **dsa, *dsa = malloc(sizeof(libssh2_dsa_ctx)); if (!(*dsa)) { BCryptDestroyKey(hKey); - free(key); + _libssh2_wincng_mfree(key, keylen); return -1; } @@ -1135,7 +1147,7 @@ _libssh2_wincng_dsa_new_private(libssh2_dsa_ctx **dsa, ret = _libssh2_wincng_asn_decode_bns(pbEncoded, cbEncoded, &rpbDecoded, &rcbDecoded, &length); - free(pbEncoded); + _libssh2_wincng_mfree(pbEncoded, cbEncoded); if (ret) { return -1; @@ -1154,10 +1166,9 @@ _libssh2_wincng_dsa_new_private(libssh2_dsa_ctx **dsa, } for (index = 0; index < length; index++) { - if (rpbDecoded[index]) { - free(rpbDecoded[index]); - rpbDecoded[index] = NULL; - } + _libssh2_wincng_mfree(rpbDecoded[index], rcbDecoded[index]); + rpbDecoded[index] = NULL; + rcbDecoded[index] = 0; } free(rpbDecoded); @@ -1215,14 +1226,14 @@ _libssh2_wincng_dsa_sha1_sign(libssh2_dsa_ctx *dsa, memcpy(sig_fixed, sig, siglen); } - free(sig); + _libssh2_wincng_mfree(sig, siglen); } else ret = STATUS_NO_MEMORY; } else ret = STATUS_NO_MEMORY; } - free(data); + _libssh2_wincng_mfree(data, datalen); return BCRYPT_SUCCESS(ret) ? 0 : -1; } @@ -1234,12 +1245,10 @@ _libssh2_wincng_dsa_free(libssh2_dsa_ctx *dsa) return; BCryptDestroyKey(dsa->hKey); + dsa->hKey = NULL; - if (dsa->pbKeyObject) - free(dsa->pbKeyObject); - - memset(dsa, 0, sizeof(libssh2_dsa_ctx)); - free(dsa); + _libssh2_wincng_mfree(dsa->pbKeyObject, dsa->cbKeyObject); + _libssh2_wincng_mfree(dsa, sizeof(libssh2_dsa_ctx)); } #endif @@ -1289,7 +1298,7 @@ _libssh2_wincng_pub_priv_keyfile(LIBSSH2_SESSION *session, ret = _libssh2_wincng_asn_decode_bns(pbEncoded, cbEncoded, &rpbDecoded, &rcbDecoded, &length); - free(pbEncoded); + _libssh2_wincng_mfree(pbEncoded, cbEncoded); if (ret) { return -1; @@ -1362,10 +1371,9 @@ _libssh2_wincng_pub_priv_keyfile(LIBSSH2_SESSION *session, for (index = 0; index < length; index++) { - if (rpbDecoded[index]) { - free(rpbDecoded[index]); - rpbDecoded[index] = NULL; - } + _libssh2_wincng_mfree(rpbDecoded[index], rcbDecoded[index]); + rpbDecoded[index] = NULL; + rcbDecoded[index] = 0; } free(rpbDecoded); @@ -1461,10 +1469,10 @@ _libssh2_wincng_cipher_init(_libssh2_cipher_ctx *ctx, ret = BCryptImportKey(*type.phAlg, NULL, BCRYPT_KEY_DATA_BLOB, &hKey, pbKeyObject, dwKeyObject, key, keylen, 0); - free(key); + _libssh2_wincng_mfree(key, keylen); if (!BCRYPT_SUCCESS(ret)) { - free(pbKeyObject); + _libssh2_wincng_mfree(pbKeyObject, dwKeyObject); return -1; } @@ -1472,7 +1480,7 @@ _libssh2_wincng_cipher_init(_libssh2_cipher_ctx *ctx, pbIV = malloc(dwBlockLength); if (!pbIV) { BCryptDestroyKey(hKey); - free(pbKeyObject); + _libssh2_wincng_mfree(pbKeyObject, dwKeyObject); return -1; } dwIV = dwBlockLength; @@ -1531,7 +1539,7 @@ _libssh2_wincng_cipher_crypt(_libssh2_cipher_ctx *ctx, memcpy(block, pbOutput, cbOutput); } - free(pbOutput); + _libssh2_wincng_mfree(pbOutput, cbOutput); } else ret = STATUS_NO_MEMORY; } @@ -1543,13 +1551,11 @@ void _libssh2_wincng_cipher_dtor(_libssh2_cipher_ctx *ctx) { BCryptDestroyKey(ctx->hKey); + ctx->hKey = NULL; - if (ctx->pbKeyObject) { - free(ctx->pbKeyObject); - ctx->pbKeyObject = NULL; - } - - memset(ctx, 0, sizeof(_libssh2_cipher_ctx)); + _libssh2_wincng_mfree(ctx->pbKeyObject, ctx->dwKeyObject); + ctx->pbKeyObject = NULL; + ctx->dwKeyObject = 0; } @@ -1581,6 +1587,12 @@ _libssh2_wincng_bignum_resize(_libssh2_bn *bn, unsigned long length) if (length == bn->length) return 0; +#ifdef LIBSSH2_CLEAR_MEMORY + if (bn->bignum && bn->length > 0 && length < bn->length) { + SecureZeroMemory(bn->bignum + length, bn->length - length); + } +#endif + bignum = realloc(bn->bignum, length); if (!bignum) return -1; @@ -1688,7 +1700,7 @@ _libssh2_wincng_bignum_mod_exp(_libssh2_bn *r, r->bignum, r->length, &offset, BCRYPT_PAD_NONE); - free(bignum); + _libssh2_wincng_mfree(bignum, length); if (BCRYPT_SUCCESS(ret)) { _libssh2_wincng_bignum_resize(r, offset); @@ -1702,7 +1714,7 @@ _libssh2_wincng_bignum_mod_exp(_libssh2_bn *r, BCryptDestroyKey(hKey); } - free(key); + _libssh2_wincng_mfree(key, keylen); return BCRYPT_SUCCESS(ret) ? 0 : -1; } @@ -1780,6 +1792,10 @@ _libssh2_wincng_bignum_from_bin(_libssh2_bn *bn, unsigned long len, if (offset > 0) { memmove(bn->bignum, bn->bignum + offset, length); +#ifdef LIBSSH2_CLEAR_MEMORY + SecureZeroMemory(bn->bignum + length, offset); +#endif + bignum = realloc(bn->bignum, length); if (bignum) { bn->bignum = bignum; @@ -1801,11 +1817,11 @@ _libssh2_wincng_bignum_free(_libssh2_bn *bn) { if (bn) { if (bn->bignum) { - free(bn->bignum); + _libssh2_wincng_mfree(bn->bignum, bn->length); bn->bignum = NULL; } bn->length = 0; - free(bn); + _libssh2_wincng_mfree(bn, sizeof(_libssh2_bn)); } } -- 1.9.2.msysgit.0