| Commit message (Collapse) | Author | Age | Files | Lines |
| ... | |
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
- 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
|
| |
|
|
|
|
|
| |
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47608
|
| |
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
| |
No functional change intended.
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47606
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
| |
No functional change intended.
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47604
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Allowing to change the rules specification on a jail other than the
requesting's thread one is a security issue, as it will immediately
apply to the jail we inherited from and all its other descendants that
inherit from it.
With this change, setting the 'mdo_rules' sysctl in a jail forces that
jail to no more inherit from its parent.
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47601
|
| |
|
|
|
|
|
|
|
|
|
| |
We are not guaranteed that the 'rules' storage stays stable if we don't
hold the prison lock. For this reason, always copy the specification
string (under the lock).
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47600
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
It isn't really needed, since common jail code destroys jail OSD storage
at jail destruction (via osd_jail_exit()), triggering our destructor
dealloc_osd(). Leveraging this mechanism is arguably even better as it
causes deallocation to always happen without the 'allprison_lock' lock.
While here, make the static definition of 'methods' top-level, renaming
it to 'osd_methods'.
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47599
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Stop recycling the top-level 'struct rules' already assigned to jails.
This considerably simplifies the code, as now changing rules on a jail
amounts to just changing the OSD pointer.
Also, this is to increase potential concurrency in preparation for
incoming fixes about enforcing rules. Indeed, keeping these changes
relatively simple requires rules assigned to a jail to slightly outlive
resetting them, which is most easily done by just operating on pointers
to separate rules objects.
The (negligible) price to pay for this change is that setting rules on
a jail now systematically needs to allocate memory (and also that the
OSD slot needs to be accessed twice, once to get the old rules to free
them and another one to set the rules, which was already the case before
when memory had to be allocated).
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47598
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This generally removes duplicate code and clarifies higher-level
operations, allowing to fix several important bugs.
New (internal) functions:
- ensure_rules(): Ensure that a jail has a populated
'mac_do_osd_jail_slot', and returns the corresponding 'struct rules'.
- dealloc_rules(): Destroy the 'mac_do_osd_jail_slot' slot of a jail.
- set_rules(): Assign already parsed rules to a jail. Leverages
ensure_rules().
- parse_and_set_rules(): Combination of parse_rules() and set_rules().
Bugs fixed in mac_do_prison_set():
- A panic if "mdo" is explicitly passed to JAIL_SYS_NEW but "mdo.rules"
is absent, in which case 'rules_string' wasn't set (setting 'rules' at
this point would do nothing).
- In the JAIL_SYS_NEW case, would release the prison lock and reacquire
it, but still using the same 'rules' pointer that can have been freed
and changed concurrently, as the prison lock is temporary
unlocked. (This is generally a bug of the mac_do_alloc_prison()'s
interface when 'lrp' is not NULL.)
Suppress mac_do_alloc_prison(), as it has the following bugs:
- The interface bug mentioned just above.
- Wrong locking, leading to deadlocks in case of setting jail parameters
multiple times (concurrently or not).
It has been replaced by either parse_and_set_rules(), or by
ensure_rules() directly coupled with prison_unlock().
Rename mac_do_dealloc_prison(), the OSD destructor, to dealloc_osd(),
and make it free the 'struct rules' itself (which was leaking).
While here, in parse_rules(): Clarify the contract by adding comments,
and check (again) for the rules specification's length.
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47597
|
| |
|
|
|
|
|
|
|
| |
While here, rename an internal variable.
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47596
|
| |
|
|
|
|
|
|
|
| |
Instead of fiddling directly with 'pr_mtx'.
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47595
|
| |
|
|
|
|
|
|
|
|
|
|
| |
To simplify, be consistent with the rename 'struct mac_do_rule' =>
'struct rules' and other functions, and because this function is
internal (and thus is never the first mac_do(4)'s function to appear in
a stack trace).
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47594
|
| |
|
|
|
|
|
|
|
| |
To simplify and be consistent with 'struct rule'.
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47593
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This function checks whether a rule applies in the current context,
i.e., if the subject's users/groups match that of the rule. By
contrast, it doesn't check if the rule as specified by the user is valid
(i.e., consistent).
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47592
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Since all callers have to do it, save them that burden and do it in
parse_rules() instead.
While here, replace "strlen(x) == 0" with the simpler and more efficient
"x[0] == '\0'".
Reviewed by: bapt
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47591
|
| |
|
|
|
|
|
|
|
| |
In accordance with style(9).
Reviewed by: bapt, emaste
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47771
|
| |
|
|
|
|
|
|
|
|
|
| |
Needed by the upcoming setcred() system call. More generally, is a step
on the way to support 32-bit compatibility for MAC-related system calls.
Reviewed by: brooks
Approved by: markj (mentor)
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47878
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This is in preparation for enabling the new setcred() system call to set
a process' MAC label.
No functional change (intended).
MFC after: 2 weeks
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46905
|
| |
|
|
|
|
|
|
|
|
| |
Besides simplifying existing code, this will later enable the new
setcred() system call to copy MAC labels.
MFC after: 2 weeks
Approved by: markj (mentor)
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46904
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Do this only when the headers for these functionalities were included
prior to this one. Indeed, if they need to be included, style(9)
mandates they should have been so before this one.
Remove the common MAC sysctl declaration from
<security/mac/mac_internal.h>, as it is now redundant (all its includers
also include <security/mac/mac_policy.h>).
Remove local such declarations from all policies' files.
Reviewed by: jamie
Approved by: markj (mentor)
MFC after: 5 days
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46903
|
| |
|
|
|
|
|
|
|
|
|
| |
To be used by MAC/do.
Reviewed by: jamie
Approved by: markj (mentor)
MFC after: 5 days
Relnotes: yes
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46899
|
| |
|
|
|
|
|
|
|
|
| |
No functional change.
Reviewed by: jamie
Approved by: markj (mentor)
MFC after: 5 days
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D46898
|
| |
|
|
|
|
| |
No functional change intended.
MFC after: 1 week
|
| |
|
|
|
|
|
| |
sysctl(8) prints a newline after the description; the description should
not end with one itself.
Sponsored by: The FreeBSD Foundation
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
sys_setgroups() (and sys_getgroups()) was changed in commit "kern: fail
getgroup and setgroup with negative int" (4bc2174a1b48) to take the
number of groups as an 'int' (for sys_getgroups(), POSIX mandates this
change; for sys_setgroups(), which it does not standardize, it's
arguably for consistency).
All our internal APIs related to groups on 'struct ucred', as well as
related members on the latter, treat that number as an 'int' as well
(and not a 'u_int').
Consequently, to avoid surprises, change kern_setgroups() to behave the
same, and fix audit_arg_groupset() accordingly. With that change,
everything is handled with signed integers internally.
Update sanity checks accordingly.
Reviewed by: mhorne
Approved by: markj (mentor)
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D46912
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously, the rule validation only checked the primary GID (cr_gid).
This caused issues when applying GID-based rules, as users with matching
secondary groups were not considered valid. This patch modifies both
functions to iterate through all groups in cr_groups to ensure all group
memberships are considered when validating GID-based rules.
For example, a user's primary group is staff (20) and they are also in
the wheel (0) group, this change allows the rule gid=0:any to enable
them to run commands as any user.
Reviewed by: delphij (earlier version), bapt
Differential Revision: https://reviews.freebsd.org/D47304
|
| |
|
|
|
|
|
|
|
| |
so_peerlabel can only be used when the socket is not listening.
Reviewed by: markj
MFC after: 1 week
Sponsored by: Netflix, Inc.
Differential Revision: https://reviews.freebsd.org/D46755
|
| |
|
|
|
|
|
|
|
|
| |
Whenever mac_syncache_init() returns an error, ensure that
*label = NULL. This simplifies the error handling by the caller.
Reviewed by: rscheff
MFC after: 1 week
Sponsored by: Netflix, Inc.
Differential Revision: https://reviews.freebsd.org/D46701
|
| |
|
|
|
|
|
|
| |
NDFREE_PNBUF should be called after checking the return value of
vn_open(), and should only be called once.
Reviewed by: imp, zlei, Kornel Dulęba <mindal@semihalf.com>, Elliott Mitchell
Pull Request: https://github.com/freebsd/freebsd-src/pull/1338
|
| |
|
|
|
|
|
|
|
|
| |
Add a priv_check for PRIV_PROC_MEM_WRITE which will be blocked
by mac_veriexec if being enforced, unless the process has a maclabel
to grant priv.
Reviewed by: stevek
Sponsored by: Juniper Networks, Inc.
Differential Revision: https://reviews.freebsd.org/D46692
|
| |
|
|
|
| |
This fixed sshd not able to call restore_uid when MAC/do policy is
loaded
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This policy enables a user to become another user without having to be
root (hence no setuid binary). it is configured via rules using sysctl
security.mac.do.rules
For example:
security.mac.do.rules=uid=1001:80,gid=0:any
The above rule means the user identifier by the uid 1001 is able to
become user 80
Any user of the group 0 are allowed to become any user on the system.
The mdo(1) utility expects the MAC/do policy to be installed and its
rules defined.
Reviewed by: des
Differential Revision: https://reviews.freebsd.org/D45145
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Apply the following automated changes to try to eliminate
no-longer-needed sys/cdefs.h includes as well as now-empty
blank lines in a row.
Remove /^#if.*\n#endif.*\n#include\s+<sys/cdefs.h>.*\n/
Remove /\n+#include\s+<sys/cdefs.h>.*\n+#if.*\n#endif.*\n+/
Remove /\n+#if.*\n#endif.*\n+/
Remove /^#if.*\n#endif.*\n/
Remove /\n+#include\s+<sys/cdefs.h>\n#include\s+<sys/types.h>/
Remove /\n+#include\s+<sys/cdefs.h>\n#include\s+<sys/param.h>/
Remove /\n+#include\s+<sys/cdefs.h>\n#include\s+<sys/capsicum.h>/
Sponsored by: Netflix
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
The get operations change the data pointed to by the structure, but do
not update the contents of the struct.
Mark the struct mac arguments of mac_[gs]etsockopt_*label() and
mac_check_structmac_consistent() const to prevent this from changing
in the future.
Reviewed by: markj
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D14488
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The loader tunable 'security.mac.veriexec.block_unlink' has been
already flagged with CTLFLAG_RDTUN, no need to re-fetch it with
TUNABLE_INT_FETCH.
While here move the definition of sysctl knob out of function body,
which is more common in FreeBSD.
No functional change intended.
Reviewed by: stevek
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D42132
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Use priv_check_cred() with a new privilege (PRIV_SEEJAILPROC) instead of
explicitly testing for UID 0 (the former has been the rule for almost 20
years).
As a consequence, cr_canseejailproc() now abides by the
'security.bsd.suser_enabled' sysctl and MAC policies.
Update the MAC policies Biba and LOMAC, and prison_priv_check() so that
they don't deny this privilege. This preserves the existing behavior
(the 'root' user is not restricted, even when jailed, unless
'security.bsd.suser_enabled' is not 0) and is consistent with what is
done for the related policies/privileges (PRIV_SEEOTHERGIDS,
PRIV_SEEOTHERUIDS).
Reviewed by: emaste (earlier version), mhorne
MFC after: 2 weeks
Sponsored by: Kumacom SAS
Differential Revision: https://reviews.freebsd.org/D40626
|
| |
|
|
|
|
| |
Use `if_t` instead of `struct ifnet *`, and if_name() accessor.
Sponsored by: Juniper Networks, Inc.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This module allows controlled privilege escallation via mac labels
securely associated with a process via mac_veriexec.
There are over 700 PRIV_* but we can compress many of them into
a single GBL_* thus constraining the size of gbl labels.
The goal is to allow a daemon to run as an unprivileged process while
still being able a set of privileged operations needed.
We add APIs to libveriexec so that userland processes can check labels
and an exec_script API that allows a suitably labeled process to run
something like a python interpreter directly if necessary;
overcomming the 'indirect' flag applied to the interpreter.
Add -l option to sbin/veriexec to report labels.
Reviewed by: stevek
Sponsored by: Juniper Networks, Inc.
Differential Revision: https://reviews.freebsd.org/D41431
|
| |
|
|
| |
Remove /^[\s*]*__FBSDID\("\$FreeBSD\$"\);?\s*\n/
|
| |
|
|
| |
Remove /^\s*\*\n \*\s+\$FreeBSD\$$\n/
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
The mac_ipacl policy module enables fine-grained control over IP address
configuration within VNET jails from the base system.
It allows the root user to define rules governing IP addresses for
jails and their interfaces using the sysctl interface.
Requested by: multiple
Sponsored by: Google, Inc. (GSoC 2019)
MFC after: 2 months
Reviewed by: bz, dch (both earlier versions)
Differential Revision: https://reviews.freebsd.org/D20967
|