Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 31 Mar 2017 00:07:03 +0000 (UTC)
From:      Allan Jude <allanjude@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r316312 - head/sys/geom/eli
Message-ID:  <201703310007.v2V073nn043874@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: allanjude
Date: Fri Mar 31 00:07:03 2017
New Revision: 316312
URL: https://svnweb.freebsd.org/changeset/base/316312

Log:
  sys/geom/eli: Switch bzero() to explicit_bzero() for sensitive data
  
  In GELI, anywhere we are zeroing out possibly sensitive data, like
  the metadata struct, the metadata sector (both contain the encrypted
  master key), the user key, or the master key, use explicit_bzero.
  
  Didn't touch the bzero() used to initialize structs.
  
  Reviewed by:	delphij, oshogbo
  Sponsored by:	ScaleEngine Inc.
  Differential Revision:	https://reviews.freebsd.org/D9809

Modified:
  head/sys/geom/eli/g_eli_ctl.c
  head/sys/geom/eli/g_eli_key.c
  head/sys/geom/eli/g_eli_key_cache.c

Modified: head/sys/geom/eli/g_eli_ctl.c
==============================================================================
--- head/sys/geom/eli/g_eli_ctl.c	Fri Mar 31 00:04:32 2017	(r316311)
+++ head/sys/geom/eli/g_eli_ctl.c	Fri Mar 31 00:07:03 2017	(r316312)
@@ -85,6 +85,11 @@ g_eli_ctl_attach(struct gctl_req *req, s
 		return;
 	}
 
+	if (*detach && *readonly) {
+		gctl_error(req, "Options -d and -r are mutually exclusive.");
+		return;
+	}
+
 	name = gctl_get_asciiparam(req, "arg0");
 	if (name == NULL) {
 		gctl_error(req, "No 'arg%u' argument.", 0);
@@ -104,44 +109,39 @@ g_eli_ctl_attach(struct gctl_req *req, s
 		return;
 	}
 	if (md.md_keys == 0x00) {
-		bzero(&md, sizeof(md));
+		explicit_bzero(&md, sizeof(md));
 		gctl_error(req, "No valid keys on %s.", pp->name);
 		return;
 	}
 
 	key = gctl_get_param(req, "key", &keysize);
 	if (key == NULL || keysize != G_ELI_USERKEYLEN) {
-		bzero(&md, sizeof(md));
+		explicit_bzero(&md, sizeof(md));
 		gctl_error(req, "No '%s' argument.", "key");
 		return;
 	}
 
 	error = g_eli_mkey_decrypt(&md, key, mkey, &nkey);
-	bzero(key, keysize);
+	explicit_bzero(key, keysize);
 	if (error == -1) {
-		bzero(&md, sizeof(md));
+		explicit_bzero(&md, sizeof(md));
 		gctl_error(req, "Wrong key for %s.", pp->name);
 		return;
 	} else if (error > 0) {
-		bzero(&md, sizeof(md));
+		explicit_bzero(&md, sizeof(md));
 		gctl_error(req, "Cannot decrypt Master Key for %s (error=%d).",
 		    pp->name, error);
 		return;
 	}
 	G_ELI_DEBUG(1, "Using Master Key %u for %s.", nkey, pp->name);
 
-	if (*detach && *readonly) {
-		bzero(&md, sizeof(md));
-		gctl_error(req, "Options -d and -r are mutually exclusive.");
-		return;
-	}
 	if (*detach)
 		md.md_flags |= G_ELI_FLAG_WO_DETACH;
 	if (*readonly)
 		md.md_flags |= G_ELI_FLAG_RO;
 	g_eli_create(req, mp, pp, &md, mkey, nkey);
-	bzero(mkey, sizeof(mkey));
-	bzero(&md, sizeof(md));
+	explicit_bzero(mkey, sizeof(mkey));
+	explicit_bzero(&md, sizeof(md));
 }
 
 static struct g_eli_softc *
@@ -362,8 +362,8 @@ g_eli_ctl_onetime(struct gctl_req *req, 
 	}
 
 	g_eli_create(req, mp, pp, &md, mkey, -1);
-	bzero(mkey, sizeof(mkey));
-	bzero(&md, sizeof(md));
+	explicit_bzero(mkey, sizeof(mkey));
+	explicit_bzero(&md, sizeof(md));
 }
 
 static void
@@ -549,8 +549,8 @@ g_eli_ctl_configure(struct gctl_req *req
 			    "Cannot store metadata on %s (error=%d).",
 			    prov, error);
 		}
-		bzero(&md, sizeof(md));
-		bzero(sector, pp->sectorsize);
+		explicit_bzero(&md, sizeof(md));
+		explicit_bzero(sector, pp->sectorsize);
 		free(sector, M_ELI);
 	}
 }
@@ -574,6 +574,11 @@ g_eli_ctl_setkey(struct gctl_req *req, s
 		gctl_error(req, "No 'arg%u' argument.", 0);
 		return;
 	}
+	key = gctl_get_param(req, "key", &keysize);
+	if (key == NULL || keysize != G_ELI_USERKEYLEN) {
+		gctl_error(req, "No '%s' argument.", "key");
+		return;
+	}
 	sc = g_eli_find_device(mp, name);
 	if (sc == NULL) {
 		gctl_error(req, "Provider %s is invalid.", name);
@@ -627,13 +632,6 @@ g_eli_ctl_setkey(struct gctl_req *req, s
 		md.md_iterations = *valp;
 	}
 
-	key = gctl_get_param(req, "key", &keysize);
-	if (key == NULL || keysize != G_ELI_USERKEYLEN) {
-		bzero(&md, sizeof(md));
-		gctl_error(req, "No '%s' argument.", "key");
-		return;
-	}
-
 	mkeydst = md.md_mkeys + nkey * G_ELI_MKEYLEN;
 	md.md_keys |= (1 << nkey);
 
@@ -641,9 +639,9 @@ g_eli_ctl_setkey(struct gctl_req *req, s
 
 	/* Encrypt Master Key with the new key. */
 	error = g_eli_mkey_encrypt(md.md_ealgo, key, md.md_keylen, mkeydst);
-	bzero(key, keysize);
+	explicit_bzero(key, keysize);
 	if (error != 0) {
-		bzero(&md, sizeof(md));
+		explicit_bzero(&md, sizeof(md));
 		gctl_error(req, "Cannot encrypt Master Key (error=%d).", error);
 		return;
 	}
@@ -651,10 +649,10 @@ g_eli_ctl_setkey(struct gctl_req *req, s
 	sector = malloc(pp->sectorsize, M_ELI, M_WAITOK | M_ZERO);
 	/* Store metadata with fresh key. */
 	eli_metadata_encode(&md, sector);
-	bzero(&md, sizeof(md));
+	explicit_bzero(&md, sizeof(md));
 	error = g_write_data(cp, pp->mediasize - pp->sectorsize, sector,
 	    pp->sectorsize);
-	bzero(sector, pp->sectorsize);
+	explicit_bzero(sector, pp->sectorsize);
 	free(sector, M_ELI);
 	if (error != 0) {
 		gctl_error(req, "Cannot store metadata on %s (error=%d).",
@@ -752,7 +750,7 @@ g_eli_ctl_delkey(struct gctl_req *req, s
 	sector = malloc(pp->sectorsize, M_ELI, M_WAITOK | M_ZERO);
 	for (i = 0; i <= g_eli_overwrites; i++) {
 		if (i == g_eli_overwrites)
-			bzero(mkeydst, keysize);
+			explicit_bzero(mkeydst, keysize);
 		else
 			arc4rand(mkeydst, keysize, 0);
 		/* Store metadata with destroyed key. */
@@ -769,8 +767,8 @@ g_eli_ctl_delkey(struct gctl_req *req, s
 		 */
 		(void)g_io_flush(cp);
 	}
-	bzero(&md, sizeof(md));
-	bzero(sector, pp->sectorsize);
+	explicit_bzero(&md, sizeof(md));
+	explicit_bzero(sector, pp->sectorsize);
 	free(sector, M_ELI);
 	if (*all)
 		G_ELI_DEBUG(1, "All keys removed from %s.", pp->name);
@@ -817,12 +815,12 @@ g_eli_suspend_one(struct g_eli_softc *sc
 	/*
 	 * Clear sensitive data on suspend, they will be recovered on resume.
 	 */
-	bzero(sc->sc_mkey, sizeof(sc->sc_mkey));
+	explicit_bzero(sc->sc_mkey, sizeof(sc->sc_mkey));
 	g_eli_key_destroy(sc);
-	bzero(sc->sc_akey, sizeof(sc->sc_akey));
-	bzero(&sc->sc_akeyctx, sizeof(sc->sc_akeyctx));
-	bzero(sc->sc_ivkey, sizeof(sc->sc_ivkey));
-	bzero(&sc->sc_ivctx, sizeof(sc->sc_ivctx));
+	explicit_bzero(sc->sc_akey, sizeof(sc->sc_akey));
+	explicit_bzero(&sc->sc_akeyctx, sizeof(sc->sc_akeyctx));
+	explicit_bzero(sc->sc_ivkey, sizeof(sc->sc_ivkey));
+	explicit_bzero(&sc->sc_ivctx, sizeof(sc->sc_ivctx));
 	mtx_unlock(&sc->sc_queue_mtx);
 	G_ELI_DEBUG(0, "Device %s has been suspended.", sc->sc_name);
 }
@@ -915,6 +913,11 @@ g_eli_ctl_resume(struct gctl_req *req, s
 		gctl_error(req, "No 'arg%u' argument.", 0);
 		return;
 	}
+	key = gctl_get_param(req, "key", &keysize);
+	if (key == NULL || keysize != G_ELI_USERKEYLEN) {
+		gctl_error(req, "No '%s' argument.", "key");
+		return;
+	}
 	sc = g_eli_find_device(mp, name);
 	if (sc == NULL) {
 		gctl_error(req, "Provider %s is invalid.", name);
@@ -929,26 +932,19 @@ g_eli_ctl_resume(struct gctl_req *req, s
 		return;
 	}
 	if (md.md_keys == 0x00) {
-		bzero(&md, sizeof(md));
+		explicit_bzero(&md, sizeof(md));
 		gctl_error(req, "No valid keys on %s.", pp->name);
 		return;
 	}
 
-	key = gctl_get_param(req, "key", &keysize);
-	if (key == NULL || keysize != G_ELI_USERKEYLEN) {
-		bzero(&md, sizeof(md));
-		gctl_error(req, "No '%s' argument.", "key");
-		return;
-	}
-
 	error = g_eli_mkey_decrypt(&md, key, mkey, &nkey);
-	bzero(key, keysize);
+	explicit_bzero(key, keysize);
 	if (error == -1) {
-		bzero(&md, sizeof(md));
+		explicit_bzero(&md, sizeof(md));
 		gctl_error(req, "Wrong key for %s.", pp->name);
 		return;
 	} else if (error > 0) {
-		bzero(&md, sizeof(md));
+		explicit_bzero(&md, sizeof(md));
 		gctl_error(req, "Cannot decrypt Master Key for %s (error=%d).",
 		    pp->name, error);
 		return;
@@ -966,8 +962,8 @@ g_eli_ctl_resume(struct gctl_req *req, s
 		wakeup(sc);
 	}
 	mtx_unlock(&sc->sc_queue_mtx);
-	bzero(mkey, sizeof(mkey));
-	bzero(&md, sizeof(md));
+	explicit_bzero(mkey, sizeof(mkey));
+	explicit_bzero(&md, sizeof(md));
 }
 
 static int

Modified: head/sys/geom/eli/g_eli_key.c
==============================================================================
--- head/sys/geom/eli/g_eli_key.c	Fri Mar 31 00:04:32 2017	(r316311)
+++ head/sys/geom/eli/g_eli_key.c	Fri Mar 31 00:07:03 2017	(r316312)
@@ -69,7 +69,7 @@ g_eli_mkey_verify(const unsigned char *m
 	g_eli_crypto_hmac(hmkey, sizeof(hmkey), mkey, G_ELI_DATAIVKEYLEN,
 	    chmac, 0);
 
-	bzero(hmkey, sizeof(hmkey));
+	explicit_bzero(hmkey, sizeof(hmkey));
 
 	/*
 	 * Compare calculated HMAC with HMAC from metadata.
@@ -97,7 +97,7 @@ g_eli_mkey_hmac(unsigned char *mkey, con
 	g_eli_crypto_hmac(hmkey, sizeof(hmkey), mkey, G_ELI_DATAIVKEYLEN,
 	    odhmac, 0);
 
-	bzero(hmkey, sizeof(hmkey));
+	explicit_bzero(hmkey, sizeof(hmkey));
 }
 
 /*
@@ -131,21 +131,21 @@ g_eli_mkey_decrypt(const struct g_eli_me
 		error = g_eli_crypto_decrypt(md->md_ealgo, tmpmkey,
 		    G_ELI_MKEYLEN, enckey, md->md_keylen);
 		if (error != 0) {
-			bzero(tmpmkey, sizeof(tmpmkey));
-			bzero(enckey, sizeof(enckey));
+			explicit_bzero(tmpmkey, sizeof(tmpmkey));
+			explicit_bzero(enckey, sizeof(enckey));
 			return (error);
 		}
 		if (g_eli_mkey_verify(tmpmkey, key)) {
 			bcopy(tmpmkey, mkey, G_ELI_DATAIVKEYLEN);
-			bzero(tmpmkey, sizeof(tmpmkey));
-			bzero(enckey, sizeof(enckey));
+			explicit_bzero(tmpmkey, sizeof(tmpmkey));
+			explicit_bzero(enckey, sizeof(enckey));
 			if (nkeyp != NULL)
 				*nkeyp = nkey;
 			return (0);
 		}
 	}
-	bzero(enckey, sizeof(enckey));
-	bzero(tmpmkey, sizeof(tmpmkey));
+	explicit_bzero(enckey, sizeof(enckey));
+	explicit_bzero(tmpmkey, sizeof(tmpmkey));
 	return (-1);
 }
 
@@ -175,7 +175,7 @@ g_eli_mkey_encrypt(unsigned algo, const 
 	 */
 	error = g_eli_crypto_encrypt(algo, mkey, G_ELI_MKEYLEN, enckey, keylen);
 
-	bzero(enckey, sizeof(enckey));
+	explicit_bzero(enckey, sizeof(enckey));
 
 	return (error);
 }

Modified: head/sys/geom/eli/g_eli_key_cache.c
==============================================================================
--- head/sys/geom/eli/g_eli_key_cache.c	Fri Mar 31 00:04:32 2017	(r316311)
+++ head/sys/geom/eli/g_eli_key_cache.c	Fri Mar 31 00:07:03 2017	(r316312)
@@ -117,7 +117,7 @@ g_eli_key_allocate(struct g_eli_softc *s
 	keysearch.gek_keyno = keyno;
 	ekey = RB_FIND(g_eli_key_tree, &sc->sc_ekeys_tree, &keysearch);
 	if (ekey != NULL) {
-		bzero(key, sizeof(*key));
+		explicit_bzero(key, sizeof(*key));
 		free(key, M_ELI);
 		key = ekey;
 		TAILQ_REMOVE(&sc->sc_ekeys_queue, key, gek_next);
@@ -174,7 +174,7 @@ g_eli_key_remove(struct g_eli_softc *sc,
 	RB_REMOVE(g_eli_key_tree, &sc->sc_ekeys_tree, key);
 	TAILQ_REMOVE(&sc->sc_ekeys_queue, key, gek_next);
 	sc->sc_ekeys_allocated--;
-	bzero(key, sizeof(*key));
+	explicit_bzero(key, sizeof(*key));
 	free(key, M_ELI);
 }
 
@@ -239,7 +239,7 @@ g_eli_key_destroy(struct g_eli_softc *sc
 
 	mtx_lock(&sc->sc_ekeys_lock);
 	if ((sc->sc_flags & G_ELI_FLAG_SINGLE_KEY) != 0) {
-		bzero(sc->sc_ekey, sizeof(sc->sc_ekey));
+		explicit_bzero(sc->sc_ekey, sizeof(sc->sc_ekey));
 	} else {
 		struct g_eli_key *key;
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703310007.v2V073nn043874>