Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Strip access_token from outgoing requests #3327

Merged
merged 2 commits into from
Jun 5, 2018
Merged

Strip access_token from outgoing requests #3327

merged 2 commits into from
Jun 5, 2018

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Jun 2, 2018

Fixes #2396

Signed-off-by: Michael Telatynski [email protected]

@turt2live
Copy link
Member

How does this differ from #3230 ?

@t3chguy
Copy link
Member Author

t3chguy commented Jun 2, 2018

@turt2live this doesn't apply a regex to every line being logged, only applies regex to the URIs being logged from outgoing requests
The other PR would run two regexes;
one fixed one to strip access tokens from incoming reqs,
one configurable one to strip them from every log line.

Also: #3230 (comment)

@richvdh
Copy link
Member

richvdh commented Jun 5, 2018

looks sensible in principle, but please could we export a redact_uri function from synapse.http (not site) and use that in both locations, rather than C&Ping the re.sub code?

@richvdh
Copy link
Member

richvdh commented Jun 5, 2018

\o/

@richvdh richvdh changed the title Strip access_token from outgoing requests using existing regex Strip access_token from outgoing requests Jun 5, 2018
@richvdh richvdh merged commit e316407 into matrix-org:develop Jun 5, 2018
@turt2live
Copy link
Member

There's still a few missed cases:

Errors:

2018-06-05 23:17:07,996 - synapse.http.client - 116 - INFO - - Error sending request to  PUT http://172.16.0.11:8184/_matrix/appservice/r0/transactions/1?access_token=ManuallyRedcated: TimeoutError

During startup the hs_token and as_token are leaked:

2018-06-05 23:18:46,627 - synapse.config.appservice - 80 - INFO - None- Loaded application service: ApplicationService: {'sender': '@_xyz:dev.t2bot.io', 'server_name': 'dev.t2bot.io', 'url': 'http://172.16.0.11:9005', 'rate_limited': True, 'id': '836ce04b44dd2358a2bf977364139eb3e6cf8e363a3374fcf062bddfcc30c5d52', 'token': 'ManuallyRedcated', 'namespaces': {'aliases': [{'regex': <_sre.SRE_Pattern object at 0x7ff7d9c07570>, 'exclusive': True}], 'rooms': [], 'users': [{'regex': <_sre.SRE_Pattern object at 0x7ff7d9c07490>, 'exclusive': True, 'group_id': '+.xyz:dev.t2bot.io'}]}, 'hs_token': 'ManuallyRedcated', 'protocols': set([])}

@turt2live
Copy link
Member

and:

2018-06-05 23:27:30,871 - synapse.http.client - 109 - INFO - - Received response to  PUT https://REDACTED/transactions/1?access_token=REDACTED: 410

@t3chguy
Copy link
Member Author

t3chguy commented Jun 6, 2018

fixing this now @turt2live

richvdh added a commit that referenced this pull request Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants