Page MenuHomeSoftware Heritage
Differential D6914

api/graph: Implement anti-DoS policies for graph visits
ClosedPublic
Actions

Authored by anlambert on Jan 11 2022, 2:32 PM.

Details

Reviewers
seirl
Group Reviewers
Reviewers
Maniphest Tasks
T3836: Define and implement an anti-DoS policy for graph visits using the max_edges parameter
Commits
rDWAPPSe870ec2b9a40: api/graph: Implement anti-DoS policies for graph visits
Summary

In order to allow public access to the graph API, we need to prevent
users to DoS the graph service with resource intensive queries.

Fortunately, the graph API supports a max_edges parameter to limit the
number of edges traversed by the graph.

According to the type of user querying the graph service through the
Web API proxy, a maximum value for the max_edges will be set before
sending a request to the graph API.

Three type of users are considered: anonymous, standard and staff.
The maximum values for the max_edges query parameter according to
user types are retrieved from the webapp configuration.

Related to T3836

Depends on D6919

Diff Detail

Repository
rDWAPPS Web applications
Branch
api-graph-anti-dos-policies
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25928
Build 40523: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 40522: arc lint + arc unit

Event Timeline

anlambert created this revision.Jan 11 2022, 2:32 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJan 11 2022, 2:32 PM
swh-public-ci added a comment.Jan 11 2022, 2:47 PM

Build is green

Patch application report for D6914 (id=25057)

Rebasing onto e0a181daf4...

Current branch diff-target is up to date.
Changes applied before test
commit 86675f4b3c3349ca4671291fdcd038a6d5734fec
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jan 11 14:25:08 2022 +0100

    api/graph: Implement anti-DoS policies for graph visits
    
    In order to allow public access to the graph API, we need to prevent
    users to DoS the graph service with resource intensive queries.
    
    Fortunately, the graph API supports a max_edges parameter to limit the
    number of edges traversed by the graph.
    
    According to the type of user querying the graph service through the
    Web API proxy, a maximum value for the max_edges will be set before
    sending a request to the graph API.
    
    Three type of users are considered: anonymous, standard and staff.
    The maximum values for the max_edges query parameter according to
    user types are retrieved from the webapp configuration.
    
    Related to T3836

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1284/ for more details.

Harbormaster completed remote builds in B25928: Diff 25057.Jan 11 2022, 2:47 PM
anlambert requested review of this revision.Jan 11 2022, 2:47 PM
seirl requested changes to this revision.Jan 11 2022, 3:15 PM
seirl added a subscriber: seirl.

I have just one security consideration, otherwise LGTM. Thanks!

swh/web/api/views/graph.py
157

I think this is potentially dangerous if an endpoint gets passed with a query parameter.

If someone passes something like graph_query == "/graph/visit/nodes?prod=true" to the function, the resulting URL will look like https://graph.internal.softwareheritage.org/graph/visit/nodes?prod=true?max_edges=42 and the max_edges parameter will get silently ignored.

Instead, we could use something from urllib like this: https://stackoverflow.com/a/2506477 to ensure that the argument is properly added to the query (with a & if there is already a ?, or with a ? otherwise).

This revision now requires changes to proceed.Jan 11 2022, 3:15 PM
anlambert added inline comments.Jan 11 2022, 4:03 PM
swh/web/api/views/graph.py
157

This should not happen as the Web API URL will be parsed by django so graph_query will not contain any ? character and the query parameters will be available in request.GET.

Nevertheless, we can have such issue if the ? got quoted to %3F in the Web API URL. I will handle that case in another diff.

I also found out that I did not handle really well the error responses coming from the graph API, I will also handle that in a new diff.

anlambert added a comment.Jan 11 2022, 5:48 PM

Nevertheless, we can have such issue if the ? got quoted to %3F in the Web API URL. I will handle that case in another diff.

D6919

I also found out that I did not handle really well the error responses coming from the graph API, I will also handle that in a new diff.

D6918

I will rebase that diff on top of those once they are landed.

anlambert updated this revision to Diff 25072.Jan 12 2022, 11:06 AM

Rebase

anlambert edited the summary of this revision. (Show Details)Jan 12 2022, 11:06 AM
anlambert added a parent revision: D6919: api/graph: Handle query parameters that might be passed in graph_query.
swh-public-ci added a comment.Jan 12 2022, 11:07 AM

Build was aborted

Patch application report for D6914 (id=25072)

Could not rebase; Attempt merge onto 15e6e1988a...

Updating 15e6e198..a0d3ab36
Fast-forward
 swh/web/api/views/graph.py            |  32 +++++++--
 swh/web/config.py                     |   5 +-
 swh/web/tests/api/views/test_graph.py | 130 +++++++++++++++++++++++++++++++++-
 3 files changed, 160 insertions(+), 7 deletions(-)
Changes applied before test
commit a0d3ab367464a55e946a4adaff5e3de388eb6011
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jan 11 14:25:08 2022 +0100

    api/graph: Implement anti-DoS policies for graph visits
    
    In order to allow public access to the graph API, we need to prevent
    users to DoS the graph service with resource intensive queries.
    
    Fortunately, the graph API supports a max_edges parameter to limit the
    number of edges traversed by the graph.
    
    According to the type of user querying the graph service through the
    Web API proxy, a maximum value for the max_edges will be set before
    sending a request to the graph API.
    
    Three type of users are considered: anonymous, standard and staff.
    The maximum values for the max_edges query parameter according to
    user types are retrieved from the webapp configuration.
    
    Related to T3836

commit 43525e69eaeb058b5d41c7031c422e7de088af16
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jan 11 17:40:34 2022 +0100

    api/graph: Handle query parameters that might be passed in graph_query
    
    It is possible to pass query parameters in the graph_query URL argument
    of the Web API graph endpoint if the ? character is quoted.
    
    So add extra processing in the endpoint implementation to merge the
    query parameters extracted from the graph_query value and those
    coming from the django request.
    
    The purpose is to avoid some query parameters to be silently ignored
    when requesting the graph API.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1290/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1290/console

Harbormaster failed remote builds in B25947: Diff 25072!Jan 12 2022, 11:07 AM
swh-public-ci added a comment.Jan 12 2022, 11:23 AM

Build has FAILED

Patch application report for D6914 (id=25072)

Could not rebase; Attempt merge onto 15e6e1988a...

Updating 15e6e198..a0d3ab36
Fast-forward
 swh/web/api/views/graph.py            |  32 +++++++--
 swh/web/config.py                     |   5 +-
 swh/web/tests/api/views/test_graph.py | 130 +++++++++++++++++++++++++++++++++-
 3 files changed, 160 insertions(+), 7 deletions(-)
Changes applied before test
commit a0d3ab367464a55e946a4adaff5e3de388eb6011
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jan 11 14:25:08 2022 +0100

    api/graph: Implement anti-DoS policies for graph visits
    
    In order to allow public access to the graph API, we need to prevent
    users to DoS the graph service with resource intensive queries.
    
    Fortunately, the graph API supports a max_edges parameter to limit the
    number of edges traversed by the graph.
    
    According to the type of user querying the graph service through the
    Web API proxy, a maximum value for the max_edges will be set before
    sending a request to the graph API.
    
    Three type of users are considered: anonymous, standard and staff.
    The maximum values for the max_edges query parameter according to
    user types are retrieved from the webapp configuration.
    
    Related to T3836

commit 43525e69eaeb058b5d41c7031c422e7de088af16
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jan 11 17:40:34 2022 +0100

    api/graph: Handle query parameters that might be passed in graph_query
    
    It is possible to pass query parameters in the graph_query URL argument
    of the Web API graph endpoint if the ? character is quoted.
    
    So add extra processing in the endpoint implementation to merge the
    query parameters extracted from the graph_query value and those
    coming from the django request.
    
    The purpose is to avoid some query parameters to be silently ignored
    when requesting the graph API.

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1291/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1291/console

swh-public-ci added a comment.Jan 12 2022, 11:41 AM

Build is green

Patch application report for D6914 (id=25072)

Could not rebase; Attempt merge onto 15e6e1988a...

Updating 15e6e198..a0d3ab36
Fast-forward
 swh/web/api/views/graph.py            |  32 +++++++--
 swh/web/config.py                     |   5 +-
 swh/web/tests/api/views/test_graph.py | 130 +++++++++++++++++++++++++++++++++-
 3 files changed, 160 insertions(+), 7 deletions(-)
Changes applied before test
commit a0d3ab367464a55e946a4adaff5e3de388eb6011
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jan 11 14:25:08 2022 +0100

    api/graph: Implement anti-DoS policies for graph visits
    
    In order to allow public access to the graph API, we need to prevent
    users to DoS the graph service with resource intensive queries.
    
    Fortunately, the graph API supports a max_edges parameter to limit the
    number of edges traversed by the graph.
    
    According to the type of user querying the graph service through the
    Web API proxy, a maximum value for the max_edges will be set before
    sending a request to the graph API.
    
    Three type of users are considered: anonymous, standard and staff.
    The maximum values for the max_edges query parameter according to
    user types are retrieved from the webapp configuration.
    
    Related to T3836

commit 43525e69eaeb058b5d41c7031c422e7de088af16
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jan 11 17:40:34 2022 +0100

    api/graph: Handle query parameters that might be passed in graph_query
    
    It is possible to pass query parameters in the graph_query URL argument
    of the Web API graph endpoint if the ? character is quoted.
    
    So add extra processing in the endpoint implementation to merge the
    query parameters extracted from the graph_query value and those
    coming from the django request.
    
    The purpose is to avoid some query parameters to be silently ignored
    when requesting the graph API.

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1292/ for more details.

Harbormaster completed remote builds in B25947: Diff 25072.Jan 12 2022, 11:41 AM
seirl accepted this revision.Jan 12 2022, 2:08 PM
This revision is now accepted and ready to land.Jan 12 2022, 2:08 PM
anlambert updated this revision to Diff 25092.Jan 12 2022, 2:38 PM

Rebase

swh-public-ci added a comment.Jan 12 2022, 2:55 PM

Build is green

Patch application report for D6914 (id=25092)

Could not rebase; Attempt merge onto 15e6e1988a...

Updating 15e6e198..e870ec2b
Fast-forward
 swh/web/api/views/graph.py            |  32 +++++++--
 swh/web/config.py                     |   5 +-
 swh/web/tests/api/views/test_graph.py | 130 +++++++++++++++++++++++++++++++++-
 3 files changed, 160 insertions(+), 7 deletions(-)
Changes applied before test
commit e870ec2b9a40f9f61d2e50407b1795473487329a
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jan 11 14:25:08 2022 +0100

    api/graph: Implement anti-DoS policies for graph visits
    
    In order to allow public access to the graph API, we need to prevent
    users to DoS the graph service with resource intensive queries.
    
    Fortunately, the graph API supports a max_edges parameter to limit the
    number of edges traversed by the graph.
    
    According to the type of user querying the graph service through the
    Web API proxy, a maximum value for the max_edges will be set before
    sending a request to the graph API.
    
    Three type of users are considered: anonymous, standard and staff.
    The maximum values for the max_edges query parameter according to
    user types are retrieved from the webapp configuration.
    
    Related to T3836

commit 9e7732ccebe33b7f4f37ad92a13998136580b2c9
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jan 11 17:40:34 2022 +0100

    api/graph: Handle query parameters that might be passed in graph_query
    
    It is possible to pass query parameters in the graph_query URL argument
    of the Web API graph endpoint if the ? character is quoted.
    
    So add extra processing in the endpoint implementation to merge the
    query parameters extracted from the graph_query value and those
    coming from the django request.
    
    The purpose is to avoid some query parameters to be silently ignored
    when requesting the graph API.

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1295/ for more details.

Harbormaster completed remote builds in B25966: Diff 25092.Jan 12 2022, 2:55 PM
Closed by commit rDWAPPSe870ec2b9a40: api/graph: Implement anti-DoS policies for graph visits (authored by anlambert). · Explain WhyJan 12 2022, 3:00 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDWAPPSe870ec2b9a40: api/graph: Implement anti-DoS policies for graph visits.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
web/
api/
views/
22 lines96%
Loading...
5 lines95%
Loading...
tests/
api/
views/
83 lines100%
Loading...

Diff 25057

View Options

swh/web/api/views/graph.py

Loading...
View Options

swh/web/config.py

Loading...
View Options

swh/web/tests/api/views/test_graph.py

Loading...
About · Developer information · Legal