Subject: [apps-discuss] Suggestions for draft-ietf-appsawg-sieve-duplicate-02

From: Robert Sparks <[email-masked]>
Date: Thu, 23 Jan 2014 16:34:41 -0600

I was asked to take a look at draft-ietf-appsawg-sieve-duplicate-02.

I haven't been following the list discussion, but I skimmed it briefly 
and I don't _think_ any of the following are duplicate or already 
discussed. Apologies if that's wrong.

1) The draft suggests a server protect itself from large :seconds 
arguments by providing a maximum limit. Should it also suggest a limit 
on how long a string it would accept from a :header, a :uniqueid, or a 
:handle (especially given that the strings might come from using the 
variables extension)?

2) Should the draft say anything about potential interaction with 
duplicate suppression in already deployed systems (such as what's 
described in the first entry at 
<http://cyrusimap.web.cmu.edu/mediawiki/index.php/FAQ>?

3) Comparison of the values taken from :header is assumed to be bitwise 
(though case insensitive). Should that be made explicit? In particular, 
you're not expecting a duplicate check on :header "Foo" to treat 
messages with
Foo: a=one-thing; b=the-other
and
Foo: b=the-other; a=one-thing
as duplicates.

4) (minor, editorial) At the beginning of the description, I suggest 
adding "by default" to "Messages are identified by their message ID as 
contained in the Message-ID header".

5) (minor, editorial) It might make implementation less error prone to 
not use "message ID" at all, but say something like "Messages are 
identified by a 'seen' value computed from the message, which defaults 
to the contents of the Message-ID header. That would lead to sentences 
like "The 'duplicate' test MUST only check for duplicates amongst 'seen' 
values encountered in previous executions of the Sieve script; it MUST 
NOT consider 'seen' values encountered earlier in the current Sieve 
script execution as potential duplicates.

6) How did the group arrive at 7 days as the default "deemed to be 
appropriate"? (I don't particularly disagree with the number, but can't 
really support it either - can the document say why it was chosen and 
when a server might choose a different default?)

7) I would expect a sieve script writer to be surprised by one 
consequence of the duplicate test comparison happening independent from 
the source of the value. If there were two rules, one trying to look for 
duplicates based on the "Foo" header and one on the "Bar" header, and a 
pair of messages arrived, one containing
Foo: a
and the other
Bar: a
the second will be identified as a duplicate.
If that's not what the writer wanted, they can use :handle to straighten 
it out, but it's not immediately obvious that they need to.
Could a note be added pointing them at using :handle for cases like this?

Hope this helps,

RjS

---

From: Ned Freed <[email-masked]>
Date: Thu, 23 Jan 2014 16:11:51 -0800 (PST)

> :handle (especially given that the strings might come from using the

In regards to :header, I note that exactly the same considerations apply to the
header and address tests as well as the addheader and deleteheader actions. Not
having text about this elsewhere doesn't appear to have been a problem. It also
means that I don't really know what limit to suggest, if any. (Our
implementation limit is 256 characters in such a field name, but that's
probably needlessly large.)

As for :uniqueid and :handle, given that any sane implementation is going to
hash the values given to thsse parameters if they are large, and the variables
extension already lets you impose some fairly drastic limits on variable size,
I really don't see much point in imposing a specific limit here, especially
since Sieve allows for reasonable implementations limits in the base
specification.

> <http://cyrusimap.web.cmu.edu/mediawiki/index.php/FAQ>?

I don't think it's incumbent on any document to talk about nonstandard stuff
done by particular implementations that happens to have some degree of overlap.

> Foo: b=the-other; a=one-thing

Sorry, that's incorrect on both counts. First, the document is very clear that
the comparison is case-sensitive and you have to force the case if you want
sometihng different. From section 3:

   The tracked unique ID value MUST be matched case-sensitively,
   irrespective of whether it originates from a header or is specified
   explicitly using the ":uniqueid" argument.  To achieve case-
   insensitive behavior, the "set" command added by the "variables"
   [VARIABLES] extension can be used in combination with the ":uniqueid"
   argument to normalize the tracked unique ID value to upper or lower
   case.

Second, it's not an octet-by-octet comprison with :header - whitespace trimming
is part of the operation. Again from section 3:

   If the tracked unique ID value is extracted directly from a message
   header field, i.e., when the ":uniqueid" argument is not used,
   leading and trailing whitespace MUST first be trimmed from the value
   before performing the actual duplicate verification (see Section 2.2
   of RFC 5228 [SIEVE]).  Note that this also applies to the Message-ID
   header field used by the basic "duplicate" test without a ":header"
   or ":uniqueid" argument.  When the ":uniqueid" argument is used, such
   normalization concerns are the responsibility of the user.

> contained in the Message-ID header".

WFM.

> script execution as potential duplicates.

This doesn't sound much better to me. Or any worse, for that matter.

> when a server might choose a different default?)

A week seems like a reasonable default. I really don't know what else
can be said about it. (I think I said as much on the list when the topic
came up.)

> Could a note be added pointing them at using :handle for cases like this?

Couldn't hurt.

				Ned

---

From: Robert Sparks <[email-masked]>
Date: Thu, 30 Jan 2014 12:51:55 -0600

(Apologies for lag)
Trimming to a couple of points:

On 1/23/14, 6:11 PM, Ned Freed wrote:
> the comparison is case-sensitive 
Apologies, I got that bit flipped when writing this up. My calling out 
sensitivity
or not was triggered specifically by the text you quote below. However, 
that's
not really the main point I was hoping to discuss.
> and you have to force the case if you want
And I missed this. Does it apply also to what comes out of variables, or 
does
variable handling already ensure that you wouldn't have leading or trailing
whitespace to worry about?

But what I was really trying to is confirm (and I don't think you 
verified in your response)
that you aren't asking this code to learn about the details of any 
particular header
field and treat things that are semantically the same (such as what 
might result
from reordering name=value pairs as in the Foo example I gave above) as 
equal
for this comparison.

> This doesn't sound much better to me. Or any worse, for that matter.
I had asked a few non-IETF sysadmin/implementers to read this, and they
consistently hung on this point, saying they had to stop halfway through
and start rereading the document to make sure they kept when message ID
mean Message-ID and when it might not straight as they were reading.
The other suggestion to point out early that Message ID is Message-ID by
default might be enough, but this suggestion seemed to resonate with those
folks. Maybe you can see something else that would do just as well. (And
I'm ok if you leave it as is).

RjS

---

From: t.petch <[email-masked]>
Date: Fri, 31 Jan 2014 10:07:08 +0000

Robert

I am with you on these points.  Thus you cite the ambiguity of message
id while I cited unique id, same difference.

Looking back at those first comments I made, I now see that this I-D is
fine for Sieve expert implementers but lacks clarity for a wider
audience (which is where I fall - perhaps you do too).  Whether or not
that is ok I am less sure of; my starting point was that it should be
and am still inclined to that.

Tom Petch

----- Original Message -----
From: "Robert Sparks" <[email-masked]>
To: "Ned Freed" <[email-masked]>
Cc: <[email-masked]>; "IETF Apps
Discuss" <[email-masked]>
Sent: Thursday, January 30, 2014 6:51 PM
Subject: Re: [apps-discuss] Suggestions
fordraft-ietf-appsawg-sieve-duplicate-02

> > Second, it's not an octet-by-octet comprison with :header -
whitespace
> > trimming

---

From: Ned Freed <[email-masked]>
Date: Fri, 31 Jan 2014 11:42:04 -0800 (PST)

> does variable handling already ensure that you wouldn't have leading or trailing

The answer is no, it doesn't apply to stuff coming out of variables - and I
think the draft is clear about this.

As for stuff coming out of variables not needing such processing, it depends.
The header test strips leading and trailing spaces, so if it was used to load
the variable, then yes, there won't be leading and trailing spaces. But
variables can be assigned values in lots of ways, and if you do it in a way
that adds leading or trailing spaces, then that's going to be part of the
compared value. Which is the way it needs to work, because who's to say those
spaces aren't put there intentionally?

That said, given the way sieve works it's actually kind of hard to end up with
leading or trailing whitespace in a variable that you don't want or need. If
you do end up with it, then a string :matches test can always be used to get
rid of it.

> from reordering name=value pairs as in the Foo example I gave above) as

Of course not. And I have no idea how you could have possibly gotten the 
notion that would be desireable, let alone required, by anything in the draft.

More generally, this discussion is becoming less and less about the specifics
of this draft and more and more about general sieve semantics. The whitespace
semantics  here were intentionally chosen so they match those of the sieve
header and string tests - the former drops leading and trailing whitespace
while the latter does not.

And as for how the language embeds handling of header syntax, the general sieve
model for that is to define a separate tests for each semantic you're
interested in. The sieve base specification defines the header (entire content
of header minus leading and trailing whitespace) and the address (parse out
address from header, test specified part) tests, and subsequent drafts have
defined the date test (parse out date, test specified part(s) in specified
form), the index extension (test Nth appearing field), the subaddress extension
(add detail part to address test), and the mime extension (parse MIME
content-type and content-disposition fields, test specified part or parameter).
A combination of the header and string tests can also be used to implement
limited forms of header parsing. And it may make sense to add additional tests
in the future.

And since the matching part(s) of any test can be stored in a variable, all of
these provide ways to parse a header field and store the result in a variable.
Which then can be used as part of a uniqueid value.

But all of this is just part and parcel of using sieve; none of it seems
relevant to what is after all a draft defining a small, simple duplicate
elimination facility. If you want to argue that there should be some document
somewhere that explains all this, fine, but this isn't that document.

				Ned

---

From: Robert Sparks <[email-masked]>
Date: Fri, 31 Jan 2014 14:31:59 -0600

I think you're reading far more into what I'm saying than what's there.

Let me try to bring this up a level, and make a slightly different 
suggestion
based on your feedback.

Would you be willing to point out in the draft that if a script writer 
wanted
comparisons of headers to match even when parameters are reordered, they
need to do some work (probably using variables) themselves?

No is ok (and I'm guessing you don't think it's necessary based on the
exposition below), but I suggest it's a case where the script writers (who
aren't always going to be deeply steeped sieve experts when they start
out) are likely to stub their toe and a short nudge that this is something
for them to watch out for will help.

RjS

On 1/31/14, 1:42 PM, Ned Freed wrote:
> Of course not. And I have no idea how you could have possibly gotten 
> the notion that would be desireable, let alone required, by anything 
> in the draft.
>

---

From: Ned Freed <[email-masked]>
Date: Fri, 31 Jan 2014 15:29:00 -0800 (PST)

> Would you be willing to point out in the draft that if a script writer wanted

I'm not the author; what goes in the draft is not up to me.

That said, IMO saying something like this would be both pointless and a bad
idea. If you're checking headers that contain parameters (as opposed to the
straightforward checks of things like message-id and subject fields, which
don't contain parameter lists), you're either doing something wrong or you're
doing something very tricky indeed.

And if parameters were reordered, it's a reasonable supposition that it was
done as a consequence of something other change being made. How do you propose
to accomodate such a change in the context of a duplicate test? Given that, I
see any discussion of accomodating parameter reordering as likely to lead users
into doing things they should not be doing.

Frankly, I can't even think of a use-case where I'd want to feed a header field
whose syntax involves an unordered set of parameters into a duplicate test, let
alone one where I'd want to do the test in a way that needed to ignore
parameter ordering.

				Ned

---

From: Robert Sparks <[email-masked]>
Date: Fri, 31 Jan 2014 18:27:05 -0600

> alone one where I'd want to do the test in a way that needed to ignore
So, finding _safe_ use cases where you use anything besides Message-ID seems
really suspect, but that bridge appears to have been crossed.

The draft calls out an example of getting effectively the same message from
more than one monitoring agent, and looking at things like Subject to decide
to mark some messages "seen". Such a test might well want to look at the 
mime
parameters in Content-Type and see if any attachments are likely the 
same before
making than seen marking. Since these came from different monitoring agents,
the order of the parameters might well be very different.

Yeah, that's nuts, and I wouldn't want to rely on it, but it seems to be 
what's
motivating matching more than Message-ID in the first place.

RjS

---

From: Ned Freed <[email-masked]>
Date: Fri, 31 Jan 2014 19:01:05 -0800 (PST)

> really suspect, but that bridge appears to have been crossed.

Given that the draft gives not one but two perfectly reasonable examples of
such uses, this  claim appears to be bogus.

> the order of the parameters might well be very different.

Actually, no, it would not want to do that. The point of the example is to
avoid being showered with alerts about the same underlying event from different
systems. In such cases the alerts will at a minimum be formatted different and
may well have different content-types.

> what's motivating matching more than Message-ID in the first place.

I'm afraid I find this modification to be entirely unpersuasive.

				Ned

---

From: Ned Freed <[email-masked]>
Date: Fri, 31 Jan 2014 19:13:35 -0800 (PST)

> id while I cited unique id, same difference.

I have no idea what ambiguity you're referring to here.

> and am still inclined to that.

This makes no sense to me either. Are you saying that the extension is hard for
people writing sieves to use? If so, then I don't think you've made  a case for
that. Indeed, one of the things I find to be most appealing about this
extension is how simple it is to use.

And if you actually mean the people who implement sieve extensions, well,
yeah, if you're adding an extension to a Sieve interpreter you'd damn well
be familiar with the semantics of the language. How is it in any way
unreasonable to expect that?

				Ned

---

From: Robert Sparks <[email-masked]>
Date: Fri, 31 Jan 2014 23:44:49 -0600

> I'm afraid I find this modification to be entirely unpersuasive.
Wow.

I would seriously not want a message that had a pcap dump of what led to 
a serious situation marked read
because an earlier text message from some other system provided a 
shallow text overview of what happened,
but I wouldn't try to use this extension this way anyway.

Maybe that's your point - that someone that cared wouldn't write a 
script that does that.
But I feel the current text points down that path.

RjS

---

From: Ned Freed <[email-masked]>
Date: Fri, 31 Jan 2014 22:08:49 -0800 (PST)

> but I wouldn't try to use this extension this way anyway.

So?  Your point appears to be that if the situation was completely different -
in particular, if these weren't simple alerts the text is clearly talking
about, but rather a mixture of alerts and messages containing problem details -
then it wouldn't make sense to handle them this way.

That's obviously true, but what it has to do with the actual example given
escapes me completely.

> Maybe that's your point - that someone that cared wouldn't write a

Actually, while I didn't come up with this example, I do something very close
to it to limit the amount of useless email I get from my Sonicwall appliance.

I also note that this entirely different use-case you've constructed still
fails to provide an example where reordering parameters would be part of the
resulting script. If the presence of a certain type of attachment makes the
content critical to see, then the right thing to do is to exempt such 
messages from the test. Something like:

   require ["duplicate", "variables", "imap4flags",
     "fileinto", "mime"];

   if header :matches "subject" "ALERT: *" {
     if allof (not (header :mime :anychild :contentype :is
               "content-type" "application/vnd.pcap"),
               duplicate :seconds 60 :uniqueid "${1}" {
       setflag "\\seen";
     }
     fileinto "Alerts";
   }

Or maybe even:

   require ["duplicate", "variables", "imap4flags",
     "fileinto", "mime"];

   if header :matches "subject" "ALERT: *" {
     if header :mime :anychild :contentype :is
               "content-type" "application/vnd.pcap" {
       fileinto "Pcap-info";
     } elsif duplicate :seconds 60 :uniqueid "${1}" {
       setflag "\\seen";
     }
     fileinto "Alerts";
   }

> But I feel the current text points down that path.

What text? The example? I really have no idea where you're getting the notion
that the text is saying that everything will still be copacetic if you feed
insufficiently precise information into the test.

				Ned