You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spamassassin.apache.org by bu...@spamassassin.apache.org on 2020/06/27 01:07:35 UTC

[Bug 7831] New: DKIM_VALID_AU does not get set properly when mailing from a subdomain

https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

            Bug ID: 7831
           Summary: DKIM_VALID_AU does not get set properly when mailing
                    from a subdomain
           Product: Spamassassin
           Version: SVN Trunk (Latest Devel Version)
          Hardware: All
                OS: All
            Status: NEW
          Severity: major
          Priority: P2
         Component: Plugins
          Assignee: dev@spamassassin.apache.org
          Reporter: nyt-apachebz@countercultured.net
  Target Milestone: Undefined

RFC6376[1] states that mail authored from a subdomain of the domain specified
by d= in the DKIM signature are valid unless t=s is set in the DKIM DNS record.

Spamassassin's DKIM plugin fails to handle subdomains correctly, only checking
the sender against the domain specified in the DKIM mail header.  This causes
DKIM_VALID_AU not to be set when there is a valid signed DKIM message.

[1] https://tools.ietf.org/html/rfc6376#section-3.10

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

--- Comment #3 from RW <rw...@googlemail.com> ---
I should elaborate a bit. As I understand it section 3.10 is about the validity
of signature headers like the one in A. It's not about alignment with
RFC5322.From. However, if a combination like this:

  DKIM-Signature: ... i=john@foo.example.com ... d=example.com
  From: john@foo.example.com

hits DKIM_VALID but not DKIM_VALID_AU then that's clearly a bug.

The case of B is either handled by DMARC or, in the case of DKIM_VALID_AU, it's
a design decision - DKIM_VALID_AU is equivalent to DMARC's strict DKIM
alignment.

IMO SA could benefit from a relaxed version of  DKIM_VALID_AU, as well as a
relaxed author aligned version of SPF_PASS

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

Henrik Krohns <ap...@hege.li> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
   Target Milestone|Undefined                   |4.0.0

--- Comment #15 from Henrik Krohns <ap...@hege.li> ---
This looks finished in trunk.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

Kevin A. McGrail <km...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kmcgrail@apache.org

--- Comment #1 from Kevin A. McGrail <km...@apache.org> ---
Thank you.  That's a good find.  Can you work on a patch to resolve the issue?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

RW <rw...@googlemail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rwmaillists@googlemail.com

--- Comment #2 from RW <rw...@googlemail.com> ---
Just to be clear, which of these two have you seen fail:

A.  DKIM-Signature: ... i=john@foo.example.com ... d=example.com


B   DKIM-Signature: ... d=example.com 
    From: john@foo.example.com


My understanding is that the DKIM RFC passage is referring to A only.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

--- Comment #7 from Rob Mosher <ny...@countercultured.net> ---
Created attachment 5705
  --> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5705&action=edit
Patch to verify DKIM identities properly

patch attached as file

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

Rob Mosher <ny...@countercultured.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nyt-apachebz@counterculture
                   |                            |d.net

--- Comment #4 from Rob Mosher <ny...@countercultured.net> ---
The bug is here, as it only checks author domain against d=

https://github.com/apache/spamassassin/blob/df83e64408fe1eb76d4a4fd7e0c6a730187ce2f8/lib/Mail/SpamAssassin/Plugin/DKIM.pm#L917


  DKIM-Signature: ... i=john@foo.example.com ... d=example.com
  From: john@foo.example.com

This would only hit DKIM_VALID, and not DKIM_VALID_AU

I'll put a patch together when I get a few minutes

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

Rob Mosher <ny...@countercultured.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #5705|0                           |1
        is obsolete|                            |

--- Comment #10 from Rob Mosher <ny...@countercultured.net> ---
Created attachment 5707
  --> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5707&action=edit
v2 of Patch to verify DKIM identities properly (trunk)

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

--- Comment #5 from Rob Mosher <ny...@countercultured.net> ---
As long as i= is a subdomain of d=, it should check the message From: against
i= unless t=s is present in the DKIM domain record

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

--- Comment #6 from Rob Mosher <ny...@countercultured.net> ---
It's a simple patch but it took a bit of research as I'm not familiar with any
of these code bases.  Some review would be good.

Some notes...

In Mail::DKIM, the identity will not validate if t=s is set and the identity is
a subdomain of the domain.  This is already handled correctly so I don't think
we need to add any additional logic.

If an identity is not specified, it is filled in with @domain, so it should be
safe to use in place in those cases.

https://metacpan.org/release/Mail-DKIM/source/lib/Mail/DKIM/Signature.pm#L458

check_dkim_valid_envelopefrom also wasn't checking the full return path domain,
and just the base domain.

Test from valid subdomain
X-Spam-Status: No, score=1.4 required=4.0 tests=BAYES_00,BODY_SINGLE_WORD,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FSL_BULK_SIG,
        PYZOR_CHECK,SPF_HELO_PASS,SPF_PASS autolearn=no autolearn_force=no
        version=3.4.2

Test from the base domain
X-Spam-Status: No, score=1.4 required=4.0 tests=BAYES_00,BODY_SINGLE_WORD,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FSL_BULK_SIG,
        PYZOR_CHECK,SPF_HELO_PASS,SPF_PASS autolearn=no autolearn_force=no
        version=3.4.2

Test mailing from subdomain with t=s set in domain record
X-Spam-Status: No, score=1.8 required=4.0 tests=BAYES_00,BODY_SINGLE_WORD,
        DKIM_INVALID,DKIM_SIGNED,FSL_BULK_SIG,PYZOR_CHECK,SPF_HELO_PASS,
        SPF_PASS autolearn=no autolearn_force=no version=3.4.2


--- a/lib/Mail/SpamAssassin/Plugin/DKIM.pm
+++ b/lib/Mail/SpamAssassin/Plugin/DKIM.pm
@@ -561,9 +561,10 @@ sub check_dkim_valid_author_sig {
 sub check_dkim_valid_envelopefrom {
   my ($self, $pms, $full_ref) = @_;
   my $result = 0;
-  my
$envfrom=$self->{'main'}->{'registryboundaries'}->uri_to_domain($pms->get("EnvelopeFrom"));
+  my ( $envfrom ) = $pms->get("EnvelopeFrom") =~ /@([a-z0-9\-\.]*)/i;
   # if no envelopeFrom, it cannot be valid
   return $result if !$envfrom;
+  $envfrom = lc $envfrom;
   $self->_check_dkim_signature($pms)  if !$pms->{dkim_checked_signature};
   if (!$pms->{dkim_valid}) {
     # don't bother
@@ -720,7 +721,7 @@ sub _check_dkim_signed_by {
       next if $minimum_key_bits && $sig->{_spamassassin_key_size} &&
               $sig->{_spamassassin_key_size} < $minimum_key_bits;
     }
-    my $sdid = $sig->domain;
+    my ( $sdid ) = $sig->identity =~ /@(.*)/;
     next if !defined $sdid;  # a signature with a missing required tag 'd' ?
     $sdid = lc $sdid;
     if ($must_be_author_domain_signature) {
@@ -909,7 +910,7 @@ sub _check_dkim_signature {
       push(@valid_signatures, $signature)  if $valid && !$expired;

       # check if we have a potential Author Domain Signature, valid or not
-      my $d = $signature->domain;
+      my ( $d ) = $signature->identity =~ /@(.*)/;
       if (!defined $d) {
         # can be undefined on a broken signature with missing required tags
       } else {
@@ -1261,7 +1262,7 @@ sub _wlcheck_list {
       }
     }

-    my $sdid = $signature->domain;
+    my ( $sdid ) = $signature->identity =~ /@(.*)/;
     $sdid = lc $sdid  if defined $sdid;

     my %tried_authors;

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

John Hardin <jh...@impsec.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jhardin@impsec.org

--- Comment #13 from John Hardin <jh...@impsec.org> ---
I'm getting "uninitialized value in pattern match" errors with this patch,
where it's trying to extract the domain from a signature identity:

Jan 23 10:35:40.102 [19982] dbg: dkim: performing public key lookup and
signature verification
Jan 23 10:35:40.104 [19982] dbg: dkim: FAILED DKIM,
i=transfer-your-credit-balance@3harmfullfoods.com, d=3harmfullfoods.com,
s=dkim, a=rsa-sha1, c=relaxed/relaxed, unknown key size, invalid, matches
author domain
Use of uninitialized value in pattern match (m//) at
/home/jhardin/develop/spamassassin/svn/trunk/masses/../blib/lib/Mail/SpamAssassin/Plugin/DKIM.pm
line 913.
Jan 23 10:35:40.104 [19982] dbg: dkim: FAILED DK, i=(undef), d=(undef), s=dkim,
a=rsa-sha1, c=nofws, unknown key size, invalid, does not match author domain
Jan 23 10:35:40.104 [19982] dbg: dkim: signature verification result: INVALID
(PUBLIC KEY: NOT AVAILABLE)
Jan 23 10:35:40.104 [19982] dbg: dkim: FAILED signature by 3harmfullfoods.com,
author transfer-your-credit-balance@3harmfullfoods.com, no valid matches
Use of uninitialized value in pattern match (m//) at
/home/jhardin/develop/spamassassin/svn/trunk/masses/../blib/lib/Mail/SpamAssassin/Plugin/DKIM.pm
line 1265.
Jan 23 10:35:40.104 [19982] dbg: dkim: FAILED signature by (undef), author
transfer-your-credit-balance@3harmfullfoods.com, no valid matches
Jan 23 10:35:40.104 [19982] dbg: dkim: author
transfer-your-credit-balance@3harmfullfoods.com, not in any dkim whitelist

A DK header referring a domain that does not exist/does not publish any DKIM
record seems to be a failure case.


> If an identity is not specified, it is filled in with @domain, so it should be safe to use in place in those cases.

This doesn't actually appear to be happening for the DK header check.

The DK signature check is *not* setting the identity from the domain (which
*is* present - the "d=" value in the log above is the domain extracted from the
identity value, not $signature->domain) and the SA code can't update that value
to repair it even though the Mail::DKIM documentation suggests that is
possible.

This appears to be a bug in Mail::DKIM - it's occurring on the latest version,
1.20200907. I have not filed an upstream bug.


Here are the DKIM/DK headers from that message:

DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; s=dkim;
d=3harmfullfoods.com;
 h=Date:From:To:Subject:MIME-Version:Content-Type:List-Unsubscribe:Message-ID;
i=transfer-your-credit-balance@3harmfullfoods.com;
 bh=6Md+1tEoP1V8A8eusTw2Aml04jw=;
 b=Rp6RdJadb6WCcr3WQRh4ArRFaX+SZERqDJfbBhUFc5cUPZeBXNjfoFxRZ+cnSF9sMbcK5GhJ6FyU
   rgTcnZxOiMtABwizp+94SVa3i3oSi5wf9H7kl25rZy/yydPOMdd1Gq1xx2xI3HjmqkUFFZDnt4YY
   C8KEIiqJ1jX2agM4atU=
DomainKey-Signature: a=rsa-sha1; c=nofws; q=dns; s=dkim; d=3harmfullfoods.com;
 b=DW+bNtslRBdaAIIoQlwVJTbdj13CQ06RVB/bhG+hWucu3JZz2rMHPN3r1vr6j0Q9UrZVdyy+X5iy
   4RxwkXnx2Kb6Wj96v24TuyLkN+IS3S64g9xD/8eehFqkkBgXlfBPpBySjXOjCRLcP9KVv6Ite6QN
   ujl/lQsqYxoBS7AyoaI=;

The DKIM header processes cleanly, the DK one blows up. I have multiple
messages from various (apparently bogus) domains in my corpus that exhibit this
behavior.


Fixing the code to react gracefully to missing identity, and adding a bit more
logging...

Modified: trunk/lib/Mail/SpamAssassin/Plugin/DKIM.pm
Committed revision 1885854.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

--- Comment #8 from Rob Mosher <ny...@countercultured.net> ---
also, PerMsgStatus->_get("EnvelopeFrom") is broken when there are multiple
Return-Path headers specified.

I should be using get("EnvelopeFrom:addr") in this patch, but it wasn't working
right in my tests.  I'll update this patch to use it properly and include a
patch to fix PerMsgStatus.pm

I've also found some bugs in SPF.pm, some of that due it returning junk when
there are multiple Return-Path headers found.  Others due to not checking for
empty strings.  I'll open up a bug report for all of these.

Testing:

    $sender = $scanner->get("EnvelopeFrom:addr");
    dbg("spf: pms:from:addr " . $sender);
    dbg("spf: pms:from:raw " . $scanner->get("EnvelopeFrom:raw"));
    dbg("spf: pms:from " . $scanner->get("EnvelopeFrom"));
    dbg("spf: pms:get:rp " . $scanner->get("Return-Path"));

$ grep Return-Path: mail2.txt
Return-Path: <us...@domain.com>
Return-Path: <us...@domain.com>

dbg: spf: pms:from:addr user@domain.com> <user@domain.com
dbg: spf: pms:from:raw user@domain.com>
dbg: spf: pms:from user@domain.com>
dbg: spf: pms:get:rp <us...@domain.com>

But with only one Return-Path:
dbg: spf: pms:from:addr user@domain.com
dbg: spf: pms:from:raw user@domain.com
dbg: spf: pms:from user@domain.com
dbg: spf: pms:get:rp <us...@domain.com>

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

Stingertough <wo...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wolfsplat@gmail.com

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

--- Comment #11 from Rob Mosher <ny...@countercultured.net> ---
Created attachment 5708
  --> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5708&action=edit
v2 of Patch to verify DKIM identities properly (3.4)

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

--- Comment #9 from Rob Mosher <ny...@countercultured.net> ---
adjusted... 

Here's the trunk patch

--- a/lib/Mail/SpamAssassin/Plugin/DKIM.pm
+++ b/lib/Mail/SpamAssassin/Plugin/DKIM.pm
@@ -561,9 +561,10 @@ sub check_dkim_valid_author_sig {
 sub check_dkim_valid_envelopefrom {
   my ($self, $pms, $full_ref) = @_;
   my $result = 0;
-  my
$envfrom=$self->{'main'}->{'registryboundaries'}->uri_to_domain($pms->get("EnvelopeFrom"));
+  my ( $envfrom ) = $pms->get("EnvelopeFrom:addr") =~ /@(\S*)/i;
   # if no envelopeFrom, it cannot be valid
   return $result if !$envfrom;
+  $envfrom = lc $envfrom;
   $self->_check_dkim_signature($pms)  if !$pms->{dkim_checked_signature};
   if (!$pms->{dkim_valid}) {
     # don't bother
@@ -720,7 +721,7 @@ sub _check_dkim_signed_by {
       next if $minimum_key_bits && $sig->{_spamassassin_key_size} &&
               $sig->{_spamassassin_key_size} < $minimum_key_bits;
     }
-    my $sdid = $sig->domain;
+    my ( $sdid ) = $sig->identity =~ /@(\S*)/;
     next if !defined $sdid;  # a signature with a missing required tag 'd' ?
     $sdid = lc $sdid;
     if ($must_be_author_domain_signature) {
@@ -909,7 +910,7 @@ sub _check_dkim_signature {
       push(@valid_signatures, $signature)  if $valid && !$expired;

       # check if we have a potential Author Domain Signature, valid or not
-      my $d = $signature->domain;
+      my ( $d ) = $signature->identity =~ /@(\S*)/;
       if (!defined $d) {
         # can be undefined on a broken signature with missing required tags
       } else {
@@ -1261,7 +1262,7 @@ sub _wlcheck_list {
       }
     }

-    my $sdid = $signature->domain;
+    my ( $sdid ) = $signature->identity =~ /@(\S+)/;
     $sdid = lc $sdid  if defined $sdid;

     my %tried_authors;

And here's the 3.4 patch

--- spamassassin-3.4.4/lib/Mail/SpamAssassin/Plugin/DKIM.pm     2020-01-18
03:44:49.000000000 -0500
+++ /usr/share/perl5/Mail/SpamAssassin/Plugin/DKIM.pm   2020-07-05
21:55:06.897221239 -0400
@@ -560,9 +560,10 @@ sub check_dkim_valid_author_sig {
 sub check_dkim_valid_envelopefrom {
   my ($self, $pms, $full_ref) = @_;
   my $result = 0;
-  my
$envfrom=$self->{'main'}->{'registryboundaries'}->uri_to_domain($pms->get("EnvelopeFrom"));
+  my ( $envfrom ) = $pms->get("EnvelopeFrom:addr") =~ /@(\S*)/i;
   # if no envelopeFrom, it cannot be valid
   return $result if !$envfrom;
+  $envfrom = lc $envfrom;
   $self->_check_dkim_signature($pms)  if !$pms->{dkim_checked_signature};
   if (!$pms->{dkim_valid}) {
     # don't bother
@@ -719,7 +720,7 @@ sub _check_dkim_signed_by {
       next if $minimum_key_bits && $sig->{_spamassassin_key_size} &&
               $sig->{_spamassassin_key_size} < $minimum_key_bits;
     }
-    my $sdid = $sig->domain;
+    my ( $sdid ) = $sig->identity =~ /@(\S*)/;
     next if !defined $sdid;  # a signature with a missing required tag 'd' ?
     $sdid = lc $sdid;
     if ($must_be_author_domain_signature) {
@@ -902,7 +903,7 @@ sub _check_dkim_signature {
       push(@valid_signatures, $signature)  if $valid && !$expired;

       # check if we have a potential Author Domain Signature, valid or not
-      my $d = $signature->domain;
+      my ( $d ) = $signature->identity =~ /@(\S*)/;
       if (!defined $d) {
         # can be undefined on a broken signature with missing required tags
       } else {
@@ -1254,7 +1255,7 @@ sub _wlcheck_list {
       }
     }

-    my $sdid = $signature->domain;
+    my ( $sdid ) = $signature->identity =~ /@(\S+)/;
     $sdid = lc $sdid  if defined $sdid;

     my %tried_authors;

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

Benny Pedersen <me...@junc.eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |me@junc.eu

--- Comment #14 from Benny Pedersen <me...@junc.eu> ---

https://bugs.gentoo.org/758935

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 7831] DKIM_VALID_AU does not get set properly when mailing from a subdomain

Posted by bu...@spamassassin.apache.org.
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7831

Henrik Krohns <ap...@hege.li> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |apache@hege.li

--- Comment #12 from Henrik Krohns <ap...@hege.li> ---
Looks reasonable, committing to trunk for testing..

Sending        trunk/lib/Mail/SpamAssassin/Plugin/DKIM.pm
Transmitting file data .done
Committing transaction...
Committed revision 1879692.

I'd like to make a test case for this too, will look later..

-- 
You are receiving this mail because:
You are the assignee for the bug.