Stream: implementers
Topic: Security Problem
Grahame Grieve (Oct 17 2017 at 02:17):
I've run into a security issue I'm interested in opinions on
Grahame Grieve (Oct 17 2017 at 02:18):
say that you have a server that allows a client to PUT to a new location
Grahame Grieve (Oct 17 2017 at 02:19):
(some servers allow this - it's a valid option for some use cases)
Grahame Grieve (Oct 17 2017 at 02:19):
now, a server is serving a client with a user session - e.g. from Smart on FHIR - that is restricted to only resources linked with one patient
Grahame Grieve (Oct 17 2017 at 02:20):
and this client PUTs a new resource to a location that is already in use for another patient
Jim Steel (Oct 17 2017 at 02:20):
If its already in use, its not a new resource?
Grahame Grieve (Oct 17 2017 at 02:23):
no, but as far as the client is concerned it is, because if they do a GET on that location, they'll geta 404 not found
Jim Steel (Oct 17 2017 at 02:24):
Why do they get a 404 and not a 401?
Jim Steel (Oct 17 2017 at 02:25):
or 403...
Grahame Grieve (Oct 17 2017 at 02:30):
see http://hl7.org/fhir/security.html#AccessDenied
Grahame Grieve (Oct 17 2017 at 02:31):
and the question really relates to this section - you can't return a 404 if the operation is PUT POST DELETE
Grahame Grieve (Oct 17 2017 at 02:31):
so I'm interested in @John Moehrke opinions
Grahame Grieve (Oct 17 2017 at 03:15):
@Rick Geimer there's an impact to SD from this as well - what should a server do if a client requests for a document to be generated, and the user doesn't have the rights to all the resources they're asking for? fail gracefully, or blow up?
Kevin Shekleton (Oct 17 2017 at 03:23):
@Grahame Grieve - This is an interesting scenario. I understand the concept but do you have a concrete example of a URI that the client decides with their PUT that would already exist (but not visible to them for security purposes)
Grahame Grieve (Oct 17 2017 at 03:28):
well, I'm testing with this now. PUT [base]/Observation/1 where Observation/1 already exists, and is assocaited with Patient/example, but the client session is restricted to Patient/1
Kevin Shekleton (Oct 17 2017 at 03:34):
I cannot think of a status that doesn't allow the client to use PUT to determine the existence of a resource
Kevin Shekleton (Oct 17 2017 at 03:37):
If we return anything other than a 2xx response the client can simply retry the PUT with a different id and if it succeeds, they know a resource exists at the other id
Grahame Grieve (Oct 17 2017 at 03:37):
no indeed. that's kind of my point. I couldn't think of one either. Wanted to check I hadn't overlooked anything. Especially since security made such a big deal about this
Grahame Grieve (Oct 17 2017 at 03:38):
I think we need to add a note to that section about the 404 supression only working for get not the other operations
Kevin Shekleton (Oct 17 2017 at 03:38):
However, I think PUT with some URI id that can collide easily is a bad idea
Jim Steel (Oct 17 2017 at 03:39):
405 doesn't work?
Jim Steel (Oct 17 2017 at 03:39):
i guess its no better than 403 in terms of concealing the id space
Kevin Shekleton (Oct 17 2017 at 03:40):
@Jim Steel - No. The server on a 405 must respond with the allow methods on that URI and if the user is not authorized, they will get no methods. This now leaks that the resource exists
Grahame Grieve (Oct 17 2017 at 03:40):
I chose simple ids for ease of testing and description. In production, you'd expect to use UUIDs and this would be very unlikely.. unless we think about malicious use, say, on a patient portal
Kevin Shekleton (Oct 17 2017 at 03:41):
Really, anything than a 2xx reveals the existence of the resource (if the error can be resolved by simply replaying the tx with a different id)
Grahame Grieve (Oct 17 2017 at 03:41):
there's no way to stop leakage if write is allowed. What do you do about this?
Jim Steel (Oct 17 2017 at 03:41):
I can't see any around it. at the end of the day, the client must be able to attempt to create, and be told whether or not it was successful
Kevin Shekleton (Oct 17 2017 at 03:41):
Don't allow PUT?
Grahame Grieve (Oct 17 2017 at 03:41):
though leaking that a resource you can't see exists doesn't really mean anything
Kevin Shekleton (Oct 17 2017 at 03:41):
This problem doesn't exist for POST
Grahame Grieve (Oct 17 2017 at 03:41):
no, but do you allow updating a resource? then you allow a PUT
Jim Steel (Oct 17 2017 at 03:42):
Yeah, you're not leaking the resources themselves, only their ids (and then only by guesswork, not in the form of a list
Kevin Shekleton (Oct 17 2017 at 03:43):
True, leaking a resource URI is less bad than the full resource. Leaking resource URIs is really bad when the URI itself tells you information (eg, /patients/123/hiv-results/456 <--not FHIR but you get the example)
Kevin Shekleton (Oct 17 2017 at 03:44):
In REST, POST can be used to update a resource; you don't have to use PUT
Grahame Grieve (Oct 17 2017 at 03:44):
yes. but in practice, sensitive resources will not have semantic ids (unlike conformance resources which might)
Kevin Shekleton (Oct 17 2017 at 03:44):
Agreed
Grahame Grieve (Oct 17 2017 at 03:44):
it doesn't matter if you use POST or PUT to a known resource.
Kevin Shekleton (Oct 17 2017 at 03:47):
I was thinking of POSTing to the collection (eg, /thing) rather than the resource (/thing/123)
Kevin Shekleton (Oct 17 2017 at 03:47):
But I guess if you don't have access to view the entire collection (/thing) now we're back to the initial problem
Grahame Grieve (Oct 17 2017 at 03:48):
yep
Grahame Grieve (Oct 17 2017 at 03:49):
it's a little easier if you say that you can't PUT or POST to a resource that doesn't exist; then you can always say 'not found' if there's no access.
Christiaan Knaap (Oct 17 2017 at 13:54):
In my implementation I chose to return 401 Unauthorized. And a TODO: "Check whether this leaks too much information on existing resources to the user.", so I would have come to the same question as you Grahame :-)
And how about 409 Conflict?
Christiaan Knaap (Oct 17 2017 at 13:55):
Presuming that the id in itself does not leak information.
Lloyd McKenzie (Oct 17 2017 at 14:07):
Well, if you're hitting up the local HIV clinic for [base]/Patient/[known patient id]/Condition/[search the possible identifier space], that may be leaking more information than is safe. Granted, it's dependent on knowing the patient id, so if that risk is managed, so is the leaking. In general, compartments allow for the possibility of greater leaking, so they should be used cautiously.
Grahame Grieve (Oct 17 2017 at 21:50):
I think we should add a note in the security section, and a cross reference in the section about allowing PUT to a new location
John Moehrke (Oct 18 2017 at 18:34):
just catching up... and you all seem to have come to the understanding. The guidance on authorization failure codes is intended to provide security considerations, it is not demanding any specific policy. If your risk assessment allows exposing 'some' information, because the benefit of that error code; then you are free to make that policy decision. We are just trying to warn the reader, especially clients that sometimes they might not get as specific of an error code because of security policy reasons. The space you all have zeroed in on is a user/system that is Authorized to PUT, but is somehow not trusted to see a collision. This is a very small exposure to a client that you know, and have a high trust for. As was also said, there are usecases for trusting a POST, while not trusting for anything else; thus trusting POST would be less risk than PUT.
John Moehrke (Oct 18 2017 at 18:36):
Im happy for someone to recommend improvements of the text based on this thread. I however am not clear what that might be, without also taking up many paragraphs to explain the specific case.
Grahame Grieve (Oct 22 2017 at 20:14):
Rick Geimer (Nov 07 2017 at 17:41):
@Grahame Grieve Getting back to this a bit late. Let's use the use case of someone with HIV as a Condition and someone without permission tries to access the document. In CDA we would probably represent this by including a Problem Observation where everything is nulled out (@nullFlavor = MSK or something similar), but the rest of the document would show as usual. Not sure how we would represent this in FHIR though. Generate the document and include the reference to the resource but not copy it into the bundle? That way someone with proper permissions could theoretically retrieve it later, but how do we flag that it was omitted for privacy/permission reasons?
John Moehrke (Nov 07 2017 at 18:19):
Provided that the user has some rights, that is enough rights to know that they got a redacted object... (See http://build.fhir.org/security.html#AccessDenied ) Then the object can be tagged as REDACT or some other appropriate tag http://build.fhir.org/v3/ActCode/cs.html#v3-ActCode-REDACT
Grahame Grieve (Nov 08 2017 at 00:32):
sure or you could use the masked data absent reason.
Grahame Grieve (Nov 08 2017 at 00:32):
But in most cases, the ruling is that you can't tell people about the information they don't know because knowing it is there is enough to infer what is being kept from view anyway
John Moehrke (Nov 08 2017 at 14:44):
Grahame, there are plenty of policies that would agree with you, but there are other policies that allow for letting the user know that they have not received everything. A well known example is the one that precedes a 'break-glass' opportunity. Where the user gets only "N" Normal data, with an indication that break-glass would reveal more information. They make the determination that within-policy they are correct in declaring a break-glass emergency, they indicate they are breaking-glass, they get "R" Restricted data too.. Also, their broken-glass is recorded in an audit log, the Privacy and Safety office review that, and either praise the clinician for saving a life or punish them for violating policy.
Jenni Syed (Nov 08 2017 at 15:16):
In our system, we usually tell them anytime they may have additional information, even if they wouldn't always. EG: we know they don't have access to all facilities, even if the patient doesn't have data at those facilities. When they break the glass, they may not actually get more info
Last updated: Apr 12 2022 at 19:14 UTC
