From patchwork Sun Aug 16 16:43:19 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zack Weinberg X-Patchwork-Id: 507708 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 95D51140129 for ; Mon, 17 Aug 2015 02:43:51 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=EV4CDjt6; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:from:message-id:date :mime-version:in-reply-to:content-type; q=dns; s=default; b=Bczk 91wjJYWWIAnZnJ1jRM0Lgkrn4H+bDUGOZM5GQB630CPrX8btT1KZmysDe9CucNZu wykhQS+DWgXb76gdWA5YZilCzfj0RLghtsp8zq+ucQ0BWk72bZDriabRKkAzUlSN WcZJuB6okzSBm4Y9+b0rdYGbmpOBfaAWz5CzLzw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:from:message-id:date :mime-version:in-reply-to:content-type; s=default; bh=b+3Pxj2HLo +XcQWwRcrV+0Pn1LU=; b=EV4CDjt68XxdlO2oGa0xtfYydYsljylXsTnvOxG1r6 AkXTMEaH7TfpwxC8x9mQmzX5UH2DVfpRXQO2Q6jmBUVO7+HM3JYYD4tRZ4FStWzm L4yPebaxL5jvk6eS/lHXnQDWWFu7yxnzAos6Ixm3RJypyd6EtK8omLR9D9rKjj54 k= Received: (qmail 99187 invoked by alias); 16 Aug 2015 16:43:42 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 99177 invoked by uid 89); 16 Aug 2015 16:43:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=4.4 required=5.0 tests=AWL, BAYES_50, FREEMAIL_FROM, RAR_ATTACHED, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-yk0-f177.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-type; bh=goI5pRtuaNcfoCSTCL5/bYakd4cAd4WDCZASOdgGvkU=; b=Br6200dhYJHggaYiGsXCayqC74/pg6KvgsnWerGmJisDBP3zJG9A+3IlojA5BjA1S9 vNNmEcWDGcQumNQEzWh88pMxcNg1qKTAyfhBPP+wZfUOLz8mrOej2DtU+wTSldtdifOC 6VRfplvy1oslyobKfIsJCXKH0mLZ+veiJ/v/Mh1xjr5lAPRdjmmMUCMi7TMzchs+Opi9 jA8E6kUb0lDGx4PcPl5Mjqpk7TMAsPgfNsbvdRnY2KldbKDBcSDyzMpjqDzqUXslDiTr MBTcWucwFBwK6VhOg9rIHhSxOf2jEzplo2zIvpql1NxEKTkR/qCVki0hA9+Px+Q3rAel 1Kuw== X-Received: by 10.170.150.7 with SMTP id r7mr55312842ykc.48.1439743413007; Sun, 16 Aug 2015 09:43:33 -0700 (PDT) Subject: Re: [PATCH RFC] explicit_bzero, again To: GNU C Library References: <55C7E246.3000006@panix.com> From: Zack Weinberg Message-ID: <55D0BDA7.40402@panix.com> Date: Sun, 16 Aug 2015 12:43:19 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.1.0 MIME-Version: 1.0 In-Reply-To: <55C7E246.3000006@panix.com> On 08/09/2015 07:29 PM, Zack Weinberg wrote: > I'd like to resume the discussion of adding explicit_bzero or similar > to glibc. This was originally proposed by Nick Mathewson last > December (https://sourceware.org/ml/libc-alpha/2014-12/msg00506.html) > and I have prepared an updated patchset, which is attached to this > message. One patch adds the function, and the second adds several > intra-libc uses; I suspect I haven't found all of the places where > it should be used. This updated patchset corrects a number of minor errors (thanks, Joseph); adds tests for the fortification of explicit_bzero; and revises the new manual section a little. zw Use explicit_bzero where appropriate I *believe* these are the only places where memset was being used to clear buffers containing sensitive data. The compiler probably couldn't optimize *all* of them out but it seems best to change them all. The legacy DES implementation wasn't bothering to clear its buffers, so I added that, mostly for consistency's sake. [ChangeLog:] YYYY-MM-DD Zack Weinberg * crypt/crypt-entry.c (__crypt_r): Clear key-dependent intermediate data before returning, using explicit_bzero. * crypt/md5-crypt.c (__md5_crypt_r): Likewise. * crypt/sha256-crypt.c (__sha256_crypt_r): Likewise. * crypt/sha512-crypt.c (__sha512_crypt_r): Likewise. --- crypt/crypt-entry.c | 11 +++++++++++ crypt/md5-crypt.c | 8 ++++---- crypt/sha256-crypt.c | 14 +++++++------- crypt/sha512-crypt.c | 14 +++++++------- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/crypt/crypt-entry.c b/crypt/crypt-entry.c index 7e655ba..47db32c 100644 --- a/crypt/crypt-entry.c +++ b/crypt/crypt-entry.c @@ -143,6 +143,17 @@ __crypt_r (key, salt, data) * And convert back to 6 bit ASCII */ _ufc_output_conversion_r (res[0], res[1], salt, data); + +#ifdef _LIBC + /* + * Erase key-dependent intermediate data. Data dependent only on + * the salt is not considered sensitive. + */ + explicit_bzero (ktab, sizeof (ktab)); + explicit_bzero (data->keysched, sizeof (data->keysched)); + explicit_bzero (res, sizeof (res)); +#endif + return data->crypt_3_buf; } weak_alias (__crypt_r, crypt_r) diff --git a/crypt/md5-crypt.c b/crypt/md5-crypt.c index 1b890bc..b95766d 100644 --- a/crypt/md5-crypt.c +++ b/crypt/md5-crypt.c @@ -292,13 +292,13 @@ __md5_crypt_r (key, salt, buffer, buflen) #ifndef USE_NSS __md5_init_ctx (&ctx); __md5_finish_ctx (&ctx, alt_result); - memset (&ctx, '\0', sizeof (ctx)); - memset (&alt_ctx, '\0', sizeof (alt_ctx)); + explicit_bzero (&ctx, sizeof (ctx)); + explicit_bzero (&alt_ctx, sizeof (alt_ctx)); #endif if (copied_key != NULL) - memset (copied_key, '\0', key_len); + explicit_bzero (copied_key, key_len); if (copied_salt != NULL) - memset (copied_salt, '\0', salt_len); + explicit_bzero (copied_salt, salt_len); free (free_key); return buffer; diff --git a/crypt/sha256-crypt.c b/crypt/sha256-crypt.c index d90e291..189183a 100644 --- a/crypt/sha256-crypt.c +++ b/crypt/sha256-crypt.c @@ -375,16 +375,16 @@ __sha256_crypt_r (key, salt, buffer, buflen) #ifndef USE_NSS __sha256_init_ctx (&ctx); __sha256_finish_ctx (&ctx, alt_result); - memset (&ctx, '\0', sizeof (ctx)); - memset (&alt_ctx, '\0', sizeof (alt_ctx)); + explicit_bzero (&ctx, sizeof (ctx)); + explicit_bzero (&alt_ctx, sizeof (alt_ctx)); #endif - memset (temp_result, '\0', sizeof (temp_result)); - memset (p_bytes, '\0', key_len); - memset (s_bytes, '\0', salt_len); + explicit_bzero (temp_result, sizeof (temp_result)); + explicit_bzero (p_bytes, key_len); + explicit_bzero (s_bytes, salt_len); if (copied_key != NULL) - memset (copied_key, '\0', key_len); + explicit_bzero (copied_key, key_len); if (copied_salt != NULL) - memset (copied_salt, '\0', salt_len); + explicit_bzero (copied_salt, salt_len); free (free_key); free (free_pbytes); diff --git a/crypt/sha512-crypt.c b/crypt/sha512-crypt.c index 9c581ab..bbd5e24 100644 --- a/crypt/sha512-crypt.c +++ b/crypt/sha512-crypt.c @@ -397,16 +397,16 @@ __sha512_crypt_r (key, salt, buffer, buflen) #ifndef USE_NSS __sha512_init_ctx (&ctx); __sha512_finish_ctx (&ctx, alt_result); - memset (&ctx, '\0', sizeof (ctx)); - memset (&alt_ctx, '\0', sizeof (alt_ctx)); + explicit_bzero (&ctx, sizeof (ctx)); + explicit_bzero (&alt_ctx, sizeof (alt_ctx)); #endif - memset (temp_result, '\0', sizeof (temp_result)); - memset (p_bytes, '\0', key_len); - memset (s_bytes, '\0', salt_len); + explicit_bzero (temp_result, sizeof (temp_result)); + explicit_bzero (p_bytes, key_len); + explicit_bzero (s_bytes, salt_len); if (copied_key != NULL) - memset (copied_key, '\0', key_len); + explicit_bzero (copied_key, key_len); if (copied_salt != NULL) - memset (copied_salt, '\0', salt_len); + explicit_bzero (copied_salt, salt_len); free (free_key); free (free_pbytes); -- 2.5.0