From 69c2d78f690d6b23f5dd51eebaf706d094cc8780 Mon Sep 17 00:00:00 2001 From: W-Mark Kubacki Date: Fri, 15 Apr 2016 22:21:55 +0200 Subject: [PATCH] Support configuring less restrictive TLS client auth requirements Caddyfile parameter "clients" of "tls" henceforth accepts a special first modifier. It is one of, and effects: * request = tls.RequestClientCert * require = tls.RequireAnyClientCert * verify_if_given = tls.VerifyClientCertIfGiven * (none) = tls.RequireAndVerifyClientCert The use-case for this is as follows: A middleware would serve items to the public, but if a certificate were given the middleware would permit file manipulation. And, in a different plugin such as a forum or blog, not verifying a client cert would be nice for registration: said blog would subsequently only compare the SPKI of a client certificate. --- caddy/https/setup.go | 24 +++++++++++- caddy/https/setup_test.go | 77 ++++++++++++++++++++++++++++----------- server/config.go | 2 + server/server.go | 14 ++++--- 4 files changed, 88 insertions(+), 29 deletions(-) diff --git a/caddy/https/setup.go b/caddy/https/setup.go index ac275466..eebfc62d 100644 --- a/caddy/https/setup.go +++ b/caddy/https/setup.go @@ -83,10 +83,30 @@ func Setup(c *setup.Controller) (middleware.Middleware, error) { c.TLS.Ciphers = append(c.TLS.Ciphers, value) } case "clients": - c.TLS.ClientCerts = c.RemainingArgs() - if len(c.TLS.ClientCerts) == 0 { + clientCertList := c.RemainingArgs() + if len(clientCertList) == 0 { return nil, c.ArgErr() } + + listStart, mustProvideCA := 1, true + switch clientCertList[0] { + case "request": + c.TLS.ClientAuth = tls.RequestClientCert + mustProvideCA = false + case "require": + c.TLS.ClientAuth = tls.RequireAnyClientCert + mustProvideCA = false + case "verify_if_given": + c.TLS.ClientAuth = tls.VerifyClientCertIfGiven + default: + c.TLS.ClientAuth = tls.RequireAndVerifyClientCert + listStart = 0 + } + if mustProvideCA && len(clientCertList) <= listStart { + return nil, c.ArgErr() + } + + c.TLS.ClientCerts = clientCertList[listStart:] case "load": c.Args(&loadDir) c.TLS.Manual = true diff --git a/caddy/https/setup_test.go b/caddy/https/setup_test.go index 378a24f8..59a772c4 100644 --- a/caddy/https/setup_test.go +++ b/caddy/https/setup_test.go @@ -189,34 +189,69 @@ func TestSetupParseWithWrongOptionalParams(t *testing.T) { } func TestSetupParseWithClientAuth(t *testing.T) { + // Test missing client cert file params := `tls ` + certFile + ` ` + keyFile + ` { - clients client_ca.crt client2_ca.crt + clients }` c := setup.NewTestController(params) _, err := Setup(c) - if err != nil { - t.Errorf("Expected no errors, got: %v", err) - } - - if count := len(c.TLS.ClientCerts); count != 2 { - t.Fatalf("Expected two client certs, had %d", count) - } - if actual := c.TLS.ClientCerts[0]; actual != "client_ca.crt" { - t.Errorf("Expected first client cert file to be '%s', but was '%s'", "client_ca.crt", actual) - } - if actual := c.TLS.ClientCerts[1]; actual != "client2_ca.crt" { - t.Errorf("Expected second client cert file to be '%s', but was '%s'", "client2_ca.crt", actual) - } - - // Test missing client cert file - params = `tls ` + certFile + ` ` + keyFile + ` { - clients - }` - c = setup.NewTestController(params) - _, err = Setup(c) if err == nil { t.Errorf("Expected an error, but no error returned") } + + noCAs, twoCAs := []string{}, []string{"client_ca.crt", "client2_ca.crt"} + for caseNumber, caseData := range []struct { + params string + clientAuthType tls.ClientAuthType + expectedErr bool + expectedCAs []string + }{ + {"", tls.NoClientCert, false, noCAs}, + {`tls ` + certFile + ` ` + keyFile + ` { + clients client_ca.crt client2_ca.crt + }`, tls.RequireAndVerifyClientCert, false, twoCAs}, + // now come modifier + {`tls ` + certFile + ` ` + keyFile + ` { + clients request + }`, tls.RequestClientCert, false, noCAs}, + {`tls ` + certFile + ` ` + keyFile + ` { + clients require + }`, tls.RequireAnyClientCert, false, noCAs}, + {`tls ` + certFile + ` ` + keyFile + ` { + clients verify_if_given client_ca.crt client2_ca.crt + }`, tls.VerifyClientCertIfGiven, false, twoCAs}, + {`tls ` + certFile + ` ` + keyFile + ` { + clients verify_if_given + }`, tls.VerifyClientCertIfGiven, true, noCAs}, + } { + c := setup.NewTestController(caseData.params) + _, err := Setup(c) + if caseData.expectedErr { + if err == nil { + t.Errorf("In case %d: Expected an error, got: %v", caseNumber, err) + } + continue + } + if err != nil { + t.Errorf("In case %d: Expected no errors, got: %v", caseNumber, err) + } + + if caseData.clientAuthType != c.TLS.ClientAuth { + t.Errorf("In case %d: Expected TLS client auth type %v, got: %v", + caseNumber, caseData.clientAuthType, c.TLS.ClientAuth) + } + + if count := len(c.TLS.ClientCerts); count < len(caseData.expectedCAs) { + t.Fatalf("In case %d: Expected %d client certs, had %d", caseNumber, len(caseData.expectedCAs), count) + } + + for idx, expected := range caseData.expectedCAs { + if actual := c.TLS.ClientCerts[idx]; actual != expected { + t.Errorf("In case %d: Expected %dth client cert file to be '%s', but was '%s'", + caseNumber, idx, expected, actual) + } + } + } } func TestSetupParseWithKeyType(t *testing.T) { diff --git a/server/config.go b/server/config.go index 1f4acdb6..e66ec801 100644 --- a/server/config.go +++ b/server/config.go @@ -1,6 +1,7 @@ package server import ( + "crypto/tls" "net" "github.com/mholt/caddy/middleware" @@ -75,4 +76,5 @@ type TLSConfig struct { ProtocolMaxVersion uint16 PreferServerCipherSuites bool ClientCerts []string + ClientAuth tls.ClientAuthType } diff --git a/server/server.go b/server/server.go index 40689dfa..41d1a637 100644 --- a/server/server.go +++ b/server/server.go @@ -379,17 +379,19 @@ func DefaultErrorFunc(w http.ResponseWriter, r *http.Request, status int) { // setupClientAuth sets up TLS client authentication only if // any of the TLS configs specified at least one cert file. func setupClientAuth(tlsConfigs []TLSConfig, config *tls.Config) error { - var clientAuth bool + whatClientAuth := tls.NoClientCert for _, cfg := range tlsConfigs { - if len(cfg.ClientCerts) > 0 { - clientAuth = true - break + if whatClientAuth < cfg.ClientAuth { // Use the most restrictive. + whatClientAuth = cfg.ClientAuth } } - if clientAuth { + if whatClientAuth != tls.NoClientCert { pool := x509.NewCertPool() for _, cfg := range tlsConfigs { + if len(cfg.ClientCerts) == 0 { + continue + } for _, caFile := range cfg.ClientCerts { caCrt, err := ioutil.ReadFile(caFile) // Anyone that gets a cert from this CA can connect if err != nil { @@ -401,7 +403,7 @@ func setupClientAuth(tlsConfigs []TLSConfig, config *tls.Config) error { } } config.ClientCAs = pool - config.ClientAuth = tls.RequireAndVerifyClientCert + config.ClientAuth = whatClientAuth } return nil