Apply: fix ambient caps error handling #3

kolyshkin · 2024-07-22T21:27:38Z

Commit e7cb7fa 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: e7cb7fa

Based on syndtr#24

kolyshkin · 2024-07-22T21:28:58Z

This was LGTMed by @avagin here

Commit e7cb7fa 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: e7cb7fa Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Fixing a long standing bug in github.com/syndtr/gocapability package (ignoring errors when setting ambient caps, see [1]) revealed that it's not possible to raise those ambient capabilities for which inheritable capabilities are not raised. In other words, "the Ambient vector cannot contain values not raised in the Inh vector" ([2]). The example spec in libct/specconv has a few ambient capabilities set but no inheritable ones. As a result, with capability package with fix in [1] we get an error trying to start a container ("unable to apply caps: permission denied"). The only decent way to fix this is to ignore raised ambient capabilities for which inheritable capabilities are not raised (essentially mimicking the old behavior). Let's also add a warning about ignored capabilities. Fix the example spec (remove the ambient caps). [1]: kolyshkin/capability#3 [2]: https://pkg.go.dev/kernel.org/pub/linux/libs/security/libcap/cap#IAB.SetVector Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Fixing a long standing bug in github.com/syndtr/gocapability package (ignoring errors when setting ambient caps, see [1]) revealed that it's not possible to raise those ambient capabilities for which inheritable capabilities are not raised. In other words, "the Ambient vector cannot contain values not raised in the Inh vector" ([2]). The example spec in libct/specconv has a few ambient capabilities set but no inheritable ones. As a result, when capability package with fix from [1] is used, we get an error trying to start a container ("unable to apply caps: permission denied"). The only decent way to fix this is to ignore raised ambient capabilities for which inheritable capabilities are not raised (essentially mimicking the old behavior). Let's also add a warning about ignored capabilities. Fix the example spec (remove the ambient caps). This is in preparation to switch to github.com/kolyshkin/capability. [1]: kolyshkin/capability#3 [2]: https://pkg.go.dev/kernel.org/pub/linux/libs/security/libcap/cap#IAB.SetVector Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Commit 98fe566 removes setting inheritable capabilities from runc exec --cap, but neglected to remove ambient capabilities. An ambient capability could only be set if the same inheritable capability is set, so as a result of the above change ambient capabilities were not set (but due to a bug in gocapability package, those errors were never reported). Now, once we start using capability package with the fix [1], that bug would emerge. Also, we do not have any tests for runc exec --cap, so add one. [1]: kolyshkin/capability#3 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Fixing a long standing bug in github.com/syndtr/gocapability package (ignoring errors when setting ambient caps, see [1]) revealed that it's not possible to raise those ambient capabilities for which inheritable capabilities are not raised. In other words, "the Ambient vector cannot contain values not raised in the Inh vector" ([2]). The example spec in libct/specconv has a few ambient capabilities set but no inheritable ones. As a result, when capability package with fix from [1] is used, we get an error trying to start a container ("unable to apply caps: permission denied"). The only decent way to fix this is to ignore raised ambient capabilities for which inheritable capabilities are not raised (essentially mimicking the old behavior). Let's also add a warning about ignored capabilities. Fix the example spec (remove the ambient caps). This is in preparation to switch to github.com/kolyshkin/capability. [1]: kolyshkin/capability#3 [2]: https://pkg.go.dev/kernel.org/pub/linux/libs/security/libcap/cap#IAB.SetVector Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Commit 98fe566 removes setting inheritable capabilities from runc exec --cap, but neglected to remove ambient capabilities. An ambient capability could only be set if the same inheritable capability is set, so as a result of the above change ambient capabilities were not set (but due to a bug in gocapability package, those errors were never reported). Now, once we start using a library with the fix [1], that bug would become apparent. Alas, we do not have any tests for runc exec --cap, so add one. [1]: kolyshkin/capability#3 Fixes: 98fe566 ("runc: do not set inheritable capabilities") Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Commit 98fe566 removes inheritable capabilities from the example spec (used by runc spec) and from the libctontainer/integration config, but neglected to remove ambient capabilities. An ambient capability could only be set if the same inheritable capability is set, so as a result of the above change ambient capabilities were not set (but due to a bug in gocapability package, those errors were never reported). Once we start using a library with the fix [1], that bug will become apparent (both bats-based and libct/int tests will fail). [1]: kolyshkin/capability#3 Fixes: 98fe566 ("runc: do not set inheritable capabilities") Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Commit 98fe566 removed setting inheritable capabilities from runc exec --cap, but neglected to also remove ambient capabilities. An ambient capability could only be set if the same inheritable capability is set, so as a result of the above change ambient capabilities were not set (but due to a bug in gocapability package, those errors are never reported). Once we start using a library with the fix [1], that bug will become apparent. Alas, we do not have any tests for runc exec --cap, so add one. [1]: kolyshkin/capability#3 Fixes: 98fe566 ("runc: do not set inheritable capabilities") Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Commit 98fe566 removed inheritable capabilities from the example spec (used by runc spec) and from the libcontainer/integration test config, but neglected to also remove ambient capabilities. An ambient capability could only be set if the same inheritable capability is set, so as a result of the above change ambient capabilities were not set (but due to a bug in gocapability package, those errors are never reported). Once we start using a library with the fix [1], that bug will become apparent (both bats-based and libct/int tests will fail). [1]: kolyshkin/capability#3 Fixes: 98fe566 ("runc: do not set inheritable capabilities") Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Fixing a long standing bug in github.com/syndtr/gocapability package (ignoring errors when setting ambient caps, see [1]) revealed that it's not possible to raise those ambient capabilities for which inheritable capabilities are not raised. In other words, "the Ambient vector cannot contain values not raised in the Inh vector" ([2]). The example spec in libct/specconv had a few ambient capabilities set but no inheritable ones. As a result, when capability package with fix from [1] is used, we get an error trying to start a container ("unable to apply caps: permission denied"). The only decent way to fix this is to ignore raised ambient capabilities for which inheritable capabilities are not raised (essentially mimicking the old behavior). Let's also add a warning about ignored capabilities. Fix the example spec (remove the ambient caps). This is in preparation to switch to github.com/kolyshkin/capability. [1]: kolyshkin/capability#3 [2]: https://pkg.go.dev/kernel.org/pub/linux/libs/security/libcap/cap#IAB.SetVector Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>