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.

Sign up for GitHub

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

Jump to bottom

feat!: renaming of TLS configuration parameters #1503

Merged
merged 8 commits into from
Oct 4, 2024
Merged

feat!: renaming of TLS configuration parameters #1503

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
12a74cf
feat!: renaming of TLS configuration parameters
gabrik Oct 3, 2024
898b975
chore: fix typo
gabrik Oct 3, 2024
3d5da78
fix: setting proper default value in default config file
gabrik Oct 3, 2024
2226121
fix: using default values for verify_name_on_connect
gabrik Oct 3, 2024
22ee3f9
chore: fix default config
gabrik Oct 3, 2024
bf10a91
chore: updating error strings
gabrik Oct 4, 2024
55f41b2
chore: updating error strings
gabrik Oct 4, 2024
d82231c
style: make cargo fmt happy
gabrik Oct 4, 2024
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
25 changes: 13 additions & 12 deletions DEFAULT_CONFIG.json5
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,20 +431,21 @@
/// or the client's keys and certificates, depending on the node's mode. If not specified
/// on router mode then the default WebPKI certificates are used instead.
root_ca_certificate: null,
/// Path to the TLS server private key
server_private_key: null,
/// Path to the TLS server public certificate
server_certificate: null,
/// Client authentication, if true enables mTLS (mutual authentication)
client_auth: false,
/// Path to the TLS client private key
client_private_key: null,
/// Path to the TLS client public certificate
client_certificate: null,
// Whether or not to use server name verification, if set to false zenoh will disregard the common names of the certificates when verifying servers.
/// Path to the TLS listening side private key
listen_private_key: null,
/// Path to the TLS listening side public certificate
listen_certificate: null,
/// Enables mTLS (mutual authentication), client authentication
enable_mtls: false,
/// Path to the TLS connecting side private key
connect_private_key: null,
/// Path to the TLS client connecting side certificate
Copy link
Member

Choose a reason for hiding this comment

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

Remove client in comment.

connect_certificate: null,
// Whether or not to verify the matching between hostname/dns and certificate when connecting,
// if set to false zenoh will disregard the common names of the certificates when verifying servers.
// This could be dangerous because your CA can have signed a server cert for foo.com, that's later being used to host a server at baz.com. If you wan't your
// ca to verify that the server at baz.com is actually baz.com, let this be true (default).
server_name_verification: null,
verify_name_on_connect: null,
Copy link
Member

Choose a reason for hiding this comment

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

The comments above indicate the value for thic config can be false or true (default).
But here it's null. Why ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH, I do not know, let me set it as true

},
},
/// Shared memory configuration.
Expand Down
20 changes: 10 additions & 10 deletions commons/zenoh-config/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,23 +463,23 @@ validated_struct::validator! {
pub tls: #[derive(Default)]
TLSConf {
root_ca_certificate: Option<String>,
server_private_key: Option<String>,
server_certificate: Option<String>,
client_auth: Option<bool>,
client_private_key: Option<String>,
client_certificate: Option<String>,
server_name_verification: Option<bool>,
listen_private_key: Option<String>,
listen_certificate: Option<String>,
enable_mtls: Option<bool>,
connect_private_key: Option<String>,
connect_certificate: Option<String>,
verify_name_on_connect: Option<bool>,
// Skip serializing field because they contain secrets
#[serde(skip_serializing)]
root_ca_certificate_base64: Option<SecretValue>,
#[serde(skip_serializing)]
server_private_key_base64: Option<SecretValue>,
listen_private_key_base64: Option<SecretValue>,
#[serde(skip_serializing)]
server_certificate_base64: Option<SecretValue>,
listen_certificate_base64: Option<SecretValue>,
#[serde(skip_serializing)]
client_private_key_base64 : Option<SecretValue>,
connect_private_key_base64 : Option<SecretValue>,
#[serde(skip_serializing)]
client_certificate_base64 : Option<SecretValue>,
connect_certificate_base64 : Option<SecretValue>,
},
pub unixpipe: #[derive(Default)]
UnixPipeConf {
Expand Down
30 changes: 15 additions & 15 deletions io/zenoh-links/zenoh-link-quic/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,24 +92,24 @@ pub mod config {
pub const TLS_ROOT_CA_CERTIFICATE_RAW: &str = "root_ca_certificate_raw";
pub const TLS_ROOT_CA_CERTIFICATE_BASE64: &str = "root_ca_certificate_base64";

pub const TLS_SERVER_PRIVATE_KEY_FILE: &str = "server_private_key_file";
pub const TLS_SERVER_PRIVATE_KEY_RAW: &str = "server_private_key_raw";
pub const TLS_SERVER_PRIVATE_KEY_BASE64: &str = "server_private_key_base64";
pub const TLS_LISTEN_PRIVATE_KEY_FILE: &str = "listen_private_key_file";
pub const TLS_LISTEN_PRIVATE_KEY_RAW: &str = "listen_private_key_raw";
pub const TLS_LISTEN_PRIVATE_KEY_BASE64: &str = "listen_private_key_base64";

pub const TLS_SERVER_CERTIFICATE_FILE: &str = "server_certificate_file";
pub const TLS_SERVER_CERTIFICATE_RAW: &str = "server_certificate_raw";
pub const TLS_SERVER_CERTIFICATE_BASE64: &str = "server_certificate_base64";
pub const TLS_LISTEN_CERTIFICATE_FILE: &str = "listen_certificate_file";
pub const TLS_LISTEN_CERTIFICATE_RAW: &str = "listen_certificate_raw";
pub const TLS_LISTEN_CERTIFICATE_BASE64: &str = "listen_certificate_base64";

pub const TLS_CLIENT_PRIVATE_KEY_FILE: &str = "client_private_key_file";
pub const TLS_CLIENT_PRIVATE_KEY_RAW: &str = "client_private_key_raw";
pub const TLS_CLIENT_PRIVATE_KEY_BASE64: &str = "client_private_key_base64";
pub const TLS_CONNECT_PRIVATE_KEY_FILE: &str = "connect_private_key_file";
pub const TLS_CONNECT_PRIVATE_KEY_RAW: &str = "connect_private_key_raw";
pub const TLS_CONNECT_PRIVATE_KEY_BASE64: &str = "connect_private_key_base64";

pub const TLS_CLIENT_CERTIFICATE_FILE: &str = "client_certificate_file";
pub const TLS_CLIENT_CERTIFICATE_RAW: &str = "client_certificate_raw";
pub const TLS_CLIENT_CERTIFICATE_BASE64: &str = "client_certificate_base64";
pub const TLS_CONNECT_CERTIFICATE_FILE: &str = "connect_certificate_file";
pub const TLS_CONNECT_CERTIFICATE_RAW: &str = "connect_certificate_raw";
pub const TLS_CONNECT_CERTIFICATE_BASE64: &str = "connect_certificate_base64";

pub const TLS_CLIENT_AUTH: &str = "client_auth";
pub const TLS_ENABLE_MTLS: &str = "enable_mtls";

pub const TLS_SERVER_NAME_VERIFICATION: &str = "server_name_verification";
pub const TLS_SERVER_NAME_VERIFICATION_DEFAULT: &str = "true";
pub const TLS_VERIFY_NAME_ON_CONNECT: &str = "verify_name_on_connect";
pub const TLS_VERIFY_NAME_ON_CONNECT_DEFAULT: &str = "true";
Copy link
Member

@JEnoch JEnoch Oct 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This TLS_VERIFY_NAME_ON_CONNECT_DEFAULT const seems never used, and doesn't exist in zenoh-link-tls

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep I noticed it while doing the changes. I think in both QUIC and TLS we should use the default value when getting from the conf.

I'll fix it

}
66 changes: 33 additions & 33 deletions io/zenoh-links/zenoh-link-quic/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,81 +62,81 @@ impl ConfigurationInspector<ZenohConfig> for TlsConfigurator {
_ => {}
}

match (c.server_private_key(), c.server_private_key_base64()) {
match (c.listen_private_key(), c.listen_private_key_base64()) {
(Some(_), Some(_)) => {
bail!("Only one between 'server_private_key' and 'server_private_key_base64' can be present!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Update error message

}
(Some(server_private_key), None) => {
ps.push((TLS_SERVER_PRIVATE_KEY_FILE, server_private_key));
ps.push((TLS_LISTEN_PRIVATE_KEY_FILE, server_private_key));
}
(None, Some(server_private_key)) => {
ps.push((
TLS_SERVER_PRIVATE_KEY_BASE64,
TLS_LISTEN_PRIVATE_KEY_BASE64,
server_private_key.expose_secret(),
));
}
_ => {}
}

match (c.server_certificate(), c.server_certificate_base64()) {
match (c.listen_certificate(), c.listen_certificate_base64()) {
(Some(_), Some(_)) => {
bail!("Only one between 'server_certificate' and 'server_certificate_base64' can be present!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Update error message

}
(Some(server_certificate), None) => {
ps.push((TLS_SERVER_CERTIFICATE_FILE, server_certificate));
ps.push((TLS_LISTEN_CERTIFICATE_FILE, server_certificate));
}
(None, Some(server_certificate)) => {
ps.push((
TLS_SERVER_CERTIFICATE_BASE64,
TLS_LISTEN_CERTIFICATE_BASE64,
server_certificate.expose_secret(),
));
}
_ => {}
}

if let Some(client_auth) = c.client_auth() {
if let Some(client_auth) = c.enable_mtls() {
match client_auth {
true => ps.push((TLS_CLIENT_AUTH, "true")),
false => ps.push((TLS_CLIENT_AUTH, "false")),
true => ps.push((TLS_ENABLE_MTLS, "true")),
false => ps.push((TLS_ENABLE_MTLS, "false")),
};
}

match (c.client_private_key(), c.client_private_key_base64()) {
match (c.connect_private_key(), c.connect_private_key_base64()) {
(Some(_), Some(_)) => {
bail!("Only one between 'client_private_key' and 'client_private_key_base64' can be present!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Update error message with new keys

}
(Some(client_private_key), None) => {
ps.push((TLS_CLIENT_PRIVATE_KEY_FILE, client_private_key));
ps.push((TLS_CONNECT_PRIVATE_KEY_FILE, client_private_key));
}
(None, Some(client_private_key)) => {
ps.push((
TLS_CLIENT_PRIVATE_KEY_BASE64,
TLS_CONNECT_PRIVATE_KEY_BASE64,
client_private_key.expose_secret(),
));
}
_ => {}
}

match (c.client_certificate(), c.client_certificate_base64()) {
match (c.connect_certificate(), c.connect_certificate_base64()) {
(Some(_), Some(_)) => {
bail!("Only one between 'client_certificate' and 'client_certificate_base64' can be present!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Update error message with new keys

}
(Some(client_certificate), None) => {
ps.push((TLS_CLIENT_CERTIFICATE_FILE, client_certificate));
ps.push((TLS_CONNECT_CERTIFICATE_FILE, client_certificate));
}
(None, Some(client_certificate)) => {
ps.push((
TLS_CLIENT_CERTIFICATE_BASE64,
TLS_CONNECT_CERTIFICATE_BASE64,
client_certificate.expose_secret(),
));
}
_ => {}
}

if let Some(server_name_verification) = c.server_name_verification() {
if let Some(server_name_verification) = c.verify_name_on_connect() {
match server_name_verification {
true => ps.push((TLS_SERVER_NAME_VERIFICATION, "true")),
false => ps.push((TLS_SERVER_NAME_VERIFICATION, "false")),
true => ps.push((TLS_VERIFY_NAME_ON_CONNECT, "true")),
false => ps.push((TLS_VERIFY_NAME_ON_CONNECT, "false")),
};
}

Expand All @@ -150,7 +150,7 @@ pub(crate) struct TlsServerConfig {

impl TlsServerConfig {
pub async fn new(config: &Config<'_>) -> ZResult<TlsServerConfig> {
let tls_server_client_auth: bool = match config.get(TLS_CLIENT_AUTH) {
let tls_server_client_auth: bool = match config.get(TLS_ENABLE_MTLS) {
Some(s) => s
.parse()
.map_err(|_| zerror!("Unknown client auth argument: {}", s))?,
Copy link
Contributor

Choose a reason for hiding this comment

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

Update error message

Expand Down Expand Up @@ -224,19 +224,19 @@ impl TlsServerConfig {
async fn load_tls_private_key(config: &Config<'_>) -> ZResult<Vec<u8>> {
load_tls_key(
config,
TLS_SERVER_PRIVATE_KEY_RAW,
TLS_SERVER_PRIVATE_KEY_FILE,
TLS_SERVER_PRIVATE_KEY_BASE64,
TLS_LISTEN_PRIVATE_KEY_RAW,
TLS_LISTEN_PRIVATE_KEY_FILE,
TLS_LISTEN_PRIVATE_KEY_BASE64,
)
.await
}

async fn load_tls_certificate(config: &Config<'_>) -> ZResult<Vec<u8>> {
load_tls_certificate(
config,
TLS_SERVER_CERTIFICATE_RAW,
TLS_SERVER_CERTIFICATE_FILE,
TLS_SERVER_CERTIFICATE_BASE64,
TLS_LISTEN_CERTIFICATE_RAW,
TLS_LISTEN_CERTIFICATE_FILE,
TLS_LISTEN_CERTIFICATE_BASE64,
)
.await
}
Expand All @@ -248,14 +248,14 @@ pub(crate) struct TlsClientConfig {

impl TlsClientConfig {
pub async fn new(config: &Config<'_>) -> ZResult<TlsClientConfig> {
let tls_client_server_auth: bool = match config.get(TLS_CLIENT_AUTH) {
let tls_client_server_auth: bool = match config.get(TLS_ENABLE_MTLS) {
Some(s) => s
.parse()
.map_err(|_| zerror!("Unknown client auth argument: {}", s))?,
Copy link
Contributor

Choose a reason for hiding this comment

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

Update error message

None => false,
};

let tls_server_name_verification: bool = match config.get(TLS_SERVER_NAME_VERIFICATION) {
let tls_server_name_verification: bool = match config.get(TLS_VERIFY_NAME_ON_CONNECT) {
Some(s) => {
let s: bool = s
.parse()
Expand Down Expand Up @@ -360,19 +360,19 @@ impl TlsClientConfig {
async fn load_tls_private_key(config: &Config<'_>) -> ZResult<Vec<u8>> {
load_tls_key(
config,
TLS_CLIENT_PRIVATE_KEY_RAW,
TLS_CLIENT_PRIVATE_KEY_FILE,
TLS_CLIENT_PRIVATE_KEY_BASE64,
TLS_CONNECT_PRIVATE_KEY_RAW,
TLS_CONNECT_PRIVATE_KEY_FILE,
TLS_CONNECT_PRIVATE_KEY_BASE64,
)
.await
}

async fn load_tls_certificate(config: &Config<'_>) -> ZResult<Vec<u8>> {
load_tls_certificate(
config,
TLS_CLIENT_CERTIFICATE_RAW,
TLS_CLIENT_CERTIFICATE_FILE,
TLS_CLIENT_CERTIFICATE_BASE64,
TLS_CONNECT_CERTIFICATE_RAW,
TLS_CONNECT_CERTIFICATE_FILE,
TLS_CONNECT_CERTIFICATE_BASE64,
)
.await
}
Expand Down
28 changes: 14 additions & 14 deletions io/zenoh-links/zenoh-link-tls/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,23 @@ pub mod config {
pub const TLS_ROOT_CA_CERTIFICATE_RAW: &str = "root_ca_certificate_raw";
pub const TLS_ROOT_CA_CERTIFICATE_BASE64: &str = "root_ca_certificate_base64";

pub const TLS_SERVER_PRIVATE_KEY_FILE: &str = "server_private_key_file";
pub const TLS_SERVER_PRIVATE_KEY_RAW: &str = "server_private_key_raw";
pub const TLS_SERVER_PRIVATE_KEY_BASE_64: &str = "server_private_key_base64";
pub const TLS_LISTEN_PRIVATE_KEY_FILE: &str = "listen_private_key_file";
pub const TLS_LISTEN_PRIVATE_KEY_RAW: &str = "listen_private_key_raw";
pub const TLS_LISTEN_PRIVATE_KEY_BASE_64: &str = "listen_private_key_base64";

pub const TLS_SERVER_CERTIFICATE_FILE: &str = "server_certificate_file";
pub const TLS_SERVER_CERTIFICATE_RAW: &str = "server_certificate_raw";
pub const TLS_SERVER_CERTIFICATE_BASE64: &str = "server_certificate_base64";
pub const TLS_LISTEN_CERTIFICATE_FILE: &str = "listen_certificate_file";
pub const TLS_LISTEN_CERTIFICATE_RAW: &str = "listen_certificate_raw";
pub const TLS_LISTEN_CERTIFICATE_BASE64: &str = "listen_certificate_base64";

pub const TLS_CLIENT_PRIVATE_KEY_FILE: &str = "client_private_key_file";
pub const TLS_CLIENT_PRIVATE_KEY_RAW: &str = "client_private_key_raw";
pub const TLS_CLIENT_PRIVATE_KEY_BASE64: &str = "client_private_key_base64";
pub const TLS_CONNECT_PRIVATE_KEY_FILE: &str = "connect_private_key_file";
pub const TLS_CONNECT_PRIVATE_KEY_RAW: &str = "connect_private_key_raw";
pub const TLS_CONNECT_PRIVATE_KEY_BASE64: &str = "connect_private_key_base64";

pub const TLS_CLIENT_CERTIFICATE_FILE: &str = "client_certificate_file";
pub const TLS_CLIENT_CERTIFICATE_RAW: &str = "client_certificate_raw";
pub const TLS_CLIENT_CERTIFICATE_BASE64: &str = "client_certificate_base64";
pub const TLS_CONNECT_CERTIFICATE_FILE: &str = "connect_certificate_file";
pub const TLS_CONNECT_CERTIFICATE_RAW: &str = "connect_certificate_raw";
pub const TLS_CONNECT_CERTIFICATE_BASE64: &str = "connect_certificate_base64";

pub const TLS_CLIENT_AUTH: &str = "client_auth";
pub const TLS_ENABLE_MTLS: &str = "enable_mtls";

pub const TLS_SERVER_NAME_VERIFICATION: &str = "server_name_verification";
pub const TLS_VERIFY_NAME_ON_CONNECT: &str = "verify_name_on_connect";
}
Loading