d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

netdump: Don't store sensitive key data we don't need

Browse Source 
Prior to this revision, struct diocskerneldump_arg (and struct netdump_conf
with embedded diocskerneldump_arg before r347192), were copied in their
entirety to the global 'nd_conf' variable.  Also prior to this revision,
de-configuring netdump would *not* remove the the key material from global
nd_conf.

As part of Encrypted Kernel Crash Dumps (EKCD), which was developed
contemporaneously with netdump but happened to land first, the
diocskerneldump_arg structure will contain sensitive key material
(kda_key[]) when encrypted dumps are configured.

Netdump doesn't have any use for the key data -- encryption is handled in
the core dumper code -- so in this revision, we no longer store it.

Unfortunately, I think this leak dates to the initial import of netdump in
r333283; so it's present in FreeBSD 12.0.

Fortunately, the impact *seems* relatively minor.  Any new *netdump*
configuration would overwrite the key material; for active encrypted netdump
configurations, the key data stored was just a duplicate of the key material
already in the core dumper code; and no user interface (other than
/dev/kmem) actually exposed the leaked material to userspace.

Reviewed by:	markj, rpokala (earlier commit message)
MFC after:	2 weeks
Security:	yes (minor)
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D20233
This commit is contained in:
Conrad Meyer 2019-05-10 21:55:11 +00:00
parent 54bb7ac0c4
commit 6144b50f8b
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=347467
1 changed files with 26 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
sys/netinet/netdump/netdump_client.c
View File

@ -119,10 +119,16 @@ static uint64_t rcvd_acks;
CTASSERT(sizeof(rcvd_acks) * NBBY == NETDUMP_MAX_IN_FLIGHT);
/* Configuration parameters. */
static struct diocskerneldump_arg nd_conf;
#define nd_server nd_conf.kda_server.in4
#define nd_client nd_conf.kda_client.in4
#define nd_gateway nd_conf.kda_gateway.in4
static struct {
char ndc_iface[IFNAMSIZ];
union kd_ip ndc_server;
union kd_ip ndc_client;
union kd_ip ndc_gateway;
uint8_t ndc_af;
} nd_conf;
#define nd_server nd_conf.ndc_server.in4
#define nd_client nd_conf.ndc_client.in4
#define nd_gateway nd_conf.ndc_gateway.in4
/* General dynamic settings. */
static struct ether_addr nd_gw_mac;
@ -1087,8 +1093,20 @@ netdump_configure(struct diocskerneldump_arg *conf, struct thread *td)
return (ENODEV);
nd_ifp = ifp;
netdump_reinit(ifp);
memcpy(&nd_conf, conf, sizeof(nd_conf));
#define COPY_SIZED(elm) do { \
_Static_assert(sizeof(nd_conf.ndc_ ## elm) == \
sizeof(conf->kda_ ## elm), "elm " __XSTRING(elm) " mismatch"); \
memcpy(&nd_conf.ndc_ ## elm, &conf->kda_ ## elm, \
sizeof(nd_conf.ndc_ ## elm)); \
} while (0)
COPY_SIZED(iface);
COPY_SIZED(server);
COPY_SIZED(client);
COPY_SIZED(gateway);
COPY_SIZED(af);
#undef COPY_SIZED
nd_enabled = 1;
return (0);
}
@ -1193,7 +1211,7 @@ netdump_ioctl(struct cdev *dev __unused, u_long cmd, caddr_t addr,
error = ENXIO;
break;
}
if (nd_conf.kda_af != AF_INET) {
if (nd_conf.ndc_af != AF_INET) {
error = EOPNOTSUPP;
break;
}
@ -1225,7 +1243,7 @@ netdump_ioctl(struct cdev *dev __unused, u_long cmd, caddr_t addr,
memcpy(&conf->kda_server, &nd_server, sizeof(nd_server));
memcpy(&conf->kda_client, &nd_client, sizeof(nd_client));
memcpy(&conf->kda_gateway, &nd_gateway, sizeof(nd_gateway));
conf->kda_af = nd_conf.kda_af;
conf->kda_af = nd_conf.ndc_af;
conf = NULL;
break;
@ -1397,7 +1415,7 @@ netdump_modevent(module_t mod __unused, int what, void *priv __unused)
bzero(&kda, sizeof(kda));
kda.kda_index = KDA_REMOVE_DEV;
(void)dumper_remove(nd_conf.kda_iface, &kda);
(void)dumper_remove(nd_conf.ndc_iface, &kda);
netdump_mbuf_drain();
nd_enabled = 0;