Implement first-path-match ACL semantics

We've been actually assuming ACLs were working this way in all our
configuration files, so better make it so: the first path match in the
ACL allow list will cause the list iteration to stop (the final result
will then depend on the CN match).

serverutil/tls.go

@@ -14,7 +14,8 @@ import (
 
 // TLSAuthACL describes a single access control entry. Path and
 // CommonName are anchored regular expressions (they must match the
-// entire string).
+// entire string). The first path to match in a list of ACLs will
+// identify the ACL to be applied.
 type TLSAuthACL struct {
 	Path       string `yaml:"path"`
 	CommonName string `yaml:"cn"`
@@ -32,10 +33,11 @@ func (p *TLSAuthACL) compile() error {
 	return err
 }
 
-func (p *TLSAuthACL) match(req *http.Request) bool {
-	if !p.pathRx.MatchString(req.URL.Path) {
-		return false
-	}
+func (p *TLSAuthACL) matchPath(req *http.Request) bool {
+	return p.pathRx.MatchString(req.URL.Path)
+}
+
+func (p *TLSAuthACL) matchCN(req *http.Request) bool {
 	for _, cert := range req.TLS.PeerCertificates {
 		if p.cnRx.MatchString(cert.Subject.CommonName) {
 			return true
@@ -81,9 +83,13 @@ func (c *TLSAuthConfig) match(req *http.Request) bool {
 	if c == nil || len(c.Allow) == 0 {
 		return true
 	}
+
 	for _, acl := range c.Allow {
-		if acl.match(req) {
-			return true
+		if acl.matchPath(req) {
+			if acl.matchCN(req) {
+				return true
+			}
+			break
 		}
 	}
 	return false

serverutil/tls_test.go

@@ -145,6 +145,18 @@ func TestTLS_Serve(t *testing.T) {
 			Cert: dir + "/server_cert.pem",
 			Key:  dir + "/server_key.pem",
 			CA:   dir + "/ca.pem",
+			Auth: &TLSAuthConfig{
+				Allow: []*TLSAuthACL{
+					&TLSAuthACL{
+						Path:       "/testpath",
+						CommonName: "client1.*",
+					},
+					&TLSAuthACL{
+						Path:       ".*",
+						CommonName: ".*",
+					},
+				},
+			},
 		},
 	}
 	h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -162,15 +174,35 @@ func TestTLS_Serve(t *testing.T) {
 			},
 		},
 	}
-	_, err := c.Get("https://localhost:19898/")
-	if err == nil {
-		t.Fatal("client without certificate got a successful reply")
-	}
 
 	// A client with a properly signed cert will get a successful reply.
-	c = newTestClient(t, dir, "client1")
-	_, err = c.Get("https://localhost:19898/")
-	if err != nil {
-		t.Fatalf("client with cert got error: %v", err)
+	c1 := newTestClient(t, dir, "client1")
+	c2 := newTestClient(t, dir, "client2")
+	testdata := []struct {
+		tag        string
+		client     *http.Client
+		uri        string
+		expectedOk bool
+	}{
+		{"no-cert", c, "/", false},
+		{"client1", c1, "/", true},
+		{"client2", c2, "/", true},
+		{"client1", c1, "/testpath", true},
+		{"client2", c2, "/testpath", false},
+	}
+
+	for _, td := range testdata {
+		resp, err := td.client.Get("https://localhost:19898" + td.uri)
+		ok := false
+		if err == nil {
+			if resp.StatusCode == 200 {
+				ok = true
+			} else {
+				err = fmt.Errorf("HTTP status %s", resp.Status)
+			}
+		}
+		if ok != td.expectedOk {
+			t.Errorf("client %s requesting %s got ok=%v, expected=%v (err=%v)", td.tag, td.uri, td.expectedOk, ok, err)
+		}
 	}
 }