Subject: Re: [sieve] Working group last call on draft-ietf-sieve-include

From: Stephan Bosch <[email-masked]>
Date: Fri, 23 Sep 2011 18:54:48 +0200

Hi Aaron,

Op 12-8-2011 15:54, Barry Leiba schreef:
>>> Aaron, please make changes based on the comments by Alexey, Stephan,
>>> and me, and post a new version of the draft.  When you do, I'll sent
>>> it on to the ADs for processing.
>> Excellent! Will do this week.
> Aaron, update forthcoming?

Any updates on this one?

Regards,

Stephan.

---

From: Aaron Stone <[email-masked]>
Date: Fri, 23 Sep 2011 11:58:21 -0700

Nits included in my -11. Question below about naming.

On Mon, Jul 18, 2011 at 6:21 AM, Alexey Melnikov
<[email-masked]> wrote:
> Barry Leiba wrote:
>
>> The sieve-include doc looks ready to go.  Because Aaron and Cyrus are
>> both editors of this document, I'm playing doc shepherd and handling
>> chair duties related to the document.  And so here we start working
>> group last call on draft-ietf-sieve-include-10, which you can find
>> here: http://tools.ietf.org/html/draft-ietf-sieve-include-10
>>
>> We've had good feedback on this, and I think it's solid.  Everyone,
>> please give it a last review and let us know whether you agree, or
>> whether you find anything that needs changing.  WGLC will end at the
>> end of July, just after the upcoming IETF meeting finishes.
>>
>> Posts confirming that you've reviewed this version and agree that it's
>> ready are welcome.
>>
> I am mostly happy with this version, but I spotted several nits or minor
> errors:
>
> 2.  Conventions Used in This Document
>
>  script execution
>     an instance of a Sieve interpreter invoked for a given message
>     delivery, starting with the user's active script and continuing
>     through any included scripts until the message is delivered.
>
> Pedantic comment: does "reject" or "discard" qualify as "message delivery"?
>
> 3.1.  General Considerations
>
>  Sieve implementations MUST generate an error at execution time if an
>  included script does not exist.
>
> This is no longer true due to the addition of the :optional tag.
>
>
> 3.2.  Control Structure include
>
>        Usage:   include *[PARAMETERS] <value: string>
>                 PARAMETERS = LOCATION / :once / :optional
>                 LOCATION = :personal / :global
>
>  The "include" command takes an optional "location" parameter, an
>  optional ":once" parameter, an optional ":optional" parameter, and a
>  single string argument representing the name of the script to include
>  for processing at that point.  It is RECOMMENDED that implementations
>  restrict script names according to MANAGESIEVE [RFC5804] Section 1.7.
>
> Did you mean Section 1.6 ("Script Names") instead of 1.7 ("Capabilities")?

Fixed.

> Also, I think this requirement makes [RFC5804] Normative.
>
> And finally, why is this not a MUST? Making a SHOULD level requirement seems
> a bit weak in this case.

Is it normative even without the MUST (i.e. if I leave it as RECOMMENDED)?

In the case of a MUST, it means that a valid includes implementation
imposes a script naming restriction. If a site isn't using
managesieve, would that site really need to accept the name
restrictions?

I'm comfortable either way.

> 4.  Security Considerations
>
>  Sieve implementations MUST ensure that script names are checked for
>  validity and proper permissions prior to inclusion, in order to
>  prevent a malicious user from gaining acess to files accessible to
>
> typo: access
>
>  the mail server software that should not be accessible to the user.
>
>
> 5.1.  "include" Extension Registration
>
>     Capability name: include
>     Description:     adds the "include" command to execute other Sieve
>                      scripts, and the "global" command and "global"
>                      variables namespace to access variables shared
>                      among included scripts.
>
> The "return" action is not mentioned here.
>
> _______________________________________________
> sieve mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sieve
>

---

From: Aaron Stone <[email-masked]>
Date: Fri, 23 Sep 2011 12:00:41 -0700

On Mon, Jul 18, 2011 at 6:35 AM, Barry Leiba <[email-masked]> wrote:
> Alexey said:
> Good points, all.  Some text suggestions for some:
>
>>  script execution
> NEW
>   through any included scripts until the final disposition of the
>   message.
>
> NEW
>   Sieve implementations MUST generate an error at execution
>   time if an included script does not exist and the ":optional"
>   parameter was not used.
>
> I agree.
>> And finally, why is this not a MUST? Making a SHOULD level requirement
>> seems a bit weak in this case.
>
> I'd be happy with MUST.

Ok, nevermind my questions from a moment ago, I'll make it MUST.

---

From: Barry Leiba <[email-masked]>
Date: Fri, 23 Sep 2011 15:06:16 -0400

> Ok, nevermind my questions from a moment ago, I'll make it MUST.

That's fine.  But, for the record, a SHOULD would also make the
reference normative.

Barry

---

From: Alexey Melnikov <[email-masked]>
Date: Sat, 24 Sep 2011 16:08:53 +0100

>Is it normative even without the MUST (i.e. if I leave it as RECOMMENDED)?
Yes.

>managesieve, would that site really need to accept the name
If you don't make it a MUST, then nobody can be relied upon the rule.

---

From: Robert Burrell Donkin <[email-masked]>
Date: Sat, 24 Sep 2011 19:19:20 +0100

> Aaron Stone wrote:

<snip>

> If you don't make it a MUST, then nobody can be relied upon the rule.

I'm a little unclear why allowing people to rely on this rule should
be seen as good thing...

1. Can anyone think of a use case that could be satisfied best by an
author intentionally including a restricted script name?

2. Allowing implementors to rely on this rule may create a false sense
of security, and so may encourage them to neglect proper checks on
names before accessing their backing store. What are the positive
benefits that outweigh this risk?

In any case, I think reminding people about potential attacks in "4.
Security Considerations" would be useful, so I would like to see
something like [1] included

Robert

[1] Sieve implementations MUST check that script names are safe for
use with their storage system. Any script including a name which could
be used as a vector to attack the system used to store scripts MUST be
rejected.

---

From: Alexey Melnikov <[email-masked]>
Date: Mon, 26 Sep 2011 22:06:25 +0100

Hi Robert,

>author intentionally including a restricted script name?
ManageSieve script names are quite permissive: basically various control 
characters are disallowed. IMHO, disallowing control characters in 
script names is a good thing.

>benefits that outweigh this risk?
Quite the opposite: if the rule is a MUST, then implementations can 
validate names and reject invalid ones.

>be used as a vector to attack the system used to store scripts MUST be
This is probably worth adding irrespectively of my comments above.

---

From: Aaron Stone <[email-masked]>
Date: Mon, 26 Sep 2011 15:35:34 -0700

On Mon, Sep 26, 2011 at 2:06 PM, Alexey Melnikov
<[email-masked]> wrote:
> characters are disallowed. IMHO, disallowing control characters in script
> names is a good thing.
>
> Quite the opposite: if the rule is a MUST, then implementations can validate
> names and reject invalid ones.
>

Agreed, I'll include this language. I also have Stephan's list of his
updates that I missed, so I'll integrate both and post an update.

Cheers,
Aaron

---

From: Robert Burrell Donkin <[email-masked]>
Date: Tue, 27 Sep 2011 19:55:40 +0100

> characters are disallowed. IMHO, disallowing control characters in script

(I'd like to avoid second guessing the design choices made in RFC5804)

I just wanted to understand what benefit (if any) changing SHOULD to
MUST has for script authors. I assume this silence means there is no
benefit.

> names and reject invalid ones.

Any reasonable, security-conscious implementation will reject
malicious names whether this action is allowed by the specification or
not. A well written, security-conscious specification would
acknowledge this behaviour and permit it.

A reasonable implementation supporting a related standard (whether
RFC5804 or some other future standard) will reject and allow script
names for reasons of compatibility  whether this action is allowed by
the specification or not.  A durable specification would acknowledge
this behaviour and permit it.

Therefore, I believe that authoring tools should strictly validate to
ensure wide portability but that servers should be allowed more
freedom for reasons of security and compatibility. So, I think that
perhaps something like [1] would be an improvement.

Robert

"Be conservative in what you do, be liberal in what you accept from
others." - Jon Postel

[1]
<original>
   Implementations MUST restrict script
   names according to MANAGESIEVE [RFC5804], Section 1.6.  The script
   name argument MUST be a constant string as defined in VARIABLES
   [RFC5229], Section 3;
</original>

<replacement>
Before resource resolution, implementations MUST validate script
names. Before attempting to access the resolved resource, if an
invalid name is detected implementations MUST terminate execution by
raising an error. A name is invalid when:

 * name resolution could compromise the security of the backing storage; or
 * the name is incompatible with one or more standard supported by the
implementation.

After resource resolution, implementations MUST verify the rights of
the script owner to access this resource. If access is not permitted,
implementations MUST terminate execution by raising a runtime-error.

Creators of script authoring tools SHOULD validate script names to
ensure portability and MUST restrict script names
  * to constant strings as defined in VARIABLES [RFC5229], Section 3;
  * to MANAGESIEVE [RFC5804], Section 1.6.
</replacement>

---

From: Alexey Melnikov <[email-masked]>
Date: Fri, 30 Sep 2011 20:40:56 +0100

Robert,

>>>author intentionally including a restricted script name?
This question looks entirely backwards to me.

>MUST has for script authors.
>
I don't see any point of having requirement on script authors (humans). 
However I believe knowing exact syntax and limitations has benefits for 
agents that generate and validate scripts.

>I assume this silence means there is no benefit.
>
SHOULD is pointless for syntactic restrictions, because no agent can 
rely that it will be complied with.

>this behaviour and permit it.
If you are arguing that these things are obvious and thus must not be 
document, then I strongly disagree.

>names.
This MUST is insufficient to have two interoperable implementations, 
because it doesn't suggest how this can be achieved. Unless the text 
below is really trying to explain how this can be done.

The rest of your proposed text seems Ok.

>Before attempting to access the resolved resource, if an