Subject: [secdir] Review of draft-ietf-marf-dkim-reporting-11

From: Tero Kivinen <[email-masked]>
Date: Tue, 6 Mar 2012 18:01:09 +0200

I have reviewed this document as part of the security directorate's 
ongoing effort to review all IETF documents being processed by the 
IESG.  These comments were written primarily for the benefit of the 
security area directors.  Document editors and WG chairs should treat 
these comments just like any other last call comments.

This document adds a way to DKIM verifier to send reports to the DKIM
signer when something goes wrong with the signature verification.

It provides two fold authentication to verify that signer has really
requested the reports to protect signers from bogus reports: The
DKIM-Signature field of the email in question needs to have tag "r=y"
to indicate signer wants to see reports, and the _report._domainkey
subdomain needs to have TXT record which also indicates that signer
really wanted these reports.

This is all fine, but the security considerations section should
really point out that the TXT record is the only part that is really
protecting the signer from distributed bogus reports. Even if signer
does not ever put "r=y" tag in any of the messages, but still
publishes the TXT records "just in case" they ever want to get those
reports, the attacker can modify every single email in transit to
include bogus DKIM-signature field with "r=y" and "d=" matching the
signer and DKIM verifiers will start flooding reports to the signer.
Note, that those emails do not even need to be originally have
anything to do with the domain being attacked.

Actually just adding "r=y" to valid DKIM-Signatures will cause the
signature to fail (if I have understood things correctly), thus
triggering the report.

So the only way the signer can protect himself against bogus reports
is to remove the TXT records from the DNS. There should be text about
this in the security considerations sections, as otherwise
adminstrators might put those TXT records out there just in case they
are needed, and open themselves to the attack.

On the other hand, even when signer requests reports to verify nobody
is messing up its DKIM-Signatures the attacker can remove the "r=y"
tag from the email (or the whole DKIM-Signature) and in that case the
verifier do not send report to the signer (unless the Author Domain
Signing Practices (ADSP) is in use, but I didn't really check whether
those records are checked if no DKIM fields are found in the email).

Attacker who wants to modify the emails do not want to advertise this,
thus it will of course remove the "r=y", so it can fly under the
radar...

The proposed Extension to the DKIM-Signature tag does not really
protect or detect against attacks, but it might be useful for
debugging and detecting misconfigurations in the system.

I think the DKIM-ADSP extensions are more useful for detecting
attacks, as those will be checed even when there are no DKIM-Signature
fields in the email (at least I think so, as otherwise they could not
report "unsigned" messages.

----------------------------------------------------------------------

There is also some smaller issues:

----------------------------------------------------------------------
ADSP is not spelled out ever, and the reference to the RFC5617 uses
different title than what is the actual title of the RFC5617:

	"DomainKeys Identified Mail (DKIM) Author Domain Signing
	Practices (ADSP)"

	vs

	"DKIM Sender Signing Practises".

so it was bit hard to see what ADSP actually meant, until I actually
checked the RFC5617. I am not sure why the references are getting
wrong titles, as shouldn't the
http://xml.resource.org/public/rfc/bibxml/reference.RFC.2119.xml
references get those directly from RFCs. On the other hand it might be
the references are written out manually...
----------------------------------------------------------------------
In section 4:

      only.  To construct the actual address to which the report is
      sent, the Verifier simply appends to this value an "@" followed by
      the domain whose policy was queried in order to evaluate the
      sender's ADSP, i.e., the one taken from the RFC5322.From domain of
							^^^
      the message under evaluation.  Therefore, a signer making use of
      this extension tag MUST ensure that an email address thus
      constructed can receive reports generated as described in
      Section 6.  ABNF:

It seems there is extra . between the "RFC5322" and "From".
----------------------------------------------------------------------
In section 5:

   This memo also includes, as the "ro" tags defined above, the means by
				    ^^

I do not think this document defines "ro" tag, I assume it was meant
to mean "rr" tag instead?
----------------------------------------------------------------------
In section 5.2:

How can DKIM ADSP failures ever report "d" type errors, as if they
have DNS issues for fetching ADSP records, they will not get the "rr"
tag saying "d" or "all" types should be reported...
-- 
[email-masked]

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Tue, 6 Mar 2012 22:20:59 +0000

Hi Tero, thanks for your review.

> -----Original Message-----
> From: Tero Kivinen [mailto:[email-masked]]
> Sent: Tuesday, March 06, 2012 8:01 AM
> To: [email-masked]; [email-masked]
> Cc: [email-masked]
> Subject: Review of draft-ietf-marf-dkim-reporting-11
> 
> [...]
> This is all fine, but the security considerations section should really
> point out that the TXT record is the only part that is really
> does not ever put "r=y" tag in any of the messages, but still publishes
> the TXT records "just in case" they ever want to get those reports, the
> attacker can modify every single email in transit to include bogus
> DKIM-signature field with "r=y" and "d=" matching the signer and DKIM
> verifiers will start flooding reports to the signer.
> Note, that those emails do not even need to be originally have anything
> to do with the domain being attacked.
> 
> So the only way the signer can protect himself against bogus reports is
> to remove the TXT records from the DNS. There should be text about this
> in the security considerations sections, as otherwise adminstrators
> might put those TXT records out there just in case they are needed, and
> open themselves to the attack.

I've added this paragraph to the end of the current Security Considerations "Deliberate Misuse" section (pardon the XML):

            <t> The presence of the DNS record that indicates
                willingness to accept reports opens the
                recipient to abuse by any attacker that generates
                or alters signatures bearing the victim's
                domain in a DKIM "d=" tag by adding an "r=y" tag.
                Positive caching of this DNS reply also means
                turning off the flow of reports
                by removing the record is not likely to have
                immediate effect.  A low time-to-live on the
                record needs to be considered. </t>

Is that sufficient?

> thus it will of course remove the "r=y", so it can fly under the

We cover that already in the "DKIM Reporting Algorithm", final paragraph.

> checked the RFC5617. I am not sure why the references are getting wrong
> titles, as shouldn't the

They are, and actually it seems it was imported from a much older document.  ADSP went through several name changes in its lifetime, and that's a fairly old one.  Anyway, fixed; expanded on first use, and the reference has been repaired.

Actually it's correct to strike "one taken from the", so I've done that.  The RFC5322.From notation is introduced by one of the normative references, namely RFC5598, so I believe it's correct.

> I do not think this document defines "ro" tag, I assume it was meant to
> mean "rr" tag instead?

Yes, it was renamed during the document's evolution and I missed this one.  Fixed.

> How can DKIM ADSP failures ever report "d" type errors, as if they have
> DNS issues for fetching ADSP records, they will not get the "rr"

It looks like that was copied from the DKIM reporting flags without realizing this discontinuity.  I'll remove it.

Thanks again,
-MSK

---

From: Tero Kivinen <[email-masked]>
Date: Thu, 8 Mar 2012 15:59:39 +0200

Murray S. Kucherawy writes:
> I've added this paragraph to the end of the current Security
> Considerations "Deliberate Misuse" section (pardon the XML): 
> 

It is actually inaccurate, as the "d=" tag is not needed, as attacker
can add (or modify the existing d= tag) it as needed. The attacker can
even add DKIM-Signature headers to completely unrelated emails with
"d=" matching the intended victim.

I.e spambot system could add DKIM signature header to all spams it is
sending out and put d=victim.example.com and r=y to the headers and
cause all spam recipient seeing this kind of email to attack the
victim.

The comment about the positive caching and low TTL is actually very
good, and I missed those when reading the document myself.

> We cover that already in the "DKIM Reporting Algorithm", final

Partially yes, but I think it should also be mentioned in the Security
Considerations section from the attackers point of view. The text in
the DKIM Reporting Algorithm section is mostly from the Signers and
Verifiers point of view. 

> Actually it's correct to strike "one taken from the", so I've done
> that.  The RFC5322.From notation is introduced by one of the
> normative references, namely RFC5598, so I believe it's correct.

It would be useful to put some kind of quotes around it, i.e. either
<RFC5322.From> or "RFC5322.From" or similar so reader will understand
it is one token, not just mistake. Also I searched that form from
RFC5322, as that was where I expected to find it, so adding reference
to [RFC5598] at this point would help a lot too. 
-- 
[email-masked]

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Thu, 8 Mar 2012 18:51:23 +0000

> Sent: Thursday, March 08, 2012 6:00 AM
> To: Murray S. Kucherawy
> Cc: [email-masked]; [email-masked]; [email-masked]
> Subject: RE: Review of draft-ietf-marf-dkim-reporting-11
> 

The proposed text above says "generates", which covers the case of deliberately adding bad signatures to messages.  The "alters" case would be someone taking a valid signature and adding "r=y" to it, optionally changing the "d=", to cause the attack.

"d=" is always needed in a DKIM signature, or it's syntactically invalid to begin with.

> cause all spam recipient seeing this kind of email to attack the

Doesn't the "generates" case in the above text already cover that instance?

Let's try this instead, which I hope is more clear and explicit about the problem and the solution:

   The presence of the DNS record that indicates willingness to accept
   reports opens the recipient to abuse.  In particular, it is possible
   for an attacker to attempt to cause a flood of reports toward the
   domain identified in a signature's "d=" tag in one of these ways:

   1.  Alter existing signatures by adding an "r=y" tag (and possibly
       altering the "d=" tag to point at the target domain);

   2.  Add a new but invalid signature bearing an "r=y" tag and a "d="
       tag pointing at the target domain;

   3.  Generate a completely new message bearing an "r=y" tag and a "d="
       tag pointing at the target domain.

   Implementers are therefore strongly advised not to advertise that DNS
   record except when reports desired, including the risk of receiving
   this kind of attack.

   Positive caching of this DNS reply also means turning off the flow of
   reports by removing the record is not likely to have immediate
   effect.  A low time-to-live on the record needs to be considered.

How does that look?

> > > On the other hand, even when signer requests reports to verify
> > > nobody is messing up its DKIM-Signatures the attacker can remove the "r=y"
> > > tag from the email (or the whole DKIM-Signature) and in that case
> > > the verifier do not send report to the signer (unless the Author
> > > Domain Signing Practices (ADSP) is in use, but I didn't really check
> > > whether those records are checked if no DKIM fields are found in the email).
> > >
> > > Attacker who wants to modify the emails do not want to advertise
> > > this, thus it will of course remove the "r=y", so it can fly under
> > > the radar...
> >

I think to address this I'll move the text at the end of 3.3 into Security Considerations, where I will describe the problem from both sides.  Here's the new subsection:

8.3.  Unreported Fraud

   An attacker can craft fraudulent DKIM-Signature fields on messages,
   without using "r=" tags, and avoid having these reported.  The
   procedure described in Section 3.3 does not permit the detection and
   reporting of such cases.

   It might be useful to some Signers to receive such reports, but the
   mechanism does not support it.  To offer such support, a Verifier
   would have to violate the first step in the algorithm and continue
   even in the absence of an "r=" tag.  Although that would enable the
   desired report, it would also create a possible denial-of-service
   attack: such Verifiers would always look for the reporting TXT
   record, so a generator of fraudulent messages could simply send a
   large volume of messages without an "r=" tag to a number of
   destinations.  To avoid that outcome, reports of fraudulent DKIM-
   Signature header fields are not possible using the published mechanism.

> > that.  The RFC5322.From notation is introduced by one of the normative
> > references, namely RFC5598, so I believe it's correct.
> 

Fair enough, but I prefer to have references like that, which introduce notational conventions, up in the "Definitions" section.  I'll take care of that now too.

Thanks,

-MSK

---

From: Barry Leiba <[email-masked]>
Date: Thu, 8 Mar 2012 22:02:44 -0500

>   effect.  A low time-to-live on the record needs to be considered.

And I've now seen it in context, in the -12 version.  Some typos in
there, but we'll let the RFC Editor sort it out.

But...
You're going to hate this, but I think the copious new text in section
8.2 really wants another paragraph.  In between the three numbered
items and this:

...I'd like to see an extreme-case (but a very-possible-case) example, thus:

       Consider, for example, the situation if someone should send out a
       multi-million-message spam run, and include in the messages a fake
       DKIM signature containing "d=example.com; r=y".  It won't matter that
       those signatures couldn't possibly be real: each will fail verification,
       and any implementations that support this specification will report
       those failures, in the millions and in short order, to example.com.

I don't think the text that's there lays out the scary possibilities
clearly enough.  I think something like this does.

Barry

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Fri, 9 Mar 2012 04:49:50 +0000

> From: [email-masked] [mailto:[email-masked]] On Behalf Of Barry Leiba
> Sent: Thursday, March 08, 2012 7:03 PM
> To: Murray S. Kucherawy
> Cc: Tero Kivinen; [email-masked];
> [email-masked]; [email-masked]
> Subject: Re: Review of draft-ietf-marf-dkim-reporting-11
> 

Fair enough.  Added almost verbatim for -13.

-MSK

---

From: Tero Kivinen <[email-masked]>
Date: Fri, 9 Mar 2012 13:52:32 +0200

Murray S. Kucherawy writes:
> The proposed text above says "generates", which covers the case of
> deliberately adding bad signatures to messages.  The "alters" case
> would be someone taking a valid signature and adding "r=y" to it,
> optionally changing the "d=", to cause the attack. 

Actually yes. I missed the word "generates" first. It is always hard
when the difference needs to be derived from a single word, meaning
other readers might miss the same word too, especially as the rest of
the sentence ("by adding an "r=y" tag") could be more readily
interpreted only in the "alter" case...

I think we both have seme understanding what the attack is, so the
problem is just how to express it so it is clear for normal reader of
this text... 

> Doesn't the "generates" case in the above text already cover that

Partially, but when I read the rest of it saying 'by adding an "r=y"
tag', that gave me feeling that DKIM-signature needs to be there, and
thats why I missed the word generates before. If those two cases
(alters vs generates) are separated in separate senteces I think that
might be more clear for everybidy. 

>    effect.  A low time-to-live on the record needs to be considered.

That looks very clear and good...

> I think to address this I'll move the text at the end of 3.3 into
> Security Considerations, where I will describe the problem from both
> sides.  Here's the new subsection: 
> 
> 8.3.  Unreported Fraud
> 

I think it might be useful to point out here that this whole mechanism
does not really work agains attack, but is more useful detecting
misconfigurations or errors in setup, because of attacker can do what
is desribed above...

							  ^^^

Add reference to section 3.3 (i.e. ... in the algorithm described in
section 3.3 and continue ...). Or use single term (either procedure or
algorithm) describing it. Now you have first paragraph talking about
the procedure described in section 3.3, and then the next paragraph
talks about the algorithm with out specifying which algorithm...

> Fair enough, but I prefer to have references like that, which
> introduce notational conventions, up in the "Definitions" section.
> I'll take care of that now too. 

Ok.
-- 
[email-masked]

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Fri, 9 Mar 2012 17:22:10 +0000

> Sent: Friday, March 09, 2012 3:53 AM
> To: Murray S. Kucherawy

OK, it's in the version I'll post when LC closes.

> misconfigurations or errors in setup, because of attacker can do what

It does work where there's an attack in progress, with the caveat that asking for reports about the attack could produce a huge amount of feedback.  But I think the text we've landed on spells that out.

> talks about the algorithm with out specifying which algorithm...

Right; changed the first one to "procedure".

Thanks,
-MSK