From a7e134e54ff9fdeca075a27b277c12e80459f3e9 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Wed, 18 Oct 2023 19:47:05 +0200 Subject: [PATCH 1/2] fix(gw): use real FQDN in DNSLink errors The old version was not very smart and wasted DNS lookup per every inlined DNSLink label (my-v--long-example-com) before retrying with un-inlined one (my.v-long.example.com). This version is optimized to avoid unnecessary DNS lookup for the golden path majority of inlined DNS names on subdomain will hit. It also ensures that in case of missing DNSLink, the un-inlined domain name is used in error path, which makes better UX. --- gateway/hostname.go | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/gateway/hostname.go b/gateway/hostname.go index 758d8499c6..a986ef24f5 100644 --- a/gateway/hostname.go +++ b/gateway/hostname.go @@ -165,15 +165,32 @@ func NewHostnameHandler(c Config, backend IPFSBackend, next http.Handler) http.H // 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 ns == "ipns" && !strings.Contains(rootID, ".") { - // if there is no TXT recordfor rootID - if !hasDNSLinkRecord(r.Context(), backend, rootID) { - // my-v--long-example-com → my.v-long.example.com - dnslinkFQDN := toDNSLinkFQDN(rootID) - if hasDNSLinkRecord(r.Context(), backend, dnslinkFQDN) { - // update path prefix to use real FQDN with DNSLink - pathPrefix = "/ipns/" + dnslinkFQDN - } + if ns == "ipns" && !strings.Contains(rootID, ".") && strings.Contains(rootID, "-") { + // If there are no '.' but '-' is present in rootID, we most + // likely have an inlined DNSLink (like my-v--long-example-com) + + // We un-inline and check for DNSLink presence on domain with '.' + // first to minimize the amount of DNS lookups: + // my-v--long-example-com → my.v-long.example.com + dnslinkFQDN := toDNSLinkFQDN(rootID) + + // Does _dnslink.my.v-long.example.com exist? + if hasDNSLinkRecord(r.Context(), backend, dnslinkFQDN) { + // Un-inlined DNS name has a valid DNSLink record. + // Update path prefix to use un-inlined FQDN in gateway processing. + pathPrefix = "/ipns/" + dnslinkFQDN // → /ipns/my.v-long.example.com + + } else if !hasDNSLinkRecord(r.Context(), backend, rootID) { + // Inspected _dnslink.my-v--long-example-com as a + // fallback, but it had no DNSLink record either. + + // At this point it is more likely the un-inlined + // dnslinkFQDN is what the end user wanted to load, so + // we switch to that. This ensures the error message + // about missing DNSLink will use the un-inlined FQDN, + // and not the inlined one. + pathPrefix = "/ipns/" + dnslinkFQDN + } } } From 40fb162d5e2bc2c55476566a5f9fb5563c617bb4 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Wed, 18 Oct 2023 22:59:29 +0200 Subject: [PATCH 2/2] feat(gw): improved dnslink inliner faster implementation + exported funcs for reuse outside of boxo/gateway --- gateway/hostname.go | 47 +++++++++++++++++++++++++---- gateway/hostname_test.go | 65 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 8 deletions(-) diff --git a/gateway/hostname.go b/gateway/hostname.go index a986ef24f5..6b485f0b4c 100644 --- a/gateway/hostname.go +++ b/gateway/hostname.go @@ -172,7 +172,7 @@ func NewHostnameHandler(c Config, backend IPFSBackend, next http.Handler) http.H // We un-inline and check for DNSLink presence on domain with '.' // first to minimize the amount of DNS lookups: // my-v--long-example-com → my.v-long.example.com - dnslinkFQDN := toDNSLinkFQDN(rootID) + dnslinkFQDN := UninlineDNSLink(rootID) // Does _dnslink.my.v-long.example.com exist? if hasDNSLinkRecord(r.Context(), backend, dnslinkFQDN) { @@ -323,22 +323,59 @@ func isHTTPSRequest(r *http.Request) bool { // Converts a FQDN to DNS-safe representation that fits in 63 characters: // my.v-long.example.com → my-v--long-example-com -func toDNSLinkDNSLabel(fqdn string) (dnsLabel string, err error) { +// InlineDNSLink implements specification from https://specs.ipfs.tech/http-gateways/subdomain-gateway/#host-request-header +func InlineDNSLink(fqdn string) (dnsLabel string, err error) { + /* What follows is an optimized version this three-liner: dnsLabel = strings.ReplaceAll(fqdn, "-", "--") dnsLabel = strings.ReplaceAll(dnsLabel, ".", "-") if len(dnsLabel) > dnsLabelMaxLength { return "", fmt.Errorf("DNSLink representation incompatible with DNS label length limit of 63: %s", dnsLabel) } return dnsLabel, nil + */ + result := make([]byte, 0, len(fqdn)) + for i := 0; i < len(fqdn); i++ { + char := fqdn[i] + if char == '-' { + result = append(result, '-', '-') + } else if char == '.' { + result = append(result, '-') + } else { + result = append(result, char) + } + } + if len(result) > dnsLabelMaxLength { + return "", fmt.Errorf("inlined DNSLink incompatible with DNS label length limit of 63: %q", result) + } + return string(result), nil } // Converts a DNS-safe representation of DNSLink FQDN to real FQDN: // my-v--long-example-com → my.v-long.example.com -func toDNSLinkFQDN(dnsLabel string) (fqdn string) { +// UninlineDNSLink implements specification from https://specs.ipfs.tech/http-gateways/subdomain-gateway/#host-request-header +func UninlineDNSLink(dnsLabel string) (fqdn string) { + /* What follows is an optimized version this three-liner: fqdn = strings.ReplaceAll(dnsLabel, "--", "@") // @ placeholder is unused in DNS labels fqdn = strings.ReplaceAll(fqdn, "-", ".") fqdn = strings.ReplaceAll(fqdn, "@", "-") return fqdn + */ + result := make([]byte, 0, len(dnsLabel)) + for i := 0; i < len(dnsLabel); i++ { + if dnsLabel[i] == '-' { + if i+1 < len(dnsLabel) && dnsLabel[i+1] == '-' { + // Handle '--' by appending a single '-' + result = append(result, '-') + i++ + } else { + // Handle single '-' by appending '.' + result = append(result, '.') + } + } else { + result = append(result, dnsLabel[i]) + } + } + return string(result) } // Converts a hostname/path to a subdomain-based URL, if applicable. @@ -419,7 +456,7 @@ func toSubdomainURL(hostname, path string, r *http.Request, inlineDNSLink bool, // 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 + dnsLinkFqdn := UninlineDNSLink(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 @@ -442,7 +479,7 @@ func toSubdomainURL(hostname, path string, r *http.Request, inlineDNSLink bool, // 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) + dnsLabel, err := InlineDNSLink(rootID) if err != nil { return "", err } diff --git a/gateway/hostname_test.go b/gateway/hostname_test.go index 7a8d7a170a..1a150facb3 100644 --- a/gateway/hostname_test.go +++ b/gateway/hostname_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "strings" "testing" "github.com/ipfs/boxo/path" @@ -82,10 +83,13 @@ func TestToDNSLinkDNSLabel(t *testing.T) { err error }{ {"dnslink.long-name.example.com", "dnslink-long--name-example-com", nil}, - {"dnslink.too-long.f1siqrebi3vir8sab33hu5vcy008djegvay6atmz91ojesyjs8lx350b7y7i1nvyw2haytfukfyu2f2x4tocdrfa0zgij6p4zpl4u5o.example.com", "", errors.New("DNSLink representation incompatible with DNS label length limit of 63: dnslink-too--long-f1siqrebi3vir8sab33hu5vcy008djegvay6atmz91ojesyjs8lx350b7y7i1nvyw2haytfukfyu2f2x4tocdrfa0zgij6p4zpl4u5o-example-com")}, + {"singlelabel", "singlelabel", nil}, + {"example.com", "example-com", nil}, + {"en.wikipedia-on-ipfs.org", "en-wikipedia--on--ipfs-org", nil}, + {"dnslink.too-long.f1siqrebi3vir8sab33hu5vcy008djegvay6atmz91ojesyjs8lx350b7y7i1nvyw2haytfukfyu2f2x4tocdrfa0zgij6p4zpl4u5o.example.com", "", errors.New(`inlined DNSLink incompatible with DNS label length limit of 63: "dnslink-too--long-f1siqrebi3vir8sab33hu5vcy008djegvay6atmz91ojesyjs8lx350b7y7i1nvyw2haytfukfyu2f2x4tocdrfa0zgij6p4zpl4u5o-example-com"`)}, } { t.Run(test.in, func(t *testing.T) { - out, err := toDNSLinkDNSLabel(test.in) + out, err := InlineDNSLink(test.in) require.Equal(t, test.out, out) require.Equal(t, test.err, err) }) @@ -99,11 +103,14 @@ func TestToDNSLinkFQDN(t *testing.T) { out string }{ {"singlelabel", "singlelabel"}, + {"no--tld", "no-tld"}, + {"example.com", "example.com"}, {"docs-ipfs-tech", "docs.ipfs.tech"}, + {"en-wikipedia--on--ipfs-org", "en.wikipedia-on-ipfs.org"}, {"dnslink-long--name-example-com", "dnslink.long-name.example.com"}, } { t.Run(test.in, func(t *testing.T) { - out := toDNSLinkFQDN(test.in) + out := UninlineDNSLink(test.in) require.Equal(t, test.out, out) }) } @@ -305,3 +312,55 @@ func TestKnownSubdomainDetails(t *testing.T) { }) } } + +const testInlinedDNSLinkA = "example-com" +const testInlinedDNSLinkB = "docs-ipfs-tech" +const testInlinedDNSLinkC = "en-wikipedia--on--ipfs-org" +const testDNSLinkA = "example.com" +const testDNSLinkB = "docs.ipfs.tech" +const testDNSLinkC = "en.wikipedia-on-ipfs.org" + +func inlineDNSLinkSimple(fqdn string) (dnsLabel string, err error) { + dnsLabel = strings.ReplaceAll(fqdn, "-", "--") + dnsLabel = strings.ReplaceAll(dnsLabel, ".", "-") + if len(dnsLabel) > dnsLabelMaxLength { + return "", fmt.Errorf("inlined DNSLink incompatible with DNS label length limit of 63: %q", dnsLabel) + } + return dnsLabel, nil +} +func uninlineDNSLinkSimple(dnsLabel string) (fqdn string) { + fqdn = strings.ReplaceAll(dnsLabel, "--", "@") // @ placeholder is unused in DNS labels + fqdn = strings.ReplaceAll(fqdn, "-", ".") + fqdn = strings.ReplaceAll(fqdn, "@", "-") + return fqdn +} + +func BenchmarkUninlineDNSLinkSimple(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = uninlineDNSLinkSimple(testInlinedDNSLinkA) + _ = uninlineDNSLinkSimple(testInlinedDNSLinkB) + _ = uninlineDNSLinkSimple(testInlinedDNSLinkC) + } +} +func BenchmarkUninlineDNSLink(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = UninlineDNSLink(testInlinedDNSLinkA) + _ = UninlineDNSLink(testInlinedDNSLinkB) + _ = UninlineDNSLink(testInlinedDNSLinkC) + } +} + +func BenchmarkInlineDNSLinkSimple(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = inlineDNSLinkSimple(testDNSLinkA) + _, _ = inlineDNSLinkSimple(testDNSLinkB) + _, _ = inlineDNSLinkSimple(testDNSLinkC) + } +} +func BenchmarkInlineDNSLink(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = InlineDNSLink(testDNSLinkA) + _, _ = InlineDNSLink(testDNSLinkB) + _, _ = InlineDNSLink(testDNSLinkC) + } +}