Subject: Re: Working Group Last Call: draft-ietf-sieve-spamtestbis-01.txt

From: Alexey Melnikov <[email-masked]>
Date: Thu, 01 Sep 2005 16:10:16 +0100

Alexey Melnikov wrote:

> I would like to draw your attention to the following draft:
>
> <http://www.ietf.org/internet-drafts/draft-ietf-sieve-spamtestbis-01.txt>
>
> A two week working group last call of this document starts today on 
> August 15th 2005 and ends on Monday, August 29th 2005 at 6 pm EST.

Hi folks,
The WGLC has ended. I've sent 2 editorial corrections directly to the 
author.
I've seen no comments on the mailing list, however I can't interpret 
that that the document is just perfect and can be sent to IESG.
So can I please ask people who have reviewed the document to send me a 
short note saying that the document is Ok. I suggest that you volunteer, 
or I will start picking some victims!

Alexey

---

From: Ned Freed <[email-masked]>
Date: Thu, 01 Sep 2005 09:45:49 -0700 (PDT)

> or I will start picking some victims!

I just reviewed the draft. A few comments...

First and most important, the specification fails to state what value is
checked by spamtest :percent when no spam test has been done. There are
basically four possibilities:

(1) Return an error. This has the effect of requiring robust scripts to
    perform a spamtest :value "eq" :comparator "i;ascii-numeric" "0"
    prior to doing a spamtest :percent. I don't think this is a good
    idea.

(2) Return 0, which of course conflates no test with test and found to
    be clear.

(3) Return an out of range value like "" or "-1" or "<untested>" or whatever.

(4) State that implementations are free to return whatever they want in this
    case. This more or less reduces to (1).

I think either (2) or (3) is the right choice.

Having the specification be silent on this issue is not good since people
will code their scripts to whatever behavior they see locally.

The document states that require ["spamtest", "spamtestplus"] SHOULD NOT be
done. It is hard to see how the use of requirements language can be usefully
applied to script construction. An implementation has to accept this and most
implementations have no way of warning about problematic usage (nor do I think
generating a warning would be appropriate in this case).

It would be better to say something like "In the interests of brevity and
clarity, scripts should specify either spamtestplus or spamtest in their
require clause, but not both".

It would also be good to reword the "MUST NOT use spamtest :percent without
require spamtestplus" as a sieve implementation requirement rather than a
script coding requirement. Maybe something like "sieve implementations MUST
return an error if :percent is used and spamtestplus is not specified".

Finally, the first and second parts of the security considerations, although
identical to RFC 3685, could use some clarification. THe first part currently
says:

   SIEVE implementations SHOULD ensure that "spamtest" and "virustest"
   tests can only occur for messages that have gone through a legitimate
   spam or virus check process.  If such checks rely on the addition of
   special headers to messages, it is the responsibility of the
   implementation to ensure that such headers cannot be spoofed by the
   sender, to prevent the implementation from being tricked into
   returning the wrong result for the test.

How can an implementation possibly control when scripts perform certain tests?
The intent, I think, is to say that the tests should only return an indication
that a test has been done when a legitimate test really has been done. So how
about:

   SIEVE implementations SHOULD ensure that "spamtest" and "virustest"
   tests only report spam and virus test results for messages that actually
   have gone through a legitimate spam or virus check process.  In particular,
   if such checks rely on the addition and subsequent checking of private
   header fields, it is the responsibility of the implementation to ensure
   that such headers cannot be spoofed by the sender or intermediary and
   thereby prevent the implementation from being tricked into returning the
   wrong result for the test.

The second part says:

   Server administrators MUST ensure that the virus checking tools are
   kept up to date, to provide reasonable protection for users using the
   "virustest" test.  Users should be made aware of the fact that the
   "virustest" test does not provide a 100% reliable way to remove all
   viruses, and they should continue to exercise caution when dealing
   with messages of unknown content and origin.

This is a fine bit of motherhood and apple pie, but the use of MUST here is
inappropriate IMO since the behavior of server admins has little if anything to
do with sieve implementation compliaance.

That's it!

				Ned