You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "WHBANG (via GitHub)" <gi...@apache.org> on 2023/02/22 04:49:38 UTC
[GitHub] [incubator-pegasus] WHBANG opened a new pull request, #1360: feat(security): Use Apache Ranger for access control(1/8)
WHBANG opened a new pull request, #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360
### What problem does this PR solve? <!--add issue link with summary if exists-->
https://github.com/apache/incubator-pegasus/issues/1051
This patch aim to define ranger policy:
- Define the ranger policy data structure.
- Realized the judgment logic function when performing ACL with different priorities.
- Add some unit test for test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/8)
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1117881837
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -0,0 +1,79 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "ranger_resource_policy.h"
+
+namespace dsn {
+namespace ranger {
+
+bool policy_item::match(const access_type &ac_type, const std::string &user_name) const
+{
+ return (access_types & ac_type) != 0 && users.count(user_name) != 0;
+}
+
+bool acl_policies::allowed(const access_type &ac_type, const std::string &user_name) const
+{
+ // 1. Check if it is not allowed.
+ for (const auto &deny_policy : deny_policies) {
+ // 1.1. In 'deny_policies'.
+ if (!deny_policy.match(ac_type, user_name)) {
+ continue;
+ }
+ bool in_deny_policies_exclude = false;
+ for (const auto &deny_policy_exclude : deny_policies_exclude) {
+ if (deny_policy_exclude.match(ac_type, user_name)) {
+ in_deny_policies_exclude = true;
+ break;
+ }
+ }
+ // 1.2. Not in any 'deny_policies_exclude', it's not allowed.
+ if (!in_deny_policies_exclude) {
+ return false;
+ }
+ }
+
+ // 2. Check if it is allowed.
+ for (const auto &allow_policy : allow_policies) {
+ // 2.1. In 'allow_policies'.
+ if (!allow_policy.match(ac_type, user_name)) {
+ continue;
+ }
+ for (const auto &allow_policy_exclude : allow_policies_exclude) {
+ // 2.2. In some 'allow_policies_exclude', it's not allowed.
+ if (allow_policy_exclude.match(ac_type, user_name)) {
+ return false;
+ }
+ }
+ // 2.3. Not in any 'allow_policies_exclude', it's allowed.
+ return true;
+ }
+
+ // 3. Otherwise, it's not allowed.
+ return false;
+}
+
+void ranger_resource_policy::create_default_database_policy(ranger_resource_policy &acl)
+{
+ acl.name = "default database policy";
+ acl.database_names = {"*"};
+ policy_item item;
+ item.access_types = 127;
Review Comment:
How about use `READ | WRITE | CREATE | ...` instead of `127`, it could improve the readability.
##########
src/runtime/test/ranger_resource_policy_test.cpp:
##########
@@ -0,0 +1,75 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include "runtime/ranger/ranger_resource_policy.h"
+
+namespace dsn {
+namespace ranger {
+
+TEST(ranger_resource_policy_test, policy_item_match)
+{
+ policy_item item = {7, {"user1", "user2"}};
Review Comment:
Avoid to use magic number.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 merged pull request #1360: feat(security): Use Apache Ranger for access control(1/n)
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 merged PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/8)
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1117881492
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
+{
+ INVALID = 0,
+ READ = 1,
+ WRITE = 1 << 1,
+ CREATE = 1 << 2,
+ DROP = 1 << 3,
+ LIST = 1 << 4,
+ METADATA = 1 << 5,
+ CONTROL = 1 << 6,
+ ALL = 1 << 7,
Review Comment:
When will `INVALID` and `ALL` be used? Are they necessary? Is it correct to set all bits for `ALL` but not only 1 bit?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/n)
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1119067815
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,85 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <unordered_set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum class access_type : int8_t
Review Comment:
```suggestion
enum class access_type : uint8_t
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/8)
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1117882086
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
+{
+ INVALID = 0,
+ READ = 1,
+ WRITE = 1 << 1,
+ CREATE = 1 << 2,
+ DROP = 1 << 3,
+ LIST = 1 << 4,
+ METADATA = 1 << 5,
+ CONTROL = 1 << 6,
+ ALL = 1 << 7,
+};
+
+// Ranger policy data structure
+struct policy_item
+{
+ int8_t access_types;
+ std::set<std::string> users;
Review Comment:
How many user typically in a policy? Would it better to use unordered_set?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/8)
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1116564295
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,112 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
+{
+ READ = 0,
+ WRITE,
+ CREATE,
+ DROP,
+ LIST,
+ METADATA,
+ CONTROL,
+ ALL,
+ INVALID,
+};
+
+ENUM_BEGIN(access_type, INVALID)
+ENUM_REG(READ)
+ENUM_REG(WRITE)
+ENUM_REG(CREATE)
+ENUM_REG(DROP)
+ENUM_REG(LIST)
+ENUM_REG(METADATA)
+ENUM_REG(CONTROL)
+ENUM_REG(ALL)
+ENUM_END(access_type)
+
+ENUM_TYPE_SERIALIZATION(access_type, INVALID)
+
+// Ranger policy data structure
+struct policy_item
+{
+ std::set<access_type> access_types;
+ std::set<std::string> users;
+
+ DEFINE_JSON_SERIALIZATION(access_types, users);
+
+ // Match a policy according to acl_type and user_name
Review Comment:
```suggestion
// Check if the 'acl_type' - 'user_name' pair is matched to the policy.
// Return true if it is matched, otherwise return false.
```
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,112 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
+{
+ READ = 0,
+ WRITE,
+ CREATE,
+ DROP,
+ LIST,
+ METADATA,
+ CONTROL,
+ ALL,
+ INVALID,
+};
+
+ENUM_BEGIN(access_type, INVALID)
+ENUM_REG(READ)
+ENUM_REG(WRITE)
+ENUM_REG(CREATE)
+ENUM_REG(DROP)
+ENUM_REG(LIST)
+ENUM_REG(METADATA)
+ENUM_REG(CONTROL)
+ENUM_REG(ALL)
+ENUM_END(access_type)
+
+ENUM_TYPE_SERIALIZATION(access_type, INVALID)
+
+// Ranger policy data structure
+struct policy_item
+{
+ std::set<access_type> access_types;
+ std::set<std::string> users;
+
+ DEFINE_JSON_SERIALIZATION(access_types, users);
+
+ // Match a policy according to acl_type and user_name
+ bool match(const access_type &ac_type, const std::string &user_name) const;
+};
+
+// Data structure of policies with different priorities
+struct acl_policies
+{
+ // policy priority: deny_policies_exclude > deny_policies > allow_policies_exclude >
+ // allow_policies
+ std::vector<policy_item> allow_policies;
+ std::vector<policy_item> allow_policies_exclude;
+ std::vector<policy_item> deny_policies;
+ std::vector<policy_item> deny_policies_exclude;
+
+ DEFINE_JSON_SERIALIZATION(allow_policies,
+ allow_policies_exclude,
+ deny_policies,
+ deny_policies_exclude);
+
+ // Check whether the user is allowed to access the resource.
+ bool allowed(const access_type &ac_type, const std::string &user_name) const;
+};
+
+// A policy data structure definition of ranger resources
+struct ranger_resource_policy
+{
+public:
+ ranger_resource_policy() = default;
+ ~ranger_resource_policy() = default;
+
+ // Create the default policy to adapt legacy tables.
Review Comment:
```suggestion
// Create the default policy to adapt legacy tables.
// In Ranger policy, database prefix is required to identify the database it belongs to, but for the tables which are created before Ranger policy is introduced to Pegasus, they don't have such a prefix. So we create a default database policy for them.
```
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -0,0 +1,79 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "ranger_resource_policy.h"
+
+namespace dsn {
+namespace ranger {
+
+bool policy_item::match(const access_type &ac_type, const std::string &user_name) const
+{
+ return access_types.count(ac_type) != 0 && users.count(user_name) != 0;
+}
+
+bool acl_policies::allowed(const access_type &ac_type, const std::string &user_name) const
+{
+ // 1. Check if it is not allowed.
+ for (const auto &deny_policy : deny_policies) {
+ // 1.1. In 'deny_policies'.
+ if (!deny_policy.match(ac_type, user_name)) {
+ continue;
+ }
+ bool in_deny_policies_exclude = false;
+ for (const auto &deny_policy_exclude : deny_policies_exclude) {
+ if (deny_policy_exclude.match(ac_type, user_name)) {
+ in_deny_policies_exclude = true;
+ break;
+ }
+ }
+ // 1.2. Not in any 'deny_policies_exclude', it's not allowed.
+ if (!in_deny_policies_exclude) {
+ return false;
+ }
+ }
+
+ // 2. Check if it is allowed.
+ for (const auto &allow_policy : allow_policies) {
+ // 2.1. In 'allow_policies'.
+ if (!allow_policy.match(ac_type, user_name)) {
+ continue;
+ }
+ for (const auto &allow_policy_exclude : allow_policies_exclude) {
+ // 2.2. In some 'allow_policies_exclude', it's not allowed.
+ if (allow_policy_exclude.match(ac_type, user_name)) {
+ return false;
+ }
+ }
+ // 2.3. Not in any 'allow_policies_exclude', it's allowed.
+ return true;
+ }
+
+ // 3. Otherwise, it's not allowed.
+ return false;
+}
+
+void ranger_resource_policy::create_default_database_policy(ranger_resource_policy &acl)
+{
+ acl.name = "default database policy";
+ acl.database_names = {"*"};
+ policy_item item;
+ item.access_types.insert(access_type::ALL);
Review Comment:
`users` field of policy_item is left as empty, is it what we expect?
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,112 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
+{
+ READ = 0,
+ WRITE,
+ CREATE,
+ DROP,
+ LIST,
+ METADATA,
+ CONTROL,
+ ALL,
+ INVALID,
+};
+
+ENUM_BEGIN(access_type, INVALID)
+ENUM_REG(READ)
+ENUM_REG(WRITE)
+ENUM_REG(CREATE)
+ENUM_REG(DROP)
+ENUM_REG(LIST)
+ENUM_REG(METADATA)
+ENUM_REG(CONTROL)
+ENUM_REG(ALL)
+ENUM_END(access_type)
+
+ENUM_TYPE_SERIALIZATION(access_type, INVALID)
+
+// Ranger policy data structure
+struct policy_item
+{
+ std::set<access_type> access_types;
Review Comment:
Use bitmap instead of std::set, `match` is a very frequently called function in hot path, we must ensure it's performance.
It's recommend to add a simple benchmark test for `match`.
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,112 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
+{
+ READ = 0,
+ WRITE,
+ CREATE,
+ DROP,
+ LIST,
+ METADATA,
+ CONTROL,
+ ALL,
+ INVALID,
+};
+
+ENUM_BEGIN(access_type, INVALID)
+ENUM_REG(READ)
+ENUM_REG(WRITE)
+ENUM_REG(CREATE)
+ENUM_REG(DROP)
+ENUM_REG(LIST)
+ENUM_REG(METADATA)
+ENUM_REG(CONTROL)
+ENUM_REG(ALL)
+ENUM_END(access_type)
+
+ENUM_TYPE_SERIALIZATION(access_type, INVALID)
+
+// Ranger policy data structure
+struct policy_item
+{
+ std::set<access_type> access_types;
+ std::set<std::string> users;
+
+ DEFINE_JSON_SERIALIZATION(access_types, users);
+
+ // Match a policy according to acl_type and user_name
+ bool match(const access_type &ac_type, const std::string &user_name) const;
+};
+
+// Data structure of policies with different priorities
+struct acl_policies
+{
+ // policy priority: deny_policies_exclude > deny_policies > allow_policies_exclude >
+ // allow_policies
+ std::vector<policy_item> allow_policies;
+ std::vector<policy_item> allow_policies_exclude;
+ std::vector<policy_item> deny_policies;
+ std::vector<policy_item> deny_policies_exclude;
+
+ DEFINE_JSON_SERIALIZATION(allow_policies,
+ allow_policies_exclude,
+ deny_policies,
+ deny_policies_exclude);
+
+ // Check whether the user is allowed to access the resource.
Review Comment:
```suggestion
// Check whether the 'user_name' is allowed to access the resource by type of 'ac_type'.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] WHBANG commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/n)
Posted by "WHBANG (via GitHub)" <gi...@apache.org>.
WHBANG commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1118484737
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -0,0 +1,70 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "ranger_resource_policy.h"
+
+namespace dsn {
+namespace ranger {
+
+bool policy_item::match(const access_type &ac_type, const std::string &user_name) const
+{
+ return (access_types & ac_type) != 0 && users.count(user_name) != 0;
Review Comment:
One rpc request can only match one `ac_type`, so `access_types = READ | DROP` will not occur.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] WHBANG commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/8)
Posted by "WHBANG (via GitHub)" <gi...@apache.org>.
WHBANG commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1116650187
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -0,0 +1,79 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "ranger_resource_policy.h"
+
+namespace dsn {
+namespace ranger {
+
+bool policy_item::match(const access_type &ac_type, const std::string &user_name) const
+{
+ return access_types.count(ac_type) != 0 && users.count(user_name) != 0;
+}
+
+bool acl_policies::allowed(const access_type &ac_type, const std::string &user_name) const
+{
+ // 1. Check if it is not allowed.
+ for (const auto &deny_policy : deny_policies) {
+ // 1.1. In 'deny_policies'.
+ if (!deny_policy.match(ac_type, user_name)) {
+ continue;
+ }
+ bool in_deny_policies_exclude = false;
+ for (const auto &deny_policy_exclude : deny_policies_exclude) {
+ if (deny_policy_exclude.match(ac_type, user_name)) {
+ in_deny_policies_exclude = true;
+ break;
+ }
+ }
+ // 1.2. Not in any 'deny_policies_exclude', it's not allowed.
+ if (!in_deny_policies_exclude) {
+ return false;
+ }
+ }
+
+ // 2. Check if it is allowed.
+ for (const auto &allow_policy : allow_policies) {
+ // 2.1. In 'allow_policies'.
+ if (!allow_policy.match(ac_type, user_name)) {
+ continue;
+ }
+ for (const auto &allow_policy_exclude : allow_policies_exclude) {
+ // 2.2. In some 'allow_policies_exclude', it's not allowed.
+ if (allow_policy_exclude.match(ac_type, user_name)) {
+ return false;
+ }
+ }
+ // 2.3. Not in any 'allow_policies_exclude', it's allowed.
+ return true;
+ }
+
+ // 3. Otherwise, it's not allowed.
+ return false;
+}
+
+void ranger_resource_policy::create_default_database_policy(ranger_resource_policy &acl)
+{
+ acl.name = "default database policy";
+ acl.database_names = {"*"};
+ policy_item item;
+ item.access_types.insert(access_type::ALL);
Review Comment:
Yes, `users` of database resource policy is empty
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/n)
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1118383491
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,86 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <unordered_set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
Review Comment:
Better to use enum classes instead of traditional enumerations which could lead to problems such as name clashes (see https://www.stroustrup.com/C++11FAQ.html#enum).
You can define the class just like:
```c++
enum class access_type : uint8_t
```
where the underlying type is `uint8_t`.
Overload operator `&` and `|` where `std::underlying_type` can be used to extract `uint8_t`, see https://www.sandordargo.com/blog/2022/06/22/bitwise-enums for details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/8)
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1115283603
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+enum access_type
Review Comment:
In header files, it's necessary to add required comments to describe enums, structs/classes, member functions and variables of structs/classes.
##########
src/runtime/test/ranger_resource_policy_test.cpp:
##########
@@ -0,0 +1,81 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include "runtime/ranger/ranger_resource_policy.h"
+
+namespace dsn {
+namespace ranger {
+
+TEST(ranger_resource_policy_test, policy_item_match)
+{
+ policy_item item = {
+ {access_type::READ, access_type::WRITE, access_type::CREATE}, {"user1", "user2"}, {}, {}};
+ struct test_case
+ {
+ access_type ac_type;
+ std::string user_name;
+ bool expected_result;
+ } tests[] = {{access_type::INVALID, "", false},
+ {access_type::INVALID, "user", false},
+ {access_type::READ, "user", false},
+ {access_type::LIST, "user1", false},
+ {access_type::READ, "user1", true},
+ {access_type::WRITE, "user2", true}};
+ for (const auto &test : tests) {
+ auto actual_result = item.match(test.ac_type, test.user_name);
+ EXPECT_EQ(actual_result, test.expected_result);
Review Comment:
it would be better to swap the order of "expect" value and "actual" value, i.e. `EXPECT_EQ(test.expected_result, actual_result);`
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+enum access_type
+{
+ READ = 0,
+ WRITE,
+ CREATE,
+ DROP,
+ LIST,
+ METADATA,
+ CONTROL,
+ ALL,
+ INVALID,
+};
+
+ENUM_BEGIN(access_type, INVALID)
+ENUM_REG(READ)
+ENUM_REG(WRITE)
+ENUM_REG(CREATE)
+ENUM_REG(DROP)
+ENUM_REG(LIST)
+ENUM_REG(METADATA)
+ENUM_REG(CONTROL)
+ENUM_REG(ALL)
+ENUM_END(access_type)
+
+ENUM_TYPE_SERIALIZATION(access_type, INVALID)
+
+struct policy_item
+{
+ std::set<access_type> access_types;
+ std::set<std::string> users;
+ std::set<std::string> groups; // not use
+ std::set<std::string> roles; // not use
Review Comment:
If they are no used now, can we remove them?
##########
src/runtime/test/ranger_resource_policy_test.cpp:
##########
@@ -0,0 +1,81 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include "runtime/ranger/ranger_resource_policy.h"
+
+namespace dsn {
+namespace ranger {
+
+TEST(ranger_resource_policy_test, policy_item_match)
+{
+ policy_item item = {
+ {access_type::READ, access_type::WRITE, access_type::CREATE}, {"user1", "user2"}, {}, {}};
+ struct test_case
+ {
+ access_type ac_type;
+ std::string user_name;
+ bool expected_result;
+ } tests[] = {{access_type::INVALID, "", false},
+ {access_type::INVALID, "user", false},
+ {access_type::READ, "user", false},
+ {access_type::LIST, "user1", false},
+ {access_type::READ, "user1", true},
+ {access_type::WRITE, "user2", true}};
Review Comment:
How about check all access types for user1 and user2?
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+enum access_type
+{
+ READ = 0,
+ WRITE,
+ CREATE,
+ DROP,
+ LIST,
+ METADATA,
+ CONTROL,
+ ALL,
+ INVALID,
+};
+
+ENUM_BEGIN(access_type, INVALID)
+ENUM_REG(READ)
+ENUM_REG(WRITE)
+ENUM_REG(CREATE)
+ENUM_REG(DROP)
+ENUM_REG(LIST)
+ENUM_REG(METADATA)
+ENUM_REG(CONTROL)
+ENUM_REG(ALL)
+ENUM_END(access_type)
+
+ENUM_TYPE_SERIALIZATION(access_type, INVALID)
+
+struct policy_item
+{
+ std::set<access_type> access_types;
+ std::set<std::string> users;
+ std::set<std::string> groups; // not use
+ std::set<std::string> roles; // not use
+
+ DEFINE_JSON_SERIALIZATION(access_types, users, groups, roles);
+
+ bool match(const access_type &ac_type, const std::string &user_name) const;
+};
+
+struct acl_policies
+{
+ // policy priority: deny_policies_exclude > deny_policies > allow_policies_exclude >
+ // allow_policies
+ std::vector<policy_item> allow_policies;
+ std::vector<policy_item> allow_policies_exclude;
+ std::vector<policy_item> deny_policies;
+ std::vector<policy_item> deny_policies_exclude;
+
+ DEFINE_JSON_SERIALIZATION(allow_policies,
+ allow_policies_exclude,
+ deny_policies,
+ deny_policies_exclude);
+
+ // Check whether the user is allowed to access the resource.
+ bool allowed(const access_type &ac_type, const std::string &user_name) const;
+};
+
+struct ranger_resource_policy
+{
+public:
+ ranger_resource_policy() = default;
+ ~ranger_resource_policy() = default;
+
+ // Create the default policy to adapt legacy tables.
+ static void create_default_database_policy(ranger_resource_policy &acl);
+
+public:
+ std::string name;
+ std::set<std::string> global_names; // TODO(yingchun): not in use
Review Comment:
can we remove it?
##########
src/runtime/test/ranger_resource_policy_test.cpp:
##########
@@ -0,0 +1,81 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include "runtime/ranger/ranger_resource_policy.h"
+
+namespace dsn {
+namespace ranger {
+
+TEST(ranger_resource_policy_test, policy_item_match)
+{
+ policy_item item = {
+ {access_type::READ, access_type::WRITE, access_type::CREATE}, {"user1", "user2"}, {}, {}};
+ struct test_case
+ {
+ access_type ac_type;
+ std::string user_name;
+ bool expected_result;
+ } tests[] = {{access_type::INVALID, "", false},
+ {access_type::INVALID, "user", false},
+ {access_type::READ, "user", false},
+ {access_type::LIST, "user1", false},
+ {access_type::READ, "user1", true},
+ {access_type::WRITE, "user2", true}};
+ for (const auto &test : tests) {
+ auto actual_result = item.match(test.ac_type, test.user_name);
+ EXPECT_EQ(actual_result, test.expected_result);
+ }
+}
+
+TEST(ranger_resource_policy_test, acl_policies_allowed)
+{
+ acl_policies policy;
+ policy.allow_policies = {{{access_type::READ, access_type::WRITE, access_type::CREATE},
+ {"user1", "user2", "user3", "user4"},
+ {},
+ {}}};
+ policy.allow_policies_exclude = {
+ {{access_type::WRITE, access_type::CREATE}, {"user2"}, {}, {}}};
+ policy.deny_policies = {{{access_type::READ, access_type::WRITE}, {"user3", "user4"}, {}, {}}};
+ policy.deny_policies_exclude = {{{access_type::READ}, {"user4"}, {}, {}}};
+ struct test_case
+ {
+ access_type ac_type;
+ std::string user_name;
+ bool expected_result;
+ } tests[] = {{access_type::READ, "user", false},
+ {access_type::READ, "user1", true},
+ {access_type::LIST, "user1", false},
+ {access_type::READ, "user2", true},
+ {access_type::WRITE, "user2", false},
+ {access_type::LIST, "user2", false},
+ {access_type::READ, "user3", false},
+ {access_type::CREATE, "user3", true},
+ {access_type::LIST, "user3", false},
+ {access_type::READ, "user4", true},
+ {access_type::WRITE, "user4", false},
+ {access_type::CREATE, "user4", true},
+ {access_type::LIST, "user4", false}};
+ for (const auto &test : tests) {
+ auto actual_result = policy.allowed(test.ac_type, test.user_name);
+ EXPECT_EQ(actual_result, test.expected_result);
Review Comment:
same
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/n)
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1118383491
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,86 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <unordered_set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
Review Comment:
Better to use enum classes instead of traditional enumerations which could lead to problems such as name clashes (see https://www.stroustrup.com/C++11FAQ.html#enum).
You can define the class just like:
```c++
enum access_type : uint8_t
```
where the underlying type is `uint8_t`.
Overload operator `&` and `|` where `std::underlying_type` can be used to extract `uint8_t`, see https://www.sandordargo.com/blog/2022/06/22/bitwise-enums for details.
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,86 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <unordered_set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
+{
+ READ = 1,
+ WRITE = 1 << 1,
+ CREATE = 1 << 2,
+ DROP = 1 << 3,
+ LIST = 1 << 4,
+ METADATA = 1 << 5,
+ CONTROL = 1 << 6,
+};
+
+// Ranger policy data structure
+struct policy_item
+{
+ int8_t access_types;
+ std::unordered_set<std::string> users;
+
+ // Check if the 'acl_type' - 'user_name' pair is matched to the policy.
+ // Return true if it is matched, otherwise return false.
+ // TODO(wanghao): add benchmark test
+ bool match(const access_type &ac_type, const std::string &user_name) const;
+};
+
+// Data structure of policies with different priorities
+struct acl_policies
+{
+ // policy priority: deny_policies_exclude > deny_policies > allow_policies_exclude >
+ // allow_policies
+ std::vector<policy_item> allow_policies;
+ std::vector<policy_item> allow_policies_exclude;
+ std::vector<policy_item> deny_policies;
+ std::vector<policy_item> deny_policies_exclude;
+
+ // Check whether the 'user_name' is allowed to access the resource by type of 'ac_type'.
+ bool allowed(const access_type &ac_type, const std::string &user_name) const;
+};
+
+// A policy data structure definition of ranger resources
+struct ranger_resource_policy
+{
+public:
+ ranger_resource_policy() = default;
+ ~ranger_resource_policy() = default;
+
+public:
Review Comment:
`public` could be omitted.
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -0,0 +1,70 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "ranger_resource_policy.h"
+
+namespace dsn {
+namespace ranger {
+
+bool policy_item::match(const access_type &ac_type, const std::string &user_name) const
+{
+ return (access_types & ac_type) != 0 && users.count(user_name) != 0;
Review Comment:
Is `ac_type` allowed to just include single type or multiple types ? Are following examples matched ?
```
access_types = READ | DROP, ac_type = READ | CREATE
access_types = WRITE, ac_type = READ | WRITE | CREATE
```
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,86 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <unordered_set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
+{
+ READ = 1,
Review Comment:
Prefer constant-style naming such as `kRead`, `kWrite`, `kCreate`, etc., see https://google.github.io/styleguide/cppguide.html#Enumerator_Names.
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,86 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <unordered_set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+// ACL type defined in Range service for RPC matching policy
+enum access_type
+{
+ READ = 1,
+ WRITE = 1 << 1,
+ CREATE = 1 << 2,
+ DROP = 1 << 3,
+ LIST = 1 << 4,
+ METADATA = 1 << 5,
+ CONTROL = 1 << 6,
+};
+
+// Ranger policy data structure
+struct policy_item
+{
+ int8_t access_types;
+ std::unordered_set<std::string> users;
+
+ // Check if the 'acl_type' - 'user_name' pair is matched to the policy.
+ // Return true if it is matched, otherwise return false.
+ // TODO(wanghao): add benchmark test
+ bool match(const access_type &ac_type, const std::string &user_name) const;
+};
+
+// Data structure of policies with different priorities
+struct acl_policies
+{
+ // policy priority: deny_policies_exclude > deny_policies > allow_policies_exclude >
+ // allow_policies
+ std::vector<policy_item> allow_policies;
+ std::vector<policy_item> allow_policies_exclude;
+ std::vector<policy_item> deny_policies;
+ std::vector<policy_item> deny_policies_exclude;
+
+ // Check whether the 'user_name' is allowed to access the resource by type of 'ac_type'.
+ bool allowed(const access_type &ac_type, const std::string &user_name) const;
+};
+
+// A policy data structure definition of ranger resources
+struct ranger_resource_policy
+{
+public:
+ ranger_resource_policy() = default;
Review Comment:
Is it necessary to add both default constructor and destructor here ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/8)
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1118250747
##########
src/runtime/ranger/ranger_resource_policy.cpp:
##########
@@ -0,0 +1,79 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "ranger_resource_policy.h"
+
+namespace dsn {
+namespace ranger {
+
+bool policy_item::match(const access_type &ac_type, const std::string &user_name) const
+{
+ return (access_types & ac_type) != 0 && users.count(user_name) != 0;
+}
+
+bool acl_policies::allowed(const access_type &ac_type, const std::string &user_name) const
+{
+ // 1. Check if it is not allowed.
+ for (const auto &deny_policy : deny_policies) {
+ // 1.1. In 'deny_policies'.
+ if (!deny_policy.match(ac_type, user_name)) {
+ continue;
+ }
+ bool in_deny_policies_exclude = false;
+ for (const auto &deny_policy_exclude : deny_policies_exclude) {
+ if (deny_policy_exclude.match(ac_type, user_name)) {
+ in_deny_policies_exclude = true;
+ break;
+ }
+ }
+ // 1.2. Not in any 'deny_policies_exclude', it's not allowed.
+ if (!in_deny_policies_exclude) {
+ return false;
+ }
+ }
+
+ // 2. Check if it is allowed.
+ for (const auto &allow_policy : allow_policies) {
+ // 2.1. In 'allow_policies'.
+ if (!allow_policy.match(ac_type, user_name)) {
+ continue;
+ }
+ for (const auto &allow_policy_exclude : allow_policies_exclude) {
+ // 2.2. In some 'allow_policies_exclude', it's not allowed.
+ if (allow_policy_exclude.match(ac_type, user_name)) {
+ return false;
+ }
+ }
+ // 2.3. Not in any 'allow_policies_exclude', it's allowed.
+ return true;
+ }
+
+ // 3. Otherwise, it's not allowed.
+ return false;
+}
+
+void ranger_resource_policy::create_default_database_policy(ranger_resource_policy &acl)
+{
+ acl.name = "default database policy";
+ acl.database_names = {"*"};
+ policy_item item;
+ item.access_types = CREATE | DROP | LIST | METADATA | CONTROL;
Review Comment:
miss READ and WRITE?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1360: feat(security): Use Apache Ranger for access control(1/8)
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1360:
URL: https://github.com/apache/incubator-pegasus/pull/1360#discussion_r1116602136
##########
src/runtime/ranger/ranger_resource_policy.h:
##########
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include <map>
+#include <string>
+#include <set>
+#include <vector>
+
+#include <rapidjson/document.h>
+
+#include "common/json_helper.h"
+#include "utils/fmt_logging.h"
+
+namespace dsn {
+namespace ranger {
+
+enum access_type
+{
+ READ = 0,
+ WRITE,
+ CREATE,
+ DROP,
+ LIST,
+ METADATA,
+ CONTROL,
+ ALL,
+ INVALID,
+};
+
+ENUM_BEGIN(access_type, INVALID)
+ENUM_REG(READ)
+ENUM_REG(WRITE)
+ENUM_REG(CREATE)
+ENUM_REG(DROP)
+ENUM_REG(LIST)
+ENUM_REG(METADATA)
+ENUM_REG(CONTROL)
+ENUM_REG(ALL)
+ENUM_END(access_type)
+
+ENUM_TYPE_SERIALIZATION(access_type, INVALID)
+
+struct policy_item
+{
+ std::set<access_type> access_types;
+ std::set<std::string> users;
+ std::set<std::string> groups; // not use
+ std::set<std::string> roles; // not use
+
+ DEFINE_JSON_SERIALIZATION(access_types, users, groups, roles);
+
+ bool match(const access_type &ac_type, const std::string &user_name) const;
+};
+
+struct acl_policies
+{
+ // policy priority: deny_policies_exclude > deny_policies > allow_policies_exclude >
+ // allow_policies
+ std::vector<policy_item> allow_policies;
+ std::vector<policy_item> allow_policies_exclude;
+ std::vector<policy_item> deny_policies;
+ std::vector<policy_item> deny_policies_exclude;
+
+ DEFINE_JSON_SERIALIZATION(allow_policies,
+ allow_policies_exclude,
+ deny_policies,
+ deny_policies_exclude);
+
+ // Check whether the user is allowed to access the resource.
+ bool allowed(const access_type &ac_type, const std::string &user_name) const;
+};
+
+struct ranger_resource_policy
+{
+public:
+ ranger_resource_policy() = default;
+ ~ranger_resource_policy() = default;
+
+ // Create the default policy to adapt legacy tables.
+ static void create_default_database_policy(ranger_resource_policy &acl);
+
+public:
+ std::string name;
+ std::set<std::string> global_names; // TODO(yingchun): not in use
Review Comment:
I meant remove the whole line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org