Skip to content

Add support for profiles in Get and Probe RPC. #213

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

Open
LarsxGitHub wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add support for profiles in Get and Probe RPC. #213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3e036b
Add support for profiles in Get and Probe RPC.
LarsxGitHub Dec 17, 2024
9e1644f
Fix lint issues.
LarsxGitHub Dec 17, 2024
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
130 changes: 87 additions & 43 deletions authz/authz.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 22 additions & 3 deletions authz/authz.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ option go_package = "github.com/openconfig/gnsi/authz";
// all other RPCs is implementation dependent.

service Authz {

// Rotate will replace an existing gRPC-level Authorization Policy on the
// target.
//
Expand Down Expand Up @@ -117,8 +116,7 @@ service Authz {
// Step 4: Final commit.
// Client ---> FinalizeRequest ----> Target
//
rpc Rotate(stream RotateAuthzRequest)
returns (stream RotateAuthzResponse);
rpc Rotate(stream RotateAuthzRequest) returns (stream RotateAuthzResponse);

// Probe allows for evaluation of the gRPC-level Authorization Policy engine
// response to a gRPC call performed by a user.
Expand Down Expand Up @@ -230,6 +228,16 @@ message ProbeRequest {
// It has to be a fully qualified name, like:
// "/gnsi.ssh.Ssh/MutateHostCredentials"
string rpc = 2;

// The profile for which the authz policy is being probed. In the case that
// this field is not specified, the default authz policy which applies to all
// gRPC endpoints run by the target is assumed. Where non-default policies
// are to supported by an endpoint, the value of the profile determines which
// set of policies are to be probed.
//
// Note that the authz profile is considered independent from a SSL profile
// ID (as referenced by gnsi.Certz).
string authz_profile_id = 3;
Copy link
Member

@robshakir robshakir Dec 18, 2024

Choose a reason for hiding this comment

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

I discussed with Marcus too. I think this makes sense to have here so that you can know exactly what authz policy you're probing.

However, can we add a clarification that says that you can only have one rotate operation in progress at once, to avoid the complexity of the state machine having to handle "what happens if you start multiple operations and then probe another one".

Copy link
Member

@robshakir robshakir Dec 18, 2024

Choose a reason for hiding this comment

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

I made a similar change for Certz here: #201

Copy link
Author

@LarsxGitHub LarsxGitHub Dec 18, 2024

Choose a reason for hiding this comment

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

Maybe I'm missing something, but the authz_profile_id in RotateAuthzRequest already has an extra docstring paragraph specifying "It is not permitted to rotate multiple profiles' policies at the same time ..." (lines 156--160).

My understanding from the current probe docstring is that it is supposed to query whatever the current engine state (for that profile) is. Even within a rotation, there is only one such engine state (either the initial one or the one after upload request), so I'm not quite sure where the state machine complexity is supposed to come from?

I think my point is: In contrast to the Rotate RPC, Probe and Get read to me as an "atomic", isolated operation rather than a state transition, so I don't see how it may complicate the state machine.

Copy link
Contributor

@marcushines marcushines Jan 6, 2025

Choose a reason for hiding this comment

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

+1

}

// ProbeResponse returns the ACK/NACK for a single user request
Expand All @@ -247,10 +255,20 @@ message ProbeResponse {

Action action = 1;
string version = 2;
string authz_profile_id = 3;
}

// GetRequest used to request the gRPC-level Authorization Policy.
message GetRequest {
// The profile for which the authz policy is being requested. In the case that
// this field is not specified, the default authz policy which applies to all
// gRPC endpoints run by the target is assumed. Where non-default policies
// are to supported by an endpoint, the value of the profile determines which
// set of policies are returned.
//
// Note that the authz profile is considered independent from a SSL profile
// ID (as referenced by gnsi.Certz).
string authz_profile_id = 1;
}

// GetResponse returns the requested instance of the gRPC-level Authorization
Expand Down Expand Up @@ -282,4 +300,5 @@ message GetResponse {
// It is provided as a JSON formatted string whose structure is defined by
// gRPC.
string policy = 3;
string authz_profile_id = 4;
}