spf-discuss
[Top] [All Lists]

Re: draft-schlitt-spf-00pre4 now available

2004-11-03 05:31:53
On Wed, 2004-11-03 at 02:35 -0600, wayne wrote:

I've done a bunch more work on the draft document for describing the
SPF-classic that I know of and that I will implement in libspf2.
Basically, I am trying to create a better written specification than
spf-draft-200406, but with all the same semantics that I can keep.


This document is *not* intended to be an official statement of what
SPF-classic is, that's up to Meng and MarkL.  I am *not* going to
submit this document to the IETF.  I *do* think people may find this
document useful, so I'm announcing pre-release versions here.


The documents can be found at:

http://www.midwestcs.com/spf/spf_classic_libspf2/draft-schlitt-spf-00pre4.html
http://www.midwestcs.com/spf/spf_classic_libspf2/draft-schlitt-spf-00pre4.txt
http://www.midwestcs.com/spf/spf_classic_libspf2/draft-schlitt-spf-00pre4.xml

A unix diff between draft-lentczner-spf-00.txt and
draft-schlitt-spf-00pre4.txt can be found at:
http://www.midwestcs.com/spf/spf_classic_libspf2/spf-lentczner-schlitt.diff.txt


Again, I am very interested in any feedback about this document.  I
want to know how it conflicts with existing SPF implementations and
how it differs from what you think SPF-classic should be.



One thing I tried hard to do with this my editing is to keep the spec
short.  While I have made a great deal of changes, restored a lot of
semantics from spf-draft-200406, and added a lot of stuff here and
there, the resulting document is only two pages longer than MarkL's.
If you remove the Received-SPF header stuff, it is almost exactly the
same length as MarkL's.

Basically, for everything I've added, I've gone and found less
important stuff (IMHO) to remove.  This is something I'm going to try
hard to continue to do.   This also explains what may seem like petty
deletions.


The following is an list of differences between the official
SPF-classic specification and my libspf2 documentation.  The
differences are listed in order that I found them by reviewing the
above mentioned diff.  As a result, the most important changes are
kind of mixed in with the rest.


* The "HELO" identity is explicitly defined

I'm torn here.

1) Its a bit ambigious as to WHEN one should perform a HELO evaluation.

2) When a HELO check fails, the session fails.  This is great if "you
MUST perform HELO evaluations", and everyone does it but we both know
that so many clients out there don't even do HELO, or pass it
forged/useless information.

I think you should word it like this:

SPF clients MUST check the "HELO" identity by calling the check_host()
function (Section 4The check_host() Function) with the "HELO" identity
as the <sender>. If the HELO test returns a "pass", the overall result
for the SMTP session is "pass", and there is no need to test the "MAIL
FROM" identity.  If the HELO test returns a "fail", the client may
continue with checking the "MAIL FROM" in search of a better result,
otherwise falling back on this "fail" result and ending the session.

This way, its easy to start doing HELO checks, but ignorant hosts won't
suffer.  If I am having to implement SPF, and I have a choice to perform
HELO, its not an easy one with much flexibility.  Written the way I have
it, it might be more palatable.

* I say that the SPF-classic spec has not been changing much since
  last winter. 

Yep.

* cosmetic changes:

  * MAIL FROM vs Mail From  --  most RFCs use the capitalized form

ok

  * Domain name vs host name  --  host names are a subset of domain names.

goood

* restored HELO checking from spf-draft-200406

* NXDOMAIN causes a "None" result rather than a "fail".  Note that
  resolvers will return NXDOMAIN when presented with a malformed
  domain names.

  This is kind of messy.  The various SPF classic specs have been
  inconsistent on this subject, and the various implementations that
  I've checked have not been consistent.   *sigh*.

I disagree... NXDOMAIN should return unknown.  This is the first
occurrence of conflicting logic. 

* The only thing that causes a "fail" result is a domain owner having
  mechanism in their SPF record that says so.

I'm undecided here.

* warning added about domain names with source routes, percent hacks
  and bang paths.

Good.

* note about what a Pass result is useful for

Good

* note about what you should do with a SoftFail

Good

* The examples in the spec use TXT RRs instead of SPF RRs

Good

* deleted section that says "additional records" should use _spf.%{d}

Ok

* process limit:  DNS packets that are too large MAY be ignored.

Ignoring leads to no discovery of ignorant / poorly published records.
Elsewhere you have made changes with the mindset that errors or problems
are caught and raised.  This is the second occurrence of conflicting
logic.

* examples and text encourage use of the implicit + on mechanisms.
  e.g. "mx:foo.com" instead of "+mx:foo.com".

* note that the definition of the check_host() function will likely
  need more arguments in real life than what is shown in the spec.
 
fair enough.

* deleted redundant explanation of results codes in section 4.2

ok

* all syntax errors must be detected, not just when they are
  "encountered". 

OUCH.  I definitely don't like this.  This is the third occurrence of
conflicting logic.

* removed support for unknown mechanisms.

ok

* redundant descriptions of modifiers removed from 4.6.3

ok

* the ptr: mechanism and the %{p} macro ignore DNS errors instead of
  triggering TempError.

Why?  This is the fourth occurrence of conflicting logic.

* process limits:  a limit of 10 MX name lookups per mx: mechanism
  evaluation.
  
* process limits:  a limit of 10 PTR lookups per ptr: mechanism
  evaluation.

I would prefer an overall recursion limit period.  This offers more
flexibility whilst still providing some security.

* explicitly say that IP address of 10.23.45 is invalid and not just
  the same as 10.23.45.00/24

GOOD.  I like this.

* allow other specifications to use unrecognized modifiers, not just
  newer versions of SPF.  (e.g., accredit= and ses=)

I wasn't aware that it was worded as such, I was under the impression
that it encouraged what you have there.  I like this.

* SPF implementations can provide a default explanation string if no
  exp= modifier is found.

Good.

* process limit:  no more than 10 mechanisms that do DNS lookups.

* process limit:  removed limit on the number of include: mechanisms
  because that is a subset of 10 mechanism limit.

*UGH*  This is getting overly complex.  I re-state my desire for a
simple global level of recursion.  Its more flexible, and a lot less
complex then having to track recursion for now four different things?
IMO this is unnecessary.  A global counter makes more sense.

This wording was perfect IMO:

If a loop is detected, or if more than 20 subqueries are triggered, 
   an SPF client MAY abort the lookup and return the result "unknown".

   Regular non-recursive lookups due to mechanisms like "a" and "mx" or
   due to modifiers like "exp" do not count toward this total.


* restored Received-SPF: header from spf-draft-200406

Ok.

* ABNF cleanup.  (Including allowing the "/" in domain specs again.)

Ok.

* Unknown % escapes are now syntax errors.  Trying to get the old
  semantics working with ABNF was really messy.

I agree with this.

* Unknown macro variables are syntax errors.

Not really sure if I like or don't like this... it affects extensibility
somewhat, but its still warranted I suppose.

* restored %{h} macro variable

no comment

* The %{p} macro variable should favor returning the sending domain.

no comment

* minor %{t} macro variable clarifications.

no comment

* give a suggestion about using "tracking exists: mechanisms" for finding
  your outgoing mail servers.

Interesting.

* section 9.2 "implications for mailing lists" replaced with a note
  that you have to follow the stuff that RFC2821 tells says you MUST
  do.

I should be surprised that there exist those that do not follow MUST,
but I'm not.

* security consideration:  don't let data provided by third parties
  cause problems.

..... if you include it, its your fault.  I like this better.  If you
include something thats broken, the spec should provide wording
appropriate to "a failure anywhere as a result of any included or
redirected query results in a warning or notification of some kind being
added to the Received-SPF: header" or something.

* When a macro string is expanded to more than 255 characters, the
  spec now says to truncate until it is less than *or equal* to 255

Good.

Overall I like it.  

I VEHEMENTLY disagree with:

* all syntax errors must be detected, not just when they are
  "encountered". 

You should not be specifying things like this.  It is unnecessary to
parse the entire SPF record.  Doing so requires a pre-parse, and its a
waste.  Currently you can halt whenever you encounter an explicit result
or error.  If I find one error I do not need to go and find more.  There
is only a single method or placeholder for returning an error, and no
way to specify that multiple errors were found, nor should there be.

This is bad bad bad, and entirely unnecessary.  SPF is already enough of
a pig with the DNS queries, suffice to say, being able to abort the
parse upon an explicit result is a huge win, please do not take this
away.

Also:

Four times your changes logically conflict with the overall tone in your
changes.  I dislike this lack of consistency.  Let me explain.  Overall
I get the tone that the changes are to aide in problems being found and
reported, so that overall SPF records do not sit idle in DNS for months
on end failing mail with no warning.  Am I on track?  Four times you
specify changes that completely conflict with this:

#1
--

* NXDOMAIN causes a "None" result rather than a "fail".  Note that
  resolvers will return NXDOMAIN when presented with a malformed
  domain names.

My spec does not return a "fail", is this a typo, or is this specified
in another draft?

My draft has what I believe to be correct:

If the domain does not exist (NXDOMAIN), SPF clients MUST return 
   "unknown".

You should use this.  Its correct.  NXDOMAIN by its very meaning is the
nonexistence of a domain.  This doesn't mean we should literally
interpret this to mean that its also not publishing SPF.  Since the
domain name is literally unknown, returning unknown is appropriate.
None will likely lead to confusion or assumption that a domain was
there, and it just wasn't publishing.  The key point here is that by
returning unknown it raises an eye brow because things should not be
returning unknown, if they are, there is a problem.  Returning NONE
hides this.

#2
--

* process limit:  DNS packets that are too large MAY be ignored.

Bad!  This again goes against the overall tone of your draft changes.
If a packet is ignored, then how do we know it happened?

I recommend this sentence be removed or changed to read:

* process limit: DNS packets that are too large MUST NOT be ignored and
MUST cause the client to return error

A packet that exceeds the maximum allowable length is an error and
should return such.  Any argument that the packet should be ignored or
accepted and truncated is not considering the necessity to explicitly
only accept valid records and the parsing thereof.

#3
--

* all syntax errors must be detected, not just when they are
  "encountered". 

I've already been over this one, but it is indeed the third logical
discrepancy that I've encountered.  There is no value in doing this.
PLEASE do not include it.

#4
--

* the ptr: mechanism and the %{p} macro ignore DNS errors instead of
  triggering TempError. 

This is the fourth one which again conflicts completely with the overall
tone presented here, and even in view of ignoring that, its a bad idea.
Its a bad idea because you should not IGNORE anything.  What if your
spell checker opted to ignore words without user input?  We are parsing
records for validity.  We need to be black and white here.  There should
be no MAY's, and only MUST.  I realize that this probably isn't likely,
but at the very least, you can't go about ignoring DNS related errors.
You __MUST__ return error.  How else will people know?  Sure DNS is
flakey, we accepted that when we started using DNS to be host to our
records.

I recommend you also remove this, in addition to the other three I
raised.


Overall again, I like the changes, but please, __PLEASE__ consider what
I have raised in complaint.  

I honestly think you should drop the entire 'libspf2' aspect of this,
and I urge that you work further with the rest of this group to publish
and submit a proper SPF draft.  Its clear you are capable of it.  This
document suffers because of personal preference decisions as opposed to
being logically consistent.

Thanks for taking the time to publish this Wayne, and I hope you pay
heed to my suggestions and my encouragement that you pursue this
document being published at some stage.

Cheers,

James

-- 
James Couzens,
Programmer
                                                     ( ( (      
      ((__))         __\|/__        __|-|__        '. ___ .'    
       (00)           (o o)          (0~0)        '  (> <) '    
---nn-(o__o)-nn---ooO--(_)--Ooo--ooO--(_)--Ooo---ooO--(_)--Ooo---
http://libspf.org -- ANSI C Sender Policy Framework library
http://libsrs.org -- ANSI C Sender Rewriting Scheme library
-----------------------------------------------------------------
PGP: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x7A7C7DCF

-------
Sender Policy Framework: http://spf.pobox.com/
Archives at http://archives.listbox.com/spf-discuss/current/
http://www.InboxEvent.com/?s=d --- Inbox Event Nov 17-19 in Atlanta features 
SPF and Sender ID.
To unsubscribe, change your address, or temporarily deactivate your 
subscription, 
please go to 
http://v2.listbox.com/member/?listname=spf-discuss(_at_)v2(_dot_)listbox(_dot_)com

Attachment: signature.asc
Description: This is a digitally signed message part