From b5e80e1a34883d679de8a03a89a79f9818bcea32 Mon Sep 17 00:00:00 2001 From: Crystal Chun Date: Thu, 1 Feb 2024 00:37:23 -0800 Subject: [PATCH] OCPBUGS-27376: Allow domain names starting with a number (#5914) (#5938) https://issues.redhat.com/browse/OCPBUGS-27376 RFC 1034 initially stated domain names should only start with a letter, but that was later revised by RFC 1123. Updates the domain regex to allow domains to start with a number. Also adds a check to prevent dotted decimal domains. Includes changes that also check for the length of domains. RFC 1123 also states that a label must be no more than 63 characters and a full domain must be no more than 255 characters. --- pkg/validations/validations.go | 48 +++++++++++-- pkg/validations/validations_test.go | 106 +++++++++++++++++++++++++--- subsystem/cluster_test.go | 5 ++ 3 files changed, 142 insertions(+), 17 deletions(-) diff --git a/pkg/validations/validations.go b/pkg/validations/validations.go index f0cc2da00f1..20b5811a279 100644 --- a/pkg/validations/validations.go +++ b/pkg/validations/validations.go @@ -1,6 +1,8 @@ package validations import ( + "crypto/x509" + "encoding/base64" "fmt" "net" "net/http" @@ -14,8 +16,9 @@ import ( ) const ( - baseDomainRegex = "^([a-z0-9]+(-[a-z0-9]+)*)+$" - dnsNameRegex = "^([a-z0-9]+(-[a-z0-9]+)*[.])+[a-z]{2,}$" + baseDomainRegex = `^[a-z\d]([\-]*[a-z\d]+)+$` + dnsNameRegex = `^([a-z\d]([\-]*[a-z\d]+)*\.)+[a-z\d]+([\-]*[a-z\d]+)+$` + wildCardDomainRegex = `^(validateNoWildcardDNS\.).+\.?$` hostnameRegex = `^[a-z0-9][a-z0-9\-\.]{0,61}[a-z0-9]$` installerArgsValuesRegex = `^[A-Za-z0-9@!#$%*()_+-=//.,";':{}\[\]]+$` ) @@ -43,19 +46,28 @@ func ValidateInstallerArgs(args []string) error { } func ValidateDomainNameFormat(dnsDomainName string) (int32, error) { - matched, err := regexp.MatchString(baseDomainRegex, dnsDomainName) + domainName := dnsDomainName + wildCardMatched, wildCardMatchErr := regexp.MatchString(wildCardDomainRegex, dnsDomainName) + if wildCardMatchErr == nil && wildCardMatched { + trimmedDomain := strings.TrimPrefix(dnsDomainName, "validateNoWildcardDNS.") + domainName = strings.TrimSuffix(trimmedDomain, ".") + } + matched, err := regexp.MatchString(baseDomainRegex, domainName) if err != nil { return http.StatusInternalServerError, errors.Wrapf(err, "Single DNS base domain validation for %s", dnsDomainName) } - if matched { + if matched && len(domainName) > 1 && len(domainName) < 63 { return 0, nil } - matched, err = regexp.MatchString(dnsNameRegex, dnsDomainName) + matched, err = regexp.MatchString(dnsNameRegex, domainName) if err != nil { return http.StatusInternalServerError, errors.Wrapf(err, "DNS name validation for %s", dnsDomainName) } - if !matched { - return http.StatusBadRequest, errors.Errorf("DNS format mismatch: %s domain name is not valid", dnsDomainName) + + if !matched || isDottedDecimalDomain(domainName) || len(domainName) > 255 { + return http.StatusBadRequest, errors.Errorf( + "DNS format mismatch: %s domain name is not valid. Must match regex [%s], be no more than 255 characters, and not be in dotted decimal format (##.##.##.##)", + dnsDomainName, dnsNameRegex) } return 0, nil } @@ -173,3 +185,25 @@ func IsValidTag(tag string) bool { tagRegex := `^\w+( \w+)*$` // word characters and whitespace return regexp.MustCompile(tagRegex).MatchString(tag) } + +// ValidateCaCertificate ensures the specified base64 CA certificate +// is valid by trying to decode and parse it. +func ValidateCaCertificate(certificate string) error { + decodedCaCert, err := base64.StdEncoding.DecodeString(certificate) + if err != nil { + return errors.Wrap(err, "failed to decode certificate") + } + caCertPool := x509.NewCertPool() + if ok := caCertPool.AppendCertsFromPEM(decodedCaCert); !ok { + return errors.Errorf("unable to parse certificate") + } + + return nil +} + +// RFC 1123 (https://datatracker.ietf.org/doc/html/rfc1123#page-13) +// states that domains cannot resemble the format ##.##.##.## +func isDottedDecimalDomain(domain string) bool { + regex := `([\d]+\.){3}[\d]+` + return regexp.MustCompile(regex).MatchString(domain) +} diff --git a/pkg/validations/validations_test.go b/pkg/validations/validations_test.go index bfa3d5f49e3..9b53a377f8e 100644 --- a/pkg/validations/validations_test.go +++ b/pkg/validations/validations_test.go @@ -161,55 +161,141 @@ var _ = Describe("URL validations", func() { var _ = Describe("dns name", func() { hugeDomainName := "DNSnamescancontainonlyalphabeticalcharactersa-znumericcharacters0-9theminussign-andtheperiod" - fqnHugeDomain := hugeDomainName + ".com" + fqnHugeDomain := hugeDomainName + "." + hugeDomainName + "." + hugeDomainName + ".com" tests := []struct { domainName string + reason string valid bool }{ { domainName: hugeDomainName, + reason: "base domain: should fail - name exceeds max character limit of 63 characters", valid: false, }, { domainName: fqnHugeDomain, + reason: "full domain: should fail - name exceeds max character limit of 255 characters", valid: false, }, { - domainName: "a.com", + domainName: "a", + reason: "base domain: should fail - name requires at least two characters", + valid: false, + }, + { + domainName: "-", + reason: "base domain: should fail - requires alphanumerical characters and not just a dash", + valid: false, + }, + { + domainName: "a-", + reason: "base domain: should fail - names can only end in alphanumerical characters and not a dash", + valid: false, + }, + { + domainName: "a.c", + reason: "full domain: should fail - top level domain must be at least two characters", + valid: false, + }, + { + domainName: "-aaa.com", + reason: "full domain: should fail - labels can only start with alphanumerical characters and not a dash", + valid: false, + }, + { + domainName: "aaa-.com", + reason: "full domain: should fail - labels can only end in an alphanumerical character and not a dash", + valid: false, + }, + { + domainName: "11.11.11.11", + reason: "full domain: should fail - dotted decimal domains (##.##.##.##) are not allowed", + valid: false, + }, + { + domainName: "1c", + reason: "base domain: should pass - two character domain starting with a number", valid: true, }, { - domainName: "a", + domainName: "1-c", + reason: "base domain: should pass - minimum of two characters and starts and ends with alphanumerical characters", + valid: true, + }, + { + domainName: "1--c", + reason: "base domain: should pass - testing multiple consecutive dashes", + valid: true, + }, + { + domainName: "exam-ple", + reason: "base domain: should pass - multiple characters on either side of dash", + valid: true, + }, + { + domainName: "exam--ple", + reason: "base domain: should pass - multiple characters on either side of multiple consecutive dashes", valid: true, }, { domainName: "co", + reason: "base domain: should pass - regular two character domain", valid: true, }, { - domainName: "aaa", + domainName: "a.com", + reason: "full domain: should pass - just one character as the first subdomain label", valid: true, }, { domainName: "abc.def", + reason: "full domain: should pass - regular domain", valid: true, }, { - domainName: "-aaa.com", - valid: false, + domainName: "a-aa.com", + reason: "full domain: should pass - single dash within label", + valid: true, }, { - domainName: "aaa-.com", - valid: false, + domainName: "a--aa.com", + reason: "full domain: should pass - multiple consecutive dashes within label", + valid: true, }, { - domainName: "a-aa.com", + domainName: "0-example.com0", + reason: "full domain: should pass - starting and ending with a number", + valid: true, + }, + { + domainName: "example-example-example.com", + reason: "full domain: should pass - multiple dashes present within a label", + valid: true, + }, + { + domainName: "example.example-example.com", + reason: "full domain: should pass - dash within a different level subdomain label", + valid: true, + }, + { + domainName: "exam--ple.example--example.com", + reason: "full domain: should pass - multiple consecutive dashes within multiple subdomain labels", + valid: true, + }, + { + domainName: "validateNoWildcardDNS.test.com", + reason: "full domain: should pass - allows wildcard dns test", + valid: true, + }, + { + domainName: "validateNoWildcardDNS.test.com.", + reason: "full domain: should pass - allows wildcard dns test (format with period at the end)", valid: true, }, } for _, t := range tests { t := t - It(fmt.Sprintf("Domain name \"%s\"", t.domainName), func() { + It(fmt.Sprintf("Domain name \"%s\" - testing: \"%s\"", t.domainName, t.reason), func() { _, err := ValidateDomainNameFormat(t.domainName) if t.valid { Expect(err).ToNot(HaveOccurred()) diff --git a/subsystem/cluster_test.go b/subsystem/cluster_test.go index 7760b125228..3c0cc70d1d3 100644 --- a/subsystem/cluster_test.go +++ b/subsystem/cluster_test.go @@ -966,6 +966,11 @@ var _ = Describe("Validate BaseDNSDomain when creating a cluster", func() { BaseDNSDomain: "-example.com", ShouldThrow: true, }, + { + It: "V2RegisterCluster should not throw an error. BaseDNSDomain='1-example.com', valid DNS", + BaseDNSDomain: "1-example.com", + ShouldThrow: false, + }, { It: "V2RegisterCluster should not throw an error. BaseDNSDomain='example.com', valid DNS", BaseDNSDomain: "example.com",