IHE / ITI.BasicAudit

The Basic Audit Log Patterns (BALP) Implementation Guide is a Content Profile that defines some basic and reusable AuditEvent patterns. This includes basic audit log profiles for FHIR RESTful operations to be used when there is not a more specific audit event defined. A focus is enabling Privacy centric AuditEvent logs.
https://profiles.ihe.net/ITI/BALP/index.html
Creative Commons Attribution 4.0 International
4 stars 3 forks source link

Too much information contained in .query #90

Open jkiddo opened 11 months ago

jkiddo commented 11 months ago

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 https://github.com/IHE/ITI.BasicAudit/blob/6b858fc41d44d23fa93414003724c8d2ad5bae31/input/pagecontent/volume-1.md?plain=1#L127. Its simply too dangerous and it will be a gold mine for exploiting long lived tokens from other users. -this is also related to https://github.com/IHE/ITI.BasicAudit/issues/87

JohnMoehrke commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

JohnMoehrke commented 2 months ago

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 commented 2 months ago

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