Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.5] Support multiple values for allowed client and peer TLS identities #18160

Merged
merged 1 commit into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 47 additions & 3 deletions client/pkg/transport/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,23 @@ type TLSInfo struct {
parseFunc func([]byte, []byte) (tls.Certificate, error)

// AllowedCN is a CN which must be provided by a client.
//
// Deprecated: use AllowedCNs instead.
AllowedCN string

// AllowedHostname is an IP address or hostname that must match the TLS
// certificate provided by a client.
//
// Deprecated: use AllowedHostnames instead.
AllowedHostname string

// AllowedCNs is a list of acceptable CNs which must be provided by a client.
AllowedCNs []string

// AllowedHostnames is a list of acceptable IP addresses or hostnames that must match the
// TLS certificate provided by a client.
AllowedHostnames []string

// Logger logs TLS errors.
// If nil, all logs are discarded.
Logger *zap.Logger
Expand Down Expand Up @@ -407,19 +418,52 @@ func (info TLSInfo) baseConfig() (*tls.Config, error) {
// Client certificates may be verified by either an exact match on the CN,
// or a more general check of the CN and SANs.
var verifyCertificate func(*x509.Certificate) bool

if info.AllowedCN != "" && len(info.AllowedCNs) > 0 {
return nil, fmt.Errorf("AllowedCN and AllowedCNs are mutually exclusive (cn=%q, cns=%q)", info.AllowedCN, info.AllowedCNs)
}
if info.AllowedHostname != "" && len(info.AllowedHostnames) > 0 {
return nil, fmt.Errorf("AllowedHostname and AllowedHostnames are mutually exclusive (hostname=%q, hostnames=%q)", info.AllowedHostname, info.AllowedHostnames)
}
if info.AllowedCN != "" && info.AllowedHostname != "" {
return nil, fmt.Errorf("AllowedCN and AllowedHostname are mutually exclusive (cn=%q, hostname=%q)", info.AllowedCN, info.AllowedHostname)
}
if len(info.AllowedCNs) > 0 && len(info.AllowedHostnames) > 0 {
return nil, fmt.Errorf("AllowedCNs and AllowedHostnames are mutually exclusive (cns=%q, hostnames=%q)", info.AllowedCNs, info.AllowedHostnames)
}

if info.AllowedCN != "" {
if info.AllowedHostname != "" {
return nil, fmt.Errorf("AllowedCN and AllowedHostname are mutually exclusive (cn=%q, hostname=%q)", info.AllowedCN, info.AllowedHostname)
}
info.Logger.Warn("AllowedCN is deprecated, use AllowedCNs instead")
verifyCertificate = func(cert *x509.Certificate) bool {
return info.AllowedCN == cert.Subject.CommonName
}
}
if info.AllowedHostname != "" {
info.Logger.Warn("AllowedHostname is deprecated, use AllowedHostnames instead")
verifyCertificate = func(cert *x509.Certificate) bool {
return cert.VerifyHostname(info.AllowedHostname) == nil
}
}
if len(info.AllowedCNs) > 0 {
verifyCertificate = func(cert *x509.Certificate) bool {
for _, allowedCN := range info.AllowedCNs {
if allowedCN == cert.Subject.CommonName {
return true
}
}
return false
}
}
if len(info.AllowedHostnames) > 0 {
verifyCertificate = func(cert *x509.Certificate) bool {
for _, allowedHostname := range info.AllowedHostnames {
if cert.VerifyHostname(allowedHostname) == nil {
return true
}
}
return false
}
}
if verifyCertificate != nil {
cfg.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
for _, chains := range verifiedChains {
Expand Down
22 changes: 11 additions & 11 deletions server/embed/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,15 +443,15 @@ 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"`
AllowedCN string `json:"allowed-cn"`
AllowedHostname 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"`
}

// NewConfig creates a new Config populated with default values.
Expand Down Expand Up @@ -631,8 +631,8 @@ func (cfg *configYAML) configFromFile(path string) error {
tls.ClientKeyFile = ysc.ClientKeyFile
tls.ClientCertAuth = ysc.CertAuth
tls.TrustedCAFile = ysc.TrustedCAFile
tls.AllowedCN = ysc.AllowedCN
tls.AllowedHostname = ysc.AllowedHostname
tls.AllowedCNs = ysc.AllowedCNs
tls.AllowedHostnames = ysc.AllowedHostnames
}
copySecurityDetails(&cfg.ClientTLSInfo, &cfg.ClientSecurityJSON)
copySecurityDetails(&cfg.PeerTLSInfo, &cfg.PeerSecurityJSON)
Expand Down
18 changes: 15 additions & 3 deletions server/embed/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,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", AllowedCN: "etcd", AllowedHostname: "whatever.example.com"}
ptls := securityConfig{TrustedCAFile: "pca", CertFile: "pcert", KeyFile: "pkey", AllowedCNs: []string{"etcd"}, AllowedHostnames: []string{"whatever.example.com"}}
yc := struct {
ClientSecurityCfgFile securityConfig `json:"client-transport-security"`
PeerSecurityCfgFile securityConfig `json:"peer-transport-security"`
Expand Down Expand Up @@ -160,8 +160,20 @@ func (s *securityConfig) equals(t *transport.TLSInfo) bool {
s.ClientCertFile == t.ClientCertFile &&
s.ClientKeyFile == t.ClientKeyFile &&
s.KeyFile == t.KeyFile &&
s.AllowedCN == t.AllowedCN &&
s.AllowedHostname == t.AllowedHostname
compareSlices(s.AllowedCNs, t.AllowedCNs) &&
compareSlices(s.AllowedHostnames, t.AllowedHostnames)
jmhbnz marked this conversation as resolved.
Show resolved Hide resolved
}

func compareSlices(slice1, slice2 []string) bool {
if len(slice1) != len(slice2) {
return false
}
for i, v := range slice1 {
if v != slice2[i] {
return false
}
}
return true
}

func mustCreateCfgFile(t *testing.T, b []byte) *os.File {
Expand Down
10 changes: 7 additions & 3 deletions server/etcdmain/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func newConfig() *config {
fs.StringVar(&cfg.ec.ClientTLSInfo.ClientKeyFile, "client-key-file", "", "Path to an explicit peer client TLS key file otherwise key file will be used when client auth is required.")
fs.BoolVar(&cfg.ec.ClientTLSInfo.ClientCertAuth, "client-cert-auth", false, "Enable client cert authentication.")
fs.StringVar(&cfg.ec.ClientTLSInfo.CRLFile, "client-crl-file", "", "Path to the client certificate revocation list file.")
fs.StringVar(&cfg.ec.ClientTLSInfo.AllowedHostname, "client-cert-allowed-hostname", "", "Allowed TLS hostname for client cert authentication.")
fs.Var(flags.NewStringsValue(""), "client-cert-allowed-hostname", "Comma-separated list of allowed SAN hostnames for client cert authentication.")
fs.StringVar(&cfg.ec.ClientTLSInfo.TrustedCAFile, "trusted-ca-file", "", "Path to the client server TLS trusted CA cert file.")
fs.BoolVar(&cfg.ec.ClientAutoTLS, "auto-tls", false, "Client TLS using generated certificates")
fs.StringVar(&cfg.ec.PeerTLSInfo.CertFile, "peer-cert-file", "", "Path to the peer server TLS cert file.")
Expand All @@ -238,8 +238,8 @@ func newConfig() *config {
fs.BoolVar(&cfg.ec.PeerAutoTLS, "peer-auto-tls", false, "Peer TLS using generated certificates")
fs.UintVar(&cfg.ec.SelfSignedCertValidity, "self-signed-cert-validity", 1, "The validity period of the client and peer certificates, unit is year")
fs.StringVar(&cfg.ec.PeerTLSInfo.CRLFile, "peer-crl-file", "", "Path to the peer certificate revocation list file.")
fs.StringVar(&cfg.ec.PeerTLSInfo.AllowedCN, "peer-cert-allowed-cn", "", "Allowed CN for inter peer authentication.")
fs.StringVar(&cfg.ec.PeerTLSInfo.AllowedHostname, "peer-cert-allowed-hostname", "", "Allowed TLS hostname for inter peer authentication.")
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.ec.PeerTLSInfo.SkipClientSANVerify, "experimental-peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.")
fs.StringVar(&cfg.ec.TlsMinVersion, "tls-min-version", string(tlsutil.TLSVersion12), "Minimum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3.")
Expand Down Expand Up @@ -409,6 +409,10 @@ func (cfg *config) configFromCmdLine() error {
cfg.ec.CORS = flags.UniqueURLsMapFromFlag(cfg.cf.flagSet, "cors")
cfg.ec.HostWhitelist = flags.UniqueStringsMapFromFlag(cfg.cf.flagSet, "host-whitelist")

cfg.ec.ClientTLSInfo.AllowedHostnames = flags.StringsFromFlag(cfg.cf.flagSet, "client-cert-allowed-hostname")
cfg.ec.PeerTLSInfo.AllowedCNs = flags.StringsFromFlag(cfg.cf.flagSet, "peer-cert-allowed-cn")
cfg.ec.PeerTLSInfo.AllowedHostnames = flags.StringsFromFlag(cfg.cf.flagSet, "peer-cert-allowed-hostname")

cfg.ec.CipherSuites = flags.StringsFromFlag(cfg.cf.flagSet, "cipher-suites")

cfg.ec.MaxConcurrentStreams = flags.Uint32FromFlag(cfg.cf.flagSet, "max-concurrent-streams")
Expand Down
6 changes: 3 additions & 3 deletions server/etcdmain/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ Security:
--client-crl-file ''
Path to the client certificate revocation list file.
--client-cert-allowed-hostname ''
Allowed TLS hostname for client cert authentication.
Comma-separated list of SAN hostnames for client cert authentication.
--trusted-ca-file ''
Path to the client server TLS trusted CA cert file.
--auto-tls 'false'
Expand All @@ -173,9 +173,9 @@ Security:
--peer-trusted-ca-file ''
Path to the peer server TLS trusted CA file.
--peer-cert-allowed-cn ''
Required CN for client certs connecting to the peer endpoint.
Comma-separated list of allowed CNs for inter-peer TLS authentication.
--peer-cert-allowed-hostname ''
Allowed TLS hostname for inter peer authentication.
Comma-separated list of allowed SAN hostnames for inter-peer TLS authentication.
--peer-auto-tls 'false'
Peer TLS using self-generated certificates if --peer-key-file and --peer-cert-file are not provided.
--peer-client-cert-file ''
Expand Down
90 changes: 90 additions & 0 deletions tests/e2e/etcd_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,96 @@ func TestEtcdPeerCNAuth(t *testing.T) {
}
}

// TestEtcdPeerMultiCNAuth checks that the inter peer auth based on CN of cert is working correctly
// when there are multiple allowed values for the CN.
func TestEtcdPeerMultiCNAuth(t *testing.T) {
e2e.SkipInShortMode(t)

peers, tmpdirs := make([]string, 3), make([]string, 3)
for i := range peers {
peers[i] = fmt.Sprintf("e%d=https://127.0.0.1:%d", i, e2e.EtcdProcessBasePort+i)
tmpdirs[i] = t.TempDir()
}
ic := strings.Join(peers, ",")
procs := make([]*expect.ExpectProcess, len(peers))
defer func() {
for i := range procs {
if procs[i] != nil {
procs[i].Stop()
procs[i].Close()
}
}
}()

// all nodes have unique certs with different CNs
// node 0 and 1 have a cert with one of the correct CNs, node 2 doesn't
for i := range procs {
commonArgs := []string{
e2e.BinDir + "/etcd",
"--name", fmt.Sprintf("e%d", i),
"--listen-client-urls", "http://0.0.0.0:0",
"--data-dir", tmpdirs[i],
"--advertise-client-urls", "http://0.0.0.0:0",
"--listen-peer-urls", fmt.Sprintf("https://127.0.0.1:%d,https://127.0.0.1:%d", e2e.EtcdProcessBasePort+i, e2e.EtcdProcessBasePort+len(peers)+i),
"--initial-advertise-peer-urls", fmt.Sprintf("https://127.0.0.1:%d", e2e.EtcdProcessBasePort+i),
"--initial-cluster", ic,
}

var args []string
switch i {
case 0:
args = []string{
"--peer-cert-file", e2e.CertPath, // server.crt has CN "example.com".
"--peer-key-file", e2e.PrivateKeyPath,
"--peer-client-cert-file", e2e.CertPath,
"--peer-client-key-file", e2e.PrivateKeyPath,
"--peer-trusted-ca-file", e2e.CaPath,
"--peer-client-cert-auth",
"--peer-cert-allowed-cn", "example.com,example2.com",
}
case 1:
args = []string{
"--peer-cert-file", e2e.CertPath2, // server2.crt has CN "example2.com".
"--peer-key-file", e2e.PrivateKeyPath2,
"--peer-client-cert-file", e2e.CertPath2,
"--peer-client-key-file", e2e.PrivateKeyPath2,
"--peer-trusted-ca-file", e2e.CaPath,
"--peer-client-cert-auth",
"--peer-cert-allowed-cn", "example.com,example2.com",
}
default:
args = []string{
"--peer-cert-file", e2e.CertPath3, // server3.crt has CN "ca".
"--peer-key-file", e2e.PrivateKeyPath3,
"--peer-client-cert-file", e2e.CertPath3,
"--peer-client-key-file", e2e.PrivateKeyPath3,
"--peer-trusted-ca-file", e2e.CaPath,
"--peer-client-cert-auth",
"--peer-cert-allowed-cn", "example.com,example2.com",
}
}

commonArgs = append(commonArgs, args...)
p, err := e2e.SpawnCmd(commonArgs, nil)
if err != nil {
t.Fatal(err)
}
procs[i] = p
}

for i, p := range procs {
var expect []string
if i <= 1 {
expect = e2e.EtcdServerReadyLines
} else {
expect = []string{"remote error: tls: bad certificate"}
}
if err := e2e.WaitReadyExpectProc(p, expect); err != nil {
t.Fatal(err)
}
}
}

// TestEtcdPeerNameAuth checks that the inter peer auth based on cert name validation is working correctly.
func TestEtcdPeerNameAuth(t *testing.T) {
e2e.SkipInShortMode(t)
Expand Down
Loading