Skip to content

Commit 13efe8c

Browse files
committed
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 <[email protected]> On-behalf-of: SAP [email protected]
1 parent 07ab7ec commit 13efe8c

File tree

2 files changed

+35
-0
lines changed

2 files changed

+35
-0
lines changed

src/rgw/rgw_kms.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <rapidjson/writer.h>
1919
#include "rapidjson/error/error.h"
2020
#include "rapidjson/error/en.h"
21+
#include <regex>
2122

2223
#define dout_context g_ceph_context
2324
#define dout_subsys ceph_subsys_rgw
@@ -119,6 +120,14 @@ static void concat_url(std::string &url, std::string path) {
119120
}
120121
}
121122

123+
static bool validate_barbican_key_id(std::string_view key_id) {
124+
// Barbican expects UUID4 secret ids.
125+
// See barbican: common/utils.py, api/controllers/secrets.py
126+
static const std::regex uuid_4_re{
127+
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}$)"};
128+
return std::regex_match(key_id.data(), uuid_4_re);
129+
}
130+
122131
/**
123132
* Determine if a string (url) ends with a given suffix.
124133
* Must deal with (ignore) trailing slashes.
@@ -919,6 +928,10 @@ static int request_key_from_barbican(const DoutPrefixProvider *dpp,
919928
const std::string& barbican_token,
920929
optional_yield y,
921930
std::string& actual_key) {
931+
if (!validate_barbican_key_id(key_id)) {
932+
return -EINVAL;
933+
}
934+
922935
int res;
923936

924937
CephContext* cct = dpp->get_cct();

src/test/rgw/test_rgw_kms.cc

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
22
// vim: ts=8 sw=2 smarttab
33

4+
#include "gtest/gtest.h"
45
#include <gtest/gtest.h>
56
#include <gmock/gmock.h>
67
#include "common/ceph_context.h"
@@ -262,6 +263,27 @@ TEST_F(TestSSEKMS, concat_url)
262263
}
263264
}
264265

266+
class BarbicanKeyIdValidationTest
267+
: public ::testing::TestWithParam<std::pair<std::string_view, bool>> {};
268+
269+
TEST_P(BarbicanKeyIdValidationTest, ValidateKeyId) {
270+
const auto &param = GetParam();
271+
EXPECT_EQ(validate_barbican_key_id(param.first), param.second);
272+
}
273+
274+
INSTANTIATE_TEST_SUITE_P(
275+
KeyIDTests, BarbicanKeyIdValidationTest,
276+
::testing::Values(
277+
std::make_pair("asdf", false),
278+
std::make_pair("cb6f82b2-aace-464f-bd50-c3103b97ad92", true),
279+
std::make_pair("7cd71431-7f9b-5a2f-8215-126164bda0e4", true),
280+
std::make_pair("{xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}", false),
281+
std::make_pair("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", false),
282+
std::make_pair("", false),
283+
std::make_pair("../cb6f82b2-aace-464f-bd50-c3103b97ad92", false),
284+
std::make_pair("/cb6f82b2-aace-464f-bd50-c3103b97ad92", false),
285+
std::make_pair("cb6f82b2/aace../464f-bd50-c3103b97ad92", false),
286+
std::make_pair(" ", false)));
265287

266288
TEST_F(TestSSEKMS, string_ends_maybe_slash)
267289
{

0 commit comments

Comments
 (0)