From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web10.10421.1672998094885385324 for ; Fri, 06 Jan 2023 01:41:35 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=PN3cIPgj; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: jiewen.yao@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1672998094; x=1704534094; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=urIRgsRkRzPc2NbYZvcl3P0ksChBtLfSjm1jp/1LsUk=; b=PN3cIPgjFSjKMpjxRHkQoKLLtfr6MRk/YRPoSlU/IC+UDAP4xcyMAiTg 9xcixcST027EGtNrwOzgIWX86tTe+Gkja7BXcjOS1kDb3TI1QIRv+AqgO Gpn/i0pF7PuMjQq/KZbUfILIObKL0fG+wmlmiRIVscttbJ2563WSvtIHp wJVmZmdzIDllMHuZ4wH0K5han3B0cvxcRkSERNTQRS4g8i6wASR2QtTGT jlFAnb8N/aFg6PE6gExmmdLE1DU6kip60arnYtmsGhoIwOiwYkSWm99us A2y3i0/0GARDbxLNLKVYXYlaiBaJ1oGjzO2L/4HQ7axYOleaH+lQOoqLG g==; X-IronPort-AV: E=McAfee;i="6500,9779,10581"; a="322515223" X-IronPort-AV: E=Sophos;i="5.96,304,1665471600"; d="scan'208";a="322515223" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Jan 2023 01:41:31 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10581"; a="829855658" X-IronPort-AV: E=Sophos;i="5.96,304,1665471600"; d="scan'208";a="829855658" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga005.jf.intel.com with ESMTP; 06 Jan 2023 01:41:31 -0800 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Fri, 6 Jan 2023 01:41:30 -0800 Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Fri, 6 Jan 2023 01:41:30 -0800 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.102) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Fri, 6 Jan 2023 01:41:30 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OXYGl1WwNj2hLFwAFG4SxJYIg9+PyoKEVkOcCAH0KSsQ1hjgQe9HIUkQyth8oc4b+B7wIOJzz3Tj/e5vGRCrEO9xhhm0dSYmsHjD58WlNexhUzGRM2/4hYByAzel+INqUz0tgfXX/lpDgBpl34YeKZUpPxoxTaZhO4LEUNma8GHBV9FQXh6IoBknX7qwXUUJd4KnPiri/028V5qR+gW3O0XysuSh0PMr8fWiKUlOLM6EfMiRi4WVm1yNWZajybmVqyOa9sDn5ukg1SquHG63t8Q7fYPScwbI7igfowxYO7n3U2vg6wUJZxe6G4088VrpqLzzKryWo8mwR5VpQ3gkvw== 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=QzfMQgEgOJtDRDyDGESPQJZqEM2hbV77Wvqxk5DXsgA=; b=lbyQkHEOOmWY0YMb1LXA4NIl95P1HfipUkP0UsvMkQOuytEDeTcxh4lO6usQITkOBcEOKKOqbLxjWmPi5i3n+2zcLJMWWm420ENXWTaIk00DQOViVlXHrTXSlN2no/e+AGV5wcoLhVEAQJnzQyTMo3s0u5mdr+LEdtspN9GHMONHAqKSwjqjUM1LUI0oaNmZ30J4WO2xcbkEpL35hlACIgXuLsqfSDF5Mr80NHP7jeiNDMQ6TNwmoos34IMEeAxJZ2cIK3WSOE25jn9bRF4VDPNuGxc4pTxiZRVKtcInEEyc+Y3wikAJIiHmk5jgKZtfgNc4YXYSYmJ+CsgJVYdxVA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from MW4PR11MB5872.namprd11.prod.outlook.com (2603:10b6:303:169::14) by PH7PR11MB7552.namprd11.prod.outlook.com (2603:10b6:510:26a::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5944.19; Fri, 6 Jan 2023 09:41:28 +0000 Received: from MW4PR11MB5872.namprd11.prod.outlook.com ([fe80::5f56:1bdc:2eae:c041]) by MW4PR11MB5872.namprd11.prod.outlook.com ([fe80::5f56:1bdc:2eae:c041%9]) with mapi id 15.20.5944.019; Fri, 6 Jan 2023 09:41:28 +0000 From: "Yao, Jiewen" To: "devel@edk2.groups.io" , "jbobek@nvidia.com" CC: Jeff Brasen , Girish Mahadevan , "Wang, Jian J" , "Xu, Min M" Subject: Re: [edk2-devel] [PATCH 1/1] SecurityPkg/AuthVariableLib: Check SHA-256 OID with ContentInfo present Thread-Topic: [edk2-devel] [PATCH 1/1] SecurityPkg/AuthVariableLib: Check SHA-256 OID with ContentInfo present Thread-Index: AQHZD01vUaLeD/My8km5SQQat2zlp66NZyqAgAPbmBA= Date: Fri, 6 Jan 2023 09:41:28 +0000 Message-ID: References: <872cc00fa231a6a5a1edbe6d56082e44c38a0c0f.1670026872.git.jbobek@nvidia.com> <87y1raoofs.fsf@nvidia.com> <87pmbvz219.fsf@nvidia.com> In-Reply-To: <87pmbvz219.fsf@nvidia.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MW4PR11MB5872:EE_|PH7PR11MB7552:EE_ x-ms-office365-filtering-correlation-id: 33e10a2e-f7ec-4ae4-84d5-08daefca2fa1 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: SAc6oMZhDKu8Z0iqYJs3htW0dCj6Q5SeXVcOXv9xqSrx/c0UFb4P6/msrneOuXCJOn3JZr2YJz4l8iCel3s8y8Pp+pkEGpN8eVKVw2hoDBP2HUv3C8yRpHmPthQVuCUIKa1DzOWLVQ0l9mApwa9u4PUrHpzuSl4DfNCgCbYsJvrfXRVjc0DI5xtFf9RcKz7gw+lfQKvivEbSU8dzssMMKKoe/MwQ5N5o//Z6VJfYmOEs2VO4IwQKYhWW5GikB5tazYCEq6Us3t3DkPv9AYTYaMdUEa1JpTneWwBLRML9GnUkfYxUdiVrqAef7MoBwkNMtavAIlx5qaYn+D6ob8TCYn80R5HSvbTseVVbtJMIJw4/tGMd74rC1bnHchhDygeUAp06SVB2FEhdOkpzUrpADgrnPlgp03xIRW/F3CirqzivPapnS4sgJR6EHlt90YAXY0GRBdbs1g++/8OknY5crky/HNhmdPTFlDhrpHVvkEAkWUGH/zRT2nIe93ph1hxc4RHyYqmB1pkIGJhAW4REAvsLS6GTi6Y0sqdT7Tb+xJCwD2OsfIVLmA4/4BQTIgX4HkOMaUyrH/5c2l52Ms+z+XBHgtGUdLgUR1uCHMpbjr/al+qKf37zGmwl7TONQC3sDaxGesD6NmIDyzI8vIpeaQAYgI6kQpfQemmFV7rjbKwYwa9dyN9GhSVVVKYTJ38dDRdJw2iApFE2uV+1AsIERModFsTDkSVrN0JPl04VGG4HfUz3SWweOmbORAPyHAvCGNUptMfzykT6XcJul0pnfg== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MW4PR11MB5872.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(346002)(366004)(136003)(396003)(39860400002)(376002)(451199015)(7696005)(9686003)(186003)(26005)(71200400001)(83380400001)(53546011)(478600001)(966005)(66899015)(107886003)(4326008)(8676002)(41300700001)(52536014)(54906003)(110136005)(6506007)(316002)(33656002)(66946007)(8936002)(5660300002)(122000001)(64756008)(2906002)(38070700005)(55016003)(66556008)(86362001)(76116006)(66476007)(15650500001)(66446008)(38100700002)(82960400001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?XGAtWYvBJGIfB2B6XcPoe+t0aTd7yT2qKYqSFDH7P6NusBRVsj82oZhxTFON?= =?us-ascii?Q?+KZUoaIKEk05n6l2gNaghDCnWzhpO33pw05EkWYof46pr5IEfHqzhAWgHwwD?= =?us-ascii?Q?lcT3R6eTLlcVJTb4tsJxQGXB0jdzlzNjW3H2juMWSMQkKSJwGdBELO1Tv97q?= =?us-ascii?Q?zAZGJx5bSo1LkJ8Tzz7KxEztEqcF7uJHCbM8sKPl5rfeIpJMzfCm++RKW7y3?= =?us-ascii?Q?WO4WsfFnR2CUMsoP5EBIZvyNNViqCNOPmSwCJqKoPaBn7HLY6g9QZ0mD+y+j?= =?us-ascii?Q?dgnCDbPFGaPnEEqby8f/CCuROO45flrWFYwOJ1+dRpWXFmWKMxHd5LqSXngm?= =?us-ascii?Q?z5qzsoTBPJo4wlidoUJ/mpquk47m1jdm/WMeTasgLPFEvsuIlf/AJgT5Vfwk?= =?us-ascii?Q?TFPWOkGpA8w7G9+hYr6d4h2cswWYUU4m8lUcYF95v0bd2VpJIanbYQuBo+If?= =?us-ascii?Q?pUNICoEcEG8H5TSMyLIpgtGIWB4rJl6k5HIC3GpE54vKc4cQ6uB/m1g7oM8k?= =?us-ascii?Q?EzJYRiNhLccfDWvo9pDNYXTKiVo9rdN4wYOL/CASutDLvfzPbgRdzc01jvD5?= =?us-ascii?Q?Pw34O/rsZ/yJyih58Ib/x7rHcpHWoqF7ewCSl+HPlGGO5H3Zpko/+m0QTsh2?= =?us-ascii?Q?55DMlCeO3uOvLIcYofX9723EHF+KF2xV9wM71rhT/pZcIhuGlnh4GHU11AiO?= =?us-ascii?Q?QxH+Hit5cHLiQLBfhd8YPVhR0nmHbL4xSS7xJgsp5urFvBSqqSnvEdBnK/dL?= =?us-ascii?Q?M8AUmitHojtlxCMGIVFSTcdYSBGyI6UvQ11MSuE5M0t4ti7ZY0LWrvO05TWf?= =?us-ascii?Q?hgr+8GhL4thV6ffjKLS4f0SbimLyd3NyptMW20e5TTStdDCJcMk2xURpbChy?= =?us-ascii?Q?mU62kKBZ9Oq/Y1S9z7D/tWn0PDU968RWbCmydVCBfDaKgDeoQ7M7xWV2eU93?= =?us-ascii?Q?2FEJLrKwyDflJW+AmqG2xG7st7ewl00u7zktMXaBLoil0nVpJU3TBYL61Old?= =?us-ascii?Q?9DsXes0+5aPb0N26RPVTpY4oYTVlmHL0QZnom1NyrD86QH3uVq82janoVhQ0?= =?us-ascii?Q?udWaQW43Cks5Q+ZaQDnwR1JVxec4c6/oY07z6+JspyR6c2qD4hmnAhLXVWhW?= =?us-ascii?Q?MfSEEKjsvY7DDqRKfiwA6NsiLO638dlpAGWyuAcH+yfrp4jN7b+3n5ByYIW6?= =?us-ascii?Q?jgsOgghJr94rx5lbRRqQMVoTq9ER4nYCQEZgXgWc3WraXcjp1lhm1clnTGBW?= =?us-ascii?Q?EA4DKDNCproROkBQIhHAEgSTHbq7q4XSrB+wHRIUz67wIkUAkhfjmdA/fyW4?= =?us-ascii?Q?I3CQNr2TZ9d3HUxSYLzRzNiRTc0SQHna2XrxJ7mj6LTYzsnD1K43NhviTqou?= =?us-ascii?Q?62zHY99Sv4MYWyj6ToOCVhn7aMiVY9Xg7AIr7q2FzZSXwlrLxrFq/mtSbO5+?= =?us-ascii?Q?qaxtMJQPNFZI9A40+xEH4+iNS4cnXgWySg0RiG/vjY2zig9tp66TRJfnJYAW?= =?us-ascii?Q?rex21SpmNJOodqtPG5+TbTILcLxc1J8b+iAGMht1Ymmg9q4u647nTvRSDEnm?= =?us-ascii?Q?6XRYwiQxyjTUXj3KaGWztTPk/F/28t/4x/QcrYEO?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5872.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 33e10a2e-f7ec-4ae4-84d5-08daefca2fa1 X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jan 2023 09:41:28.7267 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: BVHWoT/deDWi1pLxoEiiWWijXn6prDkaXGAyydiqkA2+g0naWZRGVxSaqXSfL8x2cjV/eDqhmwNNw58nHbQljA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7552 Return-Path: jiewen.yao@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi That is good catch! My apology to miss it before. 1) Please file a bugzilla (https://bugzilla.tianocore.org/) to record the i= ssue and associate to the patch. 2) Would you please share with us that how you discover the issue? For example, any real use case to include ContentInfo? If yes, please share= a URL. Or this is just a purely spec compliance fix ? 3) Please describe how you validate the fix. If possible, would you please share your test case? 4) Since the new code is handling ContentInfo structure is present, I belie= ve we need also check if the ContentInfo structure is valid. For example: =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D c SignedData.contentInfo.contentType shall be set to id-data d SignedData.contentInfo.content shall be absent =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D What do you think? Thank you Yao, Jiewen > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Jan > Bobek via groups.io > Sent: Wednesday, January 4, 2023 6:30 AM > To: devel@edk2.groups.io > Cc: Jeff Brasen ; Girish Mahadevan > ; Jan Bobek ; Yao, Jiewen > ; Wang, Jian J ; Xu, Min M > > Subject: Re: [edk2-devel] [PATCH 1/1] SecurityPkg/AuthVariableLib: Check > SHA-256 OID with ContentInfo present >=20 > Anothing ping. Comments/reviews/merge highly appreciated. >=20 > Thank you, > -Jan >=20 > Jan Bobek writes: >=20 > > Ping. Can I get a review and/or some comments on this patch, please? > > > > Thanks, > > -Jan > > > > Jan Bobek writes: > > > >> Based on whether the DER-encoded ContentInfo structure is present in > >> authenticated SetVariable payload or not, the SHA-256 OID can be > >> located at different places. > >> > >> UEFI specification explicitly states the driver shall support both > >> cases, but the old code assumed ContentInfo was not present and > >> incorrectly rejected authenticated variable updates when it were > >> present. > >> > >> Cc: Jiewen Yao > >> Cc: Jian J Wang > >> Cc: Min Xu > >> Signed-off-by: Jan Bobek > >> --- > >> .../Library/AuthVariableLib/AuthService.c | 18 +++++++++++------= - > >> 1 file changed, 11 insertions(+), 7 deletions(-) > >> > >> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c > b/SecurityPkg/Library/AuthVariableLib/AuthService.c > >> index 054ee4d1d988..de8baccab410 100644 > >> --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c > >> +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c > >> @@ -1933,15 +1933,19 @@ VerifyTimeBasedPayload ( > >> // .... } > >> // The DigestAlgorithmIdentifiers can be used to determine the h= ash > algorithm > >> // in VARIABLE_AUTHENTICATION_2 descriptor. > >> - // This field has the fixed offset (+13) and be calculated based= on two > bytes of length encoding. > >> + // This field has the fixed offset (+13) or (+32) based on wheth= er the > DER-encoded > >> + // ContentInfo structure is present or not, and can be calculate= d > based on two > >> + // bytes of length encoding. > >> // > >> if ((Attributes & > EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) !=3D 0) { > >> - if (SigDataSize >=3D (13 + sizeof (mSha256OidValue))) { > >> - if (((*(SigData + 1) & TWO_BYTE_ENCODE) !=3D TWO_BYTE_ENCODE) |= | > >> - (CompareMem (SigData + 13, &mSha256OidValue, sizeof > (mSha256OidValue)) !=3D 0)) > >> - { > >> - return EFI_SECURITY_VIOLATION; > >> - } > >> + if ( ( (SigDataSize >=3D (13 + sizeof (mSha256OidValue))) > >> + && ( ((*(SigData + 1) & TWO_BYTE_ENCODE) !=3D > TWO_BYTE_ENCODE) > >> + || (CompareMem (SigData + 13, &mSha256OidValue, sizeof > (mSha256OidValue)) !=3D 0))) > >> + && ( (SigDataSize >=3D (32 + sizeof (mSha256OidValue))) > >> + && ( ((*(SigData + 20) & TWO_BYTE_ENCODE) !=3D > TWO_BYTE_ENCODE) > >> + || (CompareMem (SigData + 32, &mSha256OidValue, sizeof > (mSha256OidValue)) !=3D 0)))) > >> + { > >> + return EFI_SECURITY_VIOLATION; > >> } > >> } >=20 >=20 >=20 >=20