Skip to content

Commit 366bbc3

Browse files
authored
add core client cert validation (#273)
* update protos * update core dependencies * store new certs during setup * load new certs on startup * adjust TLS config naming * add helper for loading tls certs * update purge handler * add helper for removing files * validate received certs during setup * add interceptor to validate core cert serial * update interceptor signature * add basic mtls tests * limit tokio features * review cleanup * use shared setup function * cleanup * align setup channel approach with gateway * review fixes * update flake inputs * update dependencies * remove unused env vars * update cargo deny config * update protos * review fixes * update core dependencies * review fixes
1 parent e0d95fb commit 366bbc3

15 files changed

Lines changed: 2864 additions & 628 deletions

File tree

Cargo.lock

Lines changed: 1781 additions & 70 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ homepage = "https://github.com/DefGuard/proxy"
77
repository = "https://github.com/DefGuard/proxy"
88

99
[dependencies]
10-
defguard_certs = { git = "https://github.com/DefGuard/defguard.git", rev = "01957186101fc105803d56f1190efbdb5102df2f" }
11-
defguard_version = { git = "https://github.com/DefGuard/defguard.git", rev = "01957186101fc105803d56f1190efbdb5102df2f" }
10+
defguard_certs = { git = "https://github.com/DefGuard/defguard.git", rev = "8ac70d3157d8a47b038bd1022ab9806bda642da4" }
11+
defguard_grpc_tls = { git = "https://github.com/DefGuard/defguard.git", rev = "8ac70d3157d8a47b038bd1022ab9806bda642da4" }
12+
defguard_version = { git = "https://github.com/DefGuard/defguard.git", rev = "8ac70d3157d8a47b038bd1022ab9806bda642da4" }
13+
rustls-webpki = { version = "0.103", features = ["aws-lc-rs", "std"] }
14+
rustls-pki-types = "1"
1215
# base `axum` deps
1316
axum = { version = "0.8", features = ["ws"] }
1417
axum-client-ip = "0.7"
@@ -20,7 +23,7 @@ axum-extra = { version = "0.10", features = [
2023
axum-server = { version = "0.8", features = ["tls-rustls"] }
2124
# match axum-extra -> cookies
2225
time = { version = "0.3", default-features = false }
23-
tokio = { version = "1", features = ["macros", "rt-multi-thread"] }
26+
tokio = { version = "1", features = ["macros", "rt-multi-thread", "sync", "time"] }
2427
tokio-stream = "0.1"
2528
tower-http = { version = "0.6", features = ["fs", "trace"] }
2629
# logging/tracing
@@ -61,6 +64,9 @@ rustls = { version = "0.23", default-features = false, features = [
6164
instant-acme = { version = "0.8", features = ["hyper-rustls", "aws-lc-rs"] }
6265
reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-native-roots", "json"] }
6366

67+
[dev-dependencies]
68+
tokio = { version = "1", features = ["net"] }
69+
6470
[build-dependencies]
6571
tonic-prost-build = "0.14"
6672
vergen-git2 = { version = "9.1", features = ["build"] }

deny.toml

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ feature-depth = 1
6969
#db-urls = ["https://github.com/rustsec/advisory-db"]
7070
# A list of advisory IDs to ignore. Note that ignored advisories will still
7171
# output a note when they are encountered.
72-
ignore = []
72+
ignore = [
73+
{ id = "RUSTSEC-2023-0071", reason = "https://github.com/RustCrypto/RSA/issues/19" },
74+
]
7375
# If this is true, then cargo deny will use the git executable to fetch advisory database.
7476
# If this is false, then it uses a built-in git library.
7577
# Setting this to true can be helpful if you have special authentication requirements that cargo-deny does not support.
@@ -115,6 +117,18 @@ exceptions = [
115117
"AGPL-3.0-only",
116118
"AGPL-3.0-or-later",
117119
], crate = "defguard_certs" },
120+
{ allow = [
121+
"AGPL-3.0-only",
122+
"AGPL-3.0-or-later",
123+
], crate = "defguard_grpc_tls" },
124+
{ allow = [
125+
"AGPL-3.0-only",
126+
"AGPL-3.0-or-later",
127+
], crate = "defguard_common" },
128+
{ allow = [
129+
"AGPL-3.0-only",
130+
"AGPL-3.0-or-later",
131+
], crate = "model_derive" },
118132
]
119133

120134
# Some crates don't have (easily) machine readable licensing information,

flake.lock

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

proto

src/config.rs

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,6 @@ use std::{fs::read_to_string, net::IpAddr, path::PathBuf, time::Duration};
33
use clap::Parser;
44
use log::LevelFilter;
55
use serde::Deserialize;
6-
use url::Url;
7-
8-
fn default_url() -> Url {
9-
Url::parse("http://localhost:8080").unwrap()
10-
}
116

127
fn default_adoption_timeout() -> u64 {
138
10
@@ -29,22 +24,6 @@ pub struct EnvConfig {
2924
#[arg(long, env = "DEFGUARD_PROXY_GRPC_PORT", default_value_t = 50051)]
3025
pub grpc_port: u16,
3126

32-
#[arg(long, env = "DEFGUARD_PROXY_GRPC_CERT")]
33-
#[serde(skip_serializing)]
34-
#[deprecated(
35-
since = "2.0.0",
36-
note = "Certificates are automatically generated by Core CA"
37-
)]
38-
pub grpc_cert: Option<String>,
39-
40-
#[arg(long, env = "DEFGUARD_PROXY_GRPC_KEY")]
41-
#[serde(skip_serializing)]
42-
#[deprecated(
43-
since = "2.0.0",
44-
note = "Certificates are automatically generated by Core CA"
45-
)]
46-
pub grpc_key: Option<String>,
47-
4827
#[arg(long, env = "DEFGUARD_PROXY_LOG_LEVEL", default_value_t = LevelFilter::Info)]
4928
pub log_level: LevelFilter,
5029

@@ -54,17 +33,6 @@ pub struct EnvConfig {
5433
#[arg(long, env = "DEFGUARD_PROXY_RATELIMIT_BURST", default_value_t = 0)]
5534
pub rate_limit_burst: u32,
5635

57-
#[arg(
58-
long,
59-
env = "DEFGUARD_PROXY_URL",
60-
value_parser = Url::parse,
61-
default_value = "http://localhost:8080"
62-
)]
63-
#[serde(default = "default_url")]
64-
#[serde(skip_serializing)]
65-
#[deprecated(since = "2.0.0", note = "Public URL is generated by Core instead")]
66-
pub url: Url,
67-
6836
/// Configuration file path
6937
#[arg(long = "config", short)]
7038
#[serde(skip)]

src/grpc.rs

Lines changed: 63 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,26 @@ use std::{
1111
};
1212

1313
use axum_extra::extract::cookie::Key;
14+
use defguard_certs::{CertificateError, CertificateInfo};
15+
use defguard_grpc_tls::{certs::server_tls_config, server::certificate_serial_interceptor};
1416
use defguard_version::{
1517
ComponentInfo, DefguardComponent, Version, get_tracing_variables,
1618
server::{DefguardVersionLayer, grpc::DefguardVersionInterceptor},
1719
};
18-
use tokio::sync::{broadcast, mpsc, oneshot};
19-
use tokio_stream::wrappers::UnboundedReceiverStream;
20-
use tonic::{
21-
Request, Response, Status, Streaming,
22-
transport::{Identity, Server, ServerTlsConfig},
20+
use tokio::{
21+
fs::remove_file,
22+
sync::{broadcast, mpsc, oneshot},
2323
};
24+
use tokio_stream::wrappers::UnboundedReceiverStream;
25+
use tonic::{Request, Response, Status, Streaming, service::InterceptorLayer, transport::Server};
2426
use tower::ServiceBuilder;
2527
use tracing::Instrument;
2628

2729
use crate::{
2830
LogsReceiver, MIN_CORE_VERSION, VERSION, acme,
2931
acme::Port80Permit,
3032
error::ApiError,
31-
http::{GRPC_CERT_NAME, GRPC_KEY_NAME},
33+
http::{CORE_CLIENT_CERT_NAME, GRPC_CA_CERT_NAME, GRPC_CERT_NAME, GRPC_KEY_NAME},
3234
proto::{
3335
AcmeCertificate, AcmeChallenge, AcmeIssueEvent, AcmeLogs, AcmeProgress, AcmeStep,
3436
CoreRequest, CoreResponse, DeviceInfo, acme_issue_event, core_request, core_response,
@@ -40,9 +42,13 @@ use crate::{
4042
type ClientMap = HashMap<SocketAddr, mpsc::UnboundedSender<Result<CoreRequest, Status>>>;
4143

4244
#[derive(Debug, Clone, Default)]
43-
pub struct Configuration {
45+
pub struct TlsConfig {
4446
pub grpc_key_pem: String,
4547
pub grpc_cert_pem: String,
48+
/// PEM-encoded CA certificate used to verify Core's mTLS client certificate chain.
49+
pub grpc_ca_cert_pem: String,
50+
/// DER-encoded Core client certificate; used to extract and pin the expected serial.
51+
pub core_client_cert_der: Vec<u8>,
4652
}
4753

4854
pub(crate) struct ProxyServer {
@@ -51,7 +57,7 @@ pub(crate) struct ProxyServer {
5157
results: Arc<RwLock<HashMap<u64, oneshot::Sender<core_response::Payload>>>>,
5258
pub(crate) connected: Arc<AtomicBool>,
5359
pub(crate) core_version: Arc<Mutex<Option<Version>>>,
54-
config: Arc<Mutex<Option<Configuration>>>,
60+
tls_config: Arc<Mutex<Option<TlsConfig>>>,
5561
cookie_key: Arc<RwLock<Option<Key>>>,
5662
cert_dir: PathBuf,
5763
reset_tx: broadcast::Sender<()>,
@@ -87,7 +93,7 @@ impl ProxyServer {
8793
results: Arc::new(RwLock::new(HashMap::new())),
8894
connected: Arc::new(AtomicBool::new(false)),
8995
core_version: Arc::new(Mutex::new(None)),
90-
config: Arc::new(Mutex::new(None)),
96+
tls_config: Arc::new(Mutex::new(None)),
9197
cert_dir,
9298
reset_tx,
9399
https_cert_tx,
@@ -98,17 +104,17 @@ impl ProxyServer {
98104
}
99105
}
100106

101-
pub(crate) fn configure(&self, config: Configuration) {
107+
pub(crate) fn configure(&self, config: TlsConfig) {
102108
let mut lock = self
103-
.config
109+
.tls_config
104110
.lock()
105111
.expect("Failed to acquire lock on config mutex when applying proxy configuration");
106112
*lock = Some(config);
107113
}
108114

109-
pub(crate) fn get_configuration(&self) -> Option<Configuration> {
115+
pub(crate) fn get_tls_config(&self) -> Option<TlsConfig> {
110116
let lock = self
111-
.config
117+
.tls_config
112118
.lock()
113119
.expect("Failed to acquire lock on config mutex when retrieving proxy configuration");
114120
lock.clone()
@@ -119,19 +125,27 @@ impl ProxyServer {
119125
F: Future<Output = ()> + Send + 'static,
120126
{
121127
info!("Starting gRPC server on {addr}");
122-
let config = self.get_configuration();
123-
let (grpc_cert, grpc_key) = if let Some(cfg) = config {
124-
(cfg.grpc_cert_pem, cfg.grpc_key_pem)
125-
} else {
126-
return Err(anyhow::anyhow!("gRPC server configuration is missing"));
127-
};
128-
129-
let identity = Identity::from_pem(grpc_cert, grpc_key);
130-
let mut builder =
131-
Server::builder().tls_config(ServerTlsConfig::new().identity(identity))?;
128+
let tls_config = self
129+
.get_tls_config()
130+
.ok_or_else(|| anyhow::anyhow!("gRPC server TLS configuration is missing"))?;
131+
132+
// Extract Core client cert serial for pinning (None in no-TLS mode).
133+
let expected_serial = CertificateInfo::from_der(&tls_config.core_client_cert_der)
134+
.map_err(|e: CertificateError| anyhow::anyhow!("invalid core client cert DER: {e}"))?
135+
.serial;
136+
137+
let tls_config = server_tls_config(
138+
&tls_config.grpc_cert_pem,
139+
&tls_config.grpc_key_pem,
140+
&tls_config.grpc_ca_cert_pem,
141+
)?;
142+
let mut builder = Server::builder().tls_config(tls_config)?;
132143

133144
let own_version = Version::parse(VERSION)?;
134145
let versioned_service = ServiceBuilder::new()
146+
.layer(InterceptorLayer::new(certificate_serial_interceptor(
147+
expected_serial,
148+
)))
135149
.layer(tonic::service::InterceptorLayer::new(
136150
DefguardVersionInterceptor::new(
137151
own_version.clone(),
@@ -197,7 +211,7 @@ impl ProxyServer {
197211

198212
pub(crate) fn setup_completed(&self) -> bool {
199213
let lock = self
200-
.config
214+
.tls_config
201215
.lock()
202216
.expect("Failed to acquire lock on config mutex when checking setup status");
203217
lock.is_some()
@@ -213,7 +227,7 @@ impl Clone for ProxyServer {
213227
connected: Arc::clone(&self.connected),
214228
core_version: Arc::clone(&self.core_version),
215229
cookie_key: Arc::clone(&self.cookie_key),
216-
config: Arc::clone(&self.config),
230+
tls_config: Arc::clone(&self.tls_config),
217231
cert_dir: self.cert_dir.clone(),
218232
reset_tx: self.reset_tx.clone(),
219233
https_cert_tx: self.https_cert_tx.clone(),
@@ -343,26 +357,33 @@ impl proxy_server::Proxy for ProxyServer {
343357
debug!("Received purge request, removing gRPC certificate files");
344358
let cert_path = self.cert_dir.join(GRPC_CERT_NAME);
345359
let key_path = self.cert_dir.join(GRPC_KEY_NAME);
360+
let ca_cert_path = self.cert_dir.join(GRPC_CA_CERT_NAME);
361+
let core_client_cert_path = self.cert_dir.join(CORE_CLIENT_CERT_NAME);
362+
363+
let remove_cert_file = async |path: &std::path::Path, label: &str| -> Result<(), Status> {
364+
match remove_file(path).await {
365+
Ok(()) => {
366+
info!("Removed {label} at {}", path.display());
367+
Ok(())
368+
}
369+
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
370+
debug!("{label} not found at {}, skipping removal", path.display());
371+
Ok(())
372+
}
373+
Err(err) => {
374+
error!("Failed to remove {label} at {}: {err}", path.display());
375+
Err(Status::internal(format!("Failed to remove {label}")))
376+
}
377+
}
378+
};
346379

347-
if let Err(err) = tokio::fs::remove_file(&cert_path).await
348-
&& err.kind() != std::io::ErrorKind::NotFound
349-
{
350-
error!(
351-
"Failed to remove gRPC certificate at {:?}: {err}",
352-
cert_path
353-
);
354-
return Err(Status::internal("Failed to remove gRPC certificate"));
355-
}
356-
357-
if let Err(err) = tokio::fs::remove_file(&key_path).await
358-
&& err.kind() != std::io::ErrorKind::NotFound
359-
{
360-
error!("Failed to remove gRPC key at {:?}: {err}", key_path);
361-
return Err(Status::internal("Failed to remove gRPC key"));
362-
}
380+
remove_cert_file(&cert_path, "gRPC certificate").await?;
381+
remove_cert_file(&key_path, "gRPC key").await?;
382+
remove_cert_file(&ca_cert_path, "CA certificate").await?;
383+
remove_cert_file(&core_client_cert_path, "Core client certificate").await?;
363384

364385
*self
365-
.config
386+
.tls_config
366387
.lock()
367388
.expect("Failed to lock config mutex during purge") = None;
368389
*self

0 commit comments

Comments
 (0)