security: fail closed on gRPC TLS errors, verify dialed server identity (#12263)

* security: fail closed on misconfigured gRPC TLS

LoadServerTLS/LoadClientTLS returned plaintext credentials on every
error path once TLS was configured: a typo in tls.min_version or
tls.cipher_suites, cert and key in different directories, or cert/key
set without grpc.ca all left the component serving or dialing
plaintext. A partially configured TLS setup now stops the process at
startup instead; leaving cert/key unset still means no TLS, as before.

The loaders also stat cert/key/ca files up front since the pemfile
providers read lazily and would otherwise only fail at handshake time.

Mirrored in the Rust volume server: partial grpc.volume cert/key, a
missing grpc.ca, or a rustls build failure now abort startup rather
than warn-and-continue on plaintext.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* security: optional server-identity allowlist for gRPC clients

The mTLS client accepts any certificate the configured CA signs, so a
per-workload CA lets one component stand in for any other: a filer
identity can answer as the master. The server side already screens
callers via allowed_commonNames and allowed_wildcard_domain; nothing
screened the server.

LoadClientTLS now wires AdditionalPeerVerification when the caller
configures a server allowlist: grpc.<component>.allowed_server_commonNames
(falling back to a shared grpc.allowed_server_commonNames) matched
against the peer cert's subject CN, DNS SANs, or URI SANs such as a
SPIFFE id, plus a grpc.allowed_server_wildcard_domain suffix match.
Leaving them unset keeps the previous behavior.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* security: URI SANs are exact-match only in the server identity allowlist

A URI identity whose path happens to end in the wildcard domain would
otherwise pass the DNS suffix check.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* security: tighten gRPC TLS fail-closed and server-identity paths

- URI SANs match the server-identity allowlist exactly; the wildcard
  domain suffix check applies only to CN and DNS SANs.
- A partially set client_cert/client_key pair is an error when the
  cert/key fallback is empty, instead of dialing plaintext.
- Cert/key/ca files are loaded and parsed at startup (LoadX509KeyPair
  and CA PEM validation) since pemfile providers read lazily.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* rust volume: error on a partial grpc client_cert/client_key pair

Mirror the Go loadClientTLS path: when client_cert or client_key is
set without its mate and the cert/key fallback is empty, fail instead
of dialing plaintext.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* security: wildcard domain match requires a valid hostname

A CA-signed identity like ".example.com" satisfied the bare suffix check
for wildcard ".example.com" despite not being a hostname. Require a real
subdomain: non-empty label before the suffix, a '.' boundary when the
configured wildcard has no leading dot, and DNS-name syntax. A leading
"*." label still matches so wildcard certs keep working. Applied to both
the Go server/client authenticators and the Rust volume's client-cert
verifier.

* security: ignore a trailing root dot in wildcard domain matching

* tls: reject a partial dedicated client cert/key pair instead of falling back

When exactly one of component.client_cert/client_key is set, warn-and-
fallback silently dialed with the serving identity. Fail closed like the
other TLS misconfigurations in this change; mirrored in the Rust loader.
Regression tests cover a partial pair with a valid fallback.

* rust volume: harden partial client cert pair test with real pem files

* security: verify dialed server identity per target kind on the filer

The filer reused one grpc.filer-verifying dial option for master, volume,
filer and s3 targets, so a broad allowlist accepted the wrong component at
the wrong port and a narrow one rejected legitimate peers. Outbound calls
now pick grpc.<target>.allowed_server_commonNames (falling back to the
client component's and the global list) while still presenting the
grpc.filer client identity.

* security: scope per-target server allowlists under the client component

The filer read grpc.volume.allowed_server_commonNames when dialing a
volume, but that key already denotes the volume's own outbound allowlist
(e.g. the master identity). A deployment restricting volume->master
calls would make the filer reject real volume servers. Per-target policy
now lives under the client component: grpc.filer.volume_*
falls back to grpc.filer.* then grpc.*.

---------

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Chris LuandDevin authored and GitHub committed 2026-10-30 20:04:45 -07:00
1 parent 7d8b72c2b4
commit e98adfd3bb
15 files changed
+821 -101

No files matched your search

+44 -11
View File
@@ -60,21 +60,29 @@ pub fn load_outgoing_grpc_tls(
config: &VolumeServerConfig,
) -> Result<Option<OutgoingGrpcTlsConfig>, GrpcClientError> {
// prefer a dedicated client certificate: CAs may issue certs with only one of the serverAuth/clientAuth EKUs
let (cert_file, key_file) = if !config.grpc_client_cert_file.is_empty()
&& !config.grpc_client_key_file.is_empty()
{
if config.grpc_client_cert_file.is_empty() != config.grpc_client_key_file.is_empty() {
return Err(GrpcClientError(
"grpc.volume.client_cert and grpc.volume.client_key must both be set".to_string(),
));
}
let (cert_file, key_file) = if !config.grpc_client_cert_file.is_empty() {
(&config.grpc_client_cert_file, &config.grpc_client_key_file)
} else {
if !config.grpc_client_cert_file.is_empty() || !config.grpc_client_key_file.is_empty() {
tracing::warn!(
"grpc.volume.client_cert and grpc.volume.client_key must both be set, falling back to grpc.volume.cert and grpc.volume.key"
);
}
(&config.grpc_cert_file, &config.grpc_key_file)
};
if cert_file.is_empty() || key_file.is_empty() || config.grpc_ca_file.is_empty() {
if cert_file.is_empty() && key_file.is_empty() {
return Ok(None);
}
if cert_file.is_empty() || key_file.is_empty() {
return Err(GrpcClientError(
"grpc.volume.cert and grpc.volume.key must both be set".to_string(),
));
}
if config.grpc_ca_file.is_empty() {
return Err(GrpcClientError(
"grpc.volume cert and key are set but grpc.ca is not".to_string(),
));
}
let cert_pem = std::fs::read_to_string(cert_file).map_err(|e| {
GrpcClientError(format!(
@@ -452,10 +460,21 @@ mod tests {
}
#[test]
fn test_load_outgoing_grpc_tls_requires_cert_key_and_ca() {
fn test_load_outgoing_grpc_tls_unconfigured_is_plaintext() {
let config = sample_config();
assert!(load_outgoing_grpc_tls(&config).unwrap().is_none());
}
#[test]
fn test_load_outgoing_grpc_tls_rejects_partial_config() {
let mut config = sample_config();
config.grpc_cert_file = "/tmp/client.pem".to_string();
assert!(load_outgoing_grpc_tls(&config).unwrap().is_none());
assert!(load_outgoing_grpc_tls(&config).is_err());
let mut config = sample_config();
config.grpc_cert_file = "/tmp/client.pem".to_string();
config.grpc_key_file = "/tmp/client.key".to_string();
assert!(load_outgoing_grpc_tls(&config).is_err());
}
fn write_pem_files(dir: &tempfile::TempDir, config: &mut VolumeServerConfig) {
@@ -469,6 +488,20 @@ mod tests {
config.grpc_ca_file = write("ca.pem", "ca");
}
#[test]
fn test_load_outgoing_grpc_tls_rejects_partial_client_pair() {
let dir = tempfile::TempDir::new().unwrap();
let mut config = sample_config();
write_pem_files(&dir, &mut config);
config.grpc_client_cert_file = "/tmp/client.pem".to_string();
let err = load_outgoing_grpc_tls(&config).unwrap_err();
assert_eq!(
err.to_string(),
"grpc.volume.client_cert and grpc.volume.client_key must both be set"
);
}
#[test]
fn test_load_outgoing_grpc_tls_prefers_client_cert() {
let dir = tempfile::TempDir::new().unwrap();