Hi folks, We've noticed when there's a malformed C...
# help
b
Hi folks, We've noticed when there's a malformed Cerbos policy that is added or updated, it can brick the system. Here's an example of such a policy where
request.resource.attr.
followed by
[
, which is not a valid CEL identifier reference:
Copy code
apiVersion: api.cerbos.dev/v1
description: repro
rolePolicy:
  role: ug_example
  scope: tenant-uuid-placeholder.data
  parentRoles:
    - data_access
  rules:
    - resource: "*"
      allowActions: ["*"]
      condition:
        match:
          all:
            of:
              - expr: 'request.resource.attr.[category::a][key::b][type::c] in ["*"]'
Recovering from this state is difficult because it requires us to manually go into postgres to disable or delete the offending policy. Shouldn't there be input validation to avoid such things on the Cerbos side already though? Cerbos can surely catch this before reloading at runtime. Otherwise, we need to create these wrappers around it to catch it ourselves, which to me is a bandaid solution Note: We're currently on Cerbos v0.50.0 and cerbos-sdk-go v0.2.15
s
Thanks for raising. Looks like there are some inconsistencies here--between, say, disk storage and mutable backends and the validation applied to each. I'll raise an issue to address.
b
Thanks Sam!
@Sam Lock (Cerbos) Any update on this?
c
Hi. This is a bit more complicated than it looks. Invalid CEL expressions are not the only way to "brick" the PDP. There are a bunch more other cases we'd need to account for in order to ensure everything's correct. To give you some context, the origin of Cerbos and its design is based around the idea of GitOps where policy changes are validated and tested before being promoted to active. The database storage mechanism and the Admin API endpoints for writing to the database are provided as escape hatches for cases where GitOps is not easy to implement for some reason. The caveat there is that the user takes on the responsibility of ensuring that all modifications are valid because it's not efficient or practical for the PDP to do complete validation on the fly for each write. When a policy update comes through, in order to do proper validation, the PDP would have to apply those changes to the existing repo in a temporary staging area and verify that committing them doesn't leave the repo in an invalid state. Depending on the size of the repo, that's a huge I/O amplification to read all the relevant policies from the database, write them to memory or temporary storage, verify the patch and then commit the change. Then there's the problem of concurrent writes to the database while that's going on because a concurrent request could have left the database state in a way that invalidates the verification we just did. So, the tldr is that ensuring correctness of writes at the PDP level would make the Admin API writes very slow and resource intensive from the end user perspective and lot more complicated from our perspective (the maintainers) as well. That's why it's not implemented. We should probably do a better job of documenting this so that people are not surprised by it. Longer term, if there's enough demand, a reasonable compromise might be to introduce a new RPC that validates the current state of the store on demand and ejects invalid policies but nothing's being planned at the moment.
b
@Charith (Cerbos) Is manually deleting the invalid policy in postgres the only way to unbrick the system?
Having an API that developers can use to validate policies before persistence would be really helpful and would mitigate the concerns you're outlining. But we'll implement that ourselves on our side I suppose.
c
Yes, manual deletion is the only way to do it at the moment. It'd be faster to build validation yourself for the failures that you're likely to encounter in your application.
b
the business rules that brick the system is on Cerbos though. So I wouldn't say it's faster without exposed APIs