Modified Proposal for 1.0 EL - Clarify escaping behavior of EL

[goosemanjack]

This thread supersedes the original thread on EL behavioral
modification at

http://groups.google.com/group/opensocial-and-gadgets-spec/browse_thread/thread/d5ea1185e69f405e?hl=en

Proposal is here:

http://wiki.opensocial.org/index.php?title=Clarify_escaping_behavior_of_EL

It involves allowing developer to be more explicit about desired
security behavior and container to have greater control over how and
when to escape EL statements.

[Lane LiaBraaten]

If the feature is not supported, then is the default behavior still to escape strings in EL?  (I think it has to be fore backward compatibility)

In which case, this proposal is effectively:

"Containers MUST support escaping strings in EL.  Containers MAY provide a way for developers to turn off auto-escape of string content in EL"

Am I interpreting this correctly?

-Lane

--

You received this message because you are subscribed to the Google Groups "OpenSocial and Gadgets Specification Discussion" group.
To post to this group, send email to opensocial-an...@googlegroups.com.
To unsubscribe from this group, send email to opensocial-and-gadg...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/opensocial-and-gadgets-spec?hl=en.

[Jon]

I like the idea of the Param method, but should it really only be a
"hint"? If a gadget developer really depends upon the escaping
feature, the gadget would break if it went the other way. However
since we have a new feature, perhaps we could word it like: "If the
gadget developer Requires the SecurityProfile feature, and the server
cannot support the requested escaping, the server MUST emit a standard
error message indicating that it cannot satisfy the rendering
request."

The comment on view specific escaping, got me to thinking that we
should have a method to apply features on a view specific basis. Of
course that's not a 1.0 thing. If others think view specific features
are a good idea for the future, we should defer the view specific
escaping for that modification.

Standard functions are a good idea, especially when it is only a hint.
Do we also need os:urlDecode? os:noEscape - for when the user knows
the resultant value is properly escaped from the source?

[Lev Epshteyn]

I feel this proposal doesn't really touch on the meat of the stuff that needs clarifying when it comes to EL escaping, but rather provides a mechanism to turn the escaping on and off.

Chris, correct me if I am misstating your criticism of escaping, but it seems like the issue you have with it is that context-sensitive escaping is hinted at, but not well defined, and implementing it may be difficult.

That is, <span>${foo}</span> must be escaped differently than <a href="${foo}"> and probably <a onclick="alert(${foo})">. There may be other types of escaping you want as well (CSS comes to mind)

Doing the right thing, especially in a processor that operates on Strings, not DOM is not easy.

Do you think it would be helpful to clarify the specific contexts and escaping rules that will apply in each?

[goosemanjack]

This proposal is looking for a way out of the conundrum of Shindig
wanting auto-escaping codified, but us not feeling "contextual
escaping" is a practical solution. By providing a way for the
developer to declare their desired escaping behavior we potentially
solve the problem of "Executes one way in one container, but another
way in another container". They developer is granted the ability to
specify how the app expects to execute.

For the situation where a container doesn't support this feature, we
may make a recommendation, but would ultimately want to leave it to
the container to determine behavior. In our case we pre-validate all
gadgets. Any requesting a security profile that is not supported
could either be given a warning message and a degraded app experience,
or an error and a flat refusal to render.

The built-in functions mirror to some degree the internal security
filtering we do at MySpace. It provides explicit contextual security
filtering, rather than implicit contextual security filtering. It
also provides headroom for containers to experiment with alternative
security mechanisms without degrading the developer experience, and
allow developers who can live with the more restricted behavior to be
insured a stable rendering experience.
--
clc

[Lev Epshteyn]

Chris, please help me understand why you think contextual escaping is not a practical solution? Is the objection that it is difficult to implement, or that it is bad developer experience?

I know it feels like we're going around in circles, but I think if we knew the specific  objections you have to such escaping we could work out a meaningful compromise by addressing them.

[Evan Gilbert]

Chris, thanks for taking the time on this  proposal.

I agree that autoescaping correctly in all HTML attribute contexts is a difficult problem, and we don't have a standard unless this is specified somewhat clearly. For example, getting the @href attibute down is very difficult, and onXXX handlers are nearly impossible.

Escaping in HTML text nodes is easy (security experts - please correct me if this isn't the case). You mostly need to replace "<", ">", and "&" with the appropriate HTML equivalents. This is the biggest security hole, as most user-defined data is shown in text nodes, not HTML attributes, with a few exceptions.

Given these two assumptions, here's a possibly simpler option (with 2 variations):

1. Escape in HTML text nodes. Tell developers to use <os:Html> to get raw HTML.

2a. Tell developers not to use untrusted input in HTML attributes, OR
2b. Define a limited subset of characters for strings evaluated in HTML attributes - eliminate all characters that would be part of unintended code execution.

#1 gets us 95% of the security benefits. The goal of the variations in #2 is to find a good tradeoff between developer flexibility and security.

[goosemanjack]

Lane Said:

> In which case, this proposal is effectively:

> "Containers MUST support escaping strings in EL.
> Containers MAY provide a way for developers to
> turn off auto-escape of string content in EL"

> Am I interpreting this correctly?

Yes. In essence, that is this proposal with a degree of explicitness
in how the ability to turn off escaping will function.


To Evan's query: contextual escaping requires the EL language (and
processor) to no longer be independent of the hosting environment.
There are two main reasons we have a philosophical opposition to
contextual escaping:

1. Developers will get arbitrarily varying behavior from an identical
EL statement depending on where in the code it is located.

Ex: A variable "foo" contains text "a css @import url('style.css')
call pulls a file style"

The EL statement ${foo} would evaluate...

(in an HTML block)
a css &#64;import url('style.css') call pulls a file style

(in a CSS block)
a css import url style.css call pulls a file style

(in the href attribute of an a tag)
a css %40import%28%27foo%27%29 call pulls a file style

(in a javascript block)
a css @import url(\'style.css\') call pulls a file style

Problems start to arise when a developer does something like assign
the EL statement to a javascript variable, then set that value to the
href property of link element. The dev would need to write code to
undo the (incorrect for the situation) contextual escaping and redo it
to the proper URL encoding. The end result is the dev will be
fighting with the EL processor to do anything non-trivial or outside
very strict and limited use cases.


2. Contextual escaping requires the EL to have additional information
that exists outside the EL statement - namely the type of DOM node in
which the statement exists, and in some cases even the attribute type
(think @href vs. @class vs. @style vs. @onclick). This means the
script processor is no longer encapsulated in it's behavior or
portable to other situations. The only possible implementation for the
renderer becomes a DOM-node walker, which is inefficient under heavy
load. This forces a less efficient implementation than a raw stream
processor. It would be possible to buffer the stream, rewind at every
EL statement to determine context, then move forward again.


Add on to these two points all the shortcomings of contextual escaping
WRT security (some pointed out in the original proposal
http://wiki.opensocial.org/index.php?title=Auto-escaping_of_strings_in_EL_a_MAY#Reasons).

I am unaware of any other scripting language that exhibits the type of
polymorphic output behavior that the contextual escaping part of the
spec is attempting to require. Going down the list - PHP, VBScript,
Perl, Ruby - they all emit the same output for the same statement,
regardless of where the statement is located. They do provide
explicity escaping for common scenarios and do allow for function
extension, but none of them emit contextual polymorphic output.

Blanket HTML encoding in all situations (which seems to be the true
path Shindig has taken) instead of performing contextual escaping is
slightly better from an implementation standpoint as it doesn't have
the shortcomings of requiring a DOM node walker for everything. The
reality of this path is not so much that the EL escapes everything,
but that the hosting environment of the EL (the Template processor) is
applying escaping to the result of all EL statements. This solution
still suffers from the shortcomings identified in the latter part of
item 1. WRT behavior of EL statements in Javascript that are being
assigned to DOM nodes or to attributes of DOM nodes.

Expecting <os:Html> to be the only way to undo the escaping is not
practical because EL statements may exist within Javascript blocks.
Since we honor the spirit of CDATA directives by not parsing into
them, that tag won't even evaluate. Implementing escape/unescape as
functions (in addition to os:Html) exposes that control to app
developers in a way that doesn't rely on extraneous tags in the
markup.


The point of this proposal is to establish a syntax whereby app
developers can write gadgets and expect consistent behavior across
containers, while still allowing containers to define their own
escaping behavior. The net result if this proposal is accespted is
that:

1. Shindig does not have to make code changes - their current approach
of blanket HTML encoding all EL statements can be left in place.

2. Developers that want portability between MySpace and Shindig-based
containers have a syntax to specify that their app wants the blanket
HTML-encode behavior for cross-compatibility.

3. Shindig can apply the secruity encoding rules that they feel are
appropriate, and MySpace can work with security approaches we feel are
appropriate, without asking each other to compromise their security in
specific ways.

--
clc

> >> opensocial-and-gadg...@googlegroups.com<opensocial-and-gadgets-spec%2Bunsu...@googlegroups.com>
> >> .

> >> For more options, visit this group at
> >>http://groups.google.com/group/opensocial-and-gadgets-spec?hl=en.
>
> >  --
> > You received this message because you are subscribed to the Google Groups
> > "OpenSocial and Gadgets Specification Discussion" group.
> > To post to this group, send email to
> > opensocial-an...@googlegroups.com.
> > To unsubscribe from this group, send email to

> > opensocial-and-gadg...@googlegroups.com<opensocial-and-gadgets-spec%2Bunsu...@googlegroups.com>
> > .

[Lev Epshteyn]

Can you give an example of this problem? It seems like <button onclick="x = '${foo}'"> would get expanded to <button onclick="x = ' a css @import url(\'style.css\') call pulls a file style'"/>, so in this case the variable x actually receives the correct unescaped value that ${foo} originally had.

Perhaps I am incorrectly understanding the use case you mean?
 

2. Contextual escaping requires the EL to have additional information
that exists outside the EL statement - namely the type of DOM node in
which the statement exists, and in some cases even the attribute type
(think @href vs. @class vs. @style vs. @onclick).  This means the
script processor is no longer encapsulated in it's behavior or
portable to other situations. The only possible implementation for the
renderer becomes a DOM-node walker, which is inefficient under heavy
load.  This forces a less efficient implementation than a raw stream
processor.  It would be possible to buffer the stream, rewind at every
EL statement to determine context, then move forward again.


Add on to these two points all the shortcomings of contextual escaping
WRT security (some pointed out in the original proposal
http://wiki.opensocial.org/index.php?title=Auto-escaping_of_strings_in_EL_a_MAY#Reasons).

I am unaware of any other scripting language that exhibits the type of
polymorphic output behavior that the contextual escaping part of the
spec is attempting to require.  Going down the list - PHP, VBScript,
Perl, Ruby - they all emit the same output for the same statement,
regardless of where the statement is located.  They do provide
explicity escaping for common scenarios and do allow for function
extension, but none of them emit contextual polymorphic output.

Blanket HTML encoding in all situations (which seems to be the true
path Shindig has taken) instead of performing contextual escaping is

I believe today Shindig actually distinguishes between text nodes and attributes. This means that markup like <img title="${foo}"> will not be exploitable with ${foo} = 'hello" onmouseover="doBadStuff();' - that is, in this case an attribute value doesn't ever escape to create new attributes. But yes, Shindig today doesn't evaluate different attributes differently - and I think it may make sense for us to create rules on how to do that.

The benefit is that it significantly simplifies the developer model when it comes to handling user data.
 

slightly better from an implementation standpoint as it doesn't have
the shortcomings of requiring a DOM node walker for everything.  The
reality of this path is not so much that the EL escapes everything,
but that the hosting environment of the EL (the Template processor) is
applying escaping to the result of all EL statements.  This solution

I think this is how we should think of all escaping. The result of EL evaluation is an unescaped string. The result of inserting an unescaped string into a template is context-sensitive escaping of it. This allows us to separate the EL processor (which doesn't need context sensitivity) from the template processor (which does).

still suffers from the shortcomings identified in the latter part of
item 1. WRT behavior of EL statements in Javascript that are being
assigned to DOM nodes or to attributes of DOM nodes.

Expecting <os:Html> to be the only way to undo the escaping is not
practical because EL statements may exist within Javascript blocks.
Since we honor the spirit of CDATA directives by not parsing into
them, that tag won't even evaluate.  Implementing escape/unescape as
functions (in addition to os:Html) exposes that control to app
developers in a way that doesn't rely on extraneous tags in the
markup.


The point of this proposal is to establish a syntax whereby app
developers can write gadgets and expect consistent behavior across
containers, while still allowing containers to define their own
escaping behavior.  The net result if this proposal is accespted is
that:

1. Shindig does not have to make code changes - their current approach
of blanket HTML encoding all EL statements can be left in place.

2. Developers that want portability between MySpace and Shindig-based
containers have a syntax to specify that their app wants the blanket
HTML-encode behavior for cross-compatibility.

3. Shindig can apply the secruity encoding rules that they feel are
appropriate, and MySpace can work with security approaches we feel are
appropriate, without asking each other to compromise their security in
specific ways.

My fear is that this simply becomes a Shindig|Myspace toggle with each rendering mode incompatible with the other.

[goosemanjack]

Patch created:

http://codereview.appspot.com/154162

[Lev Epshteyn]

Chris, I still have some serious reservations about this approach - which I voiced in my message from Nov 3. Specifically, you stated that

Problems start to arise when a developer does something like assign
the EL statement to a javascript variable, then set that value to the
href property of link element.  The dev would need to write code to
undo the (incorrect for the situation) contextual escaping and redo it
to the proper URL encoding.  The end result is the dev will be
fighting with the EL processor to do anything non-trivial or outside
very strict and limited use cases.

But have not demonstrated how this is so.

As I've said before, I'm very much in favor of clarifying escaping rules, but this seems instead like a modal flag for choosing between two (three?) very different escaping behaviors.

The "safe" and "none" modes you are proposing do not seem like useful ones on the global application level.  That is, I'm much more likely to want safe HTML allowed in an individual field in a template than in the application as a whole. <os:Html> currently does exaclty that. I don't see why we should ever allow no escaping at all on user data.

Given a container that is able to support the auto-escaping of EL content, I don't see why any other mode would be useful globally. Thus, the only reason I see to lobby for this proposal is if a container doesn't wish to implement auto-escaping. If that is the case, then we are not winning anything by having a flag only some values of which will be supported by different containers - it will come back to different rendering behaviors on different platforms, which will make the entire templating feature significantly less useful.

Surely this is the reason that you, me and others on the list have been going back and forth so much on this issue. We are all trying  to preserve a safe and consistent set of rules for escaping in templates. I think we need to discuss this until we can all agree on something.

Until then, I'm -1 on this proposal as it stands.

On Mon, Nov 16, 2009 at 12:54 PM, goosemanjack <cc...@myspace.com> wrote:

Patch created:

http://codereview.appspot.com/154162

--

You received this message because you are subscribed to the Google Groups "OpenSocial and Gadgets Specification Discussion" group.
To post to this group, send email to opensocial-an...@googlegroups.com.

To unsubscribe from this group, send email to opensocial-and-gadg...@googlegroups.com.

For more options, visit this group at http://groups.google.com/group/opensocial-and-gadgets-spec?hl=.

[goosemanjack]

The point of this proposal is not to re-open the contextual escaping
debate. Everyone has stated their positions in the previous thread
and is unlikely to be swayed by the information that is available
today. For anyone curious, the previous thread may be found here:

http://groups.google.com/group/opensocial-and-gadgets-spec/browse_thread/thread/d5ea1185e69f405e?hl=en

As stated before, allowing the contextual escaping wording into the
0.9 spec was an oversight on our part. The edge case problems are
great enough that we have to consider this a spec error in its current
form and will not be supporting it. This discussion was closed out
for us in the previous thread.

The point of this proposal is to allow developers a mechanism to claim
their intention for behavior. If we are willing to include this
proposal, a developer building on iGoogle and trying to deploy to
MySpace could add the following directive to their source.

<Optional feature="SecurityProfile" >
<Param name="el_escaping" >html</Param>
</Optional >

With that directive in place, MySpace is willing to make a
modification to support blanket HTML encoding of EL statements in the
name of cross-compatibility. We're not willing to make this the
default behavior because of all the edge cases and encoding issues
previously identified, but someone wishing to author a cross-container
gadget would be able to declare their intent.

I would like to think everyone is working in good faith so we can find
the compromise here, because if we simply block and drop this proposal
developers will be the ultimate losers.
--
clc

> > opensocial-and-gadg...@googlegroups.com<opensocial-and-gadgets-spec%2Bunsu...@googlegroups.com>
> > .

[Evan Gilbert]

This proposal is a good middle ground - there are a couple of additional changes we'd like to see

Caja is working towards static template analysis that would go further than type="html" escaping, and we want to set a baseline of at least HTML escaping but allow for better security guarantees.

We'd like el_escaping="html" means that the container:

- MUST: HTML escape all elements and attributes

- SHOULD: Prevent any EL statement from executing JavaScript code.

It also wasn't stated explicitly that type="html" (and possibly type="none") should ensure proper nesting - EL in an attribute should only be able affect the value of that attribute. So <a title="${expr}"> shouldn't create a security hole when @title is: \" onclick="doEvil.

We should also have a default for el_escaping - otherwise developers won't know what to expect without this tag. Can we specify el_escaping="html" SHOULD be the default?

Evan

To unsubscribe from this group, send email to opensocial-and-gadg...@googlegroups.com.

[Lev Epshteyn]

If these clarifications are added to the "html" escaping mode, I would withdraw my -1.

I would also be in favor of dropping the "safe" mode, since it would by design be inconsistent between containers. If we want to keep it in, let's come up with what we mean by "safe" and spec it.

[goosemanjack]

Those clarifications seem reasonable. I'll look again at "safe" mode
to see if it can reasonably be folded into "html" or it's something we
want to keep around. If I don't get an updated patch out tomorrow, I
should be able to have something after the Thanksgiving holiday.
--
clc

> >>http://groups.google.com/group/opensocial-and-gadgets-spec/browse_thr...

> >> <opensocial-and-gadgets-spec%2Bunsu...@googlegroups.com<opensocial-and-gadgets-spec%252Buns...@googlegroups.com>