From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web12.10709.1637671111583950126 for ; Tue, 23 Nov 2021 04:38:31 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=LOZLxPxb; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1637671110; 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=OONTKpnFlScNfwk3xpYIKmi6pyvFDEnFtlANrz0RnOs=; b=LOZLxPxbne7ysxpHcmKYtMs8VwiaqBftWGVzDMFgzeD3XByPTLXhiOZ1a9f5Fe1tBDrEtK gP7amQXmfl82tvwndSuEp2rFn5sW4p2PBS38Z8ccPFEM5F4DkuYCFTa1jqfJPtIkczr5Qj 6Kvj4e3gspCMelPwI8mUfLqtW8mQ7Zg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-440-6TMSUZQFMRKTIyiwm1I9IA-1; Tue, 23 Nov 2021 07:38:25 -0500 X-MC-Unique: 6TMSUZQFMRKTIyiwm1I9IA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id EB60D1923762; Tue, 23 Nov 2021 12:38:23 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.192.79]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 68F4C60C21; Tue, 23 Nov 2021 12:38:23 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id AD01E1800939; Tue, 23 Nov 2021 13:38:21 +0100 (CET) Date: Tue, 23 Nov 2021 13:38:21 +0100 From: "Gerd Hoffmann" To: "Yao, Jiewen" Cc: "Xu, Min M" , "devel@edk2.groups.io" , Ard Biesheuvel , "Justen, Jordan L" , Brijesh Singh , Erdem Aktas , James Bottomley , Tom Lendacky Subject: Re: [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx Message-ID: <20211123123821.q4fanslttg72n2r3@sirius.home.kraxel.org> References: <867e8a2aaf28c308b20a659057217453c6e38e00.1635769996.git.min.m.xu@intel.com> <20211103063045.kmttoxyluifwo2bq@sirius.home.kraxel.org> <20211117151942.iqow75zq2lrn5xlc@sirius.home.kraxel.org> <20211119151130.g2wcnuhivt3lxvzi@sirius.home.kraxel.org> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=kraxel@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi, > > That totally makes sense. I expect TDVF Config-B will look alot like > > the existing AmdSev configuration variant which is stripped down too. > > [Jiewen] I don't think TDVF config-B will be like the AMD SEV is right statement. > TDVF and SEV are two different platforms. Yes, of course. But even though there are differences in both implementation and supported features both platforms have similar goals and there is quite some overlap in concepts too. > Intel mainly focuses on TDVF and we will let AMD defines the feature > set in SEV. They MAY be alike if possible. But difference is also > acceptable if there is architecture difference or different decision > in different company. Yes. Whenever they are close enough that merging them makes sense remains to be seen. > > But I don't see how dropping the PEI phase altogether helps much in > > stripping down the firmware image. The initialization currently handled > > by OvmfPkg/PlatformPei must happen somewhere else instead. Given SEC is > > a very restricted environment I don't expect the code can be shared > > easily, so we will probably end up with code duplication. Also two > > different boot workflows which I fear can easily introduce subtle bugs > > due to differences like a initialization order changes. This is what I > > see as maintenance problem. > > [Jiewen] I don't think this is right statement. > In Tiano history, there were security bugs exposed in PEI phase, even the PEI Core on FV processing. > I do see the value to skip PEI core. On the other hand there are probably more eyes are looking at PEI Core because it is used by a lot of platforms, increasing the chance that bugs are actually spotted. > Again, I am very familiar with non-PEI flow. Back to 10 years ago, I > was maintainer of a non-PEI platform (named DUET) and we jumped from > SEC to DXE directly. I don't see any maintenance problem. The maintenance problem isn't a non-PEI flow. If a platform chooses that -- fine. The problem having to maintain both PEI and non-PEI workflow. > [Jiewen] I think we are debating two different things. > Your statement that "config-B is similar to AmdSev" does not support > the statement "config-B should be adopt what AmdSev chooses". I never stated "config-B should be adopt what AmdSev chooses". I stated "TDVF boot workflow should be simlar to the other OVMF platforms to simplify maintenance". AmdSev is just an example showing that you can easily strip down the firmware build without putting the boot workflow upside down (which one of the things config-b wants too). > > I don't want question all that. I still don't see the point in dropping > > the PEI phase and make config-b work different that all other ovmf > > variants though. > > [Jiewen] My point is simple - Threat Model is different. > That causes security objective difference and design difference. Does not using PEI phase have any advantages from a security point of view (other than "not using PEI Core code which might have bugs")? > > The security workflow is a serious problem indeed. Not only for TDVF, > > also for OVMF in general, and other platforms too. We should certainly > > try to improve it. > > [Jiewen] If you look at how we state config-A and config-B again, you will find we made difference statement. > I copy it here again. > 1) Config-A is to keep current architecture, to maximum compatible with OVMF. And we don't remove VMM out of TCB. > 2) Config-B is to have a new TDVF design, to maximum satisfy the security requirement. And we remove VMM out of TCB. > > Because of the threat model difference, in config-A, we can safely > make some assumption that the VMM is benign and VMM will not input > malicious data. As such, we might not perform data validation. We just > trust VMM input. > > However, in config-B, VMM is malicious, which means we need be careful to NOT trust VMM at any time. > The code in config-A and config-B may do totally different thing to handle the difference situation. > > I don't think it is hidden assumption that if TDVF need do some check, then a generic OVMF need do this check. > If OVMF trusts the VMM, the check might be totally unnecessary. Do you have a concrete example for that? I can't think of a good reason to skip checks on OVMF. When we -- for example -- review and improve virtio drivers to make sure they can't be used by the VMM to exploit a TDVF guest: We surely would use the improved sanity checks on OVMF too, for better OVMF stability and also for better test coverage of the sanity checks. > > Hmm? Seeing TDVF as "other platform" is a rather strange view given > > that we are integrating tdx support into OVMF right now ... > > [Jiewen] This is how Intel views the "platform". > In history, we call this one binary mode is "multiple-platform" or "multiple-SKU". > That means we only have one binary, and this single binary can boot different platforms, which has similar CPU or silicon but different platform board design. > And there will be platform specific code flow, such as > Switch (PlatformId) { > Case PlatformA: > {do platformA init} > Case PlatformB: > {do platformB init} > } > > If you treat CC_TYPE to be platformID, it perfectly matches. Yes. We have that in quite a few places. IoLib for example. It's required for Config-A, obviously. So, what is your plan for IoLib for Config-B? > Also a platform may has extra module (a driver or an FV) to support > the platform specific feature. Or a platform may much simpler design > and skip some drivers. Sure. Config-A will need some drivers from OvmfPkg/AmdSev/ so both SEV and TDX work, whereas Config-B will not. > I even propose config-a skip PEI phase. Care to back your proposal by posting patches to the list? I think it's easier to discuss the advantages + disadvantages with concrete code at hand. > I am persuaded to let config-a adopt the OVMF way, because the threat model of config-A is same as the normal OVMF. > But config-B is NOT. > Different threat model drives different solution. > I completely don't understand why they must be same. I don't understand why you want separate them. Improving OvmfPkg security is good for both OVMF and TDVF. For TDVF it is clearly much more important due to the different threat model, but it is good for OVMF too. Likewise edk2 in general. > If you force me to add PEI to config-B. Finally, that causes TDVF config-B is compromised due to an issue in PEI. > Who will take the responsibility? Will you or RedHat take that? Bugs happen. There could also be bugs in the additional SEC code you need for platform setup in a non-PEI configuration ... take care, Gerd