fix(gateway): normalization of DNSLink inlining (#462)

Co-authored-by: Henrique Dias <[email protected]>

.github/workflows/gateway-conformance.yml

@@ -25,7 +25,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@v3
         with:
-          go-version: 1.19.x
+          go-version: 1.21.x
       - name: Checkout boxo
         uses: actions/checkout@v3
         with:

.github/workflows/gateway-sharness.yml

@@ -18,7 +18,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@v4
         with:
-          go-version: 1.19.1
+          go-version: 1.21.x
       - name: Checkout boxo
         uses: actions/checkout@v3
         with:

CHANGELOG.md

@@ -38,6 +38,9 @@ The following emojis are used to highlight certain changes:
 
 ### Fixed
 
+* The normalization of DNSLink identifiers in `gateway` has been corrected in the edge
+  case where the value passed to the path component of the URL is already normalized.
+
 ### Security
 
 ## [v0.12.0]

gateway/hostname.go

@@ -300,7 +300,7 @@ func isHTTPSRequest(r *http.Request) bool {
 	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto
 	xproto := r.Header.Get("X-Forwarded-Proto")
 	// Is request a native TLS (not used atm, but future-proofing)
-	// or a proxied HTTPS (eg. go-ipfs behind nginx at a public gw)?
+	// or a proxied HTTPS (eg. Kubo behind nginx at a public gw)?
 	return r.URL.Scheme == "https" || xproto == "https"
 }
 
@@ -396,27 +396,40 @@ func toSubdomainURL(hostname, path string, r *http.Request, inlineDNSLink bool,
 		}
 	} else { // rootID is not a CID
 
-		// Check if rootID is a FQDN with DNSLink and convert it to TLS-safe
-		// representation that fits in a single DNS label.  We support this so
-		// loading DNSLink names over TLS "just works" on public HTTP gateways
-		// that pass 'https' in X-Forwarded-Proto to go-ipfs.
-		//
-		// Rationale can be found under "Option C"
-		// at: https://github.com/ipfs/in-web-browsers/issues/169
-		//
-		// TLDR is:
-		// /ipns/my.v-long.example.com
-		// can be loaded from a subdomain gateway with a wildcard TLS cert if
-		// represented as a single DNS label:
-		// https://my-v--long-example-com.ipns.dweb.link
+		// If rootID is an inlined notation of a FQDN with DNSLink we need to
+		// un-inline it first, to make it work in contexts where subdomain
+		// identifier is used on a path (/ipns/my-v--long-example-com)
+		// e.g. when ipfs-companion extension passes value from subdomain gateway
+		// for further normalization: https://github.com/ipfs/ipfs-companion/issues/1278#issuecomment-1724550623
+		if ns == "ipns" && !strings.Contains(rootID, ".") && strings.Contains(rootID, "-") {
+			dnsLinkFqdn := toDNSLinkFQDN(rootID) // my-v--long-example-com → my.v-long.example.com
+			if hasDNSLinkRecord(r.Context(), backend, dnsLinkFqdn) {
+				// update path prefix to use real FQDN with DNSLink
+				rootID = dnsLinkFqdn
+			}
+		}
+
 		if (inlineDNSLink || isHTTPS) && ns == "ipns" && strings.Contains(rootID, ".") {
+			// If rootID is a FQDN with DNSLink we need to inline it to make it TLS-safe
+			// representation that fits in a single DNS label.  We support this so
+			// loading DNSLink names over TLS "just works" on public HTTP gateways
+			// that pass 'https' in X-Forwarded-Proto to Kubo.
+			//
+			// Rationale can be found under "Option C"
+			// at: https://github.com/ipfs/in-web-browsers/issues/169
+			//
+			// TLDR is:
+			// /ipns/my.v-long.example.com
+			// can be loaded from a subdomain gateway with a wildcard TLS cert if
+			// represented as a single DNS label:
+			// https://my-v--long-example-com.ipns.dweb.link
 			if hasDNSLinkRecord(r.Context(), backend, rootID) {
 				// my.v-long.example.com → my-v--long-example-com
 				dnsLabel, err := toDNSLinkDNSLabel(rootID)
 				if err != nil {
 					return "", err
 				}
-				// update path prefix to use real FQDN with DNSLink
+				// update path prefix to use inlined FQDN with DNSLink as a single DNS label
 				rootID = dnsLabel
 			}
 		} else if ns == "ipfs" {

gateway/hostname_test.go

@@ -55,9 +55,12 @@ func TestToSubdomainURL(t *testing.T) {
 		{httpRequest, "dweb.link", false, "/ipns/dnslink.long-name.example.com", "http://dnslink.long-name.example.com.ipns.dweb.link/", nil},
 		{httpsRequest, "dweb.link", false, "/ipns/dnslink.long-name.example.com", "https://dnslink-long--name-example-com.ipns.dweb.link/", nil},
 		{httpsProxiedRequest, "dweb.link", false, "/ipns/dnslink.long-name.example.com", "https://dnslink-long--name-example-com.ipns.dweb.link/", nil},
-		// HTTP requests can also be converted to fit into a single DNS label - https://github.com/ipfs/kubo/issues/9243
+		// Enabling DNS label inlining: HTTP requests can also be converted to fit into a single DNS label when it matters - https://github.com/ipfs/kubo/issues/9243
 		{httpRequest, "localhost", true, "/ipns/dnslink.long-name.example.com", "http://dnslink-long--name-example-com.ipns.localhost/", nil},
 		{httpRequest, "dweb.link", true, "/ipns/dnslink.long-name.example.com", "http://dnslink-long--name-example-com.ipns.dweb.link/", nil},
+		// Disabling DNS label inlining: should un-inline any inlined DNS labels put in a path
+		{httpRequest, "localhost", false, "/ipns/dnslink-long--name-example-com", "http://dnslink.long-name.example.com.ipns.localhost/", nil},
+		{httpRequest, "dweb.link", false, "/ipns/dnslink-long--name-example-com", "http://dnslink.long-name.example.com.ipns.dweb.link/", nil},
 		// Correctly redirects paths when there is a ? (question mark) character - https://github.com/ipfs/kubo/issues/9882
 		{httpRequest, "localhost", false, "/ipns/example.com/this is a file with some spaces . dots and - but also a ?.png", "http://example.com.ipns.localhost/this%20is%20a%20file%20with%20some%20spaces%20.%20dots%20and%20-%20but%20also%20a%20%3F.png", nil},
 		{httpRequest, "localhost", false, "/ipfs/QmbCMUZw6JFeZ7Wp9jkzbye3Fzp2GGcPgC3nmeUjfVF87n/this is a file with some spaces . dots and - but also a ?.png", "http://bafybeif7a7gdklt6hodwdrmwmxnhksctcuav6lfxlcyfz4khzl3qfmvcgu.ipfs.localhost/this%20is%20a%20file%20with%20some%20spaces%20.%20dots%20and%20-%20but%20also%20a%20%3F.png", nil},