Added principal and role fields

[nmahabaleshwar]

Added principal and role fields.

[haussli]

Hi. Would you separate these changes into individual MRs and update the comment WRT the epoch in both instances in the file? I expect that the first two changes will require some discussion and settling issues/137, but the last should be easy.

[haussli]

Assuming that the UserDetail change will continue here, how is role different from group?

[nmahabaleshwar]

Hi. Would you separate these changes into individual MRs and update the comment WRT the epoch in both instances in the file? I expect that the first two changes will require some discussion and settling issues/137, but the last should be easy.

Done, this PR just has addition of new fields to the UserDetail message. Raised #141 for the other issue with timezone.

[nmahabaleshwar]

Assuming that the UserDetail change will continue here, how is role different from group?

In the comments for group, it was mentioned that it can have the privilege-level which can be an integer 0-15. But @sourcequench wanted the user role to be recorded hence we are proposing a new field. role can be implementation specific which indicates a human readable name like network-admin etc.

[haussli]

IIRC, the group field was changed to a string so that it could hold either a priv-level or something like 'network-admin'. Are both needed?

[nmahabaleshwar]

IIRC, the group field was changed to a string so that it could hold either a priv-level or something like 'network-admin'. Are both needed?

We have both pieces of information and if we want both then we recommend two separate fields. If we decide to go with just group field then its better to specify what should be recorded. Vendor-A publishes privilege-level as group, Vendor-B publishes 'role' as group, there is ambiguity.

[haussli]

That change was here: commits/7c41a353f677e259404476fab8634c34cd9d7922

I agree, two pieces of info, two fields. But, are there two pieces concurrently (both priv-lvl and role) from a particular NOS? If not, the field can hold either; the NMS must already know that a particular NOS uses one type or another, right?

If a second field is added, the comments WRT group must also be updated.

@earies should also comment here, please. IIRC, he had comment on the change to string.

[haussli]

There was further discussion about this field here:
https://github.com/openconfig/gnsi/pull/121/files/0533f1a24914336f0e6596cc87369c2414d72c0f#r1298933908

[sourcequench]

At risk of being Mr. Obvious, why would a string message called "group" be used for either a priv-lvl or a defined username or a task group? Terrible name. Further I don't see any particular relationship between these things, and proto fields are inexpensive so please let's not overload. A system role, a principle, a task group string, a user class - these are all different things. Extending the proto is the way to be able to capture that.

Predicting the likely sentiment "but we need vendor neutral fields, because openconfig". It is not helpful (to vendors, to google, to other network operators) to overload and obfuscate behind seemingly neutral fields. "But task groups is a Cisco specific thing!" to which I answer, yes, yes it is, and we need that distinct information to operate a Cisco network device.

[haussli]

To me, all of these are roughly a unix group and I do not know of a NOS that has two groups. junos user/login class and cisco xr task-group are essentially the same thing.

While I am unaware of any OC preference toward vendor-agnostic fields (meaning my OC experience is limited), I personally do not care if it is one field or multiple (or union/oneof). Choose a path and be consistent. It was the suggestion to remove priv-level from the message that initiated the change of the field, ultimately to support named-groups. The priv-level must be recorded as must the task-group, login-class, ...

Not being cheeky; a MR? Will that be a union or individual fields? What fields? priv-level (ios), task-group (xr), login-class (junos), role (EOS, Entra, OC, prob. others), admin-profile (dnos), group (unix), profile (sros), account profile (FOS), privilege-level (PANOS, different from priv-level), ....

I also do not care if the field name changes.

There was discussion about task_ids; unfortunately the proto comments were not updated to capture that discussion. Other NOS also have this concept, though likely by a different name.

[sourcequench]

Agreed, let's be consistent. We're in the early days so it probably pays to spend a bit of thought now.

Perhaps we're not being clear enough saying what "system role" is - and we'd be happy to change the name there as well, but "role" is the name of the key on a hiba grant - and in my experience people are immediately confused between the principal on a certificate and the role they are logging into. Role is not a vendor specific concept.

A system role is "the thing before the @ sign when you type ssh admin@router1. Principal is a field on the presented user certificate, right along with serial number and validity. A task group and priv-lvl are perhaps the most similar to each other, task groups being a more explicit list of tasks and levels, or tags, that represent what commands/arguments a given user can do, and the priv lvl using a simple number as an implicit representation of a similar function - this is separate from a unix group identifier as multiple groups can and would have the same task groups or priv-lvl. The only item like a group here is class, and the only opportunity for combination of mutually-exclusive similar concepts is priv-lvl and task string.

        Serial: 1
        Valid: from 2023-10-04T14:55:00 to 2024-10-04T15:56:07
        Principals: 
                haussli

While we're in here, I have some other questions. The identity field in UserInfo says it is for spiffe-id or unix-style username - but it's totally unclear to me whether this field is intended to mean "admin" or "haussli" as in the previous example. Further, the term "identity" has a different meaning in the world of ssh that makes this use even more unclear.

Perhaps another silly question, but why would we send task-group, priv-lvl, or class at all with an accounting record? We need effectively a session identifier, but at least for ssh I don't see why it is useful to send this information off to accounting in the first place.

[haussli]

Ah! I disagree about your description of task-groups, etc. But, it is now clear that the suggested role field is different from login-class, priv-lvl, etc. A role in EOS and others is essentially a group or login-class.

I think that UserInfo.identity came from the original gnmi version of accounting. It is a representation of the username on the device, ie: ssh admin@device, the identity is admin and for some x509 certificate could be something like a spiffee from which the username is derived/extracted. One way or another, its the username; use whatever fancy new name you wish, afaict. ssh docs call it the remote_username, fwiw. That username is used in various ways on modern NOSs.

So, what is a principal? it is another identifier that could be further used to permit/deny access or permissions. right? it is roughly equivalent to the spiffee or some portion of the spiffe, without having a way to derive a username from it? Or, it is equivalent, and there should be a field for spiffe/ssh-principal?

IMO, the group is important so that it is known what the priv lvl, role, etc was at the time of the acctz record. The same reason it is included in existing accounting methods.

Suggested change

[haussli]

LGTM

[sourcequench]

LGTM

[haussli]

LGTM