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

Usage of HTTP (instead of HTTPS) in for notifications #416

Closed
tlohmar opened this issue Feb 13, 2025 · 19 comments · Fixed by #418
Closed

Usage of HTTP (instead of HTTPS) in for notifications #416

tlohmar opened this issue Feb 13, 2025 · 19 comments · Fixed by #418
Labels
correction correction in documentation

Comments

@tlohmar
Copy link
Contributor

tlohmar commented Feb 13, 2025

Problem description

The API Design Guidelines mandates the usage of HTTP (instead of HTTPS) for notifications (Link to subscription attributes). Specifically the statement

Identifier of a delivery protocol. Only HTTP is allowed for now.

is misleading and should be replaced with "Only HTTPS is allowed for now".

Expected behavior
Update the attributes table (link) and replace all occurrences of 'HTTP' with 'HTTPS'.

Alternative solution

Additional context

@tlohmar tlohmar added the correction correction in documentation label Feb 13, 2025
@eric-murray
Copy link
Collaborator

In this context, HTTP refers to the protocol used to send the notifications. Alternatives might be MQTT or KAFKA. "HTTPS" means "HTTP over TLS", so it is still using the HTTP protocol to send the notifications (they just happen to be encrypted rather than plaintext).

Probably you are assuming that the API consumer must specify a sink with an https:// url. Certainly if they do, then the notifications will be sent using "HTTP over TLS". But I don't think that is explicitly mandated in the API design guidelines, and some implementations may choose to support http:// sinks.

Maybe we should mandate that sink urls use https://. But even then, the protocol would remain "HTTP".

@tlohmar
Copy link
Contributor Author

tlohmar commented Feb 13, 2025

hmm, this is a bit confusing now. The scheme identifier of the URL (http:// or https:// ) already defines the protocol. There are scheme identifies for MQTT (not standardized WIRC), which allows derivation of the protocol from the URL (e.g. mqtts://<broker>/...). Certainly fine to exclude other protocols. However, the restriction should be on the URL scheme, i.e. the data type of the sink property shall by URL formatted and the uri-scheme https shall be used.

Thus, it might be better to remove the protocol property, since it is redundant to the information from the URL scheme identifier. In principle, the present design allows for "running HTTP via MQTT" (which is already done today). It seems to be possible to set protocol = HTTP and use a sink URL with an mqtt scheme.

@eric-murray
Copy link
Collaborator

Indeed, the API consumer may specify a sink that is not compatible with their requested protocol. That would be a "run time error" on their part.

But the protocol parameter is used as a discriminator to restrict the additional options or information that the API consumer needs to provide. For example, by choosing the HTTP protocol, they can specify additional headers they would like sent with the notifications. This option would not be applicable to other protocols, and so it is better not to present it as an option at all for those.

So using the protocol as a discriminator helps the API consumer construct their request more reliably by changing the effective interface definition dependent on the protocol selected. This cannot be done dynamically by parsing the sink url specified.

Maybe the sink definition should be incorporated into the protocol specific parameters, in which case a pattern could be enforced for the scheme, only allowing certain values. One thing that is clear about CAMARA is that few read through the whole documentation. Embedding as many rules as we can in the OAS definition is a "good thing".

@tlohmar
Copy link
Contributor Author

tlohmar commented Feb 13, 2025

Embedding as many rules as we can in the OAS definition is a "good thing".

yes, clear rules simplify interoperability.

I still dont fully understand the usage of the protocol property. In the current version, only the value HTTP is allowed. Is the idea to allow other protocols over HTTPS in a future release?

In the current release, the protocol value is limited to HTTP and the URL-scheme of the sink value will be (or should be) limited to HTTPS. Thus, it would be possible to implement a behavior like "when the sink property is present, the sink value shall be an URI with URL-scheme https and a set of http headers shall / should be present".

@eric-murray
Copy link
Collaborator

Is the idea to allow other protocols over HTTPS in a future release?

Yes, that's the plan. The issue is "interoperability". If we allow the API consumer to choose between multiple protocols, then all must be supported by all API providers.

the sink value will be (or should be) limited to HTTPS

I don't think we do explicitly mandate HTTPS for notifications, but it should be discussed. The question is whether we want to support API consumers who have a static IP address, but don't want the overhead of maintaining a domain name and associated TLS certificate. Other protocols, especially MQTT, would be better suited for such API consumers.

@tlohmar
Copy link
Contributor Author

tlohmar commented Feb 14, 2025

Ok, thanks for the clarification. Would be good to make this a bit clearer.

I don't think we do explicitly mandate HTTPS for notifications

At least, HTTPS usage should be recommended. Recommending HTTPS still allows other schemes like mqtts or mailto (See here).

The question is whether we want to support API consumers who have a static IP address

Note, TLS certificates are typically bound to domain names. Certain certificate authorities (like Lets Encrypt) dont allow IP addresses, i.e. require a domain name.

Should I create a PR?

@eric-murray
Copy link
Collaborator

Note, TLS certificates are typically bound to domain names. Certain certificate authorities (like Lets Encrypt) dont allow IP addresses, i.e. require a domain name.

Yes, hence my question - do we want to support API consumers that only have a static IP address? If we do, then they need to use a http:// sink. Anyone who has gone to the trouble of registering a domain name should also know how to get a TLS certificate for that domain name.

At least, HTTPS usage should be recommended. Recommending HTTPS still allows other schemes like mqtts or mailto (See here).

The recommendation would be that TLS is used to protect notifications. MQTTS uses TLS, but it does not use HTTP.

Recommending TLS should be non-controversial, so you could raise a PR for that. If you want to mandate TLS, that would need more discussion.

TLS adds a lot of overhead for what would otherwise be lightweight notifications. Some applications may prefer to forgo the security when the data is not particularly sensitive. API providers can always reject unsecured sinks if they take a different view.

@tlohmar
Copy link
Contributor Author

tlohmar commented Feb 14, 2025

Recommending TLS should be non-controversial, so you could raise a PR for that. If you want to mandate TLS, that would need more discussion.

Well, usage of TLS can be left for deployments. Typically, using of HTTP can be ok for debugging/development and different framework throw warnings like "not to be used in production".

MQTTS uses TLS, but it does not use HTTP.

MQTTS can act as transport for the HTTP protocol. It can carry HTTP messages (see link).

HTTP as protocol (e.g. HTTP1.1 in RFC 9112) does not mandate a certain transport. The URL scheme provides information about the transport (like via http:// or file://). Should CAMARA give some transport recommendations, e.g. usage of HTTPS is recommended?

@rartych
Copy link
Collaborator

rartych commented Feb 14, 2025

The extension of Security Consideration section has been merged into chapter 12.2 of API Design Guidelines.
It addresses the points of @tlohmar :

Camara Notifications MUST use HTTPS. The value of sink MUST be an URL with url-scheme https. The implementation of the Notification Sender MUST follow 10.2 Security Implementation.

However this requirement is not propagated to particular API definitions , it might be necessary to modify the sink field definition that should be used in every subscription API specification.

The protocol field is defined by Cloudevents: https://github.com/cloudevents/spec/blob/main/subscriptions/spec.md#protocol and HTTP is fixed there (although it is still work in progress).

@sachinvodafone
Copy link
Collaborator

I believe , @eric-murray has provided a detailed insight about the differences and the implications of explicitly requiring HTTPS. However, refining the wording for better clarity could be beneficial. A possible revision could be: 'Only HTTP is used for notifications, with optional support for TLS (HTTPS recommended for security).'

@eric-murray
Copy link
Collaborator

The extension of Security Consideration section has been merged into chapter 12.2 of API Design Guidelines.

Thanks @rartych. I'd overlooked that PR, but can understand why it was approved. It will be interesting to see if any demand for HTTP only notifications arises. The wording will anyway have to be updated once other messaging protocols are allowed.

@tlohmar
Copy link
Contributor Author

tlohmar commented Feb 16, 2025

I created PR #418 to address the issue. The protocol definitions from CloudEvents are not really clear. However, since only HTTP is allowed for the present release, there is likely clear enough.

Btw, I would not expect a requirement in a "security considerations" section.

@PedroDiez
Copy link
Collaborator

Hi,

Just taking advantage of this.

Would be benefit to?:

  • Define an schema in CAMARA_common.yaml for the sink format (e.g.)
sink:
          type: string
          format: url
          pattern: ^https:\/\/.+$
          description: The address to which events shall be delivered using the selected protocol.

WDYT?

cc @tlohmar, @eric-murray, @rartych, @bigludo7, @sachinvodafone, @patrice-conil

@eric-murray
Copy link
Collaborator

@PedroDiez
That's reasonable for now. We just need to remember to update the pattern when other protocols are allowed.

@tlohmar
Copy link
Contributor Author

tlohmar commented Feb 17, 2025

Well, certainly good to get clear yaml specs.

Could even make sense to create a template CAMARA_common.yaml for the full Explicit Subscription, at least the data type definitions of the "Create subscription" properties in the table. But this goes beyond this Pull Request.

@rartych
Copy link
Collaborator

rartych commented Feb 17, 2025

@tlohmar We have the event-subscription-template.yaml for Explicit Subscriptions.

@tlohmar
Copy link
Contributor Author

tlohmar commented Feb 17, 2025

@tlohmar We have the event-subscription-template.yaml for Explicit Subscriptions.

Thx. hmm, this yaml allows additional protocols, e.g. MQTT, AMQP, etc. Could be clarified, that only HTTP is supported.

@bigludo7
Copy link
Collaborator

@tlohmar We have described this in the error example - see lines 849-854.

@PedroDiez
Copy link
Collaborator

As commented in 17/FEB meeting, I will generate a separate PR to address this comment.

It may be addressed as further clarification for the next metarelease

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

Successfully merging a pull request may close this issue.

6 participants