From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kdkib-0007wx-Bs for mharc-grub-devel@gnu.org; Fri, 13 Nov 2020 20:51:09 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:48778) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kdkiZ-0007tv-Sn for grub-devel@gnu.org; Fri, 13 Nov 2020 20:51:07 -0500 Received: from mail-qk1-x743.google.com ([2607:f8b0:4864:20::743]:41200) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kdkiX-0002nx-MI for grub-devel@gnu.org; Fri, 13 Nov 2020 20:51:07 -0500 Received: by mail-qk1-x743.google.com with SMTP id d9so11297379qke.8 for ; Fri, 13 Nov 2020 17:51:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references:reply-to :mime-version:content-transfer-encoding; bh=BoxOVywd67TIP6B61i7Ub7mkA9bSfh/FuAOxxLp/g00=; b=JgtgAZS5l3JFpj9P/UAGJowr9p748ZhYUtoIiHRYVnnI7XYHwESznIuCQ97s/SiDKC spDj2o7VcQHf+gQIJIfmGTYoWPGhd2g/Lv71slzEcw4qvg6ugkcMvmCRwTeRy8Ehp9Xs C4UrKSHKb5USut9TNbUfM2i8sHo20J33gvtG2YcCW5PnIZfuVxyFs+UHG3IhGN0lP9TI qNvh2Iy2nNNUnXtbMgGceb4LZ8wpgJy2iJOAE3WlN9gHiqg4ETDr2Yli6Mos5R2LGJnD cEduTI6pkemu5jnq5qttGDfjO2UmZ4rRbi4NGoVostobDIs5xQjHF3NJyHJCNqBTGIvQ vIvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:reply-to:mime-version:content-transfer-encoding; bh=BoxOVywd67TIP6B61i7Ub7mkA9bSfh/FuAOxxLp/g00=; b=XKQ3yF625IQkwnHVEQc4b2hAjEVj419jyvSCln4Fi8vK81OSmn7OEksGniX+B/OZng SF9znW1Us5sB2IeKo+FJrb7EYSr46qAT8ZJcrhKdqOOzFEYfx/Fom56S5UBEbWUpHUAa GcGysNO6uWU5Uo9YDxoQtlUw8Dp6kycdLFMikoBR9d0dj4UYpgGfNVUGn5dvEk6lucAK CcNcu/1t6z2w2Hw9kT9veEsnDbJ/JWRCE4P3wyFgavSE+R/zUMMW17rKjz0o33x3jnAt EoIY+9t+Vv84MTjotxMLGfXdo8dq2a3SicuXEynQ/Dgw86qWNR3F3O1+FBac595b20HI b9mA== X-Gm-Message-State: AOAM532h3MEF0kXhq8mKlMehMyo7thxrHbFwT5TZZ7Th8yvG9jgo3EqI 6b3RTmG2fxhpK5HzesJJ+Bhw6w== X-Google-Smtp-Source: ABdhPJy6foOYHtLLFUF1NCLIQCksFsSocY3Ids8lT8YVQ0AR51WXt4l5AJR8sA85eEi7K7ZierD7mQ== X-Received: by 2002:a37:8c82:: with SMTP id o124mr5015272qkd.410.1605318664702; Fri, 13 Nov 2020 17:51:04 -0800 (PST) Received: from crass-HP-ZBook-15-G2 ([136.49.182.175]) by smtp.gmail.com with ESMTPSA id j63sm8055811qke.67.2020.11.13.17.50.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Nov 2020 17:51:04 -0800 (PST) Date: Fri, 13 Nov 2020 19:50:55 -0600 From: Glenn Washburn To: James Bottomley Cc: The development of GNU GRUB , dovmurik@linux.vnet.ibm.com, Dov.Murik1@il.ibm.com, ashish.kalra@amd.com, brijesh.singh@amd.com, tobin@ibm.com, david.kaplan@amd.com, jon.grimm@amd.com, thomas.lendacky@amd.com, frankeh@us.ibm.com, "Dr . David Alan Gilbert" Subject: Re: [PATCH v2 1/3] cryptodisk: make the password getter and additional argument to recover_key Message-ID: <20201113195055.2a24dd79@crass-HP-ZBook-15-G2> In-Reply-To: <20201113222510.16958-2-jejb@linux.ibm.com> References: <20201113222510.16958-1-jejb@linux.ibm.com> <20201113222510.16958-2-jejb@linux.ibm.com> Reply-To: development@efficientek.com X-Mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=2607:f8b0:4864:20::743; envelope-from=development@efficientek.com; helo=mail-qk1-x743.google.com X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 14 Nov 2020 01:51:08 -0000 On Fri, 13 Nov 2020 14:25:08 -0800 James Bottomley wrote: > For AMD SEV environments, the grub boot password has to be retrieved > from a given memory location rather than prompted for. This means > that the standard password getter needs to be replaced with one that > gets the passphrase from the SEV area and uses that instead. Adding > the password getter as a passed in argument to recover_key() makes > this possible. > > Signed-off-by: James Bottomley > > --- > > v2: add conditional prompting to geli.c > --- > grub-core/disk/cryptodisk.c | 2 +- > grub-core/disk/geli.c | 12 +++++++----- > grub-core/disk/luks.c | 12 +++++++----- > grub-core/disk/luks2.c | 12 +++++++----- > include/grub/cryptodisk.h | 6 +++++- > 5 files changed, 27 insertions(+), 17 deletions(-) > > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c > index a3d672f68..682f5a55d 100644 > --- a/grub-core/disk/cryptodisk.c > +++ b/grub-core/disk/cryptodisk.c > @@ -997,7 +997,7 @@ grub_cryptodisk_scan_device_real (const char > *name, grub_disk_t source) if (!dev) > continue; > > - err = cr->recover_key (source, dev); > + err = cr->recover_key (source, dev, grub_password_get); > if (err) > { > cryptodisk_close (dev); > diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c > index e9d23299a..3fece3f4a 100644 > --- a/grub-core/disk/geli.c > +++ b/grub-core/disk/geli.c > @@ -398,7 +398,8 @@ configure_ciphers (grub_disk_t disk, const char > *check_uuid, } > > static grub_err_t > -recover_key (grub_disk_t source, grub_cryptodisk_t dev) > +recover_key (grub_disk_t source, grub_cryptodisk_t dev, > + grub_passwd_cb *password_get) > { > grub_size_t keysize; > grub_uint8_t digest[GRUB_CRYPTO_MAX_MDLEN]; > @@ -438,11 +439,12 @@ recover_key (grub_disk_t source, > grub_cryptodisk_t dev) tmp = NULL; > if (source->partition) > tmp = grub_partition_get_name (source->partition); > - grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), > source->name, > - source->partition ? "," : "", tmp ? : "", > - dev->uuid); > + if (password_get == grub_password_get) > + grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), > source->name, > + source->partition ? "," : "", tmp ? : "", > + dev->uuid); > grub_free (tmp); > - if (!grub_password_get (passphrase, MAX_PASSPHRASE)) > + if (!password_get (passphrase, MAX_PASSPHRASE)) > return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not > supplied"); > /* Calculate the PBKDF2 of the user supplied passphrase. */ > diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c > index 59702067a..165f4a6bd 100644 > --- a/grub-core/disk/luks.c > +++ b/grub-core/disk/luks.c > @@ -152,7 +152,8 @@ configure_ciphers (grub_disk_t disk, const char > *check_uuid, > static grub_err_t > luks_recover_key (grub_disk_t source, > - grub_cryptodisk_t dev) > + grub_cryptodisk_t dev, > + grub_passwd_cb *password_get) > { > struct grub_luks_phdr header; > grub_size_t keysize; > @@ -187,11 +188,12 @@ luks_recover_key (grub_disk_t source, > tmp = NULL; > if (source->partition) > tmp = grub_partition_get_name (source->partition); > - grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), > source->name, > - source->partition ? "," : "", tmp ? : "", > - dev->uuid); > + if (password_get == grub_password_get) > + grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), > source->name, > + source->partition ? "," : "", tmp ? : "", > + dev->uuid); I don't really like this added if statement. It feels hackish. I also don't have an unambiguously better alternative. What feels like an imperfect alternative is to define a function grub_password_prompt(char buf[], unsigned buf_size, const char *prompt), which calls grub_password_get after displaying the string prompt. Typedef grub_passwd_cb to grub_password_prompt's signature. Then some grub_passwd_cb implementations can decide when not to use the given prompt. Perhaps grub_password_prompt should have snprintf's signature to be more complete and combine the grub_printf_ and grub_password_get. Any other ideas? Glenn