From: Marcel Lauhoff <marcel.lauhoff@clyso.com>
Date: Fri, 8 Aug 2025 10:52:06 +0000 (+0200)
Subject: rgw: Verify key id before sending to Barbican
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=13efe8c6b89686595556fc0c6f68528b1e9f01ef;p=ceph.git

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 d9b7e8848a2..df71dcc7bfb 100644
--- 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 409c56d2c3e..af43fba48f8 100644
--- 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)
 {