diff options
author | Conrad Meyer <cem@FreeBSD.org> | 2019-05-10 21:55:11 +0000 |
---|---|---|
committer | Conrad Meyer <cem@FreeBSD.org> | 2019-05-10 21:55:11 +0000 |
commit | 6144b50f8bcbeaf2ba9351ed0cc0ea75915d15db (patch) | |
tree | 23b4dcde7ae390db9fb768886010a0396443d9d1 /sys/netinet/netdump | |
parent | 54bb7ac0c4bc49c75c62871bd86a5a158bf84c73 (diff) | |
download | src-6144b50f8bcbeaf2ba9351ed0cc0ea75915d15db.tar.gz src-6144b50f8bcbeaf2ba9351ed0cc0ea75915d15db.zip |
netdump: Don't store sensitive key data we don't need
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
Notes
Notes:
svn path=/head/; revision=347467
Diffstat (limited to 'sys/netinet/netdump')
-rw-r--r-- | sys/netinet/netdump/netdump_client.c | 34 |
1 files changed, 26 insertions, 8 deletions
diff --git a/sys/netinet/netdump/netdump_client.c b/sys/netinet/netdump/netdump_client.c index 7aff10609c39..7475dcd0487c 100644 --- a/sys/netinet/netdump/netdump_client.c +++ b/sys/netinet/netdump/netdump_client.c @@ -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; |