Hello everyone,
attached you will find a new patch that has been rebased to the current
master.
Please review this patch. Any feedback is welcome.
Best regards,
Marc
>From c59778974769497d9b9eba290231b0e6220bcce8 Mon Sep 17 00:00:00 2001
From: Marc Hoersken <i...@marc-hoersken.de>
Date: Sun, 16 Mar 2014 20:02:17 +0100
Subject: [PATCH] wincng: Added explicit memory overwrite 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.
---
configure.ac | 7 +++
src/wincng.c | 145 +++++++++++++++++++++++++++++++++--------------------------
2 files changed, 88 insertions(+), 64 deletions(-)
diff --git a/configure.ac b/configure.ac
index ba4dd7a..f31e3c5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -197,6 +197,13 @@ 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(memory-overwrite,
+ AC_HELP_STRING([--disable-memory-overwrite],[Disable memory overwrite before
being freed]),
+ [MEMORY_OVERWRITE=$enableval])
+if test "$MEMORY_OVERWRITE" != "no"; then
+ AC_DEFINE(LIBSSH2_MEMORY_OVERWRITE, 1, [Enable memory overwrite before being
freed])
+fi
+
dnl ************************************************************
dnl option to switch on compiler debug options
dnl
diff --git a/src/wincng.c b/src/wincng.c
index f5390d2..2471a7c 100644
--- a/src/wincng.c
+++ b/src/wincng.c
@@ -272,6 +272,23 @@ _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_MEMORY_OVERWRITE
+ if (len > 0)
+ _libssh2_wincng_random(buf, len);
+#else
+ if (len > 0)
+ memset(buf, 0, len);
+#endif
+
+ free(buf);
+}
+
/*******************************************************************/
/*
@@ -314,7 +331,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;
}
@@ -347,11 +364,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;
}
@@ -395,11 +412,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;
}
@@ -441,17 +458,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;
}
@@ -466,8 +483,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;
}
@@ -560,7 +577,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;
}
@@ -630,7 +647,7 @@ _libssh2_wincng_asn_decode_bn(unsigned char *pbEncoded,
*ppbDecoded = pbDecoded;
*pcbDecoded = cbDecoded;
}
- free(pbInteger);
+ _libssh2_wincng_mfree(pbInteger, cbInteger);
}
return ret;
@@ -675,10 +692,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);
@@ -691,7 +708,7 @@ _libssh2_wincng_asn_decode_bns(unsigned char *pbEncoded,
ret = -1;
}
- free(pbDecoded);
+ _libssh2_wincng_mfree(pbDecoded, cbDecoded);
}
return ret;
@@ -837,7 +854,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;
}
@@ -845,7 +862,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;
}
@@ -881,7 +898,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;
@@ -892,7 +909,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;
}
@@ -900,7 +917,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;
}
@@ -974,7 +991,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;
}
@@ -987,11 +1004,8 @@ _libssh2_wincng_rsa_free(libssh2_rsa_ctx *rsa)
BCryptDestroyKey(rsa->hKey);
- 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));
}
@@ -1085,7 +1099,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;
}
@@ -1093,7 +1107,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;
}
@@ -1127,7 +1141,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;
@@ -1146,10 +1160,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);
@@ -1207,14 +1220,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;
}
@@ -1227,11 +1240,8 @@ _libssh2_wincng_dsa_free(libssh2_dsa_ctx *dsa)
BCryptDestroyKey(dsa->hKey);
- 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
@@ -1281,7 +1291,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;
@@ -1354,10 +1364,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);
@@ -1453,10 +1462,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;
}
@@ -1464,7 +1473,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;
@@ -1523,7 +1532,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;
}
@@ -1535,13 +1544,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;
}
@@ -1573,6 +1580,12 @@ _libssh2_wincng_bignum_resize(_libssh2_bn *bn, unsigned
long length)
if (length == bn->length)
return 0;
+#ifdef LIBSSH2_MEMORY_OVERWRITE
+ if (bn->bignum && bn->length > 0 && length < bn->length) {
+ _libssh2_wincng_random(bn->bignum + length, bn->length - length);
+ }
+#endif
+
bignum = realloc(bn->bignum, length);
if (!bignum)
return -1;
@@ -1680,7 +1693,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);
@@ -1694,7 +1707,7 @@ _libssh2_wincng_bignum_mod_exp(_libssh2_bn *r,
BCryptDestroyKey(hKey);
}
- free(key);
+ _libssh2_wincng_mfree(key, keylen);
return BCRYPT_SUCCESS(ret) ? 0 : -1;
}
@@ -1772,6 +1785,10 @@ _libssh2_wincng_bignum_from_bin(_libssh2_bn *bn,
unsigned long len,
if (offset > 0) {
memmove(bn->bignum, bn->bignum + offset, length);
+#ifdef LIBSSH2_MEMORY_OVERWRITE
+ _libssh2_wincng_random(bn->bignum + length, offset);
+#endif
+
bignum = realloc(bn->bignum, length);
if (bignum) {
bn->bignum = bignum;
@@ -1793,7 +1810,7 @@ _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;
--
1.9.2.msysgit.0
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
