kbs: switch to Regorus for resource policy #357

fitzthum · 2024-03-29T22:35:56Z

Switch from Go-based policy engine to rust-based Regorus
Switch from Anyhow to thiserror for resource policy
Add several unit tests and test policies

This does not touch the AS policy. I will do that later although the plan may change based on whether/how we use EAR tokens.

fitzthum · 2024-03-29T22:59:27Z

Unfortunately when I rebase (and pick up az-snp-vtpm 0.5.2) I run into this dependency conflict. Not sure we can fix this ourselves? @mkulke @anakrish

error: failed to select a version for `once_cell`.
    ... required by package `jsonschema v0.17.1`
    ... which satisfies dependency `jsonschema = "^0.17.1"` of package `regorus v0.1.2`
    ... which satisfies dependency `regorus = "^0.1.2"` of package `api-server v0.1.0 (/home/tobin/kbs/kbs/src/api)`
    ... which satisfies path dependency `api-server` of package `kbs v0.1.0 (/home/tobin/kbs/kbs/src/kbs)`
versions that meet the requirements `^1.17` are: 1.19.0, 1.18.0, 1.17.2, 1.17.1, 1.17.0

all possible versions conflict with previously selected packages.

  previously selected package `once_cell v1.0.1`
    ... which satisfies dependency `once_cell = ">=1, <=1.14.0"` of package `mbox v0.6.1`
    ... which satisfies dependency `mbox = "^0.6.0"` of package `tss-esapi v7.4.0`
    ... which satisfies dependency `tss-esapi = "^7.4"` of package `az-cvm-vtpm v0.5.2`
    ... which satisfies dependency `az-cvm-vtpm = "^0.5.2"` of package `az-snp-vtpm v0.5.2`
    ... which satisfies dependency `az-snp-vtpm = "^0.5.2"` of package `attester v0.1.0 (https://github.com/confidential-containers/guest-components.git?rev=21b2c536b4d6c5c1442b53916c908b54dde136e8#21b2c536)`
    ... which satisfies git dependency `attester` of package `kbs_protocol v0.1.0 (https://github.com/confidential-containers/guest-components.git?rev=21b2c536b4d6c5c1442b53916c908b54dde136e8#21b2c536)`
    ... which satisfies git dependency `kbs_protocol` of package `kbs-client v0.1.0 (/home/tobin/kbs/kbs/tools/client)`

failed to select a version for `once_cell` which could resolve this conflict
make: *** [Makefile:21: background-check-kbs] Error 101

anakrish · 2024-03-30T01:57:27Z

Cargo.toml

@@ -31,6 +31,8 @@ hex = "0.4.3"
 kbs-types = "0.5.3"
 log = "0.4.17"
 prost = "0.11.0"
+#regorus = "0.1.2"
+regorus = { git = "https://github.com/microsoft/regorus" }


Any reason for the preference for github repo over crates.io?

Whoops. That's leftover from development when I was thinking about using the new eval_rule thing.

eva;_rule would be the way to go in future. It avoids having to parse a query, and also returns the value of a rule directly.

anakrish · 2024-03-30T02:04:35Z

Cargo.toml

@@ -31,6 +31,8 @@ hex = "0.4.3"
 kbs-types = "0.5.3"
 log = "0.4.17"
 prost = "0.11.0"
+#regorus = "0.1.2"
+regorus = { git = "https://github.com/microsoft/regorus" }


We could configure regorus to support only those builtins that will be used by the policies. The way it is used currently, we might not even need the arc feature since neither the Engine instance nor the Value instances are shared between threads.

Suggested change

regorus = { git = "https://github.com/microsoft/regorus" }

regorus = { git = "https://github.com/microsoft/regorus", default-features=false, features=["arc"] }

This seems to work. Thanks for the advice.

anakrish · 2024-03-30T02:10:30Z

Unfortunately when I rebase (and pick up az-snp-vtpm 0.5.2) I run into this dependency conflict. Not sure we can fix this ourselves? @mkulke @anakrish

error: failed to select a version for `once_cell`.
    ... required by package `jsonschema v0.17.1`
    ... which satisfies dependency `jsonschema = "^0.17.1"` of package `regorus v0.1.2`
    ... which satisfies dependency `regorus = "^0.1.2"` of package `api-server v0.1.0 (/home/tobin/kbs/kbs/src/api)`
    ... which satisfies path dependency `api-server` of package `kbs v0.1.0 (/home/tobin/kbs/kbs/src/kbs)`
versions that meet the requirements `^1.17` are: 1.19.0, 1.18.0, 1.17.2, 1.17.1, 1.17.0

all possible versions conflict with previously selected packages.

  previously selected package `once_cell v1.0.1`
    ... which satisfies dependency `once_cell = ">=1, <=1.14.0"` of package `mbox v0.6.1`
    ... which satisfies dependency `mbox = "^0.6.0"` of package `tss-esapi v7.4.0`
    ... which satisfies dependency `tss-esapi = "^7.4"` of package `az-cvm-vtpm v0.5.2`
    ... which satisfies dependency `az-cvm-vtpm = "^0.5.2"` of package `az-snp-vtpm v0.5.2`
    ... which satisfies dependency `az-snp-vtpm = "^0.5.2"` of package `attester v0.1.0 (https://github.com/confidential-containers/guest-components.git?rev=21b2c536b4d6c5c1442b53916c908b54dde136e8#21b2c536)`
    ... which satisfies git dependency `attester` of package `kbs_protocol v0.1.0 (https://github.com/confidential-containers/guest-components.git?rev=21b2c536b4d6c5c1442b53916c908b54dde136e8#21b2c536)`
    ... which satisfies git dependency `kbs_protocol` of package `kbs-client v0.1.0 (/home/tobin/kbs/kbs/tools/client)`

failed to select a version for `once_cell` which could resolve this conflict
make: *** [Makefile:21: background-check-kbs] Error 101

You could just not use the jsonschema feature in Regorus that bring in the dependency.
https://github.com/microsoft/regorus/blob/3d98c3b12ecbdfdfabdcfd6947c2019413883a9f/Cargo.toml#L38

The following table lists various Rego builtins and which feature each one depends on.
https://github.com/microsoft/regorus/blob/main/docs/builtins.md

For OPA compatibility, all features are enabled by default. But most applications won't need or use all the OPA builtins.

Xynnn007

Nice PR and good unit tests.

Xynnn007 · 2024-03-30T02:26:44Z

kbs/sample_policies/README.md

If we split docs and code, it would be better. But it looks good now.

Yeah I think we should refactor a few things in this repo including the docs.

Xynnn007 · 2024-03-30T02:28:03Z

kbs/src/api/src/policy_engine/mod.rs

 use tokio::sync::Mutex;

 #[cfg(feature = "opa")]
 mod opa;

 const DEFAULT_POLICY_PATH: &str = "/opa/confidential-containers/kbs/policy.rego";

+#[derive(Error, Debug, PartialEq)]
+pub enum ResourcePolicyError {


btw I like this definition of error types.

kbs/src/api/src/policy_engine/mod.rs

Xynnn007 · 2024-03-30T04:57:15Z

kbs/test/data/policy_4.rego

@@ -0,0 +1,31 @@
+


maybe we should delete the empty lines

The GO OPA engine is very picky about the format of the policy and would often fail if there were any extra lines. This test is supposed to make sure Regorus is more resilient. I added a comment to the policy file because it does look odd.

fitzthum · 2024-04-02T22:05:27Z

Hmm. Looks like there is some issue with building Regorus inside of our docker container

60.32 error: failed to run custom build command for `regorus v0.1.2`
60.32 
60.32 Caused by:
60.32   process didn't exit successfully: `/usr/src/kbs/target/release/build/regorus-d111907e1736880c/build-script-build` (exit status: 101)
60.32   --- stderr
60.32   thread 'main' panicked at /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/regorus-0.1.2/build.rs:20:10:
60.32   called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
60.32   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
60.32 warning: build failed, waiting for other jobs to finish...
81.04 error: failed to compile `kbs v0.1.0 (/usr/src/kbs/kbs/src/kbs)`, intermediate artifacts can be found at `/usr/src/kbs/target`.
81.04 To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.

I don't see a similar issue building locally. I have also tried pinning to v0.1.0 and v0.1.1 and the upstream head. Any ideas @anakrish

anakrish · 2024-04-03T01:04:24Z

Hmm. Looks like there is some issue with building Regorus inside of our docker container

60.32 error: failed to run custom build command for `regorus v0.1.2`
60.32 
60.32 Caused by:
60.32   process didn't exit successfully: `/usr/src/kbs/target/release/build/regorus-d111907e1736880c/build-script-build` (exit status: 101)
60.32   --- stderr
60.32   thread 'main' panicked at /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/regorus-0.1.2/build.rs:20:10:
60.32   called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
60.32   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
60.32 warning: build failed, waiting for other jobs to finish...
81.04 error: failed to compile `kbs v0.1.0 (/usr/src/kbs/kbs/src/kbs)`, intermediate artifacts can be found at `/usr/src/kbs/target`.
81.04 To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.

I don't see a similar issue building locally. I have also tried pinning to v0.1.0 and v0.1.1 and the upstream head. Any ideas @anakrish

Hmm. I haven't seen such an issue before. The failing build command is trying to do this:
https://github.com/microsoft/regorus/blob/0ebcb568cc577602c9ac64ae491ca1e45683deaa/build.rs#L17-L21

    let output = Command::new("git")
        .args(["rev-parse", "HEAD"])
        .output()
        .unwrap();
    let git_hash = String::from_utf8(output.stdout).unwrap();

It is likely that git is not present in the container.

~~Can you point me to the container image so that I can try it out locally?~~
By renaming the git executable temporarily, I could reproduce the issue, confirming that the error is due to git not being present in the docker image.
In the Regorus repo, I have changed it so that unless opa.runtime feature is enabled (that built in function requires the commit hash for which git is needed) there is no need to have git while building.

Xynnn007

lgtm. cc @jialez0

fitzthum · 2024-04-03T16:12:06Z

Ok, thanks @anakrish. I added git to our container image so we don't have to pick up a new version.

@jialez0 I would be curious about your feedback since you've worked on the policy stuff previously.

jialez0

LGTM. Thanks @fitzthum

mkulke

I have some concerns about dropping error sources, but otherwise lg!

kbs/src/api/src/policy_engine/opa/mod.rs

Switch from Go-based policy engine to rust-based Regorus Switch from Anyhow to thiserror for resource policy Add several unit tests and test policies Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>

fitzthum requested a review from sameo as a code owner March 29, 2024 22:35

anakrish reviewed Mar 30, 2024

View reviewed changes

Xynnn007 reviewed Mar 30, 2024

View reviewed changes

fitzthum force-pushed the regorus branch 4 times, most recently from 1421883 to a9b51a3 Compare April 2, 2024 20:04

Xynnn007 approved these changes Apr 3, 2024

View reviewed changes

anakrish mentioned this pull request Apr 3, 2024

Avoid build dependency on git if opa.runtime feature is not enabled microsoft/regorus#193

Closed

fitzthum force-pushed the regorus branch from a9b51a3 to db8e653 Compare April 3, 2024 15:51

jialez0 approved these changes Apr 10, 2024

View reviewed changes

mkulke reviewed Apr 10, 2024

View reviewed changes

kbs/src/api/src/policy_engine/opa/mod.rs Outdated Show resolved Hide resolved

kbs/src/api/src/policy_engine/opa/mod.rs Outdated Show resolved Hide resolved

kbs/src/api/src/policy_engine/opa/mod.rs Show resolved Hide resolved

kbs/src/api/src/policy_engine/opa/mod.rs Show resolved Hide resolved

fitzthum force-pushed the regorus branch from db8e653 to 52c608c Compare April 12, 2024 17:30

kbs: switch to Regorus for resource policy

40b1c20

Switch from Go-based policy engine to rust-based Regorus Switch from Anyhow to thiserror for resource policy Add several unit tests and test policies Signed-off-by: Tobin Feldman-Fitzthum <[email protected]>

fitzthum force-pushed the regorus branch from 52c608c to 40b1c20 Compare April 12, 2024 17:41

fitzthum mentioned this pull request Apr 16, 2024

KBS: Be more tolerant to policy rego files #367

Closed

mkulke approved these changes Apr 16, 2024

View reviewed changes

fitzthum merged commit 3d0b11d into confidential-containers:main Apr 16, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kbs: switch to Regorus for resource policy #357

kbs: switch to Regorus for resource policy #357

fitzthum commented Mar 29, 2024 •

edited

Loading

fitzthum commented Mar 29, 2024

anakrish Mar 30, 2024

fitzthum Apr 2, 2024

anakrish Apr 3, 2024

anakrish Mar 30, 2024

fitzthum Apr 2, 2024

anakrish commented Mar 30, 2024

Xynnn007 left a comment

Xynnn007 Mar 30, 2024

fitzthum Apr 2, 2024

Xynnn007 Mar 30, 2024

Xynnn007 Mar 30, 2024

fitzthum Apr 2, 2024 •

edited

Loading

fitzthum commented Apr 2, 2024

anakrish commented Apr 3, 2024 •

edited

Loading

Xynnn007 left a comment

fitzthum commented Apr 3, 2024

jialez0 left a comment

mkulke left a comment

	regorus = { git = "https://github.com/microsoft/regorus" }
	regorus = { git = "https://github.com/microsoft/regorus", default-features=false, features=["arc"] }

kbs: switch to Regorus for resource policy #357

kbs: switch to Regorus for resource policy #357

Conversation

fitzthum commented Mar 29, 2024 • edited Loading

fitzthum commented Mar 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anakrish commented Mar 30, 2024

Xynnn007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fitzthum Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

fitzthum commented Apr 2, 2024

anakrish commented Apr 3, 2024 • edited Loading

Xynnn007 left a comment

Choose a reason for hiding this comment

fitzthum commented Apr 3, 2024

jialez0 left a comment

Choose a reason for hiding this comment

mkulke left a comment

Choose a reason for hiding this comment

fitzthum commented Mar 29, 2024 •

edited

Loading

fitzthum Apr 2, 2024 •

edited

Loading

anakrish commented Apr 3, 2024 •

edited

Loading