rfc43: add constraint DoS limits#409
Merged
mergify[bot] merged 1 commit intoflux-framework:masterfrom Feb 2, 2024
Merged
Conversation
chu11
force-pushed
the
rfc43_constraint_limit
branch
from
ab59b9f to
f796a0d
Compare
chu11
force-pushed
the
rfc43_constraint_limit
branch
from
f796a0d to
9734d88
Compare
Member
Author
|
accidentally closed, re-opened and re-pushed with updated description of "comparison limits", per PR flux-framework/flux-core#5681 |
chu11
force-pushed
the
rfc43_constraint_limit
branch
from
fd98ae8 to
97ff59c
Compare
grondo
approved these changes
Feb 2, 2024
Contributor
grondo
left a comment
grondo
left a comment
There was a problem hiding this comment.
This seems good to me. Just a few wording suggestions inline.
spec_43.rst
Outdated
|
|
||
| { "and": [ { "userid": [ 42 ] }, { "t_submit": [ ">946713600.0" ] } ] } | ||
|
|
||
| In order to limit the potential for a constraint to cause a denial of service (DoS) or long job list service hang, the instance owner can configure a maximum number of *comparisons* a constraint can consume while filtering jobs. Every "check" against a job is considered a comparison. In the last example above, the constraint is looking for all jobs belonging to userid 42 and submitted after January 1, 2000. It will consume at most 2 *comparisons* for each job. The ``userid`` check will always consume 1 comparison and the submission time will consume a comparison if the ``userid`` check passes. |
Contributor
There was a problem hiding this comment.
Suggestion: reword this description to be in "RFC speak"? E.g. "The instance owner MAY configure...". "Every check SHALL be considered one comparison"...
Contributor
There was a problem hiding this comment.
Actually I wonder if "The instance owner MAY" could be dropped and we can just say "a comparison limit for constraints MAY be configured"
spec_43.rst
Outdated
|
|
||
| In order to limit the potential for a constraint to cause a denial of service (DoS) or long job list service hang, the instance owner can configure a maximum number of *comparisons* a constraint can consume while filtering jobs. Every "check" against a job is considered a comparison. In the last example above, the constraint is looking for all jobs belonging to userid 42 and submitted after January 1, 2000. It will consume at most 2 *comparisons* for each job. The ``userid`` check will always consume 1 comparison and the submission time will consume a comparison if the ``userid`` check passes. | ||
|
|
||
| After the maximum number of *comparisons* is consumed, an error is returned to the caller. The caller can decrease their search footprint by limiting their search using other inputs in the job list request or making tighter constraints. For example, take following two constraints: |
Contributor
There was a problem hiding this comment.
Same comment as above, "an error SHALL be returned to the caller. The caller MAY decrease ..."
spec_43.rst
Outdated
|
|
||
| { "and": [ { "userid": [ 42 ] }, { "queue": [ "foobar" ] } ] } | ||
|
|
||
| In these examples the caller wants to filter jobs submitted to the queue foobar and submitted by userid 42. The only difference is the order of the checks. If "foobar" is the most common queue in the system (i.e. the check for queue "foobar" typically succeeds) and ``userid`` is not the most common user in the system (i.e. the check for userid "42" typically fails), the latter constraint will consumes fewer comparisons. |
Contributor
There was a problem hiding this comment.
s/consumes/consume/
Actually just removing "will" would work too and be more succinct.
Problem: A caller has the ability to specify an unbounded sized constraint object, which could serve as a denial of service (DoS) attack on the job list service. Add description of comparison limits for constraints. By limiting total number of "checks", we can bound constraints and their ability to DoS the job list service.
chu11
force-pushed
the
rfc43_constraint_limit
branch
from
97ff59c to
4710ac0
Compare
Member
Author
|
@grondo Thanks. Re-pushed with some tweaks per comments above. |
Contributor
|
Already approved so feel free to set MWP! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem: A caller has the ability to specify an unbounded sized constraint object, which could serve as a denial of service (DoS) attack on the job list service.
Specify hard constraint limits on the size of operator arrays and constraint depth.
initial proposal based on flux-framework/flux-core#5681