Hi Aaron,
Sorry for the slow response.
Aaron Stone wrote:
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.
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?
notify :options "subject=Hello this is the Subject"
"mailto:foo(_at_)bar";
is much better than
notify "mailto:foo(_at_)bar&subject=Hello%20this%20is%20the%20Subject";
Example 3 is particularly bad, starting with this variable:
set "notif_method"
"xmpp:tim(_at_)example(_dot_)com?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:
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?
set "notif_method" "xmpp:tim(_at_)example(_dot_)com?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.
I am not sure I understand what is "full URI-decoding" and what is not.
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):
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?
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.