From 9d93c520984b7c92cc748e4f346f1971b3b5f372 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 26 Mar 2018 20:31:17 -0700
Subject: [PATCH] agent/connect: support any values in the URL

---
 agent/connect/uri.go      | 37 ++++++++++++++++++++++++++++++++-----
 agent/connect/uri_test.go | 15 +++++++++++++++
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/agent/connect/uri.go b/agent/connect/uri.go
index 3b56ec4ae..3562f2d6c 100644
--- a/agent/connect/uri.go
+++ b/agent/connect/uri.go
@@ -20,7 +20,7 @@ type CertURI interface {
 
 var (
 	spiffeIDServiceRegexp = regexp.MustCompile(
-		`^/ns/(\w+)/dc/(\w+)/svc/(\w+)$`)
+		`^/ns/([^/]+)/dc/([^/]+)/svc/([^/]+)$`)
 )
 
 // ParseCertURI parses a the URI value from a TLS certificate.
@@ -29,13 +29,40 @@ func ParseCertURI(input *url.URL) (CertURI, error) {
 		return nil, fmt.Errorf("SPIFFE ID must have 'spiffe' scheme")
 	}
 
+	// Path is the raw value of the path without url decoding values.
+	// RawPath is empty if there were no encoded values so we must
+	// check both.
+	path := input.Path
+	if input.RawPath != "" {
+		path = input.RawPath
+	}
+
 	// Test for service IDs
-	if v := spiffeIDServiceRegexp.FindStringSubmatch(input.Path); v != nil {
+	if v := spiffeIDServiceRegexp.FindStringSubmatch(path); v != nil {
+		// Determine the values. We assume they're sane to save cycles,
+		// but if the raw path is not empty that means that something is
+		// URL encoded so we go to the slow path.
+		ns := v[1]
+		dc := v[2]
+		service := v[3]
+		if input.RawPath != "" {
+			var err error
+			if ns, err = url.PathUnescape(v[1]); err != nil {
+				return nil, fmt.Errorf("Invalid namespace: %s", err)
+			}
+			if dc, err = url.PathUnescape(v[2]); err != nil {
+				return nil, fmt.Errorf("Invalid datacenter: %s", err)
+			}
+			if service, err = url.PathUnescape(v[3]); err != nil {
+				return nil, fmt.Errorf("Invalid service: %s", err)
+			}
+		}
+
 		return &SpiffeIDService{
 			Host:       input.Host,
-			Namespace:  v[1],
-			Datacenter: v[2],
-			Service:    v[3],
+			Namespace:  ns,
+			Datacenter: dc,
+			Service:    service,
 		}, nil
 	}
 
diff --git a/agent/connect/uri_test.go b/agent/connect/uri_test.go
index 247170f53..2f28c940d 100644
--- a/agent/connect/uri_test.go
+++ b/agent/connect/uri_test.go
@@ -34,6 +34,18 @@ var testCertURICases = []struct {
 		"",
 	},
 
+	{
+		"service with URL-encoded values",
+		"spiffe://1234.consul/ns/foo%2Fbar/dc/bar%2Fbaz/svc/baz%2Fqux",
+		&SpiffeIDService{
+			Host:       "1234.consul",
+			Namespace:  "foo/bar",
+			Datacenter: "bar/baz",
+			Service:    "baz/qux",
+		},
+		"",
+	},
+
 	{
 		"signing ID",
 		"spiffe://1234.consul",
@@ -56,6 +68,9 @@ func TestParseCertURI(t *testing.T) {
 
 			// Parse the ID and check the error/return value
 			actual, err := ParseCertURI(uri)
+			if err != nil {
+				t.Logf("parse error: %s", err.Error())
+			}
 			assert.Equal(tc.ParseError != "", err != nil, "error value")
 			if err != nil {
 				assert.Contains(err.Error(), tc.ParseError)