From e6518a0d3c31a9772728edd73a1521d25a76c082 Mon Sep 17 00:00:00 2001
From: "fox.cpp" <fox.cpp@disroot.org>
Date: Sun, 8 Jan 2023 15:33:22 +0300
Subject: [PATCH] Improve domain validation function

Now FQDNs are accepted as well (with trailing dot).
Empty strings are not considered valid.
Label length for IDNs is checked using Punycode form as it should.

See https://github.com/foxcpp/maddy/issues/554
---
 framework/address/validation.go      | 14 +++++++++++---
 framework/address/validation_test.go | 20 ++++++++++++++++++++
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/framework/address/validation.go b/framework/address/validation.go
index 4e2b38a6..a165adcb 100644
--- a/framework/address/validation.go
+++ b/framework/address/validation.go
@@ -20,6 +20,8 @@ package address
 
 import (
 	"strings"
+
+	"golang.org/x/net/idna"
 )
 
 /*
@@ -109,17 +111,23 @@ func ValidMailboxName(mbox string) bool {
 
 // ValidDomain checks whether the specified string is a valid DNS domain.
 func ValidDomain(domain string) bool {
-	if len(domain) > 255 {
+	if len(domain) > 255 || len(domain) == 0 {
 		return false
 	}
-	if strings.HasPrefix(domain, ".") || strings.HasSuffix(domain, ".") {
+	if strings.HasPrefix(domain, ".") {
 		return false
 	}
 	if strings.Contains(domain, "..") {
 		return false
 	}
 
-	labels := strings.Split(domain, ".")
+	// Length checks are to be applied to A-labels form.
+	// maddy uses U-labels representation across the code (for lookups, etc).
+	domainASCII, err := idna.ToASCII(domain)
+	if err != nil {
+		return false
+	}
+	labels := strings.Split(domainASCII, ".")
 	for _, label := range labels {
 		if len(label) > 64 {
 			return false
diff --git a/framework/address/validation_test.go b/framework/address/validation_test.go
index 9416e55f..fdac8349 100644
--- a/framework/address/validation_test.go
+++ b/framework/address/validation_test.go
@@ -1,6 +1,7 @@
 package address_test
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/foxcpp/maddy/framework/address"
@@ -11,3 +12,22 @@ func TestValidMailboxName(t *testing.T) {
 		t.Error("caddy.bug should be valid mailbox name")
 	}
 }
+
+func TestValidDomain(t *testing.T) {
+	for _, c := range []struct {
+		Domain string
+		Valid  bool
+	}{
+		{Domain: "maddy.email", Valid: true},
+		{Domain: "", Valid: false},
+		{Domain: "maddy.email.", Valid: true},
+		{Domain: "..", Valid: false},
+		{Domain: strings.Repeat("a", 256), Valid: false},
+		{Domain: "äõäoaõoäaõaäõaoäaoaäõoaäooaoaoiuaiauäõiuüõaõäiauõaaa.tld", Valid: true},            // https://github.com/foxcpp/maddy/issues/554
+		{Domain: "xn--oaoaaaoaoaoaooaoaoiuaiauiuaiauaaa-f1cadccdcmd01eddchqcbe07a.tld", Valid: true}, // https://github.com/foxcpp/maddy/issues/554
+	} {
+		if actual := address.ValidDomain(c.Domain); actual != c.Valid {
+			t.Errorf("expected domain %v to be valid=%v, but got %v", c.Domain, c.Valid, actual)
+		}
+	}
+}