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

Use encrytped data in example as recommended in Privacy and Security section #1290

Closed
wants to merge 4 commits into from

Conversation

AxelNennker
Copy link
Contributor

Fixes # #1288

The example should follow the recommendation of the Privacy and Security section and use encrypted data

Proposed Changes

  • change content-type in example as defined in JWS
  • change data value in example. Value taken from JWE

@duglin
Copy link
Collaborator

duglin commented May 30, 2024

@AxelNennker, if we decide to show an example with "data" encrypted, as you've shown, I'd prefer if it were a separate (secondary) example so that we can keep the simple example still there. I think seeing the existing one is better from a "learning CE" perspective.

@AxelNennker
Copy link
Contributor Author

Would you prefer the encrypted data at the end where this example is or nearer to the recommendation to encrypt data?

I was wondering about that example anyway because it shows several things like extendions that maybe should be in the place where extensions are specified.

@AxelNennker
Copy link
Contributor Author

I re-created the example and added one with encrypted data.

Signed-off-by: Axel Nennker <[email protected]>
@duglin
Copy link
Collaborator

duglin commented Jun 6, 2024

On last week's call we decided to wait for @clemensv to comment on this as he's had strong opinions on CE+Security in the past.

We also discussed whether it might be better to present this as an extension spec (even though there isn't any new CE attributes), to show how it someone might encrypt stuff using the existing CE attributes.

Also, @AxelNennker should this use the data_base64 attribute instead of data ?

@AxelNennker
Copy link
Contributor Author

On last week's call we decided to wait for @clemensv to comment on this as he's had strong opinions on CE+Security in the past.

We also discussed whether it might be better to present this as an extension spec (even though there isn't any new CE attributes), to show how it someone might encrypt stuff using the existing CE attributes.

Also, @AxelNennker should this use the data_base64 attribute instead of data ?

The primary value a receiver has to determine the handler for a notification is type. Everything can be defined into type.
E.g. whether the data is in data or in data_base64. This example exists to show a way how the recommendation in the privacy and security section to encrypt data could be followed by an implementation.

I think that "datacontenttype" is the best way to tell the receiver what the data is, so they can decide on a handler. But even that is optional if the type requires a fixed content-type.
I am strongly against using data_base64 in this encryption-example, because some people thing that base64 is encryption. I am not accusing you of having this misconception but I have seen that several time.

Regarding "extension spec", could be done but I think that's a different topic. This PR just introduces an example not a recommendation for a specific way to follow the encryption-recommendation.

I view CloudEvents as a framework that defines some parts but leaves how to do other parts of the event-system application-dependent.

For example CloudEvents does not specify the channel over which notifications are send, which means that CE cannot recommend e.g. TLS because that is too specific. That, CE-over-http, should be an extension, which then might even REQUIRE TLS and have all kinds of more specific security and privacy guidelines and requirements.

You could compare CE to OAuth2 on the being-a-framework level. OAuth2 does not define how an access token look.
OAuth2 implementations can do whatever they want and need regarding access tokens.
Likewise, CE implementation can use whatever they need regarding the content-type of data and its encryption.
Regarding OAuth2, after some years people noticed that a pattern for access tokens emerged and so RFC 9068 "JSON Web Token (JWT) Profile for OAuth 2.0 Access Tokens" was proposed by Vittorio Bertocci standardized by IETF. So, I think there could be an CE extension that recommends using JWE for encrypting data.

But this PR is just an example of one way of following the CE security guideline.

"id" : "a978702e-ef48-4032-ac18-a057e0104076",
"time" : "2024-05-30T17:31:00Z",
"datacontenttype" : "application/jose",
"data" : "eyJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkEyNTZHQ00ifQ.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be "data_base64"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because the contenttype defines what data is.
I think data_base64 is a short hand for contenttype base64 and a data field.
The contenttype in this example "application/jose" is correct.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, data_base64 doesn't mean that the content-type is base64, rather, it just means that the data serialization within this CE uses base64 rather than raw JSON. For example, it's possible to have content-type be application/json but then use data_base64 to serialize your json even if the json is as simple as {} (e.g. e30K)

"subject" : "c7bbb040-d458-4d47-82a8-45413f9f2d33",
"id" : "a978702e-ef48-4032-ac18-a057e0104076",
"time" : "2024-05-30T17:31:00Z",
"datacontenttype" : "application/jose",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clemensv
Copy link
Contributor

What makes this "encrypted" is just the "datacontenttype" reference that points to JOSE. I am okay with having this as an example for "JSON encoding with binary content encoded in base64", because we don't show that elsewhere.

For labeling that as an "encryption" scenario, there is not enough context. JOSE is doing 100% of the work here and CloudEvents none other than providing a slot for the content type indicator, so I am not comfortable with somehow implying that CloudEvents had some inherent security features, which it does not, and does not have very intentionally (also because JOSE and S/MIME exist).

I am generally unhappy with the singular example in the main spec because it has caused a JSON bias with some readers. If we add another JSON example here, I would like to see this being extended into an "Examples" section that shows a handful examples in structured mode in JSON but also with a literal hex dump of a Proto or Avro message (or an XML message, even though that is still working draft) and an example for HTTP binary as well as for one of the other protocols.

@AxelNennker
Copy link
Contributor Author

@clemensv I added xmlenc as another example. I do not have experience with other protocols, Proto, Avro or others, which do any encryption or signing of data in their own domain-specific way.
The example is something-something-payment-event where the protocol is usually HTTP based with xml or json inside.

The usecase I have is the Linux Foundation's Camara project where we are currently specifying CloudEvents into our subscription/notification requirements.
https://github.com/camaraproject/Commonalities/blob/main/documentation/API-design-guidelines.md#12-subscription-notification--event

duglin pushed a commit to duglin/spec that referenced this pull request Jul 16, 2024
duglin pushed a commit to duglin/spec that referenced this pull request Jul 16, 2024
duglin pushed a commit to duglin/spec that referenced this pull request Jul 16, 2024
duglin pushed a commit to duglin/spec that referenced this pull request Jul 18, 2024
@duglin
Copy link
Collaborator

duglin commented Jul 19, 2024

@AxelNennker please see #1305 as an alternative that addresses some of the concerns the group had with your proposal.

@duglin duglin closed this in #1305 Jul 25, 2024
@duglin duglin closed this in 6ed4369 Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants