From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 26C87941C5A for ; Thu, 28 Dec 2023 14:16:06 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=eFXZFJ2TtcMLW1Wx3lrnDMHTS9R9/f+bUYX62S/4qUs=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:Autocrypt:In-Reply-To:Feedback-ID:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1703772964; v=1; b=QA/zQS6I8aY9uTyQkCBE47bS4seDVxoobNa11w+zdnGSpUwtVPgRSwhoCWmGagnXLe6pHbSj dWduVKOwV/QxepcpUKCpf5pPbIblS5CQ2uA8wG+czob6yN5sBIXMnV7qXItUsFYg8I51qKvNrAf n4GgAp5cptJcfSo6GoqPs0f0= X-Received: by 127.0.0.2 with SMTP id 69J5YY7687511xbdAAhZGpXs; Thu, 28 Dec 2023 06:16:04 -0800 X-Received: from a7-10.smtp-out.eu-west-1.amazonses.com (a7-10.smtp-out.eu-west-1.amazonses.com [54.240.7.10]) by mx.groups.io with SMTP id smtpd.web10.123571.1703772963748617840 for ; Thu, 28 Dec 2023 06:16:04 -0800 Message-ID: <0102018cb0c83b57-ba6b133e-5f5c-4d05-85dc-bd6e32c87e41-000000@eu-west-1.amazonses.com> Date: Thu, 28 Dec 2023 14:16:01 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [RFC][PATCH 0/2] Introduce HTTPS Platform TLS policy To: devel@edk2.groups.io, abner.chang@amd.com Cc: Saloni Kasbekar , Zachary Clark-williams , Nickle Wang , Igor Kulchytskyy References: <20231226112839.1152-1-abner.chang@amd.com> <0102018cabfc96cb-073692ee-eb88-4e49-ba2b-0e21850632d8-000000@eu-west-1.amazonses.com> From: "Michael Brown" Autocrypt: addr=mcb30@ipxe.org; keydata= xsFNBGPmfF4BEAC3vcU4aLC/9Uy/rTpmYujbqxQNZje9E34jGvLxO3uYwj4BeHj1Nn5T2TDM Gkc4ngk+mGPsJsIn69YU5cfVN+ch9O7FVfsn6egZsCNeLy6Qz0o//gBaWJodFBeawuBjXXyV HnQZa1p7bA/Lws8minW7NrZ7XZgEBaiVm1v1dNbLEoWR8UL2AMtph5loCQ5jPYQNqp/wH9El /R30GjXvAd1riWyJR2TWSN23J9rnuH2Ue+N4yEnWxAsBQ6M/NFQ5z42w4mYdsnzy1w3PulrL icpSixXHkm3lQcKGtKKX41HvJukSpxCgbHfuHGEJZ7bdhgRic1DHKav0JR8kQhx3gnPh06z8 1Teu2NKkSsTR3Iv6E2x6Yy6H34lKWzBzd8TLNSevesDD/L6NU/HxT9AxrTBuypk9PZGe2VH1 W03XnR/0Mnr0QqQBXcIAERdgNzRJY4VKF75vedf8IooZFUQ4RUlqH+x3aZB9nJ9ET77mPaNi SQVQBxE68uzb7eh2Kf6z7ftOYpWPw1v5HyB3oMmafEDG36SIvNF2wnmNaLQDRnAbTcy4ERgy tpJ3wtQDJeXOePLv8hJ3q7DSuePl7cwz4xy0ZHglW/EXRXLnyRRACfDGowyENoStg06qF+qm edGu1wNtmDZ/lypWm/CkzzpUDFeGP5BLZlqwVX4hn88llfvVzwARAQABzR5NaWNoYWVsIEJy b3duIDxtY2IzMEBpcHhlLm9yZz7CwZEEEwEIADsWIQTgD69MBpjBm2slMvwCNbEKAOtEUAUC Y+Z8nwIbAwULCQgHAgIiAgYVCgkICwIEFgIDAQIeBwIXgAAKCRACNbEKAOtEUFlhD/9ElIUg JxBXpIbF8s7u79OdXLld2Z1DfVmhP5Q+GilPvEeAWHhp689S9B88aNvpwW5zJfxlxcJZO0ay jc7E/vtdNrkXGWNEEXBgdve6m+uL+pW/i5E2htqxbLyfgTJKmsvJ8graHbwrrBS/PA8KuwVJ eAGbBNi3f1gyQQWrLqfTkUpLtuj7A76iVVk0G0a78L69Al84qhK2imqpFJoZt1F8h0Z5ddGv mvf2M/DZp87UXvXjy7X6r7msbMZa6S/Jv0dtWHeZGl3Xu3qzbtjlqFyz2Q7TibHiirsgg/CV BsbH/LLbi/aNCCQ/85C6jAMB0lNzcVZ7ZiKKo+vBNMTycDFk70LA9yjlNf7exHejoXmPkLmH ddapYZ4dzwdOiJlaTu8NZgzXUCt3RDDA1qmZrAOBF/F+tPILAEhenl9kj3blD3mPV2SrWLWY dbahY9BsylUhj/qE1ik5CJXrPotmJhok9Vpg07xKDpVnZXuWLGNIE8018UumO7phLrWQwLb1 wJdN7PG165w4UWf4aQphfwaMKOVU3WDghz3aVSP9rgtm3RsUcYHPKx8IaPcDh2yf0bgG386i Axx3U3UQeyz2Pb9Vigo6DmPwXjLkFr/dukvVLVJLVkUab9ZhhERzWTEEMifUVEK2rGNvA87L VKJ2zOyxWx1e0CPj6fcGbkJ0D10XLs7BTQRj5nxeARAAz18zv2ksRiM6eEKG0qzpiKHVYlVy wtjla+m9wuAIwm314tffY5hjQN46uwTstdhQirjywF1EmcS6KNGiIjmoLim+dqyFP5d/UF5A VjLt0TYq7HjadIxbm2/CvcRnNJ01FkD99xLxV0hFTUAWAUX1mNqQ3MmWIjV89wiT06uuAUog m+jG3RRDyWbUnVELR60mhzccKsaEsjO/HqIERvBwL7tlOJewlPrVyz9Zed9Nhhv0KDAYmdEm kIEEbOfsjRu5I6nIY3NrX+QP9+nmgxADlsjvLXTSU0fT/g7IPEl3gpsQZAbgmrlGcPtvXod8 P4iOmL8GJDU1RdBE9TBOLEbu9UlDRD4zr6tdzRpB9wvXdtSUcNCdHVqJTfq2qjIlBk7x+zQD ayhxzDvTMxD/93K6txKXmVVtfMBsmt9KuD2JBUEAExjsLHqzg48nQg8wF9JYWCWGBb36qpd0 yC6VPzhSLe2Ov3/GyV5ZshO046+OiGxEeaHCwMnDTZF9xrQ5paCwWedlWKvGM2zB64AHuk+M v2ABK/gbDO7eS6p+xz11oD1NHr1HQLRtknfClIqj9AmjgX9maD+4GUrmHaxmkNilIukahotd Un9Up2gX05Wy/S3H/v8RB0kxwWg2Wh065dnyCF4Doe18bcYZvM+iMJmUBag6aDfQlryM04K7 z4ITYDkAEQEAAcLBdgQYAQgAIBYhBOAPr0wGmMGbayUy/AI1sQoA60RQBQJj5nxeAhsMAAoJ EAI1sQoA60RQZj4QAIkiRDVNWynZ4kEdpqmf6hpD++Zycz+LMne4iGRsiyyTf/rPNgskNLrU JD555yDvFiEAhOI27R8YNCJj5byXRDa/Bm6ueClFia+POibt28UEdyOFU9PVcgFaU+VxaBIP rHacHL6A7UKFjmBN7o8VkVF2xXlmFge795mP4/Y3t6qfWUTodrpw1w1t5/bZxZdWqX4pUCpY fEx87jm60+Mj0Tb4VPWXz0UD1q1BDcdYxNa2ISLaJhGJmjjks9eqdFOhPo1fTINMNWF2Alxi jA6WNT8nn9lm1kav75EMYMc8WIR9tb03i+IuKNp2IWwTGBqIUyQj00BhHkZQFl4HxZhV0gXE AWu34Q/Z7hOUXGXq2tvYCxDeaQb2wks93e62lrrUm1JGhPWkVoCI8Md8N2mkonqIfMK8lQ0W WbkYHdKBkgDqhDypNNhkjWNX3JL1kL0c3rqGL381iBAZaGQPygyCx2xH9PDNp59W6u8sXb13 +UX+kXdWU+KYbMTVoO/t4MxUJg6nXPJHz9NCkyluI820l+2OtXZZy0u196evIlUdD6RoTrNK z5OgFxNctVi9BPsQea9du+JlYJ460vZNPz180oczj7iqffd+p9DmAkeK25njWhg3qPeXiNZN 45J9eMChSOaJ0GMGUQndIIxz7PO8IzjbkSHLG5CKrR3MaphMB/0L In-Reply-To: X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00, URIBL_DBL_BLOCKED_OPENDNS,URIBL_ZEN_BLOCKED_OPENDNS autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on blyat.fensystems.co.uk Feedback-ID: 1.eu-west-1.fspj4M/5bzJ9NLRzJP0PaxRwxrpZqiDQJ1IF94CF2TA=:AmazonSES X-SES-Outgoing: 2023.12.28-54.240.7.10 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mcb30@ipxe.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: gOtyxEDT1n3Z6SDv05Gv3HXbx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b="QA/zQS6I"; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 28/12/2023 02:47, Chang, Abner via groups.io wrote: >> On 26/12/2023 11:28, Chang, Abner via groups.io wrote: >>> Platform developer can provide this protoocl to EFI HTTP driver to >>> configure TLS using TLS conifg data provided by >>> EDKII_HTTPS_TLS_PLATFORM_POLICY_PROTOCOL for the specific HTTP >>> protocol handle. How to distinguish the correct HTTP protocol >>> handle for the platform TLS policy is outside the scope of this >>> change. For Redfish, we will provide this protocol in EFI Redfish >>> REST EX driver. >> >> This looks messy to me. >> >> Did you try my suggestion of using RegisterProtocolNotify() in order to >> register a callback that will be called for any new instances of >> EFI_TLS_PROTOCOL? >> >> This would be functionally equivalent to your patch, but with zero lines >> of additional code required in HttpDxe. > > I think you suggest to hook/replace the EFI_TLS_PROTOCOL for the specific HTTP handle? > EFI_TLS_PROTOCOL is installed implicitly when the first time HTTPs request is performed. There is no connection between HTTP handle and EFI TLS protocol instance besides the HTTP driver internal structure. > Listen to the installation of EFI_TLS_PROTOCOL has no way to distinguish the dedicated HTTP handle, for example the HTTP handle created by Redfish REST EX driver. > I don’t see the chance to provide the flexibility to TLS config with using RegisterProtocolNotify for EFI_TLS_PROTOCO unless we add one line to install the same TLS_PTOTOCOL on the given HTTP instance. Or something I missed? Having looked through the code in more detail, you are correct that there is no connection between the TLS handle and the HTTP handle, since TlsCreateChild() in HttpsSupport.c currently chooses to call (*TlsSb)->CreateChild() with a NULL handle and does not ever call OpenProtocol() with BY_CHILD to set up a parent/child relationship. I would therefore suggest a mild refactoring of TlsCreateChild() so that it installs the TLS protocols directly onto the HTTP handle. This refactoring does not break any APIs, since TlsCreateChild() is purely internal to HttpDxe. Since all other functions in HttpsSupport.c seem to take HttpInstance as the first parameter, I would probably change TlsCreateChild() to do the same: EFI_STATUS EFIAPI TlsCreateChild ( IN OUT HTTP_PROTOCOL *HttpInstance ) There is only one call site of TlsCreateChild() (in EfiHttpRequest()) and so the most obvious approach would be to move the logic around the call site to be inside of TlsCreateChild(): if (HttpInstance->LocalAddressIsIPv6) { ImageHandle = HttpInstance->Service->Ip6DriverBindingHandle; } else { ImageHandle = HttpInstance->Service->Ip4DriverBindingHandle; } and then use HttpInstance->TlsSb in place of the current *TlsSb, etc, within TlsCreateChild(). The call within TlsCreateChild() to HttpInstance->TlsSb->CreateChild() can then pass &HttpInstance->Handle as the second parameter, so that the EFI_TLS_PROTOCOL is installed onto the HTTP handle. This refactoring would have the side effect of cleaning up TlsCreateChild() to be consistent with other functions in the same file, and would allow it to return an EFI_STATUS for more meaningful error reporting. With the TLS protocol installed onto the same handle, I don't think you then even need to use RegisterProtocolNotify(). On return from EFI_HTTP_PROTOCOL.Request() you can open the TLS protocol on the handle and immediately call SetSessionData() to override VerifyMethod etc. What do you think? Thanks, Michael -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112980): https://edk2.groups.io/g/devel/message/112980 Mute This Topic: https://groups.io/mt/103368438/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-