Re: OpenSocial 1.0 spec patch: Security policy and EL escaping (issue154162)

[llia...@google.com]

I'm +1 on this after the comments are addressed.

-Lane


http://codereview.appspot.com/154162/diff/1/2
File OpenSocial-Templating.xml (right):

http://codereview.appspot.com/154162/diff/1/2#newcode129
OpenSocial-Templating.xml:129: <Optional feature="SecurityProfile"
>
Can a gadget optionally depend on this? It seems like they need to
either expect output to be escaped or expect it to not be escaped.

http://codereview.appspot.com/154162/diff/1/2#newcode129
OpenSocial-Templating.xml:129: <Optional feature="SecurityProfile"
>
SecurityProfile --> security-policy

http://codereview.appspot.com/154162/diff/1/2#newcode130
OpenSocial-Templating.xml:130: <Param name="el_escaping"
>auto</Param>
auto --> html

http://codereview.appspot.com/154162/diff/1/2#newcode146
OpenSocial-Templating.xml:146: <t>html - Automatically HTML encode all
EL statements.</t>
Per Evan's comments:
MUST: HTML escape all elements and attributes
SHOULD: Prevent any EL statement from executing JavaScript code.

http://codereview.appspot.com/154162/diff/1/2#newcode146
OpenSocial-Templating.xml:146: <t>html - Automatically HTML encode all
EL statements.</t>
mention that html is the default.

http://codereview.appspot.com/154162/diff/1/2#newcode147
OpenSocial-Templating.xml:147: <t>safe - Allow a filtered subset of
markup within the EL statement. Strip out any markup deemed unsafe. It
is up to the container to define what subset of markup is safe.</t>
I think we can drop 'safe' mode for this iteration. All or nothing
seems like a reasonable place to start.

http://codereview.appspot.com/154162/diff/1/2#newcode152
OpenSocial-Templating.xml:152: Escaping rules specified with the
"el_escaping" parameter are applied to all views. An app may request a
different escaping policy for a specific view by specifying the view
name as a suffix with a preceeding colon to the el_escaping name. For
example, to specify auto escaping on the '''profile''' view and no
escaping for all other views, the following is used:
change triple quotes to single quotes

http://codereview.appspot.com/154162/diff/1/2#newcode156
OpenSocial-Templating.xml:156: &lt;Optional feature="SecurityProfile"
&gt;
SecurityProfile --> security-policy

http://codereview.appspot.com/154162/diff/1/2#newcode157
OpenSocial-Templating.xml:157: &lt;Param name="el_escaping:profile"
&gt;auto&lt;/Param&gt;
auto --> html

http://codereview.appspot.com/154162/diff/1/2#newcode165
OpenSocial-Templating.xml:165: Containers SHOULD apply HTML escaping by
default to all views that are not otherwise constrained or protected by
a security mechanism (ex: jail domained IFrame, trusted parter,
pre-filtered content).
parter --> partner

http://codereview.appspot.com/154162/diff/1/2#newcode274
OpenSocial-Templating.xml:274: <list style="symbols">
This will probably look better as a style="hanging" table:

<list style="hanging">
<t hangText="os:htmlEncode">Encode HTML markup entities. At minimum
escape greater-than and less-than symbols and quotation marks to their
encoded equivilents.</t>
...
</list>

http://codereview.appspot.com/154162/diff/1/2#newcode290
OpenSocial-Templating.xml:290: Escape string for inclusion within
javascript block inside quotes. There is an optional second parameter
to specify that the string should be escaped for inclusion within single
quotes when set to true.
javascript --> JavaScript

http://codereview.appspot.com/154162

[Evan Gilbert]

Agreed with Lane's comments.

+1 (with comments resolved / discussed).

--

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.

[Chris Cole]

I'm out of town right now, but will try to get an updated patch that addresses the comments.  We'll shoot for over the weekend.