Subject: [sieve] Comments on Sieve include (draft-ietf-sieve-include-04.txt)

From: Alexey Melnikov <[email-masked]>
Date: Wed, 04 Aug 2010 11:34:04 +0200

As discussed in Maastricht, the document should contain some text about 
use of include with ManageSieve.

3.1.  General Considerations

   Sieve implementations must track the use of actions in included

s/must/MUST ?

   scripts so that implicit "keep" behavior can be properly determined
   based on whether any actions have executed in any script.

   Sieve implementations MUST ensure that recursive includes are not
   possible.  For example, if script "A" includes script "B", and script
   "B" includes script "A" an error MUST be generated either when the
   script is uploaded to the Sieve repository,

I wonder if this is too strong and if this should be replaced with
"activated". I am thinking about a script A that includes B is being
uploaded, while the older version of B already includes A.

   or when the script is
   executed.  If such an error is detected whilst processing a Sieve
   script, an implicit "keep" action MUST be executed to prevent loss of
   any messages.

3.2.  Control Structure include

   Personal script "spam_tests"

      This script does some user-specific spam tests to catch spam
      messages not caught by the site-wide spam tests.

      require ["reject"];

This needs an informative reference to Sieve reject.

      if header :contains "Subject" "XXXX"
      {
          reject;

The human readable string is not optional for "reject" action, so this 
doesn't look like a valid Sieve script.

      }
      elsif header :is "From" "[email-masked]"
      {
          reject;

As above.
      }

Also the same issue is present in:
   Site script "spam_tests"

4.  Security Considerations

   Sieve implementations MUST ensure adequate security for the global
   script repository to prevent unauthorized changes to global scripts.

This sounds a bit handwavy. Can you add an example on how can this be
achieved?

---

From: Stephan Bosch <[email-masked]>
Date: Wed, 04 Aug 2010 15:09:06 +0200

On 08/04/2010 11:34 AM, Alexey Melnikov wrote:
> As discussed in Maastricht, the document should contain some text 
> about use of include with ManageSieve.

It does describe some parts of the interaction with ManageSieve already 
actually, but I see what you mean.

> uploaded, while the older version of B already includes A.

I guess this is related to the ManageSieve upload problem discussed at 
the meeting. A quick recap:

In the current ManageSieve specification the script is verified 
(compiled) upon upload. When include is involved, verification of each 
individual script after it is uploaded can cause trouble in multiple 
ways. Including the above, the examples we've thought of thus far are:

- Race condition of the script interpreter using a mix of new and 
outdated scripts while ManageSieve is in the process of uploading. Older 
versions of not-yet-uploaded scripts can malfunction with 
included/including scripts that are already uploaded.

- Existing scripts that make uploading a new version of another script 
impossible due to verification errors, with the circular include as a 
notable example.

- Uploading scripts could trigger missing-included-script errors when 
the scripts are not uploaded in the correct order. Currently, the 
include specification addresses this one already: "Sieve implementations 
MUST generate an error at execution time if an included script does not 
exist. Implementations MUST NOT generate errors for scripts missing at 
upload time, as this would force an upload ordering requirement upon 
script authors / generators." (Section 3.1; page 5). But, in light of 
the above, a better, more generic solution could be possible.

I think a good solution would fix all of the issues above in one stroke.

One of the posed suggestions to fix this is to re-activate the top-level 
script with SETACTIVE after the whole include tree is uploaded. The 
interpreter then won't use the newly uploaded scripts until the main 
script is re-activated, even though it may be already active. This 
partly moves the verification step (at least as far as the include 
extension is concerned) to either the activation stage or when the 
scripts are compiled/run by the interpreter. A problem with this 
solution is that existing clients only use SETACTIVE to change the 
active script and not as a means to 'commit' an upload 'transaction'.

Any other ideas?

>      messages not caught by the site-wide spam tests.

<snip>Errors in example scripts; missing reject argument</snip>

I reported a few more in april 2009 (updated to v05):

   * page 11; active script:3: "test-mailbox" is not a valid variable name
   * page 11; active script:14: missing require for relational 
extension, since :count is used
   * page 11; active script:14: the :count match type needs a string 
argument, e.g. "eq"
   * page 11; spam_filter_script:6: "spam-${test}; <- missing "

Regards,

Stephan.