summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKrzysztof Jackiewicz <k.jackiewicz@samsung.com>2019-02-14 15:30:48 +0100
committerKrzysztof Jackiewicz <k.jackiewicz@samsung.com>2019-02-19 12:52:36 +0100
commit2cb35737891323de4b0fea29e060958375f25c26 (patch)
tree4c41eaee7ba193bb5018a37b0534ffb9df69d52d
parent7765b5df357f4ad0203746eb6e99cd125d6da748 (diff)
downloadkey-manager-2cb35737891323de4b0fea29e060958375f25c26.tar.gz
key-manager-2cb35737891323de4b0fea29e060958375f25c26.tar.bz2
key-manager-2cb35737891323de4b0fea29e060958375f25c26.zip
Add a common function for zeroing sensitive data
Encryption keys and passwords are sensitive data and as such should be cleared when no longer used to prevent memory attacks. According to the "as-if" rule, the compiler is allowed to perform any changes to the program as long as the observable behavior of the program is not changed. Since the contents of unused memory are not considered an observable behavior the compiler is allowed to optimize out the call to memset(). The following solutions were considered: - Reading the memory after overwriting it with memset(). Since reading the memory has no observable effects it's perfectly legal for the compiler to remove both operations. - Using volatile asembly code to prevent optimization. It may prevent some compilers from optimizing but there's no guarantee. - Using volatile funtion pointer to memset. Apparently, it can be optimized as well during LTO. - Using memcpy_s(). The function is not widely available yet. It may be missing so we still need a fallback solution. - Locally disabling optimization with #pragma GCC optimize("O0"). It's GCC specific and it's not clear whether GCC will try to optimize it with "O0". Empirical test showed that memset() call is not removed. This commit applies the last solution adding a new unoptimized wrapper for memset(). Note that this commit will not prevent the processor from creating another copy of the sensitive data in registers, on the stack, in swap or in cache memory. It will only limit the number of places in memory where the secret data can be found. Change-Id: I80fe8ce8ce3d808b423858254d6fd23f491d2674
-rw-r--r--packaging/key-manager.spec1
-rw-r--r--src/CMakeLists.txt1
-rw-r--r--src/include/ckm/ckm-raw-buffer.h5
-rw-r--r--src/include/ckm/ckm-zero-memory.h32
-rw-r--r--src/manager/CMakeLists.txt1
-rw-r--r--src/manager/common/ckm-zero-memory.cpp42
-rw-r--r--src/manager/service/key-provider.cpp10
-rw-r--r--src/manager/service/ss-migrate.cpp1
8 files changed, 83 insertions, 10 deletions
diff --git a/packaging/key-manager.spec b/packaging/key-manager.spec
index c5f8212..3ed0478 100644
--- a/packaging/key-manager.spec
+++ b/packaging/key-manager.spec
@@ -321,6 +321,7 @@ fi
%{_includedir}/ckm/ckm/ckm-pkcs12.h
%{_includedir}/ckm/ckm/ckm-raw-buffer.h
%{_includedir}/ckm/ckm/ckm-type.h
+%{_includedir}/ckm/ckm/ckm-zero-memory.h
%{_includedir}/ckm/ckmc/ckmc-manager.h
%{_includedir}/ckm/ckmc/ckmc-control.h
%{_includedir}/ckm/ckmc/ckmc-error.h
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index ae5f83e..70dca66 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -243,6 +243,7 @@ INSTALL(FILES
${KEY_MANAGER_SRC_PATH}/include/ckm/ckm-pkcs12.h
${KEY_MANAGER_SRC_PATH}/include/ckm/ckm-raw-buffer.h
${KEY_MANAGER_SRC_PATH}/include/ckm/ckm-type.h
+ ${KEY_MANAGER_SRC_PATH}/include/ckm/ckm-zero-memory.h
DESTINATION /usr/include/ckm/ckm
)
INSTALL(FILES
diff --git a/src/include/ckm/ckm-raw-buffer.h b/src/include/ckm/ckm-raw-buffer.h
index 8ae908f..d9b41a7 100644
--- a/src/include/ckm/ckm-raw-buffer.h
+++ b/src/include/ckm/ckm-raw-buffer.h
@@ -23,9 +23,10 @@
#define _SAFE_BUFFER_H_
#include <stddef.h>
-#include <string.h>
#include <vector>
+#include <ckm/ckm-zero-memory.h>
+
namespace CKM {
template <typename T>
@@ -54,7 +55,7 @@ struct std_erase_on_dealloc {
void deallocate(T *ptr, std::size_t n)
{
// clear the memory before deleting
- memset(ptr, 0 , n * sizeof(T));
+ ZeroMemory(reinterpret_cast<unsigned char*>(ptr), n * sizeof(T));
::operator delete(ptr);
}
diff --git a/src/include/ckm/ckm-zero-memory.h b/src/include/ckm/ckm-zero-memory.h
new file mode 100644
index 0000000..bc1397c
--- /dev/null
+++ b/src/include/ckm/ckm-zero-memory.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2019 Samsung Electronics Co., Ltd All Rights Reserved
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+/*
+ * @file ckm-zero-memory.h
+ * @author Krzysztof Jackiewicz (k.jackiewicz@samsung.com)
+ * @version 1.0
+ * @brief
+ */
+
+#pragma once
+
+#include <cstddef>
+
+namespace CKM {
+
+void ZeroMemory(unsigned char* buffer, size_t size);
+
+} // namespace CKM
+
diff --git a/src/manager/CMakeLists.txt b/src/manager/CMakeLists.txt
index 2f071eb..53c572c 100644
--- a/src/manager/CMakeLists.txt
+++ b/src/manager/CMakeLists.txt
@@ -25,6 +25,7 @@ SET(COMMON_SOURCES
${COMMON_PATH}/common/key-aes-impl.cpp
${COMMON_PATH}/common/pkcs12-impl.cpp
${COMMON_PATH}/common/log-setup.cpp
+ ${COMMON_PATH}/common/ckm-zero-memory.cpp
${COMMON_PATH}/dpl/log/src/abstract_log_provider.cpp
${COMMON_PATH}/dpl/log/src/dlog_log_provider.cpp
${COMMON_PATH}/dpl/log/src/log.cpp
diff --git a/src/manager/common/ckm-zero-memory.cpp b/src/manager/common/ckm-zero-memory.cpp
new file mode 100644
index 0000000..70029a5
--- /dev/null
+++ b/src/manager/common/ckm-zero-memory.cpp
@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2019 Samsung Electronics Co., Ltd All Rights Reserved
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+/*
+ * @file ckm-zero-memory.cpp
+ * @author Krzysztof Jackiewicz (k.jackiewicz@samsung.com)
+ * @version 1.0
+ * @brief
+ */
+
+#include <ckm/ckm-zero-memory.h>
+
+#include <string.h>
+
+#include <symbol-visibility.h>
+
+namespace CKM {
+
+// Temporarily disable optimizations to make sure that memset() is not optimized out.
+#pragma GCC push_options
+#pragma GCC optimize("O0")
+
+COMMON_API void ZeroMemory(unsigned char* buffer, size_t size)
+{
+ memset(buffer, 0, size);
+}
+
+#pragma GCC pop_options
+
+} // namespace CKM
diff --git a/src/manager/service/key-provider.cpp b/src/manager/service/key-provider.cpp
index 29c8ee4..1891153 100644
--- a/src/manager/service/key-provider.cpp
+++ b/src/manager/service/key-provider.cpp
@@ -17,6 +17,7 @@
#include <exception.h>
#include <key-provider.h>
#include <dpl/log/log.h>
+#include <ckm/ckm-zero-memory.h>
#include <string.h>
#include <array>
@@ -296,14 +297,7 @@ void KeyAndInfoContainer::setKeyInfo(const KeyComponentsInfo *keyComponentsInfo)
KeyAndInfoContainer::~KeyAndInfoContainer()
{
// overwrite key
- char *ptr = reinterpret_cast<char *>(&keyAndInfo);
- memset(ptr, 0, sizeof(KeyAndInfo));
-
- // verification
- for (size_t size = 0; size < sizeof(KeyAndInfo); ++size) {
- if (ptr[size])
- LogError("Write memory error! Memory used by key was not owerwritten.");
- }
+ ZeroMemory(reinterpret_cast<unsigned char*>(&keyAndInfo), sizeof(KeyAndInfo));
}
KeyProvider::KeyProvider() :
diff --git a/src/manager/service/ss-migrate.cpp b/src/manager/service/ss-migrate.cpp
index 635c10c..cb446d9 100644
--- a/src/manager/service/ss-migrate.cpp
+++ b/src/manager/service/ss-migrate.cpp
@@ -28,6 +28,7 @@
#include <unistd.h>
#include <dirent.h>
#include <sys/stat.h>
+#include <string.h>
#include <dpl/log/log.h>
#include <ss-crypto.h>