Hi Alex, Alex Busenius wrote: > On 03/18/2010 05:57 AM, Caleb James DeLisle wrote: >> This message seems to have disappeared, resending... >> >> Alex Busenius wrote: >>> On 03/16/2010 10:23 AM, Caleb James DeLisle wrote: >>>> Marius Dumitru Florea wrote: >>>>> Hi Caleb, >>>>> >>>>> Caleb James DeLisle wrote: >>>>>> I don't want this proposal to die because of unnecessary noise which I >>>>>> introduced, >>>>>> I have thought about it and I am in agreement with the general idea of >>>>>> sending the user a hash which >>>>>> must be returned with the post in order for the data to be saved. >>>>>> >>>>>> I don't like adding code to xwiki-core so I suggest this be made into a >>>>>> component. >>>>>> We would need 2 functions: >>>>>> String getToken() >>>>>> boolean isTokenValid(String token) >>>>>> >>>>>> getToken uses org.xwiki.bridge.DocumentAccessBridge.getCurrentUser() to >>>>>> get the user who called it >>>>>> and the user name is stored in a HashMap of <String, String> with a >>>>>> random string of text. >>>>>> >>>>>> If getToken finds a token already in the map, it returns that token (so >>>>>> it may be called multiple times >>>>>> in the generation of a page) >>>>> So the token is the same for consecutive (GET) requests coming from the >>>>> same user? >>>>> >>>>>> isTokenValid checks the current user against the token then removes the >>>>>> entry from the HashMap so the token >>>>>> may not be reused. >>>>> What happens if the user opens in edit mode two different pages? Isn't >>>>> the second save invalidated by the first save? >>>> Good point I missed. I would have to have a script to disable such an >>>> onerous 'feature' :) >>>> I guess it will only work if a single number is valid basically forever. I >>>> wish components had access to the >>>> Request, Response and Session so it could expire on logout. >>> What if this map would store a mapping (user + url) -> token, >> URL sounds like a good idea, what are the chances that a user will be >> editing the same page in 2 windows? Even if >> they are, editing the same page will cause past changes to be lost. Perhaps >> if the token fails they should get a >> page with a warning message and an opportunity to save anyway. >
> Editing the same page in two windows is probably not common, but the > behavior would change. Currently, both saves work and _both_ are > recorded in the history in the order they were saved. With the tokens > the editor that was opened first would get an access denied on save, and > the one opened last would work. > I'm not sure how we can go back to editor and refresh only the token but > leave the modified text. And the natural reaction of a user would > probably be to hit the back button in browser, which would restore the > editor page with the invalid token. Maybe we can return the user to the edit page with a _new_ token and the submitted content when the received token is invalid, asking the user for a confirmation and explaining her/him why the confirmation is needed. This way the user can retry saving the content (this step can repeat over and over if "she/he" keeps opening other tabs and saving the page there instead) or continue editing. Does this still prevent the attack? Thanks, Marius > >> We can compare the URL to the referrer header which makes life easy (we >> would need access to the request but I >> think this is coming soon.) >> > True, once we have access to the request we could easily add referrer > header checking. > >>> or (user + >>> reason) -> token, where "reason" would be some description of the >>> intended change like "edit:Main.Test" or "rights:SomeUser+comment-edit"? >> This idea sounds like it would require a lot of extra infrastructure, maybe >> I am not seeing it right. > > No, the "reason" would contain only the important part of the things > that are currently usually in the URL, like document name and action. > From the viewpoint of token component the exact content doesn't matter, > it should just contain enough information to differentiate editing of > different things. > > The reason for this idea is that we would need to give the same URL to > both getToken and isTokenValid as an argument, but those methods are > usually called from totally different places. So I think it is easier to > give the (somewhat simplified) reason instead of trying to come up with > the same URL after several redirects. > > For example, URLS for editing and deleting a page clearly differ in the > action part (/delete/Main/WebHome vs. /save/Main/Webhome), but most > administrative tasks go to /admin/XWiki/XWikiPreferences, are redirected > to saverights.vm and only differ in the query part of the URL. Pages can > also be edited over /preview/... with correct parameters. Changing > access rights sometimes use /save/... as URL, etc. > > WDYT? > > > Alex > >>> getToken would return different tokens for different pages/"reasons" and >>> isTokenValid would need to provide the same url/"reason" for check to >>> succeed. >>> >>> >>>> Caleb >>>> >>>>> Thanks, >>>>> Marius >>>>> >>>>>> If a configuration parameter is specified to disable the component, >>>>>> getToken returns "" and isTokenValid >>>>>> returns true. >>>>>> >>>>>> WDYT? >>>>>> >>>>>> Caleb >>>>>> >>>>>> >>>>>> Caleb James DeLisle wrote: >>>>>>> Sergiu Dumitriu wrote: >>>>>>>> On 03/10/2010 07:44 PM, Caleb James DeLisle wrote: >>>>>>>>> Take a look at the "sammy is my hero" worm, myspace sent a hash to >>>>>>>>> the user like the one you propose, >>>>>>>>> the worm made the required get request to get the hash, then made the >>>>>>>>> post along with the hash. >>>>>>>>> What prevents javascript from opening the page with the hash in an >>>>>>>>> iframe and then reading in the iframe >>>>>>>>> to get the hash, then creating a form with the required data and >>>>>>>>> posting it to the save action? >>>>>>>>> >>>>>>>>> If we were to combine a requirement for post requests with checking >>>>>>>>> of the referrer header, then we >>>>>>>>> would block links, forms and javascript based attacks leaving only an >>>>>>>>> attack through older versions >>>>>>>>> of flash which support referrer forgery and at this point the >>>>>>>>> difficulty of the attack becomes such that >>>>>>>>> we need to consider a wider array of attack vectors. >>>>>>>> Sammy also required that XSS is enabled. >>>>>>>> >>>>>>>> Protecting from attacks originating in the wiki is kind of impossible >>>>>>>> at >>>>>>>> the moment, since JS can be inserted anywhere, and there's no (nice) >>>>>>>> way >>>>>>>> to prevent attacks from JS inside the application. As long as JS can >>>>>>>> be >>>>>>>> inserted, an attacker can do all the steps that the client would do to >>>>>>>> perform an action. >>>>>>>> >>>>>>>> The secret token works as a prevention mechanism when the attack comes >>>>>>>> from another site because the browser security model prevents the >>>>>>>> attack >>>>>>>> code to read the form. >>>>>>> I just did a few tests on iframes and I was wrong, they do have good >>>>>>> same origin policy. >>>>>>> >>>>>>> Caleb >>>>>>> >>>>>>>>> Caleb >>>>>>>>> >>>>>>>>> >>>>>>>>> Alex Busenius wrote: >>>>>>>>>> Unfortunately, using POST requests instead of GET requests is not >>>>>>>>>> enough. It will not prevent attacks that use forms and/or JavaScript >>>>>>>>>> to >>>>>>>>>> generate POST requests. >>>>>>>>>> >>>>>>>>>> Alex >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 03/09/2010 02:48 PM, Caleb James DeLisle wrote: >>>>>>>>>>> I had thought about proposing this myself but decided against it >>>>>>>>>>> because it seems >>>>>>>>>>> to me like a workaround for problems which can be solved in other >>>>>>>>>>> ways. >>>>>>>>>>> >>>>>>>>>>> Suppose we were to add a check to the actions which alter data >>>>>>>>>>> which made sure the request method >>>>>>>>>>> was 'post' and made it configurable in one of the configuration >>>>>>>>>>> files? We would have >>>>>>>>>>> to look over the default skins for incorrect links and leave the >>>>>>>>>>> configuration >>>>>>>>>>> option off by default for backward compatibility at least until the >>>>>>>>>>> next major version >>>>>>>>>>> but we could provide wiki operators the ability to prevent CSRF. >>>>>>>>>>> >>>>>>>>>>> Caleb >>>>>>>>>>> >>>>>>>>>> _______________________________________________ >>>>>>>>>> devs mailing list >>>>>>>>>> [email protected] >>>>>>>>>> http://lists.xwiki.org/mailman/listinfo/devs >>>>>>>>>> > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
