From: Radoslaw Zarzynski <rzarzyns@redhat.com>
Date: Fri, 15 Nov 2019 15:39:21 +0000 (+0100)
Subject: common: introduce ceph::crypto::zeroize_for_security().
X-Git-Tag: v14.2.8~20^2~56^2~14
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=41caed22f66e471cc357aa208baf62ce4fed1408;p=ceph.git

common: introduce ceph::crypto::zeroize_for_security().

For the sake of compliance with FIPS memory where security
material (like keys) was stored, should be cleaned when it
isn't used anymore. This is intended to limit the impact of
other security problems allowing to inspect memory.

In many cases such sanitization is performed with `memset`
or `bzero` to zeroize the memory. However, C++ language,
due to the as-if rule [1], provides less guarantees than
necessary to ensure that a call to e.g. `memset` will be
always translated into intended stores. This isn't something
specific to it nor `bzero`. All a compiler needs to know
to perform dead store elimination is the code itself (to
exclude e.g. `volatile` fencing). Presumably it could assume
how the things from standard library look internally – like
with an inlineable function [2].

[1] https://en.cppreference.com/w/cpp/language/as_if
[2] https://godbolt.org/z/XJnAnA

The problem was already discussed in the GCC bug reports
which finally have been marked as "invalid":
  * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=8537,
  * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71388.

Because of that we want to perform the security clean-ups
with a dedicated procedure that takes responsibility of
prohibiting compilers from optimizing it out. OpenSSL
already provides such utility: `OPESNSSL_cleanse`. This
commit integrates it into Ceph's abstractions over crypto.

The intended clients are some current user of `memset` and
`bzero` found using the GCC's `deprecated` attribute. See:
https://gist.github.com/rzarzynski/db9b4ca6b3d409d2ab8d38f4d6678063.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
(cherry picked from commit c16266cec254f0ca0e0330f4bb84c3b53386e0ba)
---

diff --git a/src/common/ceph_crypto.cc b/src/common/ceph_crypto.cc
index c3e84e9a57b1..62fc94ea28cc 100644
--- a/src/common/ceph_crypto.cc
+++ b/src/common/ceph_crypto.cc
@@ -115,3 +115,27 @@ void ceph::crypto::ssl::OpenSSLDigest::Final(unsigned char *digest) {
   EVP_DigestFinal_ex(mpContext, digest, &s);
 }
 #endif /*USE_OPENSSL*/
+
+
+void ceph::crypto::zeroize_for_security(void* const s, const size_t n) {
+#ifdef USE_OPENSSL
+  // NSS lacks its own cleaning procedure that would be resilient to
+  // dead-store-elimination of nowadays compilers [1]. To avoid writing
+  // our own security code, let's always use the OpenSSL's one.
+  // [1]: "NSS [3.27.1] does not have a reliable memory scrubbing
+  //      implementation since it either calls memset or uses the macro
+  //      PORT_Memset, which expands to memset"
+  // https://klevchen.ece.illinois.edu/pubs/yjoll-usesec17.pdf, page 11.
+  OPENSSL_cleanse(s, n);
+#else
+  // OpenSSL is available even when NSS is turned on. The performance-
+  // critical Cephx's signature crafting machinery already follows this
+  // assumption and uses OpenSSL directly (see src/auth/Crypto.cc).
+  // Also, in CMakeList.txt we explicitly require both NSS and OpenSSL:
+  //
+  //  find_package(NSS REQUIRED)
+  //  find_package(NSPR REQUIRED)
+  //  find_package(OpenSSL REQUIRED)
+# error "No supported crypto implementation found."
+#endif /*USE_OPENSSL*/
+}
diff --git a/src/common/ceph_crypto.h b/src/common/ceph_crypto.h
index 30206c65702a..dda3306b2a60 100644
--- a/src/common/ceph_crypto.h
+++ b/src/common/ceph_crypto.h
@@ -41,6 +41,8 @@ namespace ceph {
     void assert_init();
     void init(CephContext *cct);
     void shutdown(bool shared=true);
+
+    void zeroize_for_security(void *s, size_t n);
   }
 }