From 2ae73db5cd3607c5e80288e812194caf69b2bfac Mon Sep 17 00:00:00 2001
From: Titus Sanchez <titusjo@gmail.com>
Date: Fri, 31 May 2024 09:55:14 -0700
Subject: [PATCH] Fix domain name length check in SPF verification (#34)

* Adjust max domain length check from 63 to 255

* Check that labels aren't longer than 63 chars
---
 resources/spf/basic.yml | 21 +++++++++++++--------
 src/spf/verify.rs       | 24 ++++++++++++++++--------
 2 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/resources/spf/basic.yml b/resources/spf/basic.yml
index a00787c..ff26b09 100644
--- a/resources/spf/basic.yml
+++ b/resources/spf/basic.yml
@@ -1,17 +1,22 @@
-# If the <domain> is malformed (e.g., label longer than 63 characters,
-# zero-length label not at the end, etc.) or is not a multi-label
-# domain name, or if the DNS lookup returns "Name Error" (RCODE 3, also
-# known as "NXDOMAIN" [RFC2308]), check_host() immediately returns the
-# result "none".
+# If the <domain> is malformed (e.g., label longer than 63 characters, total
+# length longer than 255 characters, zero-length label not at the end, etc.) or
+# is not a multi-label domain name, or if the DNS lookup returns "Name Error"
+# (RCODE 3, also known as "NXDOMAIN" [RFC2308]), check_host() immediately
+# returns the result "none".
 
 name: Malformed Domains
 records:
-  spf: extremely.ridiculously.long.domain.name.that.should.fail.immediately.com v=spf1 +all
+  spf: this.domain.name.is.extremely.long.because.we.want.to.explicitly.show.that.the.maximum.length.of.a.domain.name.is.255.characters.so.this.one.will.definitely.fail.immediately.due.to.its.excessive.length.and.ridiculously.large.number.of.characters.which.makes.it.invalid.com v=spf1 +all
+  spf: thislabelisjustoverthesixtythreecharacterlimitandshouldbeanerror.com v=spf1 +all
   spf: nolabels v=spf1 +all
   spf: none.test.org v=something-else not=spf for=sure
 tests:
-  - domain: extremely.ridiculously.long.domain.name.that.should.fail.immediately.com
-    sender: sender@extremely.ridiculously.long.domain.name.that.should.fail.immediately.com
+  - domain: this.domain.name.is.extremely.long.because.we.want.to.explicitly.show.that.the.maximum.length.of.a.domain.name.is.255.characters.so.this.one.will.definitely.fail.immediately.due.to.its.excessive.length.and.ridiculously.large.number.of.characters.which.makes.it.invalid.com
+    sender: sender@this.domain.name.is.extremely.long.because.we.want.to.explicitly.show.that.the.maximum.length.of.a.domain.name.is.255.characters.so.this.one.will.definitely.fail.immediately.due.to.its.excessive.length.and.ridiculously.large.number.of.characters.which.makes.it.invalid.com
+    ip: 172.168.0.1
+    expect: none
+  - domain: thislabelisjustoverthesixtythreecharacterlimitandshouldbeanerror.com
+    sender: sender@thislabelisjustoverthesixtythreecharacterlimitandshouldbeanerror.com
     ip: 172.168.0.1
     expect: none
   - domain: nolabels
diff --git a/src/spf/verify.rs b/src/spf/verify.rs
index 22842b8..8067315 100644
--- a/src/spf/verify.rs
+++ b/src/spf/verify.rs
@@ -26,7 +26,7 @@ impl Resolver {
         helo_domain: &str,
         host_domain: &str,
     ) -> SpfOutput {
-        if helo_domain.has_labels() {
+        if helo_domain.has_valid_labels() {
             self.check_host(
                 ip,
                 helo_domain,
@@ -88,7 +88,7 @@ impl Resolver {
         sender: &str,
     ) -> SpfOutput {
         let output = SpfOutput::new(domain.to_string());
-        if domain.is_empty() || domain.len() > 63 || !domain.has_labels() {
+        if domain.is_empty() || domain.len() > 255 || !domain.has_valid_labels() {
             return output.with_result(SpfResult::None);
         }
         let mut vars = Variables::new();
@@ -495,24 +495,32 @@ impl LookupLimit {
     }
 }
 
-pub trait HasLabels {
-    fn has_labels(&self) -> bool;
+pub trait HasValidLabels {
+    fn has_valid_labels(&self) -> bool;
 }
 
-impl HasLabels for &str {
-    fn has_labels(&self) -> bool {
+impl HasValidLabels for &str {
+    fn has_valid_labels(&self) -> bool {
         let mut has_dots = false;
         let mut has_chars = false;
+        let mut label_len = 0;
         for ch in self.chars() {
+            label_len += 1;
+
             if ch.is_alphanumeric() {
                 has_chars = true;
             } else if ch == '.' {
                 has_dots = true;
+                label_len = 0;
             }
-            if has_chars && has_dots {
-                return true;
+
+            if label_len > 63 {
+                return false;
             }
         }
+        if has_chars && has_dots {
+            return true;
+        }
         false
     }
 }