Enhance subid check by using shadow library function #1180

mhjacks · 2022-11-28T23:02:47Z

This encompasses the old mechanism of looking in files as well as a newer option, using subids supplied through SSS.

This is still WIP (I have not built or tested this yet).

softwarefactory-project-zuul · 2022-11-28T23:27:44Z

Build failed.

❌ unit-test FAILURE in 8m 15s
❌ unit-test-migration-path-for-coreos-toolbox FAILURE in 8m 04s
❌ system-test-fedora-rawhide FAILURE in 19m 57s
❌ system-test-fedora-36 FAILURE in 8m 11s
❌ system-test-fedora-35 FAILURE in 7m 58s

debarshiray

Thanks for working on this @mhjacks !

src/cmd/root.go

mhjacks · 2022-11-29T14:56:09Z

OK, in the podman code I missed that there are explicit type defintions that come from a separate, vendored package. I replicated those here, and meson now compiles it (with the other fixes you suggested). I'll work on testing this on fedora for the following cases:

subuid/subgid not defined
subuid/subgid defined in /etc files, and configured to look there
subuid/subgid provided by FreeIPA (nsswitch.conf has subid: sss)

softwarefactory-project-zuul · 2022-11-29T15:56:14Z

Build failed.

❌ unit-test RETRY_LIMIT in 8m 11s
❌ unit-test-migration-path-for-coreos-toolbox RETRY_LIMIT in 7m 58s
❌ system-test-fedora-rawhide RETRY_LIMIT in 17m 43s
❌ system-test-fedora-36 RETRY_LIMIT in 8m 13s
❌ system-test-fedora-35 RETRY_LIMIT in 7m 06s

debarshiray · 2022-11-29T19:13:05Z

There's something wrong with the network today:

go: gopkg.in/alecthomas/[email protected]: invalid version: git fetch -f origin refs/heads/*:refs/heads/* refs/tags/*:refs/tags/* in /home/zuul-worker/go/pkg/mod/cache/vcs/c2c1b0712bebcf6ec5667522440cdf7ab348dcc9077f5f66c20d71b2a726d743: exit status 128:
	error: RPC failed; HTTP 502 curl 22 The requested URL returned error: 502
	fatal: the remote end hung up unexpectedly
go: gopkg.in/[email protected]: invalid version: git ls-remote -q origin in /home/zuul-worker/go/pkg/mod/cache/vcs/c0f327cd4dd133603590e688dc30587a52391afc7436996e10bee6738185a38e: exit status 128:
	remote: Cannot obtain refs from GitHub: cannot talk to GitHub: Get https://github.com/go-errgo/errgo.git/info/refs?service=git-upload-pack: net/http: request canceled (Client.Timeout exceeded while awaiting headers)
	fatal: unable to access 'https://gopkg.in/errgo.v2/': The requested URL returned error: 502
go: unrecognized import path "gopkg.in/yaml.v3": reading https://gopkg.in/yaml.v3?go-get=1: 502 Bad Gateway
	server response: Cannot obtain refs from GitHub: cannot talk to GitHub: Get https://github.com/go-yaml/yaml.git/info/refs?service=git-upload-pack: net/http: request canceled (Client.Timeout exceeded while awaiting headers)

debarshiray · 2022-11-29T19:14:00Z

I wonder what this golangci-lint complaint is about:

  Error: could not import C (cgo preprocessing failed) (typecheck)

mhjacks · 2022-11-30T00:02:07Z

golangci/golangci-lint#3289 <- on the golanci-lint issue. It seems like zuul is still having problems downloading the modules for the other check. I have downloaded golangci-lint locally and run it, and it does not complain. Github actions tend to use ubuntu, but I would think we would hardly be the first to notice this if that were the problem.

softwarefactory-project-zuul · 2022-11-30T01:25:30Z

Build failed.

❌ unit-test RETRY_LIMIT in 8m 28s
❌ unit-test-migration-path-for-coreos-toolbox RETRY_LIMIT in 8m 51s
❌ system-test-fedora-rawhide RETRY_LIMIT in 29m 06s
❌ system-test-fedora-36 RETRY_LIMIT in 8m 06s
❌ system-test-fedora-35 RETRY_LIMIT in 7m 22s

mhjacks · 2022-11-30T02:36:00Z

I built the new binary from this branch and copied it to /usr/bin/toolbox on a test host.

New output with --verbose on fedora 37 (Freeipa Backed):

toolbox --verbose enter
DEBU Running as real user ID 344800010            
DEBU Resolved absolute path to the executable as /usr/bin/toolbox 
DEBU Running on a cgroups v2 host                 
DEBU Checking for subuid and subgid have entries for user mjackson 
DEBU Found subuid range 0 for mjackson: start [2147483648] length [65536] 
DEBU Found subgid range 0 for mjackson: start [2147483648] length [65536] 
DEBU TOOLBOX_PATH is /usr/bin/toolbox

New output when backed by /etc files:

DEBU Running as real user ID 344800010            
DEBU Resolved absolute path to the executable as /usr/bin/toolbox 
DEBU Running on a cgroups v2 host                 
DEBU Checking for subuid and subgid have entries for user mjackson 
DEBU Found subuid range 0 for mjackson: start [200000] length [65536] 
DEBU Found subgid range 0 for mjackson: start [200000] length [65536] 
DEBU TOOLBOX_PATH is /usr/bin/toolbox

Need to add some code to throw the error if no ranges are found. :) Standby

mhjacks · 2022-11-30T02:49:43Z

Apparently if no ranges are found, nRanges can be 0. I expanded the error check to error on nRanges <= 0. This catches no subuid/gid defined, as well as one or the other missing (I tested all those cases).

Here's the verbose error report now:

DEBU Running as real user ID 344800010            
DEBU Resolved absolute path to the executable as /usr/bin/toolbox 
DEBU Running on a cgroups v2 host                 
DEBU Checking for subuid and subgid have entries for user mjackson 
Error: Missing subuid and/or subgid entries for user mjackson
See the podman(1), subgid(5), subuid(5) and usermod(8) manuals for more
information.

mhjacks · 2022-11-30T02:54:35Z

This will fix #1074 and fedora-silverblue/issue-tracker#263

softwarefactory-project-zuul · 2022-11-30T03:05:47Z

Build failed.

❌ unit-test FAILURE in 7m 49s
❌ unit-test-migration-path-for-coreos-toolbox FAILURE in 7m 50s
❌ system-test-fedora-rawhide FAILURE in 19m 53s
❌ system-test-fedora-36 FAILURE in 7m 56s
❌ system-test-fedora-35 FAILURE in 7m 53s

mhjacks · 2022-11-30T13:41:14Z

The zuul failures seem to be due to not requiring shadow-utils-subid-devel but I am not sure if I can do anything about that from this PR (I've never done anything with zuul-ci). shadow-utils-subid-devel is available in F35, so adding it as a dependency should work, and I would expect the code to work and the tests to pass as they are.

debarshiray · 2022-11-30T15:10:17Z

The zuul failures seem to be due to not requiring shadow-utils-subid-devel but I am not sure if I can do anything about that from this PR (I've never done anything with zuul-ci). shadow-utils-subid-devel is available in F35, so adding it as a dependency should work, and I would expect the code to work and the tests to pass as they are.

The dependencies are listed in playbooks/dependencies.yaml, which is hooked up to .zuul.yaml. For what it's worth, those YAML files are Ansible playbooks, so if you get stuck you can look for Ansible documentation.

debarshiray · 2022-11-30T15:11:55Z

I built the new binary from this branch and copied it to /usr/bin/toolbox on a test host.

New output with --verbose on fedora 37 (Freeipa Backed):

toolbox --verbose enter
DEBU Running as real user ID 344800010            
DEBU Resolved absolute path to the executable as /usr/bin/toolbox 
DEBU Running on a cgroups v2 host                 
DEBU Checking for subuid and subgid have entries for user mjackson 
DEBU Found subuid range 0 for mjackson: start [2147483648] length [65536] 
DEBU Found subgid range 0 for mjackson: start [2147483648] length [65536] 
DEBU TOOLBOX_PATH is /usr/bin/toolbox

New output when backed by /etc files:

DEBU Running as real user ID 344800010            
DEBU Resolved absolute path to the executable as /usr/bin/toolbox 
DEBU Running on a cgroups v2 host                 
DEBU Checking for subuid and subgid have entries for user mjackson 
DEBU Found subuid range 0 for mjackson: start [200000] length [65536] 
DEBU Found subgid range 0 for mjackson: start [200000] length [65536] 
DEBU TOOLBOX_PATH is /usr/bin/toolbox

Brilliant. This is exciting. :)

softwarefactory-project-zuul · 2022-11-30T16:23:00Z

Build succeeded.

✔️ unit-test SUCCESS in 8m 49s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 8m 49s
✔️ system-test-fedora-rawhide SUCCESS in 24m 24s
✔️ system-test-fedora-36 SUCCESS in 11m 24s
✔️ system-test-fedora-35 SUCCESS in 12m 32s

softwarefactory-project-zuul · 2022-11-30T17:28:12Z

Build succeeded.

✔️ unit-test SUCCESS in 8m 31s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 8m 30s
✔️ system-test-fedora-rawhide SUCCESS in 23m 05s
✔️ system-test-fedora-36 SUCCESS in 10m 56s
✔️ system-test-fedora-35 SUCCESS in 11m 36s

debarshiray

Thanks for getting this to work @mhjacks !

src/cmd/root.go

debarshiray · 2022-11-30T18:46:25Z

I wonder how difficult it would be to hack up a test for this?

The simplest I can think of is:

rm both /etc/subuid and /etc/subgid
echo "subid: sss" >>/etc/nsswitch.conf

... and then ensure that all the tests pass. This won't be a good idea when running the tests locally, but it might be OK to do on the CI?

softwarefactory-project-zuul · 2023-01-26T15:43:58Z

Build failed.

✔️ unit-test SUCCESS in 11m 21s
❌ unit-test-migration-path-for-coreos-toolbox FAILURE in 3m 04s
✔️ system-test-fedora-rawhide SUCCESS in 22m 00s
✔️ system-test-fedora-36 SUCCESS in 13m 07s
❌ system-test-fedora-35 FAILURE in 9m 45s

debarshiray · 2023-01-26T15:59:26Z

Hi @debarshiray - if possible I'd like to pick this up again. What can I do to help get this in shape and merged?

Hey, hey! There were two outstanding issues.

One is the business with the leaking file descriptor which was causing confusion in the test suite. See above. I believe I have managed to resolve it now.

The other problem is that some versions of the Go compiler (1.16.15 in Fedora 35, and 1.19.4 in CentOS Stream) don't seem to like the C preprocessor's stringize operator (ie., #). I tried to use it to cheaply convert the SUBID_ABI_VERSION number into a string for dlopen(3).

softwarefactory-project-zuul · 2023-01-26T18:24:28Z

Build failed.

✔️ unit-test SUCCESS in 11m 22s
❌ unit-test-migration-path-for-coreos-toolbox FAILURE in 3m 46s
✔️ system-test-fedora-rawhide SUCCESS in 24m 36s
✔️ system-test-fedora-36 SUCCESS in 15m 34s
❌ system-test-fedora-35 FAILURE in 9m 24s

softwarefactory-project-zuul · 2023-01-26T22:56:19Z

Build failed.

✔️ unit-test SUCCESS in 8m 28s
❌ unit-test-migration-path-for-coreos-toolbox FAILURE in 2m 43s
✔️ system-test-fedora-rawhide SUCCESS in 12m 29s
✔️ system-test-fedora-36 SUCCESS in 12m 10s
❌ system-test-fedora-35 FAILURE in 8m 43s

softwarefactory-project-zuul · 2023-01-27T19:44:53Z

Build failed.

✔️ unit-test SUCCESS in 8m 50s
❌ unit-test-migration-path-for-coreos-toolbox FAILURE in 2m 40s
✔️ system-test-fedora-rawhide SUCCESS in 13m 01s
✔️ system-test-fedora-36 SUCCESS in 12m 33s
❌ system-test-fedora-35 FAILURE in 8m 43s

softwarefactory-project-zuul · 2023-01-27T20:09:38Z

Build failed.

✔️ unit-test SUCCESS in 8m 57s
❌ unit-test-migration-path-for-coreos-toolbox FAILURE in 2m 44s
✔️ system-test-fedora-rawhide SUCCESS in 13m 13s
✔️ system-test-fedora-36 SUCCESS in 12m 43s
❌ system-test-fedora-35 FAILURE in 8m 49s

debarshiray · 2023-01-27T20:48:20Z

I am such an idiot!

I was staring at these two identical failures on CentOS Stream 9 and Fedora 35, thinking that either CGO can't handle the C preprocessor's stringize operator, or that there's something wrong in GCC 11.3.1, which they both have.

centos-9-stream | FAILED: completion/toolbox
centos-9-stream | /usr/bin/meson --internal exe --capture completion/toolbox -- /usr/bin/python3 /home/zuul-worker/src/github.com/containers/toolbox/completion/generate_completions.py /home/zuul-worker/src/github.com/containers/toolbox/src bash
centos-9-stream | --- stderr ---
centos-9-stream | level=debug msg="Running as real user ID 1000"
centos-9-stream | level=debug msg="Resolved absolute path to the executable as /tmp/go-build2155851540/b001/exe/toolbox"
centos-9-stream | level=debug msg="Running on a cgroups v2 host"
centos-9-stream | level=debug msg="Looking for sub-GID and sub-UID ranges for user zuul-worker"
centos-9-stream | level=debug msg="Looking for sub-GID and sub-UID ranges: cannot dlopen(3) libsubid.so.SUBID_ABI_VERSION"

fedora-35 | FAILED: completion/toolbox.fish
fedora-35 | /usr/bin/meson --internal exe --capture completion/toolbox.fish -- /usr/bin/python3 /home/zuul-worker/src/github.com/containers/toolbox/completion/generate_completions.py /home/zuul-worker/src/github.com/containers/toolbox/src fish
fedora-35 | --- stderr ---
fedora-35 | level=debug msg="Running as real user ID 1000"
fedora-35 | level=debug msg="Resolved absolute path to the executable as /tmp/go-build2794419376/b001/exe/toolbox"
fedora-35 | level=debug msg="Running on a cgroups v2 host"
fedora-35 | level=debug msg="Looking for sub-GID and sub-UID ranges for user zuul-worker"
fedora-35 | level=debug msg="Looking for sub-GID and sub-UID ranges: cannot dlopen(3) libsubid.so.SUBID_ABI_VERSION"

I failed to notice that they also have Shadow 4.9, which doesn't have SUBID_ABI_VERSION. Damn. :)

softwarefactory-project-zuul · 2023-01-27T21:08:26Z

Build failed.

✔️ unit-test SUCCESS in 8m 43s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 2m 57s
✔️ system-test-fedora-rawhide SUCCESS in 12m 49s
✔️ system-test-fedora-36 SUCCESS in 12m 30s
❌ system-test-fedora-35 RETRY_LIMIT in 7m 06s

debarshiray · 2023-01-27T21:10:58Z

recheck

softwarefactory-project-zuul · 2023-01-27T21:24:53Z

Build succeeded.

✔️ unit-test SUCCESS in 8m 38s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 2m 51s
✔️ system-test-fedora-rawhide SUCCESS in 12m 44s
✔️ system-test-fedora-36 SUCCESS in 12m 17s
✔️ system-test-fedora-35 SUCCESS in 13m 39s

mhjacks · 2023-01-27T22:16:30Z

Nice!

debarshiray · 2023-01-28T21:22:16Z

I am not sure what happened, because I suddenly got a permission denied when trying to update this pull request with a cleaned up final version of the branch. However, in hindsight, I am wondering if that was because I messed up the branch name in the git push.

Anyway, I ended up merging the code through #1219

debarshiray · 2023-01-28T21:24:27Z

Thanks a lot for all your work and patience, @mhjacks !

softwarefactory-project-zuul · 2023-01-28T21:35:20Z

Build succeeded.

✔️ unit-test SUCCESS in 9m 10s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 2m 56s
✔️ system-test-fedora-rawhide SUCCESS in 15m 36s
✔️ system-test-fedora-36 SUCCESS in 12m 34s
✔️ system-test-fedora-35 SUCCESS in 14m 02s

mhjacks · 2023-01-28T23:10:46Z

Absolutely my pleasure! I'm excited this is merged now. Do you a new release of toolbox would be appropriate now?

debarshiray · 2023-01-29T09:27:00Z

Yes, it's been a while since our last release. Hopefully in a week. fingers crossed :)

I have to get #1065 done by tomorrow, but otherwise I think main is in good shape now.

debarshiray · 2023-01-29T10:07:31Z

For the sake of posterity ...

I wonder what this golangci-lint complaint is about:
  Error: could not import C (cgo preprocessing failed) (typecheck)

This pull request broke the golangci-lint test because the GitHub Action runs on Ubuntu 22.04 which only has Shadow 4.8, whereas libsubid.so was introduced in Shadow 4.9.

However, that's not a big deal because we earlier added go vet to the set of tests run by meson test, and go vet is one of the linters run by golangci-lint. So, while it's not a proper replacement, it's good enough.

The previous commit broke the golangci-lint test [1] because the GitHub Action runs on Ubuntu 22.04, which only has Shadow 4.8 [2], whereas libsubid.so was introduced in Shadow 4.9 [3]. However, that's not a big deal because 'go vet' was earlier added to the set of tests run by 'meson test' [4], and 'go vet' is one of the linters run by golangci-lint [5]. So, while it's not a proper replacement, it's good enough. [1] Commit ca8007c containers#1180 [2] https://packages.ubuntu.com/source/jammy/shadow https://packages.ubuntu.com/source/jammy-updates/shadow [3] Shadow commit 0a7888b1fad613a0 shadow-maint/shadow@0a7888b1fad613a0 shadow-maint/shadow#154 [4] Commit f695012 containers#1186 [5] https://golangci-lint.run/usage/linters/ https://golangci-lint.run/usage/linters/#govet This reverts commit 7c86f30.

The previous commit broke the golangci-lint test [1] because the GitHub Action runs on Ubuntu 22.04, which only has Shadow 4.8 [2], whereas libsubid.so was introduced in Shadow 4.9 [3]. However, that's not a big deal because 'go vet' was earlier added to the set of tests run by 'meson test' [4], and 'go vet' is one of the linters run by golangci-lint [5]. So, while it's not a proper replacement, it's good enough. [1] Commit ca8007c containers#1180 [2] https://packages.ubuntu.com/source/jammy/shadow https://packages.ubuntu.com/source/jammy-updates/shadow [3] Shadow commit 0a7888b1fad613a0 shadow-maint/shadow@0a7888b1fad613a0 shadow-maint/shadow#154 [4] Commit f695012 containers#1186 [5] https://golangci-lint.run/usage/linters/ https://golangci-lint.run/usage/linters/#govet This reverts commit 7c86f30. containers#1221

mhjacks marked this pull request as draft November 28, 2022 23:02

mhjacks force-pushed the enhance-subid-check branch from eb2c53b to 2a129d7 Compare November 28, 2022 23:06

debarshiray reviewed Nov 29, 2022

View reviewed changes

src/cmd/root.go Outdated Show resolved Hide resolved

src/cmd/root.go Outdated Show resolved Hide resolved

mhjacks force-pushed the enhance-subid-check branch from 2a129d7 to d9c331a Compare November 29, 2022 14:52

mhjacks force-pushed the enhance-subid-check branch from d9c331a to 0d8baac Compare November 29, 2022 14:56

mhjacks force-pushed the enhance-subid-check branch from 87aef77 to e838d16 Compare November 30, 2022 02:24

mhjacks force-pushed the enhance-subid-check branch from e838d16 to 42308b0 Compare November 30, 2022 02:45

mhjacks marked this pull request as ready for review November 30, 2022 02:45

mhjacks force-pushed the enhance-subid-check branch 2 times, most recently from 5307d0a to 1b75d35 Compare November 30, 2022 15:34

mhjacks force-pushed the enhance-subid-check branch from 1b75d35 to 9a77dac Compare November 30, 2022 17:04

debarshiray reviewed Nov 30, 2022

View reviewed changes

src/cmd/root.go Outdated Show resolved Hide resolved

src/cmd/root.go Outdated Show resolved Hide resolved

debarshiray mentioned this pull request Nov 30, 2022

Support subordinate user and group IDs provided by SSSD on enterprise FreeIPA set-ups #1074

Closed

debarshiray force-pushed the enhance-subid-check branch from c353daa to ce09cee Compare January 26, 2023 17:57

debarshiray force-pushed the enhance-subid-check branch from ce09cee to 497c804 Compare January 26, 2023 22:42

debarshiray force-pushed the enhance-subid-check branch from 497c804 to 48b01aa Compare January 27, 2023 19:31

debarshiray force-pushed the enhance-subid-check branch 3 times, most recently from c53540d to 7729d29 Compare January 27, 2023 20:36

debarshiray closed this Jan 28, 2023

debarshiray force-pushed the enhance-subid-check branch from 7729d29 to ca8007c Compare January 28, 2023 21:18

debarshiray mentioned this pull request Jan 28, 2023

Support subordinate user and group ID ranges on enterprise set-ups #1219

Merged

mhjacks deleted the enhance-subid-check branch January 28, 2023 23:50

debarshiray mentioned this pull request Jan 29, 2023

.github/workflows: Remove golangci-lint #1221

Merged

Enhance subid check by using shadow library function #1180

Enhance subid check by using shadow library function #1180

Conversation

mhjacks commented Nov 28, 2022

softwarefactory-project-zuul bot commented Nov 28, 2022

debarshiray left a comment

Choose a reason for hiding this comment

mhjacks commented Nov 29, 2022

softwarefactory-project-zuul bot commented Nov 29, 2022

debarshiray commented Nov 29, 2022

debarshiray commented Nov 29, 2022

mhjacks commented Nov 30, 2022 • edited Loading

softwarefactory-project-zuul bot commented Nov 30, 2022

mhjacks commented Nov 30, 2022

mhjacks commented Nov 30, 2022

mhjacks commented Nov 30, 2022 • edited Loading

softwarefactory-project-zuul bot commented Nov 30, 2022

mhjacks commented Nov 30, 2022

debarshiray commented Nov 30, 2022

debarshiray commented Nov 30, 2022

softwarefactory-project-zuul bot commented Nov 30, 2022

softwarefactory-project-zuul bot commented Nov 30, 2022

debarshiray left a comment

Choose a reason for hiding this comment

debarshiray commented Nov 30, 2022

softwarefactory-project-zuul bot commented Jan 26, 2023

debarshiray commented Jan 26, 2023

softwarefactory-project-zuul bot commented Jan 26, 2023

softwarefactory-project-zuul bot commented Jan 26, 2023

softwarefactory-project-zuul bot commented Jan 27, 2023

softwarefactory-project-zuul bot commented Jan 27, 2023

debarshiray commented Jan 27, 2023

softwarefactory-project-zuul bot commented Jan 27, 2023

debarshiray commented Jan 27, 2023

softwarefactory-project-zuul bot commented Jan 27, 2023

mhjacks commented Jan 27, 2023

debarshiray commented Jan 28, 2023

debarshiray commented Jan 28, 2023

softwarefactory-project-zuul bot commented Jan 28, 2023

mhjacks commented Jan 28, 2023

debarshiray commented Jan 29, 2023

debarshiray commented Jan 29, 2023 • edited Loading

mhjacks commented Nov 30, 2022 •

edited

Loading

mhjacks commented Nov 30, 2022 •

edited

Loading

debarshiray commented Jan 29, 2023 •

edited

Loading