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

Permanence of streams #211

Open
tulshi opened this issue Oct 8, 2024 · 6 comments · May be fixed by #222
Open

Permanence of streams #211

tulshi opened this issue Oct 8, 2024 · 6 comments · May be fixed by #222

Comments

@tulshi
Copy link
Contributor

tulshi commented Oct 8, 2024

Copying discussion from meeting on 9/24:

  • [Tushar] the spec implicitly assumes that a stream is permanent once created. Should it support expirable streams?
  • [Tushar] Possible solution: verification events on a cadence
  • [Yair] A Rx (or Tx) can disable a stream
  • [Yair] As long as PUSH endpoint is returning success, nothing is wrong. But if endpoint starts sending 500, 400 etc, then Tx can do something about it.
  • [Yair] You cannot tell the difference between an Rx that has been sold, etc vs an Rx that is behaving well
  • [Yair] We shouldn't make verification event requests mandatory, but can make it optional. Possibly include a time value that indicates how long between requests before Tx kills stream.
  • [Shayne] I like the idea of an optional value in StreamConfiguration
  • [Apoorva] We could also make it any call, not just verification
  • [Shayne] Is that harder to implement?
  • [Yair] Agree that it could be any endpoint - status endpoint would work
  • [Shayne] That's nice because then you don't have to send unneccessary data
@tulshi tulshi added the spec:SSF label Oct 8, 2024
@tulshi
Copy link
Contributor Author

tulshi commented Oct 8, 2024

Action item recorded on 9/24:

  • [Tushar] Send PR to clarify that streams can optionally expire, and how to prevent that

@keylimesoda
Copy link

+1 on Yair's comments.

It can be expensive for a transmitter to continue to transmit indefinitely to defunct receivers. It is potentially insecure to allow streams to exist indefinitely.

An optional "time remaining" or "expiration timedate" property on a Stream Configuration communicates a finite lifecycle to the Receiver. The property could be updated by the Transmitter after successful verification event, or via PATCH request from receiver.

@TusharR-google
Copy link

TusharR-google commented Nov 13, 2024

I'd like to propose that this be added to v1Final instead of vFuture.

Reasoning
The main issue is security.

Permanent access to data (security events) for PUSH receivers is a potential security hole. We can contrast this with something like Oauth grants that are limited in both scope and duration.

The duration limit is missing, once the stream is opened by a PUSH Rx, the Tx is expected to continue sending events until the stream gets paused or deleted.

This also leads to asymmetry in the PUSH and PULL flows, where the PULL Rx is re-authed every time it pulls events, while the PUSH Rx may not communicate again for many years after creating the stream.

It would be great to have a way in the spec for Tx that are going to auto-expire streams (currently strongly preferred for the Google Tx implementation) to communicate this fact.

I'll send out a PR shortly.

Cost Estimate vs ID3
This only affects PUSH streams, and the Rx changes should be minor, considering that most affected Rx will already have an automated way of calling Tx methods.

Most Tx/Rx pairs will be ID3 backwards compatible. Some Tx will optionally communicate a stream expiry duration, in which case the Rx will periodically perform a stream operation or request a verification event to refresh the expiry.

@tulshi
Copy link
Contributor Author

tulshi commented Nov 15, 2024

I've added this to the agenda of the meeting on Nov 19, 2024.

@TusharR-google
Copy link

TusharR-google commented Nov 19, 2024

Thanks Atul! After thinking about this more, a similar concept will be useful for PULL streams too - except expiring the stream it's for expiring the events themselves, so that the Tx doesn't have to keep them for too long. (more an optimization instead of a security use-case)

@appsdesh
Copy link
Contributor

Thanks Atul! After thinking about this more, a similar concept will be useful for PULL streams too - except expiring the stream it's for expiring the events themselves, so that the Tx doesn't have to keep them for too long. (more an optimization instead of a security use-case)

FYI SSF spec discourages exp claim

@TusharR-google TusharR-google linked a pull request Dec 3, 2024 that will close this issue
@tulshi tulshi linked a pull request Dec 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants