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

new network driver: pasta (with port driver implicit) #358

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Mar 7, 2023

Pasta: https://passt.top/passt/
Usage: rootlesskit --net=pasta --port-driver=implicit

  • No support for explicit port forwarding (rootlessctl add-ports),
    as pasta doesn't support it yet.
    Use --port-driver=implicit to let pasta forward TCP ports implicitly.
    The forwarded ports are not visible in rootlessctl list-ports.

  • No support for forwarding UDP ports

  • Tested with pasta 2023_06_25.32660ce on Ubuntu 23.04.
    Doesn't work with 2023_06_03.429e1a7: Option --no-copy-routes needs --config-net
    (This is printed despite that --no-copy-routes is not specified)

  • Doesn't work with Ubuntu 23.04's dpkg (passt_0.0~git20230216.4663ccc-1_amd64.deb):
    Couldn't open user namespace /proc/51813/ns/user: Permission denied
    Likely to be related to AppArmor.
    sudo apparmor_parser -R /etc/apparmor.d/usr.bin.passt can eliminate this error, but pasta still fails with another error ( Couldn't get any nameserver address)

@AkihiroSuda AkihiroSuda added this to the v1.1.1 (tentative) milestone Mar 7, 2023
@sbrivio-rh
Copy link
Contributor

I just found this -- let me know if you need any support with it!

@AkihiroSuda
Copy link
Member Author

I just found this -- let me know if you need any support with it!

Thanks, the current blocker of this PR is how to dynamically expose and unexpose ports, without restarting the pasta process.
Wondering if pasta could support an API socket as in slirp4netns, or just support dynamically reloading a configuration file with inotify, or maybe just with SIGHUP.

@sbrivio-rh
Copy link
Contributor

sbrivio-rh commented Mar 29, 2023

Thanks, the current blocker of this PR is how to dynamically expose and unexpose ports, without restarting the pasta process. Wondering if pasta could support an API socket as in slirp4netns, or just support dynamically reloading a configuration file with inotify, or maybe just with SIGHUP.

pasta supports automatic port forwarding, from the init namespace based on ports bound in the target namespace (-t auto for TCP with a periodic one-second scan, -u auto for UDP without periodic scan), and from the target namespace to the init namespace, over the loopback interface if addressing allows (-T auto and -U auto).

So far, we didn't add any possibility of dynamic and explicit, user-supplied port forwarding configuration, because of these potential downsides with regard to security aspects:

  • configuration file

    • requires parsing at runtime
    • needs one further file handle to be pre-opened (pasta has no access to the filesystem after the configuration phase), meaning additional rules in AppArmor policies and SELinux type enforcements
  • socket API

    • requires parsing at runtime
    • needs one further pre-opened socket which requires a number of access control considerations
    • also implies additional rules in AppArmor and SELinux policies

I'm not claiming it's impossible to implement this in a secure way, I'm just saying it's much harder.

I understand this might cause some headaches in integrations, but is the automatic port forwarding really not good enough for rootlesskit use cases?

If it's not, I would rather consider a binary configuration file, just for ports, possibly four different filesystem entries (TCP and UDP over IPv4 and IPv6), reloaded via inotify (we already use one inotify listener in pasta, to detect that a filesystem-bound namespace is going away and exit).

The user would create those files, pass them as command line options, and also be in charge of their permissions. In AppArmor and SELinux rules, we would just need to make sure that pasta is able to read from those files.

Cc: @dgibson

@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Mar 29, 2023

Thanks for explanation

I understand this might cause some headaches in integrations, but is the automatic port forwarding really not good enough for rootlesskit use cases?

Not enough for RootlessKit, as RootlessKit adopts an explicit port API: https://github.com/rootless-containers/rootlesskit/blob/v1.1.0/pkg/api/openapi.yaml#L20-L54

inotify

SGTM

@AkihiroSuda
Copy link
Member Author

The default value of fs.inotify.max_user_instances seems relatively tight on Ubuntu (128), so reloading a config file on SIGHUP might be better than watching inotify.

@sbrivio-rh
Copy link
Contributor

The default value of fs.inotify.max_user_instances seems relatively tight on Ubuntu (128), so reloading a config file on SIGHUP might be better than watching inotify.

Well, we might already use one inotify instance (unless --no-netns-quit is given), even though not in your integration as you're using "--netns", fmt.Sprintf("/proc/%d/ns/net", childPID),. In general terms, we could reuse the same instance for any additional files we want to monitor.

The SIGHUP approach comes with some additional problems:

  • we would need to consider and define who can send SIGHUP, also in LSM policies
  • pasta is currently single-threaded, and if we don't block the signal, we'll have some non-trivial amount of operations potentially interrupting, temporarily, data transfers. If we block the signal:
    • we would need to add one additional system call to the allowed set in the seccomp filter (sigtimedwait(2), which we need to fetch pending signals)
    • we would get additional overhead from periodically calling sigtimedwait()

The inotify instance is poll()able, so it integrates quite naturally with the existing event model. Unless inotify is really unusable on Ubuntu, I'd rather stick to that.

@AkihiroSuda AkihiroSuda removed this from the v1.1.1 milestone May 29, 2023
@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Jun 13, 2023

Update (18843cc)

  • No support for explicit port forwarding (rootlessctl add-ports), as pasta doesn't support it yet.
    Use --port-driver=implicit to let pasta forward ports implicitly.
    The forwarded ports are not visible in rootlessctl list-ports.

  • Child can't access the parent IP (Seems resolved with 2023_06_25.32660ce)

  • Tested with pasta 2023_05_09.96f8d55 on Ubuntu 23.04.
    Doesn't work with 2023_06_03.429e1a7: Option --no-copy-routes needs --config-net
    (This is printed despite that --no-copy-routes is not specified)
    (Seems resolved with 2023_06_25.32660ce)

  • Doesn't work with Ubuntu 23.04's dpkg (passt_0.0~git20230216.4663ccc-1_amd64.deb):
    Couldn't open user namespace /proc/51813/ns/user: Permission denied
    Likely to be related to AppArmor.
    sudo apparmor_parser -R /etc/apparmor.d/usr.bin.passt can eliminate this error, but pasta still fails with another error ( Couldn't get any nameserver address)

@AkihiroSuda AkihiroSuda changed the title new network driver: pasta new network driver: pasta (with port driver implicit) Jun 13, 2023
@sbrivio-rh
Copy link
Contributor

Update (18843cc)

Sorry for the delay on the port forwarding functionality, and thanks for the error reports! I'm currently traveling, I'll look into them next week.

@AkihiroSuda
Copy link
Member Author

2023_06_25.32660ce seems to have resolved several issues (Thanks @sbrivio-rh)


TODO: analyze the dpkg issue

@sbrivio-rh
Copy link
Contributor

2023_06_25.32660ce seems to have resolved several issues (Thanks @sbrivio-rh)

Oh, you already noticed :) I wanted to finish building new versions of the packages I maintain (Debian and Fedora) versions before updating this ticket...

TODO: analyze the dpkg issue

I updated the AppArmor profiles upstream and in the Debian packages (which are then synchronised to Ubuntu) after that package version, but I don't think I changed/fixed anything related to that. I'll try to have a look.

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Jun 29, 2023
@AkihiroSuda AkihiroSuda force-pushed the pasta branch 2 times, most recently from 5e91aa6 to 11b3f3c Compare June 29, 2023 11:39
@AkihiroSuda AkihiroSuda marked this pull request as ready for review June 29, 2023 11:39
@sbrivio-rh
Copy link
Contributor

sbrivio-rh commented Jun 29, 2023

@AkihiroSuda do you have some kind of timeline for version 2.0.0? I'm working on the port forwarding configuration stuff right now and I'm trying to understand if it makes sense that I try to rush a bit, a lot, or not at all :)

@AkihiroSuda
Copy link
Member Author

@AkihiroSuda do you have some kind of timeline for version 2.0.0? I'm working on the port forwarding configuration stuff right now and I'm trying to understand if it makes sense that I try to rush a bit, a lot, or not at all :)

Thanks.
No timeline, but probably it will be within 3-4 weeks.

@AkihiroSuda AkihiroSuda force-pushed the pasta branch 2 times, most recently from a0516f2 to 0665b27 Compare June 29, 2023 12:55
Pasta: https://passt.top/passt/
Usage: `rootlesskit --net=pasta --port-driver=implicit`

- No support for explicit port forwarding (`rootlessctl add-ports`),
  as pasta doesn't support it yet.
  Use `--port-driver=implicit` to let pasta forward TCP ports implicitly.
  The forwarded ports are not visible in `rootlessctl list-ports`.

- No support for forwarding UDP ports

- Tested with pasta 2023_06_25.32660ce on Ubuntu 23.04.
  Doesn't work with 2023_06_03.429e1a7: `Option --no-copy-routes needs --config-net`
  (This is printed despite that `--no-copy-routes` is not specified)

- Doesn't work with Ubuntu 23.04's dpkg (passt_0.0~git20230216.4663ccc-1_amd64.deb):
  `Couldn't open user namespace /proc/51813/ns/user: Permission denied`
  Likely to be related to AppArmor.

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda AkihiroSuda merged commit ecfe592 into rootless-containers:master Jun 29, 2023
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.

4 participants