Bonesnap;11060421 wrote:I'll start with a simple usage example:
- Page loads and a token is generated using Token::generate().
- It's printed in a hidden input field (or something similar).
- An AJAX call is made. The value in the hidden input field is passed as part of the request.
- In the PHP file on the server-side, the token is verified using Token::verify($thePostedToken).
- If they don't match, throw an exception or error out or do whatever you want.
- If they do match, then proceed.
So the intent is to keep it constant - hence the use of the constant. The developer can define it anyway they want to.
Assuming TOKEN_SECRET is constant and your token class is defined as above, then doesn't that mean that any output of Token::generate() will never expire and any token generated is valid for any visitor? If so, this doesn't protect against anything at all because an attacker can simply visit your site, view the form, take some valid token from the hidden input field, and then concoct a malicious form hosted on any domain under the sun, and if they trick one of your users into submitting it, then your site will accept that token and the attack will succeed. In my understanding, the way you defend against CSRF is to construct tokens with a short lifespan that are valid only for one particular user. If any valid token is also valid for any visitor, then you are not protected at all against CSRF. You may want to review owasp CSRF prevention cheatsheet.
Bonesnap;11060421 wrote:Kerckhoff's Principle honestly doesn't make much sense to me. It sounds great but to me isn't grounded in reality for a very simple reason: if an attacker has access to my source code then it's game over.
KP is a principle to guide security review. It's complicated but basically boils down to suggestions like: good encryption systems are still good even if the entire mechanism of encryption is known. Keep your keys secure and ideally separate from your source (e.g., in an environment variable somewhere). In practice, PHP applications are totally compromised if the source code gets compromised, but alls is not lost if attackers only manage some SQL injection. Ideally an SQL security failure wouldn't compromise the security of your data. In applying it to your token-generating scheme, the lesson I take away is that your system generates ALL of its tokens using one single constant. I suggest you improve your system by having it generate tokens that also depend on a user-specific (or session-specific) value.
Bonesnap;11060421 wrote:There's no security in the world that can handle that. Furthermore if that were to ever happen, the last thing I care about is my token. I don't give a rodent's rear at that point
If your PHP source were compromised, but all your sensitive credentials injected as envrionment variables (e.g., via an apache conf file) then your system would not necessarily be compromised.
Bonesnap;11060421 wrote:Database credentials, PHPMailer credentials, etc. I have bigger problems than a token.
Yes this is how most PHP apps work. I'm not trying to be pedantic. I do mean to suggest that I think your CSRF protection is perhaps not providing the protection you think it is.
Bonesnap;11060421 wrote:The token secret has nothing to do with the user's ID.
I think the token should have something that makes it specific to each visitor. If the visitor is not yet authenticated, you can still maintain a session using a session ID and store something unique for each user so that every user who visits will receive tokens that are only valid for them and no one else. This could be a randomly generated session ID stored in a cookie, for instance.
Bonesnap;11060421 wrote:This has nothing to do with passwords so I am not sure why it was mentioned. Also, each hash will be different anyway since the token will be different.
The reason I mention passwords is because MD5 hashing and SHA1 hashing are being deprecated, with more likely to follow -- there's a cautionary tale here. The best-practice of using a salt when hashing passwords and the fairly recent introduction of [man]password_hash[/man] is a to improve security. Not only is each password different, but we generate a random salt to hash that password to further insulate our hashed values against attacks. Not only does each password hash to a different value, but the same password, used twice, will never have the same hash thanks to the salt.
Bonesnap;11060421 wrote:However the developer would like. For me, I used KeePass and generated a random 100 character string.
Depending on the charset used, that sounds like 800 bits or so, which is pretty hefty. The risk, as I've pointed out lies primarily in the fact that a token that works for me also works for you. If I'm an attacker, I view your form, I extract a valid token, and I construct a malicious form you using the token and the token will still be accepted. I just have to trick you into clicking on my form. This is the essence of a CSRF attack. CSRF prevention hinges upon insuring that tokens valid for me are NOT valid for anyone else.
Bonesnap;11060421 wrote:In my specific example, yes, but as I mentioned in my original post you could harden it by including an expiration during hashing or a user's ID, etc. That is left up to the developer.
It may be impactical for any given project to invest in CSRF protection if the risk of attack is low, but you only have to do it right once. It is the expiration of a token that introduces the trouble when you have a lot of AJAX, a SPA, or multiple windows open. You can probably relax your expiration times and let your tokens last for hours without much risk. However, I think it's critically important that a token displayed in hidden input in User A's browser should not work at all for User B. You should store a secret random token in session or in a cookie -- a different one generated for each visitor -- so that User A cannot attack User B.