rgw: Verify key id before sending to Barbican

`request_key_from_barbican` is called with a raw user-defined key id.
To prevent issues like path injection match against a UUID4 regex
first. Add this check close to the Barbican calls, as other KMS
backends have other key format definitions.

Barbican secret ids are defined as "uuid" and matched against Python's
UUID 4 parser.

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

diff --git a/src/rgw/rgw_kms.cc b/src/rgw/rgw_kms.cc
index d9b7e8848a26edea352fbf138b6e3a5d33a77ea2..df71dcc7bfb25195c524dfdbb214d83ae01de2bf 100644 (file)
--- a/src/rgw/rgw_kms.cc
+++ b/src/rgw/rgw_kms.cc
@@ -18,6 +18,7 @@
 #include <rapidjson/writer.h>
 #include "rapidjson/error/error.h"
 #include "rapidjson/error/en.h"
+#include <regex>
 
 #define dout_context g_ceph_context
 #define dout_subsys ceph_subsys_rgw
@@ -119,6 +120,14 @@ static void concat_url(std::string &url, std::string path) {
   }
 }
 
+static bool validate_barbican_key_id(std::string_view key_id) {
+  // Barbican expects UUID4 secret ids.
+  // See barbican: common/utils.py, api/controllers/secrets.py
+  static const std::regex uuid_4_re{
+      R"(^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$)"};
+  return std::regex_match(key_id.data(), uuid_4_re);
+}
+
 /**
  * Determine if a string (url) ends with a given suffix.
  * Must deal with (ignore) trailing slashes.
@@ -919,6 +928,10 @@ static int request_key_from_barbican(const DoutPrefixProvider *dpp,
                                      const std::string& barbican_token,
                                      optional_yield y,
                                      std::string& actual_key) {
+  if (!validate_barbican_key_id(key_id)) {
+    return -EINVAL;
+  }
+
   int res;
 
   CephContext* cct = dpp->get_cct();

diff --git a/src/test/rgw/test_rgw_kms.cc b/src/test/rgw/test_rgw_kms.cc
index 409c56d2c3e1f242c13fb158b6906a87a7c71c38..af43fba48f8dd7a27c332f187c84c0472c984d83 100644 (file)
--- a/src/test/rgw/test_rgw_kms.cc
+++ b/src/test/rgw/test_rgw_kms.cc
@@ -1,6 +1,7 @@
 // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
 // vim: ts=8 sw=2 smarttab
 
+#include "gtest/gtest.h"
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 #include "common/ceph_context.h"
@@ -262,6 +263,27 @@ TEST_F(TestSSEKMS, concat_url)
   }
 }
 
+class BarbicanKeyIdValidationTest
+    : public ::testing::TestWithParam<std::pair<std::string_view, bool>> {};
+
+TEST_P(BarbicanKeyIdValidationTest, ValidateKeyId) {
+  const auto &param = GetParam();
+  EXPECT_EQ(validate_barbican_key_id(param.first), param.second);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    KeyIDTests, BarbicanKeyIdValidationTest,
+    ::testing::Values(
+        std::make_pair("asdf", false),
+        std::make_pair("cb6f82b2-aace-464f-bd50-c3103b97ad92", true),
+        std::make_pair("7cd71431-7f9b-5a2f-8215-126164bda0e4", true),
+        std::make_pair("{xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}", false),
+        std::make_pair("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", false),
+        std::make_pair("", false),
+        std::make_pair("../cb6f82b2-aace-464f-bd50-c3103b97ad92", false),
+        std::make_pair("/cb6f82b2-aace-464f-bd50-c3103b97ad92", false),
+        std::make_pair("cb6f82b2/aace../464f-bd50-c3103b97ad92", false),
+        std::make_pair(" ", false)));
 
 TEST_F(TestSSEKMS, string_ends_maybe_slash)
 {