Commit 0c626fbc authored by shouya's avatar shouya Committed by Matt Holt

tls: Allow client auth configs if CA filenames match (#2648)

* verify client certs

* move client cert compatible checker to an independent function

* unexport client cert compatible checker

* rename functions and add comment

* gofmt code

* add test

* add back the comment
parent af821418
......@@ -394,16 +394,9 @@ func assertConfigsCompatible(cfg1, cfg2 *Config) error {
if c1.MaxVersion != c2.MaxVersion {
return fmt.Errorf("maximum TLS version mismatch")
}
if c1.ClientAuth != c2.ClientAuth {
return fmt.Errorf("client authentication policy mismatch")
}
if c1.ClientAuth != tls.NoClientCert && c2.ClientAuth != tls.NoClientCert && c1.ClientCAs != c2.ClientCAs {
// Two hosts defined on the same listener are not compatible if they
// have ClientAuth enabled, because there's no guarantee beyond the
// hostname which config will be used (because SNI only has server name).
// To prevent clients from bypassing authentication, require that
// ClientAuth be configured in an unambiguous manner.
return fmt.Errorf("multiple hosts requiring client authentication ambiguously configured")
if err := assertClientCertsCompatible(cfg1, cfg2); err != nil {
return err
}
return nil
......@@ -550,6 +543,37 @@ func getPreferredDefaultCiphers() []uint16 {
return defaultCiphersNonAESNI
}
func assertClientCertsCompatible(cfg1, cfg2 *Config) error {
c1, c2 := cfg1.tlsConfig, cfg2.tlsConfig
if c1.ClientAuth != c2.ClientAuth {
return fmt.Errorf("client authentication policy mismatch")
}
if c1.ClientAuth == tls.NoClientCert || c2.ClientAuth == tls.NoClientCert {
return nil
}
ccerts1, ccerts2 := cfg1.ClientCerts, cfg2.ClientCerts
if len(ccerts1) != len(ccerts2) {
return fmt.Errorf("number of client certs differs")
}
// The order of client CAs matters
for i, v := range ccerts1 {
if v != ccerts2[i] {
// Two hosts defined on the same listener are not compatible if they
// have ClientAuth enabled, because there's no guarantee beyond the
// hostname which config will be used (because SNI only has server name).
// To prevent clients from bypassing authentication, require that
// ClientAuth be configured in an unambiguous manner.
return fmt.Errorf("multiple hosts requiring client authentication ambiguously configured")
}
}
return nil
}
// Map of supported curves
// https://golang.org/pkg/crypto/tls/#CurveID
var supportedCurvesMap = map[string]tls.CurveID{
......
......@@ -108,3 +108,25 @@ func TestGetPreferredDefaultCiphers(t *testing.T) {
}
}
}
func TestAssertTLSConfigCompatibleClientCert(t *testing.T) {
configs := []*Config{
{Enabled: true, ClientAuth: tls.RequestClientCert, ClientCerts: []string{}},
{Enabled: true, ClientAuth: tls.RequestClientCert, ClientCerts: []string{"ca_cert.crt"}},
}
_, err := MakeTLSConfig(configs)
if err == nil {
t.Fatalf("Expected an error, but got %v", err)
}
configs = []*Config{
{Enabled: true, ClientAuth: tls.RequestClientCert, ClientCerts: []string{"ca_cert.crt"}},
{Enabled: true, ClientAuth: tls.RequestClientCert, ClientCerts: []string{"ca_cert.crt"}},
}
_, err = MakeTLSConfig(configs)
if err != nil {
t.Fatalf("Expected no error, but got %v", err)
}
}
......@@ -38,11 +38,18 @@ func TestMain(m *testing.M) {
os.Remove(certFile)
log.Fatal(err)
}
err = ioutil.WriteFile(caCertFile, caCert, 0644)
if err != nil {
os.Remove(keyFile)
os.Remove(certFile)
log.Fatal(err)
}
result := m.Run()
os.Remove(certFile)
os.Remove(keyFile)
os.Remove(caCertFile)
os.Exit(result)
}
......@@ -440,6 +447,7 @@ func TestSetupParseWithEmail(t *testing.T) {
const (
certFile = "test_cert.pem"
keyFile = "test_key.pem"
caCertFile = "ca_cert.crt"
)
var testCert = []byte(`-----BEGIN CERTIFICATE-----
......@@ -464,3 +472,39 @@ AwEHoUQDQgAEs22MtnG79K1mvIyjEO9GLx7BFD0tBbGnwQ0VPsuCxC6IeVuXbQDL
SiVQvFZ6lUszTlczNxVkpEfqrM6xAupB7g==
-----END EC PRIVATE KEY-----
`)
var caCert = []byte(`-----BEGIN CERTIFICATE-----
MIIF2DCCA8CgAwIBAgIQTKr5yttjb+Af907YWwOGnTANBgkqhkiG9w0BAQwFADCB
hTELMAkGA1UEBhMCR0IxGzAZBgNVBAgTEkdyZWF0ZXIgTWFuY2hlc3RlcjEQMA4G
A1UEBxMHU2FsZm9yZDEaMBgGA1UEChMRQ09NT0RPIENBIExpbWl0ZWQxKzApBgNV
BAMTIkNPTU9ETyBSU0EgQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkwHhcNMTAwMTE5
MDAwMDAwWhcNMzgwMTE4MjM1OTU5WjCBhTELMAkGA1UEBhMCR0IxGzAZBgNVBAgT
EkdyZWF0ZXIgTWFuY2hlc3RlcjEQMA4GA1UEBxMHU2FsZm9yZDEaMBgGA1UEChMR
Q09NT0RPIENBIExpbWl0ZWQxKzApBgNVBAMTIkNPTU9ETyBSU0EgQ2VydGlmaWNh
dGlvbiBBdXRob3JpdHkwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQCR
6FSS0gpWsawNJN3Fz0RndJkrN6N9I3AAcbxT38T6KhKPS38QVr2fcHK3YX/JSw8X
pz3jsARh7v8Rl8f0hj4K+j5c+ZPmNHrZFGvnnLOFoIJ6dq9xkNfs/Q36nGz637CC
9BR++b7Epi9Pf5l/tfxnQ3K9DADWietrLNPtj5gcFKt+5eNu/Nio5JIk2kNrYrhV
/erBvGy2i/MOjZrkm2xpmfh4SDBF1a3hDTxFYPwyllEnvGfDyi62a+pGx8cgoLEf
Zd5ICLqkTqnyg0Y3hOvozIFIQ2dOciqbXL1MGyiKXCJ7tKuY2e7gUYPDCUZObT6Z
+pUX2nwzV0E8jVHtC7ZcryxjGt9XyD+86V3Em69FmeKjWiS0uqlWPc9vqv9JWL7w
qP/0uK3pN/u6uPQLOvnoQ0IeidiEyxPx2bvhiWC4jChWrBQdnArncevPDt09qZah
SL0896+1DSJMwBGB7FY79tOi4lu3sgQiUpWAk2nojkxl8ZEDLXB0AuqLZxUpaVIC
u9ffUGpVRr+goyhhf3DQw6KqLCGqR84onAZFdr+CGCe01a60y1Dma/RMhnEw6abf
Fobg2P9A3fvQQoh/ozM6LlweQRGBY84YcWsr7KaKtzFcOmpH4MN5WdYgGq/yapiq
crxXStJLnbsQ/LBMQeXtHT1eKJ2czL+zUdqnR+WEUwIDAQABo0IwQDAdBgNVHQ4E
FgQUu69+Aj36pvE8hI6t7jiY7NkyMtQwDgYDVR0PAQH/BAQDAgEGMA8GA1UdEwEB
/wQFMAMBAf8wDQYJKoZIhvcNAQEMBQADggIBAArx1UaEt65Ru2yyTUEUAJNMnMvl
wFTPoCWOAvn9sKIN9SCYPBMtrFaisNZ+EZLpLrqeLppysb0ZRGxhNaKatBYSaVqM
4dc+pBroLwP0rmEdEBsqpIt6xf4FpuHA1sj+nq6PK7o9mfjYcwlYRm6mnPTXJ9OV
2jeDchzTc+CiR5kDOF3VSXkAKRzH7JsgHAckaVd4sjn8OoSgtZx8jb8uk2Intzna
FxiuvTwJaP+EmzzV1gsD41eeFPfR60/IvYcjt7ZJQ3mFXLrrkguhxuhoqEwWsRqZ
CuhTLJK7oQkYdQxlqHvLI7cawiiFwxv/0Cti76R7CZGYZ4wUAc1oBmpjIXUDgIiK
boHGhfKppC3n9KUkEEeDys30jXlYsQab5xoq2Z0B15R97QNKyvDb6KkBPvVWmcke
jkk9u+UJueBPSZI9FoJAzMxZxuY67RIuaTxslbH9qh17f4a+Hg4yRvv7E491f0yL
S0Zj/gA0QHDBw7mh3aZw4gSzQbzpgJHqZJx64SIDqZxubw5lT2yHh17zbqD5daWb
QOhTsiedSrnAdyGN/4fy3ryM7xfft0kL0fJuMAsaDk527RH89elWsn2/x20Kk4yl
0MC2Hb46TpSi125sC8KKfPog88Tk5c0NqMuRkrF8hey1FGlmDoLnzc7ILaZRfyHB
NVOFBkpdn627G190
-----END CERTIFICATE-----
`)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment