Page MenuHomeFreeBSD

Allow PING(8) in jails without raw socket access permissions
Needs ReviewPublic

Authored by cneirabustos_gmail.com on Oct 14 2020, 5:31 PM.
Tags
None
Referenced Files
F81664020: D26782.diff
Fri, Apr 19, 3:55 PM
Unknown Object (File)
Sun, Apr 7, 12:18 AM
Unknown Object (File)
Dec 23 2023, 8:01 AM
Unknown Object (File)
Dec 13 2023, 4:14 PM
Unknown Object (File)
Nov 10 2023, 10:00 AM
Unknown Object (File)
Oct 18 2023, 12:58 AM
Unknown Object (File)
Oct 9 2023, 8:56 AM
Unknown Object (File)
Sep 30 2023, 5:58 AM
Subscribers

Details

Reviewers
csjp
jamie
Summary

Allow jails to use PING(8) without allowing raw socket access.
Raw socket access is allowed for ICMP protocol as is required by
PING(8) but option IP_HDRINCL is not allowed. to accomplish this
a new privilege PRIV_NETINET_ICMP_ACCESS has been added by default
for jails.

Reference for this implementation was based on:
McDougall Richard; Mauro Jim. Solaris Internals. Pearson Education.

Test Plan

Create a jail without specifying raw socket access, execute PING(8) it should work.

Diff Detail

Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 34166
Build 31322: arc lint + arc unit

Event Timeline

You've added an optional permission bit, but there's no option to change it. If it would make sense to allow it to all jails, there's no need for PR_ALLOW_ICMP_ACCESS. If it would make sense to restrict to some jails, there needs to be a matching jail.allow parameter, as defined in kern_jail.c's pr_flag_allow array and SYSCTL_JAIL_PARAM(_allow, ...).

sys/jail.h
234

Is this an old version of jail.h? 0x00000200 is taken by PR_ALLOW_UNPRIV_DEBUG, but 0x00000400 is free. Also, be sure to update PR_ALLOW_ALL_STATIC.

I have a couple of comments for this:
(a) ICMP access is not just echo requests; this will alllow jails to send other things out which might not be a great idea, so if you do, default off.
(b) I'd love to have an ICMPv6 equivalent; ICMPv6 is even worse as to what people can do; see (a)
(c) I don't think this is a good idea.
(d) Even with this there'll be other issues (as are with raw sockets), such as source address selection, which was once was discussed in 2011 [ https://people.freebsd.org/~bz/20120407-01-ping-source-addr.diff ]; probably on freebsd-net or -jails back then.
(e) lastly we do have full network stacks these days, and even if they are not always needed the tradeoff to adding more hacks on top of classic IP-jails is questionable.

Please don't get me wrong about this, I not not saying no to it, I just think if it is done it needs a fully thought out solution and not just a bit less restrictive priv hack. I'll leave it in jamie's hands.

netinet/raw_ip.c
867

Is this supposed to be a || ?

SOCK_RAW is still used by other subsystems, e.g. configuring firewalls rules among other things. This is the main reason raw sockets were restricted within jails in the first place. I am not seeing how this patch protects other subsystems.

Hi bz,

Thanks for taking the time to check this out.
You are right I'm just checking for ICMP protocol but I should check for ICMP echo requests, as my intention is just to allow what is needed to make PING(8) work.
I thought that allowing ICMP echo requests and don't allow to set IP_HDRINCL on the packets was enough to accomplish this.
In your expertise is something else needed to accomplish this ?.

Bests

In D26782#597187, @bz wrote:

I have a couple of comments for this:
(a) ICMP access is not just echo requests; this will alllow jails to send other things out which might not be a great idea, so if you do, default off.
(b) I'd love to have an ICMPv6 equivalent; ICMPv6 is even worse as to what people can do; see (a)
(c) I don't think this is a good idea.
(d) Even with this there'll be other issues (as are with raw sockets), such as source address selection, which was once was discussed in 2011 [ https://people.freebsd.org/~bz/20120407-01-ping-source-addr.diff ]; probably on freebsd-net or -jails back then.
(e) lastly we do have full network stacks these days, and even if they are not always needed the tradeoff to adding more hacks on top of classic IP-jails is questionable.

Please don't get me wrong about this, I not not saying no to it, I just think if it is done it needs a fully thought out solution and not just a bit less restrictive priv hack. I'll leave it in jamie's hands.

In D26782#597232, @csjp wrote:

SOCK_RAW is still used by other subsystems, e.g. configuring firewalls rules among other things. This is the main reason raw sockets were restricted within jails in the first place. I am not seeing how this patch protects other subsystems.

Hi csjp,

My intend is just to make PING(8) work by only allowing icmp echo requests, I don't want to give raw socket access to a jail to just use PING(8).
I thought it was straight forward to make this work.
You are right, I allowed icmp access but I'm not checking if the type if the echo request, I'll fix that.