Skip to content

Commit d7480c9

Browse files
YejneshwarYejneshwar
committed
Support for defaults in condition variables | Added parse tests with variables in policies
1 parent 4b757a9 commit d7480c9

File tree

2 files changed

+256
-43
lines changed

2 files changed

+256
-43
lines changed

‎src/rgw/rgw_iam_policy.cc‎

Lines changed: 67 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -56,21 +56,30 @@ using rapidjson::StringStream;
5656

5757
using rgw::auth::Principal;
5858

59-
//These are used during variable substitutions
60-
#if defined(__clang__)
61-
//TODO: Need to find the definition for clang
62-
#elif defined(__GNUC__) || defined(__GNUG__)
63-
using EnvRangeIterator = std::pair<std::__detail::_Node_iterator<std::pair<const std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >, false, true>, std::__detail::_Node_iterator<std::pair<const std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >, false, true> >;
64-
using ConstEnvRangeIterator = std::pair<std::__detail::_Node_const_iterator<std::pair<const std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >, false, true>,std::__detail::_Node_const_iterator<std::pair<const std::__cxx11::basic_string<char>, std::__cxx11::basic_string<char> >, false, true>>;
65-
#elif defined(_MSC_VER)
66-
using EnvRangeIterator = std::pair<std::_List_iterator<std::_List_val<std::_List_simple_types<std::pair<const std::string, std::string>>>>, std::_List_iterator<std::_List_val<std::_List_simple_types<std::pair<const std::string, std::string>>>>>;
67-
using ConstEnvRangeIterator = std::pair<std::_List_const_iterator<std::_List_val<std::_List_simple_types<std::pair<const std::string, std::string>>>>, std::_List_const_iterator<std::_List_val<std::_List_simple_types<std::pair<const std::string, std::string>>>>>;
68-
#endif
6959

7060
namespace rgw {
7161
namespace IAM {
7262
#include "rgw_iam_policy_keywords.frag.cc"
7363

64+
//These are used during variable substitutions
65+
using EnvRangeIterator = decltype(std::declval<std::unordered_multimap<std::string, std::string>>().equal_range(std::string()));
66+
using ConstEnvRangeIterator = decltype(std::declval<const std::unordered_multimap<std::string, std::string>>().equal_range(std::string()));
67+
68+
// The first capture group of the match is the full variable match
69+
// The second is the variable name
70+
// The third is the optional default value match
71+
// The fourth is the default value
72+
// For the match "${aws:user,'test'}"
73+
// Group 1 : "aws:user,'test'"
74+
// Group 2 : "aws:user"
75+
// Group 3 : ",'test'"
76+
// Group 4 : "test"
77+
// For the match "${aws:user}"
78+
// Group 1 : aws:user
79+
// Group 2 : aws:user
80+
//In anycase use group 2 as key to get the value from the environment
81+
static const std::regex varRx(R"(\$\{(([A-z-:1-9]*)(\,\s?\'(\S*)\')?)\})", std::regex_constants::ECMAScript | std::regex_constants::optimize);
82+
7483
struct actpair {
7584
const char* name;
7685
const uint64_t bit;
@@ -804,30 +813,28 @@ ostream& operator <<(ostream& m, const MaskedIP& ip) {
804813
return m;
805814
}
806815

807-
ConstEnvRangeIterator getEnvRangeFromMatch(const std::string& val, const std::smatch &match, const Environment& env) {
808-
std::string varKey = val.substr(match.position(), match.length());
809-
810-
varKey.erase(0, 2); //erase $, {
811-
varKey.erase(varKey.length() - 1, 1); //erase }
812-
return env.equal_range(varKey);
816+
ConstEnvRangeIterator getEnvRange(const std::string& Key, const Environment& env) {
817+
std::cout << "Key : " << Key << std::endl;
818+
return env.equal_range(Key);
813819
}
814820

815-
void generatrVarStrings(const Environment& env, const std::string& val, const std::vector<smatch>& matches, std::vector<string>& current, size_t index, std::vector<string>& varStrings) {
821+
void generateVarStrings(const Environment& env, const std::string& val, const std::vector<smatch>& matches, std::vector<string>& current, size_t index, std::vector<string>& varStrings) {
816822
if (index == matches.size()) {
817823
//Make a copy so we don't change the original
818824
std::string varString = val;
819825
//Substitute from the back since the string length will change
820826
for (auto i = matches.rbegin(); i != matches.rend(); ++i) {
821827
varString.replace(i->position(), i->length(), current.rbegin()[std::distance(matches.rbegin(), i)]);
822828
}
829+
replaceSpecialCharacters(varString);
823830
varStrings.push_back(varString);
824831
return;
825832
}
826833
const smatch& match = matches[index];
827-
const auto& range = getEnvRangeFromMatch(val, match, env);
834+
const auto& range = getEnvRange(match[2].str(), env);
828835
for (auto itr = range.first; itr != range.second; itr++) {
829836
current[index] = itr->second;
830-
generatrVarStrings(env, val, matches, current, index + 1, varStrings);
837+
generateVarStrings(env, val, matches, current, index + 1, varStrings);
831838
}
832839
}
833840

@@ -844,16 +851,18 @@ int calculateTotalCombinations(const std::vector<int>& lengths) {
844851
// Support "aws:RequestTag/" and "aws:ResourceTag/"
845852
// ToDo : Check if tag keys are added to the environment, if so, is the key as expected? eg. aws:ResourceTag/Department : TestDepartment
846853

847-
//Problem : Evaluates only the last element in array
848-
// eg. '{"Version":"2012-10-17","Statement":{"Effect":"Allow","Action":"s3:*","Resource":"arn:aws:s3:::*","Condition":{"StringEquals":{"s3:prefix":["folderinbucket","${aws:username}"]}}}}'
849-
// Evaluates only "${aws:username}"
850-
//regex match in 7 steps \$\{([A-z-:]*)\}
854+
// In cases like this :
855+
// "testfolder/${aws:nonExisting,'test'}/${aws:username}/thisstuff/${*}${aws:nonExisting,'testdefault'}"
856+
// notice how "aws:nonExisting" has two different defaults.
857+
// The condition will evaluate as :
858+
// "testfolder/test/${aws:username}/thisstuff/${*}testdefault"
859+
851860
bool Condition::eval(const Environment& env) const {
852-
static const std::regex rx(R"(\$\{([A-z-:1-9]*)\})",
853-
std::regex_constants::ECMAScript |
854-
std::regex_constants::optimize);
855861
std::vector<std::string> runtime_vals;
856-
dout(20) << "Evaluating condition for " << key << dendl;
862+
//Dont know why but the below dout would segfault "unittest_rgw_iam_policy"
863+
//`arn3 in TEST_F(PolicyTest, Eval3)`
864+
//printing the same with std::cout would not segfault and would print to the console
865+
// dout(20) << "Evaluating condition for " << key << dendl;
857866
auto i = env.find(key);
858867
if (op == TokenID::Null) {
859868
return i == env.end() ? true : false;
@@ -877,15 +886,34 @@ bool Condition::eval(const Environment& env) const {
877886
// ]}}
878887
// See "Evaluation logic for multiple context keys or values" in https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_condition-logic-multiple-context-keys-or-values.html
879888
if (isruntime) {
889+
std::vector<std::string> valsWithDefaults = vals;
880890
//In a lot of cases there is going to be only 1 match, 2 in some, 5 seems to be a good number for optimization purposes
881891
std::vector<smatch> matches;
882892
matches.reserve(5);
883-
static const std::regex rx(R"(\$\{([A-z-:1-9]*)\})", std::regex_constants::ECMAScript | std::regex_constants::optimize);
884893

885-
for (std::string val : vals)
894+
//makesure all keys exist in the environment - replacing them with defaults later is a pain
895+
for (std::string& val : valsWithDefaults) {
896+
for (std::sregex_iterator Iterator{ val.begin(), val.end(), varRx }; Iterator != std::sregex_iterator(); ++Iterator) {
897+
std::smatch res = *Iterator;
898+
if (res[3].matched) {
899+
auto range = getEnvRange(res[2].str(), env);
900+
auto numberOfValues = std::distance(range.first, range.second);
901+
if (numberOfValues == 0) {
902+
std::cout << "Replacing " << res.str() << " with default vlaue " << res[4].str() << std::endl;
903+
val.replace(res.position(), res.length(), res[4].str());
904+
// Resetting std::sregex_iterator to the beginning
905+
Iterator = std::sregex_iterator(val.begin(), val.end(), varRx);
906+
}
907+
908+
}
909+
910+
}
911+
}
912+
913+
for (std::string val : valsWithDefaults)
886914
{
887915
matches.clear();
888-
for (std::sregex_iterator Iterator{val.begin(), val.end(), rx}; Iterator != std::sregex_iterator(); ++Iterator)
916+
for (std::sregex_iterator Iterator{val.begin(), val.end(), varRx}; Iterator != std::sregex_iterator(); ++Iterator)
889917
{
890918
std::smatch res = *Iterator;
891919
dout(20) << "Found variable match in condition : " << res.str() << " " << res.position() << " " << res.length() << dendl;
@@ -900,7 +928,7 @@ bool Condition::eval(const Environment& env) const {
900928

901929
const auto &match = matches[0];
902930

903-
const auto &it = getEnvRangeFromMatch(val, match, env);
931+
const auto &it = getEnvRange(match[2].str(), env);
904932
for (auto itr = it.first; itr != it.second; itr++)
905933
{
906934
std::string varString = val;
@@ -919,7 +947,7 @@ bool Condition::eval(const Environment& env) const {
919947
// find variable keys and their ranges in the environment
920948
for (const smatch &match : matches)
921949
{
922-
ranges.emplace_back(getEnvRangeFromMatch(val, match, env));
950+
ranges.emplace_back(getEnvRange(match[2].str(), env));
923951
}
924952

925953
std::generate(lengths.begin(), lengths.end(), [i = int(-1), ranges]() mutable
@@ -928,11 +956,11 @@ bool Condition::eval(const Environment& env) const {
928956
return std::distance(ranges[i].first, ranges[i].second); });
929957

930958
//Make sure all variables are availabe in the environment
931-
auto lengthsIt = std::find_if(lengths.begin(), lengths.end(), [](int i)
932-
{ return i == 0; });
959+
auto lengthsIt = std::find_if(lengths.begin(), lengths.end(), [](int i) { return i == 0; });
960+
933961
if (lengthsIt != std::end(lengths))
934962
{
935-
dout(0) << "The key for the variable " << matches[std::distance(lengths.begin(), lengthsIt)].str() << " doesnot exist in the environment" << dendl;
963+
dout(0) << "The key for the variable match " << matches[std::distance(lengths.begin(), lengthsIt)].str() << " does not exist in the environment" << dendl;
936964
return false;
937965
}
938966
int totalCombinations = calculateTotalCombinations(lengths);
@@ -950,7 +978,7 @@ bool Condition::eval(const Environment& env) const {
950978

951979
std::vector<std::string> current(matches.size());
952980

953-
generatrVarStrings(env, val, matches, current, 0, runtime_vals);
981+
generateVarStrings(env, val, matches, current, 0, runtime_vals);
954982
}
955983
}
956984
}
@@ -1281,15 +1309,11 @@ Effect Statement::eval(const Environment& e,
12811309
if (!std::any_of(resource.begin(), resource.end(),
12821310
[&res,&e](const ARN& pattern) {
12831311
std::vector<smatch> matches;
1284-
// the first capture group of the match is the variable
1285-
// the second is the optional default value
1286-
// the third is the default value
1287-
static const std::regex rx(R"(\$\{([A-z-:1-9]*(\,\s?\'(\S*)\')?)\})", std::regex_constants::ECMAScript | std::regex_constants::optimize);
12881312

1289-
for (std::sregex_iterator Iterator{pattern.resource.begin(), pattern.resource.end(), rx}; Iterator != std::sregex_iterator(); ++Iterator)
1313+
for (std::sregex_iterator Iterator{pattern.resource.begin(), pattern.resource.end(), varRx}; Iterator != std::sregex_iterator(); ++Iterator)
12901314
{
12911315
std::smatch res = *Iterator;
1292-
dout(20) << "Found variable match in condition : " << res.str() << " " << res.position() << " " << res.length() << dendl;
1316+
dout(20) << "Found variable match in resource : " << res.str() << " " << res.position() << " " << res.length() << dendl;
12931317
matches.emplace_back(res);
12941318
}
12951319

@@ -1299,7 +1323,7 @@ Effect Statement::eval(const Environment& e,
12991323
std::string varString = pattern.resource;
13001324
for(auto it = matches.rbegin(); it != matches.rend(); ++it){
13011325

1302-
auto search = e.find(it->str(1));
1326+
auto search = e.find(it->str(2));
13031327

13041328
if (search == e.end() && it->operator[](2).matched) {
13051329
dout(0) << "Found no value for the variable " << it->str() << " in the environment" << dendl;

0 commit comments

Comments
 (0)