Implement role config pipeline push permissions (#1036)

This PR implements the ability to prevent a given role from pushing a pipeline by setting the `can_push_pipeline` field of its role config to `false`.
stratum · Aug 12, 2022 · a5efb94 · a5efb94
1 parent 006c89e
commit a5efb94
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 3 deletions.
diff --git a/stratum/hal/lib/common/p4_service_test.cc b/stratum/hal/lib/common/p4_service_test.cc
@@ -126,8 +126,9 @@ class P4ServiceTest
     }
   }
 
-  void AddFakeMasterController(uint64 node_id,
-                               p4runtime::SdnConnection* controller) {
+  void AddFakeMasterController(
+      uint64 node_id, p4runtime::SdnConnection* controller,
+      const P4RoleConfig& role_config = GetRoleConfig()) {
     p4::v1::MasterArbitrationUpdate request;
     request.set_device_id(node_id);
     request.mutable_election_id()->set_high(
@@ -137,7 +138,7 @@ class P4ServiceTest
     if (!role_name_.empty()) {
       request.mutable_role()->set_name(role_name_);
       ASSERT_TRUE(
-          request.mutable_role()->mutable_config()->PackFrom(GetRoleConfig()));
+          request.mutable_role()->mutable_config()->PackFrom(role_config));
     }
     ASSERT_OK(p4_service_->AddOrModifyController(node_id, request, controller));
   }
@@ -648,6 +649,44 @@ TEST_P(P4ServiceTest, PushForwardingPipelineConfigReportsRebootRequired) {
   CheckForwardingPipelineConfigs(nullptr, 0 /*ignored*/);
 }
 
+TEST_P(P4ServiceTest, SetForwardingPipelineConfigFailureForRoleProhibited) {
+  // This test is specific to role configs.
+  if (role_name_.empty()) {
+    GTEST_SKIP();
+  }
+  EXPECT_CALL(*auth_policy_checker_mock_,
+              Authorize("P4Service", "SetForwardingPipelineConfig", _))
+      .WillOnce(Return(::util::OkStatus()));
+
+  constexpr char kRoleConfigText[] = R"pb(
+      can_push_pipeline: false
+  )pb";
+  P4RoleConfig role_config;
+  CHECK_OK(ParseProtoFromString(kRoleConfigText, &role_config));
+  ::grpc::ServerContext context;
+  StreamMessageReaderWriterMock stream;
+  p4runtime::SdnConnection controller(&context, &stream);
+  controller.SetElectionId(kElectionId1);
+  AddFakeMasterController(kNodeId1, &controller, role_config);
+
+  ::p4::v1::SetForwardingPipelineConfigRequest request;
+  ::p4::v1::SetForwardingPipelineConfigResponse response;
+  request.set_device_id(kNodeId1);
+  request.mutable_election_id()->set_high(absl::Uint128High64(kElectionId1));
+  request.mutable_election_id()->set_low(absl::Uint128Low64(kElectionId1));
+  request.set_role(role_name_);
+  request.set_action(
+      ::p4::v1::SetForwardingPipelineConfigRequest::VERIFY_AND_COMMIT);
+
+  ::grpc::Status status =
+      p4_service_->SetForwardingPipelineConfig(&context, &request, &response);
+  EXPECT_FALSE(status.ok());
+  EXPECT_THAT(status.error_message(), HasSubstr("not permitted"));
+
+  ASSERT_OK(p4_service_->Teardown());
+  CheckForwardingPipelineConfigs(nullptr, 0 /*ignored*/);
+}
+
 TEST_P(P4ServiceTest, WriteSuccess) {
   ::grpc::ServerContext server_context;
   StreamMessageReaderWriterMock stream;

diff --git a/stratum/lib/p4runtime/sdn_controller_manager.cc b/stratum/lib/p4runtime/sdn_controller_manager.cc
@@ -70,6 +70,23 @@ grpc::Status VerifyElectionIdIsActive(
                       "Election ID is not active for the role.");
 }
 
+grpc::Status VerifyRoleCanPushPipeline(
+    const absl::optional<std::string>& role_name,
+    const absl::flat_hash_map<absl::optional<std::string>,
+                              absl::optional<P4RoleConfig>>& role_configs) {
+  const auto& role_config = role_configs.find(role_name);
+  if (role_config == role_configs.end()) {
+    return grpc::Status(grpc::StatusCode::NOT_FOUND, "Unknown role.");
+  }
+  if (!role_config->second.has_value()) return grpc::Status::OK;
+  if (!role_config->second->can_push_pipeline()) {
+    return grpc::Status(grpc::StatusCode::PERMISSION_DENIED,
+                        "Role not allowed to push pipelines.");
+  }
+
+  return grpc::Status::OK;
+}
+
 grpc::Status VerifyRoleConfig(
     const absl::optional<std::string>& role_name,
     const absl::optional<P4RoleConfig>& role_config,
@@ -366,6 +383,15 @@ grpc::Status SdnControllerManager::AllowRequest(
     role_name = request.role();
   }
 
+  {
+    absl::MutexLock l(&lock_);
+    grpc::Status status =
+        VerifyRoleCanPushPipeline(role_name, role_config_by_name_);
+    if (!status.ok()) {
+      return status;
+    }
+  }
+
   absl::optional<absl::uint128> election_id;
   if (request.has_election_id()) {
     election_id = absl::MakeUint128(request.election_id().high(),