Repository navigation
[bug bash] Unknown Linux capability names in an ActorTemplate are rejected at admission - #3487
Conversation
Benjamin Elder (BenTheElder)
added
area/api
kata-agent panics on a capability name it can't parse, and an unknown name in drop was silently ignored Signed-off-by: Benjamin Elder <[email protected]>
c40c2b2 to
f0707d9
Compare
| // linuxCapabilities are the Linux capability names, in bit order, without the | ||
| // "CAP_" prefix (which is added when the OCI spec is written). runsc and | ||
| // kata-agent accept exactly these, and kata-agent panics on any other name. | ||
| var linuxCapabilities = sets.New( |
There was a problem hiding this comment.
NOTE: new capabilities are rare, and even if the kernel gains them, we still have to get them in kata and gvisor first. The last time was kernel 5.8-5.9 adding a few AFAICT.
Julian Gutierrez Oschmann (juli4n)
left a comment
Julian Gutierrez Oschmann (juli4n)
left a comment
There was a problem hiding this comment.
I think rejecting unknown names at admission is a good idea, but this validation should be just for a better UX (i.e. fail-fast), not for correctness. I think that at runtime we should fail with a proper error message if an actor is asked for a CAP which is not supported.
This way, if we ever extend this list with a new CAP which is not supported across all runtimes, we don't regress.
| case !capabilityRE.MatchString(c): | ||
| errs = append(errs, field.Invalid(p, c, "must be an uppercase capability name like 'NET_BIND_SERVICE'")) | ||
| case !linuxCapabilities.Has(c): | ||
| errs = append(errs, field.Invalid(p, c, "must be a Linux capability name like 'NET_BIND_SERVICE'")) |
There was a problem hiding this comment.
Other enumeration string fields are reported as field.NotSupported so we should be consistent and use it here as well.
I think we just ... won't do that. It will require careful review. And it should be very rare. The last time linux added a capability was 2020 AFAICT. |
Signed-off-by: Benjamin Elder <[email protected]>
Eitan Yarmush (EItanya)
left a comment
Eitan Yarmush (EItanya)
left a comment
There was a problem hiding this comment.
I agree with Julian Gutierrez Oschmann (@juli4n)'s review that we should add this to the worker side, but I think this is a good step
… ActorTemplate are rejected at admission (#3514) Cherry-pick of #3487 onto `release-1.0` (`git cherry-pick -x` of the squash commit), it applied cleanly with no changes. Capability names in `add` and `drop` are checked against the 41 Linux capabilities at admission, and atelet runs the same check on Run and Restore. Before, a typo in `add` failed at actor start and a typo in `drop` was ignored. Part of #3226. AI use: AI-assisted. Clean pick. This carries the same breaking change as #3487: a template with an unknown name in `drop` is now rejected, check `drop` lists for typos. Unknown names in `add` already failed at actor start. - [x] Tests pass - [x] Docs updated Signed-off-by: Benjamin Elder <[email protected]>
Part of #3226.
We only checked the shape of capability names, so a typo passed admission. In
addit failed at actor start, and on microVM kata-agent panics converting it (trans.rs). Indropit was ignored, sodrop: [NET_BIND_SERVIC]keptNET_BIND_SERVICE.Names are now checked against the 41 Linux capabilities, the same set runsc (specutils.go) and kata's oci-spec (capability.rs) accept. atelet runs the same check on Run and Restore.
AI use: AI-assisted, I reviewed it.
Breaking change: a template with an unknown name in
dropis now rejected, checkdroplists for typos. Unknown names inaddalready failed at actor start.