Skip to content

Commit

Permalink
Support multiple values for allowed client and peer TLS identities(#1…
Browse files Browse the repository at this point in the history
…8015)

Signed-off-by: lhy1024 <admin@liudos.us>
  • Loading branch information
lhy1024 committed Jun 13, 2024
1 parent fb66ea6 commit 5ff9fd8
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 23 deletions.
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)
}

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(context.TODO(), 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

0 comments on commit 5ff9fd8

Please sign in to comment.