Hi Peter, On 19.06.2014 19:57, Peter Stuge wrote: > The configure switch should only be available when configuring with > wincng crypto. > > If that is not possible (autoconf limitations) then enabling the > option should throw an error when this functionality is not available > in code. > > Failing silently (ie. not securely zeroing memory) after a successful > configure of the library with the option enabled isn't really > acceptable IMO.
thanks for the feedback. I updated configure.ac to produce a warning if secure clearing/zeroing of memory is unsupported / not available and expanded the configure summary to look like the following, as an example for the OpenSSL backend: configure: summary of build options: version: 1.4.4_DEV Host type: x86_64-unknown-linux-gnu Install prefix: /usr/local Compiler: gcc Compiler flags: -g -O2 Library types: Shared=yes, Static=yes Crypto library: OpenSSL (AES-CTR: yes) Clear memory: unsupported Debug build: no Build examples: yes Path to sshd: /usr/sbin/sshd (only for self-tests) zlib compression: yes Clear memory shows either "yes" (enabled and available), "no" (disabled) or "unsupported" (unavailable). Please find the updated patch attached to this email. Best regards, Marc
From 1ce846ccff1a603167a2fa48d394d35853696e48 Mon Sep 17 00:00:00 2001
From: Marc Hoersken <i...@marc-hoersken.de>
Date: Fri, 20 Jun 2014 15:32:11 +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 | 16 +++++++
src/wincng.c | 146 +++++++++++++++++++++++++++++++++--------------------------
2 files changed, 97 insertions(+), 65 deletions(-)
diff --git a/configure.ac b/configure.ac
index ba4dd7a..a2367b8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -197,6 +197,21 @@ 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 "$ac_cv_libbcrypt" = "yes"; then
+ if test "$CLEAR_MEMORY" != "no"; then
+ AC_DEFINE(LIBSSH2_CLEAR_MEMORY, 1, [Enable clearing of memory before being
freed])
+ enable_clear_memory=yes
+ else
+ enable_clear_memory=no
+ fi
+else
+ AC_MSG_WARN([secure clearing/zeroing of memory is only supported by the
WinCNG backend])
+ enable_clear_memory=unsupported
+fi
+
dnl ************************************************************
dnl option to switch on compiler debug options
dnl
@@ -362,6 +377,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
