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

dnsdist: Restrict remote connection to the console via an ACL #6399

Merged
merged 5 commits into from Mar 28, 2018

Conversation

@rgacogne
Copy link
Member

@rgacogne rgacogne commented Mar 27, 2018

Short description

This PR adds an ACL to restrict remote connection to the console, only allowing local connections by default. Please note that this is a breaking change from previous versions, where using any non-loopback address allowed remote connections, although the secret key was still required provided that encryption had been enabled.
Closes #4654.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
@rgacogne rgacogne added this to the dnsdist-1.3.0 milestone Mar 27, 2018
@@ -2264,6 +2239,10 @@ try
g_ACL.setState(acl);
}

auto consoleACL = g_consoleACL.getCopy();
consoleACL.addMask("127.0.0.1/8");

This comment has been minimized.

@pieterlexis

pieterlexis Mar 27, 2018
Member

For completeness-sake, maybe we should add ::1/128 here as well?

This comment has been minimized.

@zeha

zeha Mar 27, 2018
Collaborator

yes please, various things default to ::1 now...

This comment has been minimized.

@rgacogne

rgacogne Mar 27, 2018
Author Member

Makes sense!

@@ -205,6 +229,10 @@ Access Control Lists

:param {str} netmasks: A table of CIDR netmask, e.g. ``{"192.0.2.0/24", "2001:DB8:14::/56"}``. Without a subnetmask, only the specific address is allowed.

.. function:: showACL()

Print a list of all allowed netmasks.

This comment has been minimized.

@zeha

zeha Mar 27, 2018
Collaborator

this should probably say what this ACL is used for...

This comment has been minimized.

@rgacogne

rgacogne Mar 27, 2018
Author Member

Right, I'll write a few words!

@rgacogne
Copy link
Member Author

@rgacogne rgacogne commented Mar 27, 2018

Added ::1/128 and a few words about addACL(), setACL()andshowACL()`.

@zeha
zeha approved these changes Mar 27, 2018
rgacogne added 3 commits Mar 27, 2018
@rgacogne rgacogne force-pushed the rgacogne:dnsdist-console-acl branch from 617e326 to c29d4db Mar 28, 2018
@pieterlexis
Copy link
Member

@pieterlexis pieterlexis commented Mar 28, 2018

A test would be nice. e.g. setting the acl to 127.0.0.1 and attempting to connect from 127.0.0.2?

rgacogne added 2 commits Mar 28, 2018
@rgacogne
Copy link
Member Author

@rgacogne rgacogne commented Mar 28, 2018

Regression tests added (the second version is even consistent :))!

@rgacogne rgacogne merged commit 069bdae into PowerDNS:master Mar 28, 2018
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@rgacogne rgacogne deleted the rgacogne:dnsdist-console-acl branch Mar 28, 2018
@rgacogne rgacogne mentioned this pull request Mar 29, 2018
3 of 6 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants