From edd7b0710c7b0534a5141dba0bc46c2ad6f5140d Mon Sep 17 00:00:00 2001 From: Julilla Paul <18239431+julillae@users.noreply.github.com> Date: Wed, 13 Nov 2024 11:18:38 -0800 Subject: [PATCH 1/5] chore: add validation for MX and SRV records --- provider/pdns/pdns.go | 52 ++++++++++++ provider/pdns/pdns_test.go | 158 +++++++++++++++++++++++++++++++++++++ 2 files changed, 210 insertions(+) diff --git a/provider/pdns/pdns.go b/provider/pdns/pdns.go index c0180725a..ba135e07b 100644 --- a/provider/pdns/pdns.go +++ b/provider/pdns/pdns.go @@ -27,6 +27,7 @@ import ( "net" "net/http" "sort" + "strconv" "strings" "time" @@ -433,6 +434,57 @@ func (p *PDNSProvider) Records(ctx context.Context) (endpoints []*endpoint.Endpo return endpoints, nil } +// Check endpoint if is it properly formatted according to RFC standards +func checkEndpoint(endpoint endpoint.Endpoint) bool { + switch recordType := endpoint.RecordType; recordType { + case "MX": + for _, target := range endpoint.Targets { + // MX records must have a preference value to indicate priority, e.g. "10 example.com" + // as per https://www.rfc-editor.org/rfc/rfc974.txt + targetParts := strings.Fields(strings.TrimSpace(target)) + if len(targetParts) != 2 { + return false + } + + preferenceRaw := targetParts[0] + _, err := strconv.ParseInt(preferenceRaw, 10, 32) + if err != nil { + return false + } + } + case "SRV": + for _, target := range endpoint.Targets { + // SRV records must have a priority, weight, and port value, e.g. "10 5 5060 example.com" + // as per https://www.rfc-editor.org/rfc/rfc2782.txt + targetParts := strings.Fields(strings.TrimSpace(target)) + if len(targetParts) != 4 { + return false + } + + for _, part := range targetParts[:3] { + _, err := strconv.ParseInt(part, 10, 32) + if err != nil { + return false + } + } + } + } + return true +} + +// AdjustEndpoints performs checks on the provided endpoints and will skip any potentially failing changes. +func (p *PDNSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { + var validEndpoints []*endpoint.Endpoint + for i := 0; i < len(endpoints); i++ { + if !checkEndpoint(*endpoints[i]) { + log.Warnf("Ignoring Endpoint because of invalid %v record formatting: {Target: '%v'}", endpoints[i].RecordType, endpoints[i].Targets) + continue + } + validEndpoints = append(validEndpoints, endpoints[i]) + } + return validEndpoints, nil +} + // ApplyChanges takes a list of changes (endpoints) and updates the PDNS server // by sending the correct HTTP PATCH requests to a matching zone func (p *PDNSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { diff --git a/provider/pdns/pdns_test.go b/provider/pdns/pdns_test.go index 47df67cb1..febc4ef37 100644 --- a/provider/pdns/pdns_test.go +++ b/provider/pdns/pdns_test.go @@ -161,6 +161,24 @@ var ( endpoint.NewEndpointWithTTL("example.com", endpoint.RecordTypeA, endpoint.TTL(300), "8.8.8.8", "8.8.4.4", "4.4.4.4"), } + endpointsMXRecord = []*endpoint.Endpoint{ + endpoint.NewEndpointWithTTL("mail.example.com", endpoint.RecordTypeMX, endpoint.TTL(300), "10 example.com"), + } + + endpointsMXRecordInvalidFormatTooManyArgs = []*endpoint.Endpoint{ + endpoint.NewEndpointWithTTL("mail.example.com", endpoint.RecordTypeMX, endpoint.TTL(300), "10 example.com abc"), + } + + endpointsMultipleMXRecordsWithSingleInvalid = []*endpoint.Endpoint{ + endpoint.NewEndpointWithTTL("mail.example.com", endpoint.RecordTypeMX, endpoint.TTL(300), "abc example.com"), + endpoint.NewEndpointWithTTL("mail.example.com", endpoint.RecordTypeMX, endpoint.TTL(300), "20 backup.example.com"), + } + + endpointsMultipleInvalidMXRecords = []*endpoint.Endpoint{ + endpoint.NewEndpointWithTTL("mail.example.com", endpoint.RecordTypeMX, endpoint.TTL(300), "example.com"), + endpoint.NewEndpointWithTTL("mail.example.com", endpoint.RecordTypeMX, endpoint.TTL(300), "backup.example.com"), + } + endpointsMixedRecords = []*endpoint.Endpoint{ endpoint.NewEndpointWithTTL("cname.example.com", endpoint.RecordTypeCNAME, endpoint.TTL(300), "example.com"), endpoint.NewEndpointWithTTL("example.com", endpoint.RecordTypeTXT, endpoint.TTL(300), "'would smell as sweet'"), @@ -1116,6 +1134,146 @@ func (suite *NewPDNSProviderTestSuite) TestPDNSClientPartitionZones() { assert.Equal(suite.T(), partitionResultResidualSingleFilter, residualZones) } +func (suite *NewPDNSProviderTestSuite) TestPDNScheckEndpoint() { + tests := []struct { + description string + endpoint endpoint.Endpoint + expected bool + }{ + { + description: "Valid MX record target", + endpoint: endpoint.Endpoint{ + DNSName: "example.com", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"10 example.com"}, + }, + expected: true, + }, + { + description: "Valid MX record with multiple targets", + endpoint: endpoint.Endpoint{ + DNSName: "example.com", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"10 example.com", "20 backup.example.com"}, + }, + expected: true, + }, + { + description: "MX record with valid and invalid targets", + endpoint: endpoint.Endpoint{ + DNSName: "example.com", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"example.com", "backup.example.com"}, + }, + expected: false, + }, + { + description: "Invalid MX record with missing priority value", + endpoint: endpoint.Endpoint{ + DNSName: "example.com", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"example.com"}, + }, + expected: false, + }, + { + description: "Invalid MX record with too many arguments", + endpoint: endpoint.Endpoint{ + DNSName: "example.com", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"10 example.com abc"}, + }, + expected: false, + }, + { + description: "Invalid MX record with non-integer priority", + endpoint: endpoint.Endpoint{ + DNSName: "example.com", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"abc example.com"}, + }, + expected: false, + }, + { + description: "Valid SRV record target", + endpoint: endpoint.Endpoint{ + DNSName: "_service._tls.example.com", + RecordType: endpoint.RecordTypeSRV, + Targets: endpoint.Targets{"10 20 5060 service.example.com"}, + }, + expected: true, + }, + { + description: "Invalid SRV record with missing part", + endpoint: endpoint.Endpoint{ + DNSName: "_service._tls.example.com", + RecordType: endpoint.RecordTypeSRV, + Targets: endpoint.Targets{"10 20 5060"}, + }, + expected: false, + }, + { + description: "Invalid SRV record with non-integer part", + endpoint: endpoint.Endpoint{ + DNSName: "_service._tls.example.com", + RecordType: endpoint.RecordTypeSRV, + Targets: endpoint.Targets{"10 20 abc service.example.com"}, + }, + expected: false, + }, + } + + for _, tt := range tests { + actual := checkEndpoint(tt.endpoint) + assert.Equal(suite.T(), tt.expected, actual) + } +} + +// Validate whether invalid endpoints are removed by AdjustEndpoints +func (suite *NewPDNSProviderTestSuite) TestPDNSAdjustEndpoints() { + // Function definition: AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint + + // Create a new provider to run tests against + p := &PDNSProvider{} + + tests := []struct { + description string + endpoints []*endpoint.Endpoint + expected []*endpoint.Endpoint + }{ + { + description: "Valid MX endpoint is not removed", + endpoints: endpointsMXRecord, + expected: []*endpoint.Endpoint{ + endpoint.NewEndpointWithTTL("mail.example.com", endpoint.RecordTypeMX, endpoint.TTL(300), "10 example.com"), + }, + }, + { + description: "Invalid MX endpoint with too many arguments is removed", + endpoints: endpointsMXRecordInvalidFormatTooManyArgs, + expected: []*endpoint.Endpoint([]*endpoint.Endpoint(nil)), + }, + { + description: "Invalid MX endpoint is removed among valid endpoints", + endpoints: endpointsMultipleMXRecordsWithSingleInvalid, + expected: []*endpoint.Endpoint{ + endpoint.NewEndpointWithTTL("mail.example.com", endpoint.RecordTypeMX, endpoint.TTL(300), "20 backup.example.com"), + }, + }, + { + description: "Multiple invalid MX endpoints are removed", + endpoints: endpointsMultipleInvalidMXRecords, + expected: []*endpoint.Endpoint([]*endpoint.Endpoint(nil)), + }, + } + + for _, tt := range tests { + actual, err := p.AdjustEndpoints(tt.endpoints) + assert.Nil(suite.T(), err) + assert.Equal(suite.T(), tt.expected, actual) + } +} + func TestNewPDNSProviderTestSuite(t *testing.T) { suite.Run(t, new(NewPDNSProviderTestSuite)) } From ad9f30058949c4d2725175dd41131ae0c114ecee Mon Sep 17 00:00:00 2001 From: Julilla Paul <18239431+julillae@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:46:57 -0800 Subject: [PATCH 2/5] chore: move validation to endpoint --- endpoint/endpoint.go | 39 +++++++++++++++ endpoint/endpoint_test.go | 97 ++++++++++++++++++++++++++++++++++++++ provider/pdns/pdns.go | 41 +--------------- provider/pdns/pdns_test.go | 95 ------------------------------------- 4 files changed, 137 insertions(+), 135 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 1bd2d54fb..ef11bb8d5 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -20,6 +20,7 @@ import ( "fmt" "net/netip" "sort" + "strconv" "strings" log "github.com/sirupsen/logrus" @@ -396,3 +397,41 @@ func RemoveDuplicates(endpoints []*Endpoint) []*Endpoint { return result } + +// Check endpoint if is it properly formatted according to RFC standards +func CheckEndpoint(endpoint Endpoint) bool { + switch recordType := endpoint.RecordType; recordType { + case "MX": + for _, target := range endpoint.Targets { + // MX records must have a preference value to indicate priority, e.g. "10 example.com" + // as per https://www.rfc-editor.org/rfc/rfc974.txt + targetParts := strings.Fields(strings.TrimSpace(target)) + if len(targetParts) != 2 { + return false + } + + preferenceRaw := targetParts[0] + _, err := strconv.ParseInt(preferenceRaw, 10, 32) + if err != nil { + return false + } + } + case "SRV": + for _, target := range endpoint.Targets { + // SRV records must have a priority, weight, and port value, e.g. "10 5 5060 example.com" + // as per https://www.rfc-editor.org/rfc/rfc2782.txt + targetParts := strings.Fields(strings.TrimSpace(target)) + if len(targetParts) != 4 { + return false + } + + for _, part := range targetParts[:3] { + _, err := strconv.ParseInt(part, 10, 32) + if err != nil { + return false + } + } + } + } + return true +} diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index cd38208e4..1a9482cc0 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -19,6 +19,8 @@ package endpoint import ( "reflect" "testing" + + "github.com/stretchr/testify/assert" ) func TestNewEndpoint(t *testing.T) { @@ -441,3 +443,98 @@ func TestDuplicatedEndpointsWithOverlappingZones(t *testing.T) { }) } } + +func TestPDNScheckEndpoint(t *testing.T) { + tests := []struct { + description string + endpoint Endpoint + expected bool + }{ + { + description: "Valid MX record target", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"10 example.com"}, + }, + expected: true, + }, + { + description: "Valid MX record with multiple targets", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"10 example.com", "20 backup.example.com"}, + }, + expected: true, + }, + { + description: "MX record with valid and invalid targets", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"example.com", "backup.example.com"}, + }, + expected: false, + }, + { + description: "Invalid MX record with missing priority value", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"example.com"}, + }, + expected: false, + }, + { + description: "Invalid MX record with too many arguments", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"10 example.com abc"}, + }, + expected: false, + }, + { + description: "Invalid MX record with non-integer priority", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"abc example.com"}, + }, + expected: false, + }, + { + description: "Valid SRV record target", + endpoint: Endpoint{ + DNSName: "_service._tls.example.com", + RecordType: RecordTypeSRV, + Targets: Targets{"10 20 5060 service.example.com"}, + }, + expected: true, + }, + { + description: "Invalid SRV record with missing part", + endpoint: Endpoint{ + DNSName: "_service._tls.example.com", + RecordType: RecordTypeSRV, + Targets: Targets{"10 20 5060"}, + }, + expected: false, + }, + { + description: "Invalid SRV record with non-integer part", + endpoint: Endpoint{ + DNSName: "_service._tls.example.com", + RecordType: RecordTypeSRV, + Targets: Targets{"10 20 abc service.example.com"}, + }, + expected: false, + }, + } + + for _, tt := range tests { + actual := CheckEndpoint(tt.endpoint) + assert.Equal(t, tt.expected, actual) + } +} diff --git a/provider/pdns/pdns.go b/provider/pdns/pdns.go index ba135e07b..53092cc83 100644 --- a/provider/pdns/pdns.go +++ b/provider/pdns/pdns.go @@ -27,7 +27,6 @@ import ( "net" "net/http" "sort" - "strconv" "strings" "time" @@ -434,49 +433,11 @@ func (p *PDNSProvider) Records(ctx context.Context) (endpoints []*endpoint.Endpo return endpoints, nil } -// Check endpoint if is it properly formatted according to RFC standards -func checkEndpoint(endpoint endpoint.Endpoint) bool { - switch recordType := endpoint.RecordType; recordType { - case "MX": - for _, target := range endpoint.Targets { - // MX records must have a preference value to indicate priority, e.g. "10 example.com" - // as per https://www.rfc-editor.org/rfc/rfc974.txt - targetParts := strings.Fields(strings.TrimSpace(target)) - if len(targetParts) != 2 { - return false - } - - preferenceRaw := targetParts[0] - _, err := strconv.ParseInt(preferenceRaw, 10, 32) - if err != nil { - return false - } - } - case "SRV": - for _, target := range endpoint.Targets { - // SRV records must have a priority, weight, and port value, e.g. "10 5 5060 example.com" - // as per https://www.rfc-editor.org/rfc/rfc2782.txt - targetParts := strings.Fields(strings.TrimSpace(target)) - if len(targetParts) != 4 { - return false - } - - for _, part := range targetParts[:3] { - _, err := strconv.ParseInt(part, 10, 32) - if err != nil { - return false - } - } - } - } - return true -} - // AdjustEndpoints performs checks on the provided endpoints and will skip any potentially failing changes. func (p *PDNSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { var validEndpoints []*endpoint.Endpoint for i := 0; i < len(endpoints); i++ { - if !checkEndpoint(*endpoints[i]) { + if !endpoint.CheckEndpoint(*endpoints[i]) { log.Warnf("Ignoring Endpoint because of invalid %v record formatting: {Target: '%v'}", endpoints[i].RecordType, endpoints[i].Targets) continue } diff --git a/provider/pdns/pdns_test.go b/provider/pdns/pdns_test.go index febc4ef37..773d4897c 100644 --- a/provider/pdns/pdns_test.go +++ b/provider/pdns/pdns_test.go @@ -1134,101 +1134,6 @@ func (suite *NewPDNSProviderTestSuite) TestPDNSClientPartitionZones() { assert.Equal(suite.T(), partitionResultResidualSingleFilter, residualZones) } -func (suite *NewPDNSProviderTestSuite) TestPDNScheckEndpoint() { - tests := []struct { - description string - endpoint endpoint.Endpoint - expected bool - }{ - { - description: "Valid MX record target", - endpoint: endpoint.Endpoint{ - DNSName: "example.com", - RecordType: endpoint.RecordTypeMX, - Targets: endpoint.Targets{"10 example.com"}, - }, - expected: true, - }, - { - description: "Valid MX record with multiple targets", - endpoint: endpoint.Endpoint{ - DNSName: "example.com", - RecordType: endpoint.RecordTypeMX, - Targets: endpoint.Targets{"10 example.com", "20 backup.example.com"}, - }, - expected: true, - }, - { - description: "MX record with valid and invalid targets", - endpoint: endpoint.Endpoint{ - DNSName: "example.com", - RecordType: endpoint.RecordTypeMX, - Targets: endpoint.Targets{"example.com", "backup.example.com"}, - }, - expected: false, - }, - { - description: "Invalid MX record with missing priority value", - endpoint: endpoint.Endpoint{ - DNSName: "example.com", - RecordType: endpoint.RecordTypeMX, - Targets: endpoint.Targets{"example.com"}, - }, - expected: false, - }, - { - description: "Invalid MX record with too many arguments", - endpoint: endpoint.Endpoint{ - DNSName: "example.com", - RecordType: endpoint.RecordTypeMX, - Targets: endpoint.Targets{"10 example.com abc"}, - }, - expected: false, - }, - { - description: "Invalid MX record with non-integer priority", - endpoint: endpoint.Endpoint{ - DNSName: "example.com", - RecordType: endpoint.RecordTypeMX, - Targets: endpoint.Targets{"abc example.com"}, - }, - expected: false, - }, - { - description: "Valid SRV record target", - endpoint: endpoint.Endpoint{ - DNSName: "_service._tls.example.com", - RecordType: endpoint.RecordTypeSRV, - Targets: endpoint.Targets{"10 20 5060 service.example.com"}, - }, - expected: true, - }, - { - description: "Invalid SRV record with missing part", - endpoint: endpoint.Endpoint{ - DNSName: "_service._tls.example.com", - RecordType: endpoint.RecordTypeSRV, - Targets: endpoint.Targets{"10 20 5060"}, - }, - expected: false, - }, - { - description: "Invalid SRV record with non-integer part", - endpoint: endpoint.Endpoint{ - DNSName: "_service._tls.example.com", - RecordType: endpoint.RecordTypeSRV, - Targets: endpoint.Targets{"10 20 abc service.example.com"}, - }, - expected: false, - }, - } - - for _, tt := range tests { - actual := checkEndpoint(tt.endpoint) - assert.Equal(suite.T(), tt.expected, actual) - } -} - // Validate whether invalid endpoints are removed by AdjustEndpoints func (suite *NewPDNSProviderTestSuite) TestPDNSAdjustEndpoints() { // Function definition: AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint From 10f4a261847b1d4564f70a10da5f8bde6bab0637 Mon Sep 17 00:00:00 2001 From: Julilla Paul <18239431+julillae@users.noreply.github.com> Date: Mon, 10 Feb 2025 11:20:39 -0800 Subject: [PATCH 3/5] chore: make CheckEndpoint an Endpoint type method --- endpoint/endpoint.go | 8 ++++---- endpoint/endpoint_test.go | 2 +- provider/pdns/pdns.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index ef11bb8d5..82ce58c89 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -399,10 +399,10 @@ func RemoveDuplicates(endpoints []*Endpoint) []*Endpoint { } // Check endpoint if is it properly formatted according to RFC standards -func CheckEndpoint(endpoint Endpoint) bool { - switch recordType := endpoint.RecordType; recordType { +func (e *Endpoint) CheckEndpoint() bool { + switch recordType := e.RecordType; recordType { case "MX": - for _, target := range endpoint.Targets { + for _, target := range e.Targets { // MX records must have a preference value to indicate priority, e.g. "10 example.com" // as per https://www.rfc-editor.org/rfc/rfc974.txt targetParts := strings.Fields(strings.TrimSpace(target)) @@ -417,7 +417,7 @@ func CheckEndpoint(endpoint Endpoint) bool { } } case "SRV": - for _, target := range endpoint.Targets { + for _, target := range e.Targets { // SRV records must have a priority, weight, and port value, e.g. "10 5 5060 example.com" // as per https://www.rfc-editor.org/rfc/rfc2782.txt targetParts := strings.Fields(strings.TrimSpace(target)) diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index 1a9482cc0..6c805ecf0 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -534,7 +534,7 @@ func TestPDNScheckEndpoint(t *testing.T) { } for _, tt := range tests { - actual := CheckEndpoint(tt.endpoint) + actual := tt.endpoint.CheckEndpoint() assert.Equal(t, tt.expected, actual) } } diff --git a/provider/pdns/pdns.go b/provider/pdns/pdns.go index 53092cc83..a37787f8a 100644 --- a/provider/pdns/pdns.go +++ b/provider/pdns/pdns.go @@ -437,7 +437,7 @@ func (p *PDNSProvider) Records(ctx context.Context) (endpoints []*endpoint.Endpo func (p *PDNSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { var validEndpoints []*endpoint.Endpoint for i := 0; i < len(endpoints); i++ { - if !endpoint.CheckEndpoint(*endpoints[i]) { + if !endpoints[i].CheckEndpoint() { log.Warnf("Ignoring Endpoint because of invalid %v record formatting: {Target: '%v'}", endpoints[i].RecordType, endpoints[i].Targets) continue } From 3b2453bc1aeaac0d91d034b377e423684419cdca Mon Sep 17 00:00:00 2001 From: Julilla Paul <18239431+julillae@users.noreply.github.com> Date: Mon, 10 Feb 2025 11:35:07 -0800 Subject: [PATCH 4/5] chore: add debug logs and make integer parsing strict --- endpoint/endpoint.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 82ce58c89..cdd070de4 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -401,7 +401,7 @@ func RemoveDuplicates(endpoints []*Endpoint) []*Endpoint { // Check endpoint if is it properly formatted according to RFC standards func (e *Endpoint) CheckEndpoint() bool { switch recordType := e.RecordType; recordType { - case "MX": + case RecordTypeMX: for _, target := range e.Targets { // MX records must have a preference value to indicate priority, e.g. "10 example.com" // as per https://www.rfc-editor.org/rfc/rfc974.txt @@ -411,12 +411,13 @@ func (e *Endpoint) CheckEndpoint() bool { } preferenceRaw := targetParts[0] - _, err := strconv.ParseInt(preferenceRaw, 10, 32) + _, err := strconv.ParseUint(preferenceRaw, 10, 16) if err != nil { + log.Debugf("Invalid %s record target: %s. Invalid integer value in target.", recordType, target) return false } } - case "SRV": + case RecordTypeSRV: for _, target := range e.Targets { // SRV records must have a priority, weight, and port value, e.g. "10 5 5060 example.com" // as per https://www.rfc-editor.org/rfc/rfc2782.txt @@ -426,8 +427,9 @@ func (e *Endpoint) CheckEndpoint() bool { } for _, part := range targetParts[:3] { - _, err := strconv.ParseInt(part, 10, 32) + _, err := strconv.ParseUint(part, 10, 16) if err != nil { + log.Debugf("Invalid %s record target: %s. Invalid integer value in target.", recordType, target) return false } } From f593c08aa90777285211fb30963048433f37e0e5 Mon Sep 17 00:00:00 2001 From: Julilla Paul <18239431+julillae@users.noreply.github.com> Date: Mon, 10 Feb 2025 11:46:55 -0800 Subject: [PATCH 5/5] chore: refactor CheckEndpoint into helper functions --- endpoint/endpoint.go | 67 ++++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 28 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index cdd070de4..a8a385c57 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -402,36 +402,47 @@ func RemoveDuplicates(endpoints []*Endpoint) []*Endpoint { func (e *Endpoint) CheckEndpoint() bool { switch recordType := e.RecordType; recordType { case RecordTypeMX: - for _, target := range e.Targets { - // MX records must have a preference value to indicate priority, e.g. "10 example.com" - // as per https://www.rfc-editor.org/rfc/rfc974.txt - targetParts := strings.Fields(strings.TrimSpace(target)) - if len(targetParts) != 2 { - return false - } - - preferenceRaw := targetParts[0] - _, err := strconv.ParseUint(preferenceRaw, 10, 16) - if err != nil { - log.Debugf("Invalid %s record target: %s. Invalid integer value in target.", recordType, target) - return false - } - } + return e.Targets.ValidateMXRecord() case RecordTypeSRV: - for _, target := range e.Targets { - // SRV records must have a priority, weight, and port value, e.g. "10 5 5060 example.com" - // as per https://www.rfc-editor.org/rfc/rfc2782.txt - targetParts := strings.Fields(strings.TrimSpace(target)) - if len(targetParts) != 4 { - return false - } + return e.Targets.ValidateSRVRecord() + } + return true +} - for _, part := range targetParts[:3] { - _, err := strconv.ParseUint(part, 10, 16) - if err != nil { - log.Debugf("Invalid %s record target: %s. Invalid integer value in target.", recordType, target) - return false - } +func (t Targets) ValidateMXRecord() bool { + for _, target := range t { + // MX records must have a preference value to indicate priority, e.g. "10 example.com" + // as per https://www.rfc-editor.org/rfc/rfc974.txt + targetParts := strings.Fields(strings.TrimSpace(target)) + if len(targetParts) != 2 { + log.Debugf("Invalid MX record target: %s. MX records must have a preference value to indicate priority, e.g. '10 example.com'", target) + return false + } + preferenceRaw := targetParts[0] + _, err := strconv.ParseUint(preferenceRaw, 10, 16) + if err != nil { + log.Debugf("Invalid SRV record target: %s. Invalid integer value in target.", target) + return false + } + } + return true +} + +func (t Targets) ValidateSRVRecord() bool { + for _, target := range t { + // SRV records must have a priority, weight, and port value, e.g. "10 5 5060 example.com" + // as per https://www.rfc-editor.org/rfc/rfc2782.txt + targetParts := strings.Fields(strings.TrimSpace(target)) + if len(targetParts) != 4 { + log.Debugf("Invalid SRV record target: %s. SRV records must have a priority, weight, and port value, e.g. '10 5 5060 example.com'", target) + return false + } + + for _, part := range targetParts[:3] { + _, err := strconv.ParseUint(part, 10, 16) + if err != nil { + log.Debugf("Invalid SRV record target: %s. Invalid integer value in target.", target) + return false } } }