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

Update authorization examples and use of bootz #219

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

Conversation

marcushines
Copy link
Contributor

No description provided.

@marcushines marcushines requested a review from morrowc January 10, 2025 16:18

* `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

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

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

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants