Hi Chris, Thanks a lot for the hints! Here is my PR: https://github.com/apache/tomcat/pull/452
Best Regards, Polina On Sat, Aug 28, 2021 at 12:52 AM Christopher Schultz < ch...@christopherschultz.net> wrote: > 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 > >
