Skip to content

Commit 76641c1

Browse files
committed
siguldry: reject configs with unknown keys
Similar to the change in PR #174, reject config files with unknown keys. Signed-off-by: Jeremy Cline <jeremycline@microsoft.com>
1 parent 4c86873 commit 76641c1

File tree

4 files changed

+169
-2
lines changed

4 files changed

+169
-2
lines changed

siguldry/src/bridge.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use crate::{
2626

2727
/// Configuration for the siguldry bridge.
2828
#[derive(Debug, Clone, Serialize, Deserialize)]
29+
#[serde(deny_unknown_fields)]
2930
pub struct Config {
3031
/// The socket address to listen on for incoming connections from Siguldry servers.
3132
///

siguldry/src/client/config.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::config::Credentials;
1212

1313
/// Configuration for the siguldry client.
1414
#[derive(Debug, Clone, Serialize, Deserialize)]
15+
#[serde(deny_unknown_fields)]
1516
pub struct Config {
1617
/// The Siguldry server hostname. This is used to validate the server's TLS certificate.
1718
pub server_hostname: String,
@@ -67,6 +68,7 @@ impl std::fmt::Display for Config {
6768

6869
/// A key to unlock for the client
6970
#[derive(Debug, Clone, Serialize)]
71+
#[serde(deny_unknown_fields)]
7072
pub struct Key {
7173
/// The name of the key in the Siguldry server.
7274
pub key_name: String,
@@ -158,4 +160,58 @@ mod tests {
158160

159161
Ok(())
160162
}
163+
164+
#[test]
165+
fn config_extra_key() -> anyhow::Result<()> {
166+
let config = r#"
167+
server_hostname = "server.example.com"
168+
bridge_hostname = "bridge.example.com"
169+
bridge_port = 44333
170+
another_key = 42
171+
172+
[request_timeout]
173+
secs = 30
174+
nanos = 0
175+
176+
[credentials]
177+
private_key = "siguldry.client.private_key.pem"
178+
certificate = "/etc/siguldry/client.cert"
179+
ca_certificate = "/etc/siguldry/ca.crt"
180+
"#;
181+
182+
if let Err(error) = toml::from_str::<super::Config>(config) {
183+
assert!(error.message().contains("unknown field `another_key`"));
184+
} else {
185+
panic!("Config should fail to load");
186+
}
187+
188+
Ok(())
189+
}
190+
191+
#[test]
192+
fn timeout_extra_key() -> anyhow::Result<()> {
193+
let config = r#"
194+
server_hostname = "server.example.com"
195+
bridge_hostname = "bridge.example.com"
196+
bridge_port = 44333
197+
198+
[request_timeout]
199+
secs = 30
200+
nanos = 0
201+
another_key = 42
202+
203+
[credentials]
204+
private_key = "siguldry.client.private_key.pem"
205+
certificate = "/etc/siguldry/client.cert"
206+
ca_certificate = "/etc/siguldry/ca.crt"
207+
"#;
208+
209+
if let Err(error) = toml::from_str::<super::Config>(config) {
210+
assert!(error.message().contains("unknown field `another_key`"));
211+
} else {
212+
panic!("Config should fail to load");
213+
}
214+
215+
Ok(())
216+
}
161217
}

siguldry/src/config.rs

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
// SPDX-License-Identifier: MIT
22
// Copyright (c) Microsoft Corporation.
3-
4-
use std::{env, path::PathBuf};
3+
#[cfg(feature = "cli")]
4+
use std::env;
5+
use std::path::PathBuf;
56

67
use anyhow::Context;
78
use openssl::{
@@ -16,6 +17,7 @@ use serde::{Deserialize, Serialize};
1617
/// only accessible to the service using it. If the paths provided are relative, it is assumed
1718
/// to be relative to the `$CREDENTIALS_DIRECTORY` environment variable.
1819
#[derive(Debug, Clone, Serialize, Deserialize)]
20+
#[serde(deny_unknown_fields)]
1921
pub struct Credentials {
2022
/// The systemd credentials ID of the PEM-encoded private key file.
2123
///
@@ -192,3 +194,29 @@ where
192194
},
193195
)
194196
}
197+
198+
#[cfg(test)]
199+
mod tests {
200+
use super::Credentials;
201+
202+
#[test]
203+
fn creds_extra_key() -> anyhow::Result<()> {
204+
let config = r#"
205+
private_key = "privkey.pem"
206+
certificate = "cert.pem"
207+
ca_certificate = "ca.pem"
208+
ca_key = "ca_key.pem"
209+
"#;
210+
211+
if let Err(error) = toml::from_str::<Credentials>(config) {
212+
assert_eq!(
213+
error.message(),
214+
"unknown field `ca_key`, expected one of `private_key`, `certificate`, `ca_certificate`"
215+
);
216+
} else {
217+
panic!("Config should fail to load");
218+
}
219+
220+
Ok(())
221+
}
222+
}

siguldry/src/server/config.rs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::config::Credentials;
1010

1111
/// Configuration for the siguldry server.
1212
#[derive(Debug, Clone, Serialize, Deserialize)]
13+
#[serde(deny_unknown_fields)]
1314
pub struct Config {
1415
/// The location where the server should store its state.
1516
///
@@ -83,6 +84,7 @@ pub struct Config {
8384
///
8485
/// The user provides the common name to use, all other values are defined here.
8586
#[derive(Debug, Clone, Serialize, Deserialize)]
87+
#[serde(deny_unknown_fields)]
8688
pub struct X509SubjectName {
8789
pub country: String,
8890
pub state_or_province: String,
@@ -108,6 +110,7 @@ impl Default for X509SubjectName {
108110
/// The server encrypts the secret needed to use a signing key with a user-provided password. It
109111
/// then encrypts _that_ with one or more secrets accessible only to the server.
110112
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
113+
#[serde(deny_unknown_fields)]
111114
pub struct Pkcs11Binding {
112115
/// The PEM-encoded X509 certificate to use to encrypt secrets.
113116
pub certificate: PathBuf,
@@ -193,4 +196,83 @@ mod tests {
193196

194197
Ok(())
195198
}
199+
200+
#[test]
201+
fn pkcs11_bindings_extra_key() -> anyhow::Result<()> {
202+
let config = r#"
203+
certificate = "cert.pem"
204+
private_key = "pkcs11:token"
205+
other_key = 42
206+
"#;
207+
208+
if let Err(error) = toml::from_str::<super::Pkcs11Binding>(config) {
209+
assert_eq!(
210+
error.message(),
211+
"unknown field `other_key`, expected `certificate` or `private_key`"
212+
);
213+
} else {
214+
panic!("Config should fail to load");
215+
}
216+
217+
Ok(())
218+
}
219+
220+
#[test]
221+
fn x509_subject_name_extra_key() -> anyhow::Result<()> {
222+
let config = r#"
223+
other_key = 42
224+
country = "US"
225+
state_or_province = "Maryland"
226+
locality = "Bethesda"
227+
organization = "Cat Caretaker"
228+
organizational_unit = "Primary Cat Scratcher"
229+
"#;
230+
231+
if let Err(error) = toml::from_str::<super::X509SubjectName>(config) {
232+
assert_eq!(
233+
error.message(),
234+
"unknown field `other_key`, expected one of \
235+
`country`, `state_or_province`, `locality`, `organization`, `organizational_unit`"
236+
);
237+
} else {
238+
panic!("Config should fail to load");
239+
}
240+
241+
Ok(())
242+
}
243+
244+
#[test]
245+
fn config_extra_key() -> anyhow::Result<()> {
246+
let config = r#"
247+
state_directory = "/var/lib/siguldry/"
248+
bridge_hostname = "bridge.example.com"
249+
bridge_port = 44333
250+
connection_pool_size = 16
251+
user_password_length = 64
252+
openpgp_user_id = "Fedora <fedora-openpgp@fedoraproject.org>"
253+
another_key = 42
254+
255+
pkcs11_bindings = []
256+
257+
[credentials]
258+
private_key = "siguldry.server.private_key.pem"
259+
certificate = "/etc/siguldry/server.cert"
260+
ca_certificate = "/etc/siguldry/ca.crt"
261+
262+
[certificate_subject]
263+
country = "US"
264+
state_or_province = "Maryland"
265+
locality = "Bethesda"
266+
organization = "Cat Caretaker"
267+
organizational_unit = "Primary Cat Scratcher"
268+
"#;
269+
270+
if let Err(error) = toml::from_str::<super::Config>(config) {
271+
assert!(error.message().contains("unknown field `another_key`"));
272+
} else {
273+
panic!("Config should fail to load");
274+
}
275+
276+
Ok(())
277+
}
196278
}

0 commit comments

Comments
 (0)