From 7ce62ccaecc646651f6c77ce4eb31d557bf64b12 Mon Sep 17 00:00:00 2001
From: Cuong Manh Le <cuong@windscribe.com>
Date: Wed, 9 Aug 2023 10:01:00 +0700
Subject: [PATCH] Validate DoH/DoH3 endpoint properly

When resolver type is doh/doh3, the endpoint must be a valid http url.

Updates #149
---
 cmd/cli/cli.go |  2 ++
 config.go      | 29 ++++++++++++++++++++++++++++-
 config_test.go |  8 ++++++++
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/cmd/cli/cli.go b/cmd/cli/cli.go
index 894efc0..ca48258 100644
--- a/cmd/cli/cli.go
+++ b/cmd/cli/cli.go
@@ -1383,6 +1383,8 @@ func fieldErrorMsg(fe validator.FieldError) string {
 		return fmt.Sprintf("invalid IP format: %s", fe.Value())
 	case "file":
 		return fmt.Sprintf("filed does not exist: %s", fe.Value())
+	case "http_url":
+		return fmt.Sprintf("invalid http/https url: %s", fe.Value())
 	}
 	return ""
 }
diff --git a/config.go b/config.go
index 70b165b..eef5af0 100644
--- a/config.go
+++ b/config.go
@@ -193,7 +193,7 @@ type NetworkConfig struct {
 type UpstreamConfig struct {
 	Name        string `mapstructure:"name" toml:"name,omitempty"`
 	Type        string `mapstructure:"type" toml:"type,omitempty" validate:"oneof=doh doh3 dot doq os legacy"`
-	Endpoint    string `mapstructure:"endpoint" toml:"endpoint,omitempty" validate:"required_unless=Type os"`
+	Endpoint    string `mapstructure:"endpoint" toml:"endpoint,omitempty"`
 	BootstrapIP string `mapstructure:"bootstrap_ip" toml:"bootstrap_ip,omitempty"`
 	Domain      string `mapstructure:"-" toml:"-"`
 	IPStack     string `mapstructure:"ip_stack" toml:"ip_stack,omitempty" validate:"ipstack"`
@@ -589,6 +589,7 @@ func ValidateConfig(validate *validator.Validate, cfg *Config) error {
 	_ = validate.RegisterValidation("dnsrcode", validateDnsRcode)
 	_ = validate.RegisterValidation("ipstack", validateIpStack)
 	_ = validate.RegisterValidation("iporempty", validateIpOrEmpty)
+	validate.RegisterStructValidation(upstreamConfigStructLevelValidation, UpstreamConfig{})
 	return validate.Struct(cfg)
 }
 
@@ -613,6 +614,32 @@ func validateIpOrEmpty(fl validator.FieldLevel) bool {
 	return net.ParseIP(val) != nil
 }
 
+func upstreamConfigStructLevelValidation(sl validator.StructLevel) {
+	uc := sl.Current().Addr().Interface().(*UpstreamConfig)
+	if uc.Type == ResolverTypeOS {
+		return
+	}
+
+	// Endpoint is required for non os resolver.
+	if uc.Endpoint == "" {
+		sl.ReportError(uc.Endpoint, "endpoint", "Endpoint", "required_unless", "")
+		return
+	}
+
+	// DoH/DoH3 requires endpoint is an HTTP url.
+	if uc.Type == ResolverTypeDOH || uc.Type == ResolverTypeDOH3 {
+		u, err := url.Parse(uc.Endpoint)
+		if err != nil || u.Host == "" {
+			sl.ReportError(uc.Endpoint, "endpoint", "Endpoint", "http_url", "")
+			return
+		}
+		if u.Scheme != "http" && u.Scheme != "https" {
+			sl.ReportError(uc.Endpoint, "endpoint", "Endpoint", "http_url", "")
+			return
+		}
+	}
+}
+
 func defaultPortFor(typ string) string {
 	switch typ {
 	case ResolverTypeDOH, ResolverTypeDOH3:
diff --git a/config_test.go b/config_test.go
index 83a3386..ca57372 100644
--- a/config_test.go
+++ b/config_test.go
@@ -95,6 +95,7 @@ func TestConfigValidation(t *testing.T) {
 		{"non-existed lease file", configWithNonExistedLeaseFile(t), true},
 		{"lease file format required if lease file exist", configWithExistedLeaseFile(t), true},
 		{"invalid lease file format", configWithInvalidLeaseFileFormat(t), true},
+		{"invalid doh/doh3 endpoint", configWithInvalidDoHEndpoint(t), true},
 	}
 
 	for _, tc := range tests {
@@ -225,3 +226,10 @@ func configWithInvalidLeaseFileFormat(t *testing.T) *ctrld.Config {
 	cfg.Service.DHCPLeaseFileFormat = "invalid"
 	return cfg
 }
+
+func configWithInvalidDoHEndpoint(t *testing.T) *ctrld.Config {
+	cfg := defaultConfig(t)
+	cfg.Upstream["0"].Endpoint = "1.1.1.1"
+	cfg.Upstream["0"].Type = ctrld.ResolverTypeDOH
+	return cfg
+}