From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 52901C433F5 for ; Sun, 5 Sep 2021 08:11:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2338D6056B for ; Sun, 5 Sep 2021 08:11:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233577AbhIEIMm (ORCPT ); Sun, 5 Sep 2021 04:12:42 -0400 Received: from vmicros1.altlinux.org ([194.107.17.57]:50558 "EHLO vmicros1.altlinux.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231239AbhIEIMm (ORCPT ); Sun, 5 Sep 2021 04:12:42 -0400 Received: from imap.altlinux.org (imap.altlinux.org [194.107.17.38]) by vmicros1.altlinux.org (Postfix) with ESMTP id EBA1B72C8B0; Sun, 5 Sep 2021 11:11:37 +0300 (MSK) Received: from altlinux.org (sole.flsd.net [185.75.180.6]) by imap.altlinux.org (Postfix) with ESMTPSA id C17354A46FC; Sun, 5 Sep 2021 11:11:37 +0300 (MSK) Date: Sun, 5 Sep 2021 11:11:37 +0300 From: Vitaly Chikunov To: Stefan Berger Cc: Mimi Zohar , Dmitry Kasatkin , linux-integrity@vger.kernel.org, Bruno Meneguele Subject: Re: [PATCH ima-evm-utils v4] evmctl: Use secure heap for private keys and passwords Message-ID: <20210905081137.nsslgto7mqsfxn2b@altlinux.org> References: <20210904105054.3388329-1-vt@altlinux.org> MIME-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org Stefan, On Sat, Sep 04, 2021 at 09:10:50PM -0400, Stefan Berger wrote: > > On 9/4/21 6:50 AM, Vitaly Chikunov wrote: > > After CRYPTO_secure_malloc_init OpenSSL will automatically store private > > keys in secure heap. OPENSSL_secure_malloc(3): > > > > If a secure heap is used, then private key BIGNUM values are stored > > there. This protects long-term storage of private keys, but will not > > necessarily put all intermediate values and computations there. > > > > Additionally, we try to keep user passwords in secure heap too. > > This facility is only available since OpenSSL_1_1_0-pre1. > > Signed-off-by: Vitaly Chikunov > > Reviewed-by: Bruno Meneguele > > Cc: Stefan Berger > > --- > > Change since v3: > > - Undo secure heap handling from (file2bin and) calc_evm_hmac since this > > is debugging tool only. Add comment about this. > > - Since there is only code removals and new comments I keep Reviewed-by > > tag. > > > > src/evmctl.c | 97 +++++++++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 85 insertions(+), 12 deletions(-) > > > > @@ -2596,15 +2616,41 @@ static struct option opts[] = { > > }; > > +/* > > + * Copy password from optarg into secure heap, so it could be > > + * freed in the same way as a result of get_password(). > > + */ > > +static char *optarg_password(char *optarg) > > +{ > > > Mimi applied my patch that takes the password from an environment variable > to the next-testing branch. The code there (imaevm_params.keypassš = > getenv("..."))would have to change as well calling this function here. Even > though the man page of getenv says that 'The caller must take care not to > modify this string, since that would change the environment of the process' > we should be able to overwrite the env variable's password value just like > here. Maybe for this purpose rename this function to dup_password() ? `/proc//environ' is never world readable (unlike `cmdline'), so we don't even need to overwrite it there. But, we can. (Btw, I found the 'EVMCTL_KEY_PASSWORD' patch.) > > @@ -2643,6 +2691,7 @@ int main(int argc, char *argv[]) > > ENGINE *eng = NULL; > > unsigned long keyid; > > char *eptr; > > + char *keypass = NULL; /* @secure heap */ > > #if !(OPENSSL_VERSION_NUMBER < 0x10100000) > > OPENSSL_init_crypto( > > @@ -2651,6 +2700,17 @@ int main(int argc, char *argv[]) > > #endif > > OPENSSL_INIT_ENGINE_ALL_BUILTIN, NULL); > > #endif > > +#if OPENSSL_VERSION_NUMBER > 0x10100000 > > + /* > > + * This facility is available since OpenSSL_1_1_0-pre1. > > > Shouldn't the comparison then not include 0x10100000? > > #if OPENSSL_VERSION_NUMBER >= 0x10100000 The number is from `include/openssl/opensslv.h' from the time when relevant commit is already applied. So, I use `>'. It seems good match for OpenSSL_1_1_0-pre1 too: $ git grep OPENSSL_VERSION_NUMBER OpenSSL_1_1_0-pre1:include/openssl/opensslv.h OpenSSL_1_1_0-pre1:include/openssl/opensslv.h:# define OPENSSL_VERSION_NUMBER 0x10100001L Thanks,