Hi Peter, On 20.06.2014 18:15, Peter Stuge wrote: > I think a warning is appropriate when the --enable option was not > explicitly specified. > > I think an error is neccessary when --enable *was* specified, but > support is unavailable.
okay, but I am not sure how to differentiate the two scenarios. I tried to implement the warning vs error condition, but now configure will always display an error while configuring against openssl or libgcrypt if --disable-clear-memory is not given. Please take a look at the attached updated patch. > Please don't add a new list of crypto backends to maintain. I'd > suggest to instead introduce an abstraction such as > $support_clear_memory which is set to no by default and set to yes by > backends supporting this functionality. > > The above check would then inspect only that variable. Good idea, implemented. Best regards, Marc
From efc888d18c49a8a3e0a1085d2d052660c9d1ff95 Mon Sep 17 00:00:00 2001
From: Marc Hoersken <i...@marc-hoersken.de>
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
signature.asc
Description: OpenPGP digital signature
_______________________________________________ libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
