From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 30 Sep 2021 10:39:08 +0000 (+0200)
Subject: chunk: Optionally clear mmap'd chunk before unmapping
X-Git-Tag: 5.9.4dr3~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e2e21f248621dd2f654388bbb1ccc322fc8de04c;p=thirdparty%2Fstrongswan.git

chunk: Optionally clear mmap'd chunk before unmapping

This is mostly for the non-mmap case as with mmap available, access to the
unmapped memory isn't easily possible (e.g. opening the same area with
MAP_ANONYMOUS | MAP_UNINITIALIZED is usually prevented by the missing
CONFIG_MMAP_ALLOW_UNINITIALIZED option in most kernels).
---

diff --git a/src/libstrongswan/tests/suites/test_chunk.c b/src/libstrongswan/tests/suites/test_chunk.c
index 2f6b59f816..6f276d7750 100644
--- a/src/libstrongswan/tests/suites/test_chunk.c
+++ b/src/libstrongswan/tests/suites/test_chunk.c
@@ -939,6 +939,44 @@ START_TEST(test_chunk_map)
 }
 END_TEST
 
+START_TEST(test_chunk_map_clear)
+{
+	chunk_t *map, contents = chunk_from_chars(0x01,0x02,0x03,0x04,0x05);
+#ifdef WIN32
+	char *path = "C:\\Windows\\Temp\\strongswan-chunk-map-test";
+#else
+	char *path = "/tmp/strongswan-chunk-map-clear-test";
+#endif
+
+	ck_assert(chunk_write(contents, path, 022, TRUE));
+
+	/* read */
+	map = chunk_map(path, FALSE);
+	ck_assert(map != NULL);
+	ck_assert_msg(chunk_equals(*map, contents), "%B", map);
+	ck_assert(chunk_unmap_clear(map));
+	/* we can't verify that clearing worked as we don't have access to the
+	 * memory anymore and mmap with MAP_ANONYMOUS | MAP_UNINITIALIZED of the
+	 * same area will only work if the kernel allows this */
+
+	/* write */
+	map = chunk_map(path, TRUE);
+	ck_assert(map != NULL);
+	ck_assert_msg(chunk_equals(*map, contents), "%B", map);
+	map->ptr[0] = 0x42;
+	ck_assert(chunk_unmap_clear(map));
+
+	/* verify write */
+	contents.ptr[0] = 0x42;
+	map = chunk_map(path, FALSE);
+	ck_assert(map != NULL);
+	ck_assert_msg(chunk_equals(*map, contents), "%B", map);
+	ck_assert(chunk_unmap_clear(map));
+
+	unlink(path);
+}
+END_TEST
+
 /*******************************************************************************
  * test for chunk_from_fd
  */
@@ -1144,6 +1182,7 @@ Suite *chunk_suite_create()
 
 	tc = tcase_create("chunk_map");
 	tcase_add_test(tc, test_chunk_map);
+	tcase_add_test(tc, test_chunk_map_clear);
 	suite_add_tcase(s, tc);
 
 	tc = tcase_create("chunk_from_fd");
diff --git a/src/libstrongswan/utils/chunk.c b/src/libstrongswan/utils/chunk.c
index ac19fafb43..dd4a0a0341 100644
--- a/src/libstrongswan/utils/chunk.c
+++ b/src/libstrongswan/utils/chunk.c
@@ -396,9 +396,9 @@ chunk_t *chunk_map(char *path, bool wr)
 }
 
 /**
- * See header.
+ * Unmap the given chunk and optionally clear it
  */
-bool chunk_unmap(chunk_t *public)
+static bool chunk_unmap_internal(chunk_t *public, bool clear)
 {
 	mmaped_chunk_t *chunk;
 	bool ret = FALSE;
@@ -408,6 +408,10 @@ bool chunk_unmap(chunk_t *public)
 #ifdef HAVE_MMAP
 	if (chunk->map && chunk->map != MAP_FAILED)
 	{
+		if (!chunk->wr && clear)
+		{
+			memwipe(chunk->map, chunk->len);
+		}
 		ret = munmap(chunk->map, chunk->len) == 0;
 		tmp = errno;
 	}
@@ -436,6 +440,10 @@ bool chunk_unmap(chunk_t *public)
 	{
 		ret = TRUE;
 	}
+	if (clear)
+	{
+		memwipe(chunk->map, chunk->len);
+	}
 	free(chunk->map);
 #endif /* !HAVE_MMAP */
 	close(chunk->fd);
@@ -445,6 +453,22 @@ bool chunk_unmap(chunk_t *public)
 	return ret;
 }
 
+/*
+ * Described in header
+ */
+bool chunk_unmap(chunk_t *public)
+{
+	return chunk_unmap_internal(public, FALSE);
+}
+
+/*
+ * Described in header
+ */
+bool chunk_unmap_clear(chunk_t *public)
+{
+	return chunk_unmap_internal(public, TRUE);
+}
+
 /** 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 a5eb5ed3ba..6cf6510ca4 100644
--- a/src/libstrongswan/utils/chunk.h
+++ b/src/libstrongswan/utils/chunk.h
@@ -114,7 +114,7 @@ bool chunk_write(chunk_t chunk, char *path, mode_t mask, bool force);
 bool chunk_from_fd(int fd, chunk_t *chunk);
 
 /**
- * mmap() a file to a chunk
+ * mmap() a file to a chunk.
  *
  * The returned chunk structure is allocated from heap, but it must be freed
  * through chunk_unmap(). A user may alter the chunk ptr or len, but must pass
@@ -129,16 +129,29 @@ bool chunk_from_fd(int fd, chunk_t *chunk);
 chunk_t *chunk_map(char *path, bool wr);
 
 /**
- * munmap() a chunk previously mapped with chunk_map()
+ * munmap() a chunk previously mapped with chunk_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
+ * @return				TRUE if changes written back to file
  */
 bool chunk_unmap(chunk_t *chunk);
 
+/**
+ * munmap() a chunk previously mapped with chunk_map() after clearing it.
+ *
+ * @note Writable maps (i.e. created with wr = TRUE) are NOT cleared.
+ *
+ * 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 if changes written back to file
+ */
+bool chunk_unmap_clear(chunk_t *chunk);
+
 /**
  * Convert a chunk of data to hex encoding.
  *