diff --git a/internal/configs/configmaps.go b/internal/configs/configmaps.go index d0bf1bb3b5..3c8436e175 100644 --- a/internal/configs/configmaps.go +++ b/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 } } diff --git a/internal/configs/configmaps_test.go b/internal/configs/configmaps_test.go index 397e6da9ca..0ddf61c9e2 100644 --- a/internal/configs/configmaps_test.go +++ b/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 diff --git a/internal/validation/validation.go b/internal/validation/validation.go index 2d16b89def..e62086e469 100644 --- a/internal/validation/validation.go +++ b/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 @@ func ValidateHost(host string) error { } 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 +} diff --git a/internal/validation/validation_test.go b/internal/validation/validation_test.go index 4322aece61..ac2869651d 100644 --- a/internal/validation/validation_test.go +++ b/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) + } + }) + } +}