From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (NAM10-BN7-obe.outbound.protection.outlook.com [40.107.92.54]) by mx.groups.io with SMTP id smtpd.web10.270.1674857109163816443 for ; Fri, 27 Jan 2023 14:05:09 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nvidia.com header.s=selector2 header.b=D1cvIflH; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: nvidia.com, ip: 40.107.92.54, mailfrom: jbobek@nvidia.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LX4jIL3iixySyQyT+u/j1TX8OQ2djx/F6YkQ4M915DjczQTh/IfdIpCLu6S005sk8saQitn7mkossz4lipXChfyy8JVjqH+kdi6JgRybWK7Efo6LGmZWf10wsl3r4/ntJj7wKG5kpNQHXQigtjMEEOgHNNJUeHwrzSdiI8RtUHSAKEM2krKTMtAyMu6Eg/pb0yiQD8UcD5niP9fh2vJClsByylKkTYLiKaV91WC6gvE0enZv7+AFG27BzsFgmNebBuSdzOsCIY17AEZRPl8jyyre/t6YttoFdqgdCqAFkeQFRssP+57AO+leYl2bfm0K014ByMD5POr9wizj8TPC7A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=sbBbExRiN4EIppCG9tnrT/uOI6eHXUQ78TXVmTgBlFw=; b=h/vcG8AELwxCk73T7o3XmO2X9v/1PPgX1XBtmTmSB74i/g870BIpgtOO1hCBjvti0C0GlyAho7ZC5I/Ib5oonpr/EC3KUxKTudWjQamEu8A1p9ZjOVJkmqFDCmoKPm9omB9CRQo7v+dmAEpLD3w/yAPtZlL1yIiOjfCqNxmLaftB+iqSk1+u3l3XUkoUIiZbCGbjeREaMvaLblmMYoKhtPyWa8obi0CSnbnv81+VJMooq6iDre8Uw9NZWfgEawX60z33aPBb96JQ3n/4P2T829rj5DGrRj6Y/vzOwf+nPVfXgR39jQTkcfNAxwIdTh6etoyCVOdCRYEmG2EYdQR6Tg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 216.228.117.160) smtp.rcpttodomain=outlook.com smtp.mailfrom=nvidia.com; dmarc=pass (p=reject sp=reject pct=100) action=none header.from=nvidia.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=sbBbExRiN4EIppCG9tnrT/uOI6eHXUQ78TXVmTgBlFw=; b=D1cvIflHm9bBzoz6pDbyU4QBVUK0y+iaOY0VGdJw0HKazEMNtYgUR/aDksyJH7QUuslkUCcQYA5Ql705QhVoUvPgXhIDsXEZEBia8rNgem6D4yXafXlx08NWBrR1kfUDqXEMejuO9iSlkUGmzWii8Z/qHxy35XvUgu3nxbSGXN8MK29+2thoNYvWv2RWm1Tev0FzSF7wyuddO9/EH18XEoXJg2AeDskfIGgdTSHHGHRqXwBI2ICcpKrB5llKf0ryj4SMkfmjlPlAEaO/ugDacY1YrY4zPvWQfIQcR/d4Mr4IL0oS/DrsOM6EhDpYFRTlLkod04AmH/A/2Hiy1fd5oQ== Received: from CY5PR17CA0040.namprd17.prod.outlook.com (2603:10b6:930:12::34) by SN7PR12MB7910.namprd12.prod.outlook.com (2603:10b6:806:34b::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6043.22; Fri, 27 Jan 2023 22:05:06 +0000 Received: from CY4PEPF0000C97D.namprd02.prod.outlook.com (2603:10b6:930:12:cafe::d9) by CY5PR17CA0040.outlook.office365.com (2603:10b6:930:12::34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6043.23 via Frontend Transport; Fri, 27 Jan 2023 22:05:06 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 216.228.117.160) smtp.mailfrom=nvidia.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=nvidia.com; Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates 216.228.117.160 as permitted sender) receiver=protection.outlook.com; client-ip=216.228.117.160; helo=mail.nvidia.com; pr=C Received: from mail.nvidia.com (216.228.117.160) by CY4PEPF0000C97D.mail.protection.outlook.com (10.167.241.136) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6043.16 via Frontend Transport; Fri, 27 Jan 2023 22:05:06 +0000 Received: from rnnvmail201.nvidia.com (10.129.68.8) by mail.nvidia.com (10.129.200.66) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.36; Fri, 27 Jan 2023 14:05:02 -0800 Received: from localhost (10.126.230.37) by rnnvmail201.nvidia.com (10.129.68.8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.36; Fri, 27 Jan 2023 14:05:01 -0800 References: <20230120225835.42733-1-jbobek@nvidia.com> <87bkmmxpkr.fsf@nvidia.com> User-agent: mu4e 1.4.15; emacs 27.1 From: "Jan Bobek" To: Sean Brogan CC: "devel@edk2.groups.io" , "jiewen.yao@intel.com" , Sean Brogan , "Laszlo Ersek" Subject: Re: [edk2-devel] [PATCH v1 0/4] Don't require self-signed PK in setup mode In-Reply-To: Date: Fri, 27 Jan 2023 15:05:00 -0700 Message-ID: <878rhny6pf.fsf@nvidia.com> MIME-Version: 1.0 Return-Path: jbobek@nvidia.com X-Originating-IP: [10.126.230.37] X-ClientProxiedBy: rnnvmail201.nvidia.com (10.129.68.8) To rnnvmail201.nvidia.com (10.129.68.8) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CY4PEPF0000C97D:EE_|SN7PR12MB7910:EE_ X-MS-Office365-Filtering-Correlation-Id: 730fbc7f-4951-49d7-252d-08db00b28c95 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: CadLLJf234HXP5bIQjnWGbkDWjzzcVveSp7ifwzUGAf/APOpE6VNlXYhF464b5mscPBlTOl+uqsDmCOAH4tAzlD76sQvIbsFq4lr/6c72L6QkvjJfcZS/C9wuFKjav48iXdZfOvGkcClq/yUb2/wdkqfEq7YCF6JPbIORxkhyweO7kkLrqJogmZL6KL+RCqN9wP5xn7FyCeOmTQ8nx9tthc7cDw0kBSAbLbpESgeDDohPRCJObft7vfRZ05TEodal5nk0O8E8GAGefT6FqhShGjabO1TdccwPlZYnAhHWdcrjF13bLM/0X9eRxidL2P/nI0fAvVHz5o2lMW4p8gxy5HBsfHb8c7LI5/jIsrG8dZDvxMSG3Z5e5DqbRkrI2W4ZTUtOQQ8CUZcnfCskb+/ITKF1gLhQNpjX1r3ycUZlYTZ97cuhwdK+e89I79R/+OSg22S821NYB4CWjrw7a2/zIL8C0w+8MSuDBcqhyCdi4rIavtQjAEqmvA6JAAFvLQlppjs1sDtl0q1yPS+mNw+TA4mvhjP+c0JvUZxRDoz+clicf9dOMKRP00yia4Hcb+WSvCsnjPvHxDze2J0H/k5vjGlgDu/jpO6erCF8WzxmzL9O8hmZzDD9a6PcqufN4GxlJPc2jjGwG4SMij1hxUW818nF68x4sK4MphnRam7apKhWxSi1I5DIQ3XfRjQPNoq/HM4g2qYvXNV4PP3ELg4qyEXaYi1eX8xwVI/REiF4n6JIH3maZ9P18ht8753AlqA96b8GqCBKQ7FarcR3e1a/Oeg8Bn42vbsNyahc2WiOVGZ1s30dFSrtQlIgXI1Hhux0oP7+YMhEjcrsEfzkJNRdw== X-Forefront-Antispam-Report: CIP:216.228.117.160;CTRY:US;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:mail.nvidia.com;PTR:dc6edge1.nvidia.com;CAT:NONE;SFS:(13230025)(4636009)(39860400002)(346002)(136003)(376002)(396003)(451199018)(36840700001)(40470700004)(40460700003)(7636003)(356005)(2906002)(36860700001)(2616005)(8936002)(19627235002)(53546011)(86362001)(40480700001)(426003)(186003)(16526019)(83380400001)(26005)(32650700002)(6916009)(4326008)(8676002)(70206006)(336012)(36756003)(5660300002)(70586007)(316002)(66899018)(54906003)(966005)(82740400003)(45080400002)(478600001)(41300700001)(82310400005);DIR:OUT;SFP:1101; X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jan 2023 22:05:06.4726 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 730fbc7f-4951-49d7-252d-08db00b28c95 X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.117.160];Helo=[mail.nvidia.com] X-MS-Exchange-CrossTenant-AuthSource: CY4PEPF0000C97D.namprd02.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB7910 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable > I read your replacement a little different. > > - if ((InCustomMode () && UserPhysicalPresent ()) || ((mPlatformMode =3D= =3D SETUP_MODE) && !IsPk)) { > > with > > + if ( (InCustomMode () && UserPhysicalPresent ()) > + || ( (mPlatformMode =3D=3D SETUP_MODE) > + && !(FeaturePcdGet (PcdRequireSelfSignedPk) && IsPk))) > + { > > > In the upper part you replaced it says !IsPk. What am i missing? > > > If a payload was in this function targeting a KEK change with no PK > installed it would go in the original IF condition. In the new code > it would because device is not in custom mode and the payload is not > targeted PK. Is it possible that you're missing the negation (i.e. exclamation mark) in front of the parethesis? The old code was !IsPk The new code is !(FeaturePcdGet (PcdRequireSelfSignedPk) && IsPk) If IsPk is FALSE, both of these evaluate to TRUE no matter what the PCD is. -Jan > On 1/25/2023 1:38 PM, Jan Bobek wrote: >> Hi Sean, >> >>> From looking over the patch 1/4 email i have a concern. >>> >>> In AuthService.c on the conditional change you made. Aren't we losing >>> a case where we are evaluating a nonPK payload signed by the PK? >>> Given the system is in SetupMode that means there is no PK but is this >>> the conditional path that is used when installing Secure boot keys in >>> reverse (DBX,DX,KEK,PK) order? >> Thanks for sharing your concern! They way I think about the change is >> that I've replaced the expression >> >> IsPk >> >> with >> >> FeaturePcdGet (PcdRequireSelfSignedPk) && IsPk >> >> and nothing else. When you look at it this way, it's fairly easy to >> break down how it affects the logic: >> >> 1. Assume PcdRequireSelfSignedPk is TRUE. In this case, the two >> expressions are exactly the same and no change in behavior occurs, >> just as we want. >> >> 2. Assume IsPk is FALSE. In this case, both expressions evaluate to >> FALSE, no matter what the PCD is configured to. That's also good, >> because we don't want to change how non-PK payloads are handled. >> >> 3. In fact, the only change in behavior that occurs is when >> PcdRequireSelfSignedPk is FALSE and IsPk is TRUE; here the former >> expression would be TRUE, whereas the latter is FALSE. That's exactly >> what we want: we wish to enter the first block of the if-else branch >> (which changes the variable similarly to when we're in custom mode) >> rather than falling through to the third block (which checks the >> self-signature). >> >> To directly answer your question, I don't think the behavior changes at >> all when processing non-PK payloads, by virtue of IsPk being FALSE and >> what I said in point (2.) above. >> >> Additionally, yes, the first block of the if-else branch is exactly the >> path that's taken when enrolling KEK/DB/DBX in Setup mode, and one that >> has always been available even without Custom mode. It used to be that >> you couldn't use this path to enroll PK without Custom mode (precisely >> because of !IsPk in the condition), but I'm hoping to enable this path >> with my patch. >> >> Let me know if I haven't answered or misunderstood your question. >> >>> Is there testing you have done? This code should be pretty easy to do >>> host based unit testing on. Any chance you have authored that to >>> confirm use cases are not unexpectedly impacted? Example of host >>> based unit test of library is here: >>> edk2/SecurityPkg/Library/SecureBootVariableLib/UnitTest at master =C2=B7 >>> tianocore/edk2 >>> (github.com) >> I've done an ad-hoc test by commenting out the switch to/from Custom >> mode in EnrollFromDefaultKeysApp from SecurityPkg, booting in Setup mode >> and using the modified app to enroll the keys. You could do a similar >> test from the OS, but in my case this was more straightforward. >> >> I am aware of the host-based unit testing library in edk2, and I agree >> that it would be great to have the code in AuthVariableLib tested for >> all the different cases. However, I don't have any such tests right now, >> and while I'm willing to potentially look into writing some, I'd have to >> do it more or less on the side, meaning it could take a while. >> >> Best, >> -Jan >> >>> On 1/22/2023 10:13 PM, Yao, Jiewen wrote: >>> >>> Hi Sean >>> I would like to hear your feedback, since it is a little different from= the original MSFT patch. >>> >>> Would you please take a look? >>> >>> Thank you >>> Yao, Jiewen >>> >>> >>> >>> -----Original Message----- >>> From: Jan Bobek >>> Sent: Saturday, January 21, 2023 6:59 AM >>> To: devel@edk2.groups.io >>> Cc: Jan Bobek ; Laszlo Ers= ek ; Yao, >>> Jiewen >>> Subject: [PATCH v1 0/4] Don't require self-signed PK in setup mode >>> >>> Hi all, >>> >>> I'm sending out v1 of my patch series that addresses a UEFI spec >>> non-compliance when enrolling PK in setup mode. Additional info can be >>> found in bugzilla [1]; the changes are split into 4 patches as >>> suggested by Laszlo Ersek in comment #4. >>> >>> I've based my work on the patch by Matthew Carlson; I've credited him >>> with co-authorship of the first patch even though in the end I decided >>> to do the implementation a bit differently. >>> >>> Comments & reviews welcome! >>> >>> Cheers, >>> -Jan >>> >>> References: >>> 1. https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2F= bugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2506&data=3D05%7C01%7Cjbobek%4= 0nvidia.com%7Cf2eff15ce44945cc0b4e08db00ad6625%7C43083d15727340c1b7db39efd9= ccc17a%7C0%7C0%7C638104516992386171%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA= wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata= =3DPXam5BVxMmUwgRdGsq1RcnwC8yb8nmOzcLz3oKica7s%3D&reserved=3D0 >>> >>> Jan Bobek (4): >>> SecurityPkg: limit verification of enrolled PK in setup mode >>> OvmfPkg: require self-signed PK when secure boot is enabled >>> ArmVirtPkg: require self-signed PK when secure boot is enabled >>> SecurityPkg: don't require PK to be self-signed by default >>> >>> SecurityPkg/SecurityPkg.dec | 7 +++++++ >>> ArmVirtPkg/ArmVirtCloudHv.dsc | 4 ++++ >>> ArmVirtPkg/ArmVirtQemu.dsc | 4 ++++ >>> ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 ++++ >>> OvmfPkg/Bhyve/BhyveX64.dsc | 3 +++ >>> OvmfPkg/CloudHv/CloudHvX64.dsc | 3 +++ >>> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 3 +++ >>> OvmfPkg/Microvm/MicrovmX64.dsc | 3 +++ >>> OvmfPkg/OvmfPkgIa32.dsc | 3 +++ >>> OvmfPkg/OvmfPkgIa32X64.dsc | 3 +++ >>> OvmfPkg/OvmfPkgX64.dsc | 3 +++ >>> SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf | 3 +++ >>> SecurityPkg/Library/AuthVariableLib/AuthService.c | 9 +++++++-- >>> 13 files changed, 50 insertions(+), 2 deletions(-)