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

FIREWALL: Allow to block hosts by IP address #90

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

aplopez
Copy link
Contributor

@aplopez aplopez commented Oct 24, 2024

When blocking a host, its hostname is resolved using the dig command. If an IP address is provided, dig returns nothing and it is not blocked.

We need to check whether it is an IP address before launching dig. No matter whether the correct family is requested, do not call dig when an IP address is provided, in case the caller has already blocked the DNS host.

@aplopez
Copy link
Contributor Author

aplopez commented Oct 24, 2024

tox failures existed before this PR.

@pbrezina
Copy link
Contributor

Hi, please rebase to get CI green.

@aplopez
Copy link
Contributor Author

aplopez commented Oct 24, 2024

Hi, please rebase to get CI green.

Perfect. Thank you.

Comment on lines 610 to 630
ipv4 = IPv4Address(hostname)
if type == "A":
return [hostname]
except ValueError:
ipv4 = None

try:
ipv6 = IPv6Address(hostname)
if type == "AAAA":
return [hostname]
except ValueError:
ipv6 = None

if ipv4 is not None or ipv6 is not None:
# In this case we were given an IP address but of the wrong family. We must not consider it a hostname.
return []
else:
result = self.firewall.host.conn.exec(
["dig", "+short", "-t", type, hostname], log_level=ProcessLogLevel.Error
)
return result.stdout_lines
Copy link
Contributor

Choose a reason for hiding this comment

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

How about simplifying it to:

addrs = []
try:
    ip = ipaddress.ip_address(hostname)
    ip_type = ipaddress.IPv4Address if type == "A" else ipaddress.IPv6Address
    if isinstance(ip, ip_type):
        addrs = [hostname]
except ValueError:
    result = self.firewall.host.conn.exec(
        ["dig", "+short", "-t", type, hostname], log_level=ProcessLogLevel.Error
    )
   addrs = result.stdout_lines

return addrs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! I will use it.

When blocking a host, its hostname is resolved using the `dig`
command. If an IP address is provided, `dig` returns nothing.
Check whether it is an IP address before launching `dig`.

No matter whether the correct family is requested, do not call
`dig` when an IP address is provided, in case the caller has
already blocked the DNS host.
Copy link
Contributor

@pbrezina pbrezina left a comment

Choose a reason for hiding this comment

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

Ack. Thank you. Since I contributed to the solution, somebody else should review it as well.

@pbrezina pbrezina merged commit 81105a8 into next-actions:master Oct 29, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants