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;
 
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to