From patchwork Thu Dec 8 14:56:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zack Weinberg X-Patchwork-Id: 704090 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 3tZJNT6lG6z9sCM for ; Fri, 9 Dec 2016 01:56:41 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="Rmef79cs"; 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:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; q=dns; s= default; b=vrAIayZgV9v14Np2oprEk4/Ocb6q2xo1iIvqJNXa+rYJ1KOK8CJkY +bvwUmNDtrexs+tFrGopeVuBNjJ4TRGDypEhV1rd2uI3e1co++/XtRbwI+wVWKmm H9E8PJrmK9BphUZfu02qzxLuztsPHO3Uzpl7I5mLC3+eo7kQLQzTes= 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:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; s=default; bh=W4H4YevJozLxtqbhewa2e9BIGv0=; b=Rmef79csa5KFkpJW6Vc//my/FUWJ BtE3os0wo9AXfRqSVx+MFWYrmpULYOIOCJT+aonDywFMnjJdYyC1PC5cSRDMifmc DTEcyupCpop+ghqK1OlMDCi6ppXB9fHON5GHee3D2oXrsyKLlWjEhA8JIzO6vGgZ v9zazZQJIWqgXiY= Received: (qmail 83319 invoked by alias); 8 Dec 2016 14:56:12 -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 82841 invoked by uid 89); 8 Dec 2016 14:56:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.7 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=salt, sensitive X-HELO: mailbackend.panix.com From: Zack Weinberg To: libc-alpha@sourceware.org Subject: [PATCH 3/3] Use explicit_bzero where appropriate Date: Thu, 8 Dec 2016 09:56:06 -0500 Message-Id: <20161208145606.3568-4-zackw@panix.com> In-Reply-To: <20161208145606.3568-3-zackw@panix.com> References: <20161208145606.3568-1-zackw@panix.com> <20161208145606.3568-2-zackw@panix.com> <20161208145606.3568-3-zackw@panix.com> MIME-Version: 1.0 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. * 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 a7dfccaa36..2d72691ceb 100644 --- a/crypt/crypt-entry.c +++ b/crypt/crypt-entry.c @@ -141,6 +141,17 @@ __crypt_r (const char *key, const char *salt, * 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 2243bc7aed..617ccd3ac1 100644 --- a/crypt/md5-crypt.c +++ b/crypt/md5-crypt.c @@ -288,13 +288,13 @@ __md5_crypt_r (const char *key, const char *salt, char *buffer, int 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 d768234879..53f00c041e 100644 --- a/crypt/sha256-crypt.c +++ b/crypt/sha256-crypt.c @@ -371,16 +371,16 @@ __sha256_crypt_r (const char *key, const char *salt, char *buffer, int 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 f404c88b20..86b63542ca 100644 --- a/crypt/sha512-crypt.c +++ b/crypt/sha512-crypt.c @@ -393,16 +393,16 @@ __sha512_crypt_r (const char *key, const char *salt, char *buffer, int 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);