From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mx.groups.io with SMTP id smtpd.web12.42281.1658245602015086268 for ; Tue, 19 Jul 2022 08:46:42 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=GGhxOXGA; spf=none, err=permanent DNS error (domain: linux.intel.com, ip: 134.134.136.100, mailfrom: maciej.rabeda@linux.intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1658245602; x=1689781602; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=eKnxK10mv++Buh6ALOFYiRP0hhY6GFm7DAualOmbfPI=; b=GGhxOXGAyf8sshLUzWDQmnSrJAnkPc5NIyE9z0cn8I5lXyOwc1MkrbaK xuIR2GUDJtY9Sk7tllIOdPvQvEQb2rTXvW/ulUXw6DXjehnGn2Qcoe9aJ 74eXOCXOD9+RlGsCVzXRUmCNV+vxN1Jden+kAwnA7Rrgm7QMcLtG8HGyn EL+P34vVXtDO0d0KXLjFXg9zjeDOL9ESsHq53dZ9GMOBRGrq1jaFewQGh PQJaaiq6862AaKvr5E45wYBKCTtz3LC6mccnMc0dCsj18AT/mIjcWkmev HJsBzzaYYMU7vd5NtmttibzJRD+SvDRCk1zaGy/xQR6RqV6wiTAq2xWhr w==; X-IronPort-AV: E=McAfee;i="6400,9594,10413"; a="350488817" X-IronPort-AV: E=Sophos;i="5.92,284,1650956400"; d="scan'208";a="350488817" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jul 2022 08:46:34 -0700 X-IronPort-AV: E=Sophos;i="5.92,284,1650956400"; d="scan'208";a="655827907" Received: from mkrajews-mobl1.ger.corp.intel.com (HELO [10.213.21.235]) ([10.213.21.235]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Jul 2022 08:46:32 -0700 Message-ID: Date: Tue, 19 Jul 2022 17:46:27 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v4 1/1] NetworkPkg/HttpBootDxe: Add Support for HTTP Boot Basic Authentication To: Saloni Kasbekar , devel@edk2.groups.io Cc: Wu Jiaxin , Siyuan Fu References: <1eb3e48dc7fe65db4ffc9424ace42c80c45f7e82.1658238840.git.saloni.kasbekar@intel.com> From: "Maciej Rabeda" In-Reply-To: <1eb3e48dc7fe65db4ffc9424ace42c80c45f7e82.1658238840.git.saloni.kasbekar@intel.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Thank you for the patch. Reviewed-by: Maciej Rabeda On 19 lip 2022 15:54, Saloni Kasbekar wrote: > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2504 > > Add support for TLS Client Authentication using Basic Authentication > for HTTP Boot > > Cc: Maciej Rabeda > Cc: Wu Jiaxin > Cc: Siyuan Fu > Signed-off-by: Saloni Kasbekar > --- > MdePkg/Include/IndustryStandard/Http11.h | 8 ++ > MdePkg/Include/Protocol/HttpBootCallback.h | 6 +- > NetworkPkg/HttpBootDxe/HttpBootClient.c | 99 +++++++++++++++++++++- > NetworkPkg/HttpBootDxe/HttpBootClient.h | 6 +- > NetworkPkg/HttpBootDxe/HttpBootDxe.h | 6 ++ > NetworkPkg/HttpBootDxe/HttpBootImpl.c | 23 ++++- > 6 files changed, 143 insertions(+), 5 deletions(-) > > diff --git a/MdePkg/Include/IndustryStandard/Http11.h b/MdePkg/Include/IndustryStandard/Http11.h > index f1f113e04b69..2137ef1f1ac3 100644 > --- a/MdePkg/Include/IndustryStandard/Http11.h > +++ b/MdePkg/Include/IndustryStandard/Http11.h > @@ -204,6 +204,14 @@ > /// > #define HTTP_HEADER_IF_NONE_MATCH "If-None-Match" > > +/// > +/// The WWW-Authenticate Response Header > +/// If a server receives a request for an access-protected object, and an > +/// acceptable Authorization header is not sent, the server responds with > +/// a "401 Unauthorized" status code, and a WWW-Authenticate header. > +/// > +#define HTTP_HEADER_WWW_AUTHENTICATE "WWW-Authenticate" > + > /// > /// Authorization Request Header > /// The Authorization field value consists of credentials > diff --git a/MdePkg/Include/Protocol/HttpBootCallback.h b/MdePkg/Include/Protocol/HttpBootCallback.h > index 926f6c1b3076..b56c631b1f4f 100644 > --- a/MdePkg/Include/Protocol/HttpBootCallback.h > +++ b/MdePkg/Include/Protocol/HttpBootCallback.h > @@ -32,7 +32,7 @@ typedef enum { > /// > HttpBootDhcp6, > /// > - /// Data points to an EFI_HTTP_MESSAGE structure, whichcontians a HTTP request message > + /// Data points to an EFI_HTTP_MESSAGE structure, which contains a HTTP request message > /// to be transmitted. > /// > HttpBootHttpRequest, > @@ -46,6 +46,10 @@ typedef enum { > /// buffer of the entity body data. > /// > HttpBootHttpEntityBody, > + /// > + /// Data points to the authentication information to provide to the HTTP server. > + /// > + HttpBootHttpAuthInfo, > HttpBootTypeMax > } EFI_HTTP_BOOT_CALLBACK_DATA_TYPE; > > diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c b/NetworkPkg/HttpBootDxe/HttpBootClient.c > index 62e87238fef7..40f64fcb6bf8 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c > +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c > @@ -922,6 +922,7 @@ HttpBootGetBootFileCallback ( > @retval EFI_BUFFER_TOO_SMALL The BufferSize is too small to read the current directory entry. > BufferSize has been updated with the size needed to complete > the request. > + @retval EFI_ACCESS_DENIED The server needs to authenticate the client. > @retval Others Unexpected error happened. > > **/ > @@ -951,6 +952,9 @@ HttpBootGetBootFile ( > CHAR16 *Url; > BOOLEAN IdentityMode; > UINTN ReceivedSize; > + CHAR8 BaseAuthValue[80]; > + EFI_HTTP_HEADER *HttpHeader; > + CHAR8 *Data; > > ASSERT (Private != NULL); > ASSERT (Private->HttpCreated); > @@ -1009,8 +1013,9 @@ HttpBootGetBootFile ( > // Host > // Accept > // User-Agent > + // [Authorization] > // > - HttpIoHeader = HttpIoCreateHeader (3); > + HttpIoHeader = HttpIoCreateHeader ((Private->AuthData != NULL) ? 4 : 3); > if (HttpIoHeader == NULL) { > Status = EFI_OUT_OF_RESOURCES; > goto ERROR_2; > @@ -1063,6 +1068,35 @@ HttpBootGetBootFile ( > goto ERROR_3; > } > > + // > + // Add HTTP header field 4: Authorization > + // > + if (Private->AuthData != NULL) { > + ASSERT (HttpIoHeader->MaxHeaderCount == 4); > + > + if ((Private->AuthScheme != NULL) && (CompareMem (Private->AuthScheme, "Basic", 5) != 0)) { > + Status = EFI_UNSUPPORTED; > + goto ERROR_3; > + } > + > + AsciiSPrint ( > + BaseAuthValue, > + sizeof (BaseAuthValue), > + "%a %a", > + "Basic", > + Private->AuthData > + ); > + > + Status = HttpIoSetHeader ( > + HttpIoHeader, > + HTTP_HEADER_AUTHORIZATION, > + BaseAuthValue > + ); > + if (EFI_ERROR (Status)) { > + goto ERROR_3; > + } > + } > + > // > // 2.2 Build the rest of HTTP request info. > // > @@ -1111,6 +1145,7 @@ HttpBootGetBootFile ( > goto ERROR_4; > } > > + Data = NULL; > Status = HttpIoRecvResponse ( > &Private->HttpIo, > TRUE, > @@ -1121,6 +1156,68 @@ HttpBootGetBootFile ( > StatusCode = HttpIo->RspToken.Message->Data.Response->StatusCode; > HttpBootPrintErrorMessage (StatusCode); > Status = ResponseData->Status; > + if ((StatusCode == HTTP_STATUS_401_UNAUTHORIZED) || \ > + (StatusCode == HTTP_STATUS_407_PROXY_AUTHENTICATION_REQUIRED)) > + { > + if ((Private->AuthData != NULL) || (Private->AuthScheme != NULL)) { > + if (Private->AuthData != NULL) { > + FreePool (Private->AuthData); > + Private->AuthData = NULL; > + } > + > + if (Private->AuthScheme != NULL) { > + FreePool (Private->AuthScheme); > + Private->AuthScheme = NULL; > + } > + > + Status = EFI_ACCESS_DENIED; > + goto ERROR_4; > + } > + > + // > + // Server indicates the user has to provide a user-id and password as a means of identification. > + // > + if (Private->HttpBootCallback != NULL) { > + Data = AllocateZeroPool (sizeof (CHAR8) * HTTP_BOOT_AUTHENTICATION_INFO_MAX_LEN); > + if (Data == NULL) { > + Status = EFI_OUT_OF_RESOURCES; > + goto ERROR_4; > + } > + > + Status = Private->HttpBootCallback->Callback ( > + Private->HttpBootCallback, > + HttpBootHttpAuthInfo, > + TRUE, > + HTTP_BOOT_AUTHENTICATION_INFO_MAX_LEN, > + Data > + ); > + if (EFI_ERROR (Status)) { > + if (Data != NULL) { > + FreePool (Data); > + } > + > + goto ERROR_5; > + } > + > + Private->AuthData = (CHAR8 *)Data; > + } > + > + HttpHeader = HttpFindHeader ( > + ResponseData->HeaderCount, > + ResponseData->Headers, > + HTTP_HEADER_WWW_AUTHENTICATE > + ); > + if (HttpHeader != NULL) { > + Private->AuthScheme = AllocateZeroPool (AsciiStrLen (HttpHeader->FieldValue) + 1); > + if (Private->AuthScheme == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + CopyMem (Private->AuthScheme, HttpHeader->FieldValue, AsciiStrLen (HttpHeader->FieldValue)); > + } > + > + Status = EFI_ACCESS_DENIED; > + } > } > > goto ERROR_5; > diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.h b/NetworkPkg/HttpBootDxe/HttpBootClient.h > index 406529dfd927..2fba71367950 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootClient.h > +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.h > @@ -10,8 +10,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #ifndef __EFI_HTTP_BOOT_HTTP_H__ > #define __EFI_HTTP_BOOT_HTTP_H__ > > -#define HTTP_BOOT_BLOCK_SIZE 1500 > -#define HTTP_USER_AGENT_EFI_HTTP_BOOT "UefiHttpBoot/1.0" > +#define HTTP_BOOT_BLOCK_SIZE 1500 > +#define HTTP_USER_AGENT_EFI_HTTP_BOOT "UefiHttpBoot/1.0" > +#define HTTP_BOOT_AUTHENTICATION_INFO_MAX_LEN 255 > > // > // Record the data length and start address of a data block. > @@ -106,6 +107,7 @@ HttpBootCreateHttpIo ( > @retval EFI_BUFFER_TOO_SMALL The BufferSize is too small to read the current directory entry. > BufferSize has been updated with the size needed to complete > the request. > + @retval EFI_ACCESS_DENIED The server needs to authenticate the client. > @retval Others Unexpected error happened. > > **/ > diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.h b/NetworkPkg/HttpBootDxe/HttpBootDxe.h > index 5acbae9bfa76..5ff8ad4698b2 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootDxe.h > +++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.h > @@ -183,6 +183,12 @@ struct _HTTP_BOOT_PRIVATE_DATA { > UINT64 ReceivedSize; > UINT32 Percentage; > > + // > + // Data for the server to authenticate the client > + // > + CHAR8 *AuthData; > + CHAR8 *AuthScheme; > + > // > // HII callback info block > // > diff --git a/NetworkPkg/HttpBootDxe/HttpBootImpl.c b/NetworkPkg/HttpBootDxe/HttpBootImpl.c > index 3da585a29164..b4c61925b94f 100644 > --- a/NetworkPkg/HttpBootDxe/HttpBootImpl.c > +++ b/NetworkPkg/HttpBootDxe/HttpBootImpl.c > @@ -360,7 +360,18 @@ HttpBootLoadFile ( > NULL, > &Private->ImageType > ); > - if (EFI_ERROR (Status) && (Status != EFI_BUFFER_TOO_SMALL)) { > + if ((Private->AuthData != NULL) && (Status == EFI_ACCESS_DENIED)) { > + // > + // Try to use HTTP HEAD method again since the Authentication information is provided. > + // > + Status = HttpBootGetBootFile ( > + Private, > + TRUE, > + &Private->BootFileSize, > + NULL, > + &Private->ImageType > + ); > + } else if ((EFI_ERROR (Status)) && (Status != EFI_BUFFER_TOO_SMALL)) { > // > // Failed to get file size by HEAD method, may be trunked encoding, try HTTP GET method. > // > @@ -489,6 +500,16 @@ HttpBootStop ( > } > } > > + if (Private->AuthData != NULL) { > + FreePool (Private->AuthData); > + Private->AuthData = NULL; > + } > + > + if (Private->AuthScheme != NULL) { > + FreePool (Private->AuthScheme); > + Private->AuthScheme = NULL; > + } > + > if (Private->DnsServerIp != NULL) { > FreePool (Private->DnsServerIp); > Private->DnsServerIp = NULL;