Repository: spegel-org/spegel
Pull request #785 (2025-03-17 10:40:02)

Fix verification of digests when parsing distribution path
This change validates that the reference in blob and manifests paths are valid digests. Before it was possible to send any text which is not compatible with the spec.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 686fd4b..c5f9ecf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - [#743](https://github.com/spegel-org/spegel/pull/743) Charts - removed metrics label from bootstrap service
 - [#748](https://github.com/spegel-org/spegel/pull/748) Fix topology annotation.
+- [#785](https://github.com/spegel-org/spegel/pull/785) Fix verification of digests when parsing distribution path.
 
 ### Security
 
diff --git a/pkg/oci/distribution.go b/pkg/oci/distribution.go
index f28d81c..e0cfb0f 100644
--- a/pkg/oci/distribution.go
+++ b/pkg/oci/distribution.go
@@ -79,20 +79,28 @@ func ParseDistributionPath(u *url.URL) (DistributionPath, error) {
 	}
 	comps = manifestRegexDigest.FindStringSubmatch(u.Path)
 	if len(comps) == 6 {
+		dgst, err := digest.Parse(comps[5])
+		if err != nil {
+			return DistributionPath{}, err
+		}
 		dist := DistributionPath{
 			Kind:     DistributionKindManifest,
 			Name:     comps[1],
-			Digest:   digest.Digest(comps[5]),
+			Digest:   dgst,
 			Registry: registry,
 		}
 		return dist, nil
 	}
 	comps = blobsRegexDigest.FindStringSubmatch(u.Path)
 	if len(comps) == 6 {
+		dgst, err := digest.Parse(comps[5])
+		if err != nil {
+			return DistributionPath{}, err
+		}
 		dist := DistributionPath{
 			Kind:     DistributionKindBlob,
 			Name:     comps[1],
-			Digest:   digest.Digest(comps[5]),
+			Digest:   dgst,
 			Registry: registry,
 		}
 		return dist, nil


Test patch:
diff --git a/pkg/oci/distribution_test.go b/pkg/oci/distribution_test.go
index cf5f40a..e3edb73 100644
--- a/pkg/oci/distribution_test.go
+++ b/pkg/oci/distribution_test.go
@@ -91,23 +91,59 @@ func TestParseDistributionPath(t *testing.T) {
 	}
 }
 
-func TestParseDistributionPathInvalidPath(t *testing.T) {
+func TestParseDistributionPathErrors(t *testing.T) {
 	t.Parallel()
 
-	u := &url.URL{
-		Path:     "/v2/spegel-org/spegel/v0.0.1",
-		RawQuery: "ns=example.com",
+	tests := []struct {
+		name          string
+		url           *url.URL
+		expectedError string
+	}{
+		{
+			name: "invalid path",
+			url: &url.URL{
+				Path:     "/v2/spegel-org/spegel/v0.0.1",
+				RawQuery: "ns=example.com",
+			},
+			expectedError: "distribution path could not be parsed",
+		},
+		{
+			name: "blob with tag reference",
+			url: &url.URL{
+				Path:     "/v2/spegel-org/spegel/blobs/v0.0.1",
+				RawQuery: "ns=example.com",
+			},
+			expectedError: "invalid checksum digest format",
+		},
+		{
+			name: "blob with invalid digest",
+			url: &url.URL{
+				Path:     "/v2/spegel-org/spegel/blobs/sha256:123",
+				RawQuery: "ns=example.com",
+			},
+			expectedError: "invalid checksum digest length",
+		},
+		{
+			name: "manifest tag with missing registry",
+			url: &url.URL{
+				Path: "/v2/spegel-org/spegel/manifests/v0.0.1",
+			},
+			expectedError: "registry parameter needs to be set for tag references",
+		},
+		{
+			name: "manifest with invalid digest",
+			url: &url.URL{
+				Path: "/v2/spegel-org/spegel/manifests/sha253:foobar",
+			},
+			expectedError: "unsupported digest algorithm",
+		},
 	}
-	_, err := ParseDistributionPath(u)
-	require.EqualError(t, err, "distribution path could not be parsed")
-}
-
-func TestParseDistributionPathMissingRegistry(t *testing.T) {
-	t.Parallel()
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
 
-	u := &url.URL{
-		Path: "/v2/spegel-org/spegel/manifests/v0.0.1",
+			_, err := ParseDistributionPath(tt.url)
+			require.EqualError(t, err, tt.expectedError)
+		})
 	}
-	_, err := ParseDistributionPath(u)
-	require.EqualError(t, err, "registry parameter needs to be set for tag references")
 }
diff --git a/pkg/registry/registry_test.go b/pkg/registry/registry_test.go
index 9ea7a87..4c366a4 100644
--- a/pkg/registry/registry_test.go
+++ b/pkg/registry/registry_test.go
@@ -118,10 +118,14 @@ func TestMirrorHandler(t *testing.T) {
 	unreachableAddrPort := netip.MustParseAddrPort("127.0.0.1:0")
 
 	resolver := map[string][]netip.AddrPort{
-		"no-working-peers":  {badAddrPort, unreachableAddrPort, badAddrPort},
-		"first-peer":        {goodAddrPort, badAddrPort, badAddrPort},
-		"first-peer-error":  {unreachableAddrPort, goodAddrPort},
-		"last-peer-working": {badAddrPort, badAddrPort, goodAddrPort},
+		// No working peers
+		"sha256:c3e30fbcf3b231356a1efbd30a8ccec75134a7a8b45217ede97f4ff483540b04": {badAddrPort, unreachableAddrPort, badAddrPort},
+		// First Peer
+		"sha256:3b8a55c543ccc7ae01c47b1d35af5826a6439a9b91ab0ca96de9967759279896": {goodAddrPort, badAddrPort, badAddrPort},
+		// First peer error
+		"sha256:a0daab85ec30e2809a38c32fa676515aba22f481c56fda28637ae964ff398e3d": {unreachableAddrPort, goodAddrPort},
+		// Last peer working
+		"sha256:11242d2a347bf8ab30b9f92d5ca219bbbedf95df5a8b74631194561497c1fae8": {badAddrPort, badAddrPort, goodAddrPort},
 	}
 	router := routing.NewMemoryRouter(resolver, netip.AddrPort{})
 	reg := NewRegistry(nil, router)
@@ -142,28 +146,28 @@ func TestMirrorHandler(t *testing.T) {
 		},
 		{
 			name:            "request should not timeout and give 404 if all peers fail",
-			key:             "no-working-peers",
+			key:             "sha256:c3e30fbcf3b231356a1efbd30a8ccec75134a7a8b45217ede97f4ff483540b04",
 			expectedStatus:  http.StatusNotFound,
 			expectedBody:    "",
 			expectedHeaders: nil,
 		},
 		{
 			name:            "request should work when first peer responds",
-			key:             "first-peer",
+			key:             "sha256:3b8a55c543ccc7ae01c47b1d35af5826a6439a9b91ab0ca96de9967759279896",
 			expectedStatus:  http.StatusOK,
 			expectedBody:    "hello world",
 			expectedHeaders: map[string][]string{"foo": {"bar"}},
 		},
 		{
 			name:            "second peer should respond when first gives error",
-			key:             "first-peer-error",
+			key:             "sha256:a0daab85ec30e2809a38c32fa676515aba22f481c56fda28637ae964ff398e3d",
 			expectedStatus:  http.StatusOK,
 			expectedBody:    "hello world",
 			expectedHeaders: map[string][]string{"foo": {"bar"}},
 		},
 		{
 			name:            "last peer should respond when two first fail",
-			key:             "last-peer-working",
+			key:             "sha256:11242d2a347bf8ab30b9f92d5ca219bbbedf95df5a8b74631194561497c1fae8",
 			expectedStatus:  http.StatusOK,
 			expectedBody:    "hello world",
 			expectedHeaders: map[string][]string{"foo": {"bar"}},