Skip to content

Commit d56b453

Browse files
committed
fix(desktop): treat empty cert paths as unset; error on non-UTF-8 passphrase
More review follow-ups on the native mTLS config: - Filter set-but-empty GEOLIBRE_HTTP_CA_CERT and GEOLIBRE_HTTP_CLIENT_CERT the same way as the passphrase, so ${SECRET:-}-style empty values are treated as unset instead of read as the path "" (which would fail on fs::read and, since the guarded client is cached, permanently break every native fetch). - Surface a non-UTF-8 GEOLIBRE_HTTP_CLIENT_CERT_PASSWORD as a clear error rather than silently dropping it, since the PKCS#12 loader takes a &str passphrase. - Document that the configured client certificate is held on the shared native HTTP client and is presented to any host that requests one during the TLS handshake, so it should only be configured when the app's hosts are trusted.
1 parent ec6877a commit d56b453

2 files changed

Lines changed: 26 additions & 8 deletions

File tree

apps/geolibre-desktop/src-tauri/src/lib.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -763,9 +763,11 @@ fn client_cert_password_without_path(has_cert_path: bool, has_password: bool) ->
763763
has_password && !has_cert_path
764764
}
765765

766-
/// Load extra CA certificate(s) named by [`HTTP_CA_CERT_ENV`], if any.
766+
/// Load extra CA certificate(s) named by [`HTTP_CA_CERT_ENV`], if any. A
767+
/// set-but-empty value (common from `CA_CERT=${SECRET:-}` env interpolation) is
768+
/// treated as unset rather than read as the path `""`.
767769
fn extra_ca_certificates() -> Result<Vec<reqwest::Certificate>, String> {
768-
let Some(path) = env::var_os(HTTP_CA_CERT_ENV) else {
770+
let Some(path) = env::var_os(HTTP_CA_CERT_ENV).filter(|value| !value.is_empty()) else {
769771
return Ok(Vec::new());
770772
};
771773
let path = PathBuf::from(path);
@@ -777,14 +779,22 @@ fn extra_ca_certificates() -> Result<Vec<reqwest::Certificate>, String> {
777779

778780
/// Load the mutual-TLS client identity named by [`HTTP_CLIENT_CERT_ENV`], if any.
779781
fn client_identity() -> Result<Option<ClientIdentity>, String> {
780-
// Treat an empty passphrase as unset: env interpolation in Docker/K8s/.env
782+
// Treat missing or empty as unset: env interpolation in Docker/K8s/.env
781783
// tooling (e.g. `PASSWORD=${SECRET:-}`) commonly yields "" rather than
782784
// leaving the variable unset, which must not force the PKCS#12 path or trip
783-
// the stray-passphrase error below.
784-
let password = env::var(HTTP_CLIENT_CERT_PASSWORD_ENV)
785-
.ok()
786-
.filter(|value| !value.is_empty());
787-
let Some(path) = env::var_os(HTTP_CLIENT_CERT_ENV) else {
785+
// the stray-passphrase error below. A non-UTF-8 value is surfaced as an error
786+
// rather than dropped, since the PKCS#12 loader takes a `&str` passphrase.
787+
let password = match env::var(HTTP_CLIENT_CERT_PASSWORD_ENV) {
788+
Ok(value) if value.is_empty() => None,
789+
Ok(value) => Some(value),
790+
Err(env::VarError::NotPresent) => None,
791+
Err(env::VarError::NotUnicode(_)) => {
792+
return Err(format!("{HTTP_CLIENT_CERT_PASSWORD_ENV} is not valid UTF-8"));
793+
}
794+
};
795+
// A set-but-empty cert path is likewise treated as unset, so it does not
796+
// bypass the stray-passphrase guard below or fail later on `fs::read("")`.
797+
let Some(path) = env::var_os(HTTP_CLIENT_CERT_ENV).filter(|value| !value.is_empty()) else {
788798
if client_cert_password_without_path(false, password.is_some()) {
789799
return Err(format!(
790800
"{HTTP_CLIENT_CERT_PASSWORD_ENV} is set but {HTTP_CLIENT_CERT_ENV} is not; \

docs/architecture.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,14 @@ identities use the platform native-tls backend (SChannel on Windows, Secure
129129
Transport on macOS, OpenSSL on Linux), which also reads the OS trust store.
130130
Convert a PKCS#12 export to PEM with
131131
`openssl pkcs12 -in cert.p12 -out cert.pem -nodes` if you prefer the rustls path.
132+
A set-but-empty value for any of these variables is treated as unset.
133+
134+
The configured client certificate is held on the shared native HTTP client and
135+
is therefore **presented to any HTTPS server that requests one** during a native
136+
fetch (tile, style, and OGC hosts a project points at), not only the endpoint the
137+
certificate was issued for. TLS sends a client certificate only when the server
138+
asks for one, so this is not an unconditional disclosure, but configure a client
139+
certificate only when the hosts the app talks to are trusted.
132140

133141
## Performance: map rendering on Linux (WebKitGTK)
134142

0 commit comments

Comments
 (0)