search

w3c / baggage

Propagation format for distributed context: Baggage
https://w3c.github.io/baggage/
Other
46 stars 18 forks source link

Add note that applications using baggage must be aware that data can be propagated to other systems #111

Closed kalyanaj closed 1 year ago

kalyanaj commented 1 year ago

Add note that applications using baggage must be aware that data can be propagated to other systems. Resolves #107.

Preview | Diff

yurishkuro commented 1 year ago

lgtm, but can we get opinions from people who raised #107? I am not sure if it was @pes10k or that was posted on behalf of others.

dyladan commented 1 year ago

lgtm, but can we get opinions from people who raised #107? I am not sure if it was @pes10k or that was posted on behalf of others.

@pes10k was just relaying the results of the PING review I believe.

kalyanaj commented 1 year ago

@pes10k, when you get a chance, can you please review this change, and let me know if it addresses your feedback from the review? Thanks!

pes10k commented 1 year ago

This statement seems like a signifangant reduction in the intended privacy protections / guarantees. Is this intentional?

kalyanaj commented 1 year ago

This statement seems like a signifangant reduction in the intended privacy protections / guarantees. Is this intentional?

Thanks @pes10k for taking a look. Sorry I missed providing more context earlier. Yes, this is an intentional change that our working group came up with when we were discussing the issue filed by you.

We felt that the original statement "Systems MUST ensure that the baggage header does not leak beyond defined trust boundaries and they MUST ensure that the channel that is used to transport potentially user-identifiable data is secured." is extremely hard to implement: a system (say an SDK such as OpenTelemetry SDK) that an application depends on for this does not have any context on the trust boundary of the application that is using it.

Hence, with this PR, we want to set the right expectations for applications that once they add something to baggage, those keys and values can be propagated to other systems and hence applications must make sure that they scrub any private information from baggage that they don't want to be propagated to other systems.

kalyanaj commented 1 year ago

@pes10k Did you have a chance to look at my earlier response? Please let me know if you have any objections to merging this change.

This statement seems like a signifangant reduction in the intended privacy protections / guarantees. Is this intentional?

Thanks @pes10k for taking a look. Sorry I missed providing more context earlier. Yes, this is an intentional change that our working group came up with when we were discussing the issue filed by you.

We felt that the original statement "Systems MUST ensure that the baggage header does not leak beyond defined trust boundaries and they MUST ensure that the channel that is used to transport potentially user-identifiable data is secured." is extremely hard to implement: a system (say an SDK such as OpenTelemetry SDK) that an application depends on for this does not have any context on the trust boundary of the application that is using it.

Hence, with this PR, we want to set the right expectations for applications that once they add something to baggage, those keys and values can be propagated to other systems and hence applications must make sure that they scrub any private information from baggage that they don't want to be propagated to other systems.

pes10k commented 1 year ago

I may be getting threads confused, but is it correct then that baggage is not intended / defined for implementation in browsers? Or is that only trace context?

kalyanaj commented 1 year ago

I may be getting threads confused, but is it correct then that baggage is not intended / defined for implementation in browsers? Or is that only trace context?

Both baggage and trace context are not intended / targeted for implementation by browsers / user-agents.

pes10k commented 1 year ago

I see, then i think

  1. saying so explicitly in the spec and
  2. (as discussed in the other issue) removing referneces to browsers and browser-like applications from the spec

would address the concern

kalyanaj commented 1 year ago

I see, then i think

  1. saying so explicitly in the spec and
  2. (as discussed in the other issue) removing referneces to browsers and browser-like applications from the spec

would address the concern

Thanks @pes10k , I have updated the PR to reflect the above feedback. I have also sent two other related PRs:

pes10k commented 1 year ago

I see, thank you @kalyanaj I think this change addresses the concern. Thank you and your group for your hard work!