[strongSwan-dev] Memwipe of loaded secrets through VICI

[Jean-Francois HREN]

Hello, 

I went through the code loading secrets (either private keys or PSKs) using VICI in swanctl and found several places where they were not memwipe. 
I added necessary calls to memwipe and tested it : 
---------------------------------------------------------------------------------------------------------------------------- 
diff --git a/src/libcharon/plugins/vici/vici_message.c b/src/libcharon/plugins/vici/vici_message.c 
index df5b85c64..5ce37c91a 100644 
--- a/src/libcharon/plugins/vici/vici_message.c 
+++ b/src/libcharon/plugins/vici/vici_message.c 
@@ -644,6 +644,17 @@ METHOD(vici_message_t, dump, bool, 
return FALSE; 
} 

+CALLBACK(clear_strings, void, 
+ void *ptr) 
+{ 
+ char *str; 
+ 
+ str = (char*)ptr; 
+ 
+ memwipe(str, strlen(str)); 
+ free(str); 
+} 
+ 
METHOD(vici_message_t, destroy, void, 
private_vici_message_t *this) 
{ 
@@ -651,7 +662,11 @@ METHOD(vici_message_t, destroy, void, 
{ 
chunk_clear(&this->encoding); 
} 
- this->strings->destroy_function(this->strings, free); 
+ else 
+ { 
+ memwipe(this->encoding.ptr, this->encoding.len); 
+ } 
+ this->strings->destroy_function(this->strings, clear_strings); 
free(this); 
} 

diff --git a/src/libstrongswan/credentials/keys/shared_key.c b/src/libstrongswan/credentials/keys/shared_key.c 
index 97209953a..039398cd2 100644 
--- a/src/libstrongswan/credentials/keys/shared_key.c 
+++ b/src/libstrongswan/credentials/keys/shared_key.c 
@@ -77,7 +77,7 @@ METHOD(shared_key_t, destroy, void, 
{ 
if (ref_put(&this->ref)) 
{ 
- free(this->key.ptr); 
+ chunk_clear(&this->key); 
free(this); 
} 
} 
diff --git a/src/libstrongswan/plugins/pem/pem_builder.c b/src/libstrongswan/plugins/pem/pem_builder.c 
index 3b84eb7ea..9ca96a4d2 100644 
--- a/src/libstrongswan/plugins/pem/pem_builder.c 
+++ b/src/libstrongswan/plugins/pem/pem_builder.c 
@@ -147,7 +147,7 @@ static status_t pem_decrypt(chunk_t *blob, encryption_algorithm_t alg, 
} 
crypter->destroy(crypter); 
memcpy(blob->ptr, decrypted.ptr, blob->len); 
- chunk_free(&decrypted); 
+ chunk_clear(&decrypted); 

/* determine amount of padding */ 
last_padding_pos = blob->ptr + blob->len - 1; 
@@ -354,7 +354,7 @@ static status_t pem_to_bin(chunk_t *blob, bool *pgp) 
memcpy(blob->ptr, chunk.ptr, chunk.len); 
blob->len = chunk.len; 
} 
- free(chunk.ptr); 
+ chunk_clear(&chunk); 
if (status != INVALID_ARG) 
{ /* try again only if passphrase invalid */ 
break; 
diff --git a/src/libstrongswan/utils/chunk.c b/src/libstrongswan/utils/chunk.c 
index ac19fafb4..5ab25ee42 100644 
--- a/src/libstrongswan/utils/chunk.c 
+++ b/src/libstrongswan/utils/chunk.c 
@@ -445,6 +445,20 @@ bool chunk_unmap(chunk_t *public) 
return ret; 
} 

+/** 
+ * See header. 
+ */ 
+bool chunk_unmap_clear(chunk_t *public) 
+{ 
+ mmaped_chunk_t *chunk; 
+ 
+ chunk = (mmaped_chunk_t*)public; 
+ if (!chunk->wr && chunk->map != MAP_FAILED) 
+ memwipe(chunk->map, chunk->len); 
+ 
+ return chunk_unmap(public); 
+} 
+ 
/** hex conversion digits */ 
static char hexdig_upper[] = "0123456789ABCDEF"; 
static char hexdig_lower[] = "0123456789abcdef"; 
diff --git a/src/libstrongswan/utils/chunk.h b/src/libstrongswan/utils/chunk.h 
index a5eb5ed3b..94045479c 100644 
--- a/src/libstrongswan/utils/chunk.h 
+++ b/src/libstrongswan/utils/chunk.h 
@@ -139,6 +139,18 @@ chunk_t *chunk_map(char *path, bool wr); 
*/ 
bool chunk_unmap(chunk_t *chunk); 

+/** 
+ * munmap() a chunk previously mapped with chunk_map() after clearing it (only 
+ * if not a writeable map) 
+ * 
+ * When unmapping a writeable map, the return value should be checked to 
+ * ensure changes landed on disk. 
+ * 
+ * @param chunk pointer returned from chunk_map() 
+ * @return TRUE of changes written back to file 
+ */ 
+bool chunk_unmap_clear(chunk_t *chunk); 
+ 
/** 
* Convert a chunk of data to hex encoding. 
* 
diff --git a/src/swanctl/commands/load_creds.c b/src/swanctl/commands/load_creds.c 
index 2c1947dd1..c6c24d233 100644 
--- a/src/swanctl/commands/load_creds.c 
+++ b/src/swanctl/commands/load_creds.c 
@@ -445,7 +445,7 @@ static void load_keys(load_ctx_t *ctx, char *type, char *dir) 
{ 
load_key(ctx, path, type, *map); 
} 
- chunk_unmap(map); 
+ chunk_unmap_clear(map); 
} 
else 
{ 
@@ -552,7 +552,7 @@ static void load_containers(load_ctx_t *ctx, char *type, char *dir) 
if (map) 
{ 
load_encrypted_container(ctx, rel, path, type, *map); 
- chunk_unmap(map); 
+ chunk_unmap_clear(map); 
} 
else 
{ 
---------------------------------------------------------------------------------------------------------------------------- 

Jean-François HREN 
Developper - Network Security R&D 
[ http://www.stormshield.eu/ ] 
	STORMSHIELD 
2/6 Parc de l'Horizon 
59650 Villeneuve d'Ascq - FRANCE 
Mobile : +33 (0)6 23 08 80 81 
[ https://twitter.com/Stormshield | Twitter ] . [ https://www.linkedin.com/company/22425?trk=cws-btn-overview-0-0 | LinkedIn ] . [ http://www.stormshield.eu/ | www.stormshield.eu ] 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.strongswan.org/pipermail/dev/attachments/20210930/5fec14d2/attachment.html>