Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide more relevant feedback #56

Closed
matthieusieben opened this issue Sep 14, 2015 · 6 comments
Closed

Provide more relevant feedback #56

matthieusieben opened this issue Sep 14, 2015 · 6 comments
Assignees
Labels
Milestone

Comments

@matthieusieben
Copy link

@matthieusieben matthieusieben commented Sep 14, 2015

It would be nice if a message was provided to the Boom.forbidden. In big applications, where Boom is used everywhere, it may not be trivial to detect where the error was generated.

@stongo

This comment has been minimized.

Copy link
Contributor

@stongo stongo commented Sep 14, 2015

@matthieusieben this is a touchy issue for a security related module, as giving a detailed forbidden message essentially leaks data to potential attackers

that being said, I could investigate writing a server side log message to make debugging easier

@matthieusieben

This comment has been minimized.

Copy link
Author

@matthieusieben matthieusieben commented Sep 14, 2015

Some people argue that "Security through obscurity is not security". In the present case, it is obvious to an attacker that a csrf token has to be provided (crumb sets a cookie...) and as easy to detect that the cookie/header may be missing from the request. It would be also possible to determine that the request was rejected in an early stage of its processing by a statistical analysis of the response time...

Anyways, having a server side log message that displays exactly why the csrf did not pass would be good enough imo (and could also facilitate the detection of attacks). If you choose to do this, make sure that you print a different message when the cookie is missing and than when the one present does not match the token in the headers.

@stongo

This comment has been minimized.

Copy link
Contributor

@stongo stongo commented Sep 14, 2015

Ok adding the server side logging right now with a 'logUnauthorized' option to trigger it. Also good point on this being handy for monitoring

@steevhise

This comment has been minimized.

Copy link

@steevhise steevhise commented Sep 19, 2017

Did this "logUnauthorized" ever get committed and released? Doesn't look like it, it's not in the docs, and as I'm running Crumb 6.0.3 and get an error when I try including that option in the plugin registration.
It would be useful because I'm suddenly seeing crumb not working any more, possibly due to some issue with latest hapi version.

@spanditcaa

This comment has been minimized.

Copy link
Contributor

@spanditcaa spanditcaa commented Apr 5, 2018

@jonathansamines you'll find #112 and #113 which add the logUnauthorized capability in master for hapi 17, and I pushed a 6.x.x branch for those still on hapi 16.

geek added a commit that referenced this issue Apr 23, 2018
Logunauthorized to 6.x.x - addresses #56
geek added a commit that referenced this issue Apr 23, 2018
adds logUnauthorized option - addresses #56
@geek geek added this to the 7.1.0 milestone Apr 23, 2018
@geek geek removed the request label Apr 23, 2018
@geek geek self-assigned this Apr 23, 2018
@geek

This comment has been minimized.

Copy link
Member

@geek geek commented Apr 23, 2018

Fixed by #112

@geek geek closed this Apr 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.