Hi Marius, On 03/18/2010 02:29 PM, Marius Dumitru Florea wrote: > 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? > Yes, as long as the modified content is just redirected back to the user and not saved until the token check succeeds.
Alex > 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 > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
