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

Multiple subscriptions could cause resource exhaustion #272

Closed
ianswett opened this issue Oct 10, 2023 · 10 comments · Fixed by #491
Closed

Multiple subscriptions could cause resource exhaustion #272

ianswett opened this issue Oct 10, 2023 · 10 comments · Fixed by #491
Assignees
Labels
Needs PR Security/DoS Issues with MoQ's security or ways it may be vulnerable to denial of service / resource exhaustion Subscribe Related to SUBSCRIBE message and subscription handling

Comments

@ianswett
Copy link
Collaborator

Currently, there's no way to limit the number of simultaneous subscriptions. QUIC stream limits cannot effectively be used to limit the number of subscriptions, so I'd suggest MoQ have a limit.

The simplest idea would be to have a limit specified as a parameter in the server's SETUP message.

As a related question, even with limits, what happens if a client subscribes to a number of streams that it doesn't have the bandwidth to support? ie: 10 4k video streams and they're on a slow link? Possibly the subscriptions could fail with an error code indicating that the server didn't have enough bandwidth to request them and/or serve them to the client?

@ianswett ianswett added the Subscribe Related to SUBSCRIBE message and subscription handling label Oct 10, 2023
@ianswett ianswett changed the title Subscriptions could cause resource exhaustion Multiple subscriptions could cause resource exhaustion Oct 10, 2023
@kixelated
Copy link
Collaborator

rofl I just came here to post the same issue. See: https://cloud.google.com/blog/products/identity-security/how-it-works-the-novel-http2-rapid-reset-ddos-attack

I think we follow QUIC and have a MAX_SUBSCRIBES frame. The initial value can be set in the SETUP message. The subscribe is considered active until the publisher sends SUBSCRIBE_ERROR or SUBSCRIBE_RESET (TODO).

I think we're okay with the subscriber/publisher using arbitrary subscription IDs if there's a single control channel. It would be easier if we required incremental subscribe IDs and you couldn't reuse them though.

@kixelated
Copy link
Collaborator

kixelated commented Oct 10, 2023

Also do we need MAX_ANNOUNCES? That seems like a potential attack vector too, since it involves inserting into a potentially CDN-wide routing table.

@ianswett
Copy link
Collaborator Author

A few months ago, I would have said we should go with the HTTP/2 style design of just having a limit on number of streams, but given the recent issues, I'd be inclined towards the QUIC style of specifying the max number. But I think that works better if we also have a subscription ID that's monotonically increasing for subscriptions, ie: #282

Announces also likely need some limits, but I'm less clear on how that might work.

@kixelated
Copy link
Collaborator

I propose a monotonically increasing Subscribe ID in the SUBSCRIBE message. You can't reuse Subscribe IDs. We add a MAX_SUBSCRIBES message containing the maximum allowed ID and possibly a SETUP parameter to set the initial value.

The corresponding SUBSCRIBE_OK, SUBSCRIBE_ERROR, SUBSCRIBE_RESET, SUBSCRIBE_FIN, UNSUBSCRIBE messages use that ID instead of echoing the full track name. This removes ambiguity and overhead.

We would still have Track ID for compressing the full track name (maybe renamed?). You could have multiple SUBSCRIBE reference the same Track ID but contain different Subscribe IDs. For example:

<-- MAX_SUBSCRIBES max=2
--> SUBSCRIBE id=0 track_id=2314 track_name=foobar start_group=-1
--> SUBSCRIBE id=1 track_id=2314 track_name=foobar start_group=653
<-- SUBSCRIBE_OK id=0
<-- SUBSCRIBE_ERROR id=1 code=404
<-- OBJECT track_id=2314 group=673
<-- MAX_SUBSCRIBES max=3
--> SUBSCRIBE id=2 track_id=2314 track_name=foobar start=670
...

Same for s/SUBSCRIBE/ANNOUNCE.

@ianswett
Copy link
Collaborator Author

Adding a Subscribe ID used as you're describing SGTM and as you said remove any ambiguity about what subscription is being referenced. Unless we are absolutely never going to have multiple subscriptions, I believe we'll need a Subscribe ID.

@ianswett
Copy link
Collaborator Author

This is related to #282

@suhasHere
Copy link
Collaborator

may be better to name it as transaction_id . since it is id tied to a given subscribe transaction ..

@ianswett
Copy link
Collaborator Author

ianswett commented Nov 9, 2023

As a follow-up to adding a Subscribe ID, we'll add a SETUP field or param to indicate the initial value and a MAX_SUBSCRIBE_ID frame to increase the limit, similar to QUIC flow control.

@ianswett ianswett added the Security/DoS Issues with MoQ's security or ways it may be vulnerable to denial of service / resource exhaustion label Jul 25, 2024
@ianswett
Copy link
Collaborator Author

The consensus at the session today was to go with a QUIC style approach, so I wrote #491

There was interest in a similar approach for announces, but announces don't yet have an ID, so that will have to wait.

@martinduke martinduke self-assigned this Aug 5, 2024
@martinduke
Copy link
Contributor

Will assign to Mike English

@martinduke martinduke assigned englishm and unassigned martinduke Aug 5, 2024
@afrind afrind moved this to Needs PR in MoQT Tracking Board Aug 15, 2024
@afrind afrind moved this from Needs PR to Needs Review in MoQT Tracking Board Aug 15, 2024
ianswett added a commit that referenced this issue Aug 26, 2024
Fixes #272

Some diffs are due to me reflowing text to fit the line limits, so
please ignore those.

Once we have this and a limit on announce IDs, it seems likely that
we'll be able to get rid of 'Role', but for now it can stay.
@github-project-automation github-project-automation bot moved this from Needs Review to Done in MoQT Tracking Board Aug 26, 2024
@github-project-automation github-project-automation bot moved this from Needs Review to Done in MoQT Tracking Board Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs PR Security/DoS Issues with MoQ's security or ways it may be vulnerable to denial of service / resource exhaustion Subscribe Related to SUBSCRIBE message and subscription handling
Projects
Development

Successfully merging a pull request may close this issue.

5 participants