f4879b5 Harden DNS config validation · gordon

M internal/adapters/out/acmelego/issuer.go +6 -3


@@ -92,13 +92,16 @@ func NewIssuer(cfg Config) (*Issuer, error) {
92	92		return nil, fmt.Errorf("acmelego: %w", domain.ErrCloudflareTokenMissing)
93	93		}
94	94		if len(cfg.DNSResolvers) == 0 {
95		-	return nil, fmt.Errorf("acmelego: DNSResolvers must contain at least one resolver")
	95	+	return nil, fmt.Errorf("acmelego: %w: DNSResolvers must contain at least one resolver", domain.ErrDNSConfigInvalid)
96	96		}
97	97		if cfg.DNSPropagationTimeout <= 0 {
98		-	return nil, fmt.Errorf("acmelego: DNSPropagationTimeout must be positive")
	98	+	return nil, fmt.Errorf("acmelego: %w: DNSPropagationTimeout must be positive", domain.ErrDNSConfigInvalid)
99	99		}
100	100		if cfg.DNSPollingInterval <= 0 {
101		-	return nil, fmt.Errorf("acmelego: DNSPollingInterval must be positive")
	101	+	return nil, fmt.Errorf("acmelego: %w: DNSPollingInterval must be positive", domain.ErrDNSConfigInvalid)
	102	+	}
	103	+	if cfg.DNSPollingInterval >= cfg.DNSPropagationTimeout {
	104	+	return nil, fmt.Errorf("acmelego: %w: DNSPollingInterval must be less than DNSPropagationTimeout", domain.ErrDNSConfigInvalid)
102	105		}
103	106		default:
104	107		return nil, fmt.Errorf("acmelego: %w: %s", domain.ErrACMEChallengeInvalid, cfg.Challenge)

 			return nil, fmt.Errorf("acmelego: %w", domain.ErrCloudflareTokenMissing)
 		}
 		if len(cfg.DNSResolvers) == 0 {
-			return nil, fmt.Errorf("acmelego: DNSResolvers must contain at least one resolver")
+			return nil, fmt.Errorf("acmelego: %w: DNSResolvers must contain at least one resolver", domain.ErrDNSConfigInvalid)
 		}
 		if cfg.DNSPropagationTimeout <= 0 {
-			return nil, fmt.Errorf("acmelego: DNSPropagationTimeout must be positive")
+			return nil, fmt.Errorf("acmelego: %w: DNSPropagationTimeout must be positive", domain.ErrDNSConfigInvalid)
 		}
 		if cfg.DNSPollingInterval <= 0 {
-			return nil, fmt.Errorf("acmelego: DNSPollingInterval must be positive")
+			return nil, fmt.Errorf("acmelego: %w: DNSPollingInterval must be positive", domain.ErrDNSConfigInvalid)
+		}
+		if cfg.DNSPollingInterval >= cfg.DNSPropagationTimeout {
+			return nil, fmt.Errorf("acmelego: %w: DNSPollingInterval must be less than DNSPropagationTimeout", domain.ErrDNSConfigInvalid)
 		}
 	default:
 		return nil, fmt.Errorf("acmelego: %w: %s", domain.ErrACMEChallengeInvalid, cfg.Challenge)

M internal/adapters/out/acmelego/issuer_test.go +14 -0


@@ -80,8 +80,22 @@ func TestNewIssuerValidatesCloudflareDNSConfig(t *testing.T) {
80	80		DNSPollingInterval: 5 * time.Second,
81	81		})
82	82		require.Error(t, err)
	83	+	assert.ErrorIs(t, err, domain.ErrDNSConfigInvalid)
83	84		assert.Contains(t, err.Error(), "DNSResolvers")
84	85
	86	+	_, err = NewIssuer(Config{
	87	+	Email: "test@example.com",
	88	+	Challenge: domain.ACMEChallengeCloudflareDNS01,
	89	+	Token: "token",
	90	+	Store: outmocks.NewMockCertificateStore(t),
	91	+	DNSResolvers: []string{"1.1.1.1:53"},
	92	+	DNSPropagationTimeout: 5 * time.Second,
	93	+	DNSPollingInterval: 5 * time.Second,
	94	+	})
	95	+	require.Error(t, err)
	96	+	assert.ErrorIs(t, err, domain.ErrDNSConfigInvalid)
	97	+	assert.Contains(t, err.Error(), "DNSPollingInterval")
	98	+
85	99		_, err = NewIssuer(Config{
86	100		Email: "test@example.com",
87	101		Challenge: domain.ACMEChallengeCloudflareDNS01,


@@ -80,8 +80,22 @@ func TestNewIssuerValidatesCloudflareDNSConfig(t *testing.T) {
80	DNSPollingInterval: 5 * time.Second,	80		DNSPollingInterval: 5 * time.Second,
81	})	81		})
82	require.Error(t, err)	82		require.Error(t, err)
		83	+	assert.ErrorIs(t, err, domain.ErrDNSConfigInvalid)
83	assert.Contains(t, err.Error(), "DNSResolvers")	84		assert.Contains(t, err.Error(), "DNSResolvers")
84		85
		86	+	_, err = NewIssuer(Config{
		87	+	Email: "test@example.com",
		88	+	Challenge: domain.ACMEChallengeCloudflareDNS01,
		89	+	Token: "token",
		90	+	Store: outmocks.NewMockCertificateStore(t),
		91	+	DNSResolvers: []string{"1.1.1.1:53"},
		92	+	DNSPropagationTimeout: 5 * time.Second,
		93	+	DNSPollingInterval: 5 * time.Second,
		94	+	})
		95	+	require.Error(t, err)
		96	+	assert.ErrorIs(t, err, domain.ErrDNSConfigInvalid)
		97	+	assert.Contains(t, err.Error(), "DNSPollingInterval")
		98	+
85	_, err = NewIssuer(Config{	99		_, err = NewIssuer(Config{
86	Email: "test@example.com",	100		Email: "test@example.com",
87	Challenge: domain.ACMEChallengeCloudflareDNS01,	101		Challenge: domain.ACMEChallengeCloudflareDNS01,

M internal/domain/errors.go +1 -0


@@ -96,6 +96,7 @@ var (
96	96		ErrACMEDisabled = errors.New("acme disabled")
97	97		ErrACMEEmailRequired = errors.New("acme email required")
98	98		ErrACMEChallengeInvalid = errors.New("acme challenge invalid")
	99	+	ErrDNSConfigInvalid = errors.New("dns configuration invalid")
99	100		ErrCloudflareTokenMissing = errors.New("cloudflare api token missing")
100	101		ErrCertificateStoreRequired = errors.New("certificate store required")
101	102		ErrCertificateIssuerRequired = errors.New("certificate issuer required")


@@ -96,6 +96,7 @@ var (
96	ErrACMEDisabled = errors.New("acme disabled")	96		ErrACMEDisabled = errors.New("acme disabled")
97	ErrACMEEmailRequired = errors.New("acme email required")	97		ErrACMEEmailRequired = errors.New("acme email required")
98	ErrACMEChallengeInvalid = errors.New("acme challenge invalid")	98		ErrACMEChallengeInvalid = errors.New("acme challenge invalid")
		99	+	ErrDNSConfigInvalid = errors.New("dns configuration invalid")
99	ErrCloudflareTokenMissing = errors.New("cloudflare api token missing")	100		ErrCloudflareTokenMissing = errors.New("cloudflare api token missing")
100	ErrCertificateStoreRequired = errors.New("certificate store required")	101		ErrCertificateStoreRequired = errors.New("certificate store required")
101	ErrCertificateIssuerRequired = errors.New("certificate issuer required")	102		ErrCertificateIssuerRequired = errors.New("certificate issuer required")

M internal/usecase/publictls/dns_config.go +16 -6


@@ -3,6 +3,7 @@ package publictls
3	3		import (
4	4		"fmt"
5	5		"net"
	6	+	"strconv"
6	7		"strings"
7	8		"time"
8	9		)

@@ -32,19 +33,28 @@ func DefaultDNSConfig() DNSConfig {
32	33		}
33	34		}
34	35
35		-	// Validate checks DNSConfig for ACME DNS-01 usage.
36		-	func (c DNSConfig) Validate() error {
	36	+	// Validate checks DNSConfig for ACME DNS-01 usage and normalizes resolver whitespace.
	37	+	func (c *DNSConfig) Validate() error {
37	38		if len(c.Resolvers) == 0 {
38	39		return fmt.Errorf("dns.resolvers must contain at least one resolver")
39	40		}
40	41		for i, resolver := range c.Resolvers {
41		-	resolver = strings.TrimSpace(resolver)
42		-	if resolver == "" {
	42	+	trimmed := strings.TrimSpace(resolver)
	43	+	if trimmed == "" {
43	44		return fmt.Errorf("invalid dns.resolvers[%d]: empty resolver", i)
44	45		}
45		-	if _, _, err := net.SplitHostPort(resolver); err != nil {
46		-	return fmt.Errorf("invalid dns.resolvers[%d]: %q: expected host:port: %w", i, resolver, err)
	46	+	host, port, err := net.SplitHostPort(trimmed)
	47	+	if err != nil {
	48	+	return fmt.Errorf("invalid dns.resolvers[%d]: %q: expected host:port: %w", i, trimmed, err)
47	49		}
	50	+	if host == "" {
	51	+	return fmt.Errorf("invalid dns.resolvers[%d]: %q: host must not be empty", i, trimmed)
	52	+	}
	53	+	portNumber, err := strconv.Atoi(port)
	54	+	if err != nil \|\| portNumber < 1 \|\| portNumber > 65535 {
	55	+	return fmt.Errorf("invalid dns.resolvers[%d]: %q: port must be a number between 1 and 65535", i, trimmed)
	56	+	}
	57	+	c.Resolvers[i] = trimmed
48	58		}
49	59		if c.PropagationTimeout <= 0 {
50	60		return fmt.Errorf("dns.propagation_timeout must be positive")

 import (
 	"fmt"
 	"net"
+	"strconv"
 	"strings"
 	"time"
 )
 	}
 }
-// Validate checks DNSConfig for ACME DNS-01 usage.
+// Validate checks DNSConfig for ACME DNS-01 usage and normalizes resolver whitespace.
-func (c DNSConfig) Validate() error {
+func (c *DNSConfig) Validate() error {
 	if len(c.Resolvers) == 0 {
 		return fmt.Errorf("dns.resolvers must contain at least one resolver")
 	}
 	for i, resolver := range c.Resolvers {
-		resolver = strings.TrimSpace(resolver)
+		trimmed := strings.TrimSpace(resolver)
-		if resolver == "" {
+		if trimmed == "" {
 			return fmt.Errorf("invalid dns.resolvers[%d]: empty resolver", i)
 		}
-		if _, _, err := net.SplitHostPort(resolver); err != nil {
+		host, port, err := net.SplitHostPort(trimmed)
-			return fmt.Errorf("invalid dns.resolvers[%d]: %q: expected host:port: %w", i, resolver, err)
+		if err != nil {
+			return fmt.Errorf("invalid dns.resolvers[%d]: %q: expected host:port: %w", i, trimmed, err)
 		}
+		if host == "" {
+			return fmt.Errorf("invalid dns.resolvers[%d]: %q: host must not be empty", i, trimmed)
+		}
+		portNumber, err := strconv.Atoi(port)
+		if err != nil || portNumber < 1 || portNumber > 65535 {
+			return fmt.Errorf("invalid dns.resolvers[%d]: %q: port must be a number between 1 and 65535", i, trimmed)
+		}
+		c.Resolvers[i] = trimmed
 	}
 	if c.PropagationTimeout <= 0 {
 		return fmt.Errorf("dns.propagation_timeout must be positive")

M internal/usecase/publictls/dns_config_test.go +29 -0


@@ -16,6 +16,17 @@ func TestDefaultDNSConfig(t *testing.T) {
16	16		assert.Equal(t, 5*time.Second, cfg.PollingInterval)
17	17		}
18	18
	19	+	func TestDNSConfigValidateNormalizesResolvers(t *testing.T) {
	20	+	cfg := DNSConfig{
	21	+	Resolvers: []string{" 1.1.1.1:53 ", "\t8.8.8.8:53\n"},
	22	+	PropagationTimeout: 5 * time.Minute,
	23	+	PollingInterval: 5 * time.Second,
	24	+	}
	25	+
	26	+	require.NoError(t, cfg.Validate())
	27	+	assert.Equal(t, []string{"1.1.1.1:53", "8.8.8.8:53"}, cfg.Resolvers)
	28	+	}
	29	+
19	30		func TestDNSConfigValidate(t *testing.T) {
20	31		tests := []struct {
21	32		name string

@@ -48,6 +59,24 @@ func TestDNSConfigValidate(t *testing.T) {
48	59		},
49	60		wantErr: "invalid dns.resolvers[0]",
50	61		},
	62	+	{
	63	+	name: "whitespace-only resolver",
	64	+	cfg: DNSConfig{
	65	+	Resolvers: []string{" "},
	66	+	PropagationTimeout: 5 * time.Minute,
	67	+	PollingInterval: 5 * time.Second,
	68	+	},
	69	+	wantErr: "invalid dns.resolvers[0]",
	70	+	},
	71	+	{
	72	+	name: "malformed resolver port",
	73	+	cfg: DNSConfig{
	74	+	Resolvers: []string{"1.1.1.1:abc"},
	75	+	PropagationTimeout: 5 * time.Minute,
	76	+	PollingInterval: 5 * time.Second,
	77	+	},
	78	+	wantErr: "invalid dns.resolvers[0]",
	79	+	},
51	80		{
52	81		name: "zero timeout",
53	82		cfg: DNSConfig{


@@ -16,6 +16,17 @@ func TestDefaultDNSConfig(t *testing.T) {
16	assert.Equal(t, 5*time.Second, cfg.PollingInterval)	16		assert.Equal(t, 5*time.Second, cfg.PollingInterval)
17	}	17		}
18		18
		19	+	func TestDNSConfigValidateNormalizesResolvers(t *testing.T) {
		20	+	cfg := DNSConfig{
		21	+	Resolvers: []string{" 1.1.1.1:53 ", "\t8.8.8.8:53\n"},
		22	+	PropagationTimeout: 5 * time.Minute,
		23	+	PollingInterval: 5 * time.Second,
		24	+	}
		25	+
		26	+	require.NoError(t, cfg.Validate())
		27	+	assert.Equal(t, []string{"1.1.1.1:53", "8.8.8.8:53"}, cfg.Resolvers)
		28	+	}
		29	+
19	func TestDNSConfigValidate(t *testing.T) {	30		func TestDNSConfigValidate(t *testing.T) {
20	tests := []struct {	31		tests := []struct {
21	name string	32		name string

@@ -48,6 +59,24 @@ func TestDNSConfigValidate(t *testing.T) {
48	},	59		},
49	wantErr: "invalid dns.resolvers[0]",	60		wantErr: "invalid dns.resolvers[0]",
50	},	61		},
		62	+	{
		63	+	name: "whitespace-only resolver",
		64	+	cfg: DNSConfig{
		65	+	Resolvers: []string{" "},
		66	+	PropagationTimeout: 5 * time.Minute,
		67	+	PollingInterval: 5 * time.Second,
		68	+	},
		69	+	wantErr: "invalid dns.resolvers[0]",
		70	+	},
		71	+	{
		72	+	name: "malformed resolver port",
		73	+	cfg: DNSConfig{
		74	+	Resolvers: []string{"1.1.1.1:abc"},
		75	+	PropagationTimeout: 5 * time.Minute,
		76	+	PollingInterval: 5 * time.Second,
		77	+	},
		78	+	wantErr: "invalid dns.resolvers[0]",
		79	+	},
51	{	80		{
52	name: "zero timeout",	81		name: "zero timeout",
53	cfg: DNSConfig{	82		cfg: DNSConfig{