Add a common function for zeroing sensitive data 14/199814/3
authorKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Thu, 14 Feb 2019 14:30:48 +0000 (15:30 +0100)
committerKrzysztof Jackiewicz <k.jackiewicz@samsung.com>
Tue, 19 Feb 2019 11:52:36 +0000 (12:52 +0100)
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

packaging/key-manager.spec
src/CMakeLists.txt
src/include/ckm/ckm-raw-buffer.h
src/include/ckm/ckm-zero-memory.h [new file with mode: 0644]
src/manager/CMakeLists.txt
src/manager/common/ckm-zero-memory.cpp [new file with mode: 0644]
src/manager/service/key-provider.cpp
src/manager/service/ss-migrate.cpp

index c5f8212..3ed0478 100644 (file)
@@ -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
index ae5f83e..70dca66 100644 (file)
@@ -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
index 8ae908f..d9b41a7 100644 (file)
 #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 (file)
index 0000000..bc1397c
--- /dev/null
@@ -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
+
index 2f071eb..53c572c 100644 (file)
@@ -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 (file)
index 0000000..70029a5
--- /dev/null
@@ -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
index 29c8ee4..1891153 100644 (file)
@@ -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() :
index 635c10c..cb446d9 100644 (file)
@@ -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>