From e77c7e16b6700cdaddef3a0d8b16579173505436 Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Tue, 14 Jan 2025 14:23:18 +0000 Subject: [PATCH] [chore] better dns validation (#3644) * add seperate PunifyValidate() function for properly validating domain names when converting to punycode * rename function, strip port from domain validation --- .../api/client/instance/instancepatch_test.go | 2 + internal/db/bundb/account.go | 3 +- internal/db/bundb/domain.go | 56 +++---- internal/db/bundb/domainpermissiondraft.go | 15 +- internal/db/bundb/domainpermissionexclude.go | 14 +- internal/db/bundb/instance.go | 17 ++- internal/subscriptions/domainperms.go | 34 +---- internal/util/domain.go | 138 ++++++++++++++++++ .../util/{puny_test.go => domain_test.go} | 0 internal/util/punycode.go | 97 ------------ 10 files changed, 203 insertions(+), 173 deletions(-) create mode 100644 internal/util/domain.go rename internal/util/{puny_test.go => domain_test.go} (100%) delete mode 100644 internal/util/punycode.go diff --git a/internal/api/client/instance/instancepatch_test.go b/internal/api/client/instance/instancepatch_test.go index 0ba0f9881..47126e8b5 100644 --- a/internal/api/client/instance/instancepatch_test.go +++ b/internal/api/client/instance/instancepatch_test.go @@ -28,6 +28,7 @@ "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/api/client/instance" + "github.com/superseriousbusiness/gotosocial/internal/middleware" "github.com/superseriousbusiness/gotosocial/internal/oauth" "github.com/superseriousbusiness/gotosocial/testrig" ) @@ -51,6 +52,7 @@ func (suite *InstancePatchTestSuite) instancePatch(fieldName string, fileName st ctx := suite.newContext(recorder, http.MethodPatch, instance.InstanceInformationPathV1, requestBody.Bytes(), w.FormDataContentType(), true) suite.instanceModule.InstanceUpdatePATCHHandler(ctx) + middleware.Logger(false)(ctx) result := recorder.Result() defer result.Body.Close() diff --git a/internal/db/bundb/account.go b/internal/db/bundb/account.go index f054b1412..3d85df381 100644 --- a/internal/db/bundb/account.go +++ b/internal/db/bundb/account.go @@ -137,8 +137,9 @@ func(account *gtsmodel.Account) error { func (a *accountDB) GetAccountByUsernameDomain(ctx context.Context, username string, domain string) (*gtsmodel.Account, error) { if domain != "" { - // Normalize the domain as punycode var err error + + // Normalize the domain as punycode domain, err = util.Punify(domain) if err != nil { return nil, err diff --git a/internal/db/bundb/domain.go b/internal/db/bundb/domain.go index fd75fec4c..925387bd9 100644 --- a/internal/db/bundb/domain.go +++ b/internal/db/bundb/domain.go @@ -36,12 +36,12 @@ type domainDB struct { state *state.State } -func (d *domainDB) CreateDomainAllow(ctx context.Context, allow *gtsmodel.DomainAllow) error { - // Normalize the domain as punycode - var err error - allow.Domain, err = util.Punify(allow.Domain) +func (d *domainDB) CreateDomainAllow(ctx context.Context, allow *gtsmodel.DomainAllow) (err error) { + // Normalize the domain as punycode, note the extra + // validation step for domain name write operations. + allow.Domain, err = util.PunifySafely(allow.Domain) if err != nil { - return err + return gtserror.Newf("error punifying domain %s: %w", allow.Domain, err) } // Attempt to store domain allow in DB @@ -58,10 +58,10 @@ func (d *domainDB) CreateDomainAllow(ctx context.Context, allow *gtsmodel.Domain } func (d *domainDB) GetDomainAllow(ctx context.Context, domain string) (*gtsmodel.DomainAllow, error) { - // Normalize the domain as punycode + // Normalize domain as punycode for lookup. domain, err := util.Punify(domain) if err != nil { - return nil, err + return nil, gtserror.Newf("error punifying domain %s: %w", domain, err) } // Check for easy case, domain referencing *us* @@ -111,12 +111,12 @@ func (d *domainDB) GetDomainAllowByID(ctx context.Context, id string) (*gtsmodel return &allow, nil } -func (d *domainDB) UpdateDomainAllow(ctx context.Context, allow *gtsmodel.DomainAllow, columns ...string) error { - // Normalize the domain as punycode - var err error - allow.Domain, err = util.Punify(allow.Domain) +func (d *domainDB) UpdateDomainAllow(ctx context.Context, allow *gtsmodel.DomainAllow, columns ...string) (err error) { + // Normalize the domain as punycode, note the extra + // validation step for domain name write operations. + allow.Domain, err = util.PunifySafely(allow.Domain) if err != nil { - return err + return gtserror.Newf("error punifying domain %s: %w", allow.Domain, err) } // Ensure updated_at is set. @@ -142,10 +142,10 @@ func (d *domainDB) UpdateDomainAllow(ctx context.Context, allow *gtsmodel.Domain } func (d *domainDB) DeleteDomainAllow(ctx context.Context, domain string) error { - // Normalize the domain as punycode + // Normalize domain as punycode for lookup. domain, err := util.Punify(domain) if err != nil { - return err + return gtserror.Newf("error punifying domain %s: %w", domain, err) } // Attempt to delete domain allow @@ -163,11 +163,13 @@ func (d *domainDB) DeleteDomainAllow(ctx context.Context, domain string) error { } func (d *domainDB) CreateDomainBlock(ctx context.Context, block *gtsmodel.DomainBlock) error { - // Normalize the domain as punycode var err error - block.Domain, err = util.Punify(block.Domain) + + // Normalize the domain as punycode, note the extra + // validation step for domain name write operations. + block.Domain, err = util.PunifySafely(block.Domain) if err != nil { - return err + return gtserror.Newf("error punifying domain %s: %w", block.Domain, err) } // Attempt to store domain block in DB @@ -184,10 +186,10 @@ func (d *domainDB) CreateDomainBlock(ctx context.Context, block *gtsmodel.Domain } func (d *domainDB) GetDomainBlock(ctx context.Context, domain string) (*gtsmodel.DomainBlock, error) { - // Normalize the domain as punycode + // Normalize domain as punycode for lookup. domain, err := util.Punify(domain) if err != nil { - return nil, err + return nil, gtserror.Newf("error punifying domain %s: %w", domain, err) } // Check for easy case, domain referencing *us* @@ -238,11 +240,13 @@ func (d *domainDB) GetDomainBlockByID(ctx context.Context, id string) (*gtsmodel } func (d *domainDB) UpdateDomainBlock(ctx context.Context, block *gtsmodel.DomainBlock, columns ...string) error { - // Normalize the domain as punycode var err error - block.Domain, err = util.Punify(block.Domain) + + // Normalize the domain as punycode, note the extra + // validation step for domain name write operations. + block.Domain, err = util.PunifySafely(block.Domain) if err != nil { - return err + return gtserror.Newf("error punifying domain %s: %w", block.Domain, err) } // Ensure updated_at is set. @@ -268,10 +272,10 @@ func (d *domainDB) UpdateDomainBlock(ctx context.Context, block *gtsmodel.Domain } func (d *domainDB) DeleteDomainBlock(ctx context.Context, domain string) error { - // Normalize the domain as punycode + // Normalize domain as punycode for lookup. domain, err := util.Punify(domain) if err != nil { - return err + return gtserror.Newf("error punifying domain %s: %w", domain, err) } // Attempt to delete domain block @@ -289,10 +293,10 @@ func (d *domainDB) DeleteDomainBlock(ctx context.Context, domain string) error { } func (d *domainDB) IsDomainBlocked(ctx context.Context, domain string) (bool, error) { - // Normalize the domain as punycode + // Normalize domain as punycode for lookup. domain, err := util.Punify(domain) if err != nil { - return false, err + return false, gtserror.Newf("error punifying domain %s: %w", domain, err) } // Domain referencing *us* cannot be blocked. diff --git a/internal/db/bundb/domainpermissiondraft.go b/internal/db/bundb/domainpermissiondraft.go index dec41e9d7..281db1224 100644 --- a/internal/db/bundb/domainpermissiondraft.go +++ b/internal/db/bundb/domainpermissiondraft.go @@ -168,7 +168,7 @@ func (d *domainDB) GetDomainPermissionDrafts( if domain != "" { var err error - // Normalize domain as punycode. + // Normalize domain as punycode for lookup. domain, err = util.Punify(domain) if err != nil { return nil, gtserror.Newf("error punifying domain %s: %w", domain, err) @@ -234,22 +234,23 @@ func (d *domainDB) GetDomainPermissionDrafts( func (d *domainDB) PutDomainPermissionDraft( ctx context.Context, - permDraft *gtsmodel.DomainPermissionDraft, + draft *gtsmodel.DomainPermissionDraft, ) error { var err error - // Normalize the domain as punycode - permDraft.Domain, err = util.Punify(permDraft.Domain) + // Normalize the domain as punycode, note the extra + // validation step for domain name write operations. + draft.Domain, err = util.PunifySafely(draft.Domain) if err != nil { - return gtserror.Newf("error punifying domain %s: %w", permDraft.Domain, err) + return gtserror.Newf("error punifying domain %s: %w", draft.Domain, err) } return d.state.Caches.DB.DomainPermissionDraft.Store( - permDraft, + draft, func() error { _, err := d.db. NewInsert(). - Model(permDraft). + Model(draft). Exec(ctx) return err }, diff --git a/internal/db/bundb/domainpermissionexclude.go b/internal/db/bundb/domainpermissionexclude.go index 005c21400..1527a584b 100644 --- a/internal/db/bundb/domainpermissionexclude.go +++ b/internal/db/bundb/domainpermissionexclude.go @@ -37,11 +37,13 @@ func (d *domainDB) PutDomainPermissionExclude( ctx context.Context, exclude *gtsmodel.DomainPermissionExclude, ) error { - // Normalize the domain as punycode var err error - exclude.Domain, err = util.Punify(exclude.Domain) + + // Normalize the domain as punycode, note the extra + // validation step for domain name write operations. + exclude.Domain, err = util.PunifySafely(exclude.Domain) if err != nil { - return err + return gtserror.Newf("error punifying domain %s: %w", exclude.Domain, err) } // Attempt to store domain perm exclude in DB @@ -58,10 +60,10 @@ func (d *domainDB) PutDomainPermissionExclude( } func (d *domainDB) IsDomainPermissionExcluded(ctx context.Context, domain string) (bool, error) { - // Normalize the domain as punycode + // Normalize domain as punycode for lookup. domain, err := util.Punify(domain) if err != nil { - return false, err + return false, gtserror.Newf("error punifying domain %s: %w", domain, err) } // Func to scan list of all @@ -177,7 +179,7 @@ func (d *domainDB) GetDomainPermissionExcludes( if domain != "" { var err error - // Normalize domain as punycode. + // Normalize domain as punycode for lookup. domain, err = util.Punify(domain) if err != nil { return nil, gtserror.Newf("error punifying domain %s: %w", domain, err) diff --git a/internal/db/bundb/instance.go b/internal/db/bundb/instance.go index 613a2b13a..7852ae52e 100644 --- a/internal/db/bundb/instance.go +++ b/internal/db/bundb/instance.go @@ -158,8 +158,9 @@ func (i *instanceDB) CountInstanceDomains(ctx context.Context, domain string) (i } func (i *instanceDB) GetInstance(ctx context.Context, domain string) (*gtsmodel.Instance, error) { - // Normalize the domain as punycode var err error + + // Normalize the domain as punycode domain, err = util.Punify(domain) if err != nil { return nil, gtserror.Newf("error punifying domain %s: %w", domain, err) @@ -265,8 +266,9 @@ func (i *instanceDB) PopulateInstance(ctx context.Context, instance *gtsmodel.In func (i *instanceDB) PutInstance(ctx context.Context, instance *gtsmodel.Instance) error { var err error - // Normalize the domain as punycode - instance.Domain, err = util.Punify(instance.Domain) + // Normalize the domain as punycode, note the extra + // validation step for domain name write operations. + instance.Domain, err = util.PunifySafely(instance.Domain) if err != nil { return gtserror.Newf("error punifying domain %s: %w", instance.Domain, err) } @@ -279,9 +281,11 @@ func (i *instanceDB) PutInstance(ctx context.Context, instance *gtsmodel.Instanc } func (i *instanceDB) UpdateInstance(ctx context.Context, instance *gtsmodel.Instance, columns ...string) error { - // Normalize the domain as punycode var err error - instance.Domain, err = util.Punify(instance.Domain) + + // Normalize the domain as punycode, note the extra + // validation step for domain name write operations. + instance.Domain, err = util.PunifySafely(instance.Domain) if err != nil { return gtserror.Newf("error punifying domain %s: %w", instance.Domain, err) } @@ -349,8 +353,9 @@ func (i *instanceDB) GetInstanceAccounts(ctx context.Context, domain string, max limit = 0 } - // Normalize the domain as punycode. var err error + + // Normalize the domain as punycode domain, err = util.Punify(domain) if err != nil { return nil, gtserror.Newf("error punifying domain %s: %w", domain, err) diff --git a/internal/subscriptions/domainperms.go b/internal/subscriptions/domainperms.go index 2ecf259a4..dc4224899 100644 --- a/internal/subscriptions/domainperms.go +++ b/internal/subscriptions/domainperms.go @@ -23,7 +23,6 @@ "encoding/csv" "encoding/json" "errors" - "fmt" "io" "slices" "strconv" @@ -32,7 +31,6 @@ "codeberg.org/gruf/go-kv" - "github.com/miekg/dns" "github.com/superseriousbusiness/gotosocial/internal/admin" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/internal/config" @@ -629,7 +627,7 @@ func permsFromCSV( // Normalize + validate domain. domainRaw := record[*domainI] - domain, err := validateDomain(domainRaw) + domain, err := util.PunifySafely(domainRaw) if err != nil { l.Warnf("skipping invalid domain %s: %+v", domainRaw, err) continue @@ -702,7 +700,7 @@ func permsFromJSON( // Normalize + validate domain. domainRaw := apiPerm.Domain.Domain - domain, err := validateDomain(domainRaw) + domain, err := util.PunifySafely(domainRaw) if err != nil { l.Warnf("skipping invalid domain %s: %+v", domainRaw, err) continue @@ -757,8 +755,8 @@ func permsFromPlain( perms := make([]gtsmodel.DomainPermission, 0, len(domains)) for _, domainRaw := range domains { - // Normalize + validate domain. - domain, err := validateDomain(domainRaw) + // Normalize + validate domain as ASCII. + domain, err := util.PunifySafely(domainRaw) if err != nil { l.Warnf("skipping invalid domain %s: %+v", domainRaw, err) continue @@ -781,30 +779,6 @@ func permsFromPlain( return perms, nil } -func validateDomain(domain string) (string, error) { - // Basic validation. - if _, ok := dns.IsDomainName(domain); !ok { - err := fmt.Errorf("invalid domain name") - return "", err - } - - // Convert to punycode. - domain, err := util.Punify(domain) - if err != nil { - err := fmt.Errorf("could not punify domain: %w", err) - return "", err - } - - // Check for invalid characters - // after the punification process. - if strings.ContainsAny(domain, "*, \n") { - err := fmt.Errorf("invalid char(s) in domain") - return "", err - } - - return domain, nil -} - func (s *Subscriptions) existingCovered( ctx context.Context, permType gtsmodel.DomainPermissionType, diff --git a/internal/util/domain.go b/internal/util/domain.go new file mode 100644 index 000000000..c64cc65d7 --- /dev/null +++ b/internal/util/domain.go @@ -0,0 +1,138 @@ +// 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 + +import ( + "net/url" + "strings" + + "golang.org/x/net/idna" +) + +var ( + // IDNA (Internationalized Domain Names for Applications) + // profiles for fast punycode conv and full verification. + punifyProfile = *idna.Punycode + verifyProfile = *idna.Lookup +) + +// PunifySafely validates the provided domain name, +// and converts unicode chars to ASCII, i.e. punified form. +func PunifySafely(domain string) (string, error) { + if i := strings.LastIndexByte(domain, ':'); i >= 0 { + + // If there is a port included in domain, we + // strip it as colon is invalid in a hostname. + domain, port := domain[:i], domain[i:] + domain, err := verifyProfile.ToASCII(domain) + if err != nil { + return "", err + } + + // Then rebuild with port after. + domain = strings.ToLower(domain) + return domain + port, nil + } else { //nolint:revive + + // Otherwise we just punify domain as-is. + domain, err := verifyProfile.ToASCII(domain) + return strings.ToLower(domain), err + } +} + +// Punify is a faster form of PunifySafely() without validation. +func Punify(domain string) (string, error) { + domain, err := punifyProfile.ToASCII(domain) + return strings.ToLower(domain), err +} + +// DePunify converts any punycode-encoded unicode characters +// in domain name back to their origin unicode. Please note +// that this performs minimal validation of domain name. +func DePunify(domain string) (string, error) { + domain = strings.ToLower(domain) + return punifyProfile.ToUnicode(domain) +} + +// 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) (ok bool, err error) { + + // Create new URL to hold + // punified URI information. + punyURI := new(url.URL) + *punyURI = *expect + + // Set punified expected URL host. + punyURI.Host, err = Punify(expect.Host) + if err != nil { + return false, err + } + + // Calculate expected URI string. + expectStr := punyURI.String() + + // Use punyURI to iteratively + // store each punified URI info + // and generate punified URI + // strings to check against. + for _, uri := range uris { + *punyURI = *uri + punyURI.Host, err = Punify(uri.Host) + if err != nil { + return false, err + } + + // Check for a match against expect. + if expectStr == punyURI.String() { + return true, nil + } + } + + // Didn't match. + return false, nil +} + +// PunifyURI returns a new copy of URI with the 'host' +// part converted to punycode with PunifySafely(). +// For simple comparisons prefer the faster URIMatches(). +func PunifyURI(in *url.URL) (*url.URL, error) { + punyHost, err := PunifySafely(in.Host) + if err != nil { + return nil, err + } + out := new(url.URL) + *out = *in + out.Host = punyHost + return out, nil +} + +// PunifyURIToStr returns given URI serialized with the +// 'host' part converted to punycode with PunifySafely(). +// For simple comparisons prefer the faster URIMatches(). +func PunifyURIToStr(in *url.URL) (string, error) { + punyHost, err := PunifySafely(in.Host) + if err != nil { + return "", err + } + oldHost := in.Host + in.Host = punyHost + str := in.String() + in.Host = oldHost + return str, nil +} diff --git a/internal/util/puny_test.go b/internal/util/domain_test.go similarity index 100% rename from internal/util/puny_test.go rename to internal/util/domain_test.go diff --git a/internal/util/punycode.go b/internal/util/punycode.go deleted file mode 100644 index 3e9f71408..000000000 --- a/internal/util/punycode.go +++ /dev/null @@ -1,97 +0,0 @@ -// 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 - -import ( - "net/url" - "strings" - - "golang.org/x/net/idna" -) - -// Punify converts the given domain to lowercase -// then to punycode (for international domain names). -// -// Returns the resulting domain or an error if the -// punycode conversion fails. -func Punify(domain string) (string, error) { - domain = strings.ToLower(domain) - return idna.ToASCII(domain) -} - -// DePunify converts the given punycode string -// to its original unicode representation (lowercased). -// Noop if the domain is (already) not puny. -// -// Returns an error if conversion fails. -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. - expectStr, err := PunifyURIToStr(expect) - if err != nil { - return false, err - } - - for _, uri := range uris { - uriStr, err := PunifyURIToStr(uri) - if err != nil { - return false, err - } - - if uriStr == 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) { - punyHost, err := Punify(in.Host) - if err != nil { - return nil, err - } - out := new(url.URL) - *out = *in - out.Host = punyHost - return out, nil -} - -// PunifyURIToStr returns given URI serialized -// with the 'host' part converted to punycode. -func PunifyURIToStr(in *url.URL) (string, error) { - punyHost, err := Punify(in.Host) - if err != nil { - return "", err - } - oldHost := in.Host - in.Host = punyHost - str := in.String() - in.Host = oldHost - return str, nil -}