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.

Sign up for GitHub

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

Update authorization examples and use of bootz #219

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update authorization examples and use of bootz #219

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 44 additions & 5 deletions pathz/authorization-README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ Permitted use:
```

Not permitted use:

```proto
/a/b[key=foo]/*/d
```
Expand Down Expand Up @@ -152,6 +153,45 @@ Installed Rules
Result: DENY, both paths are the same length, have the same amount of definite keys,
apply to groups that user is a member of, so prefer DENY over PERMIT.

Example 5

Installed Rules

```sh
/interfaces/interface[name=*] -> group core-controllers, action PERMIT
/interfaces/interface[name=*] -> group core-eng, action PERMIT
/interfaces/interface[name=et-1/0/1]/state/counters -> user customer-controller1, action PERMIT
/interfaces/interface[name=et-1/0/2]/state/counters -> user customer-controller2, action PERMIT
/interfaces/interface[name=et-1/0/1] -> core-controller, action DENY
/interfaces/interface[name=et-1/0/2] -> core-controller, action DENY

```

Result:

This will assume that core-controller1 is member of core-controllers
This will assume that eng1 is member of core-eng

* `gnmi.Subscribe(10.0.0.10:515253, eng1, /interfaces/interface/state/counters, ONCE)`

Subscribe will be accepted and all subtrees will be returned as eng1 is member core-eng group

* `gnmi.Subscribe(10.0.0.10:515253, customer-controller1, /interfaces/interface/state/counters, ONCE)`

Subscribe will be rejected as user does not have access at that container
Copy link

@LimeHat LimeHat Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is "rejection" in this case? Does this mean that an RPC should return an error?

I'd argue that returning an empty response for this case should be allowed (if not required). This allows for consistent behavior (and implementation) with other scenarios (for example scenario#4 gnmi.Subscribe(10.0.0.10:515253, core-contollers, /interfaces/interface/state/counters, ONCE)), as well as a graceful support for scenarios where multiple paths are queried at the same time (e.g. if the subscription list contains 1 allowed path, and 1 disallowed path in the same query).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes the subscribe should return unauthorized

how is it graceful to provide silently discard valid paths to a subscriber based on authorization rejection?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other option here would be to add a new response message type which can at least single to a client that there are paths that were in the subscription that are unable to be displayed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In chatting with eben @ juniper (and marcus previous to that) I think the idea that the caller asks for X, Y, Z and only gets X and Z is confusing to the caller. There isn't (today) a way (except 'not-authorized') for the grpc server to say: "Sorry you get 2 of 3 paths"

Is it ok longer term for callers to keep asking for things that it'll not get?
Why not just error immediately as a signal to fix the caller's configuration?

Copy link

@LimeHat LimeHat Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's consider the following example:

  • a router has two interfaces, ethernet-1 and mgmt.
  • a client has access to /interfaces/interface[name=ethernet-1], but doesn't have access to /interfaces/interface[name=mgmt] according to the policy.

if the client subscribes to /interfaces/ tree as a whole, it will receive data only for ethernet-1.
if the client chooses to subscribe to /interfaces/interface[name=ethernet-1], /interfaces/interface[name=mgmt0], according to the new description he should receive the error (while he could be getting the same data; in this example, both requests are equivalent).

this behavior discrepancy makes less sense to me. if you want to explicitly signal an error, shouldn't the first case also return an error?

Is it ok longer term for callers to keep asking for things that it'll not get?

i don't see much of a problem with it? do we have to disclose the specifics of a security policy to clients?

there's already Probe() RPC that can be used by security teams to validate policies for a specific user id, making it feasible to perform this validation out-of-band. or access to this RPC can even be given to telemetry clients.

if anything, in b/384081671 there was a similar request (not related to security policies) to allow clients to subscribe to nonexisting and non-supported paths..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it have access to /interfaces?

if they don't then they will get a reject
but if the client subscribes to /interfaces/interface[name=ethernet-1] which it does it will get the data

i basically disagree with your foundational point around what a client should see in respect to security with regards to data it doesn't have access to

I don't believe that pathz policy should be the way to prune overly broad client subscriptions.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it have access to /interfaces?

in my example it has access to 1 out of 2 interfaces, it doesn't have access to the whole /interfaces tree.

I don't believe that pathz policy should be the way to prune overly broad client subscriptions.

Maybe it shouldn't, but it's in the spec.
And also one of your examples below (the penultimate scenario):
Subscribe will be accepted, only interfaces not matching the deny rule will be returned.

If you want to remove pruning - that's probably fine. The issue I see with the proposed changes in this PR is that you still have pruning, but now it is conditional. In some cases we have to prune, in other cases we have to reject.

In my view, a uniform approach (we either always prune data that the client don't have access to, or we always reject the subscription/get requests when a client tries to retrieve data which he doesn't have access to) is a better choice.

Copy link
Member

@dplore dplore Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pruning out the un-authorized paths may be the only practical approach. Consider a fine grained pathz policy which cherry picks a list of paths throughout several subtrees in OC. Now also consider how to construct a gnmi.Subscribe which avoids these restricted paths. The gnmi.Subscribe will need to precisely enumerate all the paths that are allowed, likely to the leaf level. This could be tens of thousands of paths for a device with many interfaces.

The suggestion at #219 (comment) sounds like a good idea to add.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so can you provide an example where that is actually true?
rather than a contrived example show me a clear use case where an agent doesn't have a clear set of paths that it can make subscription to?

as far as the adding a message in subscription response to signal caller ... i would say it seems like a good idea to add the regardless... since i did suggest it...


* `gnmi.Subscribe(10.0.0.10:515253, customer-controller1, /interfaces/interface[name=et-1/0/1]/state/counters, ONCE)`

Subscribe will be accepted and all subtrees will be returned.

* `gnmi.Subscribe(10.0.0.10:515253, core-contollers, /interfaces/interface/state/counters, ONCE)`

Subscribe will be accepted, only interfaces not matching the deny rule will be returned.

* `gnmi.Subscribe(10.0.0.10:515253, core-controllers, /interfaces/interface[name=et-1/0/1]/state/counters, ONCE)`

Subscribe will be rejected due to explicit DENY rule for path.

## Bootstrap / Install Options

System bootstrap, or install, operations may include an authorization policy
Expand All @@ -160,11 +200,10 @@ process include the complete authorization policy so all production tools
and services have immediate authorized access to finish installation and
move devices into production in a timely manner.

Using the Secure Zero Touch Provisioning (sZTP - RFC8572) process for
bootstrap/installation is a recommended method for accomplishing this
delivery, and the delivery of all other bootstrap artifacts in a secure manner.


Using [Bootz](https://github.com/openconfig/bootz) or the Secure Zero Touch
Provisioning (sZTP - RFC8572)process for bootstrap/installation is a recommended
method for accomplishing this delivery, and the delivery of all other bootstrap
artifacts in a secure manner.

## An Example Authorization Protobuf

Expand Down
Loading