Add additional validation to otel endpoint

internal/configs/configmaps.go

@@ -754,6 +754,11 @@ func parseConfigMapOpenTelemetry(l *slog.Logger, cfgm *v1.ConfigMap, cfgParams *
 	if otelExporterEndpoint, exists := cfgm.Data["otel-exporter-endpoint"]; exists {
 		otelExporterEndpoint = strings.TrimSpace(otelExporterEndpoint)
 		if otelExporterEndpoint != "" {
+			if err := validation.ValidateURI(otelExporterEndpoint); err != nil {
+				nl.Warn(l, err)
+				eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, err.Error())
+				return nil, err
+			}
 			cfgParams.MainOtelExporterEndpoint = otelExporterEndpoint
 		}
 	}

internal/configs/configmaps_test.go

@@ -1403,6 +1403,34 @@ func TestOpenTelemetryConfigurationSuccess(t *testing.T) {
 			expectedServiceName:         "nginx-ingress-controller:nginx",
 			msg:                         "endpoint set, minimal config",
 		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"otel-exporter-endpoint": "subdomain.goes.on.and.on.and.on.and.on.example.com",
+				},
+			},
+			expectedLoadModule:          true,
+			expectedExporterEndpoint:    "subdomain.goes.on.and.on.and.on.and.on.example.com",
+			expectedExporterHeaderName:  "",
+			expectedExporterHeaderValue: "",
+			expectedServiceName:         "",
+			expectedTraceInHTTP:         false,
+			msg:                         "endpoint set, complicated long subdomain",
+		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"otel-exporter-endpoint": "localhost:9933",
+				},
+			},
+			expectedLoadModule:          true,
+			expectedExporterEndpoint:    "localhost:9933",
+			expectedExporterHeaderName:  "",
+			expectedExporterHeaderValue: "",
+			expectedServiceName:         "",
+			expectedTraceInHTTP:         false,
+			msg:                         "endpoint set, hostname and port no scheme",
+		},
 		{
 			configMap: &v1.ConfigMap{
 				Data: map[string]string{
@@ -1595,6 +1623,76 @@ func TestOpenTelemetryConfigurationInvalid(t *testing.T) {
 			expectedTraceInHTTP:         true,
 			msg:                         "partially invalid, header name missing, trace in http set",
 		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"otel-exporter-endpoint": "something%invalid*30here",
+				},
+			},
+			expectedLoadModule:          false,
+			expectedExporterEndpoint:    "",
+			expectedExporterHeaderName:  "",
+			expectedExporterHeaderValue: "",
+			expectedServiceName:         "",
+			expectedTraceInHTTP:         false,
+			msg:                         "invalid, endpoint does not look like a host",
+		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"otel-exporter-endpoint": "localhost:0",
+				},
+			},
+			expectedLoadModule:          false,
+			expectedExporterEndpoint:    "",
+			expectedExporterHeaderName:  "",
+			expectedExporterHeaderValue: "",
+			expectedServiceName:         "",
+			expectedTraceInHTTP:         false,
+			msg:                         "invalid, port is outside of range down",
+		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"otel-exporter-endpoint": "localhost:99999",
+				},
+			},
+			expectedLoadModule:          false,
+			expectedExporterEndpoint:    "",
+			expectedExporterHeaderName:  "",
+			expectedExporterHeaderValue: "",
+			expectedServiceName:         "",
+			expectedTraceInHTTP:         false,
+			msg:                         "invalid, port is outside of range up",
+		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"otel-exporter-endpoint": "fe80::1",
+				},
+			},
+			expectedLoadModule:          false,
+			expectedExporterEndpoint:    "",
+			expectedExporterHeaderName:  "",
+			expectedExporterHeaderValue: "",
+			expectedServiceName:         "",
+			expectedTraceInHTTP:         false,
+			msg:                         "invalid, endpoint is an ipv6 address",
+		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"otel-exporter-endpoint": "thisisaverylongsubdomainthatexceedsatotalofsixtythreecharactersz.example.com",
+				},
+			},
+			expectedLoadModule:          false,
+			expectedExporterEndpoint:    "",
+			expectedExporterHeaderName:  "",
+			expectedExporterHeaderValue: "",
+			expectedServiceName:         "",
+			expectedTraceInHTTP:         false,
+			msg:                         "invalid, subdomain is more than 63 characters long",
+		},
 	}
 
 	isPlus := false

internal/validation/validation.go

@@ -1,12 +1,17 @@
 package validation
 
 import (
+	"errors"
 	"fmt"
+	"net/netip"
+	"net/url"
 	"regexp"
 	"strconv"
 	"strings"
 )
 
+const schemeSeparator = "://"
+
 var (
 	validDNSRegex      = regexp.MustCompile(`^(?:[A-Za-z0-9][A-Za-z0-9-]{1,62}\.)([A-Za-z0-9-]{1,63}\.)*[A-Za-z]{2,63}(?::\d{1,5})?$`)
 	validIPRegex       = regexp.MustCompile(`^(?:(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])\.){3}(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])(?::\d{1,5})?$`)
@@ -51,3 +56,125 @@
 	}
 	return fmt.Errorf("error parsing host: %s not a valid host", host)
 }
+
+// URIValidationOption defines a functional option pattern for configuring the
+// unexported uriValidator that gets used in ValidateURI.
+type URIValidationOption func(u *uriValidator)
+
+type uriValidator struct {
+	allowedSchemes map[string]struct{}
+	userAllowed    bool
+	defaultScheme  string
+}
+
+// WithAllowedSchemes configures a URIValidator with allowed URI schemes. By
+// default, http and https are the only schemes considered valid. This option
+// allows changing the allowed schemes.
+func WithAllowedSchemes(allowedSchemes ...string) URIValidationOption {
+	return func(cfg *uriValidator) {
+		schemes := make(map[string]struct{})
+		for _, scheme := range allowedSchemes {
+			schemes[scheme] = struct{}{}
+		}
+
+		cfg.allowedSchemes = schemes
+	}
+}
+
+// WithUserAllowed configures a URIValidator with a flag for whether user
+// information is allowed in the URI. Defaults to false. It is not recommended
+// to pass user information in a URL as it's generally considered to be unsafe.
+func WithUserAllowed(userAllowed bool) URIValidationOption {
+	return func(cfg *uriValidator) {
+		cfg.userAllowed = userAllowed
+	}
+}
+
+// WithDefaultScheme configures a URIValidator with a default scheme that
+// gets used if no scheme is present in the incoming URI. Defaults to "https".
+func WithDefaultScheme(defaultScheme string) URIValidationOption {
+	return func(cfg *uriValidator) {
+		cfg.defaultScheme = defaultScheme
+	}
+}
+
+// ValidateURI is a more robust extensible function to validate URIs. It
+// improved on ValidateHost as that one wasn't able to handle addresses that
+// start with a scheme.
+func ValidateURI(uri string, options ...URIValidationOption) error {
+	cfg := &uriValidator{
+		allowedSchemes: map[string]struct{}{
+			"http":  {},
+			"https": {},
+		},
+		userAllowed:   false,
+		defaultScheme: "https",
+	}
+
+	// Apply options to the configuration
+	for _, option := range options {
+		option(cfg)
+	}
+
+	// Check if the incoming uri is coming in with a scheme. At this point we'll
+	// assume that any uri that does have a scheme will also have the :// in it.
+	// If the uri does not have a scheme, let's add the default one so that
+	// url.Parse can deal with situations like "localhost:80".
+	if !strings.Contains(uri, schemeSeparator) {
+		uri = cfg.defaultScheme + schemeSeparator + uri
+	}
+
+	parsed, err := url.Parse(uri)
+	if err != nil {
+		return fmt.Errorf("error parsing uri: %w", err)
+	}
+
+	// Check whether the posted scheme is valid for the allowed list.
+	if _, ok := cfg.allowedSchemes[parsed.Scheme]; !ok {
+		return fmt.Errorf("scheme %s is not allowed", parsed.Scheme)
+	}
+
+	// Check whether the user:pass pattern is not allowed, but we have one.
+	if !cfg.userAllowed && parsed.User != nil {
+		return errors.New("user is not allowed")
+	}
+
+	// Check whether we're dealing with an IPV6 address.
+	checkIPv6 := parsed.Host
+	if strings.Contains(checkIPv6, "[") {
+		checkIPv6 = parsed.Hostname()
+	}
+
+	if ip, err := netip.ParseAddr(checkIPv6); err == nil && !ip.Is4() {
+		return fmt.Errorf("ipv6 addresses are not allowed")
+	}
+
+	// Check whether the ports posted are valid.
+	if parsed.Port() != "" {
+		// Turn the string port into an integer and check if it's in the correct
+		// range. The net.url.Parse does not check whether the port is the allowed
+		// value, only that it's syntactically correct. Similarly, the
+		// net.SplitHostPort function also doesn't check whether the value is
+		// correct.
+		numericPort, err := strconv.Atoi(parsed.Port())
+		if err != nil {
+			return fmt.Errorf("invalid port %s: %w", parsed.Port(), err)
+		}
+
+		if err = ValidatePort(numericPort); err != nil {
+			return fmt.Errorf("invalid port %s: %w", parsed.Port(), err)
+		}
+	}
+
+	// Check whether each part of the domain is not too long.
+	// This should really be octets
+	for _, part := range strings.Split(parsed.Hostname(), ".") {
+		// turn each part into a byte array to get a length of octets.
+		// max length of a subdomain is 63 octets per RFC 1035.
+		if len([]byte(part)) > 63 {
+			return fmt.Errorf("invalid hostname part %s, value must be between 1 and 63 octets", part)
+		}
+	}
+
+	return nil
+}

internal/validation/validation_test.go

@@ -95,3 +95,113 @@ func TestValidateHost(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateURI(t *testing.T) {
+	tests := []struct {
+		name    string
+		uri     string
+		options []URIValidationOption
+		wantErr bool
+	}{
+		{
+			name:    "simple uri with scheme",
+			uri:     "https://localhost:8080",
+			options: []URIValidationOption{},
+			wantErr: false,
+		},
+		{
+			name:    "simple uri without scheme",
+			uri:     "localhost:8080",
+			options: []URIValidationOption{},
+			wantErr: false,
+		},
+		{
+			name:    "uri with out of bounds port down",
+			uri:     "http://localhost:0",
+			options: []URIValidationOption{},
+			wantErr: true,
+		},
+		{
+			name:    "uri with out of bounds port up",
+			uri:     "http://localhost:65536",
+			options: []URIValidationOption{},
+			wantErr: true,
+		},
+		{
+			name:    "uri with bad port",
+			uri:     "http://localhost:abc",
+			options: []URIValidationOption{},
+			wantErr: true,
+		},
+		{
+			name: "uri with username and password and allowed",
+			uri:  "http://user:password@localhost",
+			options: []URIValidationOption{
+				WithUserAllowed(true),
+			},
+			wantErr: false,
+		},
+		{
+			name:    "uri with username and password and not allowed",
+			uri:     "http://user:password@localhost",
+			options: []URIValidationOption{},
+			wantErr: true,
+		},
+		{
+			name: "uri with http scheme but that's not allowed",
+			uri:  "http://localhost",
+			options: []URIValidationOption{
+				WithAllowedSchemes("https"),
+			},
+			wantErr: true,
+		},
+		{
+			name: "uri with https scheme but that's not allowed",
+			uri:  "https://localhost",
+			options: []URIValidationOption{
+				WithAllowedSchemes("http"),
+			},
+			wantErr: true,
+		},
+		{
+			name: "uri with no scheme, default set to https, not allowed",
+			uri:  "localhost",
+			options: []URIValidationOption{
+				WithDefaultScheme("https"),
+				WithAllowedSchemes("http"),
+			},
+			wantErr: true,
+		},
+		{
+			name:    "uri that is an ipv6 address with a port",
+			uri:     "https://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:17000",
+			options: []URIValidationOption{},
+			wantErr: true,
+		},
+		{
+			name:    "uri that is an ipv6 address without a port",
+			uri:     "https://2001:0db8:85a3:0000:0000:8a2e:0370:7334",
+			options: []URIValidationOption{},
+			wantErr: true,
+		},
+		{
+			name:    "uri that is a short ipv6 without port without scheme",
+			uri:     "fe80::1",
+			options: []URIValidationOption{},
+			wantErr: true,
+		},
+		{
+			name:    "uri that is a short ipv6 with a port without scheme",
+			uri:     "[fe80::1]:80",
+			options: []URIValidationOption{},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := ValidateURI(tt.uri, tt.options...); (err != nil) != tt.wantErr {
+				t.Errorf("ValidateURI() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}