Re: [edk2-devel] [PATCH v2 13/13] NetworkPkg: Update the PxeBcDhcp6GoogleTest due to underlying changes

["Saloni Kasbekar" <saloni.kasbekar@intel.com>]

Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>

-----Original Message-----
From: Doug Flick <doug.edk2@gmail.com> 
Sent: Wednesday, May 8, 2024 10:57 PM
To: devel@edk2.groups.io
Cc: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary <zachary.clark-williams@intel.com>
Subject: [PATCH v2 13/13] NetworkPkg: Update the PxeBcDhcp6GoogleTest due to underlying changes

From: Doug Flick <dougflick@microsoft.com>

This patch updates the PxeBcDhcp6GoogleTest due to the changes in the underlying code. The changes are as follows:
 - Random now comes from the RngLib Protocol
 - The TCP ISN is now generated by the hash function

Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
Cc: Zachary Clark-williams <zachary.clark-williams@intel.com>

Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>
---
 NetworkPkg/Test/NetworkPkgHostTest.dsc                        |   1 +
 NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf |   3 +-
 NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp   | 102 +++++++++++++++++++-
 3 files changed, 100 insertions(+), 6 deletions(-)

diff --git a/NetworkPkg/Test/NetworkPkgHostTest.dsc b/NetworkPkg/Test/NetworkPkgHostTest.dsc
index fa301a7a52ab..1772afb05815 100644
--- a/NetworkPkg/Test/NetworkPkgHostTest.dsc
+++ b/NetworkPkg/Test/NetworkPkgHostTest.dsc
@@ -30,6 +30,7 @@ [Components]
   NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf {     <LibraryClasses>       UefiRuntimeServicesTableLib|MdePkg/Test/Mock/Library/GoogleTest/MockUefiRuntimeServicesTableLib/MockUefiRuntimeServicesTableLib.inf+      UefiBootServicesTableLib|MdePkg/Test/Mock/Library/GoogleTest/MockUefiBootServicesTableLib/MockUefiBootServicesTableLib.inf   }  # Despite these library classes being listed in [LibraryClasses] below, they are not needed for the host-based unit tests.diff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf
index 301dcdf61109..8b092d9291d4 100644
--- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf
+++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/UefiPxeBcDxeGoogleTest.inf
@@ -14,7 +14,7 @@ [Defines]
 # # The following information is for reference only and not required by the build tools. #-#  VALID_ARCHITECTURES           = IA32 X64+#  VALID_ARCHITECTURES           = IA32 X64 AARCH64 #  [Sources]@@ -23,6 +23,7 @@ [Sources]
   PxeBcDhcp6GoogleTest.h   ../PxeBcDhcp6.c   ../PxeBcSupport.c+  ../../../MdePkg/Test/Mock/Library/GoogleTest/Protocol/MockRng.cpp  [Packages]   MdePkg/MdePkg.decdiff --git a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp
index bd423ebadfce..61736ff79e83 100644
--- a/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp
+++ b/NetworkPkg/UefiPxeBcDxe/GoogleTest/PxeBcDhcp6GoogleTest.cpp
@@ -7,6 +7,8 @@
 #include <Library/GoogleTestLib.h> #include <GoogleTest/Library/MockUefiLib.h> #include <GoogleTest/Library/MockUefiRuntimeServicesTableLib.h>+#include <GoogleTest/Library/MockUefiBootServicesTableLib.h>+#include <GoogleTest/Protocol/MockRng.h>  extern "C" {   #include <Uefi.h>@@ -165,7 +167,7 @@ protected:
 // Note: // Testing PxeBcHandleDhcp6Offer() is difficult because it depends on a // properly setup Private structure. Attempting to properly test this function-// without a signficant refactor is a fools errand. Instead, we will test+// without a significant refactor is a fools errand. Instead, we will test // that we can prevent an overflow in the function. TEST_F (PxeBcHandleDhcp6OfferTest, BasicUsageTest) {   PXEBC_DHCP6_PACKET_CACHE  *Cache6 = NULL;@@ -238,6 +240,7 @@ TEST_F (PxeBcCacheDnsServerAddressesTest, BasicUsageTest) {
     FreePool (Option);   } }+ // Test Description // Test that we can prevent an overflow in the function TEST_F (PxeBcCacheDnsServerAddressesTest, AttemptOverflowTest) {@@ -470,10 +473,15 @@ TEST_F (PxeBcRequestBootServiceTest, AttemptRequestOverFlowExpectFailure) {
 class PxeBcDhcp6DiscoverTest : public ::testing::Test { public:   PXEBC_PRIVATE_DATA Private = { 0 };+  // create a mock md5 hash+  UINT8 Md5Hash[16] = { 0 };+   EFI_UDP6_PROTOCOL Udp6Read;  protected:   MockUefiRuntimeServicesTableLib RtServicesMock;+  MockUefiBootServicesTableLib BsMock;+  MockRng RngMock;    // Add any setup code if needed   virtual void@@ -527,8 +535,21 @@ TEST_F (PxeBcDhcp6DiscoverTest, BasicOverflowTest) {
    Private.Dhcp6Request->Length = (UINT16)(Cursor - (UINT8 *)Private.Dhcp6Request); -  EXPECT_CALL (RtServicesMock, gRT_GetTime)-    .WillOnce (::testing::Return (0));+  EXPECT_CALL (BsMock, gBS_LocateProtocol)+    .WillOnce (+       ::testing::DoAll (+                    ::testing::SetArgPointee<2> (::testing::ByRef (gRngProtocol)),+                    ::testing::Return (EFI_SUCCESS)+                    )+       );++  EXPECT_CALL (RngMock, GetRng)+    .WillOnce (+       ::testing::DoAll (+                    ::testing::SetArgPointee<3> (::testing::ByRef (Md5Hash[0])),+                    ::testing::Return (EFI_SUCCESS)+                    )+       );    ASSERT_EQ (     PxeBcDhcp6Discover (@@ -558,8 +579,21 @@ TEST_F (PxeBcDhcp6DiscoverTest, BasicUsageTest) {
    Private.Dhcp6Request->Length = (UINT16)(Cursor - (UINT8 *)Private.Dhcp6Request); -  EXPECT_CALL (RtServicesMock, gRT_GetTime)-    .WillOnce (::testing::Return (0));+  EXPECT_CALL (BsMock, gBS_LocateProtocol)+    .WillOnce (+       ::testing::DoAll (+                    ::testing::SetArgPointee<2> (::testing::ByRef (gRngProtocol)),+                    ::testing::Return (EFI_SUCCESS)+                    )+       );++  EXPECT_CALL (RngMock, GetRng)+    .WillOnce (+       ::testing::DoAll (+                    ::testing::SetArgPointee<3> (::testing::ByRef (Md5Hash[0])),+                    ::testing::Return (EFI_SUCCESS)+                    )+       );    ASSERT_EQ (     PxeBcDhcp6Discover (@@ -572,3 +606,61 @@ TEST_F (PxeBcDhcp6DiscoverTest, BasicUsageTest) {
     EFI_SUCCESS     ); }++TEST_F (PxeBcDhcp6DiscoverTest, MultipleRequestsAttemptOverflow) {+  EFI_IPv6_ADDRESS         DestIp     = { 0 };+  EFI_DHCP6_PACKET_OPTION  RequestOpt = { 0 }; // the data section doesn't really matter++  RequestOpt.OpCode = HTONS (0x1337);+  RequestOpt.OpLen  = HTONS (REQUEST_OPTION_LENGTH); // this length would overflow without a check+  UINT8  RequestOptBuffer[REQUEST_OPTION_LENGTH] = { 0 };++  // make sure we have enough space for 10 of these options+  ASSERT_TRUE (REQUEST_OPTION_LENGTH * 10 <= PACKET_SIZE);++  UINT8             Index   = 0;+  EFI_DHCP6_PACKET  *Packet = (EFI_DHCP6_PACKET *)&Private.Dhcp6Request[Index];+  UINT8             *Cursor = (UINT8 *)(Packet->Dhcp6.Option);++  // let's add 10 of these options - this should overflow+  for (UINT8 i = 0; i < 10; i++) {+    CopyMem (Cursor, &RequestOpt, sizeof (RequestOpt));+    Cursor += sizeof (RequestOpt) - 1;+    CopyMem (Cursor, RequestOptBuffer, REQUEST_OPTION_LENGTH);+    Cursor += REQUEST_OPTION_LENGTH;+  }++  // Update the packet length+  Packet->Length = (UINT16)(Cursor - (UINT8 *)Packet);+  Packet->Size   = PACKET_SIZE;++  // Make sure we're larger than the buffer we're trying to write into+  ASSERT_TRUE (Packet->Length > sizeof (EFI_PXE_BASE_CODE_DHCPV6_PACKET));++  EXPECT_CALL (BsMock, gBS_LocateProtocol)+    .WillOnce (+       ::testing::DoAll (+                    ::testing::SetArgPointee<2> (::testing::ByRef (gRngProtocol)),+                    ::testing::Return (EFI_SUCCESS)+                    )+       );++  EXPECT_CALL (RngMock, GetRng)+    .WillOnce (+       ::testing::DoAll (+                    ::testing::SetArgPointee<3> (::testing::ByRef (Md5Hash[0])),+                    ::testing::Return (EFI_SUCCESS)+                    )+       );++  ASSERT_EQ (+    PxeBcDhcp6Discover (+      &(PxeBcDhcp6DiscoverTest::Private),+      0,+      NULL,+      FALSE,+      (EFI_IP_ADDRESS *)&DestIp+      ),+    EFI_OUT_OF_RESOURCES+    );+}-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119200): https://edk2.groups.io/g/devel/message/119200
Mute This Topic: https://groups.io/mt/105996592/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-