add validation to allowed-origins to include path (#265)

* add validation to allowed-origins to include path

* Origins with a path receive a trailing '/' and added tests for origins with paths

* [NR] add some documentation on how  works

README.md

@@ -318,7 +318,7 @@ Options:
   -forward-headers          Forwards custom headers to the image source server. -enable-url-source flag must be defined.
   -enable-url-signature     Enable URL signature (URL-safe Base64-encoded HMAC digest) [default: false]
   -url-signature-key        The URL signature key (32 characters minimum)
-  -allowed-origins <urls>   Restrict remote image source processing to certain origins (separated by commas)
+  -allowed-origins <urls>   Restrict remote image source processing to certain origins (separated by commas). Note: Origins are validated against host *AND* path. 
   -max-allowed-size <bytes> Restrict maximum size of http image source (in bytes)
   -certfile <path>          TLS certificate file path
   -keyfile <path>           TLS private key file path
@@ -441,6 +441,19 @@ curl -O "http://localhost:8088/crop?width=500&height=200&gravity=smart&url=https
 
 ## HTTP API
 
+### Allowed Origins
+
+imaginary can be configured to block all requests for images with a src URL this is not specified in the `allowed-origins` list. Imaginary will validate that the remote url matches the hostname and path of at least one origin in allowed list. Perhaps the easiest way to show how this works is to show some examples.
+
+| `allowed-origins` setting | image url | is valid |
+| ------------------------- | --------- | -------- |
+| `--allowed-origns s3.amazonaws.com/some-bucket/` | `s3.amazonaws.com/some-bucket/images/image.png` | VALID |
+| `--allowed-origns s3.amazonaws.com/some-bucket/` | `s3.amazonaws.com/images/image.png` | NOT VALID (no matching basepath) |
+| `--allowed-origns *.amazonaws.com/some-bucket/` | `anysubdomain.amazonaws.com/some-bucket/images/image.png` | VALID |
+| `--allowed-origns *.amazonaws.com` | `anysubdomain.amazonaws.comimages/image.png` | VALID |
+| `--allowed-origns *.amazonaws.com` | `www.notaws.comimages/image.png` | NOT VALID (no matching host) |
+| `--allowed-origns *.amazonaws.com, foo.amazonaws.com/some-bucket/` | `bar.amazonaws.com/some-other-bucket/image.png` | VALID (matches first condition but not second) |
+
 ### Authorization
 
 imaginary supports a simple token-based API authorization.

imaginary.go

@@ -31,7 +31,7 @@ var (
 	aEnablePlaceholder  = flag.Bool("enable-placeholder", false, "Enable image response placeholder to be used in case of error")
 	aEnableURLSignature = flag.Bool("enable-url-signature", false, "Enable URL signature (URL-safe Base64-encoded HMAC digest)")
 	aURLSignatureKey    = flag.String("url-signature-key", "", "The URL signature key (32 characters minimum)")
-	aAllowedOrigins     = flag.String("allowed-origins", "", "Restrict remote image source processing to certain origins (separated by commas)")
+	aAllowedOrigins     = flag.String("allowed-origins", "", "Restrict remote image source processing to certain origins (separated by commas). Note: Origins are validated against host *AND* path.")
 	aMaxAllowedSize     = flag.Int("max-allowed-size", 0, "Restrict maximum size of http image source (in bytes)")
 	aKey                = flag.String("key", "", "Define API key for authorization")
 	aMount              = flag.String("mount", "", "Mount server local directory")
@@ -292,6 +292,11 @@ func parseOrigins(origins string) []*url.URL {
 		if err != nil {
 			continue
 		}
+
+		if u.Path != "" && u.Path[len(u.Path)-1:] != "/" {
+			u.Path += "/"
+		}
+
 		urls = append(urls, u)
 	}
 	return urls

source_http.go

@@ -30,7 +30,7 @@ func (s *HTTPImageSource) GetImage(req *http.Request) ([]byte, error) {
 		return nil, ErrInvalidImageURL
 	}
 	if shouldRestrictOrigin(url, s.Config.AllowedOrigins) {
-		return nil, fmt.Errorf("not allowed remote URL origin: %s", url.Host)
+		return nil, fmt.Errorf("not allowed remote URL origin: %s%s", url.Host, url.Path)
 	}
 	return s.fetchImage(url, req)
 }
@@ -89,7 +89,7 @@ func (s *HTTPImageSource) setAuthorizationHeader(req *http.Request, ireq *http.R
 func (s *HTTPImageSource) setForwardHeaders(req *http.Request, ireq *http.Request) {
 	headers := s.Config.ForwardHeaders
 	for _, header := range headers {
-		if _,ok := ireq.Header[header]; ok {
+		if _, ok := ireq.Header[header]; ok {
 			req.Header.Set(header, ireq.Header.Get(header))
 		}
 	}
@@ -123,19 +123,19 @@ func shouldRestrictOrigin(url *url.URL, origins []*url.URL) bool {
 
 	for _, origin := range origins {
 		if origin.Host == url.Host {
-			return false
+			return !strings.HasPrefix(url.Path, origin.Path)
 		}
 
 		if origin.Host[0:2] == "*." {
 
 			// Testing if "*.example.org" matches "example.org"
 			if url.Host == origin.Host[2:] {
-				return false
+				return !strings.HasPrefix(url.Path, origin.Path)
 			}
 
 			// Testing if "*.example.org" matches "foo.example.org"
 			if strings.HasSuffix(url.Host, origin.Host[1:]) {
-				return false
+				return !strings.HasPrefix(url.Path, origin.Path)
 			}
 		}
 	}

source_http_test.go

@@ -179,9 +179,9 @@ func TestHttpImageSourceForwardedHeadersNotOverride(t *testing.T) {
 
 	url := createURL("http://bar.com", t)
 
-	r, _ := http.NewRequest(http.MethodGet, "http://foo/bar?url="+url.String() , nil)
+	r, _ := http.NewRequest(http.MethodGet, "http://foo/bar?url="+url.String(), nil)
 	r.Header.Set("Authorization", "foobar")
-	
+
 	source := &HTTPImageSource{&SourceConfig{Authorization: "ValidAPIKey", ForwardHeaders: cases}}
 	if !source.Matches(r) {
 		t.Fatal("Cannot match the request")
@@ -298,16 +298,18 @@ func TestHttpImageSourceExceedsMaximumAllowedLength(t *testing.T) {
 }
 
 func TestShouldRestrictOrigin(t *testing.T) {
-	plainOrigins := []*url.URL{
-		createURL("https://example.org", t),
-	}
+	plainOrigins := parseOrigins(
+		"https://example.org",
+	)
 
-	wildCardOrigins := []*url.URL{
-		createURL("https://localhost", t),
-		createURL("https://*.example.org", t),
-		createURL("https://some.s3.bucket.on.aws.org", t),
-		createURL("https://*.s3.bucket.on.aws.org", t),
-	}
+	wildCardOrigins := parseOrigins(
+		"https://localhost,https://*.example.org,https://some.s3.bucket.on.aws.org,https://*.s3.bucket.on.aws.org",
+	)
+
+	withPathOrigins := parseOrigins(
+		"https://localhost/foo/bar/,https://*.example.org/foo/,https://some.s3.bucket.on.aws.org/my/bucket/," +
+			"https://*.s3.bucket.on.aws.org/my/bucket/,https://no-leading-path-slash.example.org/assets",
+	)
 
 	t.Run("Plain origin", func(t *testing.T) {
 		testURL := createURL("https://example.org/logo.jpg", t)
@@ -334,7 +336,7 @@ func TestShouldRestrictOrigin(t *testing.T) {
 	})
 
 	t.Run("Wildcard origin, sub-sub domain URL", func(t *testing.T) {
-		testURL := createURL("https://n.s3.bucket.on.aws.org/logo.jpg", t)
+		testURL := createURL("https://n.s3.bucket.on.aws.org/our/bucket/logo.jpg", t)
 
 		if shouldRestrictOrigin(testURL, wildCardOrigins) {
 			t.Errorf("Expected '%s' to be allowed with origins: %+v", testURL, wildCardOrigins)
@@ -352,6 +354,84 @@ func TestShouldRestrictOrigin(t *testing.T) {
 			t.Errorf("Expected '%s' to not be allowed with wildcard origins: %+v", testURL, wildCardOrigins)
 		}
 	})
+
+	t.Run("Loopback origin with path, correct URL", func(t *testing.T) {
+		testURL := createURL("https://localhost/foo/bar/logo.png", t)
+
+		if shouldRestrictOrigin(testURL, withPathOrigins) {
+			t.Errorf("Expected '%s' to be allowed with origins: %+v", testURL, withPathOrigins)
+		}
+	})
+
+	t.Run("Wildcard origin with path, correct URL", func(t *testing.T) {
+		testURL := createURL("https://our.company.s3.bucket.on.aws.org/my/bucket/logo.gif", t)
+
+		if shouldRestrictOrigin(testURL, withPathOrigins) {
+			t.Errorf("Expected '%s' to be allowed with origins: %+v", testURL, withPathOrigins)
+		}
+	})
+
+	t.Run("Wildcard origin with partial path, correct URL", func(t *testing.T) {
+		testURL := createURL("https://our.company.s3.bucket.on.aws.org/my/bucket/a/b/c/d/e/logo.gif", t)
+
+		if shouldRestrictOrigin(testURL, withPathOrigins) {
+			t.Errorf("Expected '%s' to be allowed with origins: %+v", testURL, withPathOrigins)
+		}
+	})
+
+	t.Run("Wildcard origin with partial path, correct URL double slashes", func(t *testing.T) {
+		testURL := createURL("https://static.example.org/foo//a//b//c/d/e/logo.webp", t)
+
+		if shouldRestrictOrigin(testURL, withPathOrigins) {
+			t.Errorf("Expected '%s' to be allowed with origins: %+v", testURL, withPathOrigins)
+		}
+	})
+
+	t.Run("Wildcard origin with path missing trailing slash, correct URL", func(t *testing.T) {
+		testURL := createURL("https://no-leading-path-slash.example.org/assets/logo.webp", t)
+
+		if shouldRestrictOrigin(testURL, parseOrigins("https://*.example.org/assets")) {
+			t.Errorf("Expected '%s' to be allowed with origins: %+v", testURL, withPathOrigins)
+		}
+	})
+
+	t.Run("Loopback origin with path, incorrect URL", func(t *testing.T) {
+		testURL := createURL("https://localhost/wrong/logo.png", t)
+
+		if !shouldRestrictOrigin(testURL, withPathOrigins) {
+			t.Errorf("Expected '%s' to be allowed with origins: %+v", testURL, withPathOrigins)
+		}
+	})
+}
+
+func TestParseOrigins(t *testing.T) {
+	t.Run("Appending a trailing slash on paths", func(t *testing.T) {
+		origins := parseOrigins("http://foo.example.org/assets")
+		if origins[0].Path != "/assets/" {
+			t.Errorf("Expected the path to have a trailing /, instead it was: %q", origins[0].Path)
+		}
+	})
+
+	t.Run("Paths should not receive multiple trailing slashes", func(t *testing.T) {
+		origins := parseOrigins("http://foo.example.org/assets/")
+		if origins[0].Path != "/assets/" {
+			t.Errorf("Expected the path to have a single trailing /, instead it was: %q", origins[0].Path)
+		}
+	})
+
+	t.Run("Empty paths are fine", func(t *testing.T) {
+		origins := parseOrigins("http://foo.example.org")
+		if origins[0].Path != "" {
+			t.Errorf("Expected the path to remain empty, instead it was: %q", origins[0].Path)
+		}
+	})
+
+	t.Run("Root paths are fine", func(t *testing.T) {
+		origins := parseOrigins("http://foo.example.org/")
+		if origins[0].Path != "/" {
+			t.Errorf("Expected the path to remain a slash, instead it was: %q", origins[0].Path)
+		}
+	})
 }
 
 func createURL(urlStr string, t *testing.T) *url.URL {