camaraproject / Commonalities

Repository to describe, develop, document and test the common guidelines and assets for CAMARA APIs
Apache License 2.0
14 stars 28 forks source link

remove empty security scheme #204

Open AxelNennker opened 6 months ago

AxelNennker commented 6 months ago

Problem description

In OAI {}denotes empty security.

From the spec:

A declaration of which security mechanisms can be used across the API. The list of values includes alternative security requirement objects that can be used. Only one of the security requirement objects need to be satisfied to authorize a request. Individual operations can override this definition. To make security optional, an empty security requirement ({}) can be included in the array.

From notification-as-cloud-event.yaml

security:
    - notificationsBearerAuth: []
    - {}

Expected behavior No Camara API should have empty security.

Alternative solution Remove empty security from all Camara yaml files, esspecially those in Commonalities. notification-as-cloud-event.yaml

rartych commented 6 months ago

@AxelNennker the notification-as-cloud-event.yaml is the helper artifact that shows how the notification receiver can be implemented by API Consumer. The event notification endpoint is used by the API server to notify the API consumer that an event occurred. It is not the specification of CAMARA API.

I think, the empty security scheme is added deliberately here , as it is up to API Consumer what security scheme he uses for receiving notifications.

I propose to close this issue or transform it to improve the description to clarify the security of notification-as-cloud-event.yaml.

AxelNennker commented 6 months ago

@AxelNennker the notification-as-cloud-event.yaml is the helper artifact that shows how the notification receiver can be implemented by API Consumer. The event notification endpoint is used by the API server to notify the API consumer that an event occurred. It is not the specification of CAMARA API.

As I said, I think no Camara API should have no-security. API-Subgroups are going to copy the lines from this artifact and then clients think they can subscribt without any authorization.

I think, the empty security scheme is added deliberately here , as it is up to API Consumer what security scheme he uses for receiving notifications.

I don't understand. It is for the API subgroup to decide which security is implemented and THEN the client can choose if there are multiple. But I think that Commonalities should recommend which security scheme to use. And I think there should only be two:

The current "recommendation" here is:

  securitySchemes:
    notificationsBearerAuth:
      type: http
      scheme: bearer
      bearerFormat: "{$request.body#/webhook/notificationAuthToken}"

bearer does not say how the token is generated or retrieved. I do not understand the intention here. The Camara AZ knows all the clients, their client_id s, their public keys for various purposes, whatever. The AZ should create the bearer token. It is an access token created by the AZ, right? Is this just a placeholder where the subproject has to define their own security scheme? Strange.

Maybe there should be an new issue on the security schemes?

I propose to close this issue or transform it to improve the description to clarify the security of notification-as-cloud-event.yaml.

Please clarify.

Ceterum censeo no security esse delendam

AxelNennker commented 6 months ago

Maybe I am misunderstanding this completely and your intention is the other way around. This is not for subprojects but for clients. Still

AxelNennker commented 6 months ago

I was/am thinking along the lines of CIBA. The client requests to receive notifications and sends an register-for-notifications event authenticated by some client authentication. Then when the event happens the telco sends the notification to the client's endpoint.

Anyway, some clarification is needed.

rartych commented 6 months ago

@AxelNennker It looks that guidelines changes in PR https://github.com/camaraproject/Commonalities/pull/198 clarify the security of notifications. I think the https://github.com/camaraproject/Commonalities/blob/main/artifacts/notification-as-cloud-event.yaml should be changed accordingly and its purpose should be stated more explicitly. @patrice-conil @bigludo7 WDYT?

PedroDiez commented 6 months ago

The fact of indicating empty security is deliberate in order to not force an API consumer to provide a security mechanism.

security:
    - notificationsBearerAuth: []
    - {}

In case it provides one security mechanism, it is up to the API consumer its generation. So the "notificationsBearerAuth" schema. That is the current approach.

This issue opens several points: (that we will need to discuss)

AxelNennker commented 6 months ago

Specifying the need for "something-something-bearer-token" is not enough, I think.

Camara is the source and, as currently specified, needs to send the authorization that is the bearer token.

I assume Carama does not want to become a client to the API Cosumer's AZ which would be one option to retrieve an access token that is then send as a bearer token.

Nobody should setup an endpoint with no security. See e.g. Zero Trust The IT Industry has been there when it was enough for one server and second to be in e.g. a DMZ and trusted each other. Not any longer. Not even inside (telcos) internal systems networks should there be blind trust.

Some might have the view "What should Camara care if the API consumer not protected?"

I say that we must care and we should not send notifications from telco/Camara systems to some insecure system. A sink without access control could be swamped with notifications from any source. A sink without access control could receive notifications from anywhere and think the notifications are originating from a Camara server. If the sink draws the wrong conclusions from those fake notifications then some might put blaime on Camara/telcos regardless of whether we are to blaim or not.

What are the options?

PedroDiez commented 6 months ago

Discussion is fair, however we need to agree first in which direction we move on and, if we have room and agree to address within Meta-Release Fall24.

Currently, we as "CAMARA" allow the "API Consumer" endpoint to be optionally protected, and in case it is protected, API Consumer indicates within "notificationAuthToken" (what it would be sinkCredential concept, ACCESSTOKEN type)

  1. First point is to reach agreement in Commonalities and its roadmap about mandating securization of notifications (requirement for API Consumer)

    • NOTE: In this context I also assume in principle that Carama does not want to become a client to the API Consumer's AZ. Because in that case it makes no sense for the API Consumer to indicate the securization in the request and this put requirements in both sides: API Consumer in order to expose an AZ Server and Telco Operator in order to interact with it and get a Token.
  2. Second Point, separated from above it is about which securization enable. So far, "ACCESSTOKEN" was considered to be aligned with the fact API Consumer indicates and oAuth2 Token.

AxelNennker commented 6 months ago

Please see my comment https://github.com/camaraproject/Commonalities/pull/198#discussion_r1613625407

It is not in Camara's interest to ignore the security of the API consumer. It is in our interest that only the API consumer can create an access token that is e.g. like an CAMARA Security and Interoperability Profile

AxelNennker commented 6 months ago

Please see https://github.com/camaraproject/Commonalities/pull/198#discussion_r1620229350

I think the empty security scheme should be removed and the guideline should not suggest to API consumers that "credentials are not needed".

eric-murray commented 3 months ago

@AxelNennker

I think the empty security scheme should be removed and the guideline should not suggest to API consumers that "credentials are not needed".

When the notification structure was being developed in Quality on Demand, the original proposal was to "force" the API consumer to specify a "token" (an arbitrary string), which would be included in an Authorization: Bearer header with each notification. So the API consumer could generate a genuine token for the server, and validate it against their own authorisation server. Or they could specify a "fake" token that they just ignored. It was up to them.

But the objection was raised that we shouldn't pretend it was a bearer token if it was not, and hence the explicit option to have "no security" (notifications are always sent using TLS, of course) was included. This also slightly reduces the size of the notification, as no Authorization header is required.

Of course, the API consumer could still provide a fake token if they wished. So, by removing the "no security" option (and assuming that the very same objection was not raised again), we would be back to that being the only option for API consumers who do not want to validate the token in any notifications.

I don't mind that, though I don't see what is really being achieved, other than to inconvenience the API consumer slightly more by forcing them to specify a fake token they then ignore rather than none at all. And we would need to improve the documentation, as giving the impression that we require the API consumer to maintain an OAuth server in order to receive notifications (even though not true) would put many off using CAMARA APIs.

PedroDiez commented 2 months ago

The key point here is that, if we agree from Commonalities point of view that Notifications MUST be secured, then we are implicitly saying that the API consumer has to provide that token when creating the subscription (instance-based or resource-based)

cc @jlurien