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

Add syslog TLS #1166

Merged
merged 5 commits into from
Sep 12, 2024
Merged
Changes from all commits
Commits
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
24 changes: 23 additions & 1 deletion release/models/system/openconfig-system-logging.yang
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,13 @@ module openconfig-system-logging {
"This module defines configuration and operational state data
for common logging facilities on network systems.";

oc-ext:openconfig-version "0.6.0";
oc-ext:openconfig-version "0.7.0";

revision "2024-08-20" {
description
"Adding tls support for syslog.";
reference "0.7.0";
}

revision "2023-07-20" {
description
Expand Down Expand Up @@ -429,6 +435,22 @@ revision "2023-07-20" {
"Sets the destination port number for syslog UDP messages to
the server. The default for syslog is 514.";
}

leaf transport-security {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems we are likely at a point where it makes sense to define a common structure that is imported/used across all services that have the option for TLS (or stage as such to slow transition previous definitions while using the new structure going fwd)

If we look across the model-set today, we have a variation in definition/behavior. Some reference to profiles (SSL/TLS nomenclature mixed), some reference to certificates, defaults and description differences.

Outside of that and a bit outside of this PR but related:

For the tie to gNSI, we already have a nomenclature mismatch that should be reconciled (should normalize everything to TLS and firm up the relationship). In addition are references towards gNOI cert services as well as gNSI cert services

Unfortunately anything outside of these services then is undefined behavior and ymmv for anyone not intending to use gNSI. Since there is no OC YANG schema covering the configuration of profiles, it is loosely implied to just match up this string with some native definition in the implementation (config or otherwise)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi Ebben, thanks for the thoughtful feedback. You are not wrong! The OC data models have not ever tried to define how to install and manage certificates. In practice, managing certs is "out-of-band" of gnmi. CLI and ssh, scp, gnsi.certz or some other magic needs to be used to get a certificate on a device and name those certs so that OC can reference them. I see this as an area for improvement, but not blocking this PR.

Thanks for pointing out the gnoi.Certificate service. We just removed it as that approach was never implemented (to my knowledge) and is obsoleted by gnsi.

I support moving to TLS nomenclature. My opinion is we should introduce new TLS entities named with TLS only. For the SSL things, we should create equivalent entities with TLS nomenclature and deprecate (but not delete) the SSL names.

type boolean;
description
"Indicates if syslog transport layer security (TLS) is enabled.";
}

leaf tls-profile-id {
type string;
description
"The ID of this syslog client's TLS profile. TLS profiles are managed
using the gNSI Certz service or other certificate management service
provided by the system.";
reference
"https://github.com/openconfig/gnsi/tree/main/certz";
}
}

grouping logging-remote-state {
Expand Down
Loading