Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server: bundle-owned policy could be renamed #4846

Closed
srenatus opened this issue Jul 6, 2022 · 0 comments · Fixed by #4847
Closed

server: bundle-owned policy could be renamed #4846

srenatus opened this issue Jul 6, 2022 · 0 comments · Fixed by #4847
Labels

Comments

@srenatus
Copy link
Contributor

srenatus commented Jul 6, 2022

When a bundle owns a certain root, and contains policies, policies under that root could be replaced via the REST API because of a missing bundle scope check:

With a bundle manifest like

{ "roots": [ "x/y" ] }

and this policy in the bundle,

package x.y.z
foo := 1

we find:

$ curl "http://127.0.0.1:8181/v1/policies/%2fauthz%2fx%2fyz.rego"
{"result":{"id":"/authz/x/yz.rego","raw":"package x.y.z\n\nfoo := 1\n","ast":null}}
$ curl "http://127.0.0.1:8181/v1/data/x/y/z/foo"                 
{"result":1}
$ curl "http://127.0.0.1:8181/v1/policies/%2fauthz%2fx%2fyz.rego" -XDELETE
{
  "code": "invalid_parameter",
  "message": "path x/y/z is owned by bundle \"authz\""
}
$ curl "http://127.0.0.1:8181/v1/policies/%2fauthz%2fx%2fyz.rego" -XPUT --data-binary @y.rego
{}
$ curl "http://127.0.0.1:8181/v1/data/x/y/z/foo"                                             
{}
$ curl "http://127.0.0.1:8181/v1/data/a/b/foo"
{"result":2}
$

where y.rego is

package a.b
foo := 2

⚡ While the DELETE is properly protected, and PUT requests keeping the package path intact are, too; PUTs with a different target package path are processed as-is. We're missing a check here.

@srenatus srenatus added the bug label Jul 6, 2022
srenatus added a commit that referenced this issue Jul 6, 2022
Before, we'd only check if the NEW policy path was owned by a bundle. Now,
we'll also check if the to-be-updated policy is owned by a bundle. If so,
return an error.

Fixes #4846 

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
srenatus added a commit to srenatus/opa that referenced this issue Jul 7, 2022
…#4847)

Before, we'd only check if the NEW policy path was owned by a bundle. Now,
we'll also check if the to-be-updated policy is owned by a bundle. If so,
return an error.

Fixes open-policy-agent#4846 

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
ashutosh-narkar pushed a commit to ashutosh-narkar/opa that referenced this issue Jul 7, 2022
…#4847)

Before, we'd only check if the NEW policy path was owned by a bundle. Now,
we'll also check if the to-be-updated policy is owned by a bundle. If so,
return an error.

Fixes open-policy-agent#4846

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
(cherry picked from commit b2bf19f)
srenatus added a commit that referenced this issue Jul 8, 2022
Before, we'd only check if the NEW policy path was owned by a bundle. Now,
we'll also check if the to-be-updated policy is owned by a bundle. If so,
return an error.

Fixes #4846

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
(cherry picked from commit b2bf19f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant