-
Notifications
You must be signed in to change notification settings - Fork 553
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
specs-go/config: add Landlock LSM support #1111
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,8 +62,66 @@ type Process struct { | |
OOMScoreAdj *int `json:"oomScoreAdj,omitempty" platform:"linux"` | ||
// SelinuxLabel specifies the selinux context that the container process is run as. | ||
SelinuxLabel string `json:"selinuxLabel,omitempty" platform:"linux"` | ||
// Landlock specifies the Landlock unprivileged access control settings for the container process. | ||
// `noNewPrivileges` must be enabled to use Landlock. | ||
Landlock *Landlock `json:"landlock,omitempty" platform:"linux"` | ||
} | ||
|
||
// Landlock specifies the Landlock unprivileged access control settings for the container process. | ||
type Landlock struct { | ||
// Ruleset identifies a set of rules (i.e., actions on objects) that need to be handled. | ||
Ruleset *LandlockRuleset `json:"ruleset,omitempty" platform:"linux"` | ||
// Rules are the security policies (i.e., actions allowed on objects) to be added to an existing ruleset. | ||
Rules *LandlockRules `json:"rules,omitempty" platform:"linux"` | ||
// DisableBestEffort disables the best-effort security approach for Landlock access rights. | ||
// This is for conditions when the Landlock access rights explicitly configured by the container are not | ||
// supported or available in the running kernel. | ||
// Default is false, i.e., following a best-effort security approach. | ||
DisableBestEffort bool `json:"disableBestEffort,omitempty" platform:"linux"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks good to me! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking more about that, I think it would be more flexible to replace this global field with two per feature block: to be able to have a soft requirement (by default) or a hard requirement. This would enable for instance to require that the kernel supports a specific feature that would otherwise break the container (for instance, supporting file reparenting, which would probably be a new kind of option for the ruleset). To sum up, that may look like this: "ruleset": {
"handledAccessFS": [
"execute",
"write_file",
"read_file",
"read_dir",
"remove_dir",
"remove_file",
"make_char",
"make_dir",
"make_reg",
"make_sock",
"make_fifo",
"make_block",
"make_sym"
],
// optional field, default value (all handledAccessFS elements may not be known to the kernel; take into account as much as possible).
// If "required" is true and the kernel doesn't support all the handledAccessFS rights, then ignores the whole "ruleset" block and the associated "rules".
"required": false,
// optional field, default value (if an handledAccessFS element is unknown to the kernel, then log a warning). Other values could be "silent" and "error".
"unsupported": "warning"
},
"rules": {
"pathBeneath": [
{
"allowedAccess": [
"execute",
"read_file",
"read_dir"
],
"paths": [
"/usr",
"/bin"
],
// optional field, default value (all allowedAccess elements may not be known to the kernel; take into account as much as possible)
// If "required" is true and the kernel doesn't support all the allowedAccess rights, then ignores this rule.
"required": false,
// optional field, default value (if an allowedAccess element is unknown to the kernel, then log a warning)
"unsupported": "warning"
}, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @l0kod for the comments! I do agree this adds more flexibility. Just wondering whether this adds more complexity as well? If supported, the flag check and intersect functions in go-landlock may better be exposed and the logic will be handled in runc. Beyond that, a I'm currently leveraging the best effort mode provided directly in go-landlock (which is a single control point). Any thoughts/comments? @gnoack There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding soft/hard requirements for what the kernel supports: I've been playing with the thought of adding that to go-landlock, but was not convinced whether it would be actually needed. The way I considered implementing it would have been to have one AccessFSSet that is ideal and one AccessFSSet that is the minimum. So instead of .BestEffort(), you'd then call something like .BestEffortButAtLeast(AccessFSSet(...)). (or maybe I can find a nicer name for that...) It would be reasonably easy to add to go-landlock, required changes would be: (a) to add a minHandledAccessFS field to the Config struct, (b) expose a suitable method to set it, and then (c) in restrictPaths(), just before the RulesetAttr{} is created, return an error if the minimal requirement is not fulfilled. I was just not convinced whether there is actually a need for it and it felt like speculative generality... do you think this is a use case that you'd need? Regarding file reparenting: If I understand this correctly, file reparenting is currently forbidden in a Landlock-restricted process in kernel 5.13, and with kernel 5.X you want to permit this in Landlock-restricted processes if they ask for it at enforcement time. Do I understand that correctly? If that is the case, then it sounds like it should maybe not be part of the Config (RulesetAttr), but might logically fit more as an argument to RestrictPaths() (invocation of
The thinking is:
So yes, the above invocation is not compatible with kernel 5.13. The go-landlock library's BestEffort() mode should restrict the strongest rules it can, but it might have to fall back to not enforcing anything at all, if file reparenting is something that the program needs, and if that is not compatible with the current kernel. (I feel like I'm making a few assumptions here, there is a good chance that I'm misunderstanding this... :) let me know) Regarding evolution flexibility of the config: If there will be more fields added to RulesetAttr in the future, it might be easier to think of this as a "permits-more-or-the-same-as" relationship on RulesetAttr structs rather than as a "subset" relationship on AccessFSSets as it's implemented now in go-landlock. So maybe it would be better to have a "ruleset" and "min_ruleset" rather than having a "handled_access_fs" and "min_handled_access_fs" in the ruleset? WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If we really want to support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If this needs to go to
I have the same understanding...
Yes, it makes sense to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In the Rust library, I use ErrorThreshold and set_error_threshold() to be able to require or ignore specific features. By default, it is a best-effort approach.
Yes, I think it will be set in a new bitfield in the ruleset attribute struct. An additional access-right might come as well.
…potentially everything, according to the ruleset.
That is correct. :)
Yes, good idea, it would make sense to have an optional "min_ruleset" or something similar (complementary to "ruleset"). |
||
} | ||
|
||
// LandlockRuleset identifies a set of rules (i.e., actions on objects) that need to be handled. | ||
type LandlockRuleset struct { | ||
// HandledAccessFS is a list of actions that is handled by this ruleset and should then be | ||
// forbidden if no rule explicitly allow them. | ||
HandledAccessFS []LandlockFSAction `json:"handledAccessFS,omitempty" platform:"linux"` | ||
} | ||
|
||
// LandlockRules represents the security policies (i.e., actions allowed on objects). | ||
type LandlockRules struct { | ||
// PathBeneath specifies the file-hierarchy typed rules. | ||
PathBeneath []LandlockRulePathBeneath `json:"pathBeneath,omitempty" platform:"linux"` | ||
} | ||
|
||
// LandlockRulePathBeneath defines the file-hierarchy typed rule that grants the access rights specified by | ||
// `AllowedAccess` to the file hierarchies under the given `Paths`. | ||
type LandlockRulePathBeneath struct { | ||
// AllowedAccess contains a list of allowed filesystem actions for the file hierarchies. | ||
AllowedAccess []LandlockFSAction `json:"allowedAccess,omitempty" platform:"linux"` | ||
// Paths are the files or parent directories of the file hierarchies to restrict. | ||
Paths []string `json:"paths,omitempty" platform:"linux"` | ||
} | ||
|
||
// LandlockFSAction used to specify the FS actions that are handled by a ruleset or allowed by a rule. | ||
type LandlockFSAction string | ||
|
||
// Define actions on files and directories that Landlock can restrict a sandboxed process to. | ||
const ( | ||
LLFSActExecute LandlockFSAction = "execute" | ||
LLFSActWriteFile LandlockFSAction = "write_file" | ||
LLFSActReadFile LandlockFSAction = "read_file" | ||
LLFSActReadDir LandlockFSAction = "read_dir" | ||
LLFSActRemoveDir LandlockFSAction = "remove_dir" | ||
LLFSActRemoveFile LandlockFSAction = "remove_file" | ||
LLFSActMakeChar LandlockFSAction = "make_char" | ||
LLFSActMakeDir LandlockFSAction = "make_dir" | ||
LLFSActMakeReg LandlockFSAction = "make_reg" | ||
LLFSActMakeSock LandlockFSAction = "make_sock" | ||
LLFSActMakeFifo LandlockFSAction = "make_fifo" | ||
LLFSActMakeBlock LandlockFSAction = "make_block" | ||
LLFSActMakeSym LandlockFSAction = "make_sym" | ||
) | ||
|
||
// LinuxCapabilities specifies the list of allowed capabilities that are kept for a process. | ||
// http://man7.org/linux/man-pages/man7/capabilities.7.html | ||
type LinuxCapabilities struct { | ||
|
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.
is this warning something that the kernel/landlock generates? or is the runtime expected to map this?
I'm reading through the kernel docs and not seeing this best-effort parameter.
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.
The "best effort" feature is implemented in user space based on the Landlock ABI level exposed by the kernel. The idea is to use Landlock if it's available and to the extent that it is available, but to fall back to a lower level of protection if the kernel is older.
At the moment, the support matrix for file system access rights is:
And for yet-unreleased kernels:
So the kernel interface to this is:
Also see https://docs.google.com/document/d/1SkFpl_Xxyl4E6G2uYIlzL0gY2PFo-Nl8ikblLvnpvlU/edit for a bit more background and a visualization of how the API evolves