Merge pull request #19362 from siyuanfoundation/lease-checkpoint

migrate experimental-peer-skip-client-san-verification flag to peer-skip-client-san-verification
etcd-io · Feb 7, 2025 · 9de211d · 9de211d
2 parents e25d605 + a3a467b
commit 9de211d
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 10 deletions.
diff --git a/server/embed/config.go b/server/embed/config.go
@@ -150,6 +150,7 @@ var (
 		"experimental-memory-mlock":                         "memory-mlock",
 		"experimental-compaction-sleep-interval":            "compaction-sleep-interval",
 		"experimental-downgrade-check-time":                 "downgrade-check-time",
+		"experimental-peer-skip-client-san-verification":    "peer-skip-client-san-verification",
 	}
 )
 
@@ -466,6 +467,12 @@ type Config struct {
 	// Defaults to 0.
 	ExperimentalDistributedTracingSamplingRatePerMillion int `json:"experimental-distributed-tracing-sampling-rate"`
 
+	// 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"`
+
 	// Logger is logger options: currently only supports "zap".
 	// "capnslog" is removed in v3.5.
 	Logger string `json:"logger"`
@@ -561,15 +568,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"`
+	SkipClientSANVerify bool     `json:"skip-client-san-verification,omitempty"`
 }
 
 // NewConfig creates a new Config populated with default values.
@@ -792,7 +800,8 @@ 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).")
 
@@ -925,6 +934,16 @@ func (cfg *configYAML) configFromFile(path string) error {
 		cfg.FlagsExplicitlySet[flg] = true
 	}
 
+	if peerTransportSecurity, ok := cfgMap["peer-transport-security"]; ok {
+		peerTransportSecurityMap, isMap := peerTransportSecurity.(map[string]any)
+		if !isMap {
+			return fmt.Errorf("invalid peer-transport-security")
+		}
+		for k := range peerTransportSecurityMap {
+			cfg.FlagsExplicitlySet[fmt.Sprintf("peer-%s", k)] = true
+		}
+	}
+
 	getBoolFlagVal := func(flagName string) *bool {
 		flagVal, ok := cfgMap[flagName]
 		if !ok {
@@ -1019,6 +1038,7 @@ func (cfg *configYAML) configFromFile(path string) error {
 		tls.TrustedCAFile = ysc.TrustedCAFile
 		tls.AllowedCNs = ysc.AllowedCNs
 		tls.AllowedHostnames = ysc.AllowedHostnames
+		tls.SkipClientSANVerify = ysc.SkipClientSANVerify
 	}
 	copySecurityDetails(&cfg.ClientTLSInfo, &cfg.ClientSecurityJSON)
 	copySecurityDetails(&cfg.PeerTLSInfo, &cfg.PeerSecurityJSON)

diff --git a/server/etcdmain/config.go b/server/etcdmain/config.go
@@ -202,6 +202,9 @@ func (cfg *config) parse(arguments []string) error {
 	if cfg.ec.FlagsExplicitlySet["experimental-bootstrap-defrag-threshold-megabytes"] {
 		cfg.ec.BootstrapDefragThresholdMegabytes = cfg.ec.ExperimentalBootstrapDefragThresholdMegabytes
 	}
+	if cfg.ec.FlagsExplicitlySet["experimental-peer-skip-client-san-verification"] {
+		cfg.ec.PeerTLSInfo.SkipClientSANVerify = cfg.ec.ExperimentalPeerSkipClientSanVerification
+	}
 
 	if cfg.ec.FlagsExplicitlySet["experimental-max-learners"] {
 		cfg.ec.MaxLearners = cfg.ec.ExperimentalMaxLearners

diff --git 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"
@@ -1370,3 +1371,96 @@ 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:                          "cannot set both experimental flag and non experimental flag",
+			peerSkipClientSanVerification: "true",
+			experimentalPeerSkipClientSanVerification: "true",
+			expectErr: 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) {
+			type securityConfig struct {
+				SkipClientSanVerification bool `json:"skip-client-san-verification,omitempty"`
+			}
+			cmdLineArgs := []string{}
+			yc := struct {
+				ExperimentalPeerSkipClientSanVerification bool           `json:"experimental-peer-skip-client-san-verification,omitempty"`
+				PeerSecurityJSON                          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.PeerSecurityJSON.SkipClientSanVerification = 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
+			}
+			cfgFromCmdLine, errFromCmdLine, cfgFromFile, errFromFile := generateCfgsFromFileAndCmdLine(t, yc, cmdLineArgs)
+
+			if tc.expectErr {
+				if errFromCmdLine == nil || errFromFile == nil {
+					t.Fatalf("expect parse error, got errFromCmdLine=%v, errFromFile=%v", errFromCmdLine, errFromFile)
+				}
+				return
+			}
+			if errFromCmdLine != nil || errFromFile != nil {
+				t.Fatal("error parsing config")
+			}
+			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
@@ -294,6 +294,8 @@ 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. Deprecated in v3.6 and will be decommissioned in v3.7. Use 'peer-skip-client-san-verification' instead.
+  --peer-skip-client-san-verification 'false'
     Skip verification of SAN field in client certificate for peer connections.
   --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.