Mailing-List: contact jspwiki-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: jspwiki-dev@incubator.apache.org
Received-SPF: pass (nike.apache.org: domain of janne.jalkanen@ecyrd.com
 designates 193.64.5.122 as permitted sender)
Content-Type: text/plain; charset=us-ascii
Mime-Version: 1.0 (Apple Message framework v1077)
Subject: Re: svn commit: r917390 - in /incubator/jspwiki/trunk: ./
  src/java/org/apache/wiki/ src/java/org/apache/wiki/plugin/
  src/java/org/apache/wiki/preferences/ src/java/org/apache/wiki/rpc/json/
  src/java/org/apache/wiki/tags/ src/java/org/apache/wiki/ui/
From: Janne Jalkanen <janne.jalkanen@ecyrd.com>
In-Reply-To: <b90102161003070740y10727219n5a5fb7928f6af9f4@mail.gmail.com>
Date: Sun, 7 Mar 2010 19:27:21 +0200
Content-Transfer-Encoding: quoted-printable
Message-Id: <5A9FEA5B-4D4D-43CB-B77D-9F5E1D8AD1D4@ecyrd.com>
References: <15cc92001003021338x81afd82oaa5a5ccaae0b86e4@mail.gmail.com>
 <20100303162113.GA16369@ecyrd.com>
 <b90102161003031754v524be9f7tedb9a6fd958d98b@mail.gmail.com>
 <b90102161003040730y31c3e8e2nfbea401f83812c55@mail.gmail.com>
 <266B732F-3495-46B7-85A0-D96C2948A9C9@ecyrd.com>
 <4AB9BB4D-7246-4706-8131-B6FA6C4A652B@ecyrd.com>
 <b90102161003041347m1ceed4b7r5efdffe27e183a81@mail.gmail.com>
 <A5A372E9-4D0F-4410-8A11-44143AEB3DA3@ecyrd.com>
 <b90102161003060747h740cb596k16575288b23c8724@mail.gmail.com>
 <93C8DEA8-7C06-4985-91D7-48BFBC8CAF94@ecyrd.com>
 <b90102161003070740y10727219n5a5fb7928f6af9f4@mail.gmail.com>
To: jspwiki-dev@incubator.apache.org


On 7 Mar 2010, at 17:40, Andrew Jaquith wrote:

>> Can you detail exactly which security issue is raised by this?
>=20
> I'm worried about cross-site scripting. It should only be possible to
> source scripts that are stored on the server. That's why I want to
> check the requested resource against the ServletContext resource list.
> This check provides protection against plugins that are written
> maliciously, poorly or without security in mind. I don't think we can
> guarantee that every plugin will behave the way we expect -- even the
> ones that may someday be part of JSPWiki -- so this "origin check"
> provides a last line of defense.

...but you can include the script already *directly*, if you want to =
make the damage.

Also, the admin needs to make the call whether they want to install a =
plugin or not.  So it's not our problem.

>> After all, plugins are a) already running at the exact same rights as =
the rest of JSPWiki,
>=20
> That's true, but sooner or later we're going to get it to work in a
> security manager -- that's been a JIRA issue forever. And by "we" I
> mean "me", because nobody else is gonna do it. At some point, what
> JSPWiki (and third party JARs) can do will be more constrained when
> it's running in a security manager. But this issue doesn't relate to
> my real concern -- XSS.

But we don't have it now.  Besides, any plugin can write XSS code =
directly on the page.

>=20
>> I just simply cannot see that your method provides ANY added =
security, and it simply adds a TON of implementational complexity, pain =
to the developer, and still does not resolve the dynamic problem.
>=20
> Personal opinion: I don't see it as particularly complex. I'm also
> offering to do the work -- in case that wasn't clear -- and I work
> cheap. :)

But I do see that as a plugin writer, *I* need to essentially stick to =
2.8.  If adding dynamic code with plugins is not possible, I have no =
reason whatsoever to use 3.0 anywhere.

> On the security front, I hope you can understand that my concerns
> about XSS are well-founded. Here is a cheat sheet:
> http://ha.ckers.org/xss.html

I *do* know about XSS. It's my opinion that as long as a plugin can =
write <script> tags into HTML, any concerns about putting <script src> =
in the HEAD of the document are irrelevant.

>> This isn't really a theoretical question for future plugins, I've got =
a number of plugins that rely on this functionality.  We did, after all, =
agree to keep the API similar so that transferring from 2.x to 3.0 is =
not that complicated.
>=20
> I've offered you two opportunities already to name the plugins you
> have that require this capability, and you haven't named any. Not a
> single built-in JSPWiki plugin needs to add resources. But I take your
> word for it that this is something you (Janne) require even though
> JSPWiki itself does not.

*shrug* Not all of my plugins are available in an open source manner, =
and it should not matter - it is a removal of functionality and =
therefore breaks the API promise, and hence should be voted on.

>> Also, I don't think that page rendering code (incl. =
WikiPlugin.execute()) is guaranteed to execute before layout code.
>=20
> I've tested this. The layout code DOES render after the plugins.
> Assuming layout tags are used, it's guaranteed.

We don't guarantee it.  It may work that way right now, but we don't =
guarantee it.  Saying that we do would again create an API promise, and =
we should test for it and decide that that's how we're going to work in =
the future.

> - Add back the TemplateManager.addResourceRequest() method, as a
> deprecated method.

> This will delegate back to
> WikiContext.addResourceRequest(), which is a better place for this
> code.

Moving the API is fine, but this is exactly why I've called for the =
creation of an API package.

> - Add origin-validation logic when the resources are added to protect
> against XSS. The resources allowed would be limited just to those on
> the server. To keep it simple, we could just perform this check only,
> and not worry about checking against annotation/XML manifests until
> after Alpha.

I think this is much easier to do - just simply check whether the =
resource request contains "://".  This should be admin-configurable =
though; I don't use it personally but I do people who like to include =
Javascript files from CDN servers.  Like if you want to include YUI =
libs, you want to use Yahoo's CDN instead of hosting everything on your =
own.

> - Restore the InsertResources and ResourceRequest tags (retrofitted to
> use the new WikiContext method). The internal workings would be
> refactored a bit, but the results would be similar.

Since we're changing the JSP layout completely anyway, I don't think =
that those tags are necessarily needed, as long as the equivalent =
functionality is in there.

> - Rely on the assumption that because the layout tags are used, we do
> not need use markers or buffer the response. If we determine these
> things are needed, we can add them after the Alpha release.

I think this needs a set of tests.

> I'm sorry this has been so contentious. Really.

No worries. My only concern is that this discussion has been "after the =
fact" - this sort of discussion should be done on the JIRA or on the =
mailing list BEFORE the code modifications are committed to allow =
everyone to participate and weigh in - not just those of us who actually =
read commit logs.

/Janne