[bugfix] Use punycode for `host` part of `resource` query param when doing webfinger requests (#3133)

* [bugfix] use punycode when webfingering

* account for punycode when checking if final URI matches expected

* hmm

* fix test

7 files changed, 239 insertions, 13 deletions

diff --git a/internal/federation/dereferencing/account.go b/internal/federation/dereferencing/account.go
index e48507124..65436f9ea 100644
--- a/internal/federation/dereferencing/account.go
+++ b/internal/federation/dereferencing/account.go
@@ -706,13 +706,26 @@ func (d *Dereferencer) enrichAccount(
 		return nil, nil, gtserror.Newf("empty domain for %s", uri)
 	}
 
-	// Ensure the final parsed account URI / URL matches
+	// Ensure the final parsed account URI or URL matches
 	// the input URI we fetched (or received) it as.
-	if expect := uri.String(); latestAcc.URI != expect &&
-		latestAcc.URL != expect {
+	matches, err := util.URIMatches(
+		uri,
+		append(
+			ap.GetURL(apubAcc),      // account URL(s)
+			ap.GetJSONLDId(apubAcc), // account URI
+		)...,
+	)
+	if err != nil {
+		return nil, nil, gtserror.Newf(
+			"error checking dereferenced account uri %s: %w",
+			latestAcc.URI, err,
+		)
+	}
+
+	if !matches {
 		return nil, nil, gtserror.Newf(
 			"dereferenced account uri %s does not match %s",
-			latestAcc.URI, expect,
+			latestAcc.URI, uri.String(),
 		)
 	}
 
diff --git a/internal/federation/dereferencing/status.go b/internal/federation/dereferencing/status.go
index 88746fc3a..fa9906066 100644
--- a/internal/federation/dereferencing/status.go
+++ b/internal/federation/dereferencing/status.go
@@ -467,13 +467,26 @@ func (d *Dereferencer) enrichStatus(
 		)
 	}
 
-	// Ensure the final parsed status URI / URL matches
+	// Ensure the final parsed status URI or URL matches
 	// the input URI we fetched (or received) it as.
-	if expect := uri.String(); latestStatus.URI != expect &&
-		latestStatus.URL != expect {
+	matches, err := util.URIMatches(
+		uri,
+		append(
+			ap.GetURL(apubStatus),      // status URL(s)
+			ap.GetJSONLDId(apubStatus), // status URI
+		)...,
+	)
+	if err != nil {
+		return nil, nil, gtserror.Newf(
+			"error checking dereferenced status uri %s: %w",
+			latestStatus.URI, err,
+		)
+	}
+
+	if !matches {
 		return nil, nil, gtserror.Newf(
 			"dereferenced status uri %s does not match %s",
-			latestStatus.URI, expect,
+			latestStatus.URI, uri.String(),
 		)
 	}
 
diff --git a/internal/transport/finger.go b/internal/transport/finger.go
index f550769af..f82719245 100644
--- a/internal/transport/finger.go
+++ b/internal/transport/finger.go
@@ -28,6 +28,7 @@ import (
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
 	apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
+	"github.com/superseriousbusiness/gotosocial/internal/util"
 )
 
 // webfingerURLFor returns the URL to try a webfinger request against, as
@@ -73,9 +74,16 @@ func prepWebfingerReq(ctx context.Context, loc, domain, username string) (*http.
 }
 
 func (t *transport) Finger(ctx context.Context, targetUsername string, targetDomain string) ([]byte, error) {
+	// Remotes seem to prefer having their punycode
+	// domain used in webfinger requests, so let's oblige.
+	punyDomain, err := util.Punify(targetDomain)
+	if err != nil {
+		return nil, gtserror.Newf("error punifying %s: %w", targetDomain, err)
+	}
+
 	// Generate new GET request
-	url, cached := t.webfingerURLFor(targetDomain)
-	req, err := prepWebfingerReq(ctx, url, targetDomain, targetUsername)
+	url, cached := t.webfingerURLFor(punyDomain)
+	req, err := prepWebfingerReq(ctx, url, punyDomain, targetUsername)
 	if err != nil {
 		return nil, err
 	}
@@ -95,7 +103,7 @@ func (t *transport) Finger(ctx context.Context, targetUsername string, targetDom
 			// If we got a response we consider successful on a cached URL, i.e one set
 			// by us later on when a host-meta based webfinger request succeeded, set it
 			// again here to renew the TTL
-			t.controller.state.Caches.Webfinger.Set(targetDomain, url)
+			t.controller.state.Caches.Webfinger.Set(punyDomain, url)
 		}
 
 		if rsp.StatusCode == http.StatusGone {
@@ -128,7 +136,7 @@ func (t *transport) Finger(ctx context.Context, targetUsername string, targetDom
 	// So far we've failed to get a successful response from the expected
 	// webfinger endpoint. Lets try and discover the webfinger endpoint
 	// through /.well-known/host-meta
-	host, err := t.webfingerFromHostMeta(ctx, targetDomain)
+	host, err := t.webfingerFromHostMeta(ctx, punyDomain)
 	if err != nil {
 		return nil, fmt.Errorf("failed to discover webfinger URL fallback for: %s through host-meta: %w", targetDomain, err)
 	}
@@ -142,7 +150,7 @@ func (t *transport) Finger(ctx context.Context, targetUsername string, targetDom
 
 	// Now that we have a different URL for the webfinger
 	// endpoint, try the request against that endpoint instead
-	req, err = prepWebfingerReq(ctx, host, targetDomain, targetUsername)
+	req, err = prepWebfingerReq(ctx, host, punyDomain, targetUsername)
 	if err != nil {
 		return nil, err
 	}
diff --git a/internal/transport/finger_test.go b/internal/transport/finger_test.go
index db2369799..dd3449b73 100644
--- a/internal/transport/finger_test.go
+++ b/internal/transport/finger_test.go
@@ -42,6 +42,18 @@ func (suite *FingerTestSuite) TestFinger() {
 	suite.Equal(0, wc.Len(), "expect webfinger cache to be empty for normal webfinger request")
 }
 
+func (suite *FingerTestSuite) TestFingerPunycode() {
+	wc := suite.state.Caches.Webfinger
+	suite.Equal(0, wc.Len(), "expect webfinger cache to be empty")
+
+	_, err := suite.transport.Finger(context.TODO(), "brand_new_person", "pünycöde.example.org")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+
+	suite.Equal(0, wc.Len(), "expect webfinger cache to be empty for normal webfinger request")
+}
+
 func (suite *FingerTestSuite) TestFingerWithHostMeta() {
 	wc := suite.state.Caches.Webfinger
 	suite.Equal(0, wc.Len(), "expect webfinger cache to be empty")
diff --git a/internal/util/puny_test.go b/internal/util/puny_test.go
new file mode 100644
index 000000000..a0ffd30b4
--- /dev/null
+++ b/internal/util/puny_test.go
@@ -0,0 +1,101 @@
+// GoToSocial
+// Copyright (C) GoToSocial Authors admin@gotosocial.org
+// SPDX-License-Identifier: AGPL-3.0-or-later
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package util_test
+
+import (
+	"net/url"
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+	"github.com/superseriousbusiness/gotosocial/internal/util"
+	"github.com/superseriousbusiness/gotosocial/testrig"
+)
+
+type PunyTestSuite struct {
+	suite.Suite
+}
+
+func (suite *PunyTestSuite) TestMatches() {
+	for i, testCase := range []struct {
+		expect *url.URL
+		actual []*url.URL
+		match  bool
+	}{
+		{
+			expect: testrig.URLMustParse("https://%D5%A9%D5%B8%D6%82%D5%A9.%D5%B0%D5%A1%D5%B5/@ankap"),
+			actual: []*url.URL{
+				testrig.URLMustParse("https://xn--69aa8bzb.xn--y9a3aq/users/ankap"),
+				testrig.URLMustParse("https://xn--69aa8bzb.xn--y9a3aq/@ankap"),
+			},
+			match: true,
+		},
+		{
+			expect: testrig.URLMustParse("https://xn--69aa8bzb.xn--y9a3aq/@ankap"),
+			actual: []*url.URL{
+				testrig.URLMustParse("https://xn--69aa8bzb.xn--y9a3aq/users/ankap"),
+				testrig.URLMustParse("https://xn--69aa8bzb.xn--y9a3aq/@ankap"),
+			},
+			match: true,
+		},
+		{
+			expect: testrig.URLMustParse("https://թութ.հայ/@ankap"),
+			actual: []*url.URL{
+				testrig.URLMustParse("https://xn--69aa8bzb.xn--y9a3aq/users/ankap"),
+				testrig.URLMustParse("https://xn--69aa8bzb.xn--y9a3aq/@ankap"),
+			},
+			match: true,
+		},
+		{
+			expect: testrig.URLMustParse("https://թութ.հայ/@ankap"),
+			actual: []*url.URL{
+				testrig.URLMustParse("https://example.org/users/ankap"),
+				testrig.URLMustParse("https://%D5%A9%D5%B8%D6%82%D5%A9.%D5%B0%D5%A1%D5%B5/@ankap"),
+			},
+			match: true,
+		},
+		{
+			expect: testrig.URLMustParse("https://example.org/@ankap"),
+			actual: []*url.URL{
+				testrig.URLMustParse("https://xn--69aa8bzb.xn--y9a3aq/users/ankap"),
+				testrig.URLMustParse("https://xn--69aa8bzb.xn--y9a3aq/@ankap"),
+			},
+			match: false,
+		},
+	} {
+		matches, err := util.URIMatches(
+			testCase.expect,
+			testCase.actual...,
+		)
+		if err != nil {
+			suite.FailNow(err.Error())
+		}
+
+		if matches != testCase.match {
+			suite.Failf(
+				"case "+strconv.Itoa(i)+" matches not equal expected",
+				"wanted %t, got %t",
+				testCase.match, matches,
+			)
+		}
+	}
+}
+
+func TestPunyTestSuite(t *testing.T) {
+	suite.Run(t, new(PunyTestSuite))
+}
diff --git a/internal/util/punycode.go b/internal/util/punycode.go
index 4a595a281..cc1b57a27 100644
--- a/internal/util/punycode.go
+++ b/internal/util/punycode.go
@@ -18,6 +18,7 @@
 package util
 
 import (
+	"net/url"
 	"strings"
 
 	"golang.org/x/net/idna"
@@ -42,3 +43,70 @@ func DePunify(domain string) (string, error) {
 	out, err := idna.ToUnicode(domain)
 	return strings.ToLower(out), err
 }
+
+// URIMatches returns true if the expected URI matches
+// any of the given URIs, taking account of punycode.
+func URIMatches(expect *url.URL, uris ...*url.URL) (bool, error) {
+	// Normalize expect to punycode.
+	expectPuny, err := PunifyURI(expect)
+	if err != nil {
+		return false, err
+	}
+	expectStr := expectPuny.String()
+
+	for _, uri := range uris {
+		uriPuny, err := PunifyURI(uri)
+		if err != nil {
+			return false, err
+		}
+
+		if uriPuny.String() == expectStr {
+			// Looks good.
+			return true, nil
+		}
+	}
+
+	// Didn't match.
+	return false, nil
+}
+
+// PunifyURI returns a copy of the given URI
+// with the 'host' part converted to punycode.
+func PunifyURI(in *url.URL) (*url.URL, error) {
+	// Take a copy of in.
+	out := new(url.URL)
+	*out = *in
+
+	// Normalize host to punycode.
+	var err error
+	out.Host, err = Punify(in.Host)
+	return out, err
+}
+
+// PunifyURIStr returns a copy of the given URI
+// string with the 'host' part converted to punycode.
+func PunifyURIStr(in string) (string, error) {
+	inURI, err := url.Parse(in)
+	if err != nil {
+		return "", err
+	}
+
+	outURIPuny, err := Punify(inURI.Host)
+	if err != nil {
+		return "", err
+	}
+
+	if outURIPuny == in {
+		// Punify did nothing, so in was
+		// already punified, return as-is.
+		return in, nil
+	}
+
+	// Take a copy of in.
+	outURI := new(url.URL)
+	*outURI = *inURI
+
+	// Normalize host to punycode.
+	outURI.Host = outURIPuny
+	return outURI.String(), err
+}
diff --git a/testrig/transportcontroller.go b/testrig/transportcontroller.go
index a0ffa0ab7..385c620db 100644
--- a/testrig/transportcontroller.go
+++ b/testrig/transportcontroller.go
@@ -334,6 +334,17 @@ func WebfingerResponse(req *http.Request) (responseCode int, responseBytes []byt
 				},
 			},
 		}
+	case "https://xn--pnycde-zxa8b.example.org/.well-known/webfinger?resource=acct%3Abrand_new_person%40xn--pnycde-zxa8b.example.org":
+		wfr = &apimodel.WellKnownResponse{
+			Subject: "acct:brand_new_person@unknown-instance.com",
+			Links: []apimodel.Link{
+				{
+					Rel:  "self",
+					Type: applicationActivityJSON,
+					Href: "https://unknown-instance.com/users/brand_new_person",
+				},
+			},
+		}
 	case "https://turnip.farm/.well-known/webfinger?resource=acct%3Aturniplover6969%40turnip.farm":
 		wfr = &apimodel.WellKnownResponse{
 			Subject: "acct:turniplover6969@turnip.farm",