Skip to content

Commit

Permalink
fixes based on review
Browse files Browse the repository at this point in the history
  • Loading branch information
squell committed Nov 23, 2023
1 parent 4b5488c commit cc058e1
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 9 deletions.
15 changes: 9 additions & 6 deletions ntp-proto/src/nts_record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1123,6 +1123,7 @@ struct KeyExchangeServerDecoder {
server_deny: Option<String>,
}

#[cfg(feature = "nts-pool")]
#[derive(Debug, PartialEq, Eq)]
struct RequestedKeys {
c2s: Vec<u8>,
Expand Down Expand Up @@ -1382,7 +1383,7 @@ impl KeyExchangeServer {
let algorithm = result.algorithm;
let protocol = result.protocol;

tracing::debug!(?algorithm, "selected AEAD algorithm",);
tracing::debug!(?algorithm, "selected AEAD algorithm");

#[cfg(feature = "nts-pool")]
let keys = if let Some(keys) = result.fixed_keys {
Expand All @@ -1409,12 +1410,10 @@ impl KeyExchangeServer {
.extract_nts_keys(protocol, &self.tls_connection)
.map_err(KeyExchangeError::Tls);

let send_response = || -> Result<(), KeyExchangeError> {
self.send_response(protocol, algorithm, keys?)
return match keys.and_then(|keys| {
self.send_response(protocol, algorithm, keys)
.map_err(KeyExchangeError::Io)
};

return match send_response() {
}) {
Err(e) => ControlFlow::Break(Err(e)),
Ok(()) => ControlFlow::Continue(self),
};
Expand Down Expand Up @@ -1465,6 +1464,9 @@ impl KeyExchangeServer {
// TLS only works when the server name is a DNS name; an IP address does not work
let tls_connection = rustls::ServerConnection::new(tls_config)?;

#[cfg(not(feature = "nts-pool"))]
let _ = pool_certificates;

Ok(Self {
tls_connection,
decoder: Some(KeyExchangeServerDecoder::new()),
Expand Down Expand Up @@ -2284,6 +2286,7 @@ mod test {
assert_eq!(result.port, 123);
}

#[allow(dead_code)]
enum ClientType {
Uncertified,
Certified,
Expand Down
1 change: 1 addition & 0 deletions ntpd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ ntp-proto = { workspace = true, features = ["__internal-test",] }
hardware-timestamping = []
__internal-fuzz = []
unstable_ntpv5 = ["ntp-proto/ntpv5"]
unstable_nts-pool = [ "ntp-proto/nts-pool" ]

[lib]
name = "ntpd"
Expand Down
28 changes: 25 additions & 3 deletions ntpd/src/daemon/config/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ pub struct NtsKeConfig {
pub certificate_chain_path: PathBuf,
pub private_key_path: PathBuf,
#[serde(default)]
#[cfg(feature = "unstable_nts-pool")]
pub authorized_pool_server_certificates: Vec<PathBuf>,
#[serde(default = "default_nts_ke_timeout")]
pub key_exchange_timeout_ms: u64,
Expand Down Expand Up @@ -254,7 +255,6 @@ mod tests {
listen = "0.0.0.0:4460"
certificate-chain-path = "/foo/bar/baz.pem"
private-key-path = "spam.der"
authorized-pool-server-certificates = [ "foo.pem", "bar.pem" ]
"#,
)
.unwrap();
Expand All @@ -265,11 +265,33 @@ mod tests {
test.nts_ke_server.private_key_path,
PathBuf::from("spam.der")
);
assert_eq!(test.nts_ke_server.key_exchange_timeout_ms, 1000,);
assert_eq!(test.nts_ke_server.listen, "0.0.0.0:4460".parse().unwrap(),);
}

#[cfg(feature = "unstable_nts-pool")]
#[test]
fn test_deserialize_nts_ke_pool_member() {
#[derive(Deserialize, Debug)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
struct TestConfig {
nts_ke_server: NtsKeConfig,
}

let test: TestConfig = toml::from_str(
r#"
[nts-ke-server]
listen = "0.0.0.0:4460"
certificate-chain-path = "/foo/bar/baz.pem"
private-key-path = "spam.der"
authorized-pool-server-certificates = [ "foo.pem", "bar.pem" ]
"#,
)
.unwrap();

assert_eq!(
test.nts_ke_server.authorized_pool_server_certificates,
vec![PathBuf::from("foo.pem"), PathBuf::from("bar.pem")]
);
assert_eq!(test.nts_ke_server.key_exchange_timeout_ms, 1000,);
assert_eq!(test.nts_ke_server.listen, "0.0.0.0:4460".parse().unwrap(),);
}
}
4 changes: 4 additions & 0 deletions ntpd/src/daemon/keyexchange.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ async fn run_nts_ke(
.map(rustls::Certificate)
.collect();

#[cfg_attr(not(feature = "unstable_nts-pool"), allow(unused_mut))]
let mut pool_certs: Vec<rustls::Certificate> = Vec::new();
#[cfg(feature = "unstable_nts-pool")]
for client_cert in nts_ke_config.authorized_pool_server_certificates {
let pool_certificate_file = std::fs::File::open(&client_cert).map_err(|e| {
io_error(&format!(
Expand Down Expand Up @@ -580,6 +582,7 @@ mod tests {
let nts_ke_config = NtsKeConfig {
certificate_chain_path: PathBuf::from("test-keys/end.fullchain.pem"),
private_key_path: PathBuf::from("test-keys/end.key"),
#[cfg(feature = "unstable_nts-pool")]
authorized_pool_server_certificates: pool_certs.iter().map(PathBuf::from).collect(),
key_exchange_timeout_ms: 1000,
listen: "0.0.0.0:5431".parse().unwrap(),
Expand All @@ -603,6 +606,7 @@ mod tests {
assert_eq!(result.port, 123);
}

#[cfg(feature = "unstable_nts-pool")]
#[tokio::test]
async fn key_exchange_refusal_due_to_invalid_config() {
let cert_path = "testdata/certificates/nos-nl-chain.pem";
Expand Down

0 comments on commit cc058e1

Please sign in to comment.