Polina,

On 8/26/21 10:48, Polina Georgieva wrote:
Currently the RestCsrfPreventionFilter is responding with 403 response when
the csrf token sent in the request is different from the one stored in the
session.

However except the 403 response code visible in the http access log file,
there’s no indication what happened and why is the error response.

So I think introducing some logs in this filter would be beneficial at
least from two points of view:

    1. Troubleshooting

It would be easier to troubleshoot problems with clients that did not
integrate with the csrf prevention mechanism properly or could give more
clues for other situations - for example cases of session invalidation
(done by other filter for example) before the request reaches the filter.
Currently such requests are also responded with 403 though the client seems
to have sent valid session cookie and  csrf token. That’s why I believe it
would be of great help to add log(s) stating:

    - if the requested session is found
    - if there’s token stored in it
    - if there’s token and session cookie sent in the request

without revealing their actual values or other security sensitive data.

And this information could be logged only in cases of 403 responses, i.e.
would appear only when needed.

    1. Improve identifying/tracking security related incidents

According to OWASP guidelines it’s recommended to have probable malicious
attacks indicated in the logs to better identify security incidents. For
more details please refer to [1].



If you agree with these ideas, I’ll be happy to propose a patch?

This sounds like a great idea. The RestCsrfPreventionListener and its superclass CsrfPreventionListenerBase both have access to a log object via the getLogger() method. It would be trivial to:

1. Add logging for whatever situation you'd like to log
2. Configure a logger to direct the output of the CSRF failures wherever you'd like

So I think you don't need to worry too much about the logging *mechanism* but instead simply add calls to the existing logger.

I was surprised to see *zero* logging in these classes, and adding such logging would certainly be a welcome improvement.

-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to