From owner-svn-src-head@freebsd.org Fri Mar 31 00:08:20 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 07739D223AC; Fri, 31 Mar 2017 00:08:20 +0000 (UTC) (envelope-from allanjude@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [96.47.72.37]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id BD262850; Fri, 31 Mar 2017 00:08:19 +0000 (UTC) (envelope-from allanjude@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v2V073IT043878; Fri, 31 Mar 2017 00:07:03 GMT (envelope-from allanjude@FreeBSD.org) Received: (from allanjude@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v2V073nn043874; Fri, 31 Mar 2017 00:07:03 GMT (envelope-from allanjude@FreeBSD.org) Message-Id: <201703310007.v2V073nn043874@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: allanjude set sender to allanjude@FreeBSD.org using -f From: Allan Jude Date: Fri, 31 Mar 2017 00:07:03 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r316312 - head/sys/geom/eli X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 Mar 2017 00:08:20 -0000 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;