[RFC] Making libpathrs Migrations Easier

[cyphar]

While talking to folks about libpathrs, one common topic of concern is that switching to a CGo dependency for a library which is not (yet) widely packaged can be difficult, as well as some more general concerns about binary sizes (for the record, dylib builds of libpathrs are ~550K while static builds are ~2.7M, though after unused symbols are stripped after linking it might be less in practice).

It seems very likely that I will have to continue maintaining github.com/cyphar/filepath-securejoin for the foreseeable future, but at the same time there are plenty of reasons to move to libpathrs (more hardening against some attacks, cross-language compatibility, more features, better lifecycle of file objects, possibly non-Linux support in the future). I see a few potential options:

0. Abandon libpathrs

I'm mainly including this one for completeness, but it's a non-starter. The reason for writing libpathrs is that these kinds of path security issues exist all over the stack and so any library trying to solve the problem needs to be written in a language that has decent support for exporting an API via C, which isn't the case for Go.

There are also some deeper issues with Go (the lifecycle of *os.File being the most notable one) which actually make it preferable for the core implementation to be in a non-Go language, even if you end up providing Go bindings. For instance, the symlink stack implementation in libpathrs doesn't make any copies of the file descriptors (thanks to Rc and OwnedFd ownership model) but the Go implementation is forced to create copies of every symlink fd we encounter because the lookup code needs to close intermediate file handles.

1. "Just Suck It Up"

I continue to maintain github.com/cyphar/filepath-securejoin and libpathrs in parallel, and users wanting to switch to libpathrs will to switch their usage (which can be done in parallel, but still results in needless code bloat).

This is the "cleanest" approach if we want to separate the two projects, but it makes it quite unlikely that users will switch to libpathrs for a long time, and getting libpathrs packaged requires users for distributions to bother packaging libpathrs with no strong reason to. Also, lots of extra maintenance overhead, as there will be a push for features in libpathrs to be ported to filepath-securejoin (as has happened over the past year).

2. Use filepath-securejoin as a "Bridge" ("libpathrs" build tags in filepath-securejoin)

As the API in github.com/cyphar/filepath-securejoin is based quite directly on a (simplified) version of the libpathrs API, it seems that we could have two implementations of github.com/cyphar/filepath-securejoin. One will be the (current) pure Go implementation, and the other will be a dumb wrapper around libpathrs.

Then, users would be able to opt-in to libpathrs support at compile time by using -tags securejoin_libpathrs (suggestions for other names welcome). Because Go build tags are global, this has the added benefit of migrating all users of github.com/cyphar/filepath-securejoin to using libpathrs transparently at build time. Distributions will then be able to slowly migrate to libpathrs, while also supporting building on legacy platforms without Rust support.

Another option would be to make this opt-out (i.e. -tags securejoin_purego to use the current implementation). This would be ideal in a vacuum, but it would cause build errors for existing projects and so would require us to release github.com/cyphar/filepath-securejoin/v2 in order to avoid downstream breakages. We could do that, but then we would need to also use the former option to reduce the . Given that adding build tags is usually easier than removing them (when it comes to Makefiles) perhaps securejoin_libpathrs is a technically better solution anyway?

There are some obvious follow-up questions about testing these wrappers -- we might be able to just run go test -tags securejoin_libpathrs but our tests use internal functions a fair bit (though it might all "just work"...).

This option (with the opt-in -tags securejoin_libpathrs) is my current personal preference, as it doesn't increase maintainership burden but allows distributions to optionally enable libpathrs.

3. Migrate the filepath-securejoin implementations into libpathrs ("pure Go" build tags in libpathrs)

This is kind of a mirror proposal to (2). Instead of making filepath-securejoin a wrapper around libpathrs, we can copy the implementation of filepath-securejoin and make the Go bindings of libpathrs have a pure Go equivalent. This would be opt-in, so users would have to build with -tags libpathrs_purego.

The only real benefit this has in principle is that it unifies the pure Go and libpathrs implementations in one repo.

Ultimately the issue with this approach is that it has the main downsides of both (1) and (2). This doesn't help with maintaining filepath-securejoin unless I also make it a wrapper around libpathrs (and then the flag can't be opt-in unless I expose the internal Go implementation in a subpackage, which is also ugly). Also there will be a stronger push to continue to maintain the pure Go implementations when (in my view) they should just be a bridge towards users migrating to libpathrs. And it also means that we will have to maintain these wrappers in at least two repos, unless we decide to archive this repo (which is not tenable as we have quite a few users).

[cyphar]

WDYT @mrunalp @AkihiroSuda @kolyshkin @thaJeztah @lifubang @rata?

@AkihiroSuda had concerns about binary size, but if we can slowly migrate with build tags (and potentially produce two binary versions in our releases for a bit) maybe that'd be more acceptable?

[AkihiroSuda]

"Just Suck It Up" rather seems the least suckful way.
The plain go build should just work without no-go deps and tags

[cyphar]

(2) would work the same way by default though -- you would be able to opt-in to it with a build tag at build time, but the default would still be pure-go (as it is today). Personally, even if runc never switches to libpathrs for our default builds, I would want to enable this for openSUSE builds because libpathrs has more hardening.

[rata]

@cyphar thanks for long explanations, they really help :)

About option 1, I'm not sure what this means:

I continue to maintain github.com/cyphar/filepath-securejoin and libpathrs in parallel, and users wanting to switch to libpathrs will to switch their usage (which can be done in parallel, but still results in needless code bloat).

Both things are maintained in parallel, but how are both integrated into runc? That part I didn't get.

Also, just to understand, is there any lang limitation in Go to not provide the extra hardening? Or is it more convenience (it's already written in rust, it's tricky, and we want the lib to have a C API)?

[cyphar]

@rata

Both things are maintained in parallel, but how are both integrated into runc? That part I didn't get.

So, the original model I had for migration was that libpathrs would be completely separate, and I would then migrate runc to using libpathrs gradually, eventually deprecating filepath-securejoin. My personal end goal of (1) would be that runc would exclusively use libpathrs at some point in the future (with maybe some exceptions for the old insecure securejoin.SecureJoin usage in places where it is still safe to use).

However, when thinking about it some more recently, I realised this approach actually has one massive issue.

Even if we (as runc maintainers) agree to having a new required non-Go dependency (which I acknowledge is already a hurdle), we use a lot of libraries that need to do operations that need to be secured by libpathrs (basically anything that touches /proc or does almost any regular filesystem operation in relation to the container). It seems incredibly unlikely that we would be able to convince those upstreams to use a library with no pure-Go fallback (even once libpathrs gets widely packaged, many Go projects have a hard no-CGo-usage rule). So we need to have a way to silently migrate them if we want to use libpathrs everywhere, but without negatively impacting them first.

The approach in (2) to me seems far better for everyone -- for users that don't care about libpathrs, they will still get the same pure-Go implementation at no extra cost, but at build-time you can easily switch to libpathrs (the API would be exactly the same). The only downside (aside from some extra project organisation complexity due to the glue code) is that the Root/Handle newtype stuff isn't currently exported in filepath-securejoin (everything is raw *os.File handles) -- maybe I can reconsider what I want to do there. But (2) would also allow this project to run their integration tests against both backends.

The other issue is that if we add a new API to libpathrs, in order for runc to use it (without requiring libpathrs) we will need to write a new pure Go version in filepath-securejoin. This also means that the API design for new features needs to consider both the libpathrs Rust and Go binding APIs but also the filepath-securejoin APIs... I hope that one day we will be able to hard-require libpathrs, but that is a future problem.

Also, just to understand, is there any lang limitation in Go to not provide the extra hardening? Or is it more convenience (it's already written in rust, it's tricky, and we want the lib to have a C API)?

The main reason is the latter. Some of the hardenings are quite complicated to implement and (quite critically) write tests for, and they are not strictly necessary but are something that libpathrs needs. Also, at the moment filepath-securejoin only has very basic ports of Root::mkdir_all and Root::resolve -- porting the rest of libpathrs is really a lot of extra work for quite little benefit. In particular, filepath-securejoin doesn't support O_* flags for procfs operations, and implementing that is incredibly tricky to get right.

However, there are Go-specific language issues that make the pure Go implementation worse than the Rust one (*os.File lifecycles being the main one, but also some of the fragility around runtime.LockOSThread is also quite unfortunate). Also, some of the compile-time guarantees on stuff like Option<T> in Rust make some of the hardening in libpathrs feel more robust to me.

[rata]

Thanks! I think options 1-3 are quite similar and even if we choose one, maybe over time we decide to do another and it won't cause a major hassle. I'm fine with those options, as long as we don't make a pure-go implementation opt-in. IMHO CGO for libpathrs should be opt-in.

In fact, if I was you, I think I'd mix them. You are currently basically doing 1, so I think it's okay to continue like that for a while. At some point, we can make securejoin a one line wrapper of libpathrs (basically securejoin.F() calls libpathrs.F(), same params, etc. and moving all the go code there, but just c&p and no other changes) and then just use libpathrs. Not sure when to add the buildtags to runc, I don't have a strong opinion there. This would be basically taking something of each option over time :-D (without making the pure go implementation opt-in, as you mention that might break a lot of users, etc. libpathrs should always be opt-in IMHO).

What is not so clear to me, though, is what to do with old runc versions that are still supported. I'd say suck-it-up and maintain libpathrs/securejoin as they were used in that runc release, as long as it is supported (and we can support it, if there are flaws we can't get around, we can update to the new API in an old runc version). If this ends up causing more issues than it solves, and we are not seeing regressions with libpathrs upgrades, we can change our mind on the way.

What do you/others think?

[cyphar]

My main gripe with (3) is that it would be quite strange for you to do import "github.com/cyphar/libpathrs/go-pathrs" and not actually use the same libpathrs implementation as every other language unless you pass a specific build-tag. Also libpathrs has more features than filepath-securejoin, so moving the code to libpathrs would also require porting even more code to the pure Go version to maintain API compatibility.

Note that another benefit of libpathrs is that we would be able to use it for the nsexec code in runc (we do some procfs operations in nsexec that probably should use safe procfs handles). My eventual goal is that we would eventually switch from the securejoin wrapper to libpathrs directly for core runc (even if some dependencies continue to use securejoin). As it currently stands, runc will probably need to have a fair number of helper functions that duplicate features implemented in libpathrs.

For old runc versions, I suspect we will just use the pure Go implementations. For newer runc versions, maybe we can start providing two flavours of static binaries (those that were built with libpathrs, and those using the pure Go versions).

[lorenz]

I don't really have anything against Rust the language, but the toolchain situation for builds where Rust is not the top-level build target is a bit painful. Things like rust-lang/rust#127173 are just janky and break if you include a Rust crate as a static dependency in any C or Go project. Also any static builds will then need to pull in an entire libc because Rust depends on one, with all disadvantages that brings. Rust's libstd is also hard to cross-strap for people who do not want to depend on their pre-compiled binaries. All of these are a problem in upstream Rust, not directly your problems but right now Go's toolchain situation is a lot better for integrators, than Rust's.

[cyphar]

@lorenz I agree that embedding Rust in this way is less than ideal (and IMHO I find most of the FFI support in Rust to be kind of half-baked compared to what you would really like to see for a language that claims to want to allow for gradual migration to Rust) but the only real alternative would be to (re)write libpathrs to C which brings its own set of issues.

For what it's worth, I would expect most builds to be dynamically linked rather than static -- most people still use the binaries from their distribution (and I would expect distributions to opt-in to libpathrs before upstream projects if we go with approach 2 above).

At the moment, I don't have the time to do another rewrite, but if it actually ends up being a significant enough issue for users I would be open to one final rewrite (though I'm a little apprehensive about writing a bunch of string-parsing code in C). Thankfully, __attribute__((__cleanup__())) has made it much easier to write reasonable-looking C code that doesn't have memory or other resource leaks (effectively it's poor man's RAII).

[cyphar]

#70 makes it much easier for us to switch with a build-tag -- now we only need to wrap a handful of functions and add go:build tags to the top-level *_purego.go files.

[leoluk]

I'm co-maintainer of a security-focused Kubernetes distribution and the situation is a bit unclear to us. It sounds like distro maintainers will have to add the entire Rust toolchain as a new build dependency for what used to be a pure Go ecosystem, or potentially miss out on important security mitigations.

Compared to Go, the Rust toolchain situation and Cargo in particular is painful, especially for "tiny distros" and embedded use cases which try to minimize both the number of dependencies as well as binary size.

[cyphar]

@leoluk My current plan is to make this an optional dependency you would opt-into with a libpathrs build tag. If you don't do this (i.e., you build things normally), you will continue to use the pure-Go version and nothing changes.

The idea is for folks (especially libraries) to use pathrs-lite wherever they need it (this needs to "just work" for pure Go programs, as you've pointed) and then downstream applications (or distributions packaging applications) can choose to switch to libpathrs at build time if they wish.

For what it's worth, I expect libpathrs will get packaged in most distributions once runc (and other non-Go projects) start using it, and most distributions have Rust packaging figured out by now, so while Rust does have some downsides it's really nowhere near as bad as it used to be. And the binary size of libpathrs.so is 559KB at the moment -- Go binaries are generally not particularly svelte so this is fairly minuscule as far as binary size goes. But as a practical matter the pure-Go setup needs to continue to just work for the reasons I've outlined in this issue before.

potentially miss out on important security mitigations

At the moment I don't plan to expand the scope of pathrs-lite any further, but the vast majority of protections have already been ported.

The only extra security hardening libpathrs has over pathrs-lite at the moment is in very niche scenarios (pre-5.8 kernel with locked overmounts on /proc, which requires open_tree(AT_RECURSIVE) -- libpathrs has a fallback using fdinfo which should be near impossible to thwart, pathrs-lite doesn't do anything special in this case). On modern systems, the net result is the same.

There are also some extra features available in libpathrs that I have no interest in porting to pathrs-lite (more generic /proc operations including safe magiclink opening, more optimised O_* open logic). Users who want those features are free to use libpathrs.