From 68914ea368f4645de1cf6365a6948112b92c408d Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 19 Sep 2023 12:43:29 +0200 Subject: [PATCH] fix(gateway): normalization of DNSLink inlining (#462) Co-authored-by: Henrique Dias --- .github/workflows/gateway-conformance.yml | 2 +- .github/workflows/gateway-sharness.yml | 2 +- CHANGELOG.md | 3 ++ gateway/hostname.go | 43 +++++++++++++++-------- gateway/hostname_test.go | 5 ++- 5 files changed, 37 insertions(+), 18 deletions(-) diff --git a/.github/workflows/gateway-conformance.yml b/.github/workflows/gateway-conformance.yml index 61471c015..a2a3734f6 100644 --- a/.github/workflows/gateway-conformance.yml +++ b/.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: diff --git a/.github/workflows/gateway-sharness.yml b/.github/workflows/gateway-sharness.yml index 72c36a26c..25daf6141 100644 --- a/.github/workflows/gateway-sharness.yml +++ b/.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: diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a4122902..b11a4165e 100644 --- a/CHANGELOG.md +++ b/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] diff --git a/gateway/hostname.go b/gateway/hostname.go index 0bf6b4d72..758d8499c 100644 --- a/gateway/hostname.go +++ b/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" { diff --git a/gateway/hostname_test.go b/gateway/hostname_test.go index a58e0d404..f7cee35a2 100644 --- a/gateway/hostname_test.go +++ b/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},