| f01d26de | 01-Apr-2025 |
Olivier Certner <olce@FreeBSD.org> |
MAC/do: Rules: <from> and <to> parts now to be separated by '>'
Previously, we would accept only ':' as the separator, which makes
parsing of the rule specification harder for humans, especially tho
MAC/do: Rules: <from> and <to> parts now to be separated by '>'
Previously, we would accept only ':' as the separator, which makes
parsing of the rule specification harder for humans, especially those
people that are used to UNIX systems where ':' is used as the separator
in PATH. With ':', the <from> and <to> parts can look like two
different elements that are unrelated, especially to these eyes.
Change parse_single_rule() so that '>' is also accepted as a separator
between <from> and <to>, and promote it as the one to use. During
a transition period, we will still allow the use of ':' for backwards
compatibility.
The manual page update comes from separate revision D49628. ':' has
been completely removed from it on purpose.
Reviewed by: bapt, manpages (ziaee)
MFC after: 5 days
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D49627
show more ...
|
| 4a03b645 | 12-Nov-2024 |
Olivier Certner <olce@FreeBSD.org> |
MAC/do: parse_rules(): Tolerate blanks around tokens
To this end, we introduce the strsep_noblanks() function, designed to be
a drop-in replacement for strstep(), and use it in place of the latter.
MAC/do: parse_rules(): Tolerate blanks around tokens
To this end, we introduce the strsep_noblanks() function, designed to be
a drop-in replacement for strstep(), and use it in place of the latter.
We had taken care of calling strsep() even when the remaining sub-string
was not delimited (i.e., with empty string as its second argument), so
this commit only has mechanical replacements of existing calls.
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47623
show more ...
|
| 8f7e8726 | 22-Jul-2024 |
Olivier Certner <olce@FreeBSD.org> |
MAC/do: Interpret the new rules specification; Monitor setcred()
TL;DR:
Now monitor setcred() calls, and reject or grant them according to the
new rules specification.
Drop monitoring setuid() and
MAC/do: Interpret the new rules specification; Monitor setcred()
TL;DR:
Now monitor setcred() calls, and reject or grant them according to the
new rules specification.
Drop monitoring setuid() and setgroups(). As previously explained in
the commit introducing the setcred() system call, MAC/do must know the
entire new credentials while the old ones are still available to be able
to approve or reject the requested changes. To this end, the chosen
approach was to introduce a new system call, setcred(), instead of
modifying existing ones to be able to participate in a "prepare than
commit"-like protocol.
******
The MAC framework typically calls several hooks of its registered
policies as part of the privilege checking/granting process. Each
system call calls some dedicated hook early, to which it usually passes
the same arguments it received, whose goal is to forcibly deny access to
the functionality when needed (i.e., a single deny by any policy
globally denies the access). Then, the system call usually calls
priv_check() or priv_check_cred() an unspecified number of times, each
of which may trigger calls to two generic MAC hooks. The first such
call is to mac_priv_check(), and always happens. Its role is to deny
access early and forcibly, as can be done also in system calls'
dedicated early hooks (with different reach, however). The second,
mac_priv_grant(), is called only if the priv_check*() and
prison_priv_check() generic code doesn't handle the request by itself,
i.e., doesn't explicitly grant access (to the super user, or to all
users for a few specific privileges). It allows any single policy to
grant the requested access (regardless of whether the other policies do
so or not).
MAC/do currently only has an effect on processes spawned from the
'/usr/bin/mdo' executable. It implements all setcred() hooks, called
via mac_cred_setcred_enter(), mac_cred_check_setcred() and
mac_cred_setcred_exit(). In the first one, implemented in
mac_do_setcred_enter(), it checks if MAC/do has to apply to the current
process, allocates (or re-uses) per-thread data to be later used by the
other hooks (those of setcred() and the mac_priv_grant() one, called by
priv_check*()) and fills them with the current context (the rules to
apply). This is both because memory allocations cannot be performed
while holding the process lock and to ensure that all hooks called by
a single setcred() see the same rules to apply (not doing this would be
a security hazard as rules are concurrently changed by the
administrator, as explained in more details below). In the second one
(implemented by mac_do_check_setcred()), it stores in MAC/do's
per-thread data the new credentials. Indeed, the next MAC/do's hook
implementation to be called, mac_do_priv_grant() (implementing the
mac_priv_grant() hook) must have knowledge of the new credentials that
setcred() wants to install in order to validate them (or not), which the
MAC framework can't provide as the priv_check*() API only passes the
current credentials and a specific privilege number to the
mac_priv_check() and mac_priv_grant() hooks. By contrast, the very
point of MAC/do is to grant the privilege of changing credentials not
only based on the current ones but also on the seeked-for ones.
The MAC framework's constraints that mac_priv_grant() hooks are called
without context and that MAC modules must compose (each module may
implement any of the available hooks, and in particular those of
setcred()) impose some aspects of MAC/do's design. Because MAC/do's
rules are tied to jails, accessing the current rules requires holding
the corresponding jail's lock. As other policies might try to grab the
same jail's lock in the same hooks, it is not possible to keep the
rules' jail's lock between mac_do_setcred_enter() and
mac_do_priv_grant() to ensure that the rules are still alive. We have
thus augmented 'struct rules' with a reference count, and its lifecyle
is now decoupled from being referenced or not by a jail. As a thread
enters mac_cred_setcred_enter(), it grabs a hold on the current rules
and keeps a pointer to them in the per-thread data. In its
mac_do_setcred_exit(), MAC/do just "frees" the per-thread data, in
particular by dropping the referenced rules (we wrote "frees" within
guillemets, as in fact the per-thread structure is reused, and only
freed when a thread exits or the module is unloaded).
Additionally, ensuring that all hooks have a consistent view of the
rules to apply might become crucial if we augment MAC/do with forceful
access denial policies in the future (i.e., policies that forcibly
disable access regardless of other MAC policies wanting to grant that
access). Indeed, without the above-mentioned design, if newly installed
rules start to forcibly deny some specific transitions, and some thread
is past the mac_cred_check_setcred() hook but before the
mac_priv_grant() one, the latter may grant some privileges that should
have been rejected first by the former (depending on the content of
user-supplied rules).
A previous version of this change used to implement access denial
mandated by the '!' and '-' GID flags in mac_do_check_setcred() with the
goal to have this rejection prevail over potential other MAC modules
authorizing the transition. However, this approach had two drawbacks.
First, it was incompatible both conceptually and in the current
implementation with multiple rules being treated as an inclusive
disjunction, where any single rule granting access is enough for MAC/do
to grant access. Explicit denial requested by one matching rule could
prevent another rule from granting access. The implementation could
have been fixed, but the conflation of rules being considered as
disjoint for explicit granting but conjunct for forced denial would have
remained. Second, MAC/do applies only to processes spawned from
a particular executable, and imposing system-wide restrictions on only
these processes is conceptually strange and probably not very useful.
In the end, we moved the implementation of explicit access denial into
mac_do_priv_grant(), along with the interpretation of other target
clauses.
The separate definition of 'struct mac_do_data_header' may seem odd, as
it is only used in 'struct mac_do_setcred_data'. It is a remnant of an
earlier version that was not using setcred(), but rather implemented
hooks for setuid() and setgroups(). We however kept it, as it clearly
separates the machinery to pass data from dedicated system call hooks to
priv_grant() from the actual data that MAC/do needs to monitor a call to
setcred() specifically. It may be useful in the future if we evolve
MAC/do to also grant privileges through other system calls (each seen as
a complete credentials transition on its own).
The target supplementary groups are checked with merge-like algorithms
leveraging the fact that all supplementary groups in credentials
('struct ucred') and in each rule ('struct rule') are sorted, avoiding
to start a binary search for each considered GID which is asymptotically
more costly. All access granting/denial is thus at most linear and in
at most the sum of the number of requested groups, currently held ones
and those contained in the rule, per applicable rule. This should be
enough in all practical cases. There is however still room for more
optimizations, without or with changes in rules' data structures, if the
need ever arises.
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47620
show more ...
|
| 6c3def74 | 05-Jul-2024 |
Olivier Certner <olce@FreeBSD.org> |
MAC/do: Support multiple users and groups as single rule's targets
Supporting group targets is a requirement for MAC/do to be able to
enforce a limited set of valid new groups passed to setgroups().
MAC/do: Support multiple users and groups as single rule's targets
Supporting group targets is a requirement for MAC/do to be able to
enforce a limited set of valid new groups passed to setgroups().
Additionally, it must be possible for this set of groups to also depend
on the target UID, since users and groups are quite tied in UNIX (users
are automatically placed in only the groups specified through
'/etc/passwd' (primary group) and '/etc/group' (supplementary ones)).
These requirements call for a re-design of the specification of the
rules specification string and of 'struct rule'.
A rules specification string is now a list of rules separated by ';'
(instead of ','). One rule is still composed of a "from" part and
a "to" (or "target") part, both being separated by ':' (as before).
The first part, "from", is matched against the credentials of the
process calling setuid()/setgroups(). Its specification remains
unchanged: It is a '<type>=<id>' clause, where <type> is either "uid" or
"gid" and <id> an UID or GID.
The second part, "to", is now a comma-separated (',') list of
'<flags><type>=<id>' clauses similar to that of the "from" part, with
the extensions that <id> may also be "*" or "any" or ".", and that
<flags> may contain at most one of the '+', '-' and '!' characters when
<type> is GID. "*" and "any" both designate any ID for the <type>, and
are aliases to each other. In front of them, only the "+" flag is
allowed (in addition to the previous rules). "." designates the
process' current IDs for the <type>, as explained below.
For GIDs, an absence of flag indicates that the specified GID is allowed
as the real, effective and/or saved GIDs (the "primary" groups).
Conversely, the presence of any allowed flag indicates that the
specification concerns supplementary groups. The '+' flag in front of
"gid" indicates that the ID is allowed as a supplementary group. The
'!' flag indicates that the ID is mandatory, i.e., must be listed in the
supplementary groups. The '-' flag indicates that the GID must not be
listed in the supplementary groups. A specification with '-' is only
useful in conjunction with a '+'-tagged specification where only one of
them has <id> ".", or if other MAC policies are loaded that would give
access to other, unwanted groups.
"." indicates some ID that the calling process already has on privilege
check. For type "uid", it designates any of the real, effective or
saved UIDs. For type "gid", its effect depends on the presence of one
of the '+', '-' or '!' flags. If no flag is present, it designates any
of the real, effective or saved GIDs. If one is present, it designates
any of the supplementary groups.
If the "to" part doesn't specify any explicit UID, any of the UIDs of
the calling process is implied (it is as if "uid=." had been specified).
Similarly, if it doesn't specify any explicit GID, "gid=.,!gid=." is
assumed, meaning that all the groups of the calling process are implied
and must be present. More precisely, each of the desired real,
effective and saved GIDs must be one of the current real, effective or
saved GID, whereas all others (the supplementary ones) must be the same
as those that are current.
No two clauses in a single "to" list may display the same <id>, except
for GIDs but only if, each time the same <id> appears, it does so with
a different flag (no flag counting as a separate flag) and all the
specified flags are not contradictory (e.g., it is possible to have the
same GID appear with no flag and the "+" flag, but the same GID with
both "+" and "-" will be rejected).
'struct rule' now holds arrays of UIDs (field 'uids') and GIDs (field
'gids') that are admissible as targets, with accompanying flags (such as
MDF_SUPP_MUST, representing the '!' flag). Some flags are also held by
ID type, including flags associated to individual IDs, as MDF_CURRENT in
these flags stands for the process being privilege-checked's current
IDs, to which ID flags apply. As a departure from this scheme, "*" or
"any" as <id> for GIDs is either represented by MDF_ANY or MDF_ANY_SUPP.
This is to make it coexist with a "."/MDF_CURRENT specification for the
other category of groups (among primary and supplementary groups), which
needs to be qualified by the usual GID flags.
This commit contains only the changes to parse the new rules and to
build their representation. The privilege granting part is not fixed
here, beyond what making compilation work requires (and, in preparation
for some subsequent commit, minimal adaptations to the matching logic in
check_setuid()).
Approved by: markj (mentor)
Relnotes: yes
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47616
show more ...
|
| 11eb3295 | 03-Jul-2024 |
Olivier Certner <olce@FreeBSD.org> |
MAC/do: jail_check()/jail_set(): Revamp
Handle JAIL_SYS_DISABLE the same as JAIL_SYS_NEW with an empty rules
specification, coherently with jail_get(). Also accept JAIL_SYS_DISABLE
in "mac.do" with
MAC/do: jail_check()/jail_set(): Revamp
Handle JAIL_SYS_DISABLE the same as JAIL_SYS_NEW with an empty rules
specification, coherently with jail_get(). Also accept JAIL_SYS_DISABLE
in "mac.do" without "mac.do.rules" being specified.
The default value for "mac.do", if not passed explicitly, is either
JAIL_SYS_NEW if "mac.do.rules" is present and non-empty, or
JAIL_SYS_DISABLE if present and empty or not present.
Perform all cheap sanity checks in jail_check(), and have these
materialized as well in jail_set() under INVARIANTS. Cheap checks are
type and coherency checks between the values of "mac.do" and
"mac.do.rules". They don't include parsing the "mac.do.rules" string
but just checking its length (when applicable). In a nutshell,
JAIL_SYS_DISABLE and JAIL_SYS_INHERIT are allowed iff "mac.do.rules"
isn't specified or is with an empty string, and JAIL_SYS_NEW is allowed
iff "mac.do.rules" is specified (the latter may be empty, in which case
this is equivalent to JAIL_SYS_DISABLE).
Normally, vfs_getopts() is the function to use to read string options.
Because we need the length of the "mac.do.rules" string to check it, in
order to avoid double search within jail options in jail_check(), we use
vfs_getopt() instead, but perform some additional checks afterwards (the
same as those performed by vfs_getopts()).
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47610
show more ...
|