[PATCH] memset_s() and talloc_set_secure()
Andreas Schneider
asn at samba.org
Wed Apr 3 08:06:09 UTC 2019
On Wednesday, March 27, 2019 4:48:45 PM CEST Jeremy Allison via samba-
technical wrote:
> If you want this in immediately, take Andrews RB+
> and push, and let's make the flag mechanism a future
> enhancement.
I've pushed the changes using a destructor to master.
Attached is an updated patchset which reverts those changes and implements it
in talloc itself.
Andreas
--
Andreas Schneider asn at samba.org
Samba Team www.samba.org
GPG-ID: 8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
-------------- next part --------------
>From 0dd871b7d295471311259f1b0ad610a8f19b5dc8 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 3 Apr 2019 10:02:54 +0200
Subject: [PATCH 1/8] Revert "lib:util: Include talloc_keep_secret.h in
samba_util.h"
This reverts commit c7f403d373016374fc96b7fa113f4723a41788a0.
---
lib/util/samba_util.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/lib/util/samba_util.h b/lib/util/samba_util.h
index 0722426216e..20adae39bcf 100644
--- a/lib/util/samba_util.h
+++ b/lib/util/samba_util.h
@@ -48,7 +48,6 @@ extern const char *panic_action;
#include "lib/util/data_blob.h"
#include "lib/util/byteorder.h"
#include "lib/util/talloc_stack.h"
-#include "lib/util/talloc_keep_secret.h"
#ifndef ABS
#define ABS(a) ((a)>0?(a):(-(a)))
--
2.21.0
>From 41da361e0dc5e9d246fe36502c047dafcde28d79 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 3 Apr 2019 10:03:00 +0200
Subject: [PATCH 2/8] Revert "lib:util: Add test for talloc_keep_secret()"
This reverts commit c4baf2f6857718ea0d94b4134e8c12f0737a9e23.
---
lib/util/tests/test_talloc_keep_secret.c | 94 ------------------------
lib/util/wscript_build | 6 --
selftest/tests.py | 3 -
3 files changed, 103 deletions(-)
delete mode 100644 lib/util/tests/test_talloc_keep_secret.c
diff --git a/lib/util/tests/test_talloc_keep_secret.c b/lib/util/tests/test_talloc_keep_secret.c
deleted file mode 100644
index 1462dabe956..00000000000
--- a/lib/util/tests/test_talloc_keep_secret.c
+++ /dev/null
@@ -1,94 +0,0 @@
-#include <stdarg.h>
-#include <stddef.h>
-#include <stdint.h>
-#include <setjmp.h>
-#include <cmocka.h>
-
-#include <string.h>
-#include <talloc.h>
-#include "lib/util/talloc_keep_secret.h"
-
-int rep_memset_s(void *dest, size_t destsz, int ch, size_t count);
-
-int rep_memset_s(void *dest, size_t destsz, int ch, size_t count)
-{
- check_expected_ptr(dest);
- check_expected(destsz);
- check_expected(ch);
- check_expected(count);
-
- return 0;
-}
-
-static void test_talloc_keep_secret(void ** state)
-{
- TALLOC_CTX *pool = NULL;
- char *ptr1 = NULL;
- char *ptr2 = NULL;
- const char *ptr1_talloc_name = NULL;
- size_t ptr1_size;
- size_t i;
-
- pool = talloc_pool(NULL, 256);
- assert_non_null(pool);
-
- ptr1 = talloc_strdup(pool, "secret");
- assert_non_null(ptr1);
- assert_string_equal(ptr1, "secret");
-
- talloc_keep_secret(ptr1);
-
- ptr1_talloc_name = talloc_get_name(ptr1);
- assert_string_equal(ptr1_talloc_name, "ptr1");
-
- ptr1_size = talloc_get_size(ptr1);
- assert_int_equal(ptr1_size, strlen(ptr1) + 1);
-
- expect_string(rep_memset_s, dest, "secret");
- expect_value(rep_memset_s, destsz, strlen(ptr1) + 1);
- expect_value(rep_memset_s, ch, (int)'\0');
- expect_value(rep_memset_s, count, strlen(ptr1) + 1);
-
- talloc_free(ptr1);
-
- ptr2 = talloc_size(pool, ptr1_size);
- assert_ptr_equal(ptr1, ptr2);
-
- for (i = 1; i < ptr1_size; i++) {
- assert_int_not_equal(ptr2[0], ptr2[i]);
- }
-
- talloc_free(pool);
-}
-
-static void test_talloc_keep_secret_validate_memset(void **state)
-{
- TALLOC_CTX *mem_ctx = NULL;
- char *password = NULL;
-
- mem_ctx = talloc_new(NULL);
- assert_non_null(mem_ctx);
-
- password = talloc_strdup(mem_ctx, "secret");
- assert_non_null(password);
- talloc_keep_secret(password);
-
- expect_string(rep_memset_s, dest, "secret");
- expect_value(rep_memset_s, destsz, strlen(password) + 1);
- expect_value(rep_memset_s, ch, (int)'\0');
- expect_value(rep_memset_s, count, strlen(password) + 1);
-
- talloc_free(mem_ctx);
-}
-
-int main(void)
-{
- const struct CMUnitTest tests[] = {
- cmocka_unit_test(test_talloc_keep_secret),
- cmocka_unit_test(test_talloc_keep_secret_validate_memset),
- };
-
- cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
-
- return cmocka_run_group_tests(tests, NULL, NULL);
-}
diff --git a/lib/util/wscript_build b/lib/util/wscript_build
index 9af1bb5d801..0a3fb09981f 100644
--- a/lib/util/wscript_build
+++ b/lib/util/wscript_build
@@ -269,9 +269,3 @@ else:
deps='cmocka replace samba-util',
local_include=False,
install=False)
-
- bld.SAMBA_BINARY('test_talloc_keep_secret',
- source='tests/test_talloc_keep_secret.c',
- deps='cmocka replace samba-util',
- local_include=False,
- install=False)
diff --git a/selftest/tests.py b/selftest/tests.py
index 133f227ab82..01afdaea2d0 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -246,9 +246,6 @@ plantestsuite("samba.unittests.lib_util_modules", "none",
plantestsuite("samba.unittests.smb1cli_session", "none",
[os.path.join(bindir(), "default/libcli/smb/test_smb1cli_session")])
-plantestsuite("samba.unittests.talloc_keep_secret", "none",
- [os.path.join(bindir(), "default/lib/util/test_talloc_keep_secret")])
-
plantestsuite("samba.unittests.tldap", "none",
[os.path.join(bindir(), "default/source3/test_tldap")])
plantestsuite("samba.unittests.rfc1738", "none",
--
2.21.0
>From 496b35d100bd40b1f296aa99c9bc6d080875197c Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 3 Apr 2019 10:03:14 +0200
Subject: [PATCH 3/8] Revert "lib:util: Add support to keep talloc chunks
secret"
This reverts commit b7f7e5a37b0b0196e07a2829486f56ce32acdeff.
---
lib/util/talloc_keep_secret.c | 52 -----------------------------------
lib/util/talloc_keep_secret.h | 42 ----------------------------
lib/util/wscript_build | 38 +++++++------------------
3 files changed, 10 insertions(+), 122 deletions(-)
delete mode 100644 lib/util/talloc_keep_secret.c
delete mode 100644 lib/util/talloc_keep_secret.h
diff --git a/lib/util/talloc_keep_secret.c b/lib/util/talloc_keep_secret.c
deleted file mode 100644
index d6aa38265f6..00000000000
--- a/lib/util/talloc_keep_secret.c
+++ /dev/null
@@ -1,52 +0,0 @@
-/*
- * Copyright (c) 2019 Andreas Schneider <asn at samba.org>
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include "includes.h"
-#include "talloc_keep_secret.h"
-
-static int talloc_keep_secret_destructor(void *ptr)
-{
- size_t size = talloc_get_size(ptr);
-
- if (unlikely(size == 0)) {
- return 0;
- }
-
- memset_s(ptr, size, 0, size);
-
- return 0;
-}
-
-void _talloc_keep_secret(void *ptr, const char *name)
-{
- size_t size;
-
- if (unlikely(ptr == NULL)) {
-#ifdef DEVELOPER
- smb_panic("Invalid talloc pointer");
-#endif
- return;
- }
-
- size = talloc_get_size(ptr);
- if (unlikely(size == 0)) {
- return;
- }
-
- talloc_set_name_const(ptr, name);
- talloc_set_destructor(ptr, talloc_keep_secret_destructor);
-}
diff --git a/lib/util/talloc_keep_secret.h b/lib/util/talloc_keep_secret.h
deleted file mode 100644
index 44a26aef542..00000000000
--- a/lib/util/talloc_keep_secret.h
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
- * Copyright (c) 2019 Andreas Schneider <asn at samba.org>
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef _TALLOC_KEEP_SECRET_H
-#define _TALLOC_KEEP_SECRET_H
-
-#ifdef DOXYGEN
-/**
- * @brief Keep the memory secret when freeing.
- *
- * This can be used to define memory as secret. For example memory which holds
- * passwords or other secrets like session keys. The memory will be zeroed
- * before is being freed.
- *
- * If you duplicate memory, e.g. using talloc_strdup() or talloc_asprintf() you
- * need to call talloc_keep_secret() on the newly allocated memory too!
- *
- * @param[in] ptr The talloc chunk to mark as secure.
- *
- * @warning Do not use this in combination with talloc_realloc().
- */
-void talloc_keep_secret(const void *ptr);
-#else
-#define talloc_keep_secret(ptr) _talloc_keep_secret(ptr, #ptr);
-void _talloc_keep_secret(void *ptr, const char *name);
-#endif
-
-#endif /* _TALLOC_KEEP_SECRET_H */
diff --git a/lib/util/wscript_build b/lib/util/wscript_build
index 0a3fb09981f..5ca72c5c03c 100644
--- a/lib/util/wscript_build
+++ b/lib/util/wscript_build
@@ -115,34 +115,16 @@ else:
install=False)
bld.SAMBA_LIBRARY('samba-util',
- source='''
- base64.c
- dprintf.c
- fsusage.c
- genrand_util.c
- getpass.c
- idtree_random.c
- memcache.c
- ms_fnmatch.c
- params.c
- rbtree.c
- rfc1738.c
- server_id.c
- smb_threads.c
- system.c
- talloc_keep_secret.c
- talloc_stack.c
- tevent_debug.c
- tfork.c
- tftw.c
- unix_match.c
- util_id.c
- util_net.c
- util_paths.c
- util_str.c
- util_str_common.c
- util_strlist_v3.c
- ''',
+ source='''talloc_stack.c smb_threads.c
+ rbtree.c rfc1738.c system.c getpass.c
+ genrand_util.c fsusage.c
+ params.c util_id.c util_net.c
+ util_strlist_v3.c util_paths.c
+ idtree_random.c base64.c
+ util_str.c util_str_common.c ms_fnmatch.c
+ server_id.c dprintf.c
+ tevent_debug.c memcache.c unix_match.c tfork.c
+ tftw.c''',
deps='samba-util-core DYNCONFIG close-low-fd tiniparser genrand util_str_hex',
public_deps='talloc tevent execinfo pthread LIBCRYPTO charset util_setid',
public_headers='''
--
2.21.0
>From 6d4b492a916e2af46b162be12f6295a94feced5d Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 11 Oct 2018 07:54:41 +0200
Subject: [PATCH 4/8] talloc: Add support to keep talloc chunks secret
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/talloc.c | 26 +++++++++++++++++++-------
lib/talloc/talloc.h | 20 ++++++++++++++++++++
2 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 518ffbdbfdf..ba7ae9f32ab 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -65,12 +65,13 @@
#define TALLOC_FLAG_LOOP 0x02
#define TALLOC_FLAG_POOL 0x04 /* This is a talloc pool */
#define TALLOC_FLAG_POOLMEM 0x08 /* This is allocated in a pool */
+#define TALLOC_FLAG_SECRET 0x10
/*
* Bits above this are random, used to make it harder to fake talloc
* headers during an attack. Try not to change this without good reason.
*/
-#define TALLOC_FLAG_MASK 0x0F
+#define TALLOC_FLAG_MASK 0x1F
#define TALLOC_MAGIC_REFERENCE ((const char *)1)
@@ -143,10 +144,10 @@ static struct {
* double-free logic to still work
*/
#define TC_INVALIDATE_FULL_FILL_CHUNK(_tc) do { \
- if (unlikely(talloc_fill.enabled)) { \
+ if (unlikely(talloc_fill.enabled || (_tc)->flags & TALLOC_FLAG_SECRET)) { \
size_t _flen = (_tc)->size; \
char *_fptr = (char *)TC_PTR_FROM_CHUNK(_tc); \
- memset(_fptr, talloc_fill.fill_value, _flen); \
+ memset_s(_fptr, _flen, talloc_fill.fill_value, _flen); \
} \
} while (0)
@@ -167,11 +168,11 @@ static struct {
} while (0)
#define TC_INVALIDATE_SHRINK_FILL_CHUNK(_tc, _new_size) do { \
- if (unlikely(talloc_fill.enabled)) { \
+ if (unlikely(talloc_fill.enabled || (_tc)->flags & TALLOC_FLAG_SECRET)) { \
size_t _flen = (_tc)->size - (_new_size); \
char *_fptr = (char *)TC_PTR_FROM_CHUNK(_tc); \
_fptr += (_new_size); \
- memset(_fptr, talloc_fill.fill_value, _flen); \
+ memset_s(_fptr, _flen, talloc_fill.fill_value, _flen); \
} \
} while (0)
@@ -193,11 +194,11 @@ static struct {
} while (0)
#define TC_UNDEFINE_SHRINK_FILL_CHUNK(_tc, _new_size) do { \
- if (unlikely(talloc_fill.enabled)) { \
+ if (unlikely(talloc_fill.enabled || (_tc)->flags & TALLOC_FLAG_SECRET)) { \
size_t _flen = (_tc)->size - (_new_size); \
char *_fptr = (char *)TC_PTR_FROM_CHUNK(_tc); \
_fptr += (_new_size); \
- memset(_fptr, talloc_fill.fill_value, _flen); \
+ memset_s(_fptr, _flen, talloc_fill.fill_value, _flen); \
} \
} while (0)
@@ -967,6 +968,9 @@ static int talloc_reference_destructor(struct talloc_reference_handle *handle)
static inline void _tc_set_name_const(struct talloc_chunk *tc,
const char *name)
{
+ if (unlikely(tc->flags & TALLOC_FLAG_SECRET)) {
+ return;
+ }
tc->name = name;
}
@@ -1523,6 +1527,14 @@ _PUBLIC_ const char *talloc_set_name(const void *ptr, const char *fmt, ...)
return name;
}
+_PUBLIC_ void _talloc_keep_secret(const void *ptr, const char *name)
+{
+ struct talloc_chunk *tc = talloc_chunk_from_ptr(ptr);
+
+ _tc_set_name_const(tc, name);
+
+ tc->flags |= TALLOC_FLAG_SECRET;
+}
/*
create a named talloc pointer. Any talloc pointer can be named, and
diff --git a/lib/talloc/talloc.h b/lib/talloc/talloc.h
index 34fe772d2df..1cc0f882d1b 100644
--- a/lib/talloc/talloc.h
+++ b/lib/talloc/talloc.h
@@ -400,6 +400,26 @@ void *_talloc_steal_loc(const void *new_ctx, const void *ptr, const char *locati
*/
const char *talloc_set_name(const void *ptr, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3);
+#ifdef DOXYGEN
+/**
+ * @brief Keep the memory secret when freeing.
+ *
+ * This can be used to define memory as secret. For example memory which holds
+ * passwords or other tokens. The memory will be zeroed before is being freed.
+ *
+ * If you duplicate memory, e.g. using talloc_strdup() or talloc_asprintf() you
+ * need to call talloc_keep_secret() on the newly allocated memory too!
+ *
+ * @param[in] ptr The talloc chunk to mark as secure.
+ *
+ * @warning Do not use this in combination with talloc_realloc().
+ */
+void talloc_keep_secret(const void *ptr);
+#else
+#define talloc_keep_secret(ptr) _talloc_keep_secret(ptr, #ptr);
+void _talloc_keep_secret(const void *ptr, const char *name);
+#endif
+
#ifdef DOXYGEN
/**
* @brief Change a talloc chunk's parent.
--
2.21.0
>From 48ec62880caae4b3656421f7512349868951f828 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 12 Oct 2018 11:16:15 +0200
Subject: [PATCH 5/8] talloc: Don't return the size of secret chunks
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/talloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index ba7ae9f32ab..515c78c8b6e 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -2146,7 +2146,7 @@ static inline size_t _talloc_total_mem_internal(const void *ptr,
return tc->limit->cur_size;
}
- if (tc->flags & TALLOC_FLAG_LOOP) {
+ if (tc->flags & (TALLOC_FLAG_LOOP|TALLOC_FLAG_SECRET)) {
return 0;
}
--
2.21.0
>From 1676d22db993cfb7506955464ed3cc335bab5dc1 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 11 Oct 2018 08:25:22 +0200
Subject: [PATCH 6/8] talloc: Add test for talloc_keep_secret()
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/tests/test_talloc_keep_secret.c | 97 ++++++++++++++++++++++
lib/talloc/wscript | 6 ++
selftest/tests.py | 6 ++
3 files changed, 109 insertions(+)
create mode 100644 lib/talloc/tests/test_talloc_keep_secret.c
diff --git a/lib/talloc/tests/test_talloc_keep_secret.c b/lib/talloc/tests/test_talloc_keep_secret.c
new file mode 100644
index 00000000000..3fd743aec94
--- /dev/null
+++ b/lib/talloc/tests/test_talloc_keep_secret.c
@@ -0,0 +1,97 @@
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <string.h>
+#include <talloc.h>
+
+int rep_memset_s(void *dest, size_t destsz, int ch, size_t count);
+
+int rep_memset_s(void *dest, size_t destsz, int ch, size_t count)
+{
+ check_expected_ptr(dest);
+ check_expected(destsz);
+ check_expected(ch);
+ check_expected(count);
+
+ return 0;
+}
+
+static void test_talloc_keep_secret(void ** state)
+{
+ TALLOC_CTX *pool = NULL;
+ char *ptr1 = NULL;
+ char *ptr2 = NULL;
+ const char *ptr1_talloc_name = NULL;
+ size_t ptr1_size;
+ size_t total_size;
+ size_t i;
+
+ pool = talloc_pool(NULL, 256);
+ assert_non_null(pool);
+
+ ptr1 = talloc_strdup(pool, "secret");
+ assert_non_null(ptr1);
+ assert_string_equal(ptr1, "secret");
+
+ talloc_keep_secret(ptr1);
+
+ ptr1_talloc_name = talloc_get_name(ptr1);
+ assert_string_equal(ptr1_talloc_name, "ptr1");
+
+ ptr1_size = talloc_get_size(ptr1);
+ assert_int_equal(ptr1_size, strlen(ptr1) + 1);
+
+ total_size = talloc_total_size(ptr1);
+ assert_int_equal(total_size, 0);
+
+ expect_string(rep_memset_s, dest, "secret");
+ expect_value(rep_memset_s, destsz, strlen(ptr1) + 1);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, strlen(ptr1) + 1);
+
+ talloc_free(ptr1);
+
+ ptr2 = talloc_size(pool, ptr1_size);
+ assert_ptr_equal(ptr1, ptr2);
+
+ for (i = 1; i < ptr1_size; i++) {
+ assert_int_not_equal(ptr2[0], ptr2[i]);
+ }
+
+ talloc_free(pool);
+}
+
+static void test_talloc_keep_secret_validate_memset(void **state)
+{
+ TALLOC_CTX *mem_ctx = NULL;
+ char *password = NULL;
+
+ mem_ctx = talloc_new(NULL);
+ assert_non_null(mem_ctx);
+
+ password = talloc_strdup(mem_ctx, "secret");
+ assert_non_null(password);
+ talloc_keep_secret(password);
+
+ expect_string(rep_memset_s, dest, "secret");
+ expect_value(rep_memset_s, destsz, strlen(password) + 1);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, strlen(password) + 1);
+
+ talloc_free(mem_ctx);
+}
+
+int main(void)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(test_talloc_keep_secret),
+ cmocka_unit_test(test_talloc_keep_secret_validate_memset),
+ };
+
+ cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+
+ return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/lib/talloc/wscript b/lib/talloc/wscript
index e4020692d60..beaa884a14d 100644
--- a/lib/talloc/wscript
+++ b/lib/talloc/wscript
@@ -124,6 +124,12 @@ def build(bld):
private_library=private_library,
manpages='man/talloc.3')
+ # We only check for cmocka in the Samba tree
+ bld.SAMBA_BINARY('test_talloc_keep_secret',
+ 'tests/test_talloc_keep_secret.c',
+ 'talloc cmocka',
+ install=False)
+
if not bld.CONFIG_SET('USING_SYSTEM_PYTALLOC_UTIL'):
name = bld.pyembed_libname('pytalloc-util')
diff --git a/selftest/tests.py b/selftest/tests.py
index 01afdaea2d0..f846cbf7191 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -46,6 +46,8 @@ have_man_pages_support = ("XSLTPROC_MANPAGES" in config_hash)
with_pam = ("WITH_PAM" in config_hash)
pam_wrapper_so_path = config_hash["LIBPAM_WRAPPER_SO_PATH"]
pam_set_items_so_path = config_hash["PAM_SET_ITEMS_SO_PATH"]
+with_internal_talloc = ("TALLOC_BUILD_VERSION_MAJOR" in config_hash)
+with_memset_s = ("HAVE_MEMSET_S" in config_hash)
planpythontestsuite("none", "samba.tests.source")
if have_man_pages_support:
@@ -246,6 +248,10 @@ plantestsuite("samba.unittests.lib_util_modules", "none",
plantestsuite("samba.unittests.smb1cli_session", "none",
[os.path.join(bindir(), "default/libcli/smb/test_smb1cli_session")])
+if with_internal_talloc and not with_memset_s:
+ plantestsuite("talloc.keep_secret", "none",
+ [os.path.join(bindir(), "default/lib/talloc/test_talloc_keep_secret")])
+
plantestsuite("samba.unittests.tldap", "none",
[os.path.join(bindir(), "default/source3/test_tldap")])
plantestsuite("samba.unittests.rfc1738", "none",
--
2.21.0
>From 8f8797d89c885cd68f0b0c31e0816ba28afd5852 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 12 Oct 2018 11:07:07 +0200
Subject: [PATCH 7/8] talloc: Handle TALLOC_FLAG_SECRET in talloc_realloc()
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/talloc.c | 63 ++++++++++++++++++++++++---------------------
1 file changed, 34 insertions(+), 29 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 515c78c8b6e..8d3e3f64149 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -54,11 +54,6 @@
#include <valgrind.h>
#endif
-/* use this to force every realloc to change the pointer, to stress test
- code that might not cope */
-#define ALWAYS_REALLOC 0
-
-
#define MAX_TALLOC_SIZE 0x10000000
#define TALLOC_FLAG_FREE 0x01
@@ -1855,7 +1850,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
pool_hdr = tc->pool;
}
-#if (ALWAYS_REALLOC == 0)
/* don't shrink if we have less than 1k to gain */
if (size < tc->size && tc->limit == NULL) {
if (pool_hdr) {
@@ -1875,6 +1869,9 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
* testing a lot :-(.
*
* That is why we only mark memory as undefined here.
+ *
+ * In case of secret memory, TC_UNDEFINE_SHRINK_CHUNK
+ * will zero it.
*/
TC_UNDEFINE_SHRINK_CHUNK(tc, size);
@@ -1889,7 +1886,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
*/
return ptr;
}
-#endif
/*
* by resetting magic we catch users of the old memory
@@ -1908,32 +1904,42 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
*/
_talloc_chunk_set_free(tc, NULL);
-#if ALWAYS_REALLOC
- if (pool_hdr) {
- new_ptr = tc_alloc_pool(tc, size + TC_HDR_SIZE, 0);
- pool_hdr->object_count--;
+ if (tc->flags & TALLOC_FLAG_SECRET) {
+ if (pool_hdr != NULL) {
+ new_ptr = tc_alloc_pool(tc, size + TC_HDR_SIZE, 0);
+ pool_hdr->object_count--;
- if (new_ptr == NULL) {
- new_ptr = malloc(TC_HDR_SIZE+size);
- malloced = true;
+ if (new_ptr == NULL) {
+ new_ptr = malloc(TC_HDR_SIZE+size);
+ malloced = true;
+ new_size = size;
+ }
+
+ if (new_ptr != NULL) {
+ memcpy(new_ptr, tc, MIN(tc->size,size) + TC_HDR_SIZE);
+ TC_INVALIDATE_FULL_CHUNK(tc);
+ }
+ } else {
+ /*
+ * We're doing malloc then free here, so record the
+ * difference.
+ */
+ old_size = tc->size;
new_size = size;
+ new_ptr = malloc(size + TC_HDR_SIZE);
+ if (new_ptr) {
+ memcpy(new_ptr, tc, MIN(tc->size, size) + TC_HDR_SIZE);
+ if (tc->flags & TALLOC_FLAG_SECRET) {
+ uint8_t *zero_ptr = (uint8_t *)TC_PTR_FROM_CHUNK(tc);
+ memset_s(zero_ptr, old_size, talloc_fill.fill_value, old_size);
+ }
+ free(tc);
+ }
}
- if (new_ptr) {
- memcpy(new_ptr, tc, MIN(tc->size,size) + TC_HDR_SIZE);
- TC_INVALIDATE_FULL_CHUNK(tc);
- }
- } else {
- /* We're doing malloc then free here, so record the difference. */
- old_size = tc->size;
- new_size = size;
- new_ptr = malloc(size + TC_HDR_SIZE);
- if (new_ptr) {
- memcpy(new_ptr, tc, MIN(tc->size, size) + TC_HDR_SIZE);
- free(tc);
- }
+ goto got_new_ptr;
}
-#else
+
if (pool_hdr) {
struct talloc_chunk *pool_tc;
void *next_tc = tc_next_chunk(tc);
@@ -2048,7 +2054,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
new_ptr = realloc(tc, size + TC_HDR_SIZE);
}
got_new_ptr:
-#endif
if (unlikely(!new_ptr)) {
/*
* Ok, this is a strange spot. We have to put back
--
2.21.0
>From f60c7001a521f7facff9edaf94186ba31b4c2178 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 12 Oct 2018 12:47:34 +0200
Subject: [PATCH 8/8] talloc: Add keep secret tests for talloc_realloc()
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/tests/test_talloc_keep_secret.c | 98 ++++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/lib/talloc/tests/test_talloc_keep_secret.c b/lib/talloc/tests/test_talloc_keep_secret.c
index 3fd743aec94..0faac8549ff 100644
--- a/lib/talloc/tests/test_talloc_keep_secret.c
+++ b/lib/talloc/tests/test_talloc_keep_secret.c
@@ -84,11 +84,109 @@ static void test_talloc_keep_secret_validate_memset(void **state)
talloc_free(mem_ctx);
}
+static void test_talloc_keep_secret_realloc_increase(void **state)
+{
+ TALLOC_CTX *mem_ctx = NULL;
+ char *password = NULL;
+ size_t pwdlen;
+
+ mem_ctx = talloc_new(NULL);
+ assert_non_null(mem_ctx);
+
+ password = talloc_strdup(mem_ctx, "secret");
+ assert_non_null(password);
+ talloc_keep_secret(password);
+
+ pwdlen = strlen(password);
+
+ expect_string(rep_memset_s, dest, "secret");
+ expect_value(rep_memset_s, destsz, pwdlen + 1);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, pwdlen + 1);
+
+ password = talloc_realloc(mem_ctx, password, char, strlen(password) + 2);
+ assert_non_null(password);
+
+ password[pwdlen + 1] = '\0';
+ password[pwdlen] = '1';
+
+ expect_string(rep_memset_s, dest, "secret1");
+ expect_value(rep_memset_s, destsz, pwdlen + 2);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, pwdlen + 2);
+
+ talloc_free(mem_ctx);
+}
+
+static void test_talloc_keep_secret_realloc_decrease(void **state)
+{
+ TALLOC_CTX *mem_ctx = NULL;
+ char *password = NULL;
+ size_t pwdlen;
+
+ mem_ctx = talloc_new(NULL);
+ assert_non_null(mem_ctx);
+
+ password = talloc_strdup(mem_ctx, "secret");
+ assert_non_null(password);
+ talloc_keep_secret(password);
+
+ pwdlen = strlen(password) + 1;
+
+ /* We decrease the memory by 2 bytes, we remove 't\0' */
+ expect_string(rep_memset_s, dest, "t");
+ expect_value(rep_memset_s, destsz, 2);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, 2);
+
+ password = talloc_realloc(mem_ctx, password, char, pwdlen - 2);
+ assert_non_null(password);
+
+ pwdlen -= 2;
+ password[pwdlen] = '\0';
+
+ expect_string(rep_memset_s, dest, "secre");
+ expect_value(rep_memset_s, destsz, pwdlen);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, pwdlen);
+
+ talloc_free(mem_ctx);
+}
+
+static void test_talloc_keep_secret_realloc_zero(void **state)
+{
+ TALLOC_CTX *mem_ctx = NULL;
+ char *password = NULL;
+ size_t pwdlen;
+
+ mem_ctx = talloc_new(NULL);
+ assert_non_null(mem_ctx);
+
+ password = talloc_strdup(mem_ctx, "secret");
+ assert_non_null(password);
+ talloc_keep_secret(password);
+
+ pwdlen = strlen(password);
+
+ expect_string(rep_memset_s, dest, "secret");
+ expect_value(rep_memset_s, destsz, pwdlen + 1);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, pwdlen + 1);
+
+ password = talloc_realloc(mem_ctx, password, char, 0);
+ assert_null(password);
+
+ talloc_free(mem_ctx);
+}
+
int main(void)
{
const struct CMUnitTest tests[] = {
cmocka_unit_test(test_talloc_keep_secret),
cmocka_unit_test(test_talloc_keep_secret_validate_memset),
+ cmocka_unit_test(test_talloc_keep_secret_realloc_increase),
+ cmocka_unit_test(test_talloc_keep_secret_realloc_decrease),
+ cmocka_unit_test(test_talloc_keep_secret_realloc_zero),
};
cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
--
2.21.0
More information about the samba-technical
mailing list