globaleaks / whistleblowing-software

GlobaLeaks is free, open-source whistleblowing software enabling anyone to easily set up and maintain a secure reporting platform.
https://www.globaleaks.org
Other
1.23k stars 269 forks source link

Unhandled Exception report does not report which tenant is generating an error #2221

Open fpietrosanti opened 6 years ago

fpietrosanti commented 6 years ago

Actually the Unhandled Exception Reporting report the hostname of the platform triggering the error, but in a multi-tenancy condition does not report the Tenant ID and Tenant Hostname required to understand what's triggering the error.

Actually we see: Platform: try.globaleaks.org (bvidswzoj322engc.onion) Version: 3.0.11

But it should be: Platform: try.globaleaks.org (bvidswzoj322engc.onion) Site: blahblah.try.globaleaks.org (blahblah.onion) Version: 3.0.11

evilaliv3 commented 6 years ago

@fpietrosanti: actually this is a wrong reported error that lies in a misunderstanding.

For clientside exceptions the tenant id is always reported as part of the hostname. If this was a clientside exception "Platform: try.globaleaks.org (bvidswzoj322engc.onion)" would identify that the exception happened on the root tenant (during signup or signup activation).

On backend exceptions instead all the exceptions are notified to the root tenant and there is no possibility for discerning what caused the issue programmatically but only analytically looking at the stacktrace.

evilaliv3 commented 6 years ago

Actually for handlers we could create an exception handler in order to be able to wrap all the exception happened inside the context of the handler and identify on which tenant the handler was operating.

evilaliv3 commented 6 years ago

With the previous patch i've addressed the issue by:

Example of the mail header:

Platform: PietrosantiLove
Host:  antani.gov (wacxrmkjvwhmyz2g.onion)
Version: 3.0.12

Site ID: 2
Host:  pietrosanti.love (xwdfdacxrmkjvw.onion)

URL: http://p2.antani.gov/public

[...]
evilaliv3 commented 6 years ago

Actually the fix implemented in https://github.com/globaleaks/GlobaLeaks/commit/7db571cbc2939f25719cfc8ef6a3f769d11cc5f7 has some instability.

With the previous patch i've reverted part of the patch that reopens this ticket.

Not all the exceptions in fact accepts injection of additional attributes and this lead to an exception in the management of the exception; the integration of this patch is so postponed for further analysis.

A more proper and robust solution probably would be to sandbox requests inside a thread and attach these variable to the thread using the local thread storage of python threads.

This would be a good option for many other reasons. Spawning a single thread at the start of the handler would reduce the number of the threads necessary during the life of the handler; currently in fact multiple threads are spawned in relation to file operations and database queries. Having a single thread would also give more benefit to database queries that would make better reuse of resources and cache.

fpietrosanti commented 6 years ago

@evilaliv3 doing this is a design change of some kind, where each HTTP request create a new thread?

evilaliv3 commented 6 years ago

Yes exactly.

Currently the typical handler spawns more than one thread and each of this thread is independent in the usage of internal caches; e.g. the typical handler spawns a thread for each query + a thread for each file handling.

if by design we could just always use a thread from the thread pool the code would be more optimized and we could store variables in the thread local storage that would behave as a global namespace for the handler execution.