Don't set ambient caps without inheritable ones

[kolyshkin]

Commit 98fe566 removed setting inheritable capabilities from

• runc exec --cap;
• example spec (used by runc spec);
• libcontainer/integration test config;
• libcontainer/README.md;
  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 package with the fix, that bug will become
apparent (both bats-based and inct/int tests will fail).

This PR removes setting ambient capabilities where inheritable ones
are not set. This has no effect in the current codebase (since those
ambient capabilities were not set anyway, and the errors were ignored),
but will prevent errors once we switch to the fixed capability package.

As we do not have any tests for runc exec --cap, add one.

[rata]

LGTM, thanks!

However, IMHO, it would be great to document why we removed inheritable capabilities in the first place. It is clear that if we remove inh, we should remove ambient too.

[kolyshkin]

However, IMHO, it would be great to document why we removed inheritable capabilities in the first place. It is clear that if we remove inh, we should remove ambient too.

All I remember it was a security fix, originally reported by cap maintainers (Andrew Morgan). Something along the lines that inheritable capabilities are usually being set from the file system attributes, and are not expected to be set this way (runc exec --cap).

[lifubang]

Another problem, maybe there is a situation we should consider:
A user wants to exec with a cap in inheritable cap list in ‘config.json’ but not in ambient cap list in ‘config.json’. In my brain, when doing exec, the process’s inheritable cap list is inherited from ‘config.json’, so it maybe exists in ‘runc exec’. The original security patch is to prevent extend the inheritable cap list, but not delete it.

If this is true, I think this PR is wrong.

[kolyshkin]

A user wants to exec with a cap in inheritable cap list in ‘config.json’ but not in ambient cap list in ‘config.json’.

With a single --caps option which does not allow a user to specify the type of capabilities it's not possible. If needed, a user can use runc exec --process process.json and specify all the individual types of caps.

@lifubang try to look at this PR in the following way:

• it is not possible to set ambient caps without inheritable caps set;
• inheritable caps should not be set by default, and were removed to fix GHSA-f3fp-gc8g-vw66;
• without inheritable caps set, ambient caps bits are now ignored (due to a bug in gocapability);

So what this PR does is removes ambient caps which are not being set anyway (they are ignored). In other words, it changes nothing.

[lifubang]

• inheritable caps should not be set by default, and were removed to fix GHSA-f3fp-gc8g-vw66;

So, what you mean is that the ‘inheritable caps’ should always be empty in ‘config.json’?

[kolyshkin]

• inheritable caps should not be set by default, and were removed to fix GHSA-f3fp-gc8g-vw66;

So, what you mean is that the ‘inheritable caps’ should always be empty in ‘config.json’?

Probably not always, but by default they should be empty.

Whence, ambient caps should also be empty by default, since it's not possible to set ambient without inheritable.

Hope that clears things up.

[lifubang]

LGTM
Waiting #4412 merged to unblock CI.