From a6d9adbf4a72b20097c9c67e438675f7af76618b Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 31 May 2019 09:30:15 -0700 Subject: [PATCH] make sure client CA and auth type are set if CA is explicitly specified. (#2825) * make sure client CA and auth type are set if CA is explicitly specified. added some simple tests to confirm the effect. * test certificates (forgot to add them in the previous commit) * made client auth policy configurable with new client_auth option. README has been updated accordingly. * fix editorial in README --- plugin/tls/README.md | 10 ++++++++++ plugin/tls/test_ca.pem | 20 +++++++++++++++++++ plugin/tls/test_cert.pem | 20 +++++++++++++++++++ plugin/tls/test_key.pem | 28 +++++++++++++++++++++++++++ plugin/tls/tls.go | 41 ++++++++++++++++++++++++++++++++++++++- plugin/tls/tls_test.go | 42 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 plugin/tls/test_ca.pem create mode 100644 plugin/tls/test_cert.pem create mode 100644 plugin/tls/test_key.pem diff --git a/plugin/tls/README.md b/plugin/tls/README.md index 244984750..82d059ade 100644 --- a/plugin/tls/README.md +++ b/plugin/tls/README.md @@ -24,6 +24,16 @@ tls CERT KEY [CA] Parameter CA is optional. If not set, system CAs can be used to verify the client certificate +~~~ txt +tls CERT KEY [CA] { + client_auth nocert|request|require|verify_if_given|require_and_verify +} +~~~ + +If client_auth option is specified, it controls the client authentication policy. +The option value corresponds to the [ClientAuthType values of the Go tls package](https://golang.org/pkg/crypto/tls/#ClientAuthType): NoClientCert, RequestClientCert, RequireAnyClientCert, VerifyClientCertIfGiven, and RequireAndVerifyClientCert, respectively. +The default is "nocert". Note that it makes no sense to specify parameter CA unless this option is set to verify_if_given or require_and_verify. + ## Examples Start a DNS-over-TLS server that picks up incoming DNS-over-TLS queries on port 5553 and uses the diff --git a/plugin/tls/test_ca.pem b/plugin/tls/test_ca.pem new file mode 100644 index 000000000..cfcd5cc0e --- /dev/null +++ b/plugin/tls/test_ca.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDPzCCAiegAwIBAgIJAPjCWTu1wGapMA0GCSqGSIb3DQEBCwUAMDUxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMREwDwYDVQQKDAhJbmZvYmxveDAg +Fw0xOTA1MTEwMDI3NDRaGA8yMTE5MDQxNzAwMjc0NFowNTELMAkGA1UEBhMCVVMx +EzARBgNVBAgMCkNhbGlmb3JuaWExETAPBgNVBAoMCEluZm9ibG94MIIBIjANBgkq +hkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArAYiw1UjlYj+nITRUlj5hA7j8U2qWcyN +YcDfqQnt173Z8yR7NJokqt3Bd3PlrBZS2XtYSNohxRr4qeJu/g7UBre/fSEU/ZOM +Gl7NjBGKQEymJ0d8rBg52iiGNwU+ERI9pcQRA6DCEjVbOmjDiUd5yzuVotG/Sxep +GUJ2puJ0p0gWCMEL9sdqY6HHd/hdj6B6+u2xD9NUCkX9pLC7CPFJHnP0vLO4WIWL +z5C7yzpeLO9r7Nfnu+2HcRLmuFZVPNxkMq7UymqR1w5ZYJQ5p9E7pyxDVXxHnTqQ +yLaAS2/9umrOwVnD1NaN3OdAhDedXbH0cF08GcIQD9rnlkLMW4CKtwIDAQABo1Aw +TjAdBgNVHQ4EFgQUHcxJPBmHF0nSv+FJJI/kwrSThf8wHwYDVR0jBBgwFoAUHcxJ +PBmHF0nSv+FJJI/kwrSThf8wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC +AQEAByItgyhlXDv2wnnMVXHHlUCbsKCOtBJZ8EumvKjeOx5G4gqJpQIQPNeBv1Od +QT7d15HfT7RQqHSL0uAoGuNuyGjZGWWbLMkVt8T0tXY2v9Dd8eWC/lFaaA0vkqTG +GpADSmH+SoFAdPPcYN/sXmEHvZcIQ0wUxuF48ZMwOh7ZOcrZggxlA9+BKHU4fO03 +o7krzpQZQmEDXNN8bt1R0DIhVADw/G2oJAzK0LGhh4eu6hj6k/cAWS6ujRBGqN0Z +fURCrMEyjzbNybhkU1KqSr7eSJOWkl4UJ5Ns/dt9/yw2BBrKH3Mijch7UA8mlbEE +29M28u2W7GMXLSSwmtCqDBRNhg== +-----END CERTIFICATE----- diff --git a/plugin/tls/test_cert.pem b/plugin/tls/test_cert.pem new file mode 100644 index 000000000..8cc47eb08 --- /dev/null +++ b/plugin/tls/test_cert.pem @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDPzCCAiegAwIBAgIJAPezzzshGRiTMA0GCSqGSIb3DQEBCwUAMDUxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMREwDwYDVQQKDAhJbmZvYmxveDAg +Fw0xOTA1MTEwMDI2MjNaGA8yMTE5MDQxNzAwMjYyM1owNTELMAkGA1UEBhMCVVMx +EzARBgNVBAgMCkNhbGlmb3JuaWExETAPBgNVBAoMCEluZm9ibG94MIIBIjANBgkq +hkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArAYiw1UjlYj+nITRUlj5hA7j8U2qWcyN +YcDfqQnt173Z8yR7NJokqt3Bd3PlrBZS2XtYSNohxRr4qeJu/g7UBre/fSEU/ZOM +Gl7NjBGKQEymJ0d8rBg52iiGNwU+ERI9pcQRA6DCEjVbOmjDiUd5yzuVotG/Sxep +GUJ2puJ0p0gWCMEL9sdqY6HHd/hdj6B6+u2xD9NUCkX9pLC7CPFJHnP0vLO4WIWL +z5C7yzpeLO9r7Nfnu+2HcRLmuFZVPNxkMq7UymqR1w5ZYJQ5p9E7pyxDVXxHnTqQ +yLaAS2/9umrOwVnD1NaN3OdAhDedXbH0cF08GcIQD9rnlkLMW4CKtwIDAQABo1Aw +TjAdBgNVHQ4EFgQUHcxJPBmHF0nSv+FJJI/kwrSThf8wHwYDVR0jBBgwFoAUHcxJ +PBmHF0nSv+FJJI/kwrSThf8wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC +AQEAQyN9nLImdtufuSjXcrCJ3alt/vffHJIzlPgDsNw8+tjI7aRX7CzuurOOEQUC +fJ9A6O+dat5k5yqVb9hDcD42HXtOjRQDYpQ6dOGirLFThIFSMC/7RiqHk0YtxojM +ZNBbgXo4o1d+P9b25oc/+pRDzlOvqNL7IzW/LDHnJ4j6tBNguujCB5QFUF5dOa1z +UR5rupMvv2KpEgRcfW/d3kwcAxH9nI0SHKJenhtweyajUgInK88TC+aT4909c2XA +EADYyWxj1DMz3/sMpvGegHsfTPegNoDgz2yEKdu53dr4BUpF6E+eoCX9Hv78SWH3 +/rAlkbffzCL5d+I8y0jzEpLEqA== +-----END CERTIFICATE----- diff --git a/plugin/tls/test_key.pem b/plugin/tls/test_key.pem new file mode 100644 index 000000000..2ca2b2134 --- /dev/null +++ b/plugin/tls/test_key.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQCsBiLDVSOViP6c +hNFSWPmEDuPxTapZzI1hwN+pCe3XvdnzJHs0miSq3cF3c+WsFlLZe1hI2iHFGvip +4m7+DtQGt799IRT9k4waXs2MEYpATKYnR3ysGDnaKIY3BT4REj2lxBEDoMISNVs6 +aMOJR3nLO5Wi0b9LF6kZQnam4nSnSBYIwQv2x2pjocd3+F2PoHr67bEP01QKRf2k +sLsI8Ukec/S8s7hYhYvPkLvLOl4s72vs1+e77YdxEua4VlU83GQyrtTKapHXDllg +lDmn0TunLENVfEedOpDItoBLb/26as7BWcPU1o3c50CEN51dsfRwXTwZwhAP2ueW +QsxbgIq3AgMBAAECggEAF3FCnYHltoQTxnqnF+S+JAvvbjvaQiCJB9BD6oJK4kKi +B+tpytJSuuI7ci7eFqR4J+ESN+NaBMVXK7eKzp5wsHWr575xYNkRl6phsnvVbkvD +vMiWKdGnWJ57I9ZYDfWBZyyf8PGgYODajMwoEXYnF9YH30dcHTydM68GAloL8Zu9 +CtGCmlu4TER0BvG+rK2OD5lt8ORK56eMwzTTqMy0hCkP5VEq8j9RmekEzrgtWKm8 +OI3i8VnpOA0RCVhJ0q5a5jt/xbKRjFNsUNmy9HBRYg7Iw3SCEHmDtz1R9A9rvaJC +WXqwKbGZPY8W69h8BhKcJ5RrKt2PZyJxw+LB610XSQKBgQDR/LIGXdJR/90epiGC +p68W9Vc3eWxJlAtLDQCSULphLi6j7D+jesmhD3z2woBPjxkd4TaZa2t94Q1MzSeC +ON/Aux1huto9ddxvijUQJN3Ep4zPkHdNzHfRwIZsgGH8u77VY/5I4V7IgxKjWlJ6 +Ii8ez8xpWj1rnQ0azSaYIcVl7QKBgQDRt+J+iRjKxHWuXoBFfv8oMfl+iYaMdJxu +PELWb3RLsZ92hobSAmNR/gC3T7p8NFJlQVCoxZr8zt/Rvqh4aK3aSOuKeUvYAjs1 +/YbPcdSn6uTTIOi6CcHaJ8ZUXNvY5FuoT0+Q9Eb8fw5NGzxsgsfhScELLgbFKb5E +Tkw43ZqeswKBgQCxXBgZnIEaVVw0mOlQ68TNRWfnKR23f92SBGdpLdpeXp1yQwb1 +U66d5PENkvbBPAJg5GozZzGhXsbXCajHKraCmQiWFTZkFvqbE0cCXcEaatJaNpEu +GvdRKKXhWwZoa0MiBZUvhXuDLII/iviCxAC8q5LhoSCjlkENVB22/T83eQKBgQC4 +c3wRALG+fWZns5QsC5ONnc6rXXfqhxGi3vuGMMbfYF05WP6xLQp/7eBhWg1R+o7R +oc24cvxrB+TRTFhOdvsZtvL7es2bMfMz/EUapSp9edpCW3p1Temi30LPplByhf6b +nQ4FFuRsZa+FX8QYSDpWypCwLY4k0R8YYqklhrrcgwKBgFiM/GnRc230nj0GGWf1 ++Ve2M/TQCgS6ufr2F0vU7QkEWfeiN9iunhmhsggqWxOEOU77FhCkQRtztm93hG0K +eKoHNh/1HvHGBWsR0TaMDw3n8t7Yg5NmQb617nBELZbxxpd358muLiHDoix86W9Q +xM6hB159G1gOEJsi8exm5AlZ +-----END PRIVATE KEY----- diff --git a/plugin/tls/tls.go b/plugin/tls/tls.go index e08e522ab..cde543ea0 100644 --- a/plugin/tls/tls.go +++ b/plugin/tls/tls.go @@ -1,6 +1,8 @@ package tls import ( + ctls "crypto/tls" + "github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/plugin/pkg/tls" @@ -16,6 +18,14 @@ func init() { } func setup(c *caddy.Controller) error { + err := parseTLS(c) + if err != nil { + return plugin.Error("tls", err) + } + return nil +} + +func parseTLS(c *caddy.Controller) error { config := dnsserver.GetConfig(c) if config.TLSConfig != nil { @@ -27,10 +37,39 @@ func setup(c *caddy.Controller) error { if len(args) < 2 || len(args) > 3 { return plugin.Error("tls", c.ArgErr()) } + clientAuth := ctls.NoClientCert + for c.NextBlock() { + switch c.Val() { + case "client_auth": + authTypeArgs := c.RemainingArgs() + if len(authTypeArgs) != 1 { + return c.ArgErr() + } + switch authTypeArgs[0] { + case "nocert": + clientAuth = ctls.NoClientCert + case "request": + clientAuth = ctls.RequestClientCert + case "require": + clientAuth = ctls.RequireAnyClientCert + case "verify_if_given": + clientAuth = ctls.VerifyClientCertIfGiven + case "require_and_verify": + clientAuth = ctls.RequireAndVerifyClientCert + default: + return c.Errf("unknown authentication type '%s'", authTypeArgs[0]) + } + default: + return c.Errf("unknown option '%s'", c.Val()) + } + } tls, err := tls.NewTLSConfigFromArgs(args...) if err != nil { - return plugin.Error("tls", err) + return err } + tls.ClientAuth = clientAuth + // NewTLSConfigFromArgs only sets RootCAs, so we need to let ClientCAs refer to it. + tls.ClientCAs = tls.RootCAs config.TLSConfig = tls } return nil diff --git a/plugin/tls/tls_test.go b/plugin/tls/tls_test.go index 0bbba18a1..aeef9e6bc 100644 --- a/plugin/tls/tls_test.go +++ b/plugin/tls/tls_test.go @@ -1,9 +1,12 @@ package tls import ( + "crypto/tls" "strings" "testing" + "github.com/coredns/coredns/core/dnsserver" + "github.com/mholt/caddy" ) @@ -16,6 +19,11 @@ func TestTLS(t *testing.T) { }{ // positive // negative + {"tls test_cert.pem test_key.pem test_ca.pem {\nunknown\n}", true, "", "unknown option"}, + // client_auth takes exactly one parameter, which must be one of known keywords. + {"tls test_cert.pem test_key.pem test_ca.pem {\nclient_auth\n}", true, "", "Wrong argument"}, + {"tls test_cert.pem test_key.pem test_ca.pem {\nclient_auth none bogus\n}", true, "", "Wrong argument"}, + {"tls test_cert.pem test_key.pem test_ca.pem {\nclient_auth bogus\n}", true, "", "unknown authentication type"}, } for i, test := range tests { @@ -38,3 +46,37 @@ func TestTLS(t *testing.T) { } } } + +func TestTLSClientAuthentication(t *testing.T) { + // Invalid configurations are tested in the general test case. In this test we only look into specific details of valid client_auth options. + tests := []struct { + option string // tls plugin option(s) + expectedType tls.ClientAuthType // expected authentication type. + }{ + // By default, or if 'nocert' is specified, no cert should be requested. + // Other cases should be a straightforward mapping from the keyword to the type value. + {"", tls.NoClientCert}, + {"{\nclient_auth nocert\n}", tls.NoClientCert}, + {"{\nclient_auth request\n}", tls.RequestClientCert}, + {"{\nclient_auth require\n}", tls.RequireAnyClientCert}, + {"{\nclient_auth verify_if_given\n}", tls.VerifyClientCertIfGiven}, + {"{\nclient_auth require_and_verify\n}", tls.RequireAndVerifyClientCert}, + } + + for i, test := range tests { + input := "tls test_cert.pem test_key.pem test_ca.pem " + test.option + c := caddy.NewTestController("dns", input) + err := setup(c) + if err != nil { + t.Errorf("Test %d: TLS config is unexpectedly rejected: %v", i, err) + continue // there's no point in the rest of the tests. + } + cfg := dnsserver.GetConfig(c) + if cfg.TLSConfig.ClientCAs == nil { + t.Errorf("Test %d: Client CA is not configured", i) + } + if cfg.TLSConfig.ClientAuth != test.expectedType { + t.Errorf("Test %d: Unexpected client auth type: %d", i, cfg.TLSConfig.ClientAuth) + } + } +}