From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1kddew-0007RR-87 for mharc-grub-devel@gnu.org; Fri, 13 Nov 2020 13:18:54 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:51480) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kdZ2y-0005XE-Jl for grub-devel@gnu.org; Fri, 13 Nov 2020 08:23:24 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:30878) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1kdZ2v-0003va-6U for grub-devel@gnu.org; Fri, 13 Nov 2020 08:23:24 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1605273800; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Wuc+qOW24qdAgvotSP0kJqKmMxCODfhf0CaHgZF+Qlg=; b=TOfUoTC2mPq8wQWMB+0gGC/Y1hxOvaiHKC7+Z92GuFXGXXi6lGy0OZFbU0g2hvpqq5tvP0 RxJDEWEdptpKxBIgKNjDwSIxxlU8tYUodFJydQc8vlEV7+d6+HMWpv53TNkvdg4m0G6DH0 PCwDnmaJEXSPMSsZ1bZpCSZ7lf5D5mk= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-40-og_Mtb-QNzO1kIE1U_i_TA-1; Fri, 13 Nov 2020 08:23:16 -0500 X-MC-Unique: og_Mtb-QNzO1kIE1U_i_TA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A8D3A1084D68; Fri, 13 Nov 2020 13:23:14 +0000 (UTC) Received: from work-vm (ovpn-114-160.ams2.redhat.com [10.36.114.160]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6518260BE2; Fri, 13 Nov 2020 13:23:12 +0000 (UTC) Date: Fri, 13 Nov 2020 13:23:09 +0000 From: "Dr. David Alan Gilbert" To: James Bottomley Cc: grub-devel@gnu.org, 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 Subject: Re: [PATCH 2/3] cryptodisk: add OS provided secret support Message-ID: <20201113132309.GN3251@work-vm> References: <20201113012206.24246-1-jejb@linux.ibm.com> <20201113012206.24246-3-jejb@linux.ibm.com> MIME-Version: 1.0 In-Reply-To: <20201113012206.24246-3-jejb@linux.ibm.com> User-Agent: Mutt/1.14.6 (2020-07-11) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dgilbert@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Received-SPF: pass client-ip=63.128.21.124; envelope-from=dgilbert@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/11/12 16:09:27 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] [fuzzy] X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Fri, 13 Nov 2020 13:18:52 -0500 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: Fri, 13 Nov 2020 13:23:25 -0000 * James Bottomley (jejb@linux.ibm.com) wrote: > Make use of the new OS provided secrets API so that if the new '-s' > option is passed in we try to extract the secret from the API rather > than prompting for it. > > The primary consumer of this is AMD SEV, which has been programmed to > provide an injectable secret to the encrypted virtual machine. OVMF > provides the secret area and passes it into the EFI Configuration > Tables. The grub EFI layer pulls the secret out and primes the > secrets API with it. The upshot of all of this is that a SEV > protected VM can do an encrypted boot with a protected boot secret. > > Signed-off-by: James Bottomley > --- > grub-core/disk/cryptodisk.c | 60 ++++++++++++++++++++++++++++++++++--- > include/grub/cryptodisk.h | 2 ++ > 2 files changed, 58 insertions(+), 4 deletions(-) > > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c > index 682f5a55d..02104aad4 100644 > --- a/grub-core/disk/cryptodisk.c > +++ b/grub-core/disk/cryptodisk.c > @@ -41,6 +41,7 @@ static const struct grub_arg_option options[] = > /* TRANSLATORS: It's still restricted to cryptodisks only. */ > {"all", 'a', 0, N_("Mount all."), 0, 0}, > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0, 0}, > + {"secret", 's', 0, N_("Get OS provisioned secret and mount all volumes encrypted with that secret"), 0, 0}, > {0, 0, 0, 0, 0, 0} > }; > > @@ -967,6 +968,10 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk) > > static int check_boot, have_it; > static char *search_uuid; > +static char *os_passwd; > + > +/* variable to hold the passed in secret area. */ > +static char *os_secret_area; > > static void > cryptodisk_close (grub_cryptodisk_t dev) > @@ -977,6 +982,17 @@ cryptodisk_close (grub_cryptodisk_t dev) > grub_free (dev); > } > > +static int > +os_password_get(char buf[], unsigned len) > +{ > + /* os_passwd should be null terminated, so just copy everything */ > + grub_strncpy(buf, os_passwd, len); > + /* and add a terminator just in case */ > + buf[len - 1] = 0; > + > + return 1; > +} > + > static grub_err_t > grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source) > { > @@ -996,8 +1012,17 @@ grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source) > return grub_errno; > if (!dev) > continue; > - > - err = cr->recover_key (source, dev, grub_password_get); > + > + if (os_passwd) > + { > + err = cr->recover_key (source, dev, os_password_get); > + if (err) > + /* if the key doesn't work ignore the access denied error */ > + grub_error_pop(); > + } > + else > + err = cr->recover_key (source, dev, grub_password_get); > + > if (err) > { > cryptodisk_close (dev); > @@ -1013,6 +1038,14 @@ grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source) > return GRUB_ERR_NONE; > } > > +grub_err_t > +grub_cryptodisk_set_secret (char *secret) > +{ > + os_secret_area = secret; > + > + return GRUB_ERR_NONE; > +} > + > #ifdef GRUB_UTIL > #include > grub_err_t > @@ -1089,7 +1122,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args) > { > struct grub_arg_list *state = ctxt->state; > > - if (argc < 1 && !state[1].set && !state[2].set) > + if (argc < 1 && !state[1].set && !state[2].set && !state[3].set) > return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required"); > > have_it = 0; > @@ -1107,6 +1140,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args) > > check_boot = state[2].set; > search_uuid = args[0]; > + os_passwd = NULL; > grub_device_iterate (&grub_cryptodisk_scan_device, NULL); > search_uuid = NULL; > > @@ -1117,11 +1151,28 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args) > else if (state[1].set || (argc == 0 && state[2].set)) > { > search_uuid = NULL; > + os_passwd = NULL; > check_boot = state[2].set; > grub_device_iterate (&grub_cryptodisk_scan_device, NULL); > search_uuid = NULL; > return GRUB_ERR_NONE; > } > + else if (state[3].set) > + { > + /* do we have a secret? */ > + if (os_secret_area == NULL) > + return grub_error (GRUB_ERR_INVALID_COMMAND, "No OS secret is provisioned"); > + > + os_passwd = os_secret_area; > + search_uuid = NULL; > + grub_device_iterate (&grub_cryptodisk_scan_device, NULL); > + os_passwd = NULL; > + > + if (!have_it) > + return grub_error (GRUB_ERR_ACCESS_DENIED, "SEV password failed to unlock any volumes"); That's the only place you break the generality and admit to it being a SEV password I think. Dave > + return GRUB_ERR_NONE; > + } > else > { > grub_err_t err; > @@ -1132,6 +1183,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args) > grub_size_t len; > > search_uuid = NULL; > + os_passwd = NULL; > check_boot = state[2].set; > diskname = args[0]; > len = grub_strlen (diskname); > @@ -1299,7 +1351,7 @@ GRUB_MOD_INIT (cryptodisk) > { > grub_disk_dev_register (&grub_cryptodisk_dev); > cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0, > - N_("SOURCE|-u UUID|-a|-b"), > + N_("SOURCE|-u UUID|-a|-b|-s"), > N_("Mount a crypto device."), options); > grub_procfs_register ("luks_script", &luks_script); > } > diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h > index 45dae5483..55c411754 100644 > --- a/include/grub/cryptodisk.h > +++ b/include/grub/cryptodisk.h > @@ -163,4 +163,6 @@ grub_util_get_geli_uuid (const char *dev); > grub_cryptodisk_t grub_cryptodisk_get_by_uuid (const char *uuid); > grub_cryptodisk_t grub_cryptodisk_get_by_source_disk (grub_disk_t disk); > > +grub_err_t grub_cryptodisk_set_secret(char *secret); > + > #endif > -- > 2.26.2 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK