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 2022/12/07 12:16:53 UTC

[Bug 8087] New: DMARC check ignores sp= for subdomains

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

            Bug ID: 8087
           Summary: DMARC check ignores sp= for subdomains
           Product: Spamassassin
           Version: 4.0.0
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Plugins
          Assignee: dev@spamassassin.apache.org
          Reporter: wbreyha@gmx.net
  Target Milestone: Undefined

If a Domain has defined a DMARC policy with "p=reject;sp=none" spamassassin 4
still hits DMARC_REJECT if a subdomain was used in the From: Header.

This is for example true for
nic.at
and
news.nic.at

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #13 from Giovanni Bechis <gi...@paclan.it> ---
+1 to commit 3rd patch

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

Sidney Markowitz <si...@sidney.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #20 from Sidney Markowitz <si...@sidney.com> ---
trunk % svn ci -m "Bug 8087 - Fix bug that showed up in DMARC with some
subdomains" lib/Mail/SpamAssassin/Plugin/DMARC.pm
Sending        lib/Mail/SpamAssassin/Plugin/DMARC.pm
Transmitting file data .done
Committing transaction...
Committed revision 1905867.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #17 from Kevin A. McGrail <km...@apache.org> ---
(In reply to Benny Pedersen from comment #16)
> this ticket here imho open up for debate should spamassassin reject or
> quanrintine ?
> 
> historical it have never rejected any mails, its always being the glue part
> that did that, so now comes the problem how to integrate it, i use
> Mail::DMARC as dmarc local to see reports

I don't think that's correct, No.  SA rules just affect scoring and reject
might get a higher score than quarantine which would be higher than non.  If an
MTA glue with SA causees an SMTP rejection based on that, that would be up to
the implementor.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #12 from Bill Cole <bi...@apache.org> ---
(In reply to Sidney Markowitz from comment #11)
> Created attachment 5867 [details]
> DMARC disposition check v3
> 
> This is same functionality as the v2 patch, but phrased to look a bit
> cleaner. I couldn't unsee the unnecessary nesting of the if once I noticed
> it, and had to clean it up.
> 
> Sorry for requiring you to vote again, Bill :)

Which I happily do. 

+1 for v3.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #10 from Bill Cole <bi...@apache.org> ---
+1 for the new patch.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #18 from Bill Cole <bi...@apache.org> ---
(In reply to Bill Cole from comment #12)
> (In reply to Sidney Markowitz from comment #11)
> > Created attachment 5867 [details]
> > DMARC disposition check v3
> > 
> > This is same functionality as the v2 patch, but phrased to look a bit
> > cleaner. I couldn't unsee the unnecessary nesting of the if once I noticed
> > it, and had to clean it up.
> > 
> > Sorry for requiring you to vote again, Bill :)
> 
> Which I happily do. 
> 
> +1 for v3.

And +1 for v4.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #14 from Giovanni Bechis <gi...@paclan.it> ---
Created attachment 5868
  --> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5868&action=edit
DMARC disposition check v4

IMO if result is 'pass' it's better to set dmarc_policy as the policy present
in the DMARC record instead of hardcoding 'none'.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

Sidney Markowitz <si...@sidney.com> changed:

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

--- Comment #11 from Sidney Markowitz <si...@sidney.com> ---
Created attachment 5867
  --> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5867&action=edit
DMARC disposition check v3

This is same functionality as the v2 patch, but phrased to look a bit cleaner.
I couldn't unsee the unnecessary nesting of the if once I noticed it, and had
to clean it up.

Sorry for requiring you to vote again, Bill :)

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #22 from Sidney Markowitz <si...@sidney.com> ---
(In reply to Benny Pedersen from comment #21)
> in Mail::DMARC there is a file named dmarc_whitelist :)
> 
> content of that file is hopefully used in spamassassin 4 ?

SpamAssassin calls Mail::DMARC to validate the email. The code inside
Mail::DMarc seems to make use of a mail-dmarc.ini file that can specify the
location of a whitelist file of which there is a sample named dmarc_whitelist
in a share directory.

I don't know where exactly the Mail::DMARC code will look for mail-dmarc.ini
when it is installed on the system as a SpamAssassin plugin. It is using
File::ShareDir for that, so I guess there is some fairly standard place to put
it.

This is outside of my expertise and outside the scope of this bug report, so
the left is left as an exercise for people who want to configure Mail::DMARC to
do specific customizations.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #19 from Sidney Markowitz <si...@sidney.com> ---
+1 for v4

FYI, looking at the code if result is 'pass' than the actual use of
dmarc_policy makes it do the same thing as long as dmarc_policy is not set to
'no policy available' or undefined. Which makes sense, since policy is what to
do if it doesn't pass, so why should it matter if it does pass.

So v4 has the same functional result as v3. However, I agree with the
aesthetics of v4, and v4 behaves the same as v3, and already has the votes, so
yes, +1, and I'll even commit it now because I want to get this thing closed.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #21 from Benny Pedersen <me...@junc.eu> ---
in Mail::DMARC there is a file named dmarc_whitelist :)

content of that file is hopefully used in spamassassin 4 ?

my policy is to NOT ever reject maillists not even if dkim dmarc spf fails

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #23 from Benny Pedersen <me...@junc.eu> ---
maybe this https://metacpan.org/pod/Mail::DMARC::Result ?

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #24 from Sidney Markowitz <si...@sidney.com> ---
(In reply to Benny Pedersen from comment #23)
> maybe this https://metacpan.org/pod/Mail::DMARC::Result ?

That's the object that is returned by the call the SpamAssassin plugin makes to
$dmarc->valdate, just as described on the documentation page you linked to. The
SpamAssassin plugin then uses the values in the object to decide whether or not
rules have a hit, indicating whether the result was pass, reject, quarantine,
none, or a missing policy.

The result object is created completely in the Mail:DMARC::PurePerl code, not
by anything that is in SpamAssassin itself. Thus any use of a whitelist file as
specified in a mail-dmarc.ini file depends on the code in Mail:DMARC::PurePerl
and the Mail::DMARC modules that it calls. I didn't see anything in the API
that provides a way for the caller to influence that. And that's the limit of
my expertise on it, the code inside Mail::DMARC is outside of my domain. But if
you want to use the whitelist, find out where Mail:DMARC looks for the
mail-dmarc.ini file and put your whitelist path in the whitelist option instead
of the sample one that the sample mail-dmarc.ini has.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #8 from Sidney Markowitz <si...@sidney.com> ---
I looked at the code and at https://metacpan.org/pod/Mail::DMARC::Result

The specification for Mail::DMARC::Result says

"disposition
When the DMARC result is not pass, disposition is the results of applying DMARC
policy to a message."

That doesn't say what $result->disposition will be set to when result is pass.

Looking at our code, it doesn't matter what $pms->{dmarc_policy} is set to when
result is 'passed' as long as it is not set to 'no policy available' or
undefined.

Should the patch be modified to set $pms->{dmarc_policy} to 'none' if
$result->result eq 'pass' since the spec doesn't say what $result->disposition
will be in that case? Alternatively, set $pms->{dmarc_policy} to 'none' if
$result->disposition is undefined? Either should work to handle the case that
is not defined in the spec.

Even with this I still think this will be trivial enough to commit for 4.0.0
without triggering an rc5.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #15 from Kevin A. McGrail <km...@apache.org> ---
(In reply to Giovanni Bechis from comment #14)
> Created attachment 5868 [details]
> DMARC disposition check v4
> 
> IMO if result is 'pass' it's better to set dmarc_policy as the policy
> present in the DMARC record instead of hardcoding 'none'.

Correct. +1 for v4

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

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

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

--- Comment #16 from Benny Pedersen <me...@junc.eu> ---
this ticket here imho open up for debate should spamassassin reject or
quanrintine ?

historical it have never rejected any mails, its always being the glue part
that did that, so now comes the problem how to integrate it, i use Mail::DMARC
as dmarc local to see reports

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

Giovanni Bechis <gi...@paclan.it> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |giovanni@paclan.it

--- Comment #3 from Giovanni Bechis <gi...@paclan.it> ---
This bug is triggered when a domain with both "p" and "sp" fields of the policy
are defined and the policy fails, I think it could wait for a quick 4.0.1 if no
other bugs will arise.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

Sidney Markowitz <si...@sidney.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sidney@sidney.com

--- Comment #1 from Sidney Markowitz <si...@sidney.com> ---
If I understand this correctly, if a subdomain does not have its own DMARC
record, it inherits everything from the record of the base domain except for
the policy, p. Instead it uses the value in sp (subdomain policy). So nic.at
has a policy of "reject" and news.nic.at has a policy of "none". But
SpamAssassin is using the inherited policy of "reject" for new.nic.at instead
of using the sp value.

Is that a fair summary of the bug?

With the release of 4.0.0 being really imminent, I would like to hear arguments
pro and con whether this bug is a big enough issue to block the 4.0.0 release,
or if it justifies a very quick 4.0.1 release, or it can be delayed.

Presenting a patch that is small and safe enough and fixes the whole problem
could be an argument for including this in 4.0.0 or a quick 4.0.1. Otherwise a
discussion on the impacts this bug has is appropriate.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

--- Comment #2 from Giovanni Bechis <gi...@paclan.it> ---
Created attachment 5865
  --> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5865&action=edit
DMARC disposition check

Consider the disposition field returned by the DMARC check, not the "p" field.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

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

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

--- Comment #4 from Henrik Krohns <ap...@hege.li> ---

Yes I was wondering what's the reason to even use published->p.


---> nic.at

$result->result = fail
$result->disposition = reject
$result->published->p = reject
Dec  7 16:53:16.091 [2319877] dbg: DMARC: result: fail, disposition: reject,
dkim: fail, spf: fail (spf: none, spf_helo: none)


---> news.nic.at

$result->result = fail
$result->disposition = none
$result->published->p = reject
Dec  7 16:53:47.083 [2319912] dbg: DMARC: result: fail, disposition: none,
dkim: fail, spf: fail (spf: none, spf_helo: none)


I'm +1 to commit now if others deem it safe (I have really no clue about DMARC
anyway, but looks logical). You can be sure there will be tons of other bugs to
fix for 4.0.1..

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

Wolfgang Breyha <wb...@gmx.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wbreyha@gmx.net

--- Comment #6 from Wolfgang Breyha <wb...@gmx.net> ---
Sorry, was short on time today... but I added the fix to my rc4 test
installation now and it looks quite good with my example mail from the morning.

Thanks for the quick fix!

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

Bill Cole <bi...@apache.org> changed:

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

--- Comment #5 from Bill Cole <bi...@apache.org> ---
I vote to treat this as a trivial fix for 4.0.0 that does not merit another RC. 

The failure mode seems likely to be fairly uncommon, as I would expect most
people who actually need the correct behavior would publish a specific policies
for the subdomains that need to be allowed to have no authentication.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

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

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

--- Comment #7 from Kevin A. McGrail <km...@apache.org> ---
(In reply to Bill Cole from comment #5)
> I vote to treat this as a trivial fix for 4.0.0 that does not merit another
> RC. 
> 
> The failure mode seems likely to be fairly uncommon, as I would expect most
> people who actually need the correct behavior would publish a specific
> policies for the subdomains that need to be allowed to have no
> authentication.

I don't think this is accurate.  I thought you get one DMARC policy per domain
and in it you define your subdomain policy.

But it looks like the patch works so if RC4 is good to go, we can always add in
a small one line patch for a release.  RC's don't have to be mirror copies of
the release.

I'm +1 to add it to 4.0.0 if RC4 proves out.
I'm +0.5 to kick it to a 4.0.1

Thanks for the quick patch, Giovanni.

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

[Bug 8087] DMARC check ignores sp= for subdomains

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

Sidney Markowitz <si...@sidney.com> changed:

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

--- Comment #9 from Sidney Markowitz <si...@sidney.com> ---
Created attachment 5866
  --> https://bz.apache.org/SpamAssassin/attachment.cgi?id=5866&action=edit
DMARC disposition check v2

This is the same patch modified to set it to 'none' when result is 'pass' since
the DMARC module spec doesn't specify what disposition will be in that case.

Votes?

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