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

Fix broken admin ui access #1845

Merged
merged 7 commits into from
Jan 13, 2020
Merged

Fix broken admin ui access #1845

merged 7 commits into from
Jan 13, 2020

Conversation

grantrburgess
Copy link
Contributor

Adds the fix for spoofing the Admin UI, includes some minor changes around how 404 error responses are accessed and includes tests.

Copy link
Collaborator

@jcrew99 jcrew99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@jcrew99 jcrew99 merged commit 5de127a into master Jan 13, 2020
@bcoles
Copy link
Collaborator

bcoles commented Jan 13, 2020

The issue is related to access controls. These comments don't match the issue:

        # NOTE: Allowing the reverse proxy will enable a vulnerability where the ui/panel can be spoofed
        #   by altering the X-FORWARDED-FOR ip address in the request header.

Also, the Configuration wiki page should be updated to reflect the changes introduced with beef.http.allow_reverse_proxy.

jcrew99 pushed a commit that referenced this pull request Jan 13, 2020
…_access"

This reverts commit 5de127a, reversing
changes made to f5de5eb.
jcrew99 pushed a commit that referenced this pull request Jan 13, 2020
…admin_ui_access""

This reverts commit 6a8c8d7.

Some random outcomes causes it to break
@jcrew99
Copy link
Collaborator

jcrew99 commented Jan 13, 2020

@bcoles I believe that comment relates to "fix for spoofing the Admin UI", which should be this issue.#1354
It is to stop IP Spoofing via HTTP Headers to make the server believe you are coming from another IP, thus bypassing the IP whitelisting rules.

@bcoles
Copy link
Collaborator

bcoles commented Jan 13, 2020

@bcoles I believe that comment relates to "fix for spoofing the Admin UI", which should be this issue.#1354
It is to stop IP Spoofing via HTTP Headers to make the server believe you are coming from another IP, thus bypassing the IP whitelisting rules.

Right, but that's not what the comment says. where the ui/panel can be spoofed by altering the X-FORWARDED-FOR ip address in the request header sounds like the admin panel can be spoofed... somehow. Sounds more like some form of injection issue, and doesn't describe the issue, which is that of access control bypass.

@grantrburgess
Copy link
Contributor Author

@bcoles access to the admin UI is restricted to a particular subnet through beef.restrictions.permitted_ui_subnet config option. The bug was you could bypass this by changing the X-FORWARDED-FOR value in the header, allowing anyone access to the Admin UI login page. Using this IP address for non-reverse proxy applications isn't necessary which is why we've added the beef.http.allow_reverse_proxy config option with a warning. I believe this is a temporary fix until we can separate the Admin UI from the server. I believe that should resolve #1354 but if i'm mistaken please let me know.

@grantrburgess
Copy link
Contributor Author

The issue is related to access controls. These comments don't match the issue:

        # NOTE: Allowing the reverse proxy will enable a vulnerability where the ui/panel can be spoofed
        #   by altering the X-FORWARDED-FOR ip address in the request header.

Also, the Configuration wiki page should be updated to reflect the changes introduced with beef.http.allow_reverse_proxy.

Updated.

@bcoles
Copy link
Collaborator

bcoles commented Jan 14, 2020

@bcoles access to the admin UI is restricted to a particular subnet through beef.restrictions.permitted_ui_subnet config option. The bug was you could bypass this by changing the X-FORWARDED-FOR value in the header, allowing anyone access to the Admin UI login page. Using this IP address for non-reverse proxy applications isn't necessary which is why we've added the beef.http.allow_reverse_proxy config option with a warning. I believe this is a temporary fix until we can separate the Admin UI from the server. I believe that should resolve #1354 but if i'm mistaken please let me know.

This is correct, however the comment in the configuration file does not describe this behavior. My issue is that the risks associated with a security control should be clear.

where the ui/panel can be spoofed by altering the X-FORWARDED-FOR ip address in the request header doesn't make any sense. Someone editing/reading the configuration file has no idea what that means. The issue is not that the admin UI can be spoofed, the issue is that the access controls can be bypassed by using x-forwarded-for headers (and possibly others).

Additionally, the code surrounding use_x_forward_for should also be addressed. #1354 (comment)

@bcoles bcoles removed this from the 0.7.0.0-alpha milestone Aug 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants