Commit Graph

738 Commits

Author SHA1 Message Date
Bing Luo 070eadb550 NetworkPkg/UefiPxeBcDxe: Bugfix for pxe driver
Ensure the poniter is not null before free it

Signed-off-by: Bing Luo <robin.luo@jaguarmicro.com>
2024-12-31 23:26:19 +00:00
Phil Noh b8602d8fee NetworkPkg/WifiConnectionManagerDxe: Keep Setup page on no Wi-Fi module
With the driver, "Wi-Fi Configuration" Setup page is available regardless
of Wi-Fi module existence. When there is no Wi-Fi module, it is found that
entering the page causes exiting Setup browser. User would think the result
as an error. For better user experience, the update enables the page to
report the module status like a blank page. It prevents exiting Setup.

Signed-off-by: Phil Noh <Phil.Noh@amd.com>
2024-12-18 21:04:51 +00:00
Alexander Gryanko 35706d43c5 NetworkPkg: PXE boot option build flag
Currently, the only way to disable PXE boot options is to change the PCD
variables PcdIPv4PXESupport and PcdIPv6PXESupport in the source code or
use the "--pcd" option in the build script. Other boot options such
as HTTP or iSCSI can be disabled using the -D<option> flag.
NETWORK_PXE_BOOT_ENABLE will add a consistent way to disable PXE booting.
This is the third and final part of a series of patches to enable the
NETWORK_PXE_BOOT_ENABLE build flag. At this point, the flag will be able
to disable PXE functionality.

Signed-off-by: Alexander Gryanko <xpahos@gmail.com>
2024-12-06 18:26:01 +00:00
Mike Maslenkin e8cfc7beba NetworkPkg/IScsiDxe: fix HiiOpCodeHandle leak
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
2024-12-06 17:13:17 +00:00
Gerd Hoffmann 2940708eb2 NetworkPkg/DxeNetLib: drop GLOBAL_REMOVE_IF_UNREFERENCED
With mSecureHashAlgorithms being static this should not be
needed any more.

Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2024-12-04 22:25:37 +00:00
Ard Biesheuvel e54794bcc6 Remove all ArmSoftFloatLib library class resolutions
ArmSoftFloatLib is going away, so remove all residual references to it.

Continuous-integration-options: PatchCheck.ignore-multi-package
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
2024-11-26 22:00:36 +00:00
Oliver Smith-Denny 0d129450c2 NetworkPkg: Restore TPL Before Return
This patch fixes a few instances of error cases in NetworkPkg
returning after a RaiseTPL call without restoring the TPL
first.

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
2024-11-22 19:45:03 +00:00
Gerd Hoffmann fe1272dc57 NetworkPkg/DxeNetLib: make mSecureHashAlgorithms static
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2024-11-14 17:03:03 +00:00
Oliver Smith-Denny ef4f3aa3f7 MdePkg: MdeLibs.dsc.inc: Apply StackCheckLibNull to All Module Types
Now that the ResetVectors are USER_DEFINED modules, they will not
be linked against StackCheckLibNull, which were the only modules
causing issues. So, we can now remove the kludge we had before
and the requirement for every DSC to include StackCheckLibNull
for SEC modules and just apply StackCheckLibNull globally.

This also changes every DSC to drop the SEC definition of
StackCheckLibNull.

Continuous-integration-options: PatchCheck.ignore-multi-package

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
2024-11-13 21:01:46 +00:00
Aleksandr Goncharov fc9f06de6f NetworkPkg: mark fixed network PCDs
Rename `NetworkPcds` to `NetworkFixedPcds` to avoid confusion with
dynamic PCDs. The next patches in the chain will update all references
across the codebase to use the new name.

Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
Cc: Zachary Clark-williams <zachary.clark-williams@intel.com>
Signed-off-by: Aleksandr Goncharov <chat@joursoir.net>
2024-11-11 13:38:51 +00:00
Aleksandr Goncharov 1db5895c39 NetworkPkg: introduce include file for dynamic PCDs
Introduce an include file with dynamic PCDs to simplify the usage of
the network stage. This allows us to stop manually adding
`PcdIPv4PXESupport` and `PcdIPv6PXESupport` to the DSC file.

`NETWORK_IP4_ENABLE` and `NETWORK_IP6_ENABLE` are not used because
PXEv4 and PXEv6 boot support can be controlled from the QEMU command
line.

Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
Cc: Zachary Clark-williams <zachary.clark-williams@intel.com>
Signed-off-by: Aleksandr Goncharov <chat@joursoir.net>
2024-11-11 13:38:51 +00:00
Michael D Kinney 599c8309a5 NetworkPkg/Dhcp6Dxe: Fix sanitizer issues
* EFI_DHCP6_DUID structure declares Duid[1], so the size
  of that structure is not large enough to hold an entire
  Duid. Instead, compute the correct size to allocate an
  EFI_DHCP6_DUID structure.
* Dhcp6AppendOption() takes a length parameter that in
  network order. Update test cases to make sure a network
  order length is passed in. A value of 0x0004 was being
  passed in and was then converted to 0x0400 length and
  buffer overflow was detected.

Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
2024-11-11 05:07:25 +00:00
Leandro Becker b3efbda166 NetworkPkg/HttpBootDxe: Report download error when resume attempts fail
When all resume attempts to continue an interrupted NBP file
download have failed, report the failure status to the caller.

Original implementation was returning success when number of
retries reaches the limit defined by PcdMaxHttpResumeRetries.

Signed-off-by: Leandro Gustavo Biss Becker <lbecker@positivo.com.br>
2024-10-15 22:26:37 +00:00
Mike Beaton 8b295e0aad NetworkPkg/Dhcp6Dxe: Fix extraneous parentheses
Without this change we get:

  error: equality comparison with extraneous parentheses

when building with -D NETWORK_IP6_ENABLE on XCODE5.

Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>
2024-09-27 16:57:36 +00:00
Mike Beaton 73dbb68006 NetworkPkg/HttpBootDxe: Correctly uninstall HttpBootCallbackProtocol
The existing HttpBootUninstallCallback was passing the wrong handle (the
PrivateData root controller handle, not the correct child IPv4 or IPv6
NIC controller handle; cf HttpBootInstallCallback for matching logic) and
was also passing the address of a pointer to the interface to be removed
rather than the pointer itself, so always failed with EFI_NOT_FOUND.

This resulted in the prior behaviour that if multiple HTTP boot attempts
were made, on the second and subsequent attempts the instance of this
protocol installed by the first attempt would be re-used. As long as only
one driver using the protocol is installed, this ends up producing the
same results as if the protocol had been uninstalled then reinstalled
correctly.

After this commit, the protocol is installed at the start of an HTTP boot
attempt and uninstalled it at the end of it (assuming nothing else has
accessed the protocol in a way which blocks the uninstall).

It might seem attractive to add an ASSERT to confirm when debugging
that the uninstall succeeds as expected, but this is recommended against
because uninstallation of protocol interfaces is allowed to fail under
the UEFI model:
https://edk2.groups.io/g/devel/message/117469.
An ASSERT could therefore arise from a sequence of events which is
perfectly valid - or at least is out of the control of this driver.

Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>
2024-09-15 21:18:08 +00:00
Pierre Gondois bc02b255a8 MdePkg: Move PcdEnforceSecureRngAlgorithms from NetworkPkg
The PcdEnforceSecureRngAlgorithms Pcd enforces the use of RNG
algorithms defined by the UEFI spec. To re-use the Pcd in other
packages and have a generic mean to control the usage of unsecure
algorithms, move the Pcd to the MdePkg.

Continuous-integration-options: PatchCheck.ignore-multi-package
Signed-off-by: Pierre Gondois <pierre.gondois@arm.com>
2024-09-13 14:34:21 +00:00
Leandro Becker 69139e39bc NetworkPkg/HttpBootDxe: Resume an interrupted boot file download.
When the boot file download operation is interrupted for some reason,
HttpBootDxe will use HTTP Range header to try resume the download
operation reusing the bytes downloaded so far.

Signed-off-by: Leandro Gustavo Biss Becker <lbecker@positivo.com.br>
2024-09-13 10:26:09 +00:00
Taylor Beebe fefd017851 NetworkPkg: Add StackCheckLib
Remove the old stack check lib now that MdeLibs.inc includes
the new one.

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
2024-09-13 03:58:46 +00:00
Oliver Smith-Denny 837bb62661 NetworkPkg: PxeBcDhcp6GoogleTest: Fix Stack Smashing Unit Test
PxeBcDhcp6GoogleTest's MultipleDnsEntries test started to fail
with stack cookies added for host applications. Debugging this
showed that the test was attempting to copy two UINT16s to a
UINT8 Data[1] array allocated on the stack. This was moved to
a heap based allocation for a UINT32 to accommodate the proper
size. After this fix, the unit test passed with stack cookies
enabled.

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
2024-09-13 03:58:46 +00:00
Mike Beaton af60615f0e NetworkPkg: Fix unable to build OVMF with -D NETWORK_ENABLE=0
https://bugzilla.tianocore.org/show_bug.cgi?id=4829

7f17a15 (2024/02/22)
"OvmfPkg: Shell*.inc: allow building without network support"
breaks building OVMF with `-D NETWORK_ENABLE=0`.

Before this commit we could build OVMF e.g. with the following
command in the OvmfPkg directory:

./build.sh -D NETWORK_ENABLE=0

After the commit the same command fails early with:

/home/user/OpenSource/edk2/OvmfPkg/OvmfPkgX64.dsc(15):
error F001: Pcd (gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections)
defined in DSC is not declared in DEC files referenced in INF files in
FDF. Arch: ['X64']

This commit conditionally removes the undefined Pcd reference in
NetworkPkg which is part of this issue.

Similar changes are needed in separate commits for
OvmfPkg (and for ArmVirtPkg, since the issue also
exists there, although masked by another issue).

Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>
2024-09-12 21:34:33 +00:00
Oliver Smith-Denny 8f74b95a21 MdePkg: Move CompilerIntrinsicsLib from ArmPkg
As per the emailed RFC in
https://edk2.groups.io/g/devel/topic/rfc_move/107675828,
this patch moves CompilerIntrinsicsLib from ArmPkg to
MdePkg as this library provides compiler intrinsics, which
are industry standard.

This aligns with the goal of integrating ArmPkg into existing
packages: https://bugzilla.tianocore.org/show_bug.cgi?id=4121.

The newly placed CompilerIntrinsicsLib is added to MdeLibs.dsc.inc
as every DSC that builds ARM/AARCH64 needs this library added. The
old location is removed from every DSC in edk2 in this commit also
to not break bisectability with minimal hoop jumping.

Continuous-integration-options: PatchCheck.ignore-multi-package

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
2024-09-12 19:36:59 +00:00
Mike Beaton f2557032d6 NetworkPkg/MnpDxe: Convert TX buffer allocation messages to DEBUG_VERBOSE
Under normal operation, some 30 or so of these lines logged as DEBUG_INFO
on first transmit.

This is not relevant information for users of the driver, so convert these
messages to VERBOSE.

Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>
2024-09-11 19:32:39 +00:00
yhsu3 589304e67f Support Report Status Code in the UefiPxe driver.
Report PXE error status via Status Code, with this design,
it will be flexible to register a status code handler
via gEfiRscHandlerProtocolGuid to output the customized error code
to other telemetry service.

The subclass code is `EFI_IO_BUS_IP_NETWORK`

Signed-off-by: Ethan Hsu <Eathonhsu@gmail.com>
2024-09-11 16:36:03 +00:00
Mike Beaton 3885a3edad NetworkPkg/DxeNetLib: Update misleading comment
Commit 6862b9d538 makes
more explicit the previous logic of the code anyway, which is that
it is (and was) only a fatal error if all secure algorithms fail.

However the comment updated by this commit seems somewhat
incompatible with that change, and even with the previous code
(which operated as now, just logging different error messages).

This updates the comment to be more compatible with how the
code operates.

Signed-off-by: Mike Beaton <mjsbeaton@gmail.com>
2024-09-09 17:53:44 +00:00
Neo Hsueh 72cf76868c NetworkPkg/WifiConnectionManagerDxe: Fix Connection Manager HII errors
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4824

Fix the wrong logic in WifiMgrDxeHiiConfigAccessCallback with
EFI_BROWSER_ACTION_CHANGING action.

Cc: Jiangang He <jiangang.he@amd.com>
Cc: Abner Chang <abner.chang@amd.com>
Signed-off-by: Neo Hsueh <Hong-Chih.Hsueh@amd.com>
2024-09-03 06:24:43 +00:00
Joey Vagedes ea5581186e NetworkPkg/NetworkPkg.ci.yaml: Add PrEval CI config
Adds an entry to the package's CI configuration file that enable policy
5 for stuart_pr_eval. With this Policy, all INFs used by the package are
extracted from the provided DSC file and compared against the list of
changed *.inf (INF) files in the PR. If there is a match, stuart_pr_eval
will specify that this package is affected by the PR and needs to be
tested.

Signed-off-by: Joey Vagedes <joey.vagedes@gmail.com>
2024-09-02 23:20:35 +00:00
Nickle Wang 814470b834 NetworkPkg/SnpDxe: return error for unsupported parameter
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4729

From SCT testing report, Reset() does not support the case when
ExtendedVerification is set to FALSE. So, we should return
EFI_INVALID_PARAMETER in this case. For details, please refer to
Bug 4729.

Signed-off-by: Nickle Wang <nicklew@nvidia.com>
2024-09-02 22:17:12 +00:00
Ken Lautner 82c5cacd13 NetworkPkg: DxeHttpLib: Use HTTP error 429
Include a mapping for HTTP error 429 to return the correct
status code. Additionally include a link to the official
HTTP status codes in the HttpMappingToStatusCode function header.

Signed-off-by: Kenneth Lautner <kenlautner3@gmail.com>
2024-09-02 21:17:25 +00:00
Saloni Kasbekar 0e8af88034 NetworkPkg: Improve GetBootFile() code flow
Introduce state machine to improve the code flow in GetBootFile()
to make it more readable. Allows new states to be easily added
without adding further nested ifs.

Signed-off-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-08-27 01:55:39 +00:00
Saloni Kasbekar ab6ad2fbdb NetworkPkg/DxeHttpLib: Support HTTP CONNECT message in Tx path.
Add HTTP CONNECT message support in HttpGenRequestMessage()

Signed-off-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-08-07 23:01:48 +00:00
Saloni Kasbekar 5c86b0b57c NetworkPkg/HttpDxe: Track HttpInstance URL buffer length.
In EfiHttpRequest(), length of target URLs was always compared to
fixed-size value, even after allocating a larger URL buffer. Added
UrlLen to HTTP_PROTOCOL to store the size and reallocate if the size
changes.

Signed-off-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-07-11 21:42:32 +00:00
Gerd Hoffmann 6862b9d538 NetworkPkg/DxeNetLib: adjust PseudoRandom error logging
There is a list of allowed rng algorithms, if /one/ of them is not
supported this is not a problem, only /all/ of them failing is an
error condition.

Downgrade the message for a single unsupported algorithm from ERROR to
VERBOSE.  Add an error message in case we finish the loop without
finding a supported algorithm.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
2024-06-27 15:14:56 +00:00
Sam ced13b93af NetworkPkg TcpDxe: Fixed system stuck on PXE boot flow in iPXE environment
This bug fix is based on the following commit "NetworkPkg TcpDxe: SECURITY PATCH"
REF: 1904a64

Issue Description:
An "Invalid handle" error was detected during runtime when attempting to destroy a child instance of the hashing protocol. The problematic code segment was:

NetworkPkg\TcpDxe\TcpDriver.c
Status = Hash2ServiceBinding->DestroyChild(Hash2ServiceBinding, ​&mHash2ServiceHandle);

Root Cause Analysis:
The root cause of the error was the passing of an incorrect parameter type, a pointer to an EFI_HANDLE instead of an EFI_HANDLE itself, to the DestroyChild function. This mismatch resulted in the function receiving an invalid handle.

Implemented Solution:
To resolve this issue, the function call was corrected to pass mHash2ServiceHandle directly:

NetworkPkg\TcpDxe\TcpDriver.c
Status = Hash2ServiceBinding->DestroyChild(Hash2ServiceBinding, mHash2ServiceHandle);

This modification ensures the correct handle type is used, effectively rectifying the "Invalid handle" error.

Verification:
Testing has been conducted, confirming the efficacy of the fix. Additionally, the BIOS can boot into the OS in an iPXE environment.

Cc: Doug Flick [MSFT] <doug.edk2@gmail.com>

Signed-off-by: Sam Tsai [Wiwynn] <sam_tsai@wiwynn.com>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-05-30 21:06:38 +00:00
Doug Flick 207b6d68a0 NetworkPkg: Update the PxeBcDhcp6GoogleTest due to underlying changes
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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-05-24 15:48:52 +00:00
Doug Flick 1904a64bcc NetworkPkg TcpDxe: SECURITY PATCH CVE-2023-45236
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4541
REF: https://www.rfc-editor.org/rfc/rfc1948.txt
REF: https://www.rfc-editor.org/rfc/rfc6528.txt
REF: https://www.rfc-editor.org/rfc/rfc9293.txt

Bug Overview:
PixieFail Bug #8
CVE-2023-45236
CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:C/C:L/I:N/A:N
CWE-200 Exposure of Sensitive Information to an Unauthorized Actor

Updates TCP ISN generation to use a cryptographic hash of the
connection's identifying parameters and a secret key.
This prevents an attacker from guessing the ISN used for some other
connection.

This is follows the guidance in RFC 1948, RFC 6528, and RFC 9293.

RFC: 9293 Section 3.4.1.  Initial Sequence Number Selection

   A TCP implementation MUST use the above type of "clock" for clock-
   driven selection of initial sequence numbers (MUST-8), and SHOULD
   generate its initial sequence numbers with the expression:

   ISN = M + F(localip, localport, remoteip, remoteport, secretkey)

   where M is the 4 microsecond timer, and F() is a pseudorandom
   function (PRF) of the connection's identifying parameters ("localip,
   localport, remoteip, remoteport") and a secret key ("secretkey")
   (SHLD-1).  F() MUST NOT be computable from the outside (MUST-9), or
   an attacker could still guess at sequence numbers from the ISN used
   for some other connection.  The PRF could be implemented as a
   cryptographic hash of the concatenation of the TCP connection
   parameters and some secret data.  For discussion of the selection of
   a specific hash algorithm and management of the secret key data,
   please see Section 3 of [42].

   For each connection there is a send sequence number and a receive
   sequence number.  The initial send sequence number (ISS) is chosen by
   the data sending TCP peer, and the initial receive sequence number
   (IRS) is learned during the connection-establishing procedure.

   For a connection to be established or initialized, the two TCP peers
   must synchronize on each other's initial sequence numbers.  This is
   done in an exchange of connection-establishing segments carrying a
   control bit called "SYN" (for synchronize) and the initial sequence
   numbers.  As a shorthand, segments carrying the SYN bit are also
   called "SYNs".  Hence, the solution requires a suitable mechanism for
   picking an initial sequence number and a slightly involved handshake
   to exchange the ISNs.

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-05-24 15:48:52 +00:00
Doug Flick 4c4ceb2ceb NetworkPkg: SECURITY PATCH CVE-2023-45237
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4542

Bug Overview:
PixieFail Bug #9
CVE-2023-45237
CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:N
CWE-338 Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)

Use of a Weak PseudoRandom Number Generator

Change Overview:

Updates all Instances of NET_RANDOM (NetRandomInitSeed ()) to either

>
> EFI_STATUS
> EFIAPI
> PseudoRandomU32 (
>  OUT UINT32  *Output
>  );
>

or (depending on the use case)

>
> EFI_STATUS
> EFIAPI
> PseudoRandom (
>  OUT  VOID   *Output,
>  IN   UINTN  OutputLength
>  );
>

This is because the use of

Example:

The following code snippet PseudoRandomU32 () function is used:

>
> UINT32         Random;
>
> Status = PseudoRandomU32 (&Random);
> if (EFI_ERROR (Status)) {
>   DEBUG ((DEBUG_ERROR, "%a failed to generate random number: %r\n",
__func__, Status));
>   return Status;
> }
>

This also introduces a new PCD to enable/disable the use of the
secure implementation of algorithms for PseudoRandom () and
instead depend on the default implementation. This may be required for
some platforms where the UEFI Spec defined algorithms are not available.

>
> PcdEnforceSecureRngAlgorithms
>

If the platform does not have any one of the UEFI defined
secure RNG algorithms then the driver will assert.

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-05-24 15:48:52 +00:00
Liqi Liu fcfdbe2987 NetworkPkg/WifiConnectionManagerDxe: Update UI according to UEFI spec
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4736

In UEFI_Spec_2_10_Aug29.pdf page 1694 section 35.5.4 for
EFI_BROWSER_ACTION_FORM_OPEN:
NOTE: EFI_FORM_BROWSER2_PROTOCOL.BrowserCallback() cannot be used with
this browser action because question values have not been retrieved yet.

So should not call HiiGetBrowserData() and HiiSetBrowserData() in FORM_OPEN
call back function.

Now use wifi list key and enroll cert key instead of the connect action key,
move wifi info display from open action to close action.

Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
Cc: Zachary Clark-williams <zachary.clark-williams@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Felix Polyudov <Felixp@ami.com>
Signed-off-by: Liqi Liu <liqi.liu@intel.com>
Reviewed-by: Zachary Clark-williams <zachary.clark-williams@intel.com>
Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
2024-04-16 14:27:51 +00:00
Doug Flick 5fd3078a2e NetworkPkg: : Updating SecurityFixes.yaml
This captures the related security change for Dhcp6Dxe that is related
to CVE-2023-45229

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
2024-02-14 03:28:11 +00:00
Doug Flick 75deaf5c3c NetworkPkg: Dhcp6Dxe: Packet-Length is not updated before appending
In order for Dhcp6AppendIaAddrOption (..) to safely append the IA
Address option, the Packet-Length field must be updated before appending
the option.

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
2024-02-14 03:28:11 +00:00
Doug Flick af3fad99d6 NetworkPkg: Dhcp6Dxe: Removes duplicate check and replaces with macro
Removes duplicate check after merge

>
>  //
>  // Verify the PacketCursor is within the packet
>  //
>  if (  (*PacketCursor < Packet->Dhcp6.Option)
>     || (*PacketCursor >= Packet->Dhcp6.Option + (Packet->Size -
sizeof (EFI_DHCP6_HEADER))))
>  {
>    return EFI_INVALID_PARAMETER;
>  }
>

Converts the check to a macro and replaces all instances of the check
with the macro

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
2024-02-14 03:28:11 +00:00
Doug Flick 1c440a5ece NetworkPkg: Dhcp6Dxe: SECURITY PATCH CVE-2023-45229 Related Patch
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4673
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4534

This was not part of the Quarkslab bugs however the same pattern
as CVE-2023-45229 exists in Dhcp6UpdateIaInfo.

This patch replaces the code in question with the safe function
created to patch CVE-2023-45229

>
>   if (EFI_ERROR (
>        Dhcp6SeekInnerOptionSafe (
>          Instance->Config->IaDescriptor.Type,
>          Option,
>          OptionLen,
>          &IaInnerOpt,
>          &IaInnerLen
>          )
>        ))
>  {
>    return EFI_DEVICE_ERROR;
>  }
>

Additionally corrects incorrect usage of macro to read the status

> - StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)DHCP6_OFFSET_OF_OPT_LEN
 (Option)));
> + StsCode = NTOHS (ReadUnaligned16 ((UINT16 *)
DHCP6_OFFSET_OF_STATUS_CODE (Option));

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
2024-02-14 03:28:11 +00:00
Doug Flick via groups.io 1d0b95f645 NetworkPkg: : Adds a SecurityFix.yaml file
This creates / adds a security file that tracks the security fixes
found in this package and can be used to find the fixes that were
applied.

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-02-06 19:24:26 +00:00
Doug Flick ff2986358f NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45235 Unit Tests
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4540

Unit tests to confirm that the bug..

Buffer overflow when handling Server ID option from a DHCPv6 proxy
Advertise message

..has been patched.

This patch contains unit tests for the following functions:
PxeBcRequestBootService
PxeBcDhcp6Discover

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-02-06 19:24:26 +00:00
Doug Flick fac297724e NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45235 Patch
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4540

Bug Details:
PixieFail Bug #7
CVE-2023-45235
CVSS 8.3 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:L/A:H
CWE-119 Improper Restriction of Operations within the Bounds of
 a Memory Buffer

Buffer overflow when handling Server ID option from a DHCPv6 proxy
Advertise message

Change Overview:

Performs two checks

1. Checks that the length of the duid is accurate
> + //
> + // Check that the minimum and maximum requirements are met
> + //
> + if ((OpLen < PXEBC_MIN_SIZE_OF_DUID) ||
(OpLen > PXEBC_MAX_SIZE_OF_DUID)) {
> +  Status = EFI_INVALID_PARAMETER;
> +  goto ON_ERROR;
> + }

2. Ensures that the amount of data written to the buffer is tracked and
never exceeds that
> + //
> + // Check that the option length is valid.
> + //
> + if ((DiscoverLen + OpLen + PXEBC_COMBINED_SIZE_OF_OPT_CODE_AND_LEN)
 > DiscoverLenNeeded) {
> +     Status = EFI_OUT_OF_RESOURCES;
> +     goto ON_ERROR;
> + }

Additional code clean up and fix for memory leak in case Option was NULL

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-02-06 19:24:26 +00:00
Doug Flick 458c582685 NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45234 Unit Tests
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4539

Unit tests to that the bug..

Buffer overflow when processing DNS Servers option in a DHCPv6 Advertise
message

..has been patched

This contains tests for the following functions:
PxeBcHandleDhcp6Offer
PxeBcCacheDnsServerAddresses

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-02-06 19:24:26 +00:00
Doug Flick 1b53515d53 NetworkPkg: UefiPxeBcDxe: SECURITY PATCH CVE-2023-45234 Patch
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4539

Bug Details:
PixieFail Bug #6
CVE-2023-45234
CVSS 8.3 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:L/A:H
CWE-119 Improper Restriction of Operations within the Bounds of
 a Memory Buffer

Buffer overflow when processing DNS Servers option in a DHCPv6
Advertise message

Change Overview:

Introduces a function to cache the Dns Server and perform sanitizing
on the incoming DnsServerLen to ensure that the length is valid

> + EFI_STATUS
> + PxeBcCacheDnsServerAddresses (
> +  IN PXEBC_PRIVATE_DATA        *Private,
> +  IN PXEBC_DHCP6_PACKET_CACHE  *Cache6
> +  )

Additional code cleanup

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-02-06 19:24:26 +00:00
Doug Flick c9c87f08dd NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45232 Unit Tests
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Unit tests to confirm that..

Infinite loop when parsing unknown options in the Destination Options
header

and

Infinite loop when parsing a PadN option in the Destination Options
header

... have been patched

This patch tests the following functions:
Ip6IsOptionValid

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-02-06 19:24:26 +00:00
Doug Flick 4df0229ef9 NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45232 Patch
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4537
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4538

Bug Details:
PixieFail Bug #4
CVE-2023-45232
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing unknown options in the Destination Options
header

PixieFail Bug #5
CVE-2023-45233
CVSS 7.5 : CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H
CWE-835 Loop with Unreachable Exit Condition ('Infinite Loop')

Infinite loop when parsing a PadN option in the Destination Options
header

Change Overview:

Most importantly this change corrects the following incorrect math
and cleans up the code.

>   // It is a PadN option
>   //
> - Offset = (UINT8)(Offset + *(Option + Offset + 1) + 2);
> + OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
> + Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

> case Ip6OptionSkip:
> - Offset = (UINT8)(Offset + *(Option + Offset + 1));
>   OptDataLen = ((EFI_IP6_OPTION *)(Option + Offset))->Length;
>   Offset     = IP6_NEXT_OPTION_OFFSET (Offset, OptDataLen);

Additionally, this change also corrects incorrect math where the calling
function was calculating the HDR EXT optionLen as a uint8 instead of a
uint16

> - OptionLen = (UINT8)((*Option + 1) * 8 - 2);
> + OptionLen = IP6_HDR_EXT_LEN (*Option) -
IP6_COMBINED_SIZE_OF_NEXT_HDR_AND_LEN;

Additionally this check adds additional logic to santize the incoming
data

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-02-06 19:24:26 +00:00
Doug Flick 6f77463d72 NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45231 Unit Tests
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4536

Validates that the patch for...

Out-of-bounds read when handling a ND Redirect message with truncated
options

.. has been fixed

Tests the following function to ensure that an out of bounds read does
not occur
Ip6OptionValidation

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-02-06 19:24:26 +00:00
Doug Flick bbfee34f41 NetworkPkg: Ip6Dxe: SECURITY PATCH CVE-2023-45231 Patch
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4536

Bug Overview:
PixieFail Bug #3
CVE-2023-45231
CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N
CWE-125 Out-of-bounds Read

Out-of-bounds read when handling a ND Redirect message with truncated
options

Change Overview:

Adds a check to prevent truncated options from being parsed
+  //
+  // Cannot process truncated options.
+  // Cannot process options with a length of 0 as there is no Type
field.
+  //
+  if (OptionLen < sizeof (IP6_OPTION_HEADER)) {
+    return FALSE;
+  }

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>
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
2024-02-06 19:24:26 +00:00