Re: AsciiPrint() in HttpBootDxe Corrupting the Setup screen

[Karunakar P <karunakarp@amiindia.co.in>]

Hi Jiaxin,

I've filled a bug in bugzilla and attached the changes, Please find the below bug details
https://bugzilla.tianocore.org/show_bug.cgi?id=860

Thanks,
Karunakar

From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
Sent: Monday, January 22, 2018 6:23 AM
To: Karunakar P; Ye, Ting; Fu, Siyuan; 'edk2-devel@lists.01.org'
Subject: RE: AsciiPrint() in HttpBootDxe Corrupting the Setup screen

Hi Karunakar,

Please also report the issue on Bugzilla.

Thanks,
Jiaxin

From: Wu, Jiaxin
Sent: Saturday, January 20, 2018 2:37 PM
To: Karunakar P <karunakarp@amiindia.co.in<mailto:karunakarp@amiindia.co.in>>; Ye, Ting <ting.ye@intel.com<mailto:ting.ye@intel.com>>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; 'edk2-devel@lists.01.org' <edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>>
Subject: RE: AsciiPrint() in HttpBootDxe Corrupting the Setup screen

Hi Karunakar,

You should sent out the attached patches for the review:).

Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>

Thanks,
Jiaxin


From: Karunakar P [mailto:karunakarp@amiindia.co.in]
Sent: Wednesday, January 17, 2018 6:29 PM
To: Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Ye, Ting <ting.ye@intel.com<mailto:ting.ye@intel.com>>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; 'edk2-devel@lists.01.org' <edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>>
Subject: RE: AsciiPrint() in HttpBootDxe Corrupting the Setup screen

[Patch] NetworkPkg\HttpBootDxe: AsciiPrint() in HttpBootDxe Corrupting the Setup screen

NetworkPkg\HttpBootDxe\HttpBootSupport.c | 2 ----
NetworkPkg\HttpBootDxe\HttpBootClient.c|  10 ++++
2 files changed, 10 insertions(+), 2 deletions(-)

NetworkPkg\HttpBootDxe\HttpBootSupport.c
NetworkPkg\HttpBootDxe\HttpBootClient.c

EFI_STATUS
HttpBootCheckUriScheme (
  IN      CHAR8                  *Uri
  )
{
  UINTN                Index;
  EFI_STATUS           Status;.
.
.
  //
  // Return EFI_INVALID_PARAMETER if the URI is not HTTP or HTTPS.
  //
  if ((AsciiStrnCmp (Uri, "http://", 7) != 0) && (AsciiStrnCmp (Uri, "https://", 8) != 0)) {
-    AsciiPrint ("\n  Error: Invalid URI address.\n");
    DEBUG ((EFI_D_ERROR, "HttpBootCheckUriScheme: Invalid Uri.\n"));
    return EFI_INVALID_PARAMETER;
  }

  //
  // HTTP is disabled, return EFI_ACCESS_DENIED if the URI is HTTP.
  //
  if (!PcdGetBool (PcdAllowHttpConnections) && (AsciiStrnCmp (Uri, "http://", 7) == 0)) {
-    AsciiPrint ("\n  Error: Access forbidden, only HTTPS connection is allowed.\n");
    DEBUG ((EFI_D_ERROR, "HttpBootCheckUriScheme: HTTP is disabled.\n"));
    return EFI_ACCESS_DENIED;
  }
.
.
.
}


EFI_STATUS
HttpBootDhcp4ExtractUriInfo (
  IN     HTTP_BOOT_PRIVATE_DATA   *Private
  )
{
  HTTP_BOOT_DHCP4_PACKET_CACHE    *SelectOffer;
  HTTP_BOOT_DHCP4_PACKET_CACHE    *HttpOffer;
  UINT32                          SelectIndex;.
.
.
.
//
  // Check the URI scheme.
  //
  Status = HttpBootCheckUriScheme (Private->BootFileUri);
  if (EFI_ERROR (Status)) {
    DEBUG ((EFI_D_ERROR, "HttpBootDhcp4ExtractUriInfo: %r.\n", Status));
+    if (Status == EFI_INVALID_PARAMETER) {
+        AsciiPrint ("\n  Error: Invalid URI address.\n");
+   } else if (Status == EFI_ACCESS_DENIED) {
+        AsciiPrint ("\n  Error: Access forbidden, only HTTPS connection is allowed.\n");
+   }
    return Status;
  }
.
.
.
}


EFI_STATUS
HttpBootDhcp6ExtractUriInfo (
  IN     HTTP_BOOT_PRIVATE_DATA   *Private
  )
{
  HTTP_BOOT_DHCP6_PACKET_CACHE    *SelectOffer;
  HTTP_BOOT_DHCP6_PACKET_CACHE    *HttpOffer;
  UINT32                          SelectIndex;
.
.
.
Status = HttpBootCheckUriScheme (Private->BootFileUri);
  if (EFI_ERROR (Status)) {
    DEBUG ((EFI_D_ERROR, "HttpBootDhcp6ExtractUriInfo: %r.\n", Status));
+    if (Status == EFI_INVALID_PARAMETER) {
+        AsciiPrint ("\n  Error: Invalid URI address.\n");
+    } else if (Status == EFI_ACCESS_DENIED) {
+       AsciiPrint ("\n  Error: Access forbidden, only HTTPS connection is allowed.\n");
+   }
    return Status;
  }
.
.
.
}

Please review the patch.

Thanks,
Karunakar


From: Karunakar P
Sent: Wednesday, January 17, 2018 2:44 PM
To: 'Wu, Jiaxin'; Ye, Ting; Fu, Siyuan; 'edk2-devel@lists.01.org'
Subject: RE: AsciiPrint() in HttpBootDxe Corrupting the Setup screen

Hi Jiaxin,

We'll send the formal patch for review and also could you please let me know if you want to fill a bug in Bugzilla if needed.

Thank You,
Karunakar

From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
Sent: Thursday, January 11, 2018 6:21 AM
To: Karunakar P; Ye, Ting; Fu, Siyuan
Subject: RE: AsciiPrint() in HttpBootDxe Corrupting the Setup screen

Hi Karunakar,

I agree the fix, can you send out the formal patch for the review or need us to follow that?

Thanks,
Jiaxin

From: Karunakar P [mailto:karunakarp@amiindia.co.in]
Sent: Wednesday, January 10, 2018 4:48 PM
To: Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Ye, Ting <ting.ye@intel.com<mailto:ting.ye@intel.com>>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>
Subject: AsciiPrint() in HttpBootDxe Corrupting the Setup screen

Hello All,

[Issue]

1.       On giving Invalid URI in Boot URI field in "HTTP Boot Configuration" Page, doing AsciiPrint() in TSE corrupting the Screen.

AsciiPrint ("\n  Error: Invalid URI address.\n");



2.       When HTTP connection are disabled using "PcdAllowHttpConnections"

On giving http URI in Boot URI field in "HTTP Boot Configuration" Page, doing AsciiPrint() in TSE corrupting the Screen.

AsciiPrint ("\n  Error: Access forbidden, only HTTPS connection is allowed.\n");


[Fix]

1.       I guess We've added this AsciiPrint() because HttpBootCheckUriScheme() is common for both generic HTTP boot over IPv4/6 and "Http Boot Configuration" page

2.       In case of "Http Boot Configuration",  AsciiPrint() may not be needed in HttpBootCheckUriScheme because we're already using CreatePopUp() in case of Error Status

CreatePopUp (

          EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,

          &Key,

          L"ERROR: Unsupported URI!",

          L"Only supports HTTP and HTTPS",

          NULL

          );

   (Or)

CreatePopUp (

          EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,

          &Key,

          L"ERROR: Unsupported URI!",

          L"HTTP is disabled",

          NULL

          );

3.       When we do Http Boot over IPv4/6, from HttpBootCheckUriScheme() there is chance to get return status as EFI_INVALID_PARAMETER or EFI_ACCESS_DENIED

4.       In this case we can have AsciiPrint() based on return Status, instead of doing in HttpBootCheckUriScheme()

I've attached the suggested changes, could you please review and provide your comments/Suggestions.

Thanks,
Karunakar