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

USH-001 -- Secure Credentials Leak #1596

Closed
willdoran opened this issue Mar 6, 2017 · 7 comments
Closed

USH-001 -- Secure Credentials Leak #1596

willdoran opened this issue Mar 6, 2017 · 7 comments
Assignees

Comments

@willdoran
Copy link
Contributor

willdoran commented Mar 6, 2017

Expected behaviour

  • Never send OAuth 2.0 authentication headers to a third party. This undermines thewhole OAuth 2.0 concept of keeping tokens secure.

Actual behaviour

  • Ushahidi.io sends information to third party Getsentry for stacktrace logging. This can be used to troubleshoot and resolve anyissues that users experience.

Implementation

Client

  • Add sanitizeKeys configuration to ravenjs in platform client.
  • Filter any fields matching Authorization, client, secret and password fields

API

  • Add processorOptions config to sentry php to send data through Raven_Processor_SanitizeDataProcessor
  • Filter any fields matching Authorization, client, secret and password fields
  • Check for any other sensitive fields used in the API.

Acceptance criteria

  • Configure you local API and client deployments to use platform staging app
  • Trigger errors on the client and watch for sentry HTTP requests
  • Check Sentry requests do not contain sensitive info
  • Check Sentry dashboard and confirm logged errors do not contain sensitive info

NB: You may need to temporarily enable allowDuplicates and other similar config in client to more easily trigger requests to sentry.

This is set here:
https://github.com/ushahidi/platform-client/blob/develop/app/index.html#L130
and the reference for config is here:
https://github.com/getsentry/raven-js/blob/master/docs/config.rst

@willdoran willdoran changed the title Secure Credentials Leak USH-001 -- Secure Credentials Leak Mar 6, 2017
@rjmackay
Copy link
Contributor

rjmackay commented Jun 7, 2017

Not sure exactly how we do this. But perhaps the dataCallback option will allow it https://docs.sentry.io/clients/javascript/config/

@Angamanga
Copy link
Contributor

@willdoran Is this part of the security-update for this cycle? Could you move to spec once you start speccing it?

@rjmackay
Copy link
Contributor

I've added some spec details to implementation and acceptance criteria.
I believe Will already started work on this in Montreal. @willdoran what branch was your WIP one?

@kinstella
Copy link
Contributor

Where this says 'Check for any other sensitive fields used in the API.', is the intent to filter those fields with processorOptions, as well?

@rjmackay
Copy link
Contributor

Where this says 'Check for any other sensitive fields used in the API.', is the intent to filter those fields with processorOptions, as well?

Yes. I just didn't want to try to make an exhaustive list since by the time I've done that I might as well write the code at the same time

@willdoran
Copy link
Contributor Author

@rjmackay This makes sense to me though I know what allowDuplicates means, just adding a note to the description

@rjmackay
Copy link
Contributor

rjmackay commented Jul 4, 2018

I just tested this on http://csvexportsforever.v3-qa.ush.zone and there's still some data leakage in the trace somewhere. I think the API side is fine, but client needs more checking :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants