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

AGH accepts all XFF headers without restriction #2799

Closed
ZeroClover opened this issue Mar 10, 2021 · 4 comments
Closed

AGH accepts all XFF headers without restriction #2799

ZeroClover opened this issue Mar 10, 2021 · 4 comments
Assignees
Labels
enhancement external libs Issues that require changes in external libraries. P3: Medium
Milestone

Comments

@ZeroClover
Copy link

ZeroClover commented Mar 10, 2021

In #1220, AGH accepts some HTTP headers to get the visitor's raw IP.

However, the AGH does not restrict which IPs can send these headers, but accepts them from all IPs.

This means that when the AGH is deployed publicly and uses DoH, rate limiting may be completely useless. The AGH administrator will not be able to use the AGH's own functionality to block malicious users.

I constructed some malicious requests on my AGH server to illustrate the problem more clearly. Obviously, Cloudflare and Google could not have used my server as an upstream.

Screenshot: 1 2

I recommend that AGH add a separate configuration to allow users to set trusted IPs and trust the local loopback by default (127.0.0.1).

@ZeroClover ZeroClover changed the title ADG accepts all XFF headers without restriction AGH accepts all XFF headers without restriction Mar 10, 2021
@ainar-g ainar-g added this to the v0.107.0 milestone Mar 11, 2021
@ainar-g ainar-g self-assigned this Mar 11, 2021
@SukkaW
Copy link

SukkaW commented Mar 12, 2021

@ainar-g

I am afraid it is not a feature request. It should be considered as a security issue.

AGH should implement a restriction to prevent "faking IP".

@ameshkov
Copy link
Member

rate limiting may be completely useless

The thing is that rate-limiting currently only works for UDP, it is a measure for mitigating DNS amplification attacks, and other protocols aren't used for that.

This issue does make Access settings useless, though. It's labeled as "enhancement" (which is not a feature request) since it adds functionality (trusted IPs list).

@ainar-g ainar-g added the external libs Issues that require changes in external libraries. label Jun 3, 2021
@ainar-g ainar-g assigned EugeneOne1 and unassigned ainar-g Jun 28, 2021
adguard pushed a commit to AdguardTeam/dnsproxy that referenced this issue Jul 23, 2021
Merge in DNS/dnsproxy from imp-xff to master

Updates AdguardTeam/AdGuardHome#2799.

Squashed commit of the following:

commit aef68d4
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Jul 23 16:30:09 2021 +0300

    proxy: trim spaces

commit bf36249
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Jul 23 16:11:36 2021 +0300

    proxy: fix logic

commit f381fe2
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jul 22 19:56:25 2021 +0300

    all: upd go version to 1.16 in specs

commit 54b9fbd
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jul 22 19:52:59 2021 +0300

    all: upd go version to 1.16 for github

commit a2cd74d
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jul 22 19:47:10 2021 +0300

    proxy: imp code

commit a8ae14f
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jul 22 16:35:03 2021 +0300

    proxy: add trusted proxies
adguard pushed a commit that referenced this issue Jul 26, 2021
Merge in DNS/adguard-home from 2799-trusted-proxy to master

Updates #2799.

Squashed commit of the following:

commit 708a06b
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Jul 23 18:56:16 2021 +0300

    all: add trusted proxy mechanism
@EugeneOne1
Copy link
Member

@ZeroClover, there is a new build in the edge channel available. It implements the trusted proxies feature. It's also documented in an appropriate wiki section. Could you please check if it works for you?

@EugeneOne1
Copy link
Member

We'll close the issue for now. You're welcome to open the new issues in case of encounter those.

adguard pushed a commit to AdguardTeam/dnsproxy that referenced this issue Aug 2, 2021
Updates AdguardTeam/AdGuardHome#2799.

Squashed commit of the following:

commit 874cf42
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Aug 2 14:43:10 2021 +0300

    proxy: do not refuse reqs from untrusted proxies
adguard pushed a commit that referenced this issue Aug 2, 2021
Updates #2799.

Squashed commit of the following:

commit bc768fd
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Aug 2 15:00:10 2021 +0300

    all: do not refuse reqs from untrusted proxies
heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
Merge in DNS/adguard-home from 2799-trusted-proxy to master

Updates AdguardTeam#2799.

Squashed commit of the following:

commit 708a06b
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Fri Jul 23 18:56:16 2021 +0300

    all: add trusted proxy mechanism
heyxkhoa pushed a commit to heyxkhoa/AdGuardHome that referenced this issue Mar 20, 2023
Updates AdguardTeam#2799.

Squashed commit of the following:

commit bc768fd
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Aug 2 15:00:10 2021 +0300

    all: do not refuse reqs from untrusted proxies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement external libs Issues that require changes in external libraries. P3: Medium
Projects
None yet
Development

No branches or pull requests

5 participants