Support for additional factors in token

The CSRF token generated is only dependent on time by default, i.e., without additional work, generated tokens are valid for all users in multi user sites.

However, tokens should be unique per user session as stated in https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#synchronizer-token-pattern

A workaround would be to use different secret_keys per user (e.g., append the user id to the site wide secret), but this seems clumsy (and should probably get documented). @aekasitt

fastapi-csrf-protect/fastapi_csrf_protect/core.py

Lines 39 to 41 in 81bbece

    
           serializer = URLSafeTimedSerializer(secret_key, salt='fastapi-csrf-token') 
        
           token = serializer.dumps(sha1(urandom(64)).hexdigest()) 
        
           return token

I'll look into this.

The assertion is that the CSRF Token generated is dependent on time by default; But I'm not sure that is observed by the code snippet given above. The serializer (itsdangerous's URLSafeTimedSerializer) is only dependent on time by default, but then the token is generated by a secondary process of hashing sha1(urandom(64)).hexdigest() which I think qualifies as session independent unless it can be credibly observed as timestamp dependent where two sessions generated at the exact same timestamp (a rarity but not an impossibility) can cause a collision with the same CSRF Token

The following attack is trivial since the CSRF token isn't really checked against anything (but time validity):

(malicious) user A gets a (valid) CSRF token from our site
user A makes user B send a POST using user A's CSRF token

profit

fastapi-csrf-protect/fastapi_csrf_protect/core.py

Lines 112 to 118 in 3682f9d

    
           serializer = URLSafeTimedSerializer(secret_key, salt='fastapi-csrf-token') 
        
           try: 
        
             token = serializer.loads(data, max_age=time_limit) 
        
           except SignatureExpired: 
        
             raise TokenValidationError('The CSRF token has expired.') 
        
           except BadData: 
        
             raise TokenValidationError('The CSRF token is invalid.')

The lib should either send a random value (no signing necessary) and actually store this in a session, and check the submit data/header against that value (in OWASP this is the "Synchronizer Token Pattern"), or it should set a random value in a cookie, and check whether the cookie value and the submit data/header info matches (the OWASP "Double Submit Cookie" approach).

fastapi-jwt-auth uses the Double Submit Cookie approach - if fastapi-csrf-protect wants to do the same, it's missing the actual value check. In fastapi-jwt-auth, that's the following code.
https://github.com/IndominusByte/fastapi-jwt-auth/blob/a6c06193319da0e4976c7472966f3a2891e0d50c/fastapi_jwt_auth/auth_jwt.py#L516-L521

Note all the JWT/HMAC/encrypting things are not strictly necessary for CSRF. The Double Submit Cookie approach doesn't rely on encryption/HMAC but on the fact that an attacker cannot control or know the CSRF token value stored in the cookie but has to supply that value in headers/form data.

This would defeat the purpose of the library. You're looking for a Session solution. (big S)
The scope of Cross-Site Reference Protection has been met with current implementation.

Unique per user session. (small s, randomly generated per page)
Secret (Can be stored in Cookies for enhanced Secrecy)
Unpredictable (large random value generated by a secure method). (satisfied)

The library doesn't have any concept of a user session; it doesn't actually store the token in a (server-side) session but only in a cookie; and the CSRF token (value) stored in that cookie is never checked against the token (value) that gets submitted via headers or form data. The only thing that's checked is basically well-formedness of the token (HMAC, max_age) which is not enough.

@aekasitt Did you read https://github.com/IndominusByte/fastapi-jwt-auth/blob/a6c06193319da0e4976c7472966f3a2891e0d50c/fastapi_jwt_auth/auth_jwt.py#L516-L521 ?

if not hmac.compare_digest(csrf_token,decoded_token['csrf']):
                    raise CSRFError(status_code=401,message="CSRF double submit tokens do not match")

The key point there is that csrf_token comes from request headers/form data, and decoded_token is obtained from the CSRF cookie.

This check is missing completely in fastapi-csrf-protect.

I agree with you that fastapi-jwt-auth's csrf token is more sophisticated since it depends on a session id (client-side and stored in cookies, but session id nonetheless)

I am telling you that it goes beyond the scope of this simple library.

It is in fact preferable if session integrity is coveted to use alternative library such as the one you mentioned.

To prevent the attack you mentioned, with Scammer A building a real-time cloned page with stolen CSRF token for User B;
What is being attacked there is session integrity, which this library never claimed to protect against.

Such attack can be prevented using SameSite cookie, shortening Max-Age and storing CSRF Token in Cookies for enhanced secrecy.

The attacker doesn't need to "steal" any CSRF token. He can just use a token that our site gave to him, and use that token in a POST request he makes user B submit.

That is no session integrity attack, there's no session involved.

Perhaps there's a misunderstanding that a "session" is?

E.g., I was wondering in this code snippet

fastapi-csrf-protect/fastapi_csrf_protect/core.py

Lines 32 to 41 in 3682f9d

    
             def generate_csrf(self, secret_key:Optional[str]=None): 
        
               ''' 
        
               Generate a CSRF token. The token is cached for a request, so multiple 
        
               calls to this function will generate the same token. 
        
               ''' 
        
               secret_key = secret_key or self._secret_key 
        
               if secret_key is None: raise RuntimeError('A secret key is required to use CSRF.') 
        
               serializer = URLSafeTimedSerializer(secret_key, salt='fastapi-csrf-token') 
        
               token = serializer.dumps(sha1(urandom(64)).hexdigest()) 
        
               return token

it says "multiple calls to this function will generate the same token" which clearly is not the case.

In any case, as it is, this library doesn't provide protection against CSRF, at least not in the sense OWASP etc. use those terms.

In Cross Site Request Forgery protection, the point is checking form data/header info against a CSRF (server side) session token or CSRF cookie value, which this library doesn't do.

You have a misunderstanding of the OWASP requirements, and I used the word "session" because of it was used in the document.

It satisfies the OWASP requirements as I previously explained.

Unique per user session. (randomly generated using urandom(64))
Secret (Can be stored in Cookies for enhanced Secrecy)
Unpredictable (large random value generated by a secure method). (already satisfied)

The cached comment I can remove. Not sure how that got there. Thanks!

Please refer to the Defense in Depth Techniques section of the attached OWASP document
Does your attack vector make sense with SameSite Cookie Attribute properly configured?

Of course the token has to be unique per user session and it has to be validated against the user session (on POST). Every other CSRF library does this. Look at the code I mentioned a few postings above, and just for fun, https://github.com/wtforms/flask-wtf/blob/72419060c0ce71c002098e304121f7b61c7ec985/src/flask_wtf/csrf.py#L118-L119 is the corresponding code in flask-wtf.

Sorry, but as it is, fastapi-csrf-protect is completely broken.

Yeah please help with SameSite :) I've been confused as to why you raised this issue and not that.

Token generation satisfies OWASP requirements.
Session integrity preservation is outside of Cross-Site Reference Forgery Protection.
Code breaking side-problem found, SameSite not yet implemented in version 0.2.0.
Closed.

validate_csrf_in_cookies() only looks at the cookie. It doesn't check against form data/headers. Any attacker doesn't need to guess any token at all and can just make the victim POST any form data, it'll check out.

fastapi-csrf-protect/fastapi_csrf_protect/core.py

Lines 120 to 126 in 3682f9d

    
           def validate_csrf_in_cookies(self, request: Request, secret_key:Optional[str]=None, field_name:Optional[str]=None): 
        
             secret_key = secret_key or self._secret_key 
        
             if secret_key is None: raise RuntimeError('A secret key is required to use CSRF.') 
        
             field_name = field_name or self._cookie_key 
        
             cookie = request.cookies.get(field_name) 
        
             if cookie is None: raise MissingTokenError(f'Missing Cookie {field_name}') 
        
             self.validate_csrf(cookie)

Token generation satisfies OWASP requirements.

"Generation" yes, but not validation.

Can you please open another issue for this? Hard to track.

	serializer = URLSafeTimedSerializer(secret_key, salt='fastapi-csrf-token')
	token = serializer.dumps(sha1(urandom(64)).hexdigest())
	return token

	serializer = URLSafeTimedSerializer(secret_key, salt='fastapi-csrf-token')
	try:
	token = serializer.loads(data, max_age=time_limit)
	except SignatureExpired:
	raise TokenValidationError('The CSRF token has expired.')
	except BadData:
	raise TokenValidationError('The CSRF token is invalid.')

	def generate_csrf(self, secret_key:Optional[str]=None):
	'''
	Generate a CSRF token. The token is cached for a request, so multiple
	calls to this function will generate the same token.
	'''
	secret_key = secret_key or self._secret_key
	if secret_key is None: raise RuntimeError('A secret key is required to use CSRF.')
	serializer = URLSafeTimedSerializer(secret_key, salt='fastapi-csrf-token')
	token = serializer.dumps(sha1(urandom(64)).hexdigest())
	return token

	def validate_csrf_in_cookies(self, request: Request, secret_key:Optional[str]=None, field_name:Optional[str]=None):
	secret_key = secret_key or self._secret_key
	if secret_key is None: raise RuntimeError('A secret key is required to use CSRF.')
	field_name = field_name or self._cookie_key
	cookie = request.cookies.get(field_name)
	if cookie is None: raise MissingTokenError(f'Missing Cookie {field_name}')
	self.validate_csrf(cookie)