Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions client/internal/dns/handler_chain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 3 additions & 4 deletions management/cmd/management.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
Comment thread
lixmal marked this conversation as resolved.

return nil
Expand Down
8 changes: 4 additions & 4 deletions management/internals/modules/zones/records/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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")
}

Expand All @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions management/internals/modules/zones/zone.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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")
}

Expand Down
11 changes: 7 additions & 4 deletions management/server/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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) {
Expand Down
25 changes: 13 additions & 12 deletions management/server/nameserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
75 changes: 23 additions & 52 deletions management/server/nameserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
errFunc: require.NoError,
name: "Wildcard prefix rejected",
domain: "*.example.com",
errFunc: require.Error,
},
{
name: "Valid domain name with hyphen",
domain: "test-example.com",
errFunc: require.NoError,
name: "Wildcard in middle rejected",
domain: "a.*.example.com",
errFunc: require.Error,
},
// Nameserver-specific: unicode converted to punycode
{
name: "Valid domain name with only one label",
domain: "example",
name: "Unicode domain converted to punycode",
domain: "münchen.de",
errFunc: require.NoError,
},
{
name: "Valid domain name with trailing dot",
domain: "example.",
name: "Unicode domain all labels",
domain: "中国.中国",
errFunc: require.NoError,
},
// Basic validation still works (delegates to shared validation)
{
name: "Invalid wildcard domain name",
Comment thread
mlsmaycon marked this conversation as resolved.
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: "Valid multi-label domain",
domain: "example.com",
errFunc: require.NoError,
},
{
name: "Invalid domain name with a label exceeding 63 characters",
domain: "dnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdnsdns.com",
errFunc: require.Error,
name: "Valid single label",
domain: "internal",
errFunc: require.NoError,
},
{
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 {
Expand Down
6 changes: 3 additions & 3 deletions management/server/networks/resources/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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",
Comment thread
mlsmaycon marked this conversation as resolved.
Address: "example.com",
}

store, cleanUp, err := store.NewTestStoreFromSQL(context.Background(), "../../testdata/networks.sql", t.TempDir())
Expand Down
4 changes: 1 addition & 3 deletions management/server/networks/resources/types/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"
"fmt"
"net/netip"
"regexp"

"github.com/rs/xid"

Expand Down Expand Up @@ -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
}

Expand Down
6 changes: 4 additions & 2 deletions management/server/networks/resources/types/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading
Loading