From: "Michael Brown" <mcb30@ipxe.org>
To: devel@edk2.groups.io, abner.chang@amd.com
Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>,
Zachary Clark-williams <zachary.clark-williams@intel.com>,
Nickle Wang <nicklew@nvidia.com>,
Igor Kulchytskyy <igork@ami.com>
Subject: Re: [edk2-devel] [RFC][PATCH 0/2] Introduce HTTPS Platform TLS policy
Date: Tue, 2 Jan 2024 12:42:16 +0000 [thread overview]
Message-ID: <0102018cca323415-72e68e82-2db0-4821-897e-8eff33fbe586-000000@eu-west-1.amazonses.com> (raw)
In-Reply-To: <MN2PR12MB39664F99340EF3AA29886FD7EA61A@MN2PR12MB3966.namprd12.prod.outlook.com>
On 02/01/2024 06:06, Chang, Abner via groups.io wrote:
>> 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?
>
> Leverage HttpNotify is good but still has some problems, as HttpNotify is designed to notify callback owner about a specific task was done. In order to keep this HttpNotify nature, we can create a callback point at the end of TlsCreateChild() with a newly introduced event type says HttpEventTlsChildCreated. The reason we have to create this notification before TlsConfigureSession() is because this function uses the default configuration data to configure TLS. However, it doesn't have to do EfiTlsVerifyHost and TlsConfigCertificate if there is nothing to verify.
> The problem in configuring EfiTlsVerifyHost is It always checks verification method with EFI_TLS_VERIFY_PEER, while the problem of TlsConfigCertificate is it considers platform always can provide the certificate. Anyway to configure TLS after TlsConfigCertificate is to late as the error status already returned earlier. Furthermore the design of HttpNotify doesn't provide the output information for caller to determine the different code paths. So with above, how can we skip configuring TLS again with the default values in HttpSupport.c even platform code already configured it before TlsConfigureSession()?
I may not have been clear enough: I'm suggesting that we allow
TlsConfigureSession() to perform its normal configuration, and then use
the HttpEventInitSession callback to modify this configuration (e.g.
setting EFI_TLS_VERIFY_NONE).
Yes, this would mean that a tiny amount of unnecessary work is done
(e.g. first setting EFI_TLS_VERIFY_PEER, then later changing it to
EFI_TLS_VERIFY_NONE) but this is a negligible amount of execution time
and allows us to keep the code simple.
The HttpEventInitSession callback is guaranteed to be called before the
calls to HttpGenRequestMessage() and HttpTransmitTcp() (even if running
at TPL_APPLICATION with network polling taking place) and so is
guaranteed to be a safe point at which to perform additional TLS
configuration via SetData().
So, to expand on what I wrote before, I'm suggesting:
- refactor TlsCreateChild() to install the TLS protocols on the HTTP
handle (as you have already implemented).
- (optional) Remove TlsChildHandle and replace with a boolean flag.
- No further changes required to HttpDxe, as far as I can tell.
- In RedfishRestExDxe, install an EDKII_HTTP_CALLBACK_PROTOCOL before
calling EFI_HTTP_PROTOCOL.Request().
- Allow the call to Request() to perform its normal TLS configuration
via TlsConfigureSession(), as though the connection were going to
perform host verification etc as per the platform default policy. This
configuration should succeed, with no error returned.
- In the RedfishRestExDxe callback, check for HttpEventInitSession and
use calls to EFI_TLS_CONFIGURATION_PROTOCOL.SetData() to modify the TLS
configuration to e.g. set EFI_TLS_VERIFY_NONE.
To make the callback implementation easier, you may want to extend
HttpNotify() to take HTTP_PROTOCOL *HttpInstance as its first parameter,
and then pass HttpInstance->Handle as an additional parameter to the
callback method, i.e.:
typedef
VOID
(EFIAPI *EDKII_HTTP_CALLBACK)(
IN EDKII_HTTP_CALLBACK_PROTOCOL *This,
IN EFI_HANDLE HttpHandle,
IN EDKII_HTTP_CALLBACK_EVENT Event,
IN EFI_STATUS EventStatus
);
VOID
HttpNotify (
IN HTTP_PROTOCOL *HttpInstance,
IN EDKII_HTTP_CALLBACK_EVENT Event,
IN EFI_STATUS EventStatus
)
{
...
HttpCallback->Callback (
HttpCallback,
HttpInstance->Handle,
Event,
EventStatus
);
...
}
Since EDKII_HTTP_CALLBACK_PROTOCOL is internal to EDK2 (and not covered
by the UEFI specification), this change should be possible. I've
checked all of the HttpNotify() call sites in the EDK2 repo, and they do
all have an HttpInstance available to use.
Thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113033): https://edk2.groups.io/g/devel/message/113033
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]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2024-01-02 12:42 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-26 11:28 [edk2-devel] [RFC][PATCH 0/2] Introduce HTTPS Platform TLS policy Chang, Abner via groups.io
2023-12-26 11:28 ` [edk2-devel] [RFC][PATCH 1/2] NetworkPkg: EDKII HTTPS platform " Chang, Abner via groups.io
2023-12-26 11:28 ` [edk2-devel] [RFC][PATCH 2/2] NetworkPkg: Check " Chang, Abner via groups.io
2023-12-27 15:55 ` [edk2-devel] [RFC][PATCH 0/2] Introduce HTTPS Platform " Michael Brown
2023-12-28 2:47 ` Chang, Abner via groups.io
2023-12-28 14:16 ` Michael Brown
2023-12-28 15:04 ` Chang, Abner via groups.io
2023-12-28 15:31 ` Michael Brown
2023-12-28 23:37 ` Chang, Abner via groups.io
2023-12-29 0:01 ` Michael Brown
2023-12-29 15:07 ` Chang, Abner via groups.io
2023-12-30 11:31 ` Chang, Abner via groups.io
2024-01-01 23:07 ` Michael Brown
2024-01-02 6:06 ` Chang, Abner via groups.io
2024-01-02 12:42 ` Michael Brown [this message]
2024-01-02 16:31 ` Chang, Abner via groups.io
2024-01-02 17:46 ` Michael Brown
2024-01-04 3:13 ` Chang, Abner via groups.io
2024-01-05 8:41 ` Chang, Abner via groups.io
2024-01-05 17:16 ` Michael Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0102018cca323415-72e68e82-2db0-4821-897e-8eff33fbe586-000000@eu-west-1.amazonses.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox