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 EF52678003C for ; Mon, 1 Jan 2024 23:07:26 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=sPcVpBe2E3ClAqngan72K8+OPCw3nRncdGQRHUFhN9s=; 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=1704150445; v=1; b=u+P3E0+BkODrcY+47lWpjhICVabR/T99EgoQhl8TCVXvqriRZ0nsOSZ2gsHMYJONJRfdpGj5 JtU/WvWnb2f3TwJE8lZfJKStyDINjPTbk0RVXPVgpbZVKzSRoRS9VmM3Oozw9Y+GPcKZbhHqEBI 1kvYJKZh7y3WJKPkC2+8/aVM= X-Received: by 127.0.0.2 with SMTP id pNupYY7687511xmRVNKp9vv4; Mon, 01 Jan 2024 15:07:25 -0800 X-Received: from a7-19.smtp-out.eu-west-1.amazonses.com (a7-19.smtp-out.eu-west-1.amazonses.com [54.240.7.19]) by mx.groups.io with SMTP id smtpd.web11.17551.1704150443855487959 for ; Mon, 01 Jan 2024 15:07:24 -0800 Message-ID: <0102018cc7481f4b-30b20784-217d-4677-8854-055c9e509c70-000000@eu-west-1.amazonses.com> Date: Mon, 1 Jan 2024 23:07:21 +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> <0102018cb0c83b57-ba6b133e-5f5c-4d05-85dc-bd6e32c87e41-000000@eu-west-1.amazonses.com> <0102018cb10db8bd-9edca239-8a41-4946-ad58-63ddb5a25921-000000@eu-west-1.amazonses.com> <0102018cb2e039d3-9ec4b97f-d3eb-4b4c-a8fd-248d4916f6f8-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: 2024.01.01-54.240.7.19 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: eE3nedz6Qh36XUMUe6mhI8wTx7686176AA= 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=u+P3E0+B; 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 29/12/2023 15:07, Chang, Abner via groups.io wrote: > To locate TLS protocol from the HTTP handle and configure TLS configuration data at the return from EfiHttpRequest during that short window of non-blocking request is not reliable. It also doesn't make sense to ask upper layer application to do this when it first time invokes EfiHttpRequest. > I already refactored TlsCreateChild to install TLS protocol on HTTP handle. I also implemented the corresponding code in Redfish REST EX to listen the installation of TLS protocol and hook the SetSessionData. It works fine on the system, however I really don’t like having the upper layer application to do this much just for overriding TLS configuration data. The code looked a specific implementation to hack the TLS protocol interface. Plus I still have to add few code in TlsConfigCertificate to skip configure certificate with checking TlsVerifyMethod. > We should sit back to consider introducing a new protocol for upper layer application to provide their own TLS configuration data, as the root cause is that hard coded TLS configuration data in HttpSupport.c. We shouldn't have the code like that and add the burdens to application. > > What my thought is as below and maybe more elegant than the patch a sent, > - Still install TLS on HTTP handle, then upper layer application can listen to the installation of EFI TLS protocol to find the correct HTTP handle. > - Move TLS_CONFIG_DATA in a public header file. > - Introduce a new protocol called EDKII_HTTP_TLS_CONFIGURATION_DATA > - Upper layer application installs this protocol with their own TLS_CONFIG_DATA. > - TlsConfigureSession locates EDKII_HTTP_TLS_CONFIGURATION_DATA to replace the default TLS_CONFIG_DATA. > > This way we can remove that hardcoded code and fix the root cause, also the upper layer application do not have to take the burden. > What do you think? Firstly, thank you very much for taking the time to dig through this and work towards a cleaner design - I, for one, really appreciate it. I think we're completely agreed that installing the TLS protocols on the HTTP handle is the right thing to do - that seems to be a clear improvement over the status quo where there's no introspectable relationship between the two handles. I'm torn on the use of TLS_CONFIG_DATA. For better or worse, the existing and standardised EFI_TLS_CONFIGURATION_PROTOCOL is verb-based, using SetData() and GetData() methods. Adding a noun-based protocol for TLS configuration seems to cut across this, with the potential to look confusing: a new reader of the code could legitimately wonder why the codebase contains two competing solutions to what is essentially the same problem. Given that the verb-based approach of EFI_TLS_CONFIGURATION_PROTOCOL has made it as far as being standardised and included in the UEFI specification, I think we probably need to accept that this is the "correct" way to perform TLS configuration within UEFI code. The problem with HttpsSupport.c then becomes that there is no good opportunity for a consumer to call SetData(), since (a) EFI_TLS_CONFIGURATION_PROTOCOL comes into existence only halfway through the call to EFI_HTTP_PROTOCOL.Request() and (b) the call to TlsConfigureSession() will overwrite the configuration anyway. Is there a way that TlsConfigureSession() could sensibly provide an opportunity for the consumer to make calls to SetData(), so that the consumer could cleanly override any default configuration? Looking through the code, TlsConfigureSession() is called only from HttpInitSession(), which in turn is called only from EfiHttpRequest(). This call is followed immediately by the line: HttpNotify (HttpEventInitSession, Status); which seems to already use an existing EDKII_HTTP_CALLBACK_PROTOCOL to notify an arbitrary list of interested consumers that an event has taken place (in this case, that a session has just been initialised). What do you think about: - installing TLS on HTTP handle (as you have already implemented) - using EDKII_HTTP_CALLBACK_PROTOCOL to catch the HttpEventInitSession and perform whatever calls are needed to SetData() to modify the TLS configuration? Thanks, Michael -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113015): https://edk2.groups.io/g/devel/message/113015 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] -=-=-=-=-=-=-=-=-=-=-=-