Subject: Notify options and URI encoding

From: Aaron Stone <[email-masked]>
Date: Tue, 21 Aug 2007 05:41:57 -0700

Hello notifiers,

I re-read notify-08, and it jumped out at me that :options takes a
stringlist argument! I had it in mind that it was a single string, so
both :options and the method itself were equally good places for encoded
parameters. I had a significant concern about the interpretation of
something like:

    notify "method:whatever&subject=foo ${evilvar}";

where ${evilvar} might come directly from the message and might contain
nefarious data. That concern has been reflected in the draft with the
new :encodeurl variables parameter. But it's sort of a hack because
we're looking at encoding in the first place. 

I think that encoded parameters on the method name should be prohibited
outright. All ad-hoc parameters should arrive via :options. That's what
it's there for, right?

    notify :options "subject=Hello this is the Subject"
        "mailto:foo@bar";

is much better than

    notify "mailto:foo@bar&subject=Hello%20this%20is%20the%20Subject";

Example 3 is particularly bad, starting with this variable:

    set "notif_method"
     "xmpp:[email-masked]?message;subject=SIEVE;body=You%20got%20mail";

I think this should be re-written to show that :options is a string list
and to reflect that encoding is done by the implementation:

    set "notif_method" "xmpp:[email-masked]?message"
    set "notif_subject" "SIEVE"
    set "notif_body" "You've got mail"

    ... tests in the example that override the defaults ...

    notify :options [ "subject=${notif_subject}", "body=${notif_body}" ]
        "${notif_method}";

Suggested new text in the description of :options, Section 3.5:

   Values may be URI-encoded and may be expanded from variables.
   Therefore, notification methods MUST NOT perform full URI-decoding
   of each option string, lest untrusted data contained in the variable
   become indistinguishable from user-specified data.

Security Considerations, this is from -08:

   Implementations that construct URIs internally from various notify
   parameters MUST make sure that all components of such URIs are
   properly percent-encoded (see [URI]).  In particular this applies to
   values of the :from and the :message tagged arguments and may apply
   to the :options values.

I think this is good, and we can drop Section 6, Modifier encodeurl,
too, as :encodeurl is not needed if the process for assembling a URI is
something like this (psuedo-perl):

    sub make_notify_uri( method, from, importance, options )
        return error unless sanity_check( method )
        uri = concat( method, "&from=", urlencode( from ) )
        uri = concat( uri, "&importance=", urlencode( importance ) )
        for each options
            next unless (key, value) =~ /([:alnum:])=(.*)/
            uri = concat( uri, "&", key, "=", urlencode( value ) )

The two important factors here are disallowing encoded methods and being
careful in how the option strings are split. I think that covers all of
the potential encoding holes for passing data through.

Aaron

---

From: Alexey Melnikov <[email-masked]>
Date: Sun, 26 Aug 2007 17:49:01 +0100

Hi Aaron,
Sorry for the slow response.

Aaron Stone wrote:

>outright.
I think the WG failed to reach consensus on this point. That is why the 
document says that it is up to notification methods to define if URI 
parameters are allowed in :method. So, I would like to see a clear WG 
consensus before doing this change.

>All ad-hoc parameters should arrive via :options. That's what
>it's there for, right?
>
One annoying side effect of this change would be the need to create a 
new IANA registry for options and populate it with various email headers 
(possibly specified in their own namespace, e.g. email.subject)

My reading of the WG consensus was that we don't need an IANA registry. 
Do you want to poll the WG regarding this issue?

>   become indistinguishable from user-specified data.
I am not sure I understand what is "full URI-decoding" and what is not.

>something like this (psuedo-perl):
I have to admit that I am still not sure how exactly Sieve notify is 
going to be used in real world. I would like to get some experience on that.

Considering that an implementation of Sieve Notify will have to have an 
URL-encoder, I don't think it would be much of a burden to expose it to 
Sieve script writers in the form of encodeurl modifier. So my personal 
preference is to keep it just in case.
Other opinions on this?