From 13efe8c6b89686595556fc0c6f68528b1e9f01ef Mon Sep 17 00:00:00 2001
From: Marcel Lauhoff <marcel.lauhoff@clyso.com>
Date: Fri, 8 Aug 2025 12:52:06 +0200
Subject: [PATCH] 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
---
 src/rgw/rgw_kms.cc           | 13 +++++++++++++
 src/test/rgw/test_rgw_kms.cc | 22 ++++++++++++++++++++++
 2 files changed, 35 insertions(+)

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)
 {
-- 
2.47.3