common: Refactor LinuxKeyringSecret into Keyring Interface

Goal: Support multiple backends and faking / mocking for testing.

Add abstract classes Keyring (factory) and KeyringSecret. Add
"Unsupported" implementation for non-Linux platforms. Add a get_best
factory function that currently returns the LinuxKeyring impl on Linux
or Unsupported elsewhere.

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>
On-behalf-of: SAP marcel.lauhoff@sap.com

diff --git a/src/common/keyring.cc b/src/common/keyring.cc
index 860ec91ae9f88d4e635600895fb95ef0212eb3df..b6823753efb8ffd97bc844c3c6300d163937ca11 100644 (file)
--- a/src/common/keyring.cc
+++ b/src/common/keyring.cc
@@ -15,7 +15,9 @@
 
 #include "keyring.h"
 
-#ifndef _WIN32
+#include <memory>
+
+#if defined(__linux__)
 extern "C" {
 #include <keyutils.h>
 }
@@ -23,38 +25,41 @@ extern "C" {
 
 namespace ceph {
 
-#ifdef _WIN32
-
-LinuxKeyringSecret::LinuxKeyringSecret(key_serial_t serial, size_t len) noexcept
-    : _len(len), _serial(serial) {}
-
-LinuxKeyringSecret::~LinuxKeyringSecret() noexcept {}
+std::unique_ptr<Keyring> Keyring::get_best() {
+#if defined(__linux__)
+  return std::make_unique<LinuxKeyring>();
+#else
+  return std::make_unique<UnsupportedKeyring>();
+#endif
+}
 
-tl::expected<LinuxKeyringSecret, std::error_code> LinuxKeyringSecret::add(
+tl::expected<std::unique_ptr<KeyringSecret>, std::error_code>
+UnsupportedKeyring::add(
     const std::string& key, const std::string& secret) noexcept {
   return tl::unexpected(std::error_code(-ENOSYS, std::system_category()));
 }
 
-bool LinuxKeyringSecret::supported() noexcept {
+bool UnsupportedKeyring::supported(std::error_code* ec) noexcept {
+  if (ec != nullptr) {
+    *ec = {-ENOSYS, std::system_category()};
+  }
   return false;
 }
 
-void LinuxKeyringSecret::initialize_process_keyring() noexcept {
-}
-
-[[nodiscard]] std::error_code LinuxKeyringSecret::read(std::string& out) const {
+[[nodiscard]] std::error_code UnsupportedKeyringSecret::read(
+    std::string& out) const {
   return {-ENOSYS, std::system_category()};
 }
 
-[[nodiscard]] std::error_code LinuxKeyringSecret::remove() const {
+[[nodiscard]] std::error_code UnsupportedKeyringSecret::remove() const {
   return {-ENOSYS, std::system_category()};
 }
 
-[[nodiscard]] std::error_code LinuxKeyringSecret::reset() {
-  return {-ENOSYS, std::system_category()};
+[[nodiscard]] bool UnsupportedKeyringSecret::initialized() const {
+  return false;
 }
 
-#else
+#if defined(__linux__)
 
 LinuxKeyringSecret::LinuxKeyringSecret(key_serial_t serial, size_t len) noexcept
     : _len(len), _serial(serial) {}
@@ -63,7 +68,7 @@ LinuxKeyringSecret::~LinuxKeyringSecret() noexcept {
   reset();
 }
 
-tl::expected<LinuxKeyringSecret, std::error_code> LinuxKeyringSecret::add(
+tl::expected<std::unique_ptr<KeyringSecret>, std::error_code> LinuxKeyring::add(
     const std::string& key, const std::string& secret) noexcept {
   const auto serial = add_key(
       "user", key.c_str(), secret.c_str(), secret.size(),
@@ -71,25 +76,28 @@ tl::expected<LinuxKeyringSecret, std::error_code> LinuxKeyringSecret::add(
   if (serial == -1) {
     return tl::unexpected(std::error_code(errno, std::system_category()));
   }
-  return LinuxKeyringSecret(serial, secret.size());
+  return std::unique_ptr<KeyringSecret>(
+      new LinuxKeyringSecret(serial, secret.size()));
 }
 
-bool LinuxKeyringSecret::supported(std::error_code* ec) noexcept {
-  auto secret = LinuxKeyringSecret::add("ceph_test_keyring_support", "ceph");
-  if (!secret) {
+bool LinuxKeyring::supported(std::error_code* ec) noexcept {
+  LinuxKeyring keyring;
+  auto maybe_secret = keyring.add("ceph_test_keyring_support", "ceph");
+  if (!maybe_secret) {
     if (ec != nullptr) {
-      *ec = secret.error();
+      *ec = maybe_secret.error();
     }
     return false;
   }
   std::string out;
-  if (auto err = secret.value().read(out); err) {
+  auto* secret = static_cast<LinuxKeyringSecret*>(maybe_secret.value().get());
+  if (auto err = secret->read(out); err) {
     if (ec != nullptr) {
       *ec = err;
     }
     return false;
   }
-  if (auto err = secret.value().reset(); err) {
+  if (auto err = secret->reset(); err) {
     if (ec != nullptr) {
       *ec = err;
     }

diff --git a/src/common/keyring.h b/src/common/keyring.h
index 63fe29d16f5c614b4363659cbabdbda50b39bc20..48e1ee59a99cefe9df3293962c10b76ee7d7f9e5 100644 (file)
--- a/src/common/keyring.h
+++ b/src/common/keyring.h
@@ -14,6 +14,7 @@
 
 #pragma once
 
+#include <memory>
 #include <ostream>
 #include <system_error>
 #include <utility>
@@ -23,14 +24,62 @@
 
 namespace ceph {
 
+class KeyringSecret {
+ public:
+  virtual ~KeyringSecret() noexcept = default;
+  [[nodiscard]] virtual std::error_code read(std::string& out) const = 0;
+  [[nodiscard]] virtual std::error_code remove() const = 0;
+  [[nodiscard]] virtual bool initialized() const = 0;
+  friend std::ostream& operator<<(
+      std::ostream& os, const KeyringSecret& secret) {
+    return os;
+  }
+};
+
+class Keyring {
+ public:
+  virtual ~Keyring() noexcept = default;
+  virtual tl::expected<std::unique_ptr<KeyringSecret>, std::error_code> add(
+      const std::string& key, const std::string& secret) noexcept = 0;
+  virtual bool supported(std::error_code* ec) noexcept = 0;
+  [[nodiscard]] virtual std::string_view name() const noexcept = 0;
+
+  static std::unique_ptr<Keyring> get_best();
+};
+
+class UnsupportedKeyringSecret : public KeyringSecret {
+ public:
+  ~UnsupportedKeyringSecret() noexcept override = default;
+  [[nodiscard]] std::error_code read(std::string& out) const override;
+  [[nodiscard]] std::error_code remove() const override;
+  [[nodiscard]] bool initialized() const override;
+  friend std::ostream& operator<<(
+      std::ostream& os, const UnsupportedKeyringSecret& secret) {
+    os << "UnsupportedKeyringSecret{}";
+    return os;
+  }
+};
+
+class UnsupportedKeyring : public Keyring {
+ public:
+  ~UnsupportedKeyring() noexcept override = default;
+  tl::expected<std::unique_ptr<KeyringSecret>, std::error_code> add(
+      const std::string& key, const std::string& secret) noexcept override;
+  bool supported(std::error_code* ec) noexcept override;
+  [[nodiscard]] std::string_view name() const noexcept override {
+    return "Unsupported";
+  }
+};
+
 /// RAII style wrapper around a secret stored in the Linux Key
 /// Retention Service.
 /// See: add_key(2), keyctl_read(3), keyctl_invalidate(3)
-class LinuxKeyringSecret {
+class LinuxKeyringSecret : public KeyringSecret {
   size_t _len;
   int _serial;
 
   explicit LinuxKeyringSecret(int serial, size_t len) noexcept;
+
  public:
   LinuxKeyringSecret() = delete;
   LinuxKeyringSecret(const LinuxKeyringSecret&) = delete;
@@ -38,10 +87,11 @@ class LinuxKeyringSecret {
   LinuxKeyringSecret(LinuxKeyringSecret&& other) noexcept
       : _len(std::exchange(other._len, 0)),
         _serial(std::exchange(other._serial, -1)) {};
+
   LinuxKeyringSecret& operator=(LinuxKeyringSecret&& other) noexcept {
     if (this != &other) {
       if (this->initialized()) {
-       this->reset();
+        this->reset();
       }
       _len = std::exchange(other._len, 0);
       _serial = std::exchange(other._serial, -1);
@@ -49,21 +99,17 @@ class LinuxKeyringSecret {
     return *this;
   };
 
-  ~LinuxKeyringSecret() noexcept;
+  ~LinuxKeyringSecret() noexcept override;
 
-  // Add a secret to the kernel process keyring. Beware: calling this
-  // with with an existing key _updates_ the value and causes multiple
-  // instances to refer to the same key.
-  static tl::expected<LinuxKeyringSecret, std::error_code> add(
-      const std::string& key, const std::string& secret) noexcept;
-  static bool supported(std::error_code* ec = nullptr) noexcept;
   // Initialize the process keyring. Do this before starting any
   // threads that want to share possession of keys in the process
   // keyring
   static void initialize_process_keyring() noexcept;
-  [[nodiscard]] std::error_code read(std::string& out) const;
-  [[nodiscard]] std::error_code remove() const;
-  [[nodiscard]] bool initialized() const;
+
+  [[nodiscard]] std::error_code read(std::string& out) const override;
+  [[nodiscard]] std::error_code remove() const override;
+  [[nodiscard]] bool initialized() const override;
+
  private:
   std::error_code reset();
 
@@ -75,10 +121,28 @@ class LinuxKeyringSecret {
     return os << "LinuxKeyringSecret{" << secret._serial << "}";
   }
   friend class LinuxKeyringTest_LifecycleMoveAssignResetsDestination_Test;
+  friend class LinuxKeyring;
+};
+
+class LinuxKeyring : public Keyring {
+ public:
+  // Add a secret to the kernel process keyring. Beware: calling this
+  // with with an existing key _updates_ the value and causes multiple
+  // instances to refer to the same key.
+  tl::expected<std::unique_ptr<KeyringSecret>, std::error_code> add(
+      const std::string& key, const std::string& secret) noexcept override;
+  bool supported(std::error_code* ec) noexcept override;
+  [[nodiscard]] std::string_view name() const noexcept override {
+    return "Linux Kernel Key Retention Service";
+  };
+
+  ~LinuxKeyring() override = default;
 };
 }  // namespace ceph
 
 #if FMT_VERSION >= 90000
-template <>
-struct fmt::formatter<ceph::LinuxKeyringSecret> : fmt::ostream_formatter {};
+template <typename T>
+struct fmt::formatter<
+    T, std::enable_if_t<std::is_base_of_v<ceph::KeyringSecret, T>, char>>
+    : fmt::ostream_formatter {};
 #endif

diff --git a/src/test/common/test_keyring.cc b/src/test/common/test_keyring.cc
index ced857e9777270c1433e5077a51e3f3f5ff03b65..1c591c5146737b9871e4532313792cf52611d0b4 100644 (file)
--- a/src/test/common/test_keyring.cc
+++ b/src/test/common/test_keyring.cc
@@ -9,9 +9,13 @@ namespace ceph {
 
 class LinuxKeyringTest : public ::testing::Test {
  protected:
+  std::unique_ptr<Keyring> keyring;
+
+  LinuxKeyringTest() : keyring(new LinuxKeyring()) {}
+
   void SetUp() override {
     std::error_code ec;
-    if (!LinuxKeyringSecret::supported(&ec)) {
+    if (!keyring->supported(&ec)) {
       GTEST_SKIP() << "Linux Keyring is unsupported. " << ec
                    << ". Skipping test";
     }
@@ -20,37 +24,39 @@ class LinuxKeyringTest : public ::testing::Test {
 
 TEST_F(LinuxKeyringTest, Basics) {
   std::string secret("secret");
-  auto maybe_keyring_secret = LinuxKeyringSecret::add("testkey", secret);
+  auto maybe_keyring_secret = keyring->add("testkey", secret);
 
   ASSERT_TRUE(maybe_keyring_secret.has_value()) << maybe_keyring_secret.error();
   auto keyring_secret = std::move(maybe_keyring_secret.value());
 
   std::string out;
-  ASSERT_FALSE(keyring_secret.read(out));
+  ASSERT_FALSE(keyring_secret->read(out));
   ASSERT_EQ(secret, out);
 
-  ASSERT_FALSE(keyring_secret.remove());
-  ASSERT_TRUE(keyring_secret.read(out));
+  ASSERT_FALSE(keyring_secret->remove());
+  ASSERT_TRUE(keyring_secret->read(out));
 }
 
 TEST_F(LinuxKeyringTest, Lifecycle) {
   std::string secret("secret");
-  auto uut = LinuxKeyringSecret::add("testkey", secret);
-  ASSERT_TRUE(uut.has_value());
-  ASSERT_TRUE(uut->initialized());
-  auto next = std::move(uut);
-  ASSERT_FALSE(uut->initialized());
-  ASSERT_TRUE(next->initialized());
+  auto maybe = keyring->add("testkey", secret);
+  ASSERT_TRUE(maybe.has_value());
+  auto* ptr = dynamic_cast<LinuxKeyringSecret*>(maybe.value().get());
+  ASSERT_TRUE(ptr->initialized());
+  auto next = std::move(*ptr);
+  ASSERT_TRUE(next.initialized());
+  ASSERT_FALSE(ptr->initialized());
 }
 
 TEST_F(LinuxKeyringTest, LifecycleMoveAssignResetsDestination) {
   std::string secret("secret");
-  auto dest = LinuxKeyringSecret::add("testkey", secret);
-  auto source = LinuxKeyringSecret::add("testkey2", secret);
+  auto dest = keyring->add("testkey", secret);
+  auto source = keyring->add("testkey2", secret);
   ASSERT_TRUE(dest.has_value());
   ASSERT_TRUE(source.has_value());
 
-  auto dest_serial = dest->_serial;
+  const auto* lks = dynamic_cast<LinuxKeyringSecret*>(dest.value().get());
+  auto dest_serial = lks->_serial;
   ASSERT_NE(-1, dest_serial);
 
   dest = std::move(source);