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

Too much information contained in .query #90

Open
jkiddo opened this issue Oct 19, 2023 · 8 comments
Open

Too much information contained in .query #90

jkiddo opened this issue Oct 19, 2023 · 8 comments

Comments

@jkiddo
Copy link

jkiddo commented Oct 19, 2023

As discussed on https://chat.fhir.org/#narrow/stream/179166-implementers/topic/AuditEvent/near/397121962 I really don't think its a good idea to keep the entire raw request in

The raw search request is base64 encoded and placed in the .entity[query].query element. The base64 encoding of the raw search request enables preserving exactly what was requested, including possibly malicious patterns. This enables detection of malicious or malformed requests.
. Its simply too dangerous and it will be a gold mine for exploiting long lived tokens from other users.
-this is also related to #87

@JohnMoehrke
Copy link
Contributor

Is there some profile of the oauth token that can be described that preserves in the audit that which is useful while explicitly excluding the concerning portions? We need subject matter expert to define this profile of the oauth token for this use-case.

@JohnMoehrke
Copy link
Contributor

no matter what we profile, the audit log is always a gold mine for exploiting. Long lived tokens are a bad security choice. I agree that we should limit the risk. However I had not received any subject matter expert advice by publication time. We can adjust this in a CP.

@jkiddo
Copy link
Author

jkiddo commented Jul 16, 2024

The auditlog is a goldmine for sure in itself. Adding the full requests with any potential long lived tokens to it takes it to a whole other level of exploit minefield as long lived tokens can be used for the purpose of getting access to data NOT yet in the auditlog.

@jkiddo
Copy link
Author

jkiddo commented Jul 16, 2024

My subject matter expert advice is as follows: Omit the full access token from the auditevent. If the access token is in the form of a signed JWT, then remove the signature part of it in the AuditEvent.

@jkiddo
Copy link
Author

jkiddo commented Jul 16, 2024

Going ahead making a publication where the full request and all headers are added to the auditlog is not recommended. I fail to understand if publication continues without addressing this. This comment has been around for 9 months and nothing has happened. If github is the wrong place for issues and the HL7 Jira is a better fit then I suggest to remove the option to raise issues on this repo.

@jkiddo
Copy link
Author

jkiddo commented Jul 16, 2024

Keeping the access token in its full form in the auditevent increases the number of attack vectors of the system.

@JohnMoehrke
Copy link
Contributor

The problem with that is that both IUA and SMART have extensions in the oAuth token that are important to understand the transaction. We can't just not record this information. Plus there is a need to have some identity of the specific token somewhere.

@JohnMoehrke
Copy link
Contributor

I have indicated multiple times... I want to profile what we record from the oauth token... please provide guidance to that goal.

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

No branches or pull requests

2 participants