From 4378dbea0ef21ab9342436b383b9e0cfee7c0eb8 Mon Sep 17 00:00:00 2001 From: wodeyoulai <106662970@qq.com> Date: Mon, 27 Jan 2025 16:03:17 +0800 Subject: [PATCH] rename --experimental-peer-skip-client-san-verification to --peer-skip-client-san-verification --- server/embed/config.go | 32 ++++++--- server/embed/config_test.go | 2 +- server/etcdmain/config.go | 4 ++ server/etcdmain/config_test.go | 119 +++++++++++++++++++++++++++++++++ server/etcdmain/help.go | 2 +- 5 files changed, 147 insertions(+), 12 deletions(-) diff --git a/server/embed/config.go b/server/embed/config.go index 890a22e8c7db..c67c41bf0ded 100644 --- a/server/embed/config.go +++ b/server/embed/config.go @@ -415,6 +415,13 @@ type Config struct { // TODO: Delete in v3.7 ExperimentalBootstrapDefragThresholdMegabytes uint `json:"experimental-bootstrap-defrag-threshold-megabytes"` BootstrapDefragThresholdMegabytes uint `json:"bootstrap-defrag-threshold-megabytes"` + + // ExperimentalPeerSkipClientSanVerification determines whether to skip verification of SAN field + // in client certificate for peer connections. + // Deprecated in v3.6 and will be decommissioned in v3.7. + // TODO: Delete in v3.7 + ExperimentalPeerSkipClientSanVerification bool `json:"experimental-peer-skip-client-san-verification"` + // WarningUnaryRequestDuration is the time duration after which a warning is generated if applying // unary request takes more time than this value. WarningUnaryRequestDuration time.Duration `json:"warning-unary-request-duration"` @@ -533,15 +540,16 @@ type configJSON struct { } type securityConfig struct { - CertFile string `json:"cert-file"` - KeyFile string `json:"key-file"` - ClientCertFile string `json:"client-cert-file"` - ClientKeyFile string `json:"client-key-file"` - CertAuth bool `json:"client-cert-auth"` - TrustedCAFile string `json:"trusted-ca-file"` - AutoTLS bool `json:"auto-tls"` - AllowedCNs []string `json:"allowed-cn"` - AllowedHostnames []string `json:"allowed-hostname"` + CertFile string `json:"cert-file"` + KeyFile string `json:"key-file"` + ClientCertFile string `json:"client-cert-file"` + ClientKeyFile string `json:"client-key-file"` + CertAuth bool `json:"client-cert-auth"` + TrustedCAFile string `json:"trusted-ca-file"` + AutoTLS bool `json:"auto-tls"` + AllowedCNs []string `json:"allowed-cn"` + AllowedHostnames []string `json:"allowed-hostname"` + PeerSkipClientSanVerification bool `json:"peer-skip-client-san-verification"` } // NewConfig creates a new Config populated with default values. @@ -749,7 +757,10 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) { fs.Var(flags.NewStringsValue(""), "peer-cert-allowed-cn", "Comma-separated list of allowed CNs for inter-peer TLS authentication.") fs.Var(flags.NewStringsValue(""), "peer-cert-allowed-hostname", "Comma-separated list of allowed SAN hostnames for inter-peer TLS authentication.") fs.Var(flags.NewStringsValue(""), "cipher-suites", "Comma-separated list of supported TLS cipher suites between client/server and peers (empty will be auto-populated by Go).") - fs.BoolVar(&cfg.PeerTLSInfo.SkipClientSANVerify, "experimental-peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.") + + fs.BoolVar(&cfg.ExperimentalPeerSkipClientSanVerification, "experimental-peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.Deprecated in v3.6 and will be decommissioned in v3.7. Use peer-skip-client-san-verification instead") + fs.BoolVar(&cfg.PeerTLSInfo.SkipClientSANVerify, "peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.") + fs.StringVar(&cfg.TlsMinVersion, "tls-min-version", string(tlsutil.TLSVersion12), "Minimum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3.") fs.StringVar(&cfg.TlsMaxVersion, "tls-max-version", string(tlsutil.TLSVersionDefault), "Maximum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3 (empty defers to Go).") @@ -969,6 +980,7 @@ func (cfg *configYAML) configFromFile(path string) error { tls.TrustedCAFile = ysc.TrustedCAFile tls.AllowedCNs = ysc.AllowedCNs tls.AllowedHostnames = ysc.AllowedHostnames + tls.SkipClientSANVerify = ysc.PeerSkipClientSanVerification } copySecurityDetails(&cfg.ClientTLSInfo, &cfg.ClientSecurityJSON) copySecurityDetails(&cfg.PeerTLSInfo, &cfg.PeerSecurityJSON) diff --git a/server/embed/config_test.go b/server/embed/config_test.go index 757769b87223..dd1dc93714ae 100644 --- a/server/embed/config_test.go +++ b/server/embed/config_test.go @@ -48,7 +48,7 @@ func notFoundErr(service, domain string) error { func TestConfigFileOtherFields(t *testing.T) { ctls := securityConfig{TrustedCAFile: "cca", CertFile: "ccert", KeyFile: "ckey"} // Note AllowedCN and AllowedHostname are mutually exclusive, this test is just to verify the fields can be correctly marshalled & unmarshalled. - ptls := securityConfig{TrustedCAFile: "pca", CertFile: "pcert", KeyFile: "pkey", AllowedCNs: []string{"etcd"}, AllowedHostnames: []string{"whatever.example.com"}} + ptls := securityConfig{TrustedCAFile: "pca", CertFile: "pcert", KeyFile: "pkey", AllowedCNs: []string{"etcd"}, AllowedHostnames: []string{"whatever.example.com"}, PeerSkipClientSanVerification: true} yc := struct { ClientSecurityCfgFile securityConfig `json:"client-transport-security"` PeerSecurityCfgFile securityConfig `json:"peer-transport-security"` diff --git a/server/etcdmain/config.go b/server/etcdmain/config.go index b5beade8d295..3484e5b5eb76 100644 --- a/server/etcdmain/config.go +++ b/server/etcdmain/config.go @@ -199,6 +199,10 @@ func (cfg *config) parse(arguments []string) error { cfg.ec.BootstrapDefragThresholdMegabytes = cfg.ec.ExperimentalBootstrapDefragThresholdMegabytes } + if cfg.ec.FlagsExplicitlySet["experimental-peer-skip-client-san-verification"] { + cfg.ec.PeerTLSInfo.SkipClientSANVerify = cfg.ec.ExperimentalPeerSkipClientSanVerification + } + // `V2Deprecation` (--v2-deprecation) is deprecated and scheduled for removal in v3.8. The default value is enforced, ignoring user input. cfg.ec.V2Deprecation = cconfig.V2DeprDefault diff --git a/server/etcdmain/config_test.go b/server/etcdmain/config_test.go index 15d724b2228b..d0d1b026e2ef 100644 --- a/server/etcdmain/config_test.go +++ b/server/etcdmain/config_test.go @@ -21,6 +21,7 @@ import ( "net/url" "os" "reflect" + "strconv" "strings" "testing" "time" @@ -1085,3 +1086,121 @@ func TestConfigFileDeprecatedOptions(t *testing.T) { }) } } + +// TestPeerSkipClientSanVerificationFlagMigration tests the migration from +// --experimental-peer-skip-client-san-verification to --peer-skip-client-san-verification +// TODO: delete in v3.7 +func TestPeerSkipClientSanVerificationFlagMigration(t *testing.T) { + testCases := []struct { + name string + peerSkipClientSanVerification string + experimentalPeerSkipClientSanVerification string + useConfigFile bool + expectErr bool + expectedPeerSkipClientSanVerification bool + }{ + { + name: "set both experimental flag and non experimental flag,experimental = false,non experimental = true", + peerSkipClientSanVerification: "true", + experimentalPeerSkipClientSanVerification: "false", + expectedPeerSkipClientSanVerification: false, + }, + { + name: "set both experimental flag and non experimental flag,experimental = true,non experimental = false", + peerSkipClientSanVerification: "false", + experimentalPeerSkipClientSanVerification: "true", + expectedPeerSkipClientSanVerification: true, + }, + { + name: "can set experimental flag to true", + experimentalPeerSkipClientSanVerification: "true", + expectedPeerSkipClientSanVerification: true, + }, + { + name: "can set experimental flag to false", + experimentalPeerSkipClientSanVerification: "false", + expectedPeerSkipClientSanVerification: false, + }, + { + name: "can set non experimental flag to true", + peerSkipClientSanVerification: "true", + expectedPeerSkipClientSanVerification: true, + }, + { + name: "can set non experimental flag to false", + peerSkipClientSanVerification: "false", + expectedPeerSkipClientSanVerification: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cmdLineArgs := []string{} + type securityConfig struct { + PeerSkipClientSanVerification bool `json:"peer-skip-client-san-verification"` + } + yc := struct { + ExperimentalPeerSkipClientSanVerification bool `json:"experimental-peer-skip-client-san-verification,omitempty"` + PeerSkipClientSanVerification bool `json:"peer-skip-client-san-verification,omitempty"` + PeerSecurityCfgFile securityConfig `json:"peer-transport-security"` + }{} + + if tc.peerSkipClientSanVerification != "" { + cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--peer-skip-client-san-verification=%s", tc.peerSkipClientSanVerification)) + val, err := strconv.ParseBool(tc.peerSkipClientSanVerification) + if err != nil { + t.Fatal(err) + } + yc.PeerSkipClientSanVerification = val + yc.PeerSecurityCfgFile.PeerSkipClientSanVerification = val + } + + if tc.experimentalPeerSkipClientSanVerification != "" { + cmdLineArgs = append(cmdLineArgs, fmt.Sprintf("--experimental-peer-skip-client-san-verification=%s", tc.experimentalPeerSkipClientSanVerification)) + val, err := strconv.ParseBool(tc.experimentalPeerSkipClientSanVerification) + if err != nil { + t.Fatal(err) + } + yc.ExperimentalPeerSkipClientSanVerification = val + yc.PeerSecurityCfgFile.PeerSkipClientSanVerification = val + } + + b, err := yaml.Marshal(&yc) + if err != nil { + t.Fatal(err) + } + + tmpfile := mustCreateCfgFile(t, b) + defer os.Remove(tmpfile.Name()) + + cfgFromCmdLine := newConfig() + errFromCmdLine := cfgFromCmdLine.parse(cmdLineArgs) + + cfgFromFile := newConfig() + errFromFile := cfgFromFile.parse([]string{fmt.Sprintf("--config-file=%s", tmpfile.Name())}) + + if tc.expectErr { + if errFromCmdLine == nil || errFromFile == nil { + t.Fatal("expect parse error") + } + return + } + + if errFromCmdLine != nil || errFromFile != nil { + t.Fatal(err) + } + + if cfgFromCmdLine.ec.PeerTLSInfo.SkipClientSANVerify != tc.expectedPeerSkipClientSanVerification { + t.Errorf("expected SkipClientSANVerify=%v, got %v", + tc.expectedPeerSkipClientSanVerification, + cfgFromCmdLine.ec.PeerTLSInfo.SkipClientSANVerify) + } + + if cfgFromFile.ec.PeerTLSInfo.SkipClientSANVerify != tc.expectedPeerSkipClientSanVerification { + t.Errorf("expected SkipClientSANVerify=%v, got %v", + tc.expectedPeerSkipClientSanVerification, + cfgFromFile.ec.PeerTLSInfo.SkipClientSANVerify) + } + }) + } +} diff --git a/server/etcdmain/help.go b/server/etcdmain/help.go index 9d48fe0c29d4..2988cdaced36 100644 --- a/server/etcdmain/help.go +++ b/server/etcdmain/help.go @@ -292,7 +292,7 @@ Experimental feature: --compaction-batch-limit 1000 CompactionBatchLimit sets the maximum revisions deleted in each compaction batch. --experimental-peer-skip-client-san-verification 'false' - Skip verification of SAN field in client certificate for peer connections. + Skip verification of SAN field in client certificate for peer connections.Deprecated in v3.6 and will be decommissioned in v3.7. Use 'peer-skip-client-san-verification' instead. --experimental-watch-progress-notify-interval '10m' Duration of periodical watch progress notification. Deprecated in v3.6 and will be decommissioned in v3.7. Use 'watch-progress-notify-interval' instead. --watch-progress-notify-interval '10m'