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

gnmi: standardise metadata username/password keys #183

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

Conversation

brianneville
Copy link

Currently, there is no standard for what keys should be used in the metadata to identify gNMI/gNOI username and password when doing authentication.

By convention, the username and password are provided in the metadata with keys "username" and "password".
(see examples below).
It would be good to formalise this into the gNMI authentication reference spec here so that gNMI clients and servers can both be designed with this consensus.

Some examples of current gNMI clients providing "username" and "password" as keys in metadata:

  1. gnmi_cli client: https://github.com/openconfig/gnmi/blob/d5360e33fc3b22effeaaaf55f345c1f950765bed/client/gnmi/credentials.go#L35

  2. goarista client: https://github.com/aristanetworks/goarista/blob/6112fea8c7e028c64ae52412952b0a2142b8c2e9/gnmi/client.go#L311

  3. gnmic client: https://github.com/karimra/gnmic/blob/d1b198f67a99fe2f228d6119569310ebb1a50be3/target/target.go#L140
    Also note the other usages of
    metadata.AppendToOutgoingContext(ctx, "username", *t.Config.Username) and
    metadata.AppendToOutgoingContext(ctx, "password", *t.Config.Password)

  4. pygnmi client https://github.com/akarneliuk/pygnmi/blob/3090d23ae32658026a244390a296f6ade01e9fb3/pygnmi/client.py#L54

Currently, there is no standard for what keys should be
used in the metadata to identify gNMI/gNOI username and
password when doing authentication.

By convention, the username and password are provided in
the metadata with keys "username" and "password".
(see examples below).
It would be good to formalise this into the gNMI
authentication reference spec here so that gNMI clients
and servers can both be designed with this consensus.

Some examples of current gNMI clients providing "username"
and "password" as keys in metadata:

1. gnmi_cli client:
https://github.com/openconfig/gnmi/blob/d5360e33fc3b22effeaaaf55f345c1f950765bed/client/gnmi/credentials.go#L35

2. goarista client:
https://github.com/aristanetworks/goarista/blob/6112fea8c7e028c64ae52412952b0a2142b8c2e9/gnmi/client.go#L311

3. gnmic client:
https://github.com/karimra/gnmic/blob/d1b198f67a99fe2f228d6119569310ebb1a50be3/target/target.go#L140
Note the other usages of
metadata.AppendToOutgoingContext(ctx, "username", *t.Config.Username)
and
metadata.AppendToOutgoingContext(ctx, "password", *t.Config.Password)

4. pygnmi client
https://github.com/akarneliuk/pygnmi/blob/3090d23ae32658026a244390a296f6ade01e9fb3/pygnmi/client.py#L54
Comment on lines +58 to +67
#### Metadata standards
<ul>
gRPC specifies <a href="https://grpc.io/docs/what-is-grpc/core-concepts/#metadata">metadata</a> as a list of arbitrary key/value pairs.<br>
The following metadata key/value pairs are therefore standardised across gNMI/gNOI for cases where metadata is involved in authentication:

| Key | Value |
|--------------|-----------------|
| `"username"` | client username |
| `"password"` | client password |
</ul>
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: there seems to be no need in using HTML inside this markdown
ul and br can be omitted

Copy link
Author

@brianneville brianneville Mar 20, 2023

Choose a reason for hiding this comment

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

They're just being used to indent the table and text to be aligned with the other lists above
for example:

heading

    some text
    Key Value
    "username" client username
    "password" client password

vs

heading

some text

Key Value
"username" client username
"password" client password

I think the first one looks a bit nicer so I'm inclined to keep it, but I can remove this if you want - let me know

@brianneville brianneville requested a review from hellt March 27, 2023 17:56
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.

2 participants