Update openconfig-system-grpc.yang - #1531
Conversation
Adding gRPC knobs to support
There was a problem hiding this comment.
Code Review
This pull request updates the openconfig-system-grpc YANG module to version 1.2.0, introducing support for mutual TLS, max concurrent streams, and gRPC server keepalive configurations. The feedback suggests adding a must constraint to ensure tls-mutual is only enabled when transport-security is active. Additionally, it recommends improving the keepalive configuration by clarifying the min-keepalive-interval description, enforcing that timeout is less than time via a must constraint, and adding a permit-without-stream parameter to prevent rate-limiting on idle connections.
|
What is the suggested naming for enforcing mTLS under the grpc-server /system/grpc-servers/grpc-server/authenticate-client |
| are not supported, such as lab testing."; | ||
| } | ||
|
|
||
| leaf tls-mutual { |
There was a problem hiding this comment.
i would add one more container "tls" under config so that we can extend it to add ssl/tls-profile(certz reference) and other TLS specific configs which can help me control version, ciphers, signature algorithms etc. Please check the feasibility of this approach
There was a problem hiding this comment.
As it stands, this leaf is not enough for mutual auth. The schema needs to be mindful of both any gNSI approaches as well as schema-driven profiles/references (the latter being the predominant deployment across secured gRPC transports today)
Agree that TLS is in need of its own hierarchy to consolidate all TLS related parameters
e.g. JUNOS/EVO
# set system services http servers server S1 tls ?
Possible completions:
<[Enter]> Execute this command
+ apply-groups Groups from which to inherit configuration data
+ apply-groups-except Don't inherit configuration data from these groups
local-certificate Name of local X.509 certificate to use
> mutual-authentication Enable TLS mutual authentication
| Pipe through a command
[edit]
# set system services http servers server S1 tls mutual-authentication ?
Possible completions:
<[Enter]> Execute this command
+ apply-groups Groups from which to inherit configuration data
+ apply-groups-except Don't inherit configuration data from these groups
authentication-type Specify requirements for client certificate
certificate-authority Certificate authority profile
| Pipe through a command
[edit]
# set system services http servers server S1 tls mutual-authentication authentication-type ?
Possible completions:
dont-request-cert Client certificate is not requested
request-cert-but-dont-verify Request certificate from client, do not verify
request-cert-and-verify Request certificate from client, verify if provided
request-and-require-cert-but-dont-verify Client certificate is mandatory, do not verify
request-and-require-cert-and-verify Client certificate is mandatory, certificate is verified
| are not supported, such as lab testing."; | ||
| } | ||
|
|
||
| leaf tls-mutual { |
There was a problem hiding this comment.
As it stands, this leaf is not enough for mutual auth. The schema needs to be mindful of both any gNSI approaches as well as schema-driven profiles/references (the latter being the predominant deployment across secured gRPC transports today)
Agree that TLS is in need of its own hierarchy to consolidate all TLS related parameters
e.g. JUNOS/EVO
# set system services http servers server S1 tls ?
Possible completions:
<[Enter]> Execute this command
+ apply-groups Groups from which to inherit configuration data
+ apply-groups-except Don't inherit configuration data from these groups
local-certificate Name of local X.509 certificate to use
> mutual-authentication Enable TLS mutual authentication
| Pipe through a command
[edit]
# set system services http servers server S1 tls mutual-authentication ?
Possible completions:
<[Enter]> Execute this command
+ apply-groups Groups from which to inherit configuration data
+ apply-groups-except Don't inherit configuration data from these groups
authentication-type Specify requirements for client certificate
certificate-authority Certificate authority profile
| Pipe through a command
[edit]
# set system services http servers server S1 tls mutual-authentication authentication-type ?
Possible completions:
dont-request-cert Client certificate is not requested
request-cert-but-dont-verify Request certificate from client, do not verify
request-cert-and-verify Request certificate from client, verify if provided
request-and-require-cert-but-dont-verify Client certificate is mandatory, do not verify
request-and-require-cert-and-verify Client certificate is mandatory, certificate is verified
| is detected."; | ||
| } | ||
|
|
||
| leaf min-keepalive-interval { |
There was a problem hiding this comment.
| leaf min-keepalive-interval { | |
| leaf min-interval { |
Already under keepalive container
| leaf tls-mutual { | ||
| type boolean; | ||
| default false; | ||
| must "not(../transport-security) or ../transport-security = 'true'" { |
There was a problem hiding this comment.
This might be the time to question transport-security defaulting to true. Great idea to enforce secure channels but there are dependencies that need to be fulfilled prior to that assumption.
This should be able to be conveyed with just:
| must "not(../transport-security) or ../transport-security = 'true'" { | |
| must "../transport-security = 'true'" { |
However default=true is imo problematic in reality
| description | ||
| "Configuration parameters relating to gRPC server keepalive."; | ||
|
|
||
| leaf time { |
There was a problem hiding this comment.
| leaf time { | |
| leaf interval { |
| are not supported, such as lab testing."; | ||
| } | ||
|
|
||
| leaf tls-mutual { |
There was a problem hiding this comment.
In two of the referenced implementations (Nokia and Arista), mTLS is configured exclusively at the TLS-profile level.
Although a case can be made for placing it at the per-server level, I don’t see any rationale for choosing this approach in the description.
|
|
||
| leaf time { | ||
| type uint32 { | ||
| range "1..max"; |
There was a problem hiding this comment.
This probably can start at 0 (where 0 means disabled)?
Parameter Mapping
tls mutual/system/grpc-servers/grpc-server/config/tls-mutualbooleantruefalsekeepalive time 5/system/grpc-servers/grpc-server/keepalive/config/timeuint32(range1..max, seconds)5keepalive min-keepalive-interval 5/system/grpc-servers/grpc-server/keepalive/config/min-keepalive-intervaluint32(range1..max, seconds)5max-concurrent-streams 128/system/grpc-servers/grpc-server/config/max-concurrent-streamsuint32(range1..max)128Configuration Example
{ "openconfig-system:system": { "openconfig-system-grpc:grpc-servers": { "grpc-server": [ { "name": "gnxi-mgmt", "config": { "name": "gnxi-mgmt", "port": 9339, "transport-security": true, "tls-mutual": true, "max-concurrent-streams": 128, }, "keepalive": { "config": { "time": 5, "min-keepalive-interval": 5 } } } ] } } }Nokia Implementation
Nokia implementation relays on TLS profile being configured with
authenticate-clientCisco Implementation
Cisco implementations relays not just on having the profile saved to harddisk (the configuration of the profile does not shows in the configuration) rather the gRPC server needs to be aligned to the content of the profile and adding mtls dual under each server configuration.
Arista implementation