MAC/do: Fix a compilation warning about an unused function

grant_supplementary_group_from_flags() had been used in previous
versions of the recent changes, but recently has not been needed
anymore.

MAC/do: Fix a compilation warning about an unused function

grant_supplementary_group_from_flags() had been used in previous
versions of the recent changes, but recently has not been needed
anymore. It has been kept around just in case deliberately, by analogy
with grant_primary_group_from_flags() (this one still being used).

show more ...

MAC/do: Update copyright

Approved by: emaste (mentor)
Sponsored by: The FreeBSD Foundation

MAC/do: Apply a rule on real UID/GID instead of effective ones

We intend MAC/do to authorize transitions based on the "real" identity
information of the calling process, rather than transiently-acqu

MAC/do: Apply a rule on real UID/GID instead of effective ones

We intend MAC/do to authorize transitions based on the "real" identity
information of the calling process, rather than transiently-acquired
effective IDs.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47845

show more ...

MAC/do: Convert internal TAILQs to STAILQs

We only browse these forward and never need to remove arbitrary elements
from them.

No functional change (intended).

Reviewed by: bapt, emaste
Approve

MAC/do: Convert internal TAILQs to STAILQs

We only browse these forward and never need to remove arbitrary elements
from them.

No functional change (intended).

Reviewed by: bapt, emaste
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47624

show more ...

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

MAC/do: toast_rules(): Minor simplification

Use the most common pattern to browse and delete elements of a list, as it reads quicker.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored b

MAC/do: toast_rules(): Minor simplification

Use the most common pattern to browse and delete elements of a list, as it reads quicker.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47622

show more ...

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

MAC/do: Introduce rules reference counting

This is going to be used in subsequent commits to keep rules alive even
if disconnected from their jail in the meantime. We'll indeed have to
release the

MAC/do: Introduce rules reference counting

This is going to be used in subsequent commits to keep rules alive even
if disconnected from their jail in the meantime. We'll indeed have to
release the prison lock between two uses (outright rejection, final
granting) where the rules must absolutely stay the same for security reasons.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47619

show more ...

MAC/do: Output errors when parsing rules

So that administrators can more easily know what the problem is with the
rules they are trying to set.

The new sysctl 'security.mac.do.print_parse_error' co

MAC/do: Output errors when parsing rules

So that administrators can more easily know what the problem is with the
rules they are trying to set.

The new sysctl 'security.mac.do.print_parse_error' controls whether
trying to set sysctl 'security.mac.do.rules' with invalid rules triggers
printing of the error on the system console.

Setting jail parameters directlty reports an error to the calling
process thanks to the VFS options mechanism used by the jail machinery,
so is not controlled by the new sysctl setting.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47617

show more ...

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

MAC/do: Rename private OSD slot by removing 'mac_do_' prefix

This variable is static and holds the OSD slot number for jails that
MAC/do uses to store rules.

In the same vein as previous renames, s

MAC/do: Rename private OSD slot by removing 'mac_do_' prefix

This variable is static and holds the OSD slot number for jails that
MAC/do uses to store rules.

In the same vein as previous renames, simplify it by removing the
redundant prefix, as this name cannot appear in code outside of
'mac_do.c', nor in stack traces on panic.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47772

show more ...

MAC/do: Ease input/output of ID types

Have a static constant array mapping numerical ID types to their
canonical representations ('id_type_to_str').

New parse_id_type() that parses a type thanks to

MAC/do: Ease input/output of ID types

Have a static constant array mapping numerical ID types to their
canonical representations ('id_type_to_str').

New parse_id_type() that parses a type thanks to 'id_type_to_str' and
with a special case to accept also 'any'.

Have parse_rule_element() use parse_id_type(). A later commit will add
a second call to the latter for the destination ID.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47615

show more ...

MAC/do: Better parsing for IDs (strtoui_strict())

Introduce strtoui_strict(), which signals an error on overflow contrary
to the in-kernel strto*() family of functions which have no 'errno' to
set a

MAC/do: Better parsing for IDs (strtoui_strict())

Introduce strtoui_strict(), which signals an error on overflow contrary
to the in-kernel strto*() family of functions which have no 'errno' to
set and thus do not allow callers to distinguish a genuine maximum value
on input and overflow.

It is built on top of strtoq() and the 'quad_t' type in order to achieve
this distinction and also to still support negative inputs with the
usual meaning for these functions. See the introduced comments for more
details.

Use strtoui_strict() to read IDs instead of strtol().

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47614

show more ...

MAC/do: 'struct rule': IDs and types as 'u_int', rename fields

This is in preparation for introducing a common conversion function for
IDs and to simplify code a bit by removing the from-IDs union a

MAC/do: 'struct rule': IDs and types as 'u_int', rename fields

This is in preparation for introducing a common conversion function for
IDs and to simplify code a bit by removing the from-IDs union and not
having to introduce a new one for to-IDs in a later commit.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47613

show more ...

MAC/do: parse_rule_element(): Bug in parsing the origin ID

The ID field was allowed to be empty, which would be then parsed as 0 by
strtol(). There remains bugs in this function, where parsing for

MAC/do: parse_rule_element(): Bug in parsing the origin ID

The ID field was allowed to be empty, which would be then parsed as 0 by
strtol(). There remains bugs in this function, where parsing for from-
or to- IDs accepts spaces and produces 0, but this will conveniently be
fixed in a later commit introducing strtoui_strict().

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47612

show more ...

MAC/do: parse_rule_element(): Style, more clarity

Add newlines to separate logical blocks. Remove braces around 'if's
non-compound substatements.

No functional change (intended).

Reviewed by:

MAC/do: parse_rule_element(): Style, more clarity

Add newlines to separate logical blocks. Remove braces around 'if's
non-compound substatements.

No functional change (intended).

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47611

show more ...

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

MAC/do: Fix jail_get() (PR_METHOD_GET)

- Properly fill 'jsys' before copying it out (we would leak bytes from
the kernel stack). When the current jail has its own 'struct rules',
set it to the

MAC/do: Fix jail_get() (PR_METHOD_GET)

- Properly fill 'jsys' before copying it out (we would leak bytes from
the kernel stack). When the current jail has its own 'struct rules',
set it to the special value JAIL_SYS_DISABLE if it in fact holds no
rules.
- Don't forget to unlock the jail holding rules on error.
- Correctly return errors.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47609

show more ...

MAC/do: Sysctl knobs/jail parameters under MAC's common nodes

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freeb

MAC/do: Sysctl knobs/jail parameters under MAC's common nodes

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47608

show more ...

MAC/do: Prefix internal functions used as hooks/callbacks

So that we immediately know whether a kernel stack involves MAC/do.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The

MAC/do: Prefix internal functions used as hooks/callbacks

So that we immediately know whether a kernel stack involves MAC/do.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47607

show more ...

MAC/do: Re-order jail methods more logically, rename

No functional change intended.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision:

MAC/do: Re-order jail methods more logically, rename

No functional change intended.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47606

show more ...

MAC/do: parse_rule_element(): Fix a panic, harden, simplify

The panic is caused by dereferencing 'element' at a point where it can
be NULL (if string ends at the ':').

Harden and simplify by enforc

MAC/do: parse_rule_element(): Fix a panic, harden, simplify

The panic is caused by dereferencing 'element' at a point where it can
be NULL (if string ends at the ':').

Harden and simplify by enforcing the control flow rule in this function
that jumping to the end is reserved for error cases.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47605

show more ...

MAC/do: Move destroy() to a better place

No functional change intended.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://re

MAC/do: Move destroy() to a better place

No functional change intended.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47604

show more ...

MAC/do: Remove the 'prison0' special cases in the common paths

The rules on 'prison0' are initialized in init(), now using
set_empty_rules().

Until the jail is destroyed, they can never be uninitia

MAC/do: Remove the 'prison0' special cases in the common paths

The rules on 'prison0' are initialized in init(), now using
set_empty_rules().

Until the jail is destroyed, they can never be uninitialized by a call
to osd_jail_del(), since the only chain to call it is
mac_do_prison_set() -> remove_rules() -> osd_jail_del(), and
mac_do_prison_set() (method PR_METHOD_SET) can never be called on
'prison0'. This guarantees that find_rules() always find a valid
'rules' pointer to return.

There's no need to do anything special in destroy() for 'prison0', as
osd_jail_deregister() now takes care of it.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47603

show more ...

MAC/do: Enable changing 'security.mac.do.rules' from a jail

Now that sysctl_rules() has been fixed to behave.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundat

MAC/do: Enable changing 'security.mac.do.rules' from a jail

Now that sysctl_rules() has been fixed to behave.

Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47602

show more ...