On Tue, 2014-07-08 at 06:23 +0200, xor wrote: > While porting Freetalk code to WOT, I was wondering about why page rendering > code which does "writes" checks whether the request type is "POST" > - by "writes" I mean stuff which changes anything about the Freetalk database > such as posting a message. > > The "blame" feature of Git shows that it came from this commit: > > https://github.com/freenet/plugin-Freetalk-staging/commit/ea251b3957cb217fc59284f5d9ab5500dd66f728 > > I suppose the goal of this commit was to ensure that the higher-level code > had > checked whether the request contained a valid formPassword: It only does the > password-check for POST, not for GET. See: > https://github.com/freenet/plugin-Freetalk-staging/blob/ea251b3957cb217fc59284f5d9ab5500dd66f728/src/plugins/Freetalk/ui/web/WebInterfaceToadlet.java > > This made me wonder about WHY the node has formPassword at all. To understand > that lets examine which ways can be used to restrict access to a node: > The access controls which the node offers are IP-based only. We have two > configuration options: > - Which IPs can access the node > - Which IPs are allowed "full access". Internally, this can be validated when > processing a request via ToadletContext.checkFullAccess(). > > Those two options seem to target the same goal as the formPassword mechanism: > Web interface code usually only allows the user to "modify" stuff if he has > full access. And the formPassword code also does that as we have seen in the > above Freetalk code. > > This made me wonder why we HAVE the formPassword if checkFullAccess can do > the > same thing. So I grepped the source code and it turns out that there is only > one write access to the NodeClientCore.formPassword variable: In the > constructor of NodeClientCore. > If I am correct with the assumption that NodeClientCore is only created once > at startup and continues to live during the whole run of the node, then > formPassword cannot do anything which checkFullAccess() cannot do because it > never changes. In fact it isn't any access control at all because if you > obtain formPassword ONCE at the beginning of the lifetime of the node, it > will > always be valid, even if the IP-address access options are changed to your > disadvantage. > > So the only conclusion is that formPassword is unfinished code. Is that right? > And code which does not validate it is NOT dangerous yet as long as it > validates checkFullAccess(). Right as well? > > I suppose it was meant to be used as a foundation for a true > Username/Password > login to the node, which was never implemented. Then it would be needed in > addition to IP-based checkFullAccess() because we would use the IPs to > restrict who can register a username and then do further restrictions based > on > the user's account. > Also it seems to be some sort of emulation of session cookies, and probably > was implemented this way because someone was paranoid that users would > disable > cookies in their browser. > Am I right with this interpretation of the purpose of formPassword? > > If you can clear me up on what formPassword aims to do, I then might be able > to: > - Improve its JavaDoc > - Investigate whether it can be replaced with the session cookie code which I > had implemented for Freetalk/WOT. This code was implemented *after* > formPassword was already there, so it sort of duplicates it. > - Maybe remove the ugly "only modify stuff if the request is POST" check in > Freeetalk/WOT because its very non-self-explanatory. However we probably > would > have to mark formPassword as deprecated to ensure that people don't suddenly > change fred to actually use it for access control - then the client > application code would be insecure if it doesn't check for POST. > > Thanks for your help :)
The name of the variable is badly chosen: formPassword is an anti-CSRF token (https://www.owasp.org/index.php/Cross-Site_Request_Forgery_% 28CSRF%29 ); do *NOT* touch it. As for when to use one, two rules: 1) if you're changing server side state, you need a POST request 2) all POST requests need an anti-CSRF token (the exception being a login page, where credentials -that are unpredictable to an attacker- are exchanged) NextGen$
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Devl mailing list [email protected] https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
