Re: [spfbis] WGLC: draft-ietf-spfbis-4408bis-14

[Scott Kitterman <[e-mail]>]

On Tuesday, April 23, 2013 11:03:37 AM Murray S. Kucherawy wrote:
> On Mon, Apr 8, 2013 at 11:24 PM, Andrew Sullivan 
<[e-mail]>wrote:
> > Dear colleagues,
> > 
> > This message initiates a two-week working group last call on
> > draft-ietf-spfbis-4408bis-14, our only work item.
> > 
> > The draft has been through 14 revisions, and all open issues in the
> > tracker are closed.  Please review the document and indicate any
> > comments you have by sending a note to the mailing list.  If you have
> > no comments, it is valuable nevertheless to learn that you have read
> > the document and support its publication.
> > 
> > WGLC will close at 2014-04-24 00:00 UTC.
> > 
> > SM will be the shepherd for this document.
> 
> Most of this is nits; except as noted, you could ignore it all and I'd be
> happy.  Nevertheless:
> 
> Section 1:
> 
> The last paragraph makes the claim that domain reputation is likely to be
> more accurate than IP reputation.  Is it worthwhile to add a sentence or
> two explaining why this is true?

I don't think so.  First, it's speculation and I don't think enough of a case 
for it can be made without being too long/distracting.  Second, it's not part 
of the protocol, so it not something we should worry about overmuch.

> Section 1.1.2:
> 
> Suggest replacing the first sentence with "ABNF is defined in [RFC5234], as
> are the tokens ALPHA, ..."  As it is now, "ABNF" is not expanded on first
> use.

Done.

> Section 1.1.3:
> 
> Replace the last sentence with: "Note that other terms that might
> superficially look like the common terms, such as 'reverse-path', are used
> only as they are specified in their defining documents."

Done.

> Section 2.1:
> 
> I think someone else already mentioned this, but how does encouraging a
> second check decrease DNS resource use?

I did reply to this already.  It's a matter of HELO records generally being 
much 'cheaper' than mail from records.

> Also, suggest replacing the first sentence of the second paragraph with
> "Note that requirements for the domain presented in the EHLO or HELO
> command are not always clear to the sending party, and SPF verifiers MUST
> be prepared for is identity to be an IP address literal, or simply be
> malformed."   We might even reference the IP address literal ABNF in
> RFC5321 here.

Done (both changes)

> Section 2.2:
> 
> Add to the first paragraph "...or no HELO check was done." or something of
> that ilk.

Done.

> Section 2.3:
> 
> The second paragraph is loaded with negatives "no... neither... nor."
> Suggest: "ADMDs can publish SPF records that authorize no hosts to use
> their DNS domain names in HELO or MAIL FROM commands during SMTP sessions."

I added that, because it's a good point, but it doesn't replace the existing 
text which attempts to (without being too pointed about receiver policy) say 
that if you want receivers to be able to reject mail that didn't come from 
authorized hosts, you need a -all record.  Some senders care about supporting 
these negative assertions, others don't.

> Section 2.4:
> 
> Isn't the fourth paragraph implicit in any standards specification?  That
> is, do we really need it?

I see your point, but I think it should actually stay.  Consistency of 
evaluation across multiple implementations is key to the success of the 
protocol.  IIRC, you said during the debate that if you were implementing SPF, 
you'd ignore the macro requirements.  You're, of course, free to do that, just 
don't say you implemented SPF.

> Section 2.5:
> 
> I can't remember if I suggested it previously, but an appendix of RFC5451
> gives a pretty thorough treatment of the notion of doing mail
> authentication checks during the SMTP session versus later.  It might be
> helpful as an informative reference here.

Added.

> Section 2.6.7:
> 
> Does "permerror" also result when permanent DNS errors occur?

Tell me how to figure out when a DNS error is permanent and then maybe.  Given 
what's described in 4.4 Record Lookup, I think only extended SERVFAIL (if the 
receiver has implemented the temperror -> permerror promotion option) can 
currently get a permerror.

> Section 3.1:
> 
> s/alternate/alternative/

Done.
> 
> Section 4.1:
> 
> The list of check_host() inputs have been repeated here as in Section 2.4.
> Do we need both lists?

I think not.  I made 2.4 reference 4.1 and removed the duplication.

> Section 4.4:
> 
> s/suggests on an algorithm/suggests an algorithm/

Done.
> 
> Section 4.6.4:
> 
> I imagine you're going to say that 10 is the limit imposed by most
> implementations, but shouldn't we say that there should be a finite,
> perhaps configurable limit, and operational experience has shown that 10 is
> a reasonable default?

No.  We need consistent limits because if receivers vary the limits, then 
there will be inconsistent results and poor interoperability.

> Why 20 seconds for the overall run time?

This is carried forward from RFC 4408.  It was moved from section 10.1 to 
4.6.4 as part of issue #6, but the value is unchanged.  I'm not aware of any 
issues with what's defined in 4408.

> Section 4.8:
> 
> In the third paragraph, a couple of sentences start with "Domain" when you
> actually mean it as a literal parameter name.  I suggest using "domain"
> (including the quotes) instead.

Done.

> The "Some implementations ..." sentence seems to be malformed.  I can't
> parse it.

Better?

          Note: Historically, this document has made no provisions for how to
          handle domain-specs, or macro-expansions thereof, that are
          syntactically invalid per <xref target="RFC1035"/>, such as names
          with empty labels (e.g., "foo..example.com") or overlong labels
          (more than 63 characters).  Some implementations choose to treat 
          mechanisms associated with such expansions as no-match and
          ignore modifiers with such names, others return a "permerror" 
          exception. The outcome for an unexpected domain-spec without macros 
          might even differ from that for an unexpected &lt;target-name&gt; 
          after macro expansion.

> Section 5.2:
> 
> s/Check_host/check_host/ (it's not a word that starts sentences and thus
> needs capitalization)

Done.

> Missing end-quote at the end of bullet 3.
>
Done.

> The second-last paragraph is weirdly wrapped.  Is the XML ok there?

I think I fixed that already based on John Levine's comments.   Please double 
check the new draft.

> Section 5.4:
> 
> Should the limits stuff in here be removed, and simply reference Section
> 4.6.4 instead?

Yes.  Done.

> Section 5.5:
> 
> The SHOULD NOT in here is puzzling.  Does it interfere with
> interoperability to use it?  It was my impression that it's possible to get
> the same functionality some other way, but not that using "ptr" actually
> was a bad idea for interoperability reasons.

Yes, because it produces inconsistent results and is error prone.  Both those 
things impact interoperability.

> Again with the limits stuff, redundant to Section 4.6.4.

Done.

> Section 5.6:
> 
> For IPv4, shouldn't the CIDR be 1*2DIGIT?  For IPv6, shouldn't the CIDR be
> 1*3DIGIT?

I don't know.  It's unchanged from RFC 4408 right now, but I'd like other 
opinions on changes here (ABNF makes my head hurt).

> Section 6.2:
> 
> It just occurred to me that in the earlier sections (2 or 4, in
> particular), it's clear that check_host() returns one of the enumerated
> result, but it's never made explicit that it also outputs an explanation
> string.  We might want to mention this earlier than we do now so it's not a
> surprise later.

It's an optional and somewhat rare output of the protocol, so I think not, but 
did you have something specific in mind?
> 
> The SHOULD in the fifth paragraph is questionable: How does it describe an
> aspect of interoperability?

It avoids the risk of a common implementation error in SPF libraries (that 
process left to right) so it promotes consistent results.  It also helps for 
human readability, but I think it helps the protocol too.
> 
> Last paragraph, s/exp=/exp/ 2x.

Done.

> Section 7.1:
> 
> ABNF rules say literal strings in productions are case-insensitive.  That
> means the macro-letter set actually implicitly includes all of the capital
> letter equivalents.  If we want to constrain it to lowercase only, we need
> to use hex notation.

OK.  If someone would provide those, it would be great.  Otherwise, I'll take 
a shot at it, probably Friday.

> Section 7.3:
> 
> "This macro SHOULD NOT be used...." should be in its own paragraph.  Also
> the second sentence should be "See Section xxx for discussion."

Done.

> At the end of this section, there are three paragraphs that all start
> "Note:".  I don't think those tags add anything and suggest they be removed.

Done.

> Section 8:
> 
> Second paragraph, s/, and where/ and, where/

Done.

> Section 8.7:
> 
> Don't the last two sentences say the same thing?

Almost.  One is error due to macro expansion giving a bogus result.  the other 
is an error due to check_host being handed a bad domain as an input.

> Section 9.1:
> 
> Same point about the case-insensitivity of ABNF here.

Except for "Received-SPF" is there anything case insensitive there?  I'd 
greatly appreciate text for this too.

> The "SPF verifiers MUST make sure..." doesn't seem actionable to me.  Do we
> really expect Received-SPF generators to be able to identify and exclude
> malicious data?

I think you have to.  I've seen people put html pointing to malicious web 
sites in SPF records.  Thanks to someone privately pointing this class of 
attack out to me, my SPF validator now properly escapes anything that's 
retrieved from DNS.
> 
> Section 10:
> 
> s/document/protocol/ (or specification) x3

Done.

> Section 10.1.1:
> 
> s/required/necessary/

I went with needed.

> Section 10.1.2:
> 
> Add "at the cost of a DNS query per receiver" to the end of the first
> sentence.

Done.

> Section 10.1.3:
> 
> s/to hostname/to the hostname/, s/of domain/of the domain/

Done.

> Section 10.2:
> 
> Missing close parenthesis in the first paragraph.

I don't see it.  I may have fixed it already.  Please double check.

> Section 10.3:
> 
> Errant period in the first sentence.

Fixed.

> s/Address/address/

Done.

> Section 11.1:
> 
> Aren't the first and third bullets describing the same attack?

No.  One's a DoS due to lots of mail being sent.  The other's a DoS due to the 
way malicoius SPF records are constructed.  Similar, but not the same.

> Missing parenthesis at the end of the fourth bullet.

Fixed.

> Section 11.3:
> 
> Clobbering the DNS at a receiver could also result in delivery of malicious
> mail if the receiver is configured to pass on "tempfail" and just add a
> header field.

Good point.  I added a sentence on to bullet two in 11.1.  It seemed to fit 
there.  Please let me know what you thing.

> Is that second bullet true?  It appears to claim that IP address spoofing
> is effectively impossible.

In any way that would be useful for SPF, it is AFAIK.

> Section 11.5:
> 
> This appears to be an incomplete thought.  At a minimum, it doesn't flow
> nicely into the next subsection.  Maybe add a sentence to indicate that
> SMTP itself doesn't do anything to validate those parameters?

Added.

> Section 11.5.2:
> 
> Remove commas around the section reference.

Done.

> Appendix A:
> 
> Any changes made to ABNF above need to be reflected down here.

I believe they are consistent.

> Appendix B.3:
> 
> Suggest a reference to RFC5782.

Done.

> Appendix C:
> 
> s/implmentation/implementation/

Done.

> Appendix G:
> 
> Fourth paragraph: As I mentioned above, RFC5451 has an appendix that talks
> at length about why you want to do things like SPF checks at the border
> rather than inside.

Added the reference.

> Appendix I:
> 
> The second paragraph claims we're documenting extensions since 4408.  Were
> there any?
Yes.

Use of A-R in place of Received-SPF in some cases and the "void lookups" 
limit.

Sorry for the delay in responding.

Scott K