Slim down the SQL policy verifier and separate it from datalog_policy…

…_verifier.

In the early days of Raksha, we created the `datalog_policy_verifier`. This was before we understood that the Souffle language and the Souffle internal architecture meant that we would have to create a separate library per policy interface, and that having one Souffle library to serve all of our needs probably wasn't going to cut it.

Others coming to the Raksha project saw the name and, quite reasonably, believed it was the generic Raksha analysis rather than a SQL-verifier-specific analysis. Attempts to use this library as a generic policy library made it bloated and tangled.

This commit attempts to move us to a better state by separating out the SQL policy verifier into a separate `sql_policy_verifier_interface` and associated `sql_policy_verifier`. This allows slimming down that library and extracting it from the tangle. The `datalog_policy_verifier` is now not used for any production purpose, and we can clean it up at our leisure.

Fixes #747
See #728

PiperOrigin-RevId: 479691720

src/analysis/souffle/BUILD

@@ -188,6 +188,8 @@ souffle_cc_binary(
     additional_deps = ["logfunctors"],
 )
 
+# TODO(#728): This datalog_policy_verifier is very entangled and is doing too many things. We should
+# untangle it and delete it.
 raksha_policy_verifier_library(
     name = "datalog_policy_verifier",
     additional_dl_files = [
@@ -200,6 +202,15 @@ raksha_policy_verifier_library(
     policy_verifier_interfaces = [":policy_verifier_interface.dl"],
 )
 
+# This is a policy verifier focused on the exfiltration of secret columns in the SQL verification
+# engine specifically.
+raksha_policy_verifier_library(
+    name = "sql_policy_verifier",
+    policies = ["//src/backends/policy_engine/souffle/testdata:empty_policy.auth"],
+    policy_verifier_interfaces = [":sql_policy_verifier_interface.dl"],
+)
+
+# This is a policy verifier focused on the propagation of differential privacy parameter info.
 raksha_policy_verifier_library(
     name = "dp_policy_verifier",
     additional_dl_files = [

src/analysis/souffle/sql_policy_verifier_interface.dl

@@ -0,0 +1,34 @@
+//-----------------------------------------------------------------------------
+// Copyright 2022 Google LLC
+//
+// Licensed 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
+//
+//     https://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.
+//-----------------------------------------------------------------------------
+
+#ifndef SRC_ANALYSIS_SOUFFLE_SQL_POLICY_VERIFIER_INTERFACE
+#define SRC_ANALYSIS_SOUFFLE_SQL_POLICY_VERIFIER_INTERFACE
+
+#include "src/analysis/souffle/sql_output.dl"
+#include "src/analysis/souffle/tag_transforms.dl"
+#include "src/analysis/souffle/taint.dl"
+
+// An interface used for running and getting results from the policy verifier for
+// the specific purpose of the SQL verifier.
+// Does not concern itself with authorization logic facts, considers only
+// `Operation`s and `SqlPolicyRule`s. Returns as output the violatesPolicy
+// relation, which indicates whether there were failures in a way that is easy
+// to read across the Souffle C++ interface.
+.input isOperation(delimiter=";")
+.input isSqlPolicyRule(delimiter=";")
+.output violatesPolicy(delimiter=";")
+
+#endif

src/backends/policy_engine/BUILD

@@ -49,6 +49,20 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "catchall_policy_rule_policy",
+    hdrs = ["catchall_policy_rule_policy.h"],
+    visibility = [
+        "//:__pkg__",
+        "//src:__subpackages__",
+    ] + frontend_packages,
+    deps = [
+        ":policy",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/strings",
+    ],
+)
+
 cc_library(
     name = "auth_logic_policy",
     hdrs = ["auth_logic_policy.h"],

src/backends/policy_engine/catchall_policy_rule_policy.h

@@ -0,0 +1,54 @@
+//-----------------------------------------------------------------------------
+// Copyright 2022 Google LLC
+//
+// Licensed 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
+//
+//     https://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.
+//----------------------------------------------------------------------------
+#ifndef SRC_BACKENDS_POLICY_ENGINE_CATCHALL_POLICY_RULE_POLICY_H_
+#define SRC_BACKENDS_POLICY_ENGINE_CATCHALL_POLICY_RULE_POLICY_H_
+
+#include <filesystem>
+#include <optional>
+
+#include "absl/strings/string_view.h"
+#include "src/backends/policy_engine/policy.h"
+
+namespace raksha::backends::policy_engine {
+
+// TODO(#728): We ended up in a dependency mess around the SqlPolicyRulePolicy,
+// where a number of test frontends started using it that had nothing to do with
+// SQL. This is a shim to slim down the SQL infrastructure and allow us to
+// eventually eliminate the tests using this as a catchall policy.
+class CatchallPolicyRulePolicy : public Policy {
+ public:
+  explicit CatchallPolicyRulePolicy(std::string is_sql_policy_rule_facts)
+      : is_sql_policy_rule_facts_(std::move(is_sql_policy_rule_facts)) {}
+
+  std::string GetPolicyAnalysisCheckerName() const override {
+    return "datalog_policy_verifier_cxx";
+  }
+
+  std::optional<std::string> GetPolicyFactName() const override {
+    return "isSqlPolicyRule";
+  }
+
+  std::optional<std::string> GetPolicyString() const override {
+    return is_sql_policy_rule_facts_;
+  }
+
+ private:
+  std::string is_sql_policy_rule_facts_;
+};
+
+}  // namespace raksha::backends::policy_engine
+
+#endif

src/backends/policy_engine/souffle/BUILD

@@ -84,9 +84,9 @@ cc_binary(
     deps = [
         ":souffle_policy_checker",
         "//src/backends/policy_engine:auth_logic_policy",
+        "//src/backends/policy_engine:catchall_policy_rule_policy",
         "//src/backends/policy_engine:dp_parameter_policy",
         "//src/backends/policy_engine:policy",
-        "//src/backends/policy_engine:sql_policy_rule_policy",
         "//src/common/logging",
         "//src/ir:proto_to_ir",
         "//src/parser/ir:ir_parser",
@@ -371,6 +371,8 @@ cc_test(
     ],
 )
 
+# Use this version of the `souffle_policy_checker` when binary size does not matter. Linked with
+# every library that may use it.
 cc_library(
     name = "souffle_policy_checker",
     srcs = ["souffle_policy_checker.cc"],
@@ -394,6 +396,7 @@ cc_library(
         ":utils",
         "//src/analysis/souffle:datalog_policy_verifier",
         "//src/analysis/souffle:dp_policy_verifier",
+        "//src/analysis/souffle:sql_policy_verifier",
         "//src/backends/policy_engine:policy",
         "//src/backends/policy_engine:policy_checker",
         "//src/backends/policy_engine/souffle/dp_testdata/delta_1:dp_policy_verifier_delta_1_with_auth_logic_lib",
@@ -408,6 +411,35 @@ cc_library(
     ],
 )
 
+# This is a version of the `souffle_policy_checker` for use with the SQL verifier only. Links only
+# with the SQL verifier library.
+cc_library(
+    name = "souffle_policy_checker_sql",
+    srcs = ["souffle_policy_checker.cc"],
+    hdrs = ["souffle_policy_checker.h"],
+    copts = [
+        "-fexceptions",
+        "-Iexternal/souffle/src/include/souffle",
+    ],
+    # Turn off header modules, as Google precompiled headers use
+    # -fno-exceptions, and combining a precompiled header with -fno-exceptions
+    # with a binary that uses -fexceptions makes Clang upset.
+    features = ["-use_header_modules"],
+    linkopts = ["-pthread"],
+    visibility = ["//src:__subpackages__"] + frontend_packages,
+    deps = [
+        ":datalog_lowering_visitor",
+        ":utils",
+        "//src/analysis/souffle:sql_policy_verifier",
+        "//src/backends/policy_engine:policy",
+        "//src/backends/policy_engine:policy_checker",
+        "//src/common/utils:filesystem",
+        "//src/ir:module",
+        "@com_google_absl//absl/strings",
+        "@souffle//:souffle_include_lib",
+    ],
+)
+
 cc_test(
     name = "souffle_policy_checker_test",
     srcs = ["souffle_policy_checker_test.cc"],

src/backends/policy_engine/souffle/check_policy_compliance.cc

@@ -29,7 +29,7 @@
 #include "src/backends/policy_engine/dp_parameter_policy.h"
 #include "src/backends/policy_engine/policy.h"
 #include "src/backends/policy_engine/souffle/souffle_policy_checker.h"
-#include "src/backends/policy_engine/sql_policy_rule_policy.h"
+#include "src/backends/policy_engine/catchall_policy_rule_policy.h"
 #include "src/common/logging/logging.h"
 #include "src/ir/proto_to_ir.h"
 #include "src/parser/ir/ir_parser.h"
@@ -120,7 +120,7 @@ absl::StatusOr<IrGraphComponents> GetIrGraphComponentsFromProtoPath(
 using raksha::backends::policy_engine::AuthLogicPolicy;
 using raksha::backends::policy_engine::DpParameterPolicy;
 using raksha::backends::policy_engine::SoufflePolicyChecker;
-using raksha::backends::policy_engine::SqlPolicyRulePolicy;
+using raksha::backends::policy_engine::CatchallPolicyRulePolicy;
 using raksha::ir::IrProgramParser;
 
 int main(int argc, char* argv[]) {
@@ -167,7 +167,7 @@ int main(int argc, char* argv[]) {
                  << sql_policy_rules.status();
       return UnwrapExitCode(ReturnCode::ERROR);
     }
-    SqlPolicyRulePolicy policy(*sql_policy_rules);
+    CatchallPolicyRulePolicy policy(*sql_policy_rules);
     policyCheckSucceeded = SoufflePolicyChecker().IsModulePolicyCompliant(
         *components.value().ir_module, policy);
   } else if (absl::GetFlag(FLAGS_epsilon_dp_parameter).has_value() &&

src/backends/policy_engine/sql_policy_rule_policy.h

@@ -24,15 +24,15 @@
 
 namespace raksha::backends::policy_engine {
 
-// Placeholder for the Policy datastructure. Right now, the policy is hard-coded
-// in datalog directly.
+// A form of policy for the SQL verifier, which communicates tag transforms on
+// SQL expressions in its special rule format.
 class SqlPolicyRulePolicy : public Policy {
  public:
   explicit SqlPolicyRulePolicy(std::string is_sql_policy_rule_facts)
       : is_sql_policy_rule_facts_(std::move(is_sql_policy_rule_facts)) {}
 
   std::string GetPolicyAnalysisCheckerName() const override {
-    return "datalog_policy_verifier_cxx";
+    return "sql_policy_verifier_cxx";
   }
 
   std::optional<std::string> GetPolicyFactName() const override {

src/frontends/sql/BUILD

@@ -99,7 +99,7 @@ cc_library(
         ":decode",
         ":sql_ir_cc_proto",
         "//src/backends/policy_engine:policy",
-        "//src/backends/policy_engine/souffle:souffle_policy_checker",
+        "//src/backends/policy_engine/souffle:souffle_policy_checker_sql",
     ],
 )