Handle more sorts of TXT record values

octodns/processor/spf.py

@@ -2,7 +2,9 @@
 #
 #
 
+import re
 from logging import getLogger
+from typing import Optional
 
 import dns.resolver
 
@@ -26,21 +28,42 @@ class SpfDnsLookupProcessor(BaseProcessor):
         self.log.debug(f"SpfDnsLookupProcessor: {name}")
         super().__init__(name)
 
-    def _lookup(
-        self, record: Record, values: list[str], lookups: int = 0
-    ) -> int:
+    def _get_spf_from_txt_values(
+        self, values: list[str], record: Record
+    ) -> Optional[str]:
+        self.log.debug(
+            f"_get_spf_from_txt_values: record={record.fqdn} values={values}"
+        )
+
         # SPF values must begin with 'v=spf1 '
         spf = [value for value in values if value.startswith('v=spf1 ')]
 
         if len(spf) == 0:
-            return lookups
+            return None
 
         if len(spf) > 1:
             raise SpfValueException(
                 f"{record.fqdn} has more than one SPF value"
             )
 
-        spf = spf[0]
+        match = re.search(r"(v=spf1\s.+(?:all|redirect=))", "".join(values))
+
+        if match is None:
+            raise SpfValueException(f"{record.fqdn} has an invalid SPF value")
+
+        return match.group()
+
+    def _check_dns_lookups(
+        self, record: Record, values: list[str], lookups: int = 0
+    ) -> int:
+        self.log.debug(
+            f"_check_dns_lookups: record={record.fqdn} values={values} lookups={lookups}"
+        )
+
+        spf = self._get_spf_from_txt_values(values, record)
+
+        if spf is None:
+            return lookups
 
         terms = spf.removeprefix('v=spf1 ').split(' ')
 
@@ -61,7 +84,7 @@ class SpfDnsLookupProcessor(BaseProcessor):
                 answer = dns.resolver.resolve(
                     term.removeprefix('include:'), 'TXT'
                 )
-                lookups = self._lookup(
+                lookups = self._check_dns_lookups(
                     record, [value.to_text()[1:-1] for value in answer], lookups
                 )
 
@@ -75,6 +98,6 @@ class SpfDnsLookupProcessor(BaseProcessor):
             if record._octodns.get('lenient'):
                 continue
 
-            self._lookup(record, record.values)
+            self._check_dns_lookups(record, record.values)
 
         return zone

tests/test_octodns_processor_spf.py

@@ -10,9 +10,83 @@ from octodns.zone import Zone
 
 
 class TestSpfDnsLookupProcessor(TestCase):
+    def test_get_spf_from_txt_values(self):
+        processor = SpfDnsLookupProcessor('test')
+        record = Record.new(
+            Zone('unit.tests.', []),
+            '',
+            {'type': 'TXT', 'ttl': 86400, 'values': ['v=DMARC1\; p=reject\;']},
+        )
+
+        self.assertEqual(
+            'v=spf1 include:_spf.google.com ~all',
+            processor._get_spf_from_txt_values(
+                [
+                    'v=DMARC1\; p=reject\;',
+                    'v=spf1 include:_spf.google.com ~all',
+                ],
+                record,
+            ),
+        )
+
+        with self.assertRaises(SpfValueException):
+            processor._get_spf_from_txt_values(
+                [
+                    'v=spf1 include:_spf.google.com ~all',
+                    'v=spf1 include:_spf.google.com ~all',
+                ],
+                record,
+            )
+
+        self.assertEqual(
+            'v=spf1 include:_spf.google.com ~all',
+            processor._get_spf_from_txt_values(
+                [
+                    'v=DMARC1\; p=reject\;',
+                    'v=spf1 include:_spf.google.com ~all',
+                ],
+                record,
+            ),
+        )
+
+        with self.assertRaises(SpfValueException):
+            processor._get_spf_from_txt_values(
+                ['v=spf1 include:_spf.google.com'], record
+            )
+
+        self.assertIsNone(
+            processor._get_spf_from_txt_values(
+                ['v=DMARC1\; p=reject\;'], record
+            )
+        )
+
+        # SPF record split across multiple character-strings, https://www.rfc-editor.org/rfc/rfc7208#section-3.3
+        self.assertEqual(
+            'v=spf1 include:_spf.google.com ip4:1.2.3.4 ~all',
+            processor._get_spf_from_txt_values(
+                [
+                    'v=spf1 include:_spf.google.com',
+                    ' ip4:1.2.3.4 ~all',
+                    'v=DMARC1\; p=reject\;',
+                ],
+                record,
+            ),
+        )
+
+        self.assertEqual(
+            'v=spf1 +mx redirect=',
+            processor._get_spf_from_txt_values(
+                [
+                    'v=spf1 +mx redirect=_spf.example.com',
+                    'v=DMARC1\; p=reject\;',
+                ],
+                record,
+            ),
+        )
+
     def test_processor(self):
         processor = SpfDnsLookupProcessor('test')
-        assert processor.name == 'test'
+        self.assertEqual('test', processor.name)
 
         processor = SpfDnsLookupProcessor('test')
         zone = Zone('unit.tests.', [])
@@ -31,7 +105,8 @@ class TestSpfDnsLookupProcessor(TestCase):
             )
         )
 
-        assert zone == processor.process_source_zone(zone)
+        self.assertEqual(zone, processor.process_source_zone(zone))
+
         zone = Zone('unit.tests.', [])
         zone.add_record(
             Record.new(
@@ -48,7 +123,7 @@ class TestSpfDnsLookupProcessor(TestCase):
             )
         )
 
-        assert zone == processor.process_source_zone(zone)
+        self.assertEqual(zone, processor.process_source_zone(zone))
 
         zone = Zone('unit.tests.', [])
         zone.add_record(
@@ -88,6 +163,28 @@ class TestSpfDnsLookupProcessor(TestCase):
         with self.assertRaises(SpfDnsLookupException):
             processor.process_source_zone(zone)
 
+    def test_processor_with_long_txt_values(self):
+        processor = SpfDnsLookupProcessor('test')
+        zone = Zone('unit.tests.', [])
+
+        zone.add_record(
+            Record.new(
+                zone,
+                '',
+                {
+                    'type': 'TXT',
+                    'ttl': 86400,
+                    'value': (
+                        '"v=spf1 ip6:2001:0db8:85a3:0000:0000:8a2e:0370:7334 ip6:2001:0db8:85a3:0000:0000:8a2e:0370:7334"'
+                        ' " ip6:2001:0db8:85a3:0000:0000:8a2e:0370:7334 ip6:2001:0db8:85a3:0000:0000:8a2e:0370:7334"'
+                        ' " ip6:2001:0db8:85a3:0000:0000:8a2e:0370:7334 ~all"'
+                    ),
+                },
+            )
+        )
+
+        self.assertEqual(zone, processor.process_source_zone(zone))
+
     def test_processor_skips_lenient_records(self):
         processor = SpfDnsLookupProcessor('test')
         zone = Zone('unit.tests.', [])
@@ -104,9 +201,7 @@ class TestSpfDnsLookupProcessor(TestCase):
         )
         zone.add_record(lenient)
 
-        processed = processor.process_source_zone(zone)
-
-        assert zone == processed
+        self.assertEqual(zone, processor.process_source_zone(zone))
 
     def test_processor_errors_on_many_spf_values_in_record(self):
         processor = SpfDnsLookupProcessor('test')
@@ -172,4 +267,4 @@ class TestSpfDnsLookupProcessor(TestCase):
             )
         )
 
-        assert zone == processor.process_source_zone(zone)
+        self.assertEqual(zone, processor.process_source_zone(zone))