Preventing the leaking of sensitive information in error logs
Julien Phalip
Recently I've been a bit embarrassed to receive a 500-error email
report containing a client of mine's password displayed in clear
because the admin login view had encountered an unhandled exception.
This is probably OK in a debug environment but in production this can
potentially have damaging consequences when handling passwords, credit
card numbers, etc. It may also go against certain policies and
standards such as the PCI-DSS (http://en.wikipedia.org/wiki/
Payment_Card_Industry_Data_Security_Standard).
There is already an open ticket to address this issue:
https://code.djangoproject.com/ticket/14614
I first wrote a patch allowing a lot of granularity to control which
POST/GET parameters should get obfuscated when producing the error
logs: https://code.djangoproject.com/attachment/ticket/14614/14614.obfuscate-request-parameters.diff
Russell pointed out that this implementation approach was overly
complicated. It also doesn't address the stack frame variables being
logged in some situations. So I've posted another patch with a more
radical approach, where all stack frame variables and the request's
information are systematically and entirely omitted from the logs for
exceptions occurring in views marked with the @sensitive decorator:
https://code.djangoproject.com/attachment/ticket/14614/14614.sensitive-request.diff
Perhaps the ideal solution lies somewhere between the two approaches.
The default loggers should also probably provide this protection by
default but still allow easy customisation/overriding of this
behaviour, for example for debugging purposes in safe environments.
I'm bringing this up to the dev-list as I'm keen to hear if someone is
interested in this problem and has suggestions towards a robust
solution. It is a pretty serious issue that I hope can be resolved by
the 1.4 release.
Many thanks,
Julien
Fraser Nevett
> I'm bringing this up to the dev-list as I'm keen to hear if someone is
> interested in this problem and has suggestions towards a robust
> solution. It is a pretty serious issue that I hope can be resolved by
> the 1.4 release.
https://code.djangoproject.com/ticket/7472, which was closed as
wontfix.
I'm glad this is now being looked at again and I like the idea of
being able to mark individual views/requests as handling sensitive
information. This means one could write a simple middleware that sets
the is_sensitive flag if the request was made over HTTPS to achieve
what I was wanting to do originally.
In terms of the latest patch:
1. I think the "sensitive" decorator should perhaps not be defined
within django.views.debug, but somewhere within
django.views.decorators to be more consistent with the other built-in
decorators.
2. I have a minor concern that having the "sensitive" decorator
being included in a module called "debug" may lead to developers
mistakenly thinking that it is relevant for development debugging
rather for protecting production environments.
3. Someone may wish to write their own AdminEmailHandler that
produces an encrypted email containing all the sensitive details. At
present, it looks like the patch won't actually allow you to get a non-
redacted version of the traceback if you really wanted to.
As a side thought, if this functionality does get implemented, would
it be feasible/desirable to have Django emit a warning to encourage
the developer to mark a view as sensitive if it detected a
forms.PasswordField (or any other FormField known to handle sensitive
data) being used by it? I'm not actually sure it's possible as a Form/
FormField doesn't directly know about the request, but I thought I'd
throw the idea out there anyway.
Cheers,
Fraser
Yishai Beeri
wrote:
Perhaps something along the lines of how Django marks strings as safe /
need escaping for the template engine can be applied here. A
forms.PasswordField might mark its data as "sensitive", and that info
would flow up until the point it is being rendered by the debug templates,
printed to a log file, etc.
Yishai
Luke Plant
> Recently I've been a bit embarrassed to receive a 500-error email
> report containing a client of mine's password displayed in clear
> because the admin login view had encountered an unhandled exception.
> This is probably OK in a debug environment but in production this can
> potentially have damaging consequences when handling passwords, credit
> card numbers, etc. It may also go against certain policies and
> standards such as the PCI-DSS (http://en.wikipedia.org/wiki/
> Payment_Card_Industry_Data_Security_Standard).
>
> There is already an open ticket to address this issue:
> https://code.djangoproject.com/ticket/14614
>
> I first wrote a patch allowing a lot of granularity to control which
> POST/GET parameters should get obfuscated when producing the error
> logs: https://code.djangoproject.com/attachment/ticket/14614/14614.obfuscate-request-parameters.diff
>
> Russell pointed out that this implementation approach was overly
> complicated. It also doesn't address the stack frame variables being
> logged in some situations. So I've posted another patch with a more
> radical approach, where all stack frame variables and the request's
> information are systematically and entirely omitted from the logs for
> exceptions occurring in views marked with the @sensitive decorator:
> https://code.djangoproject.com/attachment/ticket/14614/14614.sensitive-request.diff
I think we probably do need to provide something here. I'm concerned
that a 'sensitive' decorator puts the decision in the hands of the app
writer, which may well not be appropriate. For example, in one case I'm
involved in, half of the data entered in the admin in sensitive to some
degree, but my ability to debug errors could be seriously hindered if
the whole thing had 'sensitive' slapped on it. (Thankfully, the data
isn't too critical, and it is enough that I just delete the error
e-mails once I've dealt with them).
So, the 'sensitive' decorator might be appropriate for login, and
possible some other views, but I'm concerned it is still going to leave
other important situations with no practical solution.
Would it possible to make the sensitive decorator add some kind of
strategy object to the request, which itself is responsible for the
filtering, rather than a simple boolean flag? The strategy object
interface might be:
class ExceptionReporterFilter(object):
def show_request(self, request):
# return True or False
def filter_request_POST(self, request, post_dict):
# if show_request is True, this is passed request.POST
# and returns a sanitised version
def show_traceback(self, request):
# True or false
def show_traceback_vars(self, request):
# called only if show_traceback() returns True
def filter_traceback_vars(self, request, tb_frame, vars):
# filters vars to be shown at each level.
OK, could get carried away there - maybe we should start simple, e.g.
just 'show_request' and 'show_traceback_vars'. But something like that
would allow us to provide a working 'sensitive' decorator, but with a
mechanism that allows for something more fine-grained, and allows us to
add more features to it easily in the future. For the admin and CBVs it
would work as well, since there are always places you can override a
method and attach something to the request object.
If I'm thinking correctly, with an additional method like
'filter_settings', it might be possible to have a default
ExceptionReporterFilter that does the filtering of settings that is
currently hardcoded into django/views/debug.py.
Regards,
Luke
--
"The truth will set your teeth free." (Calvin and Hobbes)
Luke Plant || http://lukeplant.me.uk/
Michael Manfre
On Jun 1, 5:53 am, Luke Plant <L.Plant...@cantab.net> wrote:
> Would it possible to make the sensitive decorator add some kind of
> strategy object to the request, which itself is responsible for the
> filtering, rather than a simple boolean flag? The strategy object
> interface might be:
>
> class ExceptionReporterFilter(object):
> def show_request(self, request):
> # return True or False
>
> def filter_request_POST(self, request, post_dict):
> # if show_request is True, this is passed request.POST
> # and returns a sanitised version
>
> def show_traceback(self, request):
> # True or false
>
> def show_traceback_vars(self, request):
> # called only if show_traceback() returns True
>
> def filter_traceback_vars(self, request, tb_frame, vars):
> # filters vars to be shown at each level.
>
> OK, could get carried away there - maybe we should start simple, e.g.
> just 'show_request' and 'show_traceback_vars'. But something like that
> would allow us to provide a working 'sensitive' decorator, but with a
> mechanism that allows for something more fine-grained, and allows us to
> add more features to it easily in the future. For the admin and CBVs it
> would work as well, since there are always places you can override a
> method and attach something to the request object.
approach, but I'm not sure that I like having to control filtering of
the entire traceback at the scope of a view. That could lead to
needing to repeat parts of filtering on lots of views, if code is
reused often.
I wonder if it would make sense to handle the traceback and vars
filtering with a mechanism similar to __traceback_hide__. This would
allow controlling leakage per frame and make it less likely to be
missed on view.
Example:
def do_stuff():
...
do_stuff.__traceback_filter_vars__ = ['credit_card', 'password',
'launch_codes']
> If I'm thinking correctly, with an additional method like
> 'filter_settings', it might be possible to have a default
> ExceptionReporterFilter that does the filtering of settings that is
> currently hardcoded into django/views/debug.py.
re.
Regards.
Michael Manfre
Luke Plant
I'm not quite sure what you're saying. In my proposal above, the
'filter_traceback_vars' would only be used if 'show_traceback_vars'
returned True, and by default it would return all vars. So you wouldn't
be forced to do any filtering on that level.
Since you could re-use your filtering class for multiple views, and you
can implement it how you want, there would be no need to repeat the
definition of the filtering.
> I wonder if it would make sense to handle the traceback and vars
> filtering with a mechanism similar to __traceback_hide__. This would
> allow controlling leakage per frame and make it less likely to be
> missed on view.
>
> Example:
> def do_stuff():
> ...
> do_stuff.__traceback_filter_vars__ = ['credit_card', 'password',
> 'launch_codes']
I'm not sure this even possible - can you go from a traceback frame to
the *function* object? __traceback_hide__ is a local attribute, not a
function attribute.
We could do the same with __traceback_filter_vars__ and make a local
variable. But that could be an implementation detail of the
ExceptionReporterFilter (just as respecting __traceback_hide__ would be).
Luke
--
"Trouble: Luck can't last a lifetime, unless you die young."
(despair.com)
Luke Plant || http://lukeplant.me.uk/
Julien Phalip
> Would it possible to make the sensitive decorator add some kind of
> strategy object to the request, which itself is responsible for the
> filtering, rather than a simple boolean flag?
HttpRequest's __repr__() method. So in fact my initial patch did let
the HttpRequest itself take care of the obfuscation of sensitive GET/
POST parameters, in a similar way as with HIDDEN_SETTINGS and
get_safe_settings in the django.views.debug module:
https://code.djangoproject.com/attachment/ticket/14614/14614.obfuscate-request-parameters.diff
https://code.djangoproject.com/ticket/14614#comment:6
However, Russell comment was that the issue here is purely related to
debugging or logging business, and therefore the HttpRequest shouldn't
have to be involved directly. This makes it difficult to have any fine-
grained control over request parameters though.
> The strategy object interface might be:
>
> class ExceptionReporterFilter(object):
> def show_request(self, request):
> # return True or False
>
> def filter_request_POST(self, request, post_dict):
> # if show_request is True, this is passed request.POST
> # and returns a sanitised version
>
> def show_traceback(self, request):
> # True or false
>
> def show_traceback_vars(self, request):
> # called only if show_traceback() returns True
>
> def filter_traceback_vars(self, request, tb_frame, vars):
> # filters vars to be shown at each level.
>
> OK, could get carried away there - maybe we should start simple, e.g.
> just 'show_request' and 'show_traceback_vars'. But something like that
> would allow us to provide a working 'sensitive' decorator, but with a
> mechanism that allows for something more fine-grained, and allows us to
> add more features to it easily in the future. For the admin and CBVs it
> would work as well, since there are always places you can override a
> method and attach something to the request object.
Thanks for the feedback!
Julien
Michael Manfre
the traceback. I'd prefer to have the option to filter out only the
offending variables in an easy to maintain way.
Lets assume that there are three functions with some vars that need to
be filtered out (func_a, func_b, func_c). These functions are used by
a dozen views across various apps in your project; some use all, most
use one or many of these functions. I'm looking to avoid needing to
maintain a hierarchy of filters (or a single filter that knows about
all sensitive functions) to handle all of the potential sensitive
frames that might be called by a given view. I also don't want to add
a generic filter to every view in the project to prevent the situation
where a new view uses a function that a few frames down uses one of
the sensitive functions without the author realizing it.
I guess what I proposed would be best applied to
django.views.debug.ExceptionReporter, so it is not necessary to apply
a view filter if you only need to filter out variables in various
frames.
On Jun 2, 3:33 pm, Luke Plant <L.Plant...@cantab.net> wrote:
> > I wonder if it would make sense to handle the traceback and vars
> > filtering with a mechanism similar to __traceback_hide__. This would
> > allow controlling leakage per frame and make it less likely to be
> > missed on view.
>
> > Example:
> > def do_stuff():
> > ...
> > do_stuff.__traceback_filter_vars__ = ['credit_card', 'password',
> > 'launch_codes']
>
> I'm not sure this even possible - can you go from a traceback frame to
> the *function* object? __traceback_hide__ is a local attribute, not a
> function attribute.
globals as seen by the frame.
function_name = tb.tb_frame.f_code.co_name
func = tb.tb_frame.f_globals.get(function_name)
On Jun 2, 3:33 pm, Luke Plant <L.Plant...@cantab.net> wrote:
> We could do the same with __traceback_filter_vars__ and make a local
> variable. But that could be an implementation detail of the
> ExceptionReporterFilter (just as respecting __traceback_hide__ would be).
developer can flag a 3rd party function is by forking the code.
Patching an attribute on to a function is a much more flexible option.
Regards,
Michael Manfre
Julien Phalip
welcoming any further feedback:
https://code.djangoproject.com/attachment/ticket/14614/14614.exception-reporter-filter.diff
Regards,
Julien