fix(configuration): address parsing failure (#3653)

This fixes an issue with parsing address types from strings.

config.template.yml

@@ -114,7 +114,7 @@ telemetry:
     enabled: false
 
     ## The address to listen on for metrics. This should be on a different port to the main server.port value.
-    address: '0.0.0.0:9959'
+    address: tcp://0.0.0.0:9959
 
 ##
 ## TOTP Configuration

docs/content/en/configuration/prologue/common.md

@@ -57,6 +57,32 @@ While you can use multiple of these blocks in combination, ee suggest keeping it
 |         1 day         | `1d` or `24h` or `86400` or `86400s`  |
 |       10 hours        | `10h` or `600m` or `9h60m` or `36000` |
 
+## Address
+
+The address type is a string that takes the following format:
+
+```text
+[<scheme>://]<ip>[:<port>]
+```
+
+The square brackets indicate optional sections, and the angled brackets indicate required sections. The following
+sections elaborate on this. Sections may only be optional for the purposes of parsing, there may be a configuration
+requirement that one of these is provided.
+
+### scheme
+
+The entire scheme is optional, but if the scheme host delimiter `://` is in the string, the scheme must be present. The
+scheme must be one of `tcp://`, or `udp://`. The default scheme is `tcp://`.
+
+### ip
+
+The IP is required. If specifying an IPv6 it should be wrapped in square brackets. For example for the IPv6 address
+`::1` with the `tcp://` scheme and port `80`: `tcp://[::1]:80`.
+
+### port
+
+The entire port is optional, but if the host port delimiter `:` exists it must also include a numeric port.
+
 ## Regular Expressions
 
 We have several sections of configuration that utilize regular expressions. It's recommended to validate your regex

docs/content/en/configuration/telemetry/metrics.md

@@ -20,7 +20,7 @@ toc: true
 telemetry:
   metrics:
     enabled: false
-    address: "0.0.0.0:9959"
+    address: "tcp://0.0.0.0:9959"
 ```
 
 ## Options
@@ -33,10 +33,10 @@ Determines if the [Prometheus] HTTP Metrics Exporter is enabled.
 
 ### address
 
-{{< confkey type="address" default="0.0.0.0:9959" required="no" >}}
+{{< confkey type="address" default="tcp://0.0.0.0:9959" required="no" >}}
 
-Configures the listener address for the [Prometheus] HTTP Metrics Exporter. The address must be a IPv4 or IPv6 address
+Configures the listener address for the [Prometheus] HTTP Metrics Exporter. This configuration key uses the
-followed by the port in the `<address>:<port>` format.
+[Address](../prologue/common.md#address) format. The scheme must be `tcp://` or empty.
 
 ## See More
 

internal/configuration/config.template.yml

@@ -114,7 +114,7 @@ telemetry:
     enabled: false
 
     ## The address to listen on for metrics. This should be on a different port to the main server.port value.
-    address: '0.0.0.0:9959'
+    address: tcp://0.0.0.0:9959
 
 ##
 ## TOTP Configuration

internal/configuration/decode_hooks_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/authelia/authelia/v4/internal/configuration"
+	"github.com/authelia/authelia/v4/internal/configuration/schema"
 )
 
 func TestStringToMailAddressHookFunc(t *testing.T) {
@@ -748,6 +749,120 @@ func TestStringToRegexpFuncPointers(t *testing.T) {
 	}
 }
 
+func TestStringToAddressHookFunc(t *testing.T) {
+	mustParseAddress := func(a string) (addr schema.Address) {
+		addrs, err := schema.NewAddressFromString(a)
+		if err != nil {
+			panic(err)
+		}
+
+		return *addrs
+	}
+
+	mustParseAddressPtr := func(a string) (addr *schema.Address) {
+		addr, err := schema.NewAddressFromString(a)
+		if err != nil {
+			panic(err)
+		}
+
+		return addr
+	}
+
+	testCases := []struct {
+		name     string
+		have     interface{}
+		expected interface{}
+		err      string
+		decode   bool
+		wantGrps []string
+	}{
+		{
+			name:     "ShouldDecodeNonPtr",
+			have:     "tcp://0.0.0.0:2020",
+			expected: mustParseAddress("tcp://0.0.0.0:2020"),
+			decode:   true,
+		},
+		{
+			name:     "ShouldDecodePtr",
+			have:     "tcp://0.0.0.0:2020",
+			expected: mustParseAddressPtr("tcp://0.0.0.0:2020"),
+			decode:   true,
+		},
+		{
+			name:     "ShouldNotDecodeIntegerToCorrectType",
+			have:     1,
+			expected: schema.Address{},
+			decode:   false,
+		},
+		{
+			name:     "ShouldNotDecodeIntegerToCorrectTypePtr",
+			have:     1,
+			expected: &schema.Address{},
+			decode:   false,
+		},
+		{
+			name:     "ShouldNotDecodeIntegerPtrToCorrectType",
+			have:     testInt32Ptr(1),
+			expected: schema.Address{},
+			decode:   false,
+		},
+		{
+			name:     "ShouldNotDecodeIntegerPtrToCorrectTypePtr",
+			have:     testInt32Ptr(1),
+			expected: &schema.Address{},
+			decode:   false,
+		},
+		{
+			name:     "ShouldNotDecodeToString",
+			have:     "tcp://0.0.0.0:2020",
+			expected: "",
+			decode:   false,
+		},
+		{
+			name:     "ShouldNotDecodeToIntPtr",
+			have:     "tcp://0.0.0.0:2020",
+			expected: testInt32Ptr(1),
+			decode:   false,
+		},
+		{
+			name:     "ShouldNotDecodeToIntPtr",
+			have:     "tcp://0.0.0.0:2020",
+			expected: testInt32Ptr(1),
+			decode:   false,
+		},
+		{
+			name:     "ShouldFailDecode",
+			have:     "tcp://&!@^#*&!@#&*@!:2020",
+			expected: schema.Address{},
+			err:      "could not decode 'tcp://&!@^#*&!@#&*@!:2020' to a Address: could not parse string 'tcp://&!@^#*&!@#&*@!:2020' as address: expected format is [<scheme>://]<ip>[:<port>]: parse \"tcp://&!@^\": invalid character \"^\" in host name",
+			decode:   false,
+		},
+	}
+
+	hook := configuration.StringToAddressHookFunc()
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual, err := hook(reflect.TypeOf(tc.have), reflect.TypeOf(tc.expected), tc.have)
+			if tc.err != "" {
+				assert.EqualError(t, err, tc.err)
+
+				if !tc.decode {
+					assert.Nil(t, actual)
+				}
+			} else {
+				assert.NoError(t, err)
+
+				if tc.decode {
+					assert.Equal(t, tc.expected, actual)
+				} else {
+					assert.Equal(t, tc.have, actual)
+				}
+			}
+		})
+	}
+}
+
 func testInt32Ptr(i int32) *int32 {
 	return &i
 }

internal/configuration/schema/const.go

@@ -1,6 +1,7 @@
 package schema
 
 import (
+	"regexp"
 	"time"
 )
 
@@ -52,3 +53,7 @@ const (
 	// TOTPSecretSizeMinimum is the minimum secret size.
 	TOTPSecretSizeMinimum = 20
 )
+
+// regexpHasScheme checks if a string has a scheme. Valid characters for schemes include alphanumeric, hyphen,
+// period, and plus characters.
+var regexpHasScheme = regexp.MustCompile(`^[-+.a-zA-Z\d]+://`)

internal/configuration/schema/telemetry.go

@@ -11,13 +11,13 @@ type TelemetryConfig struct {
 
 // TelemetryMetricsConfig represents the telemetry metrics config.
 type TelemetryMetricsConfig struct {
-	Enabled bool    `koanf:"enabled"`
+	Enabled bool     `koanf:"enabled"`
-	Address Address `koanf:"address"`
+	Address *Address `koanf:"address"`
 }
 
 // DefaultTelemetryConfig is the default telemetry configuration.
 var DefaultTelemetryConfig = TelemetryConfig{
 	Metrics: TelemetryMetricsConfig{
-		Address: NewAddress("tcp", net.ParseIP("0.0.0.0"), 9959),
+		Address: &Address{true, "tcp", net.ParseIP("0.0.0.0"), 9959},
 	},
 }

internal/configuration/schema/types.go

@@ -3,74 +3,64 @@ package schema
 import (
 	"fmt"
 	"net"
-	"regexp"
+	"net/url"
 	"strconv"
+	"strings"
 )
 
-var regexpAddress = regexp.MustCompile(`^((?P<Scheme>\w+)://)?((?P<IPv4>((((25[0-5]|2[0-4]\d|[01]?\d\d?)(\.)){3})(25[0-5]|2[0-4]\d|[01]?\d\d?)))|(\[(?P<IPv6>([0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4})?:)?((25[0-5]|(2[0-4]|1?\d)?\d)\.){3}(25[0-5]|(2[0-4]|1?\d)?\d)|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1?\d)?\d)\.){3}(25[0-5]|(2[0-4]|1?\d)?\d))\]):)?(?P<Port>\d+)$`)
+// NewAddressFromString returns an *Address and error depending on the ability to parse the string as an Address.
-
+func NewAddressFromString(a string) (addr *Address, err error) {
-const tcp = "tcp"
+	if len(a) == 0 {
-
+		return &Address{true, "tcp", net.ParseIP("0.0.0.0"), 0}, nil
-// NewAddress produces a valid address from input.
-func NewAddress(scheme string, ip net.IP, port int) Address {
-	return Address{
-		valid:  true,
-		Scheme: scheme,
-		IP:     ip,
-		Port:   port,
 	}
+
+	var u *url.URL
+
+	if regexpHasScheme.MatchString(a) {
+		u, err = url.Parse(a)
+	} else {
+		u, err = url.Parse("tcp://" + a)
+	}
+
+	if err != nil {
+		return nil, fmt.Errorf("could not parse string '%s' as address: expected format is [<scheme>://]<ip>[:<port>]: %w", a, err)
+	}
+
+	return NewAddressFromURL(u)
 }
 
-// NewAddressFromString parses a string and returns an *Address or error.
+// NewAddressFromURL returns an *Address and error depending on the ability to parse the *url.URL as an Address.
-func NewAddressFromString(addr string) (address *Address, err error) {
+func NewAddressFromURL(u *url.URL) (addr *Address, err error) {
-	if addr == "" {
+	addr = &Address{
-		return &Address{}, nil
+		Scheme: strings.ToLower(u.Scheme),
+		IP:     net.ParseIP(u.Hostname()),
 	}
 
-	if !regexpAddress.MatchString(addr) {
+	if addr.IP == nil {
-		return nil, fmt.Errorf("the string '%s' does not appear to be a valid address", addr)
+		return nil, fmt.Errorf("could not parse ip for address '%s': %s does not appear to be an IP", u.String(), u.Hostname())
 	}
 
-	address = &Address{
+	port := u.Port()
-		valid: true,
+	switch port {
-	}
+	case "":
-
+		break
-	submatches := regexpAddress.FindStringSubmatch(addr)
+	default:
-
+		addr.Port, err = strconv.Atoi(port)
-	var ip, port string
+		if err != nil {
-
+			return nil, fmt.Errorf("could not parse port for address '%s': %w", u.String(), err)
-	for i, name := range regexpAddress.SubexpNames() {
-		switch name {
-		case "Scheme":
-			address.Scheme = submatches[i]
-		case "IPv4":
-			ip = submatches[i]
-
-			if address.Scheme == "" || address.Scheme == tcp {
-				address.Scheme = "tcp4"
-			}
-		case "IPv6":
-			ip = submatches[i]
-
-			if address.Scheme == "" || address.Scheme == tcp {
-				address.Scheme = "tcp6"
-			}
-		case "Port":
-			port = submatches[i]
 		}
 	}
 
-	if address.IP = net.ParseIP(ip); address.IP == nil {
+	switch addr.Scheme {
-		return nil, fmt.Errorf("failed to parse '%s' as an IP address", ip)
+	case "tcp", "udp", "http", "https":
+		break
+	default:
+		return nil, fmt.Errorf("could not parse scheme for address '%s': scheme '%s' is not valid, expected to be one of 'tcp://', 'udp://'", u.String(), addr.Scheme)
 	}
 
-	address.Port, _ = strconv.Atoi(port)
+	addr.valid = true
 
-	if address.Port <= 0 || address.Port > 65535 {
+	return addr, nil
-		return nil, fmt.Errorf("failed to parse address port '%d' is invalid: ports must be between 1 and 65535", address.Port)
-	}
-
-	return address, nil
 }
 
 // Address represents an address.
@@ -78,8 +68,8 @@ type Address struct {
 	valid bool
 
 	Scheme string
-	net.IP
+	IP     net.IP
-	Port int
+	Port   int
 }
 
 // Valid returns true if the Address is valid.

internal/configuration/schema/types_test.go

@@ -0,0 +1,50 @@
+package schema
+
+import (
+	"net"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewAddressFromString(t *testing.T) {
+	testCases := []struct {
+		name                                          string
+		have                                          string
+		expected                                      *Address
+		expectedHostPort, expectedString, expectedErr string
+	}{
+		{"ShouldParseBasicAddress", "tcp://0.0.0.0:9091", &Address{true, "tcp", net.ParseIP("0.0.0.0"), 9091}, "0.0.0.0:9091", "tcp://0.0.0.0:9091", ""},
+		{"ShouldParseEmptyAddress", "", &Address{true, "tcp", net.ParseIP("0.0.0.0"), 0}, "0.0.0.0:0", "tcp://0.0.0.0:0", ""},
+		{"ShouldParseAddressMissingScheme", "0.0.0.0:9091", &Address{true, "tcp", net.ParseIP("0.0.0.0"), 9091}, "0.0.0.0:9091", "tcp://0.0.0.0:9091", ""},
+		{"ShouldParseAddressMissingPort", "tcp://0.0.0.0", &Address{true, "tcp", net.ParseIP("0.0.0.0"), 0}, "0.0.0.0:0", "tcp://0.0.0.0:0", ""},
+		{"ShouldNotParseUnknownScheme", "a://0.0.0.0", nil, "", "", "could not parse scheme for address 'a://0.0.0.0': scheme 'a' is not valid, expected to be one of 'tcp://', 'udp://'"},
+		{"ShouldNotParseInvalidPort", "tcp://0.0.0.0:abc", nil, "", "", "could not parse string 'tcp://0.0.0.0:abc' as address: expected format is [<scheme>://]<ip>[:<port>]: parse \"tcp://0.0.0.0:abc\": invalid port \":abc\" after host"},
+		{"ShouldNotParseInvalidIP", "tcp://example.com:9091", nil, "", "", "could not parse ip for address 'tcp://example.com:9091': example.com does not appear to be an IP"},
+		{"ShouldNotParseInvalidAddress", "@$@#%@#$@@", nil, "", "", "could not parse string '@$@#%@#$@@' as address: expected format is [<scheme>://]<ip>[:<port>]: parse \"tcp://@$@#%@#$@@\": invalid URL escape \"%@#\""},
+		{"ShouldNotParseInvalidAddressWithScheme", "tcp://@$@#%@#$@@", nil, "", "", "could not parse string 'tcp://@$@#%@#$@@' as address: expected format is [<scheme>://]<ip>[:<port>]: parse \"tcp://@$@#%@#$@@\": invalid URL escape \"%@#\""},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual, actualErr := NewAddressFromString(tc.have)
+
+			if len(tc.expectedErr) != 0 {
+				assert.EqualError(t, actualErr, tc.expectedErr)
+			} else {
+				assert.Nil(t, actualErr)
+
+				assert.Equal(t, actual.HostPort(), tc.expectedHostPort)
+				assert.Equal(t, actual.String(), tc.expectedString)
+			}
+
+			assert.Equal(t, tc.expected, actual)
+
+			if actual != nil {
+				assert.True(t, actual.Valid())
+				assert.NotEmpty(t, actual.String())
+				assert.NotEmpty(t, actual.HostPort())
+			}
+		})
+	}
+}

internal/configuration/validator/const.go

@@ -123,6 +123,11 @@ const (
 	errFmtStoragePostgreSQLInvalidSSLMode    = "storage: postgres: ssl: option 'mode' must be one of '%s' but it is configured as '%s'"
 )
 
+// Telemetry Error constants.
+const (
+	errFmtTelemetryMetricsScheme = "telemetry: metrics: option 'address' must have a scheme 'tcp://' but it is configured as '%s'"
+)
+
 // OpenID Error constants.
 const (
 	errFmtOIDCNoClientsConfigured = "identity_providers: oidc: option 'clients' must have one or " +

internal/configuration/validator/telemetry.go

@@ -1,14 +1,25 @@
 package validator
 
 import (
+	"fmt"
+
 	"github.com/authelia/authelia/v4/internal/configuration/schema"
 )
 
 // ValidateTelemetry validates the telemetry configuration.
 func ValidateTelemetry(config *schema.Configuration, validator *schema.StructValidator) {
-	if config.Telemetry.Metrics.Enabled {
+	if config.Telemetry.Metrics.Address == nil {
-		if config.Telemetry.Metrics.Address.String() == "" {
+		config.Telemetry.Metrics.Address = schema.DefaultTelemetryConfig.Metrics.Address
-			config.Telemetry.Metrics.Address = schema.DefaultTelemetryConfig.Metrics.Address
+	}
-		}
+
+	switch config.Telemetry.Metrics.Address.Scheme {
+	case "tcp":
+		break
+	default:
+		validator.Push(fmt.Errorf(errFmtTelemetryMetricsScheme, config.Telemetry.Metrics.Address.Scheme))
+	}
+
+	if config.Telemetry.Metrics.Address.Port == 0 {
+		config.Telemetry.Metrics.Address.Port = schema.DefaultTelemetryConfig.Metrics.Address.Port
 	}
 }

internal/configuration/validator/telemetry_test.go

@@ -0,0 +1,101 @@
+package validator
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/authelia/authelia/v4/internal/configuration/schema"
+)
+
+func TestValidateTelemetry(t *testing.T) {
+	mustParseAddress := func(a string) *schema.Address {
+		addr, err := schema.NewAddressFromString(a)
+		if err != nil {
+			panic(err)
+		}
+
+		return addr
+	}
+
+	testCases := []struct {
+		name                       string
+		have                       *schema.Configuration
+		expected                   *schema.Configuration
+		expectedWrns, expectedErrs []string
+	}{
+		{
+			"ShouldSetDefaults",
+			&schema.Configuration{},
+			&schema.Configuration{Telemetry: schema.DefaultTelemetryConfig},
+			nil,
+			nil,
+		},
+		{
+			"ShouldSetDefaultPort",
+			&schema.Configuration{Telemetry: schema.TelemetryConfig{Metrics: schema.TelemetryMetricsConfig{Address: mustParseAddress("tcp://0.0.0.0")}}},
+			&schema.Configuration{Telemetry: schema.DefaultTelemetryConfig},
+			nil,
+			nil,
+		},
+		{
+			"ShouldSetDefaultPortAlt",
+			&schema.Configuration{Telemetry: schema.TelemetryConfig{Metrics: schema.TelemetryMetricsConfig{Address: mustParseAddress("tcp://0.0.0.0:0")}}},
+			&schema.Configuration{Telemetry: schema.DefaultTelemetryConfig},
+			nil,
+			nil,
+		},
+		{
+			"ShouldSetDefaultPortWithCustomIP",
+			&schema.Configuration{Telemetry: schema.TelemetryConfig{Metrics: schema.TelemetryMetricsConfig{Address: mustParseAddress("tcp://127.0.0.1")}}},
+			&schema.Configuration{Telemetry: schema.TelemetryConfig{Metrics: schema.TelemetryMetricsConfig{Address: mustParseAddress("tcp://127.0.0.1:9959")}}},
+			nil,
+			nil,
+		},
+		{
+			"ShouldNotValidateUDP",
+			&schema.Configuration{Telemetry: schema.TelemetryConfig{Metrics: schema.TelemetryMetricsConfig{Address: mustParseAddress("udp://0.0.0.0")}}},
+			&schema.Configuration{Telemetry: schema.TelemetryConfig{Metrics: schema.TelemetryMetricsConfig{Address: mustParseAddress("udp://0.0.0.0:9959")}}},
+			nil,
+			[]string{"telemetry: metrics: option 'address' must have a scheme 'tcp://' but it is configured as 'udp'"},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			v := schema.NewStructValidator()
+
+			ValidateTelemetry(tc.have, v)
+
+			assert.Equal(t, tc.expected.Telemetry.Metrics.Enabled, tc.have.Telemetry.Metrics.Enabled)
+			assert.Equal(t, tc.expected.Telemetry.Metrics.Address, tc.have.Telemetry.Metrics.Address)
+
+			lenWrns := len(tc.expectedWrns)
+			wrns := v.Warnings()
+
+			if lenWrns == 0 {
+				assert.Len(t, wrns, 0)
+			} else {
+				require.Len(t, wrns, lenWrns)
+
+				for i, expectedWrn := range tc.expectedWrns {
+					assert.EqualError(t, wrns[i], expectedWrn)
+				}
+			}
+
+			lenErrs := len(tc.expectedErrs)
+			errs := v.Errors()
+
+			if lenErrs == 0 {
+				assert.Len(t, errs, 0)
+			} else {
+				require.Len(t, errs, lenErrs)
+
+				for i, expectedErr := range tc.expectedErrs {
+					assert.EqualError(t, errs[i], expectedErr)
+				}
+			}
+		})
+	}
+}

internal/suites/Standalone/configuration.yml

@@ -14,6 +14,7 @@ server:
 telemetry:
   metrics:
     enabled: true
+    address: tcp://0.0.0.0:9959
 
 log:
   level: debug