camaraproject / WorkingGroups

Archived repository used previously by CAMARA Working Groups.
https://wiki.camaraproject.org/display/CAM/CAMARA+Working+Groups
42 stars 60 forks source link

POST or GET to retrieve network information #212

Closed bigludo7 closed 1 year ago

bigludo7 commented 1 year ago

We are crafting the operation to retrieve raw location of a device and we're hesitating between using a POST or GET operation.

I will not detail here the pro & cons of each as it is a very old debate on REST task-based resource.

As of today we have POST use for Device status (to get roaming), to check device location but we have a GET for Device identifier (to get IMEI) and Anonymised Identifier while the parameters in the request are the same (ue identifier).

I'm not sure we can have a global rule a commonalities level bit prefer to get team feedback before to continue the API design. cc: joseluis.urienpinedo@telefonica.com

jlurien commented 1 year ago

Thanks @bigludo7 to raise this. Basically, discussion is about how to model data retrieval when input is a complex object:

A) UrlEncoded( GET /my-api/v0/data?complexInput={"property1":"value1", "property2": {"property2.1": "X", "property2.2": "Y"}} )

B) POST /my-api/v0/retrieve-data { "property1": "value1", "property2": { "property2.1": "X", "property2.2": "Y" } }

Current guidelines suggest to model data retrieval as GET but POST may be more convenient when the request body is complex. It is possible to pass a complex object as query parameter but it has to be encoded and it is less readable. Also URLs may have some length limitations.

eric-murray commented 1 year ago

@jlurien, @bigludo7 Actually, I think security considerations are more important here. URL length limits may be a factor in preventing the use of option (A) when the request includes a very complex object, but readability shouldn't be an issue, as nobody should be typing these requests in manually. They will be machine generated.

However, if any of the properties to be passed are "sensitive" (e.g. MSISDN), then they should not be passed as query parameters, as many security requirements prohibit this. The current CAMARA design guidelines say that sensitive parameters in a GET request should be passed as headers, This would work, but other design guidelines recommend that sensitive parameters be passed in the body of a POST, which is why that mechanism was originally adopted by the Device Status API and others. But it is currently prohibited by the CAMARA guidelines if the request is for information retrieval, as that has to be a GET.

So the question is whether the CAMARA design guidelines should allow POST for information retrieval. My view is that the convention of putting sensitive parameters or information of importance to the business logic of an API in the body of a POST, rather than in headers or as query / path parameters, is so prevalent that it should be allowed by CAMARA, in addition to the existing options.

Of course, the only reason we are having this debate is because the IETF did not mandate that request bodies for GET requests should be supported if present (it is allowed, but it is also allowed for proxies to drop them). But that is "water under the bridge", as the English idiom goes.

BTW - for Device Identifier and Anonymised Subscriber Identifier, my expectation is that all these device / subscription identification parameters will eventually be removed and replaced by a mechanism that allows these parameters to be determined via the OAuth token. So my intention is to leave these as header parameters for now, even though it is a bit clumsy.

jlurien commented 1 year ago

Thanks @eric-murray. I agree with your arguments. Another one that is sometimes raised in this sort of discussion is that GET are often cached while POST are not. For the retrieval of dynamic information, POST is in this case a better option.

In this table extracted from here there is good summary, but most of arguments do not apply when a browser is not involved. My view is that we should allow both POST or GET and decide the better one case by case. When using POST for data retrieval we should also indicate that the resource in the path must be a verb, to remark that we are not creating a new resource under a collection.

Compare GET vs. POST

The following table compares the two HTTP methods: GET and POST.

  | GET | POST -- | -- | -- BACK button/Reload | Harmless | Data will be re-submitted (the browser should alert the user that the data are about to be re-submitted) Bookmarked | Can be bookmarked | Cannot be bookmarked Cached | Can be cached | Not cached Encoding type | application/x-www-form-urlencoded | application/x-www-form-urlencoded or multipart/form-data. Use multipart encoding for binary data History | Parameters remain in browser history | Parameters are not saved in browser history Restrictions on data length | Yes, when sending data, the GET method adds the data to the URL; and the length of a URL is limited (maximum URL length is 2048 characters) | No restrictions Restrictions on data type | Only ASCII characters allowed | No restrictions. Binary data is also allowed Security | GET is less secure compared to POST because data sent is part of the URLNever use GET when sending passwords or other sensitive information! | POST is a little safer than GET because the parameters are not stored in browser history or in web server logs Visibility | Data is visible to everyone in the URL | Data is not displayed in the URL
RubenBG7 commented 1 year ago

When lot of information is needed to be sent to retrieve the information, POST verb would be convenient due to security criteria as @eric-murray and @jlurien say. Of course indicating that the resource in the path must be a verb remarking that we are not creating a new resource.

gregory1g commented 1 year ago

However, if any of the properties to be passed are "sensitive" (e.g. MSISDN), then they should not be passed as query parameters, as many security requirements prohibit this. The current CAMARA design guidelines say that sensitive parameters in a GET request should be passed as headers

Could you please indicate exact security requirements which prohibit that and reasons behind. May be OWASP or something like that? afaik, a path/query are covered by HTTPS same way as a body or headers. The only security problems related to path/query I saw are related to web browsing - like your browser will keep the full path in history, but this is not relevant for APIs.

for example Twilio and Sinch use MSISDN in path/query part of the request

eric-murray commented 1 year ago

@gregory1g This type of restriction can probably be traced back to the CWE-598, which itself can probably be traced to IETF 9110. OWASP have a page on this issue.

In these days of GDPR, MSISDN is as "sensitive" as a password, and has to be treated as such, so APIs that currently allow this could well come unstuck in future. I agree that TLS and using client implementations other than web browsers helps mitigate this risk, but it doesn't eliminate it. Nothing prohibits calling CAMARA APIs from a browser, and the TLS termination point is not necessarily the same as the API termination point.

So CAMARA are not about to allow the inclusion of sensitive parameters as query strings. Currently, the guidelines require information ("resource") retrieval to be done using a GET, so sensitive data should be put in a header. This issue is to discuss if POST can also be used for information retrieval in some circumstances.

gregory1g commented 1 year ago

Well, all mentioned references talk about sensitive information like session id, passwords or session tokens and focus on web browsing. These data can really help to attack the system. Not that relevant for MSISDN.

Also it should be better to separate "security sensitive" data like tokens and passwords and their handing from personal data like names and their handling.

BTW, not an expert in GDPR, but I did not found a clear explanation if MSISDN is assumed to be a personal data.

"Currently, the guidelines require information ("resource") retrieval to be done using a GET, so sensitive data should be put in a header. " This pattern is rather error-friendly: one must be very careful with "lookup-like" APIs which allow flexible query criteria. If criteria can potentially include some personal data. Considering backward compatibility, it could make sense to always put a query to a header until one 200% sure that the query can not include personal data in any form and never will.

PS: TLS termination should better be trusted, independent on this exactly discussion.

bigludo7 commented 1 year ago

So if I try to move toward a conclusion on this topic after this valuable comments:

Are we in position to agree on this or did I miss blocking point?

gregory1g commented 1 year ago

Is could make sense to separate two sitations:

  1. using POST instead of GET to bypass technical limitations like potentially too big input (if could be longer than 4k characters) or a complex structure of the input. for these cases it makes sense to relax guidelines and allow POST + function like API (a verb as a resource)

  2. deviate from RESTFul design for security/privacy reasons Here I would suggest to clarify with security and GDPR expects. And if some parameters really need such protection, explicitly list parameters which are seen as "sensitive enough" to be forbidden to be used as a resource identity or in a GET query.

shilpa-padgaonkar commented 1 year ago

@bigludo7 : Fine with the points. It would be great however if can list down the concrete conditions under which the subproject can make such a deviation as a part of the guidelines. Also explicitly listing down the parameters which are considered sensitive is a good idea as suggested above by @gregory1g

bigludo7 commented 1 year ago

@shilpa-padgaonkar : I can make a contribution to the guideline §3.1 to add a specific paragraph on this topic about handling 'task-based' resource - It works for you?

bigludo7 commented 1 year ago

Proposal to add a paragraph in §3.1:

<>

POST or GET for retrieving sensitive data

Using GET operation exposes information through query string URL. This information for example remain in browser history and could be visible to everyone checking the URL. This allow fraudsters to obtain sensitive data. Using HTTPS does not solve this vulnerability.

The classification of data tagged as sensitive should be assessed in API project but data like phone number must be cautiously handled

In such case, it it recommended to use POST verb instead of GET to perform information retrieval.

When POST is used, the resource in the path must be a verb (eg retrieve-location and not location) to differentiate from 'real' resource creation.

Note: For technical limitation, it is also fine using POST instead of GET to bypass technical limitations like potentially too big input (if could be longer than 4k characters) or a complex structure of the input.

<>

@jlurien @gregory1g @RubenBG7 @gregory1g @shilpa-padgaonkar May I ask to take a look and correct/complete if needed. Thanks

gregory1g commented 1 year ago

Using GET operation exposes information through query string URL. This information for example remain in browser history and could be visible to everyone checking the URL. This allow fraudsters to obtain sensitive data. Using HTTPS does not solve this vulnerability.

This argument makes a lot of sense for a web URI where browser is the the target client. And it is a "vulnerability" for security relevant data like session tokens (or "user name" one uses for login). Because people copy URL from browser and send them each other etc.

APIs URIs are not expected to be typed directly in the browser address bar by users. A developer can use a browser to test an API, but the same way they can use, for example, the wget and put request body in the command line - visible for anyone with access to bash history etc.

It would be great to provide an example of an attack which allows "a fraudsters to obtain sensitive data" from a production system which includes MSISDN in the resource path which will not work if MSISDN is moved to the request body as a motivation for the decision.

eric-murray commented 1 year ago

@gregory1g You also need to consider what is happening within a CSP:

So, really, it makes our lives slightly easier if sensitive information such as MSISDNs are not in URLs. In general, we want to avoid storing information subject to GDPR in locations where it is not required, and this sort of "accidental" storage is a concern.

As for why obtaining the MSISDN associated with a transaction opens up a vulnerability, well the URL will probably also give some clues as to the nature of the transaction, which might be related to a financial transaction. So you send them a text (you have their number after all) - "Please review the transaction you just made by following this link ...".

So it is not solely about the MSISDN itself, but also knowing something about what transactions are being processed for that customer.

gregory1g commented 1 year ago

@eric-murray , Thank you! I'm a bit skeptical about some of the mentioned points, like removing whole business logic parameters from request logging - without them, logging are usually hardly usable - what, rather probably will end up in adding request body in the logs as well. Anyway,

These are the only suggestion I do have.

bigludo7 commented 1 year ago

@gregory1g @eric-murray Thanks for the input !

Adding @shilpa-padgaonkar Do we really need to go so deep in the guideline document ? for me the document objectives is to help CAMARA API project team for the 'HOW' elaborate API.

For me at the level of the guidelines we should just alert about sensitive data handling (and providing an example) and provide a way to handle them (with POST operation) . After detailed discussion should be within API project group.

shilpa-padgaonkar commented 1 year ago

@bigludo7 My guess is that this request to make it concrete will return to commonalities at some point again in the future :) But I do not intend to block your update just for this point. Hopefully you will add the example as you have stated above.

bigludo7 commented 1 year ago

Thanks @shilpa-padgaonkar - will provide a PR to handle this.

shilpa-padgaonkar commented 1 year ago

Fixed with merge of PR #244