search

openconfig / public

Repository for publishing OpenConfig models, documentation, and other material for the community.
Apache License 2.0
895 stars 655 forks source link

Consider aligning gNSI's `ssl-profile-id` leaf with `grpc-server` key after moving to a system-wide subtree #1050

Closed wenovus closed 1 month ago

wenovus commented 8 months ago

@earies I have linked this this issue in https://github.com/openconfig/public/pull/1037

Original comment thread: https://github.com/openconfig/public/pull/1037#discussion_r1465876021

brianneville commented 8 months ago

the ssl-profile-id and grpc-server key are not representing the same information, i don't think that these can be aligned.

The ssl-profile-id is tracking an SSL profile (logical grouping of certs/CRLs/CAs) associated with a gRPC server. https://github.com/openconfig/public/blob/063f5a7a249f3f0566f695f220628556572d3156/release/models/gnsi/openconfig-gnsi-certz.yang#L186-L187 An SSL profile is not strictly linked to a single gRPC server, so aligning its name with a single gRPC server would be limiting. This independence is by design, so that if there are multiple gNMI servers they can all be secured with the same SSL profile.

for more context on this leaf, see https://github.com/openconfig/gnsi/issues/111

pranav-jnpr commented 8 months ago

Agreed that ssl-profile-id leaf and grpc-server key are not representing the same information

An SSL profile is not strictly linked to a single gRPC server, so aligning its name with a single gRPC server would be limiting. This independence is by design, so that if there are multiple gNMI servers they can all be secured with the same SSL profile.

How is an SSL profile mapped to gRPC service? The leaf brought in through https://github.com/openconfig/gnsi/issues/111, can help confirm which SSL profile is active for a particular server. However, how is the system instructed to use a particular SSL profile for a gRPC service? For instance, currently the certificate-id config leaf is used to specify the name of the certificate which must be used. With a gNSI.Certz rotate operation, an SSL profile can be added, which is a string, but how does the system determine how to activate said profile for a gRPC service?

LimeHat commented 8 months ago

With a gNSI.Certz rotate operation, an SSL profile can be added, which is a string, but how does the system determine how to activate said profile for a gRPC service?

https://github.com/openconfig/public/blob/9826cae9e0aa1b81dbe5abcbce842adcc35f7921/release/models/system/openconfig-system-grpc.yang#L162-L168

pranav-jnpr commented 8 months ago
  • Profiles are activated per-server, not per service. (each grpc server can implement one or more grpc services)
  • Services are mapped to servers

Sure, this is well-understood, I can see the confusion was created due to the mention of "Service" instead of "Server", of course service <> server mapping is implementation specific.

  • gRPC server(s) should be reconfigured to use the new profile, using the certificate-id leaf that you mentioned. In fact, if you look at the description of the leaf, it explicitly mentions gNOI.Certificates service, which is a precursor to the certZ

That definition made sense prior to Certz, but if the intent with Certz's implementation is to use the "certificate-id" to specify the "ssl-profile-id" that Certz brought in, this definition should perhaps be updated to reflect the same. The existing definition only refers to "name of the certificate".

While existing gNSI.Certz implementations are indeed using ssl-profile-id today in the "certificate-id" leaf to re-configure the gRPC server to use the new profile, certain points may require clarification based on the recent discussions, which is why the questions was asked to reinforce the current understanding -

  1. The gNSI.Certz. Recent discussions https://github.com/openconfig/gnsi/pull/167 & https://github.com/openconfig/attestz/pull/36, there could be an interpretation that a newly created SSL profile during enrollz/RotateOIakCert gets activated on the system through the enrollz process itself. Perhaps we need to wait for that proposal to be finalized, since it was still an open question on the attestz thread and multiple options were put forth.

  2. Recently Cert Deployment Options were listed in the Bootz spec README, where the last step was to rotate PROD certs, which would use the oIDevID on server side to secure the gNSI.Certz rotoation. Perhaps a subsequent gNMI.Set or Union-replace operation, which includes the updated "certificate-id" referring to the "ssl-profile-id" to re-configure the gRPC server, will still use the existing SSL profile containing the oIDevID cert.

LimeHat commented 8 months ago

That definition made sense prior to Certz, but if the intent with Certz's implementation is to use the "certificate-id" to specify the "ssl-profile-id" that Certz brought in, this definition should perhaps be updated to reflect the same. The existing definition only refers to "name of the certificate".

Agree, the name is not precise after gnsi.Certz replaced gnoi.Certificates, and perhaps the change is warranted.

IMO there's only one authoritative source of the grpc-servers state, and that is node config (which can be expressed as a part of bootConfig, or a part of the dynamic config), and that should stay this way.

All other protocols (certZ, attestz, gnsi artifacts in bootz bootstrap data) can create new profiles or modify the existing profiles, but that is decoupled from the grpc servers config (e.g. if a new profile is created, and a grpc server is configured to use the previous profile - nothing has changed in the server config, it still uses the previous profile).

brianneville commented 8 months ago

All other protocols (certZ, attestz, gnsi artifacts in bootz bootstrap data) can create new profiles or modify the existing profiles, but that is decoupled from the grpc servers config (e.g. if a new profile is created, and a grpc server is configured to use the previous profile - nothing has changed in the server config, it still uses the previous profile).

I agree that certz shouldn't really be in the business of managing grpc-server config - certz is for really just for installing/rotating the contents of an SSL profile - see motivation.

IMO there's only one authoritative source of the grpc-servers state, and that is node config (which can be expressed as a part of bootConfig, or a part of the dynamic config), and that should stay this way.

Having boot config / dynamic config remain as the driver for grpc-server config is fine, however having a vendor-neutral way to modify the dynamic config is also useful. The certificate-id leaf still has some utility in allowing network operators to modify the gRPC server config via YANG.

I also agree though that this leaf could do with a renaming and/or a new description specifying that it is really being used to set the ssl_profile_id for the server. Alternatively to avoid backwards-incompatability, a new leaf could be added to the grpc-servers/grpc-server/config tree, which would allow configuration of the ssl_profile_id? (I can make a PR for this if we think it sounds like a good idea)

LimeHat commented 8 months ago

however having a vendor-neutral way to modify the dynamic config is also useful. The certificate-id leaf still has some utility in allowing network operators to modify the gRPC server config via YANG.

I completely agree; for me, the OC representation (and any API interface, such as gNMI or gnoi.BootConfig) is just another way to work with the dynamic or the boot config.

Alternatively to avoid backwards-incompatability, a new leaf could be added to the grpc-servers/grpc-server/config tree, which would allow configuration of the ssl_profile_id?

Yes, flat-out renaming might be not the best idea; I'd be happy to support either a new leaf + deprecation, or a new description.

pranav-jnpr commented 8 months ago

Technically - ssl-profile-id config leaf and certificate-id could be mutually exclusive, and I agree - it would be useful to have a new leaf.

brianneville commented 8 months ago

Opened a new PR to add a leaf as discussed, feel free to take a look: https://github.com/openconfig/public/pull/1063

github-actions[bot] commented 1 month ago

This issue is stale because it has been open 180 days with no activity. If you wish to keep this issue active, please remove the stale label or add a comment, otherwise will be closed in 14 days.