diff --git a/client/internal/dns/handler_chain_test.go b/client/internal/dns/handler_chain_test.go index 72c0004d5..fa9525069 100644 --- a/client/internal/dns/handler_chain_test.go +++ b/client/internal/dns/handler_chain_test.go @@ -112,6 +112,54 @@ func TestHandlerChain_ServeDNS_DomainMatching(t *testing.T) { matchSubdomains: false, shouldMatch: false, }, + { + name: "single letter TLD exact match", + handlerDomain: "example.x.", + queryDomain: "example.x.", + isWildcard: false, + matchSubdomains: false, + shouldMatch: true, + }, + { + name: "single letter TLD subdomain match", + handlerDomain: "example.x.", + queryDomain: "sub.example.x.", + isWildcard: false, + matchSubdomains: true, + shouldMatch: true, + }, + { + name: "single letter TLD wildcard match", + handlerDomain: "*.example.x.", + queryDomain: "sub.example.x.", + isWildcard: true, + matchSubdomains: false, + shouldMatch: true, + }, + { + name: "two letter domain labels", + handlerDomain: "a.b.", + queryDomain: "a.b.", + isWildcard: false, + matchSubdomains: false, + shouldMatch: true, + }, + { + name: "single character domain", + handlerDomain: "x.", + queryDomain: "x.", + isWildcard: false, + matchSubdomains: false, + shouldMatch: true, + }, + { + name: "single character domain with subdomain match", + handlerDomain: "x.", + queryDomain: "sub.x.", + isWildcard: false, + matchSubdomains: true, + shouldMatch: true, + }, } for _, tt := range tests { diff --git a/management/cmd/management.go b/management/cmd/management.go index 7da04074b..511168823 100644 --- a/management/cmd/management.go +++ b/management/cmd/management.go @@ -16,13 +16,13 @@ import ( "strings" "syscall" - "github.com/miekg/dns" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/netbirdio/netbird/formatter/hook" "github.com/netbirdio/netbird/management/internals/server" nbconfig "github.com/netbirdio/netbird/management/internals/server/config" + nbdomain "github.com/netbirdio/netbird/shared/management/domain" "github.com/netbirdio/netbird/util" "github.com/netbirdio/netbird/util/crypt" ) @@ -78,9 +78,8 @@ var ( } } - _, valid := dns.IsDomainName(dnsDomain) - if !valid || len(dnsDomain) > 192 { - return fmt.Errorf("failed parsing the provided dns-domain. Valid status: %t, Length: %d", valid, len(dnsDomain)) + if !nbdomain.IsValidDomainNoWildcard(dnsDomain) { + return fmt.Errorf("invalid dns-domain: %s", dnsDomain) } return nil diff --git a/management/internals/modules/zones/records/record.go b/management/internals/modules/zones/records/record.go index e44de08f4..1488febb9 100644 --- a/management/internals/modules/zones/records/record.go +++ b/management/internals/modules/zones/records/record.go @@ -6,7 +6,7 @@ import ( "github.com/rs/xid" - "github.com/netbirdio/netbird/management/server/util" + "github.com/netbirdio/netbird/shared/management/domain" "github.com/netbirdio/netbird/shared/management/http/api" ) @@ -63,7 +63,7 @@ func (r *Record) Validate() error { return errors.New("record name is required") } - if !util.IsValidDomain(r.Name) { + if !domain.IsValidDomain(r.Name) { return errors.New("invalid record name format") } @@ -81,8 +81,8 @@ func (r *Record) Validate() error { return err } case RecordTypeCNAME: - if !util.IsValidDomain(r.Content) { - return errors.New("invalid CNAME record format") + if !domain.IsValidDomainNoWildcard(r.Content) { + return errors.New("invalid CNAME target format") } default: return errors.New("invalid record type, must be A, AAAA, or CNAME") diff --git a/management/internals/modules/zones/zone.go b/management/internals/modules/zones/zone.go index 27adac1ac..f5ebed26c 100644 --- a/management/internals/modules/zones/zone.go +++ b/management/internals/modules/zones/zone.go @@ -6,7 +6,7 @@ import ( "github.com/rs/xid" "github.com/netbirdio/netbird/management/internals/modules/zones/records" - "github.com/netbirdio/netbird/management/server/util" + "github.com/netbirdio/netbird/shared/management/domain" "github.com/netbirdio/netbird/shared/management/http/api" ) @@ -73,7 +73,7 @@ func (z *Zone) Validate() error { return errors.New("zone name exceeds maximum length of 255 characters") } - if !util.IsValidDomain(z.Domain) { + if !domain.IsValidDomainNoWildcard(z.Domain) { return errors.New("invalid zone domain format") } diff --git a/management/server/account.go b/management/server/account.go index d453b87c3..ba5f0cffa 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -26,6 +26,7 @@ import ( "golang.org/x/exp/maps" nbdns "github.com/netbirdio/netbird/dns" + nbdomain "github.com/netbirdio/netbird/shared/management/domain" "github.com/netbirdio/netbird/formatter/hook" "github.com/netbirdio/netbird/management/internals/controllers/network_map" nbconfig "github.com/netbirdio/netbird/management/internals/server/config" @@ -231,7 +232,7 @@ func BuildManager( // enable single account mode only if configured by user and number of existing accounts is not grater than 1 am.singleAccountMode = singleAccountModeDomain != "" && accountsCounter <= 1 if am.singleAccountMode { - if !isDomainValid(singleAccountModeDomain) { + if !nbdomain.IsValidDomainNoWildcard(singleAccountModeDomain) { return nil, status.Errorf(status.InvalidArgument, "invalid domain \"%s\" provided for a single account mode. Please review your input for --single-account-mode-domain", singleAccountModeDomain) } am.singleAccountModeDomain = singleAccountModeDomain @@ -402,7 +403,7 @@ func (am *DefaultAccountManager) validateSettingsUpdate(ctx context.Context, tra return status.Errorf(status.InvalidArgument, "peer login expiration can't be smaller than one hour") } - if newSettings.DNSDomain != "" && !isDomainValid(newSettings.DNSDomain) { + if newSettings.DNSDomain != "" && !nbdomain.IsValidDomainNoWildcard(newSettings.DNSDomain) { return status.Errorf(status.InvalidArgument, "invalid domain \"%s\" provided for DNS domain", newSettings.DNSDomain) } @@ -1691,10 +1692,12 @@ func (am *DefaultAccountManager) SyncPeerMeta(ctx context.Context, peerPubKey st return nil } -var invalidDomainRegexp = regexp.MustCompile(`^([a-z0-9]+(-[a-z0-9]+)*\.)+[a-z]{2,}$`) +// isDomainValid validates public/IDP domains using stricter rules than internal DNS domains. +// Requires at least 2-char alphabetic TLD and no single-label domains. +var publicDomainRegexp = regexp.MustCompile(`^([a-z0-9]+(-[a-z0-9]+)*\.)+[a-z]{2,}$`) func isDomainValid(domain string) bool { - return invalidDomainRegexp.MatchString(domain) + return publicDomainRegexp.MatchString(domain) } func (am *DefaultAccountManager) onPeersInvalidated(ctx context.Context, accountID string, peerIDs []string) { diff --git a/management/server/nameserver.go b/management/server/nameserver.go index a3eb4ae2e..3d8c78912 100644 --- a/management/server/nameserver.go +++ b/management/server/nameserver.go @@ -3,10 +3,10 @@ package server import ( "context" "errors" - "regexp" + "fmt" + "strings" "unicode/utf8" - "github.com/miekg/dns" "github.com/rs/xid" nbdns "github.com/netbirdio/netbird/dns" @@ -15,11 +15,10 @@ import ( "github.com/netbirdio/netbird/management/server/permissions/operations" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" + nbdomain "github.com/netbirdio/netbird/shared/management/domain" "github.com/netbirdio/netbird/shared/management/status" ) -const domainPattern = `^(?i)[a-z0-9]+([\-\.]{1}[a-z0-9]+)*[*.a-z]{1,}$` - var errInvalidDomainName = errors.New("invalid domain name") // GetNameServerGroup gets a nameserver group object from account and nameserver group IDs @@ -305,16 +304,18 @@ func validateGroups(list []string, groups map[string]*types.Group) error { return nil } -var domainMatcher = regexp.MustCompile(domainPattern) - -func validateDomain(domain string) error { - if !domainMatcher.MatchString(domain) { - return errors.New("domain should consists of only letters, numbers, and hyphens with no leading, trailing hyphens, or spaces") +// validateDomain validates a nameserver match domain. +// Converts unicode to punycode. Wildcards are not allowed for nameservers. +func validateDomain(d string) error { + if strings.HasPrefix(d, "*.") { + return errors.New("wildcards not allowed") } - _, valid := dns.IsDomainName(domain) - if !valid { - return errInvalidDomainName + // Nameservers allow trailing dot (FQDN format) + toValidate := strings.TrimSuffix(d, ".") + + if _, err := nbdomain.ValidateDomains([]string{toValidate}); err != nil { + return fmt.Errorf("%w: %w", errInvalidDomainName, err) } return nil diff --git a/management/server/nameserver_test.go b/management/server/nameserver_test.go index 0d781e0d4..90b4b9687 100644 --- a/management/server/nameserver_test.go +++ b/management/server/nameserver_test.go @@ -901,82 +901,53 @@ func initTestNSAccount(t *testing.T, am *DefaultAccountManager) (*types.Account, return account, nil } +// TestValidateDomain tests nameserver-specific domain validation. +// Core domain validation is tested in shared/management/domain/validate_test.go. +// This test only covers nameserver-specific behavior: wildcard rejection and unicode support. func TestValidateDomain(t *testing.T) { testCases := []struct { name string domain string errFunc require.ErrorAssertionFunc }{ + // Nameserver-specific: wildcards not allowed { - name: "Valid domain name with multiple labels", - domain: "123.example.com", + name: "Wildcard prefix rejected", + domain: "*.example.com", + errFunc: require.Error, + }, + { + name: "Wildcard in middle rejected", + domain: "a.*.example.com", + errFunc: require.Error, + }, + // Nameserver-specific: unicode converted to punycode + { + name: "Unicode domain converted to punycode", + domain: "münchen.de", errFunc: require.NoError, }, { - name: "Valid domain name with hyphen", - domain: "test-example.com", + name: "Unicode domain all labels", + domain: "中国.中国", + errFunc: require.NoError, + }, + // Basic validation still works (delegates to shared validation) + { + name: "Valid multi-label domain", + domain: "example.com", errFunc: require.NoError, }, { - name: "Valid domain name with only one label", - domain: "example", + name: "Valid single label", + domain: "internal", errFunc: require.NoError, }, { - name: "Valid domain name with trailing dot", - domain: "example.", - errFunc: require.NoError, - }, - { - name: "Invalid wildcard domain name", - domain: "*.example", - errFunc: require.Error, - }, - { - name: "Invalid domain name with leading dot", - domain: ".com", - errFunc: require.Error, - }, - { - name: "Invalid domain name with dot only", - domain: ".", - errFunc: require.Error, - }, - { - name: "Invalid domain name with double hyphen", - domain: "test--example.com", - errFunc: require.Error, - }, - { - name: "Invalid domain name with a label exceeding 63 characters", - domain: "dnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdns.com", - errFunc: require.Error, - }, - { - name: "Invalid domain name starting with a hyphen", + name: "Invalid leading hyphen", domain: "-example.com", errFunc: require.Error, }, - { - name: "Invalid domain name ending with a hyphen", - domain: "example.com-", - errFunc: require.Error, - }, - { - name: "Invalid domain with unicode", - domain: "example?,.com", - errFunc: require.Error, - }, - { - name: "Invalid domain with space before top-level domain", - domain: "space .example.com", - errFunc: require.Error, - }, - { - name: "Invalid domain with trailing space", - domain: "example.com ", - errFunc: require.Error, - }, } for _, testCase := range testCases { diff --git a/management/server/networks/resources/manager_test.go b/management/server/networks/resources/manager_test.go index e2dea2c6b..29b0af2cc 100644 --- a/management/server/networks/resources/manager_test.go +++ b/management/server/networks/resources/manager_test.go @@ -203,7 +203,7 @@ func Test_CreateResourceFailsWithInvalidAddress(t *testing.T) { NetworkID: "testNetworkId", Name: "testResourceId", Description: "description", - Address: "invalid-address", + Address: "-invalid", } store, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir()) @@ -227,9 +227,9 @@ func Test_CreateResourceFailsWithUsedName(t *testing.T) { resource := &types.NetworkResource{ AccountID: "testAccountId", NetworkID: "testNetworkId", - Name: "testResourceId", + Name: "used-name", Description: "description", - Address: "invalid-address", + Address: "example.com", } store, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir()) diff --git a/management/server/networks/resources/types/resource.go b/management/server/networks/resources/types/resource.go index 6b8cf9412..1fa908393 100644 --- a/management/server/networks/resources/types/resource.go +++ b/management/server/networks/resources/types/resource.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "net/netip" - "regexp" "github.com/rs/xid" @@ -166,8 +165,7 @@ func GetResourceType(address string) (NetworkResourceType, string, netip.Prefix, return Host, "", netip.PrefixFrom(ip, ip.BitLen()), nil } - domainRegex := regexp.MustCompile(`^(\*\.)?([a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}$`) - if domainRegex.MatchString(address) { + if _, err := nbDomain.ValidateDomains([]string{address}); err == nil { return Domain, address, netip.Prefix{}, nil } diff --git a/management/server/networks/resources/types/resource_test.go b/management/server/networks/resources/types/resource_test.go index 02e802300..a842b0a28 100644 --- a/management/server/networks/resources/types/resource_test.go +++ b/management/server/networks/resources/types/resource_test.go @@ -23,10 +23,12 @@ func TestGetResourceType(t *testing.T) { {"example.com", Domain, false, "example.com", netip.Prefix{}}, {"*.example.com", Domain, false, "*.example.com", netip.Prefix{}}, {"sub.example.com", Domain, false, "sub.example.com", netip.Prefix{}}, + {"example.x", Domain, false, "example.x", netip.Prefix{}}, + {"internal", Domain, false, "internal", netip.Prefix{}}, // Invalid inputs - {"invalid", "", true, "", netip.Prefix{}}, {"1.1.1.1/abc", "", true, "", netip.Prefix{}}, - {"1234", "", true, "", netip.Prefix{}}, + {"-invalid.com", "", true, "", netip.Prefix{}}, + {"", "", true, "", netip.Prefix{}}, } for _, tt := range tests { diff --git a/management/server/util/util.go b/management/server/util/util.go index eea6a72b0..ce9759864 100644 --- a/management/server/util/util.go +++ b/management/server/util/util.go @@ -1,9 +1,5 @@ package util -import "regexp" - -var domainRegex = regexp.MustCompile(`^(\*\.)?([a-zA-Z0-9-]+\.)+[a-zA-Z]{2,}$`) - // Difference returns the elements in `a` that aren't in `b`. func Difference(a, b []string) []string { mb := make(map[string]struct{}, len(b)) @@ -55,9 +51,3 @@ func contains[T comparableObject[T]](slice []T, element T) bool { return false } -func IsValidDomain(domain string) bool { - if domain == "" { - return false - } - return domainRegex.MatchString(domain) -} diff --git a/shared/management/domain/validate.go b/shared/management/domain/validate.go index bf2af7116..1858b5d55 100644 --- a/shared/management/domain/validate.go +++ b/shared/management/domain/validate.go @@ -10,7 +10,30 @@ const maxDomains = 32 var domainRegex = regexp.MustCompile(`^(?:\*\.)?(?:(?:xn--)?[a-zA-Z0-9_](?:[a-zA-Z0-9-_]{0,61}[a-zA-Z0-9])?\.)*(?:xn--)?[a-zA-Z0-9](?:[a-zA-Z0-9-_]{0,61}[a-zA-Z0-9])?$`) -// ValidateDomains checks if each domain in the list is valid and returns a punycode-encoded DomainList. +// IsValidDomain checks if a single domain string is valid. +// Does not convert unicode to punycode - domain must already be ASCII/punycode. +// Allows wildcard prefix (*.example.com). +func IsValidDomain(domain string) bool { + if domain == "" { + return false + } + return domainRegex.MatchString(strings.ToLower(domain)) +} + +// IsValidDomainNoWildcard checks if a single domain string is valid without wildcard prefix. +// Use for zone domains and CNAME targets where wildcards are not allowed. +func IsValidDomainNoWildcard(domain string) bool { + if domain == "" { + return false + } + if strings.HasPrefix(domain, "*.") { + return false + } + return domainRegex.MatchString(strings.ToLower(domain)) +} + +// ValidateDomains validates domains and converts unicode to punycode. +// Allows wildcard prefix (*.example.com). Maximum 32 domains. func ValidateDomains(domains []string) (List, error) { if len(domains) == 0 { return nil, fmt.Errorf("domains list is empty") @@ -37,7 +60,10 @@ func ValidateDomains(domains []string) (List, error) { return domainList, nil } -// ValidateDomainsList checks if each domain in the list is valid +// ValidateDomainsList validates domains without punycode conversion. +// Use this for domains that must already be in ASCII/punycode format (e.g., extra DNS labels). +// Unlike ValidateDomains, this does not convert unicode to punycode - unicode domains will fail. +// Allows wildcard prefix (*.example.com). Maximum 32 domains. func ValidateDomainsList(domains []string) error { if len(domains) == 0 { return nil diff --git a/shared/management/domain/validate_test.go b/shared/management/domain/validate_test.go index 30efcd9a9..9dbcd8ac8 100644 --- a/shared/management/domain/validate_test.go +++ b/shared/management/domain/validate_test.go @@ -2,12 +2,16 @@ package domain import ( "fmt" + "strings" "testing" "github.com/stretchr/testify/assert" ) func TestValidateDomains(t *testing.T) { + label63 := strings.Repeat("a", 63) + label64 := strings.Repeat("a", 64) + tests := []struct { name string domains []string @@ -26,6 +30,48 @@ func TestValidateDomains(t *testing.T) { expected: List{"sub.ex-ample.com"}, wantErr: false, }, + { + name: "Valid uppercase domain normalized to lowercase", + domains: []string{"EXAMPLE.COM"}, + expected: List{"example.com"}, + wantErr: false, + }, + { + name: "Valid mixed case domain", + domains: []string{"ExAmPlE.CoM"}, + expected: List{"example.com"}, + wantErr: false, + }, + { + name: "Single letter TLD", + domains: []string{"example.x"}, + expected: List{"example.x"}, + wantErr: false, + }, + { + name: "Two letter domain labels", + domains: []string{"a.b"}, + expected: List{"a.b"}, + wantErr: false, + }, + { + name: "Single character domain", + domains: []string{"x"}, + expected: List{"x"}, + wantErr: false, + }, + { + name: "Wildcard with single letter TLD", + domains: []string{"*.x"}, + expected: List{"*.x"}, + wantErr: false, + }, + { + name: "Multi-level with single letter labels", + domains: []string{"a.b.c"}, + expected: List{"a.b.c"}, + wantErr: false, + }, { name: "Valid Unicode domain", domains: []string{"münchen.de"}, @@ -45,17 +91,92 @@ func TestValidateDomains(t *testing.T) { wantErr: false, }, { - name: "Invalid domain format", + name: "Valid domain starting with digit", + domains: []string{"123.example.com"}, + expected: List{"123.example.com"}, + wantErr: false, + }, + // Numeric TLDs are allowed for internal/private DNS use cases. + // While ICANN doesn't issue all-numeric gTLDs, the DNS protocol permits them + // and resolvers like systemd-resolved handle them correctly. + { + name: "Numeric TLD allowed", + domains: []string{"example.123"}, + expected: List{"example.123"}, + wantErr: false, + }, + { + name: "Single digit TLD allowed", + domains: []string{"example.1"}, + expected: List{"example.1"}, + wantErr: false, + }, + { + name: "All numeric labels allowed", + domains: []string{"123.456"}, + expected: List{"123.456"}, + wantErr: false, + }, + { + name: "Single numeric label allowed", + domains: []string{"123"}, + expected: List{"123"}, + wantErr: false, + }, + { + name: "Valid domain with double hyphen", + domains: []string{"test--example.com"}, + expected: List{"test--example.com"}, + wantErr: false, + }, + { + name: "Invalid leading hyphen", domains: []string{"-example.com"}, expected: nil, wantErr: true, }, { - name: "Invalid domain format 2", + name: "Invalid trailing hyphen", domains: []string{"example.com-"}, expected: nil, wantErr: true, }, + { + name: "Invalid leading dot", + domains: []string{".com"}, + expected: nil, + wantErr: true, + }, + { + name: "Invalid dot only", + domains: []string{"."}, + expected: nil, + wantErr: true, + }, + { + name: "Invalid double dot", + domains: []string{"example..com"}, + expected: nil, + wantErr: true, + }, + { + name: "Invalid special characters", + domains: []string{"example?,.com"}, + expected: nil, + wantErr: true, + }, + { + name: "Invalid space in domain", + domains: []string{"space .example.com"}, + expected: nil, + wantErr: true, + }, + { + name: "Invalid trailing space", + domains: []string{"example.com "}, + expected: nil, + wantErr: true, + }, { name: "Multiple domains valid and invalid", domains: []string{"google.com", "invalid,nbdomain.com", "münchen.de"}, @@ -86,6 +207,30 @@ func TestValidateDomains(t *testing.T) { expected: nil, wantErr: true, }, + { + name: "Valid 63 char label (max)", + domains: []string{label63 + ".com"}, + expected: List{Domain(label63 + ".com")}, + wantErr: false, + }, + { + name: "Invalid 64 char label (exceeds max)", + domains: []string{label64 + ".com"}, + expected: nil, + wantErr: true, + }, + { + name: "Valid 253 char domain (max)", + domains: []string{strings.Repeat("a.", 126) + "a"}, + expected: List{Domain(strings.Repeat("a.", 126) + "a")}, + wantErr: false, + }, + { + name: "Invalid 254+ char domain (exceeds max)", + domains: []string{strings.Repeat("ab.", 85)}, + expected: nil, + wantErr: true, + }, } for _, tt := range tests { @@ -118,6 +263,57 @@ func TestValidateDomainsList(t *testing.T) { domains: []string{"sub.ex-ample.com"}, wantErr: false, }, + { + name: "Uppercase domain accepted", + domains: []string{"EXAMPLE.COM"}, + wantErr: false, + }, + { + name: "Single letter TLD", + domains: []string{"example.x"}, + wantErr: false, + }, + { + name: "Two letter domain labels", + domains: []string{"a.b"}, + wantErr: false, + }, + { + name: "Single character domain", + domains: []string{"x"}, + wantErr: false, + }, + { + name: "Wildcard with single letter TLD", + domains: []string{"*.x"}, + wantErr: false, + }, + { + name: "Multi-level with single letter labels", + domains: []string{"a.b.c"}, + wantErr: false, + }, + // Numeric TLDs are allowed for internal/private DNS use cases. + { + name: "Numeric TLD allowed", + domains: []string{"example.123"}, + wantErr: false, + }, + { + name: "Single digit TLD allowed", + domains: []string{"example.1"}, + wantErr: false, + }, + { + name: "All numeric labels allowed", + domains: []string{"123.456"}, + wantErr: false, + }, + { + name: "Single numeric label allowed", + domains: []string{"123"}, + wantErr: false, + }, { name: "Underscores in labels", domains: []string{"_jabber._tcp.gmail.com"},