diff --git a/seaweed-volume/src/config.rs b/seaweed-volume/src/config.rs index d5962d2bb..edb48fec5 100644 --- a/seaweed-volume/src/config.rs +++ b/seaweed-volume/src/config.rs @@ -256,6 +256,8 @@ pub struct VolumeServerConfig { pub https_client_ca_file: String, pub grpc_cert_file: String, pub grpc_key_file: String, + pub grpc_client_cert_file: String, + pub grpc_client_key_file: String, pub grpc_ca_file: String, pub grpc_allowed_wildcard_domain: String, pub grpc_volume_allowed_common_names: Vec, @@ -805,6 +807,8 @@ fn resolve_config(cli: Cli) -> VolumeServerConfig { https_client_ca_file: sec.https_client_ca_file, grpc_cert_file: sec.grpc_cert_file, grpc_key_file: sec.grpc_key_file, + grpc_client_cert_file: sec.grpc_client_cert_file, + grpc_client_key_file: sec.grpc_client_key_file, grpc_ca_file: sec.grpc_ca_file, grpc_allowed_wildcard_domain: sec.grpc_allowed_wildcard_domain, grpc_volume_allowed_common_names: sec.grpc_volume_allowed_common_names, @@ -837,6 +841,8 @@ pub struct SecurityConfig { pub https_client_ca_file: String, pub grpc_cert_file: String, pub grpc_key_file: String, + pub grpc_client_cert_file: String, + pub grpc_client_key_file: String, pub grpc_ca_file: String, pub grpc_allowed_wildcard_domain: String, pub grpc_volume_allowed_common_names: Vec, @@ -882,6 +888,8 @@ const SECURITY_CONFIG_FILE_NAME: &str = "security.toml"; /// [grpc.volume] /// cert = "/path/to/cert.pem" /// key = "/path/to/key.pem" +/// client_cert = "/path/to/client-cert.pem" +/// client_key = "/path/to/client-key.pem" /// allowed_commonNames = "volume-a.internal,volume-b.internal" /// ``` pub fn parse_security_config(path: &str) -> SecurityConfig { @@ -1003,6 +1011,8 @@ pub fn parse_security_config(path: &str) -> SecurityConfig { Section::GrpcVolume => match key { "cert" => cfg.grpc_cert_file = value.to_string(), "key" => cfg.grpc_key_file = value.to_string(), + "client_cert" => cfg.grpc_client_cert_file = value.to_string(), + "client_key" => cfg.grpc_client_key_file = value.to_string(), // Go only reads CA from [grpc], not [grpc.volume] "allowed_commonNames" => { cfg.grpc_volume_allowed_common_names = @@ -1134,6 +1144,12 @@ fn apply_env_overrides(cfg: &mut SecurityConfig) { if let Ok(v) = std::env::var("WEED_GRPC_VOLUME_KEY") { cfg.grpc_key_file = v; } + if let Ok(v) = std::env::var("WEED_GRPC_VOLUME_CLIENT_CERT") { + cfg.grpc_client_cert_file = v; + } + if let Ok(v) = std::env::var("WEED_GRPC_VOLUME_CLIENT_KEY") { + cfg.grpc_client_key_file = v; + } if let Ok(v) = std::env::var("WEED_GRPC_CA") { cfg.grpc_ca_file = v; } else if let Ok(v) = std::env::var("WEED_GRPC_VOLUME_CA") { @@ -1231,6 +1247,8 @@ mod tests { "WEED_HTTPS_CLIENT_CA", "WEED_GRPC_VOLUME_CERT", "WEED_GRPC_VOLUME_KEY", + "WEED_GRPC_VOLUME_CLIENT_CERT", + "WEED_GRPC_VOLUME_CLIENT_KEY", "WEED_GRPC_CA", "WEED_GRPC_VOLUME_CA", "WEED_GRPC_ALLOWED_WILDCARD_DOMAIN", @@ -1500,6 +1518,35 @@ key = "/etc/seaweedfs/volume-key.pem" }); } + #[test] + fn test_parse_security_config_uses_grpc_volume_client_cert() { + let _guard = process_state_lock(); + let tmp = tempfile::NamedTempFile::new().unwrap(); + std::fs::write( + tmp.path(), + r#" +[grpc.volume] +cert = "/etc/seaweedfs/volume-cert.pem" +key = "/etc/seaweedfs/volume-key.pem" +client_cert = "/etc/seaweedfs/volume-client-cert.pem" +client_key = "/etc/seaweedfs/volume-client-key.pem" +"#, + ) + .unwrap(); + + with_cleared_security_env(|| { + let cfg = parse_security_config(tmp.path().to_str().unwrap()); + assert_eq!( + cfg.grpc_client_cert_file, + "/etc/seaweedfs/volume-client-cert.pem" + ); + assert_eq!( + cfg.grpc_client_key_file, + "/etc/seaweedfs/volume-client-key.pem" + ); + }); + } + #[test] fn test_parse_security_config_uses_grpc_peer_name_policy() { let _guard = process_state_lock(); diff --git a/seaweed-volume/src/server/grpc_client.rs b/seaweed-volume/src/server/grpc_client.rs index 6a2f44964..114840d22 100644 --- a/seaweed-volume/src/server/grpc_client.rs +++ b/seaweed-volume/src/server/grpc_client.rs @@ -33,23 +33,31 @@ impl Error for GrpcClientError {} pub fn load_outgoing_grpc_tls( config: &VolumeServerConfig, ) -> Result, GrpcClientError> { - if config.grpc_cert_file.is_empty() - || config.grpc_key_file.is_empty() - || config.grpc_ca_file.is_empty() + // 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() { + (&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() { return Ok(None); } - let cert_pem = std::fs::read_to_string(&config.grpc_cert_file).map_err(|e| { + let cert_pem = std::fs::read_to_string(cert_file).map_err(|e| { GrpcClientError(format!( "Failed to read outgoing gRPC cert '{}': {}", - config.grpc_cert_file, e + cert_file, e )) })?; - let key_pem = std::fs::read_to_string(&config.grpc_key_file).map_err(|e| { + let key_pem = std::fs::read_to_string(key_file).map_err(|e| { GrpcClientError(format!( "Failed to read outgoing gRPC key '{}': {}", - config.grpc_key_file, e + key_file, e )) })?; let ca_pem = std::fs::read_to_string(&config.grpc_ca_file).map_err(|e| { @@ -236,6 +244,8 @@ mod tests { https_client_ca_file: String::new(), grpc_cert_file: String::new(), grpc_key_file: String::new(), + grpc_client_cert_file: String::new(), + grpc_client_key_file: String::new(), grpc_ca_file: String::new(), grpc_allowed_wildcard_domain: String::new(), grpc_volume_allowed_common_names: vec![], @@ -265,6 +275,45 @@ mod tests { assert!(load_outgoing_grpc_tls(&config).unwrap().is_none()); } + fn write_pem_files(dir: &tempfile::TempDir, config: &mut VolumeServerConfig) { + let write = |name: &str, content: &str| { + let path = dir.path().join(name); + std::fs::write(&path, content).unwrap(); + path.to_str().unwrap().to_string() + }; + config.grpc_cert_file = write("server.pem", "server-cert"); + config.grpc_key_file = write("server.key", "server-key"); + config.grpc_ca_file = write("ca.pem", "ca"); + } + + #[test] + fn test_load_outgoing_grpc_tls_prefers_client_cert() { + let dir = tempfile::TempDir::new().unwrap(); + let mut config = sample_config(); + write_pem_files(&dir, &mut config); + let client_cert = dir.path().join("client.pem"); + let client_key = dir.path().join("client.key"); + std::fs::write(&client_cert, "client-cert").unwrap(); + std::fs::write(&client_key, "client-key").unwrap(); + config.grpc_client_cert_file = client_cert.to_str().unwrap().to_string(); + config.grpc_client_key_file = client_key.to_str().unwrap().to_string(); + + let tls = load_outgoing_grpc_tls(&config).unwrap().unwrap(); + assert_eq!(tls.cert_pem, "client-cert"); + assert_eq!(tls.key_pem, "client-key"); + } + + #[test] + fn test_load_outgoing_grpc_tls_falls_back_to_server_cert() { + let dir = tempfile::TempDir::new().unwrap(); + let mut config = sample_config(); + write_pem_files(&dir, &mut config); + + let tls = load_outgoing_grpc_tls(&config).unwrap().unwrap(); + assert_eq!(tls.cert_pem, "server-cert"); + assert_eq!(tls.key_pem, "server-key"); + } + #[test] fn test_build_grpc_endpoint_without_tls_uses_http_scheme() { let endpoint = build_grpc_endpoint("127.0.0.1:19333", None).unwrap(); diff --git a/seaweed-volume/src/server/profiling.rs b/seaweed-volume/src/server/profiling.rs index 51e4f174c..1ede830c9 100644 --- a/seaweed-volume/src/server/profiling.rs +++ b/seaweed-volume/src/server/profiling.rs @@ -163,6 +163,8 @@ mod tests { https_client_ca_file: String::new(), grpc_cert_file: String::new(), grpc_key_file: String::new(), + grpc_client_cert_file: String::new(), + grpc_client_key_file: String::new(), grpc_ca_file: String::new(), grpc_allowed_wildcard_domain: String::new(), grpc_volume_allowed_common_names: vec![], diff --git a/weed/command/scaffold/security.toml b/weed/command/scaffold/security.toml index 1d9832bc5..83c23fc0c 100644 --- a/weed/command/scaffold/security.toml +++ b/weed/command/scaffold/security.toml @@ -77,6 +77,12 @@ expires_after_seconds = 10 # seconds # All gRPC TLS authentications are mutual (mTLS) # The values for ca, cert, and key are paths to the certificate/key files # The host name is not checked, so the certificate files can be shared +# Each [grpc.] section also accepts optional client_cert/client_key, +# presented when that component dials other servers. Set them when your CA +# issues separate serverAuth-only and clientAuth-only certificates; when unset, +# the component reuses cert/key for both directions. +# If client and server certificates come from different issuing CAs, put both +# CA certificates in the ca PEM file. [grpc] ca = "" # Set wildcard domain for enable TLS authentication by common names @@ -87,6 +93,8 @@ allowed_wildcard_domain = "" # .mycompany.com [grpc.volume] cert = "" key = "" +client_cert = "" +client_key = "" allowed_commonNames = "" # comma-separated SSL certificate common names # Master server gRPC options (server-side) @@ -94,6 +102,8 @@ allowed_commonNames = "" # comma-separated SSL certificate common names [grpc.master] cert = "" key = "" +client_cert = "" +client_key = "" allowed_commonNames = "" # comma-separated SSL certificate common names # Filer server gRPC options (server-side) @@ -101,6 +111,8 @@ allowed_commonNames = "" # comma-separated SSL certificate common names [grpc.filer] cert = "" key = "" +client_cert = "" +client_key = "" allowed_commonNames = "" # comma-separated SSL certificate common names # S3 server gRPC options (server-side) @@ -108,31 +120,43 @@ allowed_commonNames = "" # comma-separated SSL certificate common names [grpc.s3] cert = "" key = "" +client_cert = "" +client_key = "" allowed_commonNames = "" # comma-separated SSL certificate common names [grpc.msg_broker] cert = "" key = "" +client_cert = "" +client_key = "" allowed_commonNames = "" # comma-separated SSL certificate common names [grpc.msg_agent] cert = "" key = "" +client_cert = "" +client_key = "" allowed_commonNames = "" # comma-separated SSL certificate common names [grpc.admin] cert = "" key = "" +client_cert = "" +client_key = "" allowed_commonNames = "" # comma-separated SSL certificate common names [grpc.worker] cert = "" key = "" +client_cert = "" +client_key = "" allowed_commonNames = "" # comma-separated SSL certificate common names [grpc.mq] cert = "" key = "" +client_cert = "" +client_key = "" allowed_commonNames = "" # comma-separated SSL certificate common names # gRPC client configuration for outgoing gRPC connections diff --git a/weed/security/tls.go b/weed/security/tls.go index 07f5d5cba..29b0d3c14 100644 --- a/weed/security/tls.go +++ b/weed/security/tls.go @@ -155,7 +155,7 @@ func LoadClientTLSFromFile(configFile string, component string) (grpc.DialOption } // Resolve relative PEM paths against the config file's directory. configDir := filepath.Dir(configFile) - for _, key := range []string{"grpc.ca", component + ".cert", component + ".key"} { + for _, key := range []string{"grpc.ca", component + ".cert", component + ".key", component + ".client_cert", component + ".client_key"} { p := v.GetString(key) if p != "" && !filepath.IsAbs(p) { v.Set(key, filepath.Join(configDir, p)) @@ -169,7 +169,15 @@ func LoadClientTLS(config *util.ViperProxy, component string) grpc.DialOption { return grpc.WithTransportCredentials(insecure.NewCredentials()) } - certFileName, keyFileName, caFileName := config.GetString(component+".cert"), config.GetString(component+".key"), config.GetString("grpc.ca") + // prefer a dedicated client certificate: CAs may issue certs with only one of the serverAuth/clientAuth EKUs + certFileName, keyFileName := config.GetString(component+".client_cert"), config.GetString(component+".client_key") + if certFileName == "" || keyFileName == "" { + if certFileName != "" || keyFileName != "" { + glog.Warningf("%s.client_cert and %s.client_key must both be set, falling back to %s.cert and %s.key", component, component, component, component) + } + certFileName, keyFileName = config.GetString(component+".cert"), config.GetString(component+".key") + } + caFileName := config.GetString("grpc.ca") if certFileName == "" || keyFileName == "" || caFileName == "" { return grpc.WithTransportCredentials(insecure.NewCredentials()) } diff --git a/weed/security/tls_client_cert_test.go b/weed/security/tls_client_cert_test.go new file mode 100644 index 000000000..8161a9ee2 --- /dev/null +++ b/weed/security/tls_client_cert_test.go @@ -0,0 +1,183 @@ +package security + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "net" + "os" + "path/filepath" + "testing" + "time" + + "github.com/spf13/viper" + + "github.com/seaweedfs/seaweedfs/weed/util" + "google.golang.org/grpc" + "google.golang.org/grpc/health" + healthpb "google.golang.org/grpc/health/grpc_health_v1" +) + +type testCA struct { + cert *x509.Certificate + key *ecdsa.PrivateKey + file string +} + +func newTestCA(t *testing.T, dir string) *testCA { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "test-ca"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageCertSign, + BasicConstraintsValid: true, + IsCA: true, + } + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatal(err) + } + file := filepath.Join(dir, "ca.pem") + writePem(t, file, "CERTIFICATE", der) + return &testCA{cert: cert, key: key, file: file} +} + +// issue creates a leaf certificate restricted to the given extended key usages. +func (ca *testCA) issue(t *testing.T, dir, name string, ekus []x509.ExtKeyUsage) (certFile, keyFile string) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + template := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: name}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: ekus, + IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, + DNSNames: []string{"localhost"}, + } + der, err := x509.CreateCertificate(rand.Reader, template, ca.cert, &key.PublicKey, ca.key) + if err != nil { + t.Fatal(err) + } + keyDer, err := x509.MarshalECPrivateKey(key) + if err != nil { + t.Fatal(err) + } + certFile = filepath.Join(dir, name+".pem") + keyFile = filepath.Join(dir, name+".key") + writePem(t, certFile, "CERTIFICATE", der) + writePem(t, keyFile, "EC PRIVATE KEY", keyDer) + return certFile, keyFile +} + +func writePem(t *testing.T, file, blockType string, der []byte) { + t.Helper() + if err := os.WriteFile(file, pem.EncodeToMemory(&pem.Block{Type: blockType, Bytes: der}), 0600); err != nil { + t.Fatal(err) + } +} + +func startTestGrpcServer(t *testing.T, config *util.ViperProxy, component string) string { + t.Helper() + creds, _ := LoadServerTLS(config, component) + if creds == nil { + t.Fatal("LoadServerTLS returned nil") + } + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + server := grpc.NewServer(creds) + healthpb.RegisterHealthServer(server, health.NewServer()) + go server.Serve(lis) + t.Cleanup(server.Stop) + return lis.Addr().String() +} + +func healthCheck(t *testing.T, addr string, dialOption grpc.DialOption) error { + t.Helper() + conn, err := grpc.NewClient(addr, dialOption) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _, err = healthpb.NewHealthClient(conn).Check(ctx, &healthpb.HealthCheckRequest{}) + return err +} + +// A component configured with a serverAuth-only serving cert plus a +// clientAuth-only client_cert/client_key pair must dial with the client pair. +func TestLoadClientTLSPrefersClientCert(t *testing.T) { + dir := t.TempDir() + ca := newTestCA(t, dir) + serverCert, serverKey := ca.issue(t, dir, "server", []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}) + clientCert, clientKey := ca.issue(t, dir, "client", []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}) + + v := &util.ViperProxy{Viper: viper.New()} + v.Set("grpc.ca", ca.file) + v.Set("grpc.master.cert", serverCert) + v.Set("grpc.master.key", serverKey) + v.Set("grpc.master.client_cert", clientCert) + v.Set("grpc.master.client_key", clientKey) + + addr := startTestGrpcServer(t, v, "grpc.master") + if err := healthCheck(t, addr, LoadClientTLS(v, "grpc.master")); err != nil { + t.Fatalf("health check with split client cert failed: %v", err) + } +} + +// Without client_cert, the component keeps presenting its serving cert. +func TestLoadClientTLSFallsBackToServingCert(t *testing.T) { + dir := t.TempDir() + ca := newTestCA(t, dir) + dualCert, dualKey := ca.issue(t, dir, "dual", []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}) + + v := &util.ViperProxy{Viper: viper.New()} + v.Set("grpc.ca", ca.file) + v.Set("grpc.master.cert", dualCert) + v.Set("grpc.master.key", dualKey) + + addr := startTestGrpcServer(t, v, "grpc.master") + if err := healthCheck(t, addr, LoadClientTLS(v, "grpc.master")); err != nil { + t.Fatalf("health check with dual-EKU cert failed: %v", err) + } +} + +// A serverAuth-only cert presented as the client identity fails the peer's +// clientAuth EKU verification — the failure mode client_cert exists to fix. +func TestLoadClientTLSServerOnlyEkuRejected(t *testing.T) { + dir := t.TempDir() + ca := newTestCA(t, dir) + serverCert, serverKey := ca.issue(t, dir, "server", []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}) + + v := &util.ViperProxy{Viper: viper.New()} + v.Set("grpc.ca", ca.file) + v.Set("grpc.master.cert", serverCert) + v.Set("grpc.master.key", serverKey) + + addr := startTestGrpcServer(t, v, "grpc.master") + if err := healthCheck(t, addr, LoadClientTLS(v, "grpc.master")); err == nil { + t.Fatal("expected handshake failure when presenting a serverAuth-only cert as client identity") + } +}