Changeset View
Standalone View
sbin/dumpon/dumpon.c
Show First 20 Lines • Show All 69 Lines • ▼ Show 20 Lines | ||||||||||
#include <net/if.h> | #include <net/if.h> | |||||||||
#include <net/if_dl.h> | #include <net/if_dl.h> | |||||||||
#include <net/route.h> | #include <net/route.h> | |||||||||
#include <netinet/in.h> | #include <netinet/in.h> | |||||||||
#include <netinet/netdump/netdump.h> | #include <netinet/netdump/netdump.h> | |||||||||
#ifdef HAVE_CRYPTO | #ifdef HAVE_CRYPTO | |||||||||
#include <libcasper.h> | ||||||||||
#include <casper/cap_sysctl.h> | ||||||||||
#include <openssl/err.h> | #include <openssl/err.h> | |||||||||
#include <openssl/pem.h> | #include <openssl/pem.h> | |||||||||
#include <openssl/rand.h> | #include <openssl/rand.h> | |||||||||
#include <openssl/rsa.h> | #include <openssl/rsa.h> | |||||||||
#endif | #endif | |||||||||
static int netdump_fd = -1; | ||||||||||
static int verbose; | static int verbose; | |||||||||
static const char *sysctlname = "kern.shutdown.dumpdevname"; | ||||||||||
#ifdef HAVE_CRYPTO | ||||||||||
static cap_channel_t *capsysctl; | ||||||||||
#endif | ||||||||||
static void _Noreturn | static void _Noreturn | |||||||||
usage(void) | usage(void) | |||||||||
{ | { | |||||||||
fprintf(stderr, | fprintf(stderr, | |||||||||
"usage: dumpon [-i index] [-r] [-v] [-k <pubkey>] [-Zz] <device>\n" | "usage: dumpon [-i index] [-r] [-v] [-k <pubkey>] [-Zz] <device>\n" | |||||||||
" dumpon [-i index] [-r] [-v] [-k <pubkey>] [-Zz]\n" | " dumpon [-i index] [-r] [-v] [-k <pubkey>] [-Zz]\n" | |||||||||
" [-g <gateway>] -s <server> -c <client> <iface>\n" | " [-g <gateway>] -s <server> -c <client> <iface>\n" | |||||||||
" dumpon [-v] off\n" | " dumpon [-v] off\n" | |||||||||
▲ Show 20 Lines • Show All 113 Lines • ▼ Show 20 Lines | check_size(int fd, const char *fn) | |||||||||
if ((uintmax_t)mediasize < (uintmax_t)physmem) | if ((uintmax_t)mediasize < (uintmax_t)physmem) | |||||||||
errx(EX_IOERR, "%s is smaller than physical memory", fn); | errx(EX_IOERR, "%s is smaller than physical memory", fn); | |||||||||
} | } | |||||||||
#ifdef HAVE_CRYPTO | #ifdef HAVE_CRYPTO | |||||||||
static void | static void | |||||||||
genkey(const char *pubkeyfile, struct diocskerneldump_arg *kdap) | genkey(const char *pubkeyfile, struct diocskerneldump_arg *kdap) | |||||||||
{ | { | |||||||||
cap_channel_t *capcas; | ||||||||||
void *caplimit; | ||||||||||
FILE *fp; | FILE *fp; | |||||||||
RSA *pubkey; | RSA *pubkey; | |||||||||
assert(pubkeyfile != NULL); | assert(pubkeyfile != NULL); | |||||||||
assert(kdap != NULL); | assert(kdap != NULL); | |||||||||
fp = NULL; | fp = NULL; | |||||||||
pubkey = NULL; | pubkey = NULL; | |||||||||
Show All 9 Lines | genkey(const char *pubkeyfile, struct diocskerneldump_arg *kdap) | |||||||||
*/ | */ | |||||||||
#if OPENSSL_VERSION_NUMBER < 0x10100000L | #if OPENSSL_VERSION_NUMBER < 0x10100000L | |||||||||
{ | { | |||||||||
unsigned char c[1]; | unsigned char c[1]; | |||||||||
RAND_bytes(c, 1); | RAND_bytes(c, 1); | |||||||||
} | } | |||||||||
#endif | #endif | |||||||||
capcas = cap_init(); | ||||||||||
if (capcas == NULL) | ||||||||||
err(1, "Unable to contact Casper"); | ||||||||||
/* | ||||||||||
* Allow listdumpdev() to display verbose output later. | ||||||||||
*/ | ||||||||||
if (verbose) { | ||||||||||
markj: We only use (and close) `capcas` if `verbose` is specified. | ||||||||||
netdump_fd = open(_PATH_NETDUMP, O_RDONLY); | ||||||||||
if (netdump_fd == -1 && errno != ENOENT) | ||||||||||
err(EX_OSERR, "opening %s", _PATH_NETDUMP); | ||||||||||
capsysctl = cap_service_open(capcas, "system.sysctl"); | ||||||||||
if (capsysctl == NULL) | ||||||||||
err(1, "Unable to open system.sysctl service"); | ||||||||||
cap_close(capcas); | ||||||||||
caplimit = cap_sysctl_limit_init(capsysctl); | ||||||||||
(void)cap_sysctl_limit_name(caplimit, sysctlname, | ||||||||||
CAP_SYSCTL_READ); | ||||||||||
if (cap_sysctl_limit(caplimit) < 0) | ||||||||||
err(1, "Unable to set system.sysctl limits"); | ||||||||||
} | ||||||||||
if (caph_enter() < 0) | if (caph_enter() < 0) | |||||||||
Not Done Inline ActionsWe should indeed use caph_enter_casper() if verbose is true. Otherwise if someone tries to build dumpon without casper support, dumpon -v will fail again. markj: We should indeed use caph_enter_casper() if `verbose` is true. Otherwise if someone tries to… | ||||||||||
Not Done Inline ActionsYou're right, currently without casper the sysctl error does display with -v. # truss obj/dumpon -v -k pubkey off 2>&1 | grep kern.shutdown.dumpdevname __sysctlbyname("kern.shutdown.dumpdevname",25,0x7fffffffda50,0x7fffffffd9b0,0x0,0) ERR#1 'Operation not permitted' Sysctl get 'kern.shutdown.dumpdevname' But if we change to caph_enter_casper() it will not enter capability mode and will lie that it was successful. So -v -k results in no capability mode. diff --git sbin/dumpon/Makefile sbin/dumpon/Makefile index fa80cc4efcd4..d0700ab16a89 100644 --- sbin/dumpon/Makefile +++ sbin/dumpon/Makefile @@ -10,12 +10,6 @@ LIBADD= crypto CFLAGS+=-DHAVE_CRYPTO .endif -.if ${MK_CASPER} != "no" -LIBADD+= casper -LIBADD+= cap_sysctl -CFLAGS+= -DWITH_CASPER -.endif - MAN= dumpon.8 .include <bsd.prog.mk> diff --git sbin/dumpon/dumpon.c sbin/dumpon/dumpon.c index 42e4941463ee..3b980f5cfd0e 100644 --- sbin/dumpon/dumpon.c +++ sbin/dumpon/dumpon.c @@ -268,8 +268,9 @@ genkey(const char *pubkeyfile, struct diocskerneldump_arg *kdap) CAP_SYSCTL_READ); if (cap_sysctl_limit(caplimit) < 0) err(1, "Unable to set system.sysctl limits"); - } - if (caph_enter() < 0) + if (caph_enter_casper() < 0) + err(1, "Unable to enter capability mode"); + } else if (cap_enter() < 0) err(1, "Unable to enter capability mode"); pubkey = RSA_new(); # truss obj/dumpon -v -k pubkey off 2>&1 | grep kern.shutdown.dumpdevname __sysctlbyname("kern.shutdown.dumpdevname",25,0x7fffffffda50,0x7fffffffd9b0,0x0,0) = 0 (0x0) I think if we do genkey() in a child process with shared memory or pipe to pass the data back we could avoid the problem. diff --git sbin/dumpon/dumpon.c sbin/dumpon/dumpon.c index 183ce5f08cb3..0378142a976f 100644 --- sbin/dumpon/dumpon.c +++ sbin/dumpon/dumpon.c @@ -48,6 +48,7 @@ __FBSDID("$FreeBSD$"); #include <sys/disk.h> #include <sys/socket.h> #include <sys/sysctl.h> +#include <sys/wait.h> #include <assert.h> #include <capsicum_helpers.h> @@ -210,7 +211,7 @@ check_size(int fd, const char *fn) #ifdef HAVE_CRYPTO static void -genkey(const char *pubkeyfile, struct diocskerneldump_arg *kdap) +_genkey(const char *pubkeyfile, struct diocskerneldump_arg *kdap) { FILE *fp; RSA *pubkey; @@ -305,6 +306,44 @@ genkey(const char *pubkeyfile, struct diocskerneldump_arg *kdap) } RSA_free(pubkey); } + +/* + * Run genkey() in a child so it can use capability mode without affecting + * the rest of the runtime. + */ +static void +genkey(const char *pubkeyfile, struct diocskerneldump_arg *kdap) +{ + pid_t pid; + int error, filedes[2], status; + + if (pipe2(filedes, O_CLOEXEC) != 0) + err(1, "pipe"); + pid = fork(); + switch (pid) { + case -1: + err(1, "fork"); + break; + case 0: + close(filedes[0]); + _genkey(pubkeyfile, kdap); + /* Write the new kdap back to the parent. */ + error = write(filedes[1], kdap, sizeof(*kdap)); + if (error != sizeof(*kdap)) + err(1, "genkey pipe write"); + exit(0); + } + close(filedes[1]); + /* Read in the child's genkey() result into kdap. */ + error = read(filedes[0], kdap, sizeof(*kdap)); + if (error != sizeof(*kdap)) + errx(1, "genkey pipe read"); + waitpid(pid, &status, WEXITED); + if (status != 0) + errx(1, "genkey child failed"); + close(filedes[0]); + return; +} #endif static void bdrewery: You're right, currently without casper the sysctl error does display with `-v`.
```
# truss… | ||||||||||
Not Done Inline Actions
If WITH_CASPER isn't defined, yes. In this case the error is not very important so it might be acceptable to use caph_enter() unconditionally and live with the inconvenience of -v -k raising an error. In general I think it's better to avoid the possibility of ENOTCAPABLE errors as much as possible, i.e., code should be clean with respect to kern.trap_enotcap=1. Moving the sensitive portion of the program into a separate process seems cleaner to me. I'd want @oshogbo and @def to comment on this though. markj: > But if we change to caph_enter_casper() it will not enter capability mode and will lie that… | ||||||||||
Not Done Inline ActionsI like the idea of moving _genkey to sepreate process. For me we - we should avoid of ENOTCAPABLE - if this error is raised it means that we did something wrong. I would suggest using pdfork() instead of fork. oshogbo: I like the idea of moving _genkey to sepreate process.
For me we - we should avoid of… | ||||||||||
Done Inline ActionsWhy pdfork? bdrewery: Why `pdfork`? | ||||||||||
Not Done Inline ActionsIt ensures that the child will be killed if the parent exits unexpectedly. I don't think that's a real concern right, but it's a nicer model to have. markj: It ensures that the child will be killed if the parent exits unexpectedly. I don't think that's… | ||||||||||
err(1, "Unable to enter capability mode"); | err(1, "Unable to enter capability mode"); | |||||||||
pubkey = RSA_new(); | pubkey = RSA_new(); | |||||||||
if (pubkey == NULL) { | if (pubkey == NULL) { | |||||||||
errx(1, "Unable to allocate an RSA structure: %s", | errx(1, "Unable to allocate an RSA structure: %s", | |||||||||
ERR_error_string(ERR_get_error(), NULL)); | ERR_error_string(ERR_get_error(), NULL)); | |||||||||
} | } | |||||||||
▲ Show 20 Lines • Show All 61 Lines • ▼ Show 20 Lines | ||||||||||
static void | static void | |||||||||
listdumpdev(void) | listdumpdev(void) | |||||||||
{ | { | |||||||||
static char ip[200]; | static char ip[200]; | |||||||||
char dumpdev[PATH_MAX]; | char dumpdev[PATH_MAX]; | |||||||||
struct diocskerneldump_arg ndconf; | struct diocskerneldump_arg ndconf; | |||||||||
size_t len; | size_t len; | |||||||||
const char *sysctlname = "kern.shutdown.dumpdevname"; | int error; | |||||||||
int fd; | ||||||||||
len = sizeof(dumpdev); | len = sizeof(dumpdev); | |||||||||
if (sysctlbyname(sysctlname, &dumpdev, &len, NULL, 0) != 0) { | #ifdef HAVE_CRYPTO | |||||||||
if (capsysctl != NULL) | ||||||||||
error = cap_sysctlbyname(capsysctl, sysctlname, &dumpdev, &len, NULL, 0); | ||||||||||
else | ||||||||||
#endif | ||||||||||
error = sysctlbyname(sysctlname, &dumpdev, &len, NULL, 0); | ||||||||||
if (error != 0) { | ||||||||||
Not Done Inline ActionsWhat if -v is used without -k? markj: What if `-v` is used without `-k`? | ||||||||||
Done Inline ActionsGood point. bdrewery: Good point.
In the first revision it still worked because `caph_enter_casper()` silently failed… | ||||||||||
if (errno == ENOMEM) { | if (errno == ENOMEM) { | |||||||||
err(EX_OSERR, "Kernel returned too large of a buffer for '%s'\n", | err(EX_OSERR, "Kernel returned too large of a buffer for '%s'\n", | |||||||||
sysctlname); | sysctlname); | |||||||||
} else { | } else { | |||||||||
err(EX_OSERR, "Sysctl get '%s'\n", sysctlname); | err(EX_OSERR, "Sysctl get '%s'\n", sysctlname); | |||||||||
} | } | |||||||||
Done Inline Actions
markj: | ||||||||||
} | } | |||||||||
if (strlen(dumpdev) == 0) | if (strlen(dumpdev) == 0) | |||||||||
(void)strlcpy(dumpdev, _PATH_DEVNULL, sizeof(dumpdev)); | (void)strlcpy(dumpdev, _PATH_DEVNULL, sizeof(dumpdev)); | |||||||||
Done Inline Actionsread(2) returns a ssize_t. Same comment above for write(). markj: read(2) returns a ssize_t. Same comment above for write(). | ||||||||||
if (verbose) { | if (verbose) { | |||||||||
char *ctx, *dd; | char *ctx, *dd; | |||||||||
unsigned idx; | unsigned idx; | |||||||||
printf("kernel dumps on priority: device\n"); | printf("kernel dumps on priority: device\n"); | |||||||||
idx = 0; | idx = 0; | |||||||||
ctx = dumpdev; | ctx = dumpdev; | |||||||||
while ((dd = strsep(&ctx, ",")) != NULL) | while ((dd = strsep(&ctx, ",")) != NULL) | |||||||||
printf("%u: %s\n", idx++, dd); | printf("%u: %s\n", idx++, dd); | |||||||||
} else | } else | |||||||||
printf("%s\n", dumpdev); | printf("%s\n", dumpdev); | |||||||||
/* If netdump is enabled, print the configuration parameters. */ | /* If netdump is enabled, print the configuration parameters. */ | |||||||||
Done Inline Actions
markj: | ||||||||||
if (verbose) { | if (verbose) { | |||||||||
fd = open(_PATH_NETDUMP, O_RDONLY); | if (netdump_fd == -1) | |||||||||
if (fd < 0) { | netdump_fd = open(_PATH_NETDUMP, O_RDONLY); | |||||||||
if (netdump_fd < 0) { | ||||||||||
if (errno != ENOENT) | if (errno != ENOENT) | |||||||||
err(EX_OSERR, "opening %s", _PATH_NETDUMP); | err(EX_OSERR, "opening %s", _PATH_NETDUMP); | |||||||||
return; | return; | |||||||||
} | } | |||||||||
if (ioctl(fd, DIOCGKERNELDUMP, &ndconf) != 0) { | if (ioctl(netdump_fd, DIOCGKERNELDUMP, &ndconf) != 0) { | |||||||||
if (errno != ENXIO) | if (errno != ENXIO) | |||||||||
err(EX_OSERR, "ioctl(DIOCGKERNELDUMP)"); | err(EX_OSERR, "ioctl(DIOCGKERNELDUMP)"); | |||||||||
(void)close(fd); | (void)close(netdump_fd); | |||||||||
return; | return; | |||||||||
} | } | |||||||||
printf("server address: %s\n", | printf("server address: %s\n", | |||||||||
inet_ntop(ndconf.kda_af, &ndconf.kda_server, ip, | inet_ntop(ndconf.kda_af, &ndconf.kda_server, ip, | |||||||||
sizeof(ip))); | sizeof(ip))); | |||||||||
printf("client address: %s\n", | printf("client address: %s\n", | |||||||||
inet_ntop(ndconf.kda_af, &ndconf.kda_client, ip, | inet_ntop(ndconf.kda_af, &ndconf.kda_client, ip, | |||||||||
sizeof(ip))); | sizeof(ip))); | |||||||||
printf("gateway address: %s\n", | printf("gateway address: %s\n", | |||||||||
inet_ntop(ndconf.kda_af, &ndconf.kda_gateway, ip, | inet_ntop(ndconf.kda_af, &ndconf.kda_gateway, ip, | |||||||||
sizeof(ip))); | sizeof(ip))); | |||||||||
(void)close(fd); | (void)close(netdump_fd); | |||||||||
} | } | |||||||||
} | } | |||||||||
static int | static int | |||||||||
opendumpdev(const char *arg, char *dumpdev) | opendumpdev(const char *arg, char *dumpdev) | |||||||||
{ | { | |||||||||
int fd, i; | int fd, i; | |||||||||
▲ Show 20 Lines • Show All 224 Lines • Show Last 20 Lines |
We only use (and close) capcas if verbose is specified.