confidential-containers / trustee

Attestation and Secret Delivery Components
Apache License 2.0
51 stars 77 forks source link

OPA Engine Quirks #281

Open fitzthum opened 6 months ago

fitzthum commented 6 months ago

I am not an expert on OPA, but I have noticed a few weird things about our implementation.

First, it seems like we probably have some duplicate code between the AS and KBS. Should we pull the OPA engine into a crate that they can share?

Second, the version of the OPA verifier seems out of date. I haven't looked into this yet, but it seems like it does not support the same things that the online rego playground does.

Third, the set policy endpoint is very finicky. I often get errors about invalid padding when trying to upload a resource policy that works fine on the rego playground. This might be my fault, but it seems like the endpoint could be a little more robust.

Xynnn007 commented 5 months ago

Second, the version of the OPA verifier seems out of date. I haven't looked into this yet, but it seems like it does not support the same things that the online rego playground does.

We can add a monitor for OPA's go.mod in https://github.com/confidential-containers/kbs/blob/main/.github/dependabot.yml. The bot can help to update the OPA version.

wdyt? @jialez0

bpradipt commented 5 months ago

Where can I find more details on how OPA is used with KBS?

jialez0 commented 5 months ago

Where can I find more details on how OPA is used with KBS?

@bpradipt You can refer to here: https://github.com/confidential-containers/kbs/blob/main/kbs/src/api/src/policy_engine/opa/default_policy.rego

And the following diagram maybe helpful: resource-policy

lmilleri commented 5 months ago

I can see the default policy is quite different from what we had before: https://github.com/confidential-containers/attestation-service/blob/main/attestation-service/src/policy_engine/opa/default_policy.rego. Does anyone know what is the rationale of the change?

fitzthum commented 5 months ago

I think the default attestation policy is the same https://github.com/confidential-containers/kbs/blob/main/attestation-service/attestation-service/src/policy_engine/opa/default_policy.rego

We recently changed the default resource policy.

lmilleri commented 5 months ago

Oops, I confused the two

fitzthum commented 5 months ago

@Xynnn007 and @jialez0 and maybe @anakrish

I am thinking about replacing our Go OPA stuff with Regorus. I have a couple of questions. First, Regorus doesn't support all of OPA. Some of the limitations are listed here. Notice that a lot of the missing features involve some crypto functionality. Regorus can't do things like validate a JWT. Is this a problem for us? For the KBS the attestation token will already have been validated, but it's possible that some of the claims might include other tokens.

Second, do you think I should add a new feature for OpaRegorus or should I just replace the existing Go approach. Note that the Go implementation we have doesn't really seem to work very well.

anakrish commented 5 months ago

@fitzthum

We can add any critical missing built-ins to Regorus as needed. Do let us know what they are, and which crates are preferred to implement them (say what would be the preferred crate for implementing JWT tokens). Implementing a builtin itself isn't that difficult unless the OPA's behavior of the builtin relies on Golang specific semantics.

There is also a plan to allow the user to register custom built-ins. One could do

engine.register_builtin("coco.validate_kbs_token", coco_validate_kbs_token)?;

...

fn coco_validate_kbs_token(args : &[regorus::Value]) -> Result<regorus::Value> {
 ...
}

And then use coco.validate_kbs_token in Rego policies just the same as any other built-in.

jialez0 commented 5 months ago

@Xynnn007 and @jialez0 and maybe @anakrish

I am thinking about replacing our Go OPA stuff with Regorus. I have a couple of questions. First, Regorus doesn't support all of OPA. Some of the limitations are listed here. Notice that a lot of the missing features involve some crypto functionality. Regorus can't do things like validate a JWT. Is this a problem for us? For the KBS the attestation token will already have been validated, but it's possible that some of the claims might include other tokens.

Second, do you think I should add a new feature for OpaRegorus or should I just replace the existing Go approach. Note that the Go implementation we have doesn't really seem to work very well.

In our scenario, I think it is OK to replace the Go implementation. At least so far, the missing features of Regorus has no impact on us.

anakrish commented 5 months ago

There is also a plan to allow the user to register custom built-ins. One could do

engine.register_builtin("coco.validate_kbs_token", coco_validate_kbs_token)?;

...

fn coco_validate_kbs_token(args : &[regorus::Value]) -> Result<regorus::Value> {
 ...
}

And then use coco.validate_kbs_token in Rego policies just the same as any other built-in.

We have implemented the add_extension capability to register and use custom builtins: https://docs.rs/regorus/0.1.0/regorus/struct.Engine.html#method.add_extension

This is similar to https://www.openpolicyagent.org/docs/latest/extensions/#custom-built-in-functions-in-go

fitzthum commented 3 months ago

Btw I should have a PR for switching to Regorus ready in the next few days.