On 7 Mar 2010, at 17:40, Andrew Jaquith wrote: >> Can you detail exactly which security issue is raised by this? > > 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, > > 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. > >> 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. > > 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. > > 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. > > 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
