Subject: Re: [spfbis] Review of draft-ietf-spfbis-4408

From: "Murray S. Kucherawy" <[email-masked]>
Date: Mon, 4 Mar 2013 23:56:27 -0800

It looks like the vast majority of this stuff has been resolved in the
latest version.  I'll touch on the few that still remain open.  Other than
these, we only have three open tickets (two closely related).  Does that
mean we're almost done?

On Mon, Jul 16, 2012 at 7:30 AM, Scott Kitterman <[email-masked]> wrote:

> > The chatter in Paris suggested simply adding a paragraph before the
> > definition of check_host() that says emphatically that this is not an API
> > definition, but rather the mechanism is used to illustrate the
> algorithm; a
> > compliant SPF implementation MUST do something semantically equivalent to
> > what we describe for check_host().  Pete said this was probably
> > acceptable.  It's probably less work than all the s// I did here as well.
> > We should probably try that approach, in retrospect.
>
> OK.  I'll take a shot at that in the next revision.  Thanks.
>

I see you've done so at the top of Section 4.  However, there are six
references to check_host() prior to that point.  It might be good to
include a short intro paragraph as a new subsection of Section 1 so that
those references aren't dangling.  Something like:

1.2.  check_host()

Section 4 introduces an algorithm to evaluate an SPF policy against an
arriving email transaction.  In an early implementation, this algorithm was
encoded in a function called check_host().  That name is used in this
document as symbolic of the SPF evaluation algorithm, but of course
implementers are not required to use this name.

> > > 2.5.6.  TempError
> > > >
> > > >    A "temperror" result means that the SPF client encountered a
> > > >    transient error while performing the check.  Checking software can
> > > >    choose to accept or temporarily reject the message.  If the
> message
> > > >    is rejected during the SMTP transaction for this reason, the
> software
> > > >    SHOULD use an SMTP reply code of 451 and, if supported, the 4.4.3
> > > >    enhanced status code.
> > > >
> > > > [MSK: Do we really need to spell out SMTP reply codes and enhanced
> > > > status
> > > > codes?  Isn't it enough to say those codes are defined in RFC5321 and
> > > > RFC3463, and leave it at that?]
> > >
> > > I think we should.  There was an erratum about one of them being
> missing
> > > and
> > > it would be odd to resolve that erratum by removing them all.
> > >
> > >  Additionally,
> > >
> > > I'm not sure it's 100% obvious.  When we were reviewing that erratum,
> we
> > > discovered RFC 4408 got one of them wrong.  I think there is an
> > > interoperability point here about consistency that it's worth not
> removing
> > > text for.
> >
> > What do others think?  I always prefer to refer to other protocols (or
> > specific parts of them) rather than copying details from them.
>
> This always seemed to me like adding specificity, not mere copying.
>

I'm still partial to omitting these, but I appear to be solo on that one,
so I'll let it drop.

> > > >    For several mechanisms, the <domain-spec> is optional.  If it is
> not
> > > >    provided, the <domain> is used as the <target-name>.
> > > >
> > > > [MSK: How are <domain-spec> and <domain> different?  Do we need them
> > > both?]
> > >
> > > Domain-spec is a defined ABNF term.  Domain isn't.  It's one of the
> input
> > > arguments to check_host()/CheckHost defined in section 4.1.  In the
> case
> > > of an
> > > SPF record like:
> > >
> > > example.com In TXT "v=spf1 a:mail.example.com -all"
> > >
> > > the domain is example.com, an input value applicable to the entire
> > > evaluation
> > > (see redirect= for a limited exception to this) while domain-spec is a
> > > property associated with a particular mechanism or modifer.
> > >
> > > I think they are distinct enough that we ought not try and combine
> them.
> >
> > OK, then I think the right thing would be to say that they are
> > syntactically identical, but used in different parts of the evaluation
> > process.  In ABNF, something like this would do:
> >
> > domain = <whatever the definition we want to use is>
> >
> > domain-spec = domain
> >   ; syntactically identical to "domain", but used differently in the
> > evaluation
> >
> > Dave, do you have any other insight here?
>
> The current ABNF says:
>
>    domain-spec      = macro-string domain-end
>
> So you're proposing changing this to:
>
>    domain-spec      = macro-string domain-end / domain
>                                     ; syntactically identical to "domain",
> but
>                                     ; used differently in the evaluation
>
> This doesn't read quite right to me.  I think it's actually the other way
> around:
>
>    domain-spec      = macro-string domain-end / domain
>    domain                 = domain-spec / "input value"
>                                     ; either provided as an input for the
>                                     ; evaluation process or a domain-spec
> used
>                                     ; as an input for recursive evaluation
>
> As I've said before though ABNF is not my thing, so that might be totally
> wrong.
>

Barry, could you comment on this?

> > >    ip4-network      = qnum "." qnum "." qnum "." qnum

> > > >    qnum             = DIGIT                 ; 0-9
> > > >
> > > >                       / %x31-39 DIGIT       ; 10-99
> > > >                       / "1" 2DIGIT          ; 100-199
> > > >                       / "2" %x30-34 DIGIT   ; 200-249
> > > >                       / "25" %x30-35        ; 250-255
> > > >
> > > >             ; as per conventional dotted quad notation.  e.g.,
> 192.0.2.0
> > > >
> > > > [MSK: Is there really not a reference for this syntax?]
> > >
> > > There probably is, but we didn't find it in 2005.  If someone can dig
> it
> > > up, I'll add it.
> >
> > I actually can't find one either.  Pete or Dave, perhaps?
>
> OK.  Standing by.
>
> > > >    key-value-pair   = key [CFWS] "=" ( dot-atom / quoted-string )
> > > >
> > > >    key              = "client-ip" / "envelope-from" / "helo" /
> > > >
> > > >                       "problem" / "receiver" / "identity" /
> > > >
> > > >                        mechanism / name
> > > >
> > > >    identity         = "mailfrom"   ; for the "MAIL FROM" identity
> > > >
> > > >                       / "helo"     ; for the "HELO" identity
> > > >                       / name       ; other identities
> > > >
> > > > [MSK: Should "identity" be quoted in the "key" list?]
>
> I think it should not.
>

I agree, but it still is in the current version.

>
> > > > [MSK: There are two paths to "key" generating "name": one direct, one
> > > > through "identity"]>
> > >
> > > Is this a problem?
> >
> > I thought it was considered an ambiguous grammar in that case, and thus
> > "improper".
> >
> > You should run this through a BNF checker too just to see if any bugs are
> > found.  There's one on tools.ietf.org someplace.
>
> I did already.  The previous round of ABNF discussion we had we engendered
> by
> just such a trip to Bill's ABNF checker.
>

Barry, another ABNF question for you.

>
> > > >    macro-expand     = ( "%{" macro-letter transformers *delimiter
> "}" )
> > > >
> > > >                       / "%%" / "%_" / "%-"
> > > >
> > > > [MSK: The "*delimiter" allows this: %{s.-+,/_=.+,..+.}  What would
> that
> > > > mean?]>
> > >
> > > Good question.  It's almost 1AM, so I'm not even going to try and
> figure
> > > that
> > > one out right now.  What does it mean?
> >
> > My point is that the grammar allows it, and I'm not certain we want that.
> > What's the point of allowing lots of delimiters?  How would one apply
> more
> > than one?
>
> One could publish an SPF record that would match multiple sequential
> delimiters.  That would be odd, and not very useful, but I don't think
> it's a
> problem.  I don't think there's an interoperability benefit to modifying
> the
> ABNF to prohibit it.
>

> > > >    By default, strings are split on "." (dots).  Note that no special
> > > >    treatment is given to leading, trailing, or consecutive
> delimiters,
> > > >
> > > > [MSK: In input strings, right?]
> > >
> > > Yes.
> >
> > So if the "delimiter" part of the ABNF is "+.", what does a parser do?
> > Break on +, then within those break on .?  Break on either (like strtok()
> > does)?
>
> OK.  Now I'm not sure about my previous comment.  Let's hold this until the
> overall macro discussion is resolved.
>

What do you think now?  :-)

 -MSK

---

From: Barry Leiba <[email-masked]>
Date: Tue, 5 Mar 2013 10:06:45 -0500

> Barry, could you comment on this?

I think this is confusing because you're mixing syntactic elements
with values used in the protocol.  We do this often, and it's not
usually a problem, but here it can be: if "domain-spec" might be the
thing that's in the domain-spec spot in the ABNF, but sometimes might
come from the <domain> argument to check_host().

Happily, there's already an answer built in, right there in Section 4.8:

   This
   domain is called the <target-name> in the rest of this document.
...
   For several mechanisms, the <domain-spec> is optional.  If it is not
   provided, the <domain> is used as the <target-name>.

When you're talking about the syntactic element, use "domain-spec"
(without angle-brackets) consistently.  When you're talking about the
value that's used in an algorithm, which might have come from the
domain-spec element or might have come from the <domain> argument, use
"<target-name>" consistently.

Also happily, you're already mostly doing that, so there's almost no
change needed.  Any change to the ABNF for this seems quite wrong;
please don't do that.

I see that "<domain-spec>" is used three times with angle-brackets.
The first (quoted above), in Section 4.8, should simply lose the
brackets.  The other two, in Section 5.5, look wrong.  I think they
should be "<target-name>", but someone more familiar with the
algorithm should check that.  (Even though it's in a "do not use"
section, we should get it right.)

All the uses of "domain-spec" without the angle-brackets look correct
to me, as do all the uses of "<target-name>".  The last line of
Section 4.8 has "target-name" without angle-brackets, and should be
changed.  There's also one of those in Section 6.1.

So that's it: change the three occurrences of "<domain-spec>" and two
occurrences of "target-name", and I think everything is clear.  If you
want to address Murray's confusion, you could say that Murray's being
Martian and there's no change needed, or you could just do something
like this:

OLD
   For several mechanisms, the <domain-spec> is optional.  If it is not
   provided, the <domain> is used as the <target-name>.

NEW
   For several mechanisms, the domain-spec is optional.  If it is not
   provided, the <domain> from the check_host() arguments (see
   Section 4.1) is used as the <target-name>.

END

Barry

---

From: Scott Kitterman <[email-masked]>
Date: Sat, 16 Mar 2013 14:17:01 -0400

> implementers are not required to use this name.

Done.

> > missing and
> > we discovered RFC 4408 got one of them wrong.  I think there is an
> > removing text for.

Thanks.

> > not provided, the <domain> is used as the <target-name>.
> > inputarguments to check_host()/CheckHost defined in section 4.1.  In the
> > caseof an SPF record like:
> > > > 

Barry did comment.  Will follow up to his reply.

> > it up, I'll add it.

Via a separate discussion, we resolved this the other way around.  identity 
and mechanism are now both quoted.  The 4408 ABNF was wrong, but backwards 
from what I originally thought.

> > by just such a trip to Bill's ABNF checker.
> 

I think we've resolved this one in -11.

> > that mean?]>
> > > > figure that one out right now.  What does it mean?
> > > 
> > > more than one?

I think it's not causing problems, we ought not to mess with it.

Scott K

---

From: Scott Kitterman <[email-masked]>
Date: Sat, 16 Mar 2013 15:14:24 -0400

On Tuesday, March 05, 2013 10:06:45 AM Barry Leiba wrote:
> >> > > (see redirect= for a limited exception to this) while domain-spec is
> >> > > a property associated with a particular mechanism or modifer.
> >> > > 

Done and I agree the ones in 5.5 should be changed to target-name.

Done.

>    Section 4.1) is used as the <target-name>.

Done also.  Thank you for the detailed analysis and recommendations.

Scott K

---

From: Stuart Gathman <[email-masked]>
Date: Fri, 22 Mar 2013 21:44:27 -0400

>> these, we only have three open tickets (two closely related).  Does that
In reading through 
http://tools.ietf.org/html/draft-ietf-spfbis-4408bis-11, I had a problem 
with section 4.6.4.  While putting on my implementer/test suite hat, it 
was not at all clear whether

  SPF implementations MUST limit the number of mechanisms and modifiers
    ("terms") that cause any DNS query to at most 10 during SPF
    evaluation.

means that the *total* number of mechanisms and modifiers is limited to 
10, or that each type of mechanism/modifier is limited to 10. Looking at 
the current test suite, I see the answer is "total number of all types", 
but it was not clear from reading it while pretending I forgot all that 
stuff.

---

From: Scott Kitterman <[email-masked]>
Date: Mon, 25 Mar 2013 15:30:41 -0400

> but it was not clear from reading it while pretending I forgot all that

OK.  I guess you're better at pretending than I am.  Here's what I'll try on 
the group for the next revision:

>    SPF implementations MUST limit the total number of mechanisms and
>    modifiers ("terms") that cause any DNS query to at most 10 during SPF
>    evaluation.  Specifically, the "include", "a", "mx", "ptr", and
>    "exists" mechanisms as well as the "redirect" modifier count against
>    this collective limit.

Scott K