Page MenuHomeFreeBSD
Differential D41431

Add mac_grantbylabel
ClosedPublic
Actions

Authored by sjg on Aug 11 2023, 10:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 22, 10:19 PM
Unknown Object (File)
Thu, Mar 20, 5:37 AM
Unknown Object (File)
Mon, Mar 17, 12:09 PM
Unknown Object (File)
Tue, Mar 11, 3:00 AM
Unknown Object (File)
Sun, Mar 9, 9:25 AM
Unknown Object (File)
Wed, Feb 26, 4:42 PM
Unknown Object (File)
Feb 21 2025, 6:28 PM
Unknown Object (File)
Feb 21 2025, 6:26 PM
View All Files
Subscribers
dab
imp
jonathan

Details

Reviewers
stevek
rwatson
Commits
rG1554ba03b651: Add mac_grantbylabel
Summary

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.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 53156
Build 50047: arc lint + arc unit

Event Timeline

sjg created this revision.Aug 11 2023, 10:02 PM
Herald added subscribers: dab, jonathan, imp. · View Herald TranscriptAug 11 2023, 10:02 PM
sjg requested review of this revision.Aug 11 2023, 10:02 PM
Harbormaster completed remote builds in B53054: Diff 125891.Aug 11 2023, 10:02 PM
sjg updated this revision to Diff 125892.Aug 11 2023, 11:42 PM

Missed two files

Harbormaster completed remote builds in B53055: Diff 125892.Aug 11 2023, 11:43 PM
sjg updated this revision to Diff 125893.Aug 11 2023, 11:46 PM

Use SPDX

Harbormaster completed remote builds in B53056: Diff 125893.Aug 11 2023, 11:46 PM
sjg updated this revision to Diff 125894.Aug 12 2023, 1:22 AM

Use correct MAC_VERIEXEC_VERSION in MODULE_DEPEND

Harbormaster completed remote builds in B53057: Diff 125894.Aug 12 2023, 1:23 AM
sjg mentioned this in D36506: veriexec: add syscall to retrieve veriexec label.Aug 14 2023, 7:28 PM
sjg updated this revision to Diff 125995.Aug 14 2023, 8:20 PM

Add veriexec_get_{path,pid}_label

Harbormaster completed remote builds in B53099: Diff 125995.Aug 14 2023, 8:21 PM
sjg updated this revision to Diff 126063.Aug 16 2023, 2:34 AM

Add -l option to veriexec

Harbormaster completed remote builds in B53128: Diff 126063.Aug 16 2023, 2:34 AM
sjg updated this revision to Diff 126097.Aug 16 2023, 10:18 PM

Update copyright year

Harbormaster completed remote builds in B53139: Diff 126097.Aug 16 2023, 10:18 PM
stevek added inline comments.Aug 16 2023, 10:31 PM
sys/security/mac_grantbylabel/mac_grantbylabel.c
60

This sysctl and variable used by it should be under #ifdef MAC_DEBUG since the MAC_GRANTBYLABEL_DBG is only filled out when MAC_DEBUG is defined.

66

This line looks like it ran a bit long?

501

Either drop this bit entirely and use MPC_LOADTIME_FLAG_NOTLATE or add MAC_GRANTBYLABEL_DEBUG to the "options"

sjg updated this revision to Diff 126106.Aug 17 2023, 1:47 AM

Cleanup per feedback

Harbormaster completed remote builds in B53146: Diff 126106.Aug 17 2023, 1:47 AM
sjg updated this revision to Diff 126132.Aug 17 2023, 3:46 PM

Update more copyright years

Harbormaster completed remote builds in B53154: Diff 126132.Aug 17 2023, 3:46 PM
sjg updated this revision to Diff 126134.Aug 17 2023, 4:41 PM

rebase and remove $FreeBSD$

Harbormaster completed remote builds in B53156: Diff 126134.Aug 17 2023, 4:41 PM
sjg marked 3 inline comments as done.Aug 17 2023, 4:45 PM

If someone could explain the rubbish about libveriexc.h being copied to mac_grantbylabel.h

stevek accepted this revision.Aug 24 2023, 7:10 PM
This revision is now accepted and ready to land.Aug 24 2023, 7:10 PM
Closed by commit rG1554ba03b651: Add mac_grantbylabel (authored by sjg). · Explain WhyAug 25 2023, 12:43 AM
This revision was automatically updated to reflect the committed changes.
sjg added a commit: rG1554ba03b651: Add mac_grantbylabel.

Revision Contents
Changeset List

PathSize
etc/
mtree/
2 lines
include/
4 lines
lib/
libveriexec/
4 lines
159 lines
125 lines
13 lines
174 lines
sbin/
veriexec/
2 lines
17 lines
58 lines
sys/
conf/
1 line
1 line
security/
mac_grantbylabel/
506 lines
sys/
security/
mac_grantbylabel/
lib/
libveriexec/
mac_grantbylabel.h
libveriexec.h
49 lines

Diff 126134

View Options

etc/mtree/BSD.include.dist

Show All 12 Lines
#include <sys/dtrace.h> #include <sys/dtrace.h>
#include <cddl/dev/dtrace/dtrace_cddl.h> #include <cddl/dev/dtrace/dtrace_cddl.h>
#include <dis_tables.h> #include <dis_tables.h>
#include "kinst.h" #include "kinst.h"
#define KINST_PUSHL_RBP 0x55 #define KINST_PUSHL_RBP 0x55
#define KINST_POPL_RBP 0x5d
#define KINST_STI 0xfb #define KINST_STI 0xfb
#define KINST_POPF 0x9d #define KINST_POPF 0x9d
#define KINST_MODRM_MOD(b) (((b) & 0xc0) >> 6) #define KINST_MODRM_MOD(b) (((b) & 0xc0) >> 6)
#define KINST_MODRM_REG(b) (((b) & 0x38) >> 3) #define KINST_MODRM_REG(b) (((b) & 0x38) >> 3)
#define KINST_MODRM_RM(b) ((b) & 0x07) #define KINST_MODRM_RM(b) ((b) & 0x07)
#define KINST_SIB_SCALE(s) (((s) & 0xc0) >> 6) #define KINST_SIB_SCALE(s) (((s) & 0xc0) >> 6)
▲ Show 20 Lines • Show All 462 Lines • ▼ Show 20 Lines
int int
kinst_make_probe(linker_file_t lf, int symindx, linker_symval_t *symval, kinst_make_probe(linker_file_t lf, int symindx, linker_symval_t *symval,
void *opaque) void *opaque)
{ {
struct kinst_probe *kp; struct kinst_probe *kp;
dtrace_kinst_probedesc_t *pd; dtrace_kinst_probedesc_t *pd;
const char *func; const char *func;
int error, instrsize, n, off; int error, instrsize, n, off;
uint8_t *instr, *limit; uint8_t *instr, *limit, *tmp;
bool push_found, pop_found;
pd = opaque; pd = opaque;
func = symval->name; func = symval->name;
if (kinst_excluded(func)) if (kinst_excluded(func))
return (0); return (0);
if (strcmp(func, pd->kpd_func) != 0) if (strcmp(func, pd->kpd_func) != 0)
return (0); return (0);
instr = (uint8_t *)symval->value; instr = (uint8_t *)symval->value;
limit = (uint8_t *)symval->value + symval->size; limit = (uint8_t *)symval->value + symval->size;
if (instr >= limit) if (instr >= limit)
return (0); return (0);
/* /*
* Ignore functions not beginning with the usual function prologue. * Ignore functions not beginning with the usual function prologue.
markjUnsubmitted
Done
Inline Actions

The first sentence isn't really accurate anymore. I'd just replace the comment with something like, "refuse to instrument functions lacking the usual frame pointer manipulations since they might correspond to exception handlers."

markj: The first sentence isn't really accurate anymore. I'd just replace the comment with something…
* These might correspond to exception handlers with which we should not * These might correspond to exception handlers with which we should
* meddle. This does however exclude functions which can be safely * not meddle.
* traced, such as cpu_switch().
*/ */
if (*instr != KINST_PUSHL_RBP) tmp = instr;
push_found = pop_found = false;
while (tmp < limit) {
if (*tmp == KINST_PUSHL_RBP)
push_found = true;
else if (*tmp == KINST_POPL_RBP)
pop_found = true;
if (push_found && pop_found)
break;
tmp += dtrace_instr_size(tmp);
}
if (!push_found || !pop_found)
return (0); return (0);
n = 0; n = 0;
while (instr < limit) { while (instr < limit) {
instrsize = dtrace_instr_size(instr); instrsize = dtrace_instr_size(instr);
off = (int)(instr - (uint8_t *)symval->value); off = (int)(instr - (uint8_t *)symval->value);
if (pd->kpd_off != -1 && off != pd->kpd_off) { if (pd->kpd_off != -1 && off != pd->kpd_off) {
instr += instrsize; instr += instrsize;
▲ Show 20 Lines • Show All 91 Lines • Show Last 20 Lines
View Options

include/Makefile

Show All 12 Lines
#include <sys/dtrace.h> #include <sys/dtrace.h>
#include <cddl/dev/dtrace/dtrace_cddl.h> #include <cddl/dev/dtrace/dtrace_cddl.h>
#include <dis_tables.h> #include <dis_tables.h>
#include "kinst.h" #include "kinst.h"
#define KINST_PUSHL_RBP 0x55 #define KINST_PUSHL_RBP 0x55
#define KINST_POPL_RBP 0x5d
#define KINST_STI 0xfb #define KINST_STI 0xfb
#define KINST_POPF 0x9d #define KINST_POPF 0x9d
#define KINST_MODRM_MOD(b) (((b) & 0xc0) >> 6) #define KINST_MODRM_MOD(b) (((b) & 0xc0) >> 6)
#define KINST_MODRM_REG(b) (((b) & 0x38) >> 3) #define KINST_MODRM_REG(b) (((b) & 0x38) >> 3)
#define KINST_MODRM_RM(b) ((b) & 0x07) #define KINST_MODRM_RM(b) ((b) & 0x07)
#define KINST_SIB_SCALE(s) (((s) & 0xc0) >> 6) #define KINST_SIB_SCALE(s) (((s) & 0xc0) >> 6)
▲ Show 20 Lines • Show All 462 Lines • ▼ Show 20 Lines
int int
kinst_make_probe(linker_file_t lf, int symindx, linker_symval_t *symval, kinst_make_probe(linker_file_t lf, int symindx, linker_symval_t *symval,
void *opaque) void *opaque)
{ {
struct kinst_probe *kp; struct kinst_probe *kp;
dtrace_kinst_probedesc_t *pd; dtrace_kinst_probedesc_t *pd;
const char *func; const char *func;
int error, instrsize, n, off; int error, instrsize, n, off;
uint8_t *instr, *limit; uint8_t *instr, *limit, *tmp;
bool push_found, pop_found;
pd = opaque; pd = opaque;
func = symval->name; func = symval->name;
if (kinst_excluded(func)) if (kinst_excluded(func))
return (0); return (0);
if (strcmp(func, pd->kpd_func) != 0) if (strcmp(func, pd->kpd_func) != 0)
return (0); return (0);
instr = (uint8_t *)symval->value; instr = (uint8_t *)symval->value;
limit = (uint8_t *)symval->value + symval->size; limit = (uint8_t *)symval->value + symval->size;
if (instr >= limit) if (instr >= limit)
return (0); return (0);
/* /*
* Ignore functions not beginning with the usual function prologue. * Ignore functions not beginning with the usual function prologue.
markjUnsubmitted
Done
Inline Actions

The first sentence isn't really accurate anymore. I'd just replace the comment with something like, "refuse to instrument functions lacking the usual frame pointer manipulations since they might correspond to exception handlers."

markj: The first sentence isn't really accurate anymore. I'd just replace the comment with something…
* These might correspond to exception handlers with which we should not * These might correspond to exception handlers with which we should
* meddle. This does however exclude functions which can be safely * not meddle.
* traced, such as cpu_switch().
*/ */
if (*instr != KINST_PUSHL_RBP) tmp = instr;
push_found = pop_found = false;
while (tmp < limit) {
if (*tmp == KINST_PUSHL_RBP)
push_found = true;
else if (*tmp == KINST_POPL_RBP)
pop_found = true;
if (push_found && pop_found)
break;
tmp += dtrace_instr_size(tmp);
}
if (!push_found || !pop_found)
return (0); return (0);
n = 0; n = 0;
while (instr < limit) { while (instr < limit) {
instrsize = dtrace_instr_size(instr); instrsize = dtrace_instr_size(instr);
off = (int)(instr - (uint8_t *)symval->value); off = (int)(instr - (uint8_t *)symval->value);
if (pd->kpd_off != -1 && off != pd->kpd_off) { if (pd->kpd_off != -1 && off != pd->kpd_off) {
instr += instrsize; instr += instrsize;
▲ Show 20 Lines • Show All 91 Lines • Show Last 20 Lines
View Options

lib/libveriexec/Makefile

Show All 12 Lines
#include <sys/dtrace.h> #include <sys/dtrace.h>
#include <cddl/dev/dtrace/dtrace_cddl.h> #include <cddl/dev/dtrace/dtrace_cddl.h>
#include <dis_tables.h> #include <dis_tables.h>
#include "kinst.h" #include "kinst.h"
#define KINST_PUSHL_RBP 0x55 #define KINST_PUSHL_RBP 0x55
#define KINST_POPL_RBP 0x5d
#define KINST_STI 0xfb #define KINST_STI 0xfb
#define KINST_POPF 0x9d #define KINST_POPF 0x9d
#define KINST_MODRM_MOD(b) (((b) & 0xc0) >> 6) #define KINST_MODRM_MOD(b) (((b) & 0xc0) >> 6)
#define KINST_MODRM_REG(b) (((b) & 0x38) >> 3) #define KINST_MODRM_REG(b) (((b) & 0x38) >> 3)
#define KINST_MODRM_RM(b) ((b) & 0x07) #define KINST_MODRM_RM(b) ((b) & 0x07)
#define KINST_SIB_SCALE(s) (((s) & 0xc0) >> 6) #define KINST_SIB_SCALE(s) (((s) & 0xc0) >> 6)
▲ Show 20 Lines • Show All 462 Lines • ▼ Show 20 Lines
int int
kinst_make_probe(linker_file_t lf, int symindx, linker_symval_t *symval, kinst_make_probe(linker_file_t lf, int symindx, linker_symval_t *symval,
void *opaque) void *opaque)
{ {
struct kinst_probe *kp; struct kinst_probe *kp;
dtrace_kinst_probedesc_t *pd; dtrace_kinst_probedesc_t *pd;
const char *func; const char *func;
int error, instrsize, n, off; int error, instrsize, n, off;
uint8_t *instr, *limit; uint8_t *instr, *limit, *tmp;
bool push_found, pop_found;
pd = opaque; pd = opaque;
func = symval->name; func = symval->name;
if (kinst_excluded(func)) if (kinst_excluded(func))
return (0); return (0);
if (strcmp(func, pd->kpd_func) != 0) if (strcmp(func, pd->kpd_func) != 0)
return (0); return (0);
instr = (uint8_t *)symval->value; instr = (uint8_t *)symval->value;
limit = (uint8_t *)symval->value + symval->size; limit = (uint8_t *)symval->value + symval->size;
if (instr >= limit) if (instr >= limit)
return (0); return (0);
/* /*
* Ignore functions not beginning with the usual function prologue. * Ignore functions not beginning with the usual function prologue.
markjUnsubmitted
Done
Inline Actions

The first sentence isn't really accurate anymore. I'd just replace the comment with something like, "refuse to instrument functions lacking the usual frame pointer manipulations since they might correspond to exception handlers."

markj: The first sentence isn't really accurate anymore. I'd just replace the comment with something…
* These might correspond to exception handlers with which we should not * These might correspond to exception handlers with which we should
* meddle. This does however exclude functions which can be safely * not meddle.
* traced, such as cpu_switch().
*/ */
if (*instr != KINST_PUSHL_RBP) tmp = instr;
push_found = pop_found = false;
while (tmp < limit) {
if (*tmp == KINST_PUSHL_RBP)
push_found = true;
else if (*tmp == KINST_POPL_RBP)
pop_found = true;
if (push_found && pop_found)
break;
tmp += dtrace_instr_size(tmp);
}
if (!push_found || !pop_found)
return (0); return (0);
n = 0; n = 0;
while (instr < limit) { while (instr < limit) {
instrsize = dtrace_instr_size(instr); instrsize = dtrace_instr_size(instr);
off = (int)(instr - (uint8_t *)symval->value); off = (int)(instr - (uint8_t *)symval->value);
if (pd->kpd_off != -1 && off != pd->kpd_off) { if (pd->kpd_off != -1 && off != pd->kpd_off) {
instr += instrsize; instr += instrsize;
▲ Show 20 Lines • Show All 91 Lines • Show Last 20 Lines
View Options

lib/libveriexec/exec_script.c

Show All 12 Lines
#include <sys/dtrace.h> #include <sys/dtrace.h>
#include <cddl/dev/dtrace/dtrace_cddl.h> #include <cddl/dev/dtrace/dtrace_cddl.h>
#include <dis_tables.h> #include <dis_tables.h>
#include "kinst.h" #include "kinst.h"
#define KINST_PUSHL_RBP 0x55 #define KINST_PUSHL_RBP 0x55
#define KINST_POPL_RBP 0x5d
#define KINST_STI 0xfb #define KINST_STI 0xfb
#define KINST_POPF 0x9d #define KINST_POPF 0x9d
#define KINST_MODRM_MOD(b) (((b) & 0xc0) >> 6) #define KINST_MODRM_MOD(b) (((b) & 0xc0) >> 6)
#define KINST_MODRM_REG(b) (((b) & 0x38) >> 3) #define KINST_MODRM_REG(b) (((b) & 0x38) >> 3)
#define KINST_MODRM_RM(b) ((b) & 0x07) #define KINST_MODRM_RM(b) ((b) & 0x07)
#define KINST_SIB_SCALE(s) (((s) & 0xc0) >> 6) #define KINST_SIB_SCALE(s) (((s) & 0xc0) >> 6)
▲ Show 20 Lines • Show All 462 Lines • ▼ Show 20 Lines
int int
kinst_make_probe(linker_file_t lf, int symindx, linker_symval_t *symval, kinst_make_probe(linker_file_t lf, int symindx, linker_symval_t *symval,
void *opaque) void *opaque)
{ {
struct kinst_probe *kp; struct kinst_probe *kp;
dtrace_kinst_probedesc_t *pd; dtrace_kinst_probedesc_t *pd;
const char *func; const char *func;
int error, instrsize, n, off; int error, instrsize, n, off;
uint8_t *instr, *limit; uint8_t *instr, *limit, *tmp;
bool push_found, pop_found;
pd = opaque; pd = opaque;
func = symval->name; func = symval->name;
if (kinst_excluded(func)) if (kinst_excluded(func))
return (0); return (0);
if (strcmp(func, pd->kpd_func) != 0) if (strcmp(func, pd->kpd_func) != 0)
return (0); return (0);
instr = (uint8_t *)symval->value; instr = (uint8_t *)symval->value;
limit = (uint8_t *)symval->value + symval->size; limit = (uint8_t *)symval->value + symval->size;
if (instr >= limit) if (instr >= limit)
return (0); return (0);
/* /*
* Ignore functions not beginning with the usual function prologue. * Ignore functions not beginning with the usual function prologue.
markjUnsubmitted
Done
Inline Actions

The first sentence isn't really accurate anymore. I'd just replace the comment with something like, "refuse to instrument functions lacking the usual frame pointer manipulations since they might correspond to exception handlers."

markj: The first sentence isn't really accurate anymore. I'd just replace the comment with something…
* These might correspond to exception handlers with which we should not * These might correspond to exception handlers with which we should
* meddle. This does however exclude functions which can be safely * not meddle.
* traced, such as cpu_switch().
*/ */
if (*instr != KINST_PUSHL_RBP) tmp = instr;
push_found = pop_found = false;
while (tmp < limit) {
if (*tmp == KINST_PUSHL_RBP)
push_found = true;
else if (*tmp == KINST_POPL_RBP)
pop_found = true;
if (push_found && pop_found)
break;
tmp += dtrace_instr_size(tmp);
}
if (!push_found || !pop_found)
return (0); return (0);
n = 0; n = 0;
while (instr < limit) { while (instr < limit) {
instrsize = dtrace_instr_size(instr); instrsize = dtrace_instr_size(instr);
off = (int)(instr - (uint8_t *)symval->value); off = (int)(instr - (uint8_t *)symval->value);
if (pd->kpd_off != -1 && off != pd->kpd_off) { if (pd->kpd_off != -1 && off != pd->kpd_off) {
instr += instrsize; instr += instrsize;
▲ Show 20 Lines • Show All 91 Lines • Show Last 20 Lines
View Options

lib/libveriexec/gbl_check.c

Loading...
View Options

lib/libveriexec/libveriexec.h

Loading...
View Options

lib/libveriexec/veriexec_get.c

Loading...
View Options

sbin/veriexec/Makefile.depend

Loading...
View Options

sbin/veriexec/veriexec.8

Loading...
View Options

sbin/veriexec/veriexec.c

Loading...
View Options

sys/conf/files

Loading...
View Options

sys/conf/options

Loading...
View Options

sys/security/mac_grantbylabel/mac_grantbylabel.h

Loading...
View Options

sys/security/mac_grantbylabel/mac_grantbylabel.c

Loading...