Bug 27917 - protect against federation loops
Summary: protect against federation loops
Status: RESOLVED FIXED
Alias: None
Product: elfutils
Classification: Unclassified
Component: debuginfod (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Di Chen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-26 17:48 UTC by Frank Ch. Eigler
Modified: 2021-08-27 17:43 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
Submit a Patch for Bug 27917 (2.85 KB, patch)
2021-08-26 03:04 UTC, Di Chen
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Ch. Eigler 2021-05-26 17:48:42 UTC
If someone misconfigures a debuginfod federation to have loops, and a nonexistent buildid lookup is attempted, bad things will happen, as is documented.  Let's reduce the risk by adding an option to debuginfod that functions kind of like an IP packet's TTL: a limit on the length of XFF: header that debuginfod is willing to process.

The simplest thing could be a comma (= hop) limit: "if X-Forwarded-For: exceeds N hops, do not delegate a local lookup miss to upstream debuginfods".  The default could be reasonably high, say 8.  Then recursion is guaranteed to terminate.

Note that it wouldn't be right to simply reject requests with one's own IP address (which a server doesn't really easily know anyway), because there could be multiple servers running on any given host; or network-local RFC1918 addresses could legitimately recur.
Comment 1 Aaron Merey 2021-06-25 18:59:21 UTC
Commit ab38d167c40c99 causes federation loops for non-existent resources to result in multiple temporary livelocks, each lasting for $DEBUGINFOD_TIMEOUT seconds. Since concurrent requests for each unique resource are now serialized, federation loops can result in one server thread waiting to acquire a lock while the server thread holding the lock waits for the first thread to respond to an http request.

An implementation for this PR should help protect against this behaviour. Ex. if --forwarded-ttl-limit=0 then the timeout behaviour of local loops should be avoided.
Comment 2 Di Chen 2021-08-26 03:04:24 UTC
Created attachment 13623 [details]
Submit a Patch for Bug 27917
Comment 3 Mark Wielaard 2021-08-27 17:43:22 UTC
commit d3f914023abcd6ae76b168da97518e5e7dbd761a
Author: Di Chen <dichen@redhat.com>
Date:   Fri Aug 20 13:03:21 2021 +0800

    debuginfod: PR27917 - protect against federation loops
    
    If someone misconfigures a debuginfod federation to have loops, and
    a nonexistent buildid lookup is attempted, bad things will happen,
    as is documented.
    
    This patch aims to reduce the risk by adding an option to debuginfod
    that functions kind of like an IP packet's TTL: a limit on the length of
    XFF: header that debuginfod is willing to process. If X-Forwarded-For:
    exceeds N hops, it will not delegate a local lookup miss to upstream
    debuginfods.
    
    Commit ab38d167c40c99 causes federation loops for non-existent resources
    to result in multiple temporary deadlocks, each lasting for
    $DEBUGINFOD_TIMEOUT seconds. Since concurrent requests for each unique
    resource are now serialized, federation loops can result in one server
    thread waiting to acquire a lock while the server thread holding the
    lock waits for the first thread to respond to an http request.
    
    This PR can help protect against the above multiple temporary deadlocks
    behaviour. Ex. if --forwarded-ttl-limit=0 then the timeout behaviour of
    local loops should be avoided.
    
    https://sourceware.org/bugzilla/show_bug.cgi?id=27917
    
    Signed-off-by: Di Chen <dichen@redhat.com>