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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
