Limit permissions for Android images. #746

Alexhuszagh · 2022-06-03T01:38:15Z

Remove the use of the --privileged flag for Android images and instead use an seccomp permissions. The provided profile is derived from the docker documentation, with slight modifications to allow clone and clone3.

The documentation is docker seccomp, which details the syscalls blocked by docker. The same is true for podman. We merely modified these settings to allow personality syscall, which then allows us to use our Android images.

Alexhuszagh · 2022-06-03T01:38:41Z

Converted to a draft since this will have merge conflicts with #745.

Alexhuszagh · 2022-06-03T03:01:25Z

I've confirmed this works on Linux, WSL2, Docker Desktop for Windows. It should work without issue on macOS, but I haven't manually confirmed that yet.

src/docker.rs

Alexhuszagh · 2022-06-03T15:33:33Z

I should probably add: this still allows you to pass the --privileged which will override the --security opts seccomp=..., so if users have a crate that requires potentially dangerous syscalls to run, they can always disable these security settings. It's certainly better than defaulting into a privileged mode.

Alexhuszagh · 2022-06-04T14:53:25Z

Update: this doesn't work with writing long file names on Windows, I'll probably have to fix this to a UNC path or something, and it seems to be related to docker/for-win#12760. Or actually, it's different? It seems to be trying to open the actual file, then pass that to the container and then complain that this is the path? Weird.

Both Docker and Podman have separate issues here, weirdly enough. It was working earlier on Docker for Desktop.

EDIT: This is currently dependent on #755.

Alexhuszagh · 2022-06-04T18:24:35Z

On Windows Docker, this seems to be a Docker issue which I've filed upstream. We might just have to use unconfined instead of the path if we're using Docker rather than Podman and on Windows. I've fixed the issues but this depends on #755 currently, since we need different behavior on podman and docker both.

docker/for-win#12760

Emilgardis

nice, do we need to wait for the upstream issue to be fixed? I don't think so

src/file.rs

Alexhuszagh · 2022-06-05T06:10:32Z

nice, do we need to wait for the upstream issue to be fixed? I don't think so

Nope, I've added a workaround for Docker on Windows. I'm going to rebase, then try to check on OS X, and it all passes, I'll make this ready for review.

Alexhuszagh · 2022-06-05T18:46:43Z

Ok I've tested this on the following targets, and this passes on all (except podman on macOS, but for unrelated issues, see #756 and #757):

Windows 10 (Docker Desktop)
Windows 10 (Podman, not run inside WSL2)
Windows 10 (Docker, run inside WSL2)
Windows 10 (Podman, run inside WSL2)
macOS Catalina x86_64 (Docker Desktop)
macOS Catalina x86_64 (Podman)
Fedora 36 (Docker)
Fedora 36 (Podman)

I've had to do a few special workarounds for Windows, but those work well. This is ready for review and merging. #756 currently can't be fixed, so the failure on podman on macOS I think shouldn't be an issue. I've added a tracking issue for the unexpected requirement to provide WSL paths to podman here.

src/docker.rs

Emilgardis · 2022-06-08T16:47:50Z

src/docker.rs

+            let mut path = env::current_dir()
+                .wrap_err("couldn't get current directory")?
+                .canonicalize()
+                .wrap_err_with(|| "when canonicalizing current_dir".to_string())?
+                .join("target")
+                .join(target.triple())
+                .join("seccomp.json");
+            if !path.exists() {
+                write_file(&path, false)?.write_all(SECCOMP.as_bytes())?;
+            }
+            #[cfg(target_os = "windows")]
+            if is_podman {
+                // podman weirdly expects a WSL path here, and fails otherwise
+                path = wslpath(&path, verbose)?;
+            }


I think the windows path thing would be solved by canonicalizing with file::canonicalize, haven't tested it myself

It is not, see docker/for-win#12760.

sorry, meant the podman issue

Unfortunately that's also another issue. See containers/podman#14494. Fortunately, it will always accept a WSL path in the future, as long as it is expected to have consistent behavior with volumes (which also do).

Remove the use of the `--privileged` flag for Android images and instead use an seccomp permissions. The provided profile is derived from the docker documentation, with slight modifications to allow `clone` and `clone3`. The documentation is [docker seccomp](https://docs.docker.com/engine/security/seccomp/#significant-syscalls-blocked-by-the-default-profile), which details the syscalls blocked by docker. The same is true for podman. We merely modified these settings to allow `personality` syscall, which then allows us to use our Android images. On Windows with Docker Desktop, we currently have an issue where Docker tries to read the seccomp profile, and then interpret that as the path, rather than load the profile from the path, which is tracked by the following issue: docker/for-win#12760 On Podman (not inside WSL2), we have a separate issue where it expects a WSL path to be provided for the seccomp profile, despite the path being provided for the host.

otavio · 2022-06-08T22:26:52Z

bors r=Alexhuszagh

Alexhuszagh · 2022-06-08T22:28:49Z

bors r=Alexhuszagh

I think that's supposed to be bors r=Otavio,Emilgardis.

Emilgardis · 2022-06-08T22:28:58Z

use bors r=user when they are the reviewer, in this case you should just do bors r+

Emilgardis · 2022-06-08T22:29:38Z

bors r-

bors r=otavio,emilgardis

bors · 2022-06-08T22:29:40Z

Canceled.

745: Add `thumbv7neon-*` targets. r=otavio a=Alexhuszagh Add the `thumbv7neon-linux-androideabi` and `thumbv7neon-unknown-linux-gnueabihf` targets. Closes #254. 746: Limit permissions for Android images. r=otavio,emilgardis a=Alexhuszagh Remove the use of the `--privileged` flag for Android images and instead use an seccomp permissions. The provided profile is derived from the docker documentation, with slight modifications to allow `clone` and `clone3`. The documentation is [docker seccomp](https://docs.docker.com/engine/security/seccomp/#significant-syscalls-blocked-by-the-default-profile), which details the syscalls blocked by docker. The same is true for podman. We merely modified these settings to allow `personality` syscall, which then allows us to use our Android images. Co-authored-by: Alex Huszagh <[email protected]>

bors · 2022-06-08T22:32:15Z

This PR was included in a batch that was canceled, it will be automatically retried

bors · 2022-06-08T23:53:45Z

Build succeeded:

conclusion

Alexhuszagh force-pushed the seccomp branch from 1d261a1 to aa60441 Compare June 3, 2022 01:39

Alexhuszagh added enhancement A-android Area: android targets labels Jun 3, 2022

Alexhuszagh force-pushed the seccomp branch from aa60441 to f507fb6 Compare June 3, 2022 01:51

Alexhuszagh force-pushed the seccomp branch 2 times, most recently from 69973c0 to 8e2cfd8 Compare June 3, 2022 12:58

Emilgardis requested changes Jun 3, 2022

View reviewed changes

src/docker.rs Outdated Show resolved Hide resolved

Alexhuszagh force-pushed the seccomp branch 3 times, most recently from 7e43e26 to 70ece91 Compare June 3, 2022 15:02

Alexhuszagh force-pushed the seccomp branch 2 times, most recently from 88e6357 to 4888d06 Compare June 3, 2022 16:16

Alexhuszagh force-pushed the seccomp branch from 4888d06 to 6305fc7 Compare June 4, 2022 16:03

Alexhuszagh force-pushed the seccomp branch 2 times, most recently from 316ac6f to e7dc70a Compare June 4, 2022 18:35

Emilgardis reviewed Jun 5, 2022

View reviewed changes

src/file.rs Outdated Show resolved Hide resolved

Alexhuszagh force-pushed the seccomp branch from e7dc70a to 671be21 Compare June 5, 2022 06:21

Alexhuszagh marked this pull request as ready for review June 5, 2022 18:46

Alexhuszagh requested a review from a team as a code owner June 5, 2022 18:46

Alexhuszagh requested a review from Emilgardis June 5, 2022 18:47

Alexhuszagh force-pushed the seccomp branch from 671be21 to 61c4bb7 Compare June 8, 2022 16:02

Emilgardis reviewed Jun 8, 2022

View reviewed changes

Alexhuszagh force-pushed the seccomp branch from 61c4bb7 to 879233b Compare June 8, 2022 17:03

Emilgardis mentioned this pull request Jun 8, 2022

allow specifying a dockerfile to use for a target and implement "pre-build"ing #678

Merged

Emilgardis approved these changes Jun 8, 2022

View reviewed changes

otavio approved these changes Jun 8, 2022

View reviewed changes

Alexhuszagh force-pushed the seccomp branch from 879233b to 13f4497 Compare June 8, 2022 22:12

Alexhuszagh mentioned this pull request Jun 8, 2022

Add thumbv7neon-* targets. #745

Merged

bors bot merged commit ee2fc1b into cross-rs:main Jun 8, 2022

Alexhuszagh mentioned this pull request Jun 13, 2022

Add comprehensive support for remote docker. #785

Merged

Emilgardis added this to the v0.2.2 milestone Jun 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limit permissions for Android images. #746

Limit permissions for Android images. #746

Alexhuszagh commented Jun 3, 2022

Alexhuszagh commented Jun 3, 2022

Alexhuszagh commented Jun 3, 2022

Alexhuszagh commented Jun 3, 2022 •

edited

Loading

Alexhuszagh commented Jun 4, 2022 •

edited

Loading

Alexhuszagh commented Jun 4, 2022 •

edited

Loading

Emilgardis left a comment

Alexhuszagh commented Jun 5, 2022

Alexhuszagh commented Jun 5, 2022 •

edited

Loading

Emilgardis Jun 8, 2022

Alexhuszagh Jun 8, 2022

Emilgardis Jun 8, 2022

Alexhuszagh Jun 8, 2022 •

edited

Loading

otavio commented Jun 8, 2022

Alexhuszagh commented Jun 8, 2022

Emilgardis commented Jun 8, 2022

Emilgardis commented Jun 8, 2022

bors bot commented Jun 8, 2022

bors bot commented Jun 8, 2022

bors bot commented Jun 8, 2022

Limit permissions for Android images. #746

Limit permissions for Android images. #746

Conversation

Alexhuszagh commented Jun 3, 2022

Alexhuszagh commented Jun 3, 2022

Alexhuszagh commented Jun 3, 2022

Alexhuszagh commented Jun 3, 2022 • edited Loading

Alexhuszagh commented Jun 4, 2022 • edited Loading

Alexhuszagh commented Jun 4, 2022 • edited Loading

Emilgardis left a comment

Choose a reason for hiding this comment

Alexhuszagh commented Jun 5, 2022

Alexhuszagh commented Jun 5, 2022 • edited Loading

Emilgardis Jun 8, 2022

Choose a reason for hiding this comment

Alexhuszagh Jun 8, 2022

Choose a reason for hiding this comment

Emilgardis Jun 8, 2022

Choose a reason for hiding this comment

Alexhuszagh Jun 8, 2022 • edited Loading

Choose a reason for hiding this comment

otavio commented Jun 8, 2022

Alexhuszagh commented Jun 8, 2022

Emilgardis commented Jun 8, 2022

Emilgardis commented Jun 8, 2022

bors bot commented Jun 8, 2022

bors bot commented Jun 8, 2022

bors bot commented Jun 8, 2022

Alexhuszagh commented Jun 3, 2022 •

edited

Loading

Alexhuszagh commented Jun 4, 2022 •

edited

Loading

Alexhuszagh commented Jun 4, 2022 •

edited

Loading

Alexhuszagh commented Jun 5, 2022 •

edited

Loading

Alexhuszagh Jun 8, 2022 •

edited

Loading