-
Notifications
You must be signed in to change notification settings - Fork 43
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
Conversation
…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]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Forgot to submit some comments I had
Copyright 2013 Suryandaru Triandana <[email protected]> | ||
All rights reserved. | ||
|
||
Redistribution and use in source and binary forms, with or without | ||
modification, are permitted provided that the following conditions are | ||
met: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious what the right approach is for this license file; currently this means that golang.org/moby/sys/capability
will be licensed as MIT, but the LICENSE at the top of the repository is Apache2.
I know when vendoring other modules from this repository, go mod
copies the LICENSE from the top-level, so I wonder what it does for this module (will it copy both? will it only use the MIT license?).
Reading these two StackOverflow answers;
- https://opensource.stackexchange.com/a/7391
- https://law.stackexchange.com/questions/6081/can-i-bundle-mit-licensed-components-in-a-apache-2-0-licensed-project
- At least the original license and copyright must be retained (see below)
- Possibly as a secondary file (?)
- Possibly included in a NOTICE file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am now testing this PR in opencontainers/runc#4358, and from what I see, go mod vendor
uses top-level LICENSE file for e.g. moby/sys/mount and other packages, and it uses directory-level LICENSE for moby/sys/capability
. In other words, it works as it should.
I don't see any problem with this layout -- a LICENSE file in a subdirectory overwrites the upper level LICENSE. If they are compatible (as in our case), all is fine and dandy.
Disclaimer: IANAL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'm somewhat looking from a "module" perspective, because it may become somewhat ambiguous; for the GitHub repo, the license on the repo-level is Apache 2, with a subdirectory having the MIT license.
But from a module perspective, normally it would copy that license; in this case it only has the MIT license. And perhaps that's OK (so, the capabilities
module having a different license than the other modules), but was wondering if we wanted to re-license the new module to use Apache 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
☝️ s/MIT/BSD 2-clause/ (I was confused)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
golang.org/moby/sys/capability will be licensed as MIT
For the sake of those reading this thread. This is actually a BSD 2-clause license, not the MIT license.
See https://opensource.org/license/bsd-2-clause
5365f50
to
6a5bb11
Compare
This is to test moby/sys#149 Signed-off-by: Kir Kolyshkin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BSD-2-clause is different from Apache 2, but strictly less restrictive, so is probably OK. Trying to relicense is going to be a huge pain. 👍
This seems fine to me (currently discussing some minor suggested changes in the Moby maintainers meeting, but I don't think there's any strong opposition overall).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@AkihiroSuda PTAL if this LGTY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Also /cc @dims (FYI - you probably also have an interest in this) |
of course! this is a welcome development. thanks for the poke @thaJeztah |
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]>
Signed-off-by: Kir Kolyshkin <[email protected]>
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]>
1. Add "Copyright 2023 The Capability Authors" header to cover my work since 2023 as well as any future work in the forked package. 2. Remove (c) after the word "Copyright" (not required since 1989). 3. Remove the comma after a copyright year. 4. Minor formatting fix (move the line break earlier). Signed-off-by: Kir Kolyshkin <[email protected]>
1. Clarify the fork source. 2. Fix links to 0.1.x PRs to point to the proper (old) repo. Signed-off-by: Kir Kolyshkin <[email protected]>
It was mistakenly set to Go >= 1.20, while it fact this package requires Go >= 1.21 (due to the use of sync.OnceValues). Signed-off-by: Kir Kolyshkin <[email protected]>
3d55018
to
27f233e
Compare
Just pushed a new version, mostly addressing @AkihiroSuda's comments, i.e.
But also:
All that (except for the missing commit addition) are added as the last 2 commits, so you are re-reviewing, you can only review these two. |
I think we can address any other concerns post-merge. |
Thanks 🙏 |
This is to test moby/sys#149 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,
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: