-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add capability package #149
Open
kolyshkin
wants to merge
47
commits into
moby:main
Choose a base branch
from
kolyshkin:add-capability
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+1,587
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…urrent task In this case we can use /proc/self/, which is correct even if a task live in another pid namespace. Signed-off-by: Andrey Vagin <[email protected]>
New capabilities can be added, and we want to be sure that a bounding set will be set correctly in this case. Without this patch new capabilities are not dropped from a bounding set. Signed-off-by: Andrey Vagin <[email protected]>
fmt.Fscanf reads byte by byte from this file, but this doesn't work for sysctl-s. 29279 open("/proc/sys/kernel/cap_last_cap", O_RDONLY|O_CLOEXEC 29279 <... open resumed> ) = 3 29279 read(3, "3", 1) = 1 29279 read(3, "", 1) = 0 Reported-by: @syndtr
…t_cap file Fix docker/libcontainer#452 Signed-off-by: Alexander Morozov <[email protected]>
Signed-off-by: Alexander Morozov <[email protected]>
Signed-off-by: Alexander Morozov <[email protected]>
handle ENODATA in getVfsCap
Ambient capabilities were added in Linux 4.3 and provide a way to pass on capabilities to unprivileged processes easily. Signed-off-by: Justin Cormack <[email protected]>
After getting CapBnd, Loop break too early, can't to get CapAmb value. Signed-off-by: Ma Shimiao <[email protected]>
…by#14) The old methods had an internal Load(), which is unnecessary for some use cases. For example, if you're going to drop all capabilities, you don't need to load the current set first. This commit deprecates the old New* functions and adds New*2 functions which do not include the internal Load. Callers who do need the Load will need to call it explicitly after initializing their Capabilities object. Callers who do not need the Load can just add the "2" to the function name and get more efficient/robust behavior. The "Deprecated:" paragraph syntax is recommended in [1]: To signal that an identifier should not be used, add a paragraph to its doc comment that begins with "Deprecated:" followed by some information about the deprecation. [1]: https://blog.golang.org/godoc-documenting-go-code
* Fix capHeader.pid type In C, int is 4 bytes in 32 and 64-bit systems. In Go, int is a 8 bytes in 64-bit systems. Before this fix, pid was being ignored because the kernel will always read 0 due to padding added between version and pid fields. * Update capability_linux.go
CAP_PERFMON and CAP_BPF were introduced in kernel 5.8: https://kernelnewbies.org/Linux_5.8#Introduce_CAP_BPF_and_CAP_PERFMON_security_capabilities CAP_CHECKPOINT_RESTORE was merged on the master recently and will be available in the next version of the kernel. torvalds/linux@124ea65 The capability numbers are taken from https://github.com/torvalds/linux/blob/442489c219235991de86d0277b5d859ede6d8792/include/uapi/linux/capability.h#L373-L416 Signed-off-by: Akihiro Suda <[email protected]>
…/gocapability into AkihiroSuda-kernel58
Signed-off-by: Kir Kolyshkin <[email protected]>
Go 1.17 introduced new style of adding build tags (//go:build), and some tools no longer understand old-style (// +build) tags. Add the new tag, drop the old one. Signed-off-by: Kir Kolyshkin <[email protected]>
Move the code to the top-level directory. Signed-off-by: Kir Kolyshkin <[email protected]>
In case kernel folks will ever release capability v4, the chances are high v3 is still supported. Therefore, we should not error out upon seeing an unknown version from the kernel, but assume we can go with v3. While at it, treat the uninitialized capVers as an error. Before this patch, it was still treated as an error, but "unknown capability version" is not exactly what the error is, so let's be more specific. Reported-by: Andrei Vagin <[email protected]> Signed-off-by: Kir Kolyshkin <[email protected]>
Capabilities v3 API was added by the Linux kernel 2.6.26. Since go 1.18 (no longer supported as of go 1.20 release), the minimum Linux kernel requirement is 2.6.32 (see [1]). So, it does not make sense to support capabilities v1 and v2 any more. Drop the support, returning the appropriate error message. [1] https://tip.golang.org/doc/go1.18#linux Signed-off-by: Kir Kolyshkin <[email protected]>
Commit f3cb87f added support for ambient capabilities. Unfortunately, the code added to Apply is incorrect because it uses a local variable err which is never used or returned. Found by a linter: > capability_linux.go:480:5: ineffectual assignment to err (ineffassign) > err = nil > ^ Fixes: f3cb87f Signed-off-by: Kir Kolyshkin <[email protected]>
Fix the following warning: > capability_linux.go:407:17: S1039: unnecessary use of fmt.Sprintf (gosimple) > status_path = fmt.Sprintf("/proc/self/status") > ^ Also, - simplify the code for the common case when pid is 0; - rename a variable (to obey Go naming guidelines). Signed-off-by: Kir Kolyshkin <[email protected]>
Fix the following linter warning: > capability_linux.go:34:8: Error return value is not checked (errcheck) > capget(&hdr, nil) > ^ Here we deliberately ignore the error from capget() since we have no way to report it, and hdr.ver will be 0 in case of an error anyway. Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
This fixes errcheck linter warnings: > capability_linux.go:311:14: Error return value of `fmt.Sscanf` is not checked (errcheck) > fmt.Sscanf(line[4:], "nd: %08x%08x", &c.bounds[1], &c.bounds[0]) > ^ > capability_linux.go:315:14: Error return value of `fmt.Sscanf` is not checked (errcheck) > fmt.Sscanf(line[4:], "mb: %08x%08x", &c.ambient[1], &c.ambient[0]) > ^ Signed-off-by: Kir Kolyshkin <[email protected]>
Currently, capVers is initialized in func init, but its value is only used from NewPid*. Let's use sync.OnceValues for lazy initialization. While at it, stop ignoring syscall return value. Signed-off-by: Kir Kolyshkin <[email protected]>
One (minor) issue with this package is it has func init which reads a file in /proc, making the start of any program which imports the package a bit slower. Let's switch to lazy initialization, i.e. only read the file when needed. Unfortunately, this can not be done in a non-disruptive manner, since we have a public variable CAP_LAST_CAP. So, this is a disruptive change, and anyone who's using CAP_LAST_CAP should change their code to something like this: last, err := capability.LastCap() if err != nil { return err } Also, add a test case for LastCap. Reported-by: ningmingxiao <[email protected]> Signed-off-by: Kir Kolyshkin <[email protected]>
Rename whats -> what in test to silence codespell warnings. Signed-off-by: Kir Kolyshkin <[email protected]>
Mostly for ignoring this warning: ./capability_linux.go:311: nd ==> and, 2nd Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
Brought to you by gofumpt v0.6.0 (go1.22.4). Add a CI action to check code is gofumpt'ed. Signed-off-by: Kir Kolyshkin <[email protected]>
With the added linter, it complains like this: > capability_linux.go:349:22: type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint) In fact, errors from syscall.Syscall6 used by prctl are bare Errno values. This means there is no need for a type assertion, so let's remove it: > - if errno, ok := err.(syscall.Errno); ok && errno == syscall.EINVAL { > + if err == syscall.EINVAL { With that change, we're still getting error from the linter, a bit different one: > capability_linux.go:349:9: comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint) So, we still need to silence it, by adding a nolint annotation. Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
As LastCap is a variable, it is shown in the documentation as a variable (see [1]). This is both ugly and unsafe (a variable can be changed). Wrap it into a proper function. [1]: https://pkg.go.dev/github.com/kolyshkin/[email protected]#pkg-variables Signed-off-by: Kir Kolyshkin <[email protected]>
Earlier commit made sure we don't error out if the kernel capability version is unknown; this ensures compatibility with future kernels. Looking at the code, I realized p.hdr.version should be initialized to linuxCapVer3 in that case, not the version returned by the kernel, otherwise we supply v3 data structure with (say) v4 version set in header. Practically, this was not a real bug (yet) because v4 is not (yet) available, but if it will ever be introduced later, this fix makes us ready. Fixes: a8e5adc ("Fix future version compatibility") Signed-off-by: Kir Kolyshkin <[email protected]>
This integrates github.com/kolyshkin/capability (which itself is a fork of github.com/syndtr/gocapability) to github.com/moby/sys/capability. Some of github.com/syndtr/gocapability users are (in an alphabetical order): - https://github.com/canonical/lxd - https://github.com/containers/buildah - https://github.com/containers/podman - https://github.com/containers/skopeo - https://github.com/google/gvisor - https://github.com/hashicorp/nomad - https://github.com/linuxkit/linuxkit - https://github.com/opencontainers/runc - https://github.com/opencontainers/runtime-tools - https://github.com/slimtoolkit/slim Signed-off-by: Kir Kolyshkin <[email protected]>
9ef7ffb
to
4663aad
Compare
4663aad
to
5901006
Compare
Rename a test file so the test it implements is only run on Linux. Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
5901006
to
7d13b42
Compare
I guess we're OK with the DCO check failing, it would be unwise to rewrite 12 year old commits. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This integrates github.com/kolyshkin/capability (which itself is a fork
of github.com/syndtr/gocapability) to github.com/moby/sys/capability,
as suggested in opencontainers/runc#4358 (comment)
Some of github.com/syndtr/gocapability users are (in an alphabetical order):
The way it was done is: