From 83f880f6711e84e2c0b2098fcff0d283643c9bac Mon Sep 17 00:00:00 2001
From: Daniel <dhaavi@users.noreply.github.com>
Date: Wed, 14 Oct 2020 09:52:42 +0200
Subject: [PATCH] Implement review suggestions, simplify validation

---
 profile/endpoints/endpoint-domain.go | 69 +++++++++++++---------------
 profile/endpoints/endpoints_test.go  | 27 +++++++----
 2 files changed, 50 insertions(+), 46 deletions(-)

diff --git a/profile/endpoints/endpoint-domain.go b/profile/endpoints/endpoint-domain.go
index 4ca7f293..b02fc9a5 100644
--- a/profile/endpoints/endpoint-domain.go
+++ b/profile/endpoints/endpoint-domain.go
@@ -17,7 +17,7 @@ const (
 )
 
 var (
-	containsRegex = regexp.MustCompile(`^\*?[a-z0-9\.-]+\*$`)
+	allowedDomainChars = regexp.MustCompile(`^[a-z0-9\.-]+$`)
 )
 
 // EndpointDomain matches domains.
@@ -88,72 +88,65 @@ func (ep *EndpointDomain) String() string {
 	return ep.renderPPP(ep.OriginalValue)
 }
 
-func isValidEndpointDomain(value string) bool {
-	// Check for root domain
-	if value == "." {
-		return true
-	}
-
-	// Check for a "contains" value.
-	if containsRegex.MatchString(value) {
-		return true
-	}
-
-	// Remove special leading characters.
-	value = strings.TrimLeft(value, ".*")
-
-	// Check if value is now a valid domain.
-	return netutils.IsValidFqdn(value)
-}
-
 func parseTypeDomain(fields []string) (Endpoint, error) {
 	domain := fields[1]
-
 	ep := &EndpointDomain{
 		OriginalValue: domain,
 	}
 
-	// fix domain ending
+	// Fix domain ending.
 	switch domain[len(domain)-1] {
-	case '.':
-	case '*':
+	case '.', '*':
 	default:
 		domain += "."
 	}
 
-	// Check if domain is valid.
-	if !isValidEndpointDomain(domain) {
-		return nil, nil
-	}
-
-	// fix domain case
+	// Fix domain case.
 	domain = strings.ToLower(domain)
+	needValidFQDN := true
 
 	switch {
 	case strings.HasPrefix(domain, "*") && strings.HasSuffix(domain, "*"):
 		ep.MatchType = domainMatchTypeContains
-		ep.Domain = strings.Trim(domain, "*")
-		return ep.parsePPP(ep, fields)
+		ep.Domain = strings.TrimPrefix(domain, "*")
+		ep.Domain = strings.TrimSuffix(ep.Domain, "*")
+		needValidFQDN = false
 
 	case strings.HasSuffix(domain, "*"):
 		ep.MatchType = domainMatchTypePrefix
-		ep.Domain = strings.Trim(domain, "*")
-		return ep.parsePPP(ep, fields)
+		ep.Domain = strings.TrimSuffix(domain, "*")
+		needValidFQDN = false
+
+		// Prefix matching cannot be combined with zone matching
+		if strings.HasPrefix(ep.Domain, ".") {
+			return nil, nil
+		}
 
 	case strings.HasPrefix(domain, "*"):
 		ep.MatchType = domainMatchTypeSuffix
-		ep.Domain = strings.Trim(domain, "*")
-		return ep.parsePPP(ep, fields)
+		ep.Domain = strings.TrimPrefix(domain, "*")
+		needValidFQDN = false
 
 	case strings.HasPrefix(domain, "."):
 		ep.MatchType = domainMatchTypeZone
-		ep.Domain = strings.TrimLeft(domain, ".")
+		ep.Domain = strings.TrimPrefix(domain, ".")
 		ep.DomainZone = "." + ep.Domain
-		return ep.parsePPP(ep, fields)
 
 	default:
 		ep.MatchType = domainMatchTypeExact
 		ep.Domain = domain
-		return ep.parsePPP(ep, fields)
 	}
+
+	// Validate domain "content".
+	switch {
+	case needValidFQDN && !netutils.IsValidFqdn(ep.Domain):
+		return nil, nil
+	case !needValidFQDN && !allowedDomainChars.MatchString(ep.Domain):
+		return nil, nil
+	case strings.Contains(ep.Domain, ".."):
+		// The above regex does not catch double dots.
+		return nil, nil
+	}
+
+	return ep.parsePPP(ep, fields)
 }
diff --git a/profile/endpoints/endpoints_test.go b/profile/endpoints/endpoints_test.go
index 2584dfe8..531544d8 100644
--- a/profile/endpoints/endpoints_test.go
+++ b/profile/endpoints/endpoints_test.go
@@ -29,18 +29,29 @@ func testEndpointMatch(t *testing.T, ep Endpoint, entity *intel.Entity, expected
 	}
 }
 
-func testFormat(t *testing.T, endpoint string) {
+func testFormat(t *testing.T, endpoint string, shouldSucceed bool) {
 	_, err := parseEndpoint(endpoint)
-	assert.NoError(t, err)
+	if shouldSucceed {
+		assert.NoError(t, err)
+	} else {
+		assert.Error(t, err)
+	}
 }
 
 func TestEndpointFormat(t *testing.T) {
-	testFormat(t, "+ .")
-	testFormat(t, "+ .at")
-	testFormat(t, "+ .at.")
-	testFormat(t, "+ 1.at")
-	testFormat(t, "+ 1.at.")
-	testFormat(t, "+ 1.f.ix.de.")
+	testFormat(t, "+ .", false)
+	testFormat(t, "+ .at", true)
+	testFormat(t, "+ .at.", true)
+	testFormat(t, "+ 1.at", true)
+	testFormat(t, "+ 1.at.", true)
+	testFormat(t, "+ 1.f.ix.de.", true)
+	testFormat(t, "+ *contains*", true)
+	testFormat(t, "+ *has.suffix", true)
+	testFormat(t, "+ *.has.suffix", true)
+	testFormat(t, "+ *has.prefix*", true)
+	testFormat(t, "+ *has.prefix.*", true)
+	testFormat(t, "+ .sub.and.prefix.*", false)
+	testFormat(t, "+ *.sub..and.prefix.*", false)
 }
 
 func TestEndpointMatching(t *testing.T) {