isa/sudo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Pass a secret value to sudo_intercept.so and verify after policy check.

Browse Source 
The goal is to make it harder for someone to have a fake policy checker.
This will not stop a determined adversary since the secret is present
in the address space of the running process.
This commit is contained in:
Todd C. Miller
2021-08-13 09:10:44 -06:00
parent c9d9225469
commit eaf03a382b
8 changed files with 57 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
include/intercept.pb-c.h
View File

@@ -118,6 +118,7 @@ typedef enum {
struct _PolicyCheckResult
{
ProtobufCMessage base;
uint64_t secret;
PolicyCheckResult__TypeCase type_case;
union {
PolicyAcceptMessage *accept_msg;
@@ -127,7 +128,7 @@ struct _PolicyCheckResult
};
#define POLICY_CHECK_RESULT__INIT \
{ PROTOBUF_C_MESSAGE_INIT (&policy_check_result__descriptor) \
, POLICY_CHECK_RESULT__TYPE__NOT_SET, {0} }
, 0, POLICY_CHECK_RESULT__TYPE__NOT_SET, {0} }
/* InterceptMessage methods */

17
src/exec_intercept.c
View File

@@ -52,6 +52,7 @@ static void intercept_cb(int fd, int what, void *v);
/* Must match start of exec_closure_nopty and monitor_closure. */
struct intercept_fd_closure {
uint64_t secret;
struct command_details *details;
struct sudo_event_base *evbase;
};
@@ -65,6 +66,7 @@ struct intercept_closure {
char **run_argv; /* owned by plugin */
char **run_envp; /* dynamically allocated */
uint8_t *buf; /* dynamically allocated */
uint64_t secret;
size_t len;
int policy_result;
};
@@ -380,13 +382,14 @@ done:
}
static bool
fmt_policy_check_result(PolicyCheckResult *msg, struct intercept_closure *closure)
fmt_policy_check_result(PolicyCheckResult *res, struct intercept_closure *closure)
{
uint32_t msg_len;
bool ret = false;
debug_decl(fmt_policy_check_result, SUDO_DEBUG_EXEC);
closure->len = policy_check_result__get_packed_size(msg);
res->secret = closure->secret;
closure->len = policy_check_result__get_packed_size(res);
if (closure->len > MESSAGE_SIZE_MAX) {
sudo_warnx(U_("server message too large: %zu"), closure->len);
goto done;
@@ -404,7 +407,7 @@ fmt_policy_check_result(PolicyCheckResult *msg, struct intercept_closure *closur
goto done;
}
memcpy(closure->buf, &msg_len, sizeof(msg_len));
policy_check_result__pack(msg, closure->buf + sizeof(msg_len));
policy_check_result__pack(res, closure->buf + sizeof(msg_len));
ret = true;
@@ -565,6 +568,7 @@ intercept_fd_cb(int fd, int what, void *v)
sudo_warnx("%s", U_("unable to allocate memory"));
goto bad;
}
closure->secret = fdc->secret;
closure->details = fdc->details;
/*
@@ -598,6 +602,13 @@ intercept_fd_cb(int fd, int what, void *v)
break;
}
if (ch == INTERCEPT_REQ_SEC) {
/* Client requested secret from ctor, no fd is present. */
if (write(fd, &fdc->secret, sizeof(fdc->secret)) != sizeof(fdc->secret))
goto bad;
debug_return;
}
#if defined(HAVE_STRUCT_MSGHDR_MSG_CONTROL) && HAVE_STRUCT_MSGHDR_MSG_CONTROL == 1
cmsg = CMSG_FIRSTHDR(&msg);
if (cmsg == NULL) {

2
src/exec_nopty.c
View File

@@ -40,6 +40,7 @@
/* Note that details and evbase must come first. */
struct exec_closure_nopty {
uint64_t secret;
struct command_details *details;
struct sudo_event_base *evbase;
struct sudo_event *errpipe_event;
@@ -201,6 +202,7 @@ fill_exec_closure_nopty(struct exec_closure_nopty *ec,
debug_decl(fill_exec_closure_nopty, SUDO_DEBUG_EXEC);
/* Fill in the non-event part of the closure. */
ec->secret = arc4random() | ((uint64_t)arc4random() << 32);
ec->ppgrp = getpgrp();
ec->cstat = cstat;
ec->details = details;

2
src/exec_pty.c
View File

@@ -57,6 +57,7 @@ TAILQ_HEAD(monitor_message_list, monitor_message);
/* Note that details and evbase must come first. */
struct exec_closure_pty {
uint64_t secret;
struct command_details *details;
struct sudo_event_base *evbase;
struct sudo_event *backchannel_event;
@@ -1205,6 +1206,7 @@ fill_exec_closure_pty(struct exec_closure_pty *ec, struct command_status *cstat,
debug_decl(fill_exec_closure_pty, SUDO_DEBUG_EXEC);
/* Fill in the non-event part of the closure. */
ec->secret = arc4random() | ((uint64_t)arc4random() << 32);
ec->cmnd_pid = -1;
ec->ppgrp = ppgrp;
ec->cstat = cstat;

19
src/intercept.pb-c.c
View File

@@ -519,7 +519,7 @@ const ProtobufCMessageDescriptor policy_error_message__descriptor =
(ProtobufCMessageInit) policy_error_message__init,
NULL,NULL,NULL /* reserved[123] */
};
static const ProtobufCFieldDescriptor policy_check_result__field_descriptors[3] =
static const ProtobufCFieldDescriptor policy_check_result__field_descriptors[4] =
{
{
"accept_msg",
@@ -557,16 +557,29 @@ static const ProtobufCFieldDescriptor policy_check_result__field_descriptors[3]
0 | PROTOBUF_C_FIELD_FLAG_ONEOF, /* flags */
0,NULL,NULL /* reserved1,reserved2, etc */
},
{
"secret",
4,
PROTOBUF_C_LABEL_NONE,
PROTOBUF_C_TYPE_FIXED64,
0, /* quantifier_offset */
offsetof(PolicyCheckResult, secret),
NULL,
NULL,
0, /* flags */
0,NULL,NULL /* reserved1,reserved2, etc */
},
};
static const unsigned policy_check_result__field_indices_by_name[] = {
0, /* field[0] = accept_msg */
2, /* field[2] = error_msg */
1, /* field[1] = reject_msg */
3, /* field[3] = secret */
};
static const ProtobufCIntRange policy_check_result__number_ranges[1 + 1] =
{
{ 1, 0 },
{ 0, 3 }
{ 0, 4 }
};
const ProtobufCMessageDescriptor policy_check_result__descriptor =
{
@@ -576,7 +589,7 @@ const ProtobufCMessageDescriptor policy_check_result__descriptor =
"PolicyCheckResult",
"",
sizeof(PolicyCheckResult),
3,
4,
policy_check_result__field_descriptors,
policy_check_result__field_indices_by_name,
1, policy_check_result__number_ranges,

1
src/intercept.proto
View File

@@ -44,4 +44,5 @@ message PolicyCheckResult {
PolicyRejectMessage reject_msg = 2;
PolicyErrorMessage error_msg = 3;
}
fixed64 secret = 4;
}

1
src/sudo_exec.h
View File

@@ -80,6 +80,7 @@
#define SESH_ERR_SOME_FILES 33 /* copy error, some files copied */
#define INTERCEPT_FD_MIN 64 /* minimum fd so shell won't close it */
#define INTERCEPT_REQ_SEC 42 /* request intercept secret */
#define MESSAGE_SIZE_MAX 2097152 /* 2Mib max intercept message size */
/*

19
src/sudo_intercept_common.c
View File

@@ -53,6 +53,7 @@
extern char **environ;
static int intercept_sock = -1;
static uint64_t secret;
/*
* Look up SUDO_INTERCEPT_FD in the environment.
@@ -82,6 +83,7 @@ sudo_interposer_init(void)
if (strncmp(*p, "SUDO_INTERCEPT_FD=", sizeof("SUDO_INTERCEPT_FD=") -1) == 0) {
const char *fdstr = *p + sizeof("SUDO_INTERCEPT_FD=") - 1;
const char *errstr;
char ch = INTERCEPT_REQ_SEC;
int fd;
fd = sudo_strtonum(fdstr, 0, INT_MAX, &errstr);
@@ -90,6 +92,19 @@ sudo_interposer_init(void)
"invalid SUDO_INTERCEPT_FD: %s: %s", fdstr, errstr);
break;
}
/* Request secret from parent. */
if (send(fd, &ch, sizeof(ch), 0) != sizeof(ch)) {
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
"unable to request secret: %s", strerror(errno));
break;
}
if (recv(fd, &secret, sizeof(secret), 0) != sizeof(secret)) {
sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
"unable to read secret: %s", strerror(errno));
break;
}
intercept_sock = fd;
break;
}
@@ -308,6 +323,10 @@ command_allowed(const char *cmnd, char * const argv[], char * const envp[],
"unable to unpack %s size %u", "PolicyCheckResult", res_len);
goto done;
}
if (res->secret != secret) {
sudo_warnx("secret mismatch\r");
goto done;
}
switch (res->type_case) {
case POLICY_CHECK_RESULT__TYPE_ACCEPT_MSG:
if (sudo_debug_needed(SUDO_DEBUG_INFO)) {