-
Notifications
You must be signed in to change notification settings - Fork 29
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
Comments
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 Maybe we should mandate that sink urls use |
hmm, this is a bit confusing now. The scheme identifier of the URL ( Thus, it might be better to remove the |
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 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". |
yes, clear rules simplify interoperability. I still dont fully understand the usage of the In the current release, the |
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.
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. |
Ok, thanks for the clarification. Would be good to make this a bit clearer.
At least, HTTPS usage should be recommended. Recommending
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? |
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
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. |
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 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 |
The extension of Security Consideration section has been merged into chapter 12.2 of API Design Guidelines.
However this requirement is not propagated to particular API definitions , it might be necessary to modify the The protocol field is defined by Cloudevents: https://github.com/cloudevents/spec/blob/main/subscriptions/spec.md#protocol and |
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).' |
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. |
I created PR #418 to address the issue. The protocol definitions from CloudEvents are not really clear. However, since only Btw, I would not expect a requirement in a "security considerations" section. |
Hi, Just taking advantage of this. Would be benefit to?:
WDYT? cc @tlohmar, @eric-murray, @rartych, @bigludo7, @sachinvodafone, @patrice-conil |
@PedroDiez |
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. |
@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. |
@tlohmar We have described this in the error example - see lines 849-854. |
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 |
Problem description
The API Design Guidelines mandates the usage of HTTP (instead of HTTPS) for notifications (Link to subscription attributes). Specifically the statement
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
The text was updated successfully, but these errors were encountered: