Subject: Re: My open issues with RFC3028bis

From: Michael Haardt <[email-masked]>
Date: Fri, 1 Jul 2005 11:02:27 +0200

On Thu, Jun 30, 2005 at 10:37:29PM -0700, Philip Guenther wrote:
> Is it unclear that an 'address' or 'header' test of header fields
> that are not present in the actual message evaluates to false?

No, that's fine.  It is unclear what a test of header fields does
if the specified field name is syntactically invalid.

Compare this with redirect: What happens if a redirect address string
does not contain a syntactically valid address?

All it says is:

   Implementations MUST ensure that the addresses are syntactically
   valid, but need not ensure that they actually identify an email
   recipient.

So whatever happens, implementations MUST not execute redirect with
an invalid address.  But what else will be the result? An error?

> By 'base spec' you mean the MIME base spec(s) and not the sieve
> base spec?

Yes.

> Regarding RFC 2047, section 6.3, I would read that as permitting
> implementations to skip malformed encoded words, even as it bans
> them from refusing to process the message.

That makes sense, now that I read it again, as does section 6.2,
which (among other) allows treating it literally.

> Regarding the NUL character issue, you wrote:
> >>Note that by considering Sieve to be a MUA, RFC 2047 can be
> >>interpreted in a way that NUL characters truncating strings is
> >>allowed for Sieve implementations, although not recommended.  It
> >>is further allowed to use encoded NUL characters in headers, but
> >>that's not recommended either.

Section 5, almost at the end:

   Only printable and white space character data should be encoded using
   this scheme.  However, since these encoding schemes allow the
   encoding of arbitrary octet values, mail readers that implement this
   decoding should also ensure that display of the decoded data on the
   recipient's terminal will not cause unwanted side-effects.

NUL is not white space.  If a word contains either, its author did not
care about this recommendation is and it may be a good idea to drop it
or display its decimal representation or whatever else.  Most likely
the above addresses ESC sequences.

NUL characters do truncate strings in some Sieve implementations, clearly
an unwanted side effect of something not being printable or white space.
The Sieve implementation should ensure that does not happen, but it's just
"should", not MUST, as MUA implementations may display ESC sequences,
even if that's a bad idea.

I may be stretching it too far here, but AFAIK, there are implementations
that truncate strings, thus corrupting test results.  Trying to label them
non-conforming probably won't succeed, but we should not silently ignore
this problem.

> 	Capability string are case-sensitive; for example, "foo"
> 	and "FOO" are different capabilities.

Good.

>    Implementations MUST perform syntactic, semantic, and run-time checks
>    on code that is actually executed.  Implementations MAY perform those
>    checks or any part of them on code that is not reached during
>    execution.

Much better than my suggestion!

Michael

---

From: Michael Haardt <[email-masked]>
Date: Fri, 1 Jul 2005 11:12:31 +0200

On Thu, Jun 30, 2005 at 03:44:04PM -0700, Ned Freed wrote:
> >    A header name never contains a colon.  The "From" header refers to a
> >    line beginning "From:" (or "From   :", etc.).  No header will match
> >    the string "From:" due to the trailing colon.

> > If users
> > write "From:", most likely they made a mistake.  Should that really not
> > cause an error?
> 
> If we were specifying this for the first time, yes, absolutely. But we're not
> doing that.

Ok, if the specification is to be understood that way, then how about:

  Synactically invalid header names cause the same result as syntactically
  valid header names that are not present in the message.  In particular,
  an implementation MUST not cause an error for synactically invalid
  header names.

> (1) What to do about "incorrect" encoded words, where the "incorrectness"
>     manifests as them being syntactically invalid according to RFC 2047.
>     In this case I believe RFC 2047 rules should apply: This is simply
>     literal text, not encoded words.

Agreed, that is specified clearly.

> (2) What to do about "incorrect" encoded words where the encoded payload
>     doesn't match up with the rules for the specified encoding. From what
>     I can tell RFC 2047 is silent on this issue. However, RFC 2047 does
>     suggest that encoded-words that specify an unrecognized encoding might
>     best be treated as ordinary text. I think it appropriate to suggest, but
>     not require, this handling for encoded words with busted encoded
>     content.

What do you mean by ordinary text? Treating them literally or decoding, but
not converting them?

> We do case-sensitive matches on require strings, so this is OK as far as we're
> concerned. Hopefully there isn't a popular implementation that has spawned a
> corpus of require "ENVELOPE"; scripts out there.

The only feedback I got when I changed it from caseless to caseful was that
a regression test failed that used FILEINTO.  That was easily fixed.

Michael

---

From: Ned Freed <[email-masked]>
Date: Fri, 01 Jul 2005 14:30:53 -0700 (PDT)

>   an implementation MUST not cause an error for synactically invalid

Very nice. I support adding this text to the document. One change: "MUST not"
-> "MUST NOT".

> What do you mean by ordinary text? Treating them literally or decoding, but

Treating them literally. Hard to decode something you don't understand the
encoding for.

> a regression test failed that used FILEINTO.  That was easily fixed.

I was frankly surprised that I coded it using a case-sensitive comparison and
that nobody has complained about it. Had I thought about it I probably would
have used a case-insensitive comparison.

Sometimes you get lucky, that's all.

				Ned

---

From: Philip Guenther <[email-masked]>
Date: Fri, 01 Jul 2005 17:58:04 -0700

Ned Freed <[email-masked]> writes:
>Michael Haardt wrote:
>Very nice. I support adding this text to the document. One change:
>"MUST not" -> "MUST NOT".

Works for me.  2.4.2.2 now reads:
   Headers are a subset of strings.  In the Internet Message
   Specification [IMAIL], each header line is allowed to have whitespace
   nearly anywhere in the line, including after the field name and
   before the subsequent colon.  Extra spaces between the header name
   and the ":" in a header field are ignored.

   Similarly, synactically invalid header names cause the same result as
   syntactically valid header names that are not present in the message.
   In particular, an implementation MUST NOT cause an error for
   synactically invalid header names.

   Folding of long header lines (as described in [IMAIL] 2.2.3) is
   removed prior to interpretation of the data.  The folding syntax (the
   CRLF that ends a line plus any leading whitespace at the beginning of
   the next line that indicates folding) are interpreted as if they were
   a single space.

<pause>

Hmmm, that last paragraph actually differs from RFC 2822, section 2.2.3,
which says:
   Unfolding is accomplished by simply removing any CRLF
   that is immediately followed by WSP.

I.e., the leading whitespace should *not* be treated as a single space
but rather be left as is.  Unless I hear screams, I'm going to remove
the sentence that starts "The folding syntax..." as conflicting.

Philip Guenther

---

From: Ned Freed <[email-masked]>
Date: Fri, 01 Jul 2005 18:29:45 -0700 (PDT)

> the sentence that starts "The folding syntax..." as conflicting.

Nice catch. No screams from this quarter - this should align with RFC 2822 IMO.

				Ned

---

From: Philip Guenther <[email-masked]>
Date: Fri, 01 Jul 2005 22:16:01 -0700

>On Thu, Jun 30, 2005 at 10:37:29PM -0700, Philip Guenther wrote:
...
>[RFC 2047] Section 5, almost at the end:
>

It's valid syntax, and Unicode has a NUL codepoint, so it's not like we
can claim that something like:

From: Philip =?US-ASCII?Q?=00_Guenther?= <[email-masked]>

can't be converted to Unicode, so the new text in 2.7.2 doesn't cover it.

...
>non-conforming probably won't succeed, but we should not silently ignore

I guess there are two choices:
A) Require correct handling of NUL
B) Strongly prefer correct handling of NUL and warn about the dangers of
   not doing so in the security considerations

On that thought, there's a security consideration missing.  As a
first draft:

   As with any filter on a message stream, if the sieve implementation
   and the mail agents 'behind' sieve in the message stream differ in
   their interpretation of the messages, it may be possible for an
   attacker to subvert the filter.  Of particular note are differences
   in the interpretation of malformed messages (e.g., missing or extra
   syntax characters) or those that exhibit corner cases (e.g., NUL
   octects encoded via [HEADER-CHARSET]).

Of course, this problem is not specific to email; I recall reading
in the last 90s about using differences in IP fragment handling to evade
networking monitoring devices.  The first case of this problem in email
that I recall involved virus filters that were stricter about their MIME
handling than MS Outlook was, such that the filter would fail to scan
MIME parts which had a syntax error in their MIME headers while Outlook
would accept them sufficient to be infected.

The question is: is there anything we can say on this other than "this
is a problem"?  Recommending that sieve implementations be more liberal
(than MUAs) doesn't work because the attack works both ways.  Rewriting
messages to make them strictly compliant has a horrible history, while
rejecting everything that's even slightly suspect is politically
impossible.  Do we just document it, grunt, and move on?

Philip Guenther

---

From: Ned Freed <[email-masked]>
Date: Sat, 02 Jul 2005 07:35:13 -0700 (PDT)

>    not doing so in the security considerations

I have no major problem with A but I think B is a better choice. FWIW, the
implementation I work on has no problem handling NULs, but I worry that
this will make many other implementations non-conforming.

> impossible.  Do we just document it, grunt, and move on?

Seems like the best we can do, doesn't it?

				Ned

---

From: Alexey Melnikov <[email-masked]>
Date: Mon, 04 Jul 2005 13:43:48 +0100

>the sentence that starts "The folding syntax..." as conflicting.
RFC 2822, section 3.2.3 also contains:

   Runs of FWS, comment or CFWS that occur between lexical tokens in a
   structured field header are semantically interpreted as a single
   space character.

---

From: Kjetil Torgrim Homme <[email-masked]>
Date: Tue, 05 Jul 2005 15:11:30 +0200

On Fri, 2005-07-01 at 17:58 -0700, Philip Guenther wrote:
> Works for me.  2.4.2.2 now reads: [...]
> 

that's fine, but I find it a bit misleading to only refer to [IMAIL],
since RFC 2047 changes the folding rule in the presence of two
encoded-words:  any folding whitespace between them must be removed (see
section 6.2 paragraph 3).

e.g.,
   Subject: =?utf-8?q?hell?=
       =?utf-8?q?o?=
should be presented as:
   Subject: hello

on the other hand,
   Subject: =?utf-8?q?hell?=
       o
should be folded into
   Subject: hell    o

my suggestion is to add the reference, perhaps like so

        Long header lines are unfolded (as described in [IMAIL] 2.2.3
        and [MIME] part 3 6.2) prior to interpretation of the data.

-- 
Kjetil T.

---

From: Ned Freed <[email-masked]>
Date: Tue, 05 Jul 2005 07:48:15 -0700 (PDT)

> section 6.2 paragraph 3).

Er, no. RFC 2047 makes no changes to how header fields are folded or
unfolded. Indeed, the word "fold" appears nowhere in RFC 2047.

What RFC 2047 does is state that white space between encoded words should
be ignored when displaying. That's a VERY different thing from changing
the unfolding rule to call for space to be removed entirely under some 
circumstances.

>    Subject: =?utf-8?q?hell?=

And

  Subject: =?utf-8?q?hell?=     =?utf-8?q?o?=

should be displayed in the same way; there is no interaction between the
unfolding operation and the decoding of encoded words.

>         and [MIME] part 3 6.2) prior to interpretation of the data.

IMO it would be better to say something along the lines of

    Header lines are unfolded as described in [IMAIL] section 2.2.3.
    Interpretation of header data should be done according to [MIME3]
    section 6.2.

This makes the required sequence of operations clear. Note that I also removed
the term "long" - in practice it isn't only long lines that are folded.

				Ned

---

From: Kjetil Torgrim Homme <[email-masked]>
Date: Tue, 05 Jul 2005 17:09:22 +0200

> the term "long" - in practice it isn't only long lines that are folded.

you are of course right, and in that light the reference to [MIME3] may
be redundant.  I think it is a useful reminder for implementors, and
think it is worthwhile to keep it, though.  (it would be strange to
argue otherwise since I messed it up :-)
-- 
Kjetil T.

---

From: "Mark E. Mallett" <[email-masked]>
Date: Tue, 5 Jul 2005 15:00:01 -0400

...

>    synactically invalid header names.

Would it be reasonable to add that says this refers specifically to
header tests (e.g. "in header tests, ...")?  Otherwise something like
the 'editheader' extension will conflict with this prohibition, as
editheader says (or, at least, implies, and probably should say) that
bad header names are to cause an error.

mm

---

From: "Mark E. Mallett" <[email-masked]>
Date: Tue, 5 Jul 2005 15:30:03 -0400

> Sometimes you get lucky, that's all.

I do case-insensitive comparisons, because in general I prefer to ignore
differences in case unless there's a reason not to; however, our script
generation tools always generate lowercase.  So changing to case-sensitive
wouldn't hurt here, and I wouldn't really object to it.

However, there has been other support for case-insensitivity in other
discussions; e.g.  for variable names and in the Sieve language
definition itself.  As I say, I don't particularly object to making
"require" case-sensitive, but consistency is always good (as is the "be
liberal in what you accept" tenet).

mm

---

From: Kjetil Torgrim Homme <[email-masked]>
Date: Wed, 06 Jul 2005 00:04:23 +0200

> liberal in what you accept" tenet).

I agree, case-insensitive matching is pervasive in IETF standards.  e.g.
all MIME types and parameters are case-insensitive, and everything else
in Sieve is case-insensitive.  the problem is that either way we will be
making some existing implementations non-conforming.  if the role of
3028bis is to document current practice, case-sensitive seems to be the
appropriate choice, even if it is a wart.
-- 
Kjetil T.

---

From: Philip Guenther <[email-masked]>
Date: Tue, 05 Jul 2005 23:54:40 -0700

>the term "long" - in practice it isn't only long lines that are folded.

I like it, though I think there should be a forward reference to 2.7.2
as that's where the precise requirements are stated.  So:

   Header lines are unfolded as described in [IMAIL] section 2.2.3.
   Interpretation of header data SHOULD be done according to [MIME3]
   section 6.2 (see 2.7.2 below for details).

Hmm, applying 2047 decoding requires knowing whether the header is
'structured': if a header is structured, then encoded words can occur
after an open-paren or before a close-paren without intervening
whitespace, something which isn't true of unstructured headers.  Do we
need to give guidence to implementors on this or just punt it as a
general 2047 issue not specific to sieve?

Philip Guenther

---

From: Alexey Melnikov <[email-masked]>
Date: Wed, 06 Jul 2005 13:37:20 +0100

>general 2047 issue not specific to sieve?
I say we should punt it, as this affects USEFOR (Usenet Format) WG as well.

---

From: Ned Freed <[email-masked]>
Date: Wed, 06 Jul 2005 12:23:40 -0700 (PDT)

> I say we should punt it, as this affects USEFOR (Usenet Format) WG as well.

Punting seems like the best choice.

				Ned

---

From: Ned Freed <[email-masked]>
Date: Thu, 07 Jul 2005 12:54:58 -0700 (PDT)

>    section 6.2 (see 2.7.2 below for details).

The added reference is a useful addition IMO.

				Ned

---

From: Philip Guenther <[email-masked]>
Date: Thu, 14 Jul 2005 18:51:24 -0700

>bad header names are to cause an error.

Good point.  I've added "in tests" to the end of that sentence.

Philip Guenther

---

From: Alexey Melnikov <[email-masked]>
Date: Sat, 16 Jul 2005 15:13:37 +0100

>this will make many other implementations non-conforming.
I suspect that Cyrus Sieve doesn't handle encoded NULs properly. So I 
would prefer B.