From ecfea10e359b9c9e7c0e6b5fd092e3caa5587df6 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 26 Jul 2024 13:11:07 +0200 Subject: [PATCH] [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 --- internal/federation/dereferencing/account.go | 21 +++- internal/federation/dereferencing/status.go | 21 +++- internal/transport/finger.go | 18 +++- internal/transport/finger_test.go | 12 +++ internal/util/puny_test.go | 101 +++++++++++++++++++ internal/util/punycode.go | 68 +++++++++++++ testrig/transportcontroller.go | 11 ++ 7 files changed, 239 insertions(+), 13 deletions(-) create mode 100644 internal/util/puny_test.go 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 . + +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",