Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 78 additions & 2 deletions config.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,27 @@ For Linux-based systems, the `process` object supports the following process-spe
This is a per-process setting, where as [`disableOOMKiller`](config-linux.md#memory) is scoped for a memory cgroup.
For more information on how these two settings work together, see [the memory cgroup documentation section 10. OOM Contol][cgroup-v1-memory_2].
* **`selinuxLabel`** (string, OPTIONAL) specifies the SELinux label for the process.
For more information about SELinux, see [SELinux documentation][selinux].
For more information about SELinux, see [SELinux documentation][selinux].
* **`landlock`** (object, OPTIONAL) specifies the Landlock unprivileged access control settings for the container process.
Note that `noNewPrivileges` must be set to true to use this feature.
For more information about Landlock, see [Landlock documentation][landlock].
`landlock` contains the following properties:

* **`ruleset`** (object, OPTIONAL) the `ruleset` field identifies a set of rules (i.e., actions on objects) that need to be handled (i.e., restricted).
The `ruleset` currently contains the following types:
* **`handledAccessFS`** (array of strings, OPTIONAL) is an array of FS typed actions that are handled by a ruleset.
If no rule explicitly allow them, they should then be forbidden.
* **`rules`** (object, OPTIONAL) the `rules` field specifies the security policies (i.e., actions allowed on objects) to be added to an existing ruleset.
The `rules` currently contains the following types:
* **`pathBeneath`** (array of objects, OPTIONAL) is an array of the file-hierarchy typed rules.
Entries in the array contain the following properties:
* **`allowedAccess`** (array of strings, OPTIONAL) is an array of FS typed actions that are allowed by a rule.
* **`paths`** (array of strings, OPTIONAL) is an array of files or parent directories of the file hierarchies to restrict.
* **`disableBestEffort`** (bool, OPTIONAL) the `disableBestEffort` field disables the best-effort security approach for Landlock access rights.
Copy link
Member

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.

Copy link

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:

  • ABI v1 (kernel 5.13) supports all rights up to MAKE_SYM
  • ABI v2 (kernel 5.19) supports additionally the REFER right

And for yet-unreleased kernels:

  • ABI v3 (in upcoming kernel 6.2, if everything goes well) will support additionally the TRUNCATE right

So the kernel interface to this is:

  • use a special flag for landlock_create_ruleset(2) to figure out the ABI level
  • then use the Landlock API with the right access rights that are available at that level

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

This is for conditions when the Landlock access rights explicitly configured by the container are not supported or available in the running kernel.
If the best-effort security approach is enabled (`false`), the runtime SHOULD enforce the strongest rules configured up to the current kernel support, and only be [logged as a warning](runtime.md#warnings) for those not supported.
If disabled (`true`), the runtime MUST [generate an error](runtime.md#errors) if one or more rules specified by the container is not supported.
Default is `false`, i.e., following a best-effort security approach.

### <a name="configUser" />User

Expand Down Expand Up @@ -258,6 +278,61 @@ _Note: symbolic name for uid and gid, such as uname and gname respectively, are
],
"apparmorProfile": "acme_secure_profile",
"selinuxLabel": "system_u:system_r:svirt_lxc_net_t:s0:c124,c675",
"landlock": {
"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"
]
},
"rules": {
"pathBeneath": [
{
"allowedAccess": [
"execute",
"read_file",
"read_dir"
],
"paths": [
"/usr",
"/bin"
]
},
{
"allowedAccess": [
"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"
],
"paths": [
"/tmp"
]
}
]
},
"disableBestEffort": false
},
"noNewPrivileges": true,
"capabilities": {
"bounding": [
Expand Down Expand Up @@ -978,7 +1053,8 @@ Here is a full example `config.json` for reference.

[apparmor]: https://wiki.ubuntu.com/AppArmor
[cgroup-v1-memory_2]: https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt
[selinux]:http://selinuxproject.org/page/Main_Page
[selinux]: http://selinuxproject.org/page/Main_Page
[landlock]: https://landlock.io
[no-new-privs]: https://www.kernel.org/doc/Documentation/prctl/no_new_privs.txt
[proc_2]: https://www.kernel.org/doc/Documentation/filesystems/proc.txt
[umask.2]: http://pubs.opengroup.org/onlinepubs/009695399/functions/umask.html
Expand Down
14 changes: 14 additions & 0 deletions schema/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,20 @@
"selinuxLabel": {
"type": "string"
},
"landlock": {
"type": "object",
"properties": {
"ruleset": {
"$ref": "defs.json#/definitions/LandlockRuleset"
},
"rules": {
"$ref": "defs.json#/definitions/LandlockRules"
},
"disableBestEffort": {
"type": "boolean"
}
}
},
"noNewPrivileges": {
"type": "boolean"
},
Expand Down
57 changes: 57 additions & 0 deletions schema/defs.json
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,63 @@
},
"annotations": {
"$ref": "#/definitions/mapStringString"
},
"LandlockFSAction": {
"type": "string",
"enum": [
"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"
]
},
"ArrayOfLandlockFSActions": {
"type": "array",
"items": {
"$ref": "#/definitions/LandlockFSAction"
}
},
"LandlockRuleset": {
"type": "object",
"properties": {
"handledAccessFS": {
"$ref": "#/definitions/ArrayOfLandlockFSActions"
}
}
},
"LandlockRulePathBeneath": {
"type": "object",
"properties": {
"allowedAccess": {
"$ref": "#/definitions/ArrayOfLandlockFSActions"
},
"paths": {
"$ref": "#/definitions/ArrayOfStrings"
}
}
},
"ArrayOfLandlockRulePathBeneaths": {
"type": "array",
"items": {
"$ref": "#/definitions/LandlockRulePathBeneath"
}
},
"LandlockRules": {
"type": "object",
"properties": {
"pathBeneath": {
"$ref": "#/definitions/ArrayOfLandlockRulePathBeneaths"
}
}
}
}
}
58 changes: 58 additions & 0 deletions specs-go/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also adding a best-effort control here, see if it makes sense to you @l0kod @gnoack. Thanks!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me!

Copy link

Choose a reason for hiding this comment

The 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).
This would be complementary to a soft error (by default, best-effort: only log a warning) or a hard error (don't launch the container) if the feature is not supported. Being able to have no warning at all (not by default) could be useful too.

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"
        },

Copy link
Contributor Author

@kailun-qin kailun-qin Sep 3, 2021

Choose a reason for hiding this comment

The 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 RestrictPathsNoCheck API may need to be added as the checks in the current implementations are redundant in go-landlock.

I'm currently leveraging the best effort mode provided directly in go-landlock (which is a single control point). Any thoughts/comments? @gnoack

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The 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 landlock_add_rule), like this:

err := landlock.V99.BestEffort().RestrictPaths(
  variousPaths...,
  landlock.EnableFileReparenting(params...),
)

The thinking is:

  • the Config (landlock.V99, RulesetAttr) describes what should be forbidden (and with abi→∞, thats everything :))
  • the arguments to RestrictPaths() (landlock_add_rule) describe the exceptions to what is forbidden (the operations that the process intends to do)

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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).

If we really want to support a soft requirement (by default) or a hard requirement in ruleset to specify at least some features shall be supported, the format would be something similar to what @gnoack suggested - ruleset and min_ruleset (where one ruleset not be enough).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simplified version of the proposition would be to only add required and unsupported (names may not be clear enough) to the ruleset block. I don't know about the evolution flexibility of this specification though.

This also makes me wonder about rules not being nested into ruleset. That would make more sense. It would be lighter to merge landlock and ruleset into landlockRuleset (the same way there is apparmorProfile and selinuxLabel). What do you think?

For rules not being nested into ruleset, they are currenlty being errored out (and should be considered as misconfigured maybe?). In this case, I think it's fine to only control in the ruleset block (simplified version).

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

If this needs to go to rules as well (as @l0kod suggested in the above with sample), then this would not be enough.

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 landlock_add_rule), like this:

err := landlock.V99.BestEffort().RestrictPaths(
  variousPaths...,
  landlock.EnableFileReparenting(params...),
)

The thinking is:

  • the Config (landlock.V99, RulesetAttr) describes what should be forbidden (and with abi→∞, thats everything :))
  • the arguments to RestrictPaths() (landlock_add_rule) describe the exceptions to what is forbidden (the operations that the process intends to do)

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)

I have the same understanding...

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?

Yes, it makes sense to me.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So instead of .BestEffort(), you'd then call something like .BestEffortButAtLeast(AccessFSSet(...)). (or maybe I can find a nicer name for that...)

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.

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?

Yes, I think it will be set in a new bitfield in the ruleset attribute struct. An additional access-right might come as well.

  • the Config (landlock.V99, RulesetAttr) describes what should be forbidden (and with abi→∞, thats everything :))

potentially everything, according to the ruleset.

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)

That is correct. :)

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?

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 {
Expand Down