From: "Saloni Kasbekar" <saloni.kasbekar@intel.com>
To: Doug Flick <doug.edk2@gmail.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Clark-williams, Zachary" <zachary.clark-williams@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236
Date: Wed, 15 May 2024 21:38:48 +0000 [thread overview]
Message-ID: <SN7PR11MB82817429EEC59CA8E438DF20F1EC2@SN7PR11MB8281.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20240509055633.828642-10-doug.edk2@gmail.com>
Doug,
I see you have used Hash2Protocol here, instead of HashLib. Is there an advantage of using Hash2Protocol over HashLib?
Thanks,
Saloni
-----Original Message-----
From: Doug Flick <doug.edk2@gmail.com>
Sent: Wednesday, May 8, 2024 10:56 PM
To: devel@edk2.groups.io
Cc: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary <zachary.clark-williams@intel.com>
Subject: [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236
From: Doug Flick <dougflick@microsoft.com>
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>
---
NetworkPkg/TcpDxe/TcpDxe.inf | 8 +-
NetworkPkg/TcpDxe/TcpFunc.h | 23 +-
NetworkPkg/TcpDxe/TcpMain.h | 59 ++++-
NetworkPkg/TcpDxe/TcpDriver.c | 92 +++++++- NetworkPkg/TcpDxe/TcpInput.c | 13 +-
NetworkPkg/TcpDxe/TcpMisc.c | 242 ++++++++++++++++++--
NetworkPkg/TcpDxe/TcpTimer.c | 3 +-
NetworkPkg/SecurityFixes.yaml | 22 ++
8 files changed, 414 insertions(+), 48 deletions(-)
diff --git a/NetworkPkg/TcpDxe/TcpDxe.inf b/NetworkPkg/TcpDxe/TcpDxe.inf index cf5423f4c537..76de4cf9ec3d 100644
--- a/NetworkPkg/TcpDxe/TcpDxe.inf
+++ b/NetworkPkg/TcpDxe/TcpDxe.inf
@@ -6,6 +6,7 @@
# stack has been loaded in system. This driver supports both IPv4 and IPv6 network stack. # # Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>+# Copyright (c) Microsoft Corporation # # SPDX-License-Identifier: BSD-2-Clause-Patent #@@ -68,7 +69,6 @@ [LibraryClasses]
NetLib IpIoLib - [Protocols] ## SOMETIMES_CONSUMES ## SOMETIMES_PRODUCES@@ -81,6 +81,12 @@ [Protocols]
gEfiIp6ServiceBindingProtocolGuid ## TO_START gEfiTcp6ProtocolGuid ## BY_START gEfiTcp6ServiceBindingProtocolGuid ## BY_START+ gEfiHash2ProtocolGuid ## BY_START+ gEfiHash2ServiceBindingProtocolGuid ## BY_START++[Guids]+ gEfiHashAlgorithmMD5Guid ## CONSUMES+ gEfiHashAlgorithmSha256Guid ## CONSUMES [Depex] gEfiHash2ServiceBindingProtocolGuiddiff --git a/NetworkPkg/TcpDxe/TcpFunc.h b/NetworkPkg/TcpDxe/TcpFunc.h
index a7af01fff246..c707bee3e548 100644
--- a/NetworkPkg/TcpDxe/TcpFunc.h
+++ b/NetworkPkg/TcpDxe/TcpFunc.h
@@ -2,7 +2,7 @@
Declaration of external functions shared in TCP driver. Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.<BR>-+ Copyright (c) Microsoft Corporation SPDX-License-Identifier: BSD-2-Clause-Patent **/@@ -36,8 +36,11 @@ VOID
@param[in, out] Tcb Pointer to the TCP_CB of this TCP instance. + @retval EFI_SUCCESS The operation completed successfully+ @retval others The underlying functions failed and could not complete the operation+ **/-VOID+EFI_STATUS TcpInitTcbLocal ( IN OUT TCP_CB *Tcb );@@ -128,17 +131,6 @@ TcpCloneTcb (
IN TCP_CB *Tcb ); -/**- Compute an ISS to be used by a new connection.-- @return The result ISS.--**/-TCP_SEQNO-TcpGetIss (- VOID- );- /** Get the local mss. @@ -202,8 +194,11 @@ TcpFormatNetbuf (
@param[in, out] Tcb Pointer to the TCP_CB that wants to initiate a connection. + @retval EFI_SUCCESS The operation completed successfully+ @retval others The underlying functions failed and could not complete the operation+ **/-VOID+EFI_STATUS TcpOnAppConnect ( IN OUT TCP_CB *Tcb );diff --git a/NetworkPkg/TcpDxe/TcpMain.h b/NetworkPkg/TcpDxe/TcpMain.h
index c0c9b7f46ebe..4d5566ab9379 100644
--- a/NetworkPkg/TcpDxe/TcpMain.h
+++ b/NetworkPkg/TcpDxe/TcpMain.h
@@ -3,7 +3,7 @@
It is the common head file for all Tcp*.c in TCP driver. Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>-+ Copyright (c) Microsoft Corporation SPDX-License-Identifier: BSD-2-Clause-Patent **/@@ -13,6 +13,7 @@
#include <Protocol/ServiceBinding.h> #include <Protocol/DriverBinding.h>+#include <Protocol/Hash2.h> #include <Library/IpIoLib.h> #include <Library/DevicePathLib.h> #include <Library/PrintLib.h>@@ -31,7 +32,7 @@ extern EFI_UNICODE_STRING_TABLE *gTcpControllerNameTable;
extern LIST_ENTRY mTcpRunQue; extern LIST_ENTRY mTcpListenQue;-extern TCP_SEQNO mTcpGlobalIss;+extern TCP_SEQNO mTcpGlobalSecret; extern UINT32 mTcpTick; ///@@ -45,14 +46,6 @@ extern UINT32 mTcpTick;
#define TCP_EXPIRE_TIME 65535 -///-/// The implementation selects the initial send sequence number and the unit to-/// be added when it is increased.-///-#define TCP_BASE_ISS 0x4d7e980b-#define TCP_ISS_INCREMENT_1 2048-#define TCP_ISS_INCREMENT_2 100- typedef union { EFI_TCP4_CONFIG_DATA Tcp4CfgData; EFI_TCP6_CONFIG_DATA Tcp6CfgData;@@ -774,4 +767,50 @@ Tcp6Poll (
IN EFI_TCP6_PROTOCOL *This ); +/**+ Retrieves the Initial Sequence Number (ISN) for a TCP connection identified by local+ and remote IP addresses and ports.++ This method is based on https://datatracker.ietf.org/doc/html/rfc9293#section-3.4.1+ Where the ISN is computed as follows:+ ISN = TimeStamp + MD5(LocalIP, LocalPort, RemoteIP, RemotePort, Secret)++ Otherwise:+ ISN = M + F(localip, localport, remoteip, remoteport, secretkey)++ "Here 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."++ @param[in] LocalIp A pointer to the local IP address of the TCP connection.+ @param[in] LocalIpSize The size, in bytes, of the LocalIp buffer.+ @param[in] LocalPort The local port number of the TCP connection.+ @param[in] RemoteIp A pointer to the remote IP address of the TCP connection.+ @param[in] RemoteIpSize The size, in bytes, of the RemoteIp buffer.+ @param[in] RemotePort The remote port number of the TCP connection.+ @param[out] Isn A pointer to the variable that will receive the Initial+ Sequence Number (ISN).++ @retval EFI_SUCCESS The operation completed successfully, and the ISN was+ retrieved.+ @retval EFI_INVALID_PARAMETER One or more of the input parameters are invalid.+ @retval EFI_UNSUPPORTED The operation is not supported.++**/+EFI_STATUS+TcpGetIsn (+ IN UINT8 *LocalIp,+ IN UINTN LocalIpSize,+ IN UINT16 LocalPort,+ IN UINT8 *RemoteIp,+ IN UINTN RemoteIpSize,+ IN UINT16 RemotePort,+ OUT TCP_SEQNO *Isn+ );+ #endifdiff --git a/NetworkPkg/TcpDxe/TcpDriver.c b/NetworkPkg/TcpDxe/TcpDriver.c
index 8fe6badd687c..40bba4080c87 100644
--- a/NetworkPkg/TcpDxe/TcpDriver.c
+++ b/NetworkPkg/TcpDxe/TcpDriver.c
@@ -83,6 +83,12 @@ EFI_SERVICE_BINDING_PROTOCOL gTcpServiceBinding = {
TcpServiceBindingDestroyChild }; +//+// This is the handle for the Hash2ServiceBinding Protocol instance this driver produces+// if the platform does not provide one.+//+EFI_HANDLE mHash2ServiceHandle = NULL;+ /** Create and start the heartbeat timer for the TCP driver. @@ -165,6 +171,23 @@ TcpDriverEntryPoint (
EFI_STATUS Status; UINT32 Random; + //+ // Initialize the Secret used for hashing TCP sequence numbers+ //+ // Normally this should be regenerated periodically, but since+ // this is only used for UEFI networking and not a general purpose+ // operating system, it is not necessary to regenerate it.+ //+ Status = PseudoRandomU32 (&mTcpGlobalSecret);+ if (EFI_ERROR (Status)) {+ DEBUG ((DEBUG_ERROR, "%a failed to generate random number: %r\n", __func__, Status));+ return Status;+ }++ //+ // Get a random number used to generate a random port number+ // Intentionally not linking this to mTcpGlobalSecret to avoid leaking information about the secret+ // Status = PseudoRandomU32 (&Random); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "%a Failed to generate random number: %r\n", __func__, Status));@@ -207,9 +230,8 @@ TcpDriverEntryPoint (
} //- // Initialize ISS and random port.+ // Initialize the random port. //- mTcpGlobalIss = Random % mTcpGlobalIss; mTcp4RandomPort = (UINT16)(TCP_PORT_KNOWN + (Random % TCP_PORT_KNOWN)); mTcp6RandomPort = mTcp4RandomPort; @@ -224,6 +246,8 @@ TcpDriverEntryPoint (
@param[in] IpVersion IP_VERSION_4 or IP_VERSION_6. @retval EFI_OUT_OF_RESOURCES Failed to allocate some resources.+ @retval EFI_UNSUPPORTED Service Binding Protocols are unavailable.+ @retval EFI_ALREADY_STARTED The TCP driver is already started on the controller. @retval EFI_SUCCESS A new IP6 service binding private was created. **/@@ -234,11 +258,13 @@ TcpCreateService (
IN UINT8 IpVersion ) {- EFI_STATUS Status;- EFI_GUID *IpServiceBindingGuid;- EFI_GUID *TcpServiceBindingGuid;- TCP_SERVICE_DATA *TcpServiceData;- IP_IO_OPEN_DATA OpenData;+ EFI_STATUS Status;+ EFI_GUID *IpServiceBindingGuid;+ EFI_GUID *TcpServiceBindingGuid;+ TCP_SERVICE_DATA *TcpServiceData;+ IP_IO_OPEN_DATA OpenData;+ EFI_SERVICE_BINDING_PROTOCOL *Hash2ServiceBinding;+ EFI_HASH2_PROTOCOL *Hash2Protocol; if (IpVersion == IP_VERSION_4) { IpServiceBindingGuid = &gEfiIp4ServiceBindingProtocolGuid;@@ -272,6 +298,33 @@ TcpCreateService (
return EFI_UNSUPPORTED; } + Status = gBS->LocateProtocol (&gEfiHash2ProtocolGuid, NULL, (VOID **)&Hash2Protocol);+ if (EFI_ERROR (Status)) {+ //+ // If we can't find the Hashing protocol, then we need to create one.+ //++ //+ // Platform is expected to publish the hash service binding protocol to support TCP.+ //+ Status = gBS->LocateProtocol (+ &gEfiHash2ServiceBindingProtocolGuid,+ NULL,+ (VOID **)&Hash2ServiceBinding+ );+ if (EFI_ERROR (Status) || (Hash2ServiceBinding == NULL) || (Hash2ServiceBinding->CreateChild == NULL)) {+ return EFI_UNSUPPORTED;+ }++ //+ // Create an instance of the hash protocol for this controller.+ //+ Status = Hash2ServiceBinding->CreateChild (Hash2ServiceBinding, &mHash2ServiceHandle);+ if (EFI_ERROR (Status)) {+ return EFI_UNSUPPORTED;+ }+ }+ // // Create the TCP service data. //@@ -423,6 +476,7 @@ TcpDestroyService (
EFI_STATUS Status; LIST_ENTRY *List; TCP_DESTROY_CHILD_IN_HANDLE_BUF_CONTEXT Context;+ EFI_SERVICE_BINDING_PROTOCOL *Hash2ServiceBinding; ASSERT ((IpVersion == IP_VERSION_4) || (IpVersion == IP_VERSION_6)); @@ -439,6 +493,30 @@ TcpDestroyService (
return EFI_SUCCESS; } + //+ // Destroy the Hash2ServiceBinding instance if it is created by Tcp driver.+ //+ if (mHash2ServiceHandle != NULL) {+ Status = gBS->LocateProtocol (+ &gEfiHash2ServiceBindingProtocolGuid,+ NULL,+ (VOID **)&Hash2ServiceBinding+ );+ if (EFI_ERROR (Status) || (Hash2ServiceBinding == NULL) || (Hash2ServiceBinding->DestroyChild == NULL)) {+ return EFI_UNSUPPORTED;+ }++ //+ // Destroy the instance of the hashing protocol for this controller.+ //+ Status = Hash2ServiceBinding->DestroyChild (Hash2ServiceBinding, &mHash2ServiceHandle);+ if (EFI_ERROR (Status)) {+ return EFI_UNSUPPORTED;+ }++ mHash2ServiceHandle = NULL;+ }+ Status = gBS->OpenProtocol ( NicHandle, ServiceBindingGuid,diff --git a/NetworkPkg/TcpDxe/TcpInput.c b/NetworkPkg/TcpDxe/TcpInput.c
index 97633a3908be..a5d575ccafeb 100644
--- a/NetworkPkg/TcpDxe/TcpInput.c
+++ b/NetworkPkg/TcpDxe/TcpInput.c
@@ -724,6 +724,7 @@ TcpInput (
TCP_SEQNO Urg; UINT16 Checksum; INT32 Usable;+ EFI_STATUS Status; ASSERT ((Version == IP_VERSION_4) || (Version == IP_VERSION_6)); @@ -872,7 +873,17 @@ TcpInput (
Tcb->LocalEnd.Port = Head->DstPort; Tcb->RemoteEnd.Port = Head->SrcPort; - TcpInitTcbLocal (Tcb);+ Status = TcpInitTcbLocal (Tcb);+ if (EFI_ERROR (Status)) {+ DEBUG (+ (DEBUG_ERROR,+ "TcpInput: discard a segment because failed to init local end for TCB %p\n",+ Tcb)+ );++ goto DISCARD;+ }+ TcpInitTcbPeer (Tcb, Seg, &Option); TcpSetState (Tcb, TCP_SYN_RCVD);diff --git a/NetworkPkg/TcpDxe/TcpMisc.c b/NetworkPkg/TcpDxe/TcpMisc.c
index c93212d47ded..3310306f639c 100644
--- a/NetworkPkg/TcpDxe/TcpMisc.c
+++ b/NetworkPkg/TcpDxe/TcpMisc.c
@@ -3,7 +3,7 @@
(C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR> Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>-+ Copyright (c) Microsoft Corporation SPDX-License-Identifier: BSD-2-Clause-Patent **/@@ -20,7 +20,34 @@ LIST_ENTRY mTcpListenQue = {
&mTcpListenQue }; -TCP_SEQNO mTcpGlobalIss = TCP_BASE_ISS;+//+// The Session secret+// This must be initialized to a random value at boot time+//+TCP_SEQNO mTcpGlobalSecret;++//+// Union to hold either an IPv4 or IPv6 address+// This is used to simplify the ISN hash computation+//+typedef union {+ UINT8 IPv4[4];+ UINT8 IPv6[16];+} NETWORK_ADDRESS;++//+// The ISN is computed by hashing this structure+// It is initialized with the local and remote IP addresses and ports+// and the secret+//+//+typedef struct {+ UINT16 LocalPort;+ UINT16 RemotePort;+ NETWORK_ADDRESS LocalAddress;+ NETWORK_ADDRESS RemoteAddress;+ TCP_SEQNO Secret;+} ISN_HASH_CTX; CHAR16 *mTcpStateName[] = { L"TCP_CLOSED",@@ -41,12 +68,18 @@ CHAR16 *mTcpStateName[] = {
@param[in, out] Tcb Pointer to the TCP_CB of this TCP instance. + @retval EFI_SUCCESS The operation completed successfully+ @retval others The underlying functions failed and could not complete the operation+ **/-VOID+EFI_STATUS TcpInitTcbLocal ( IN OUT TCP_CB *Tcb ) {+ TCP_SEQNO Isn;+ EFI_STATUS Status;+ // // Compute the checksum of the fixed parts of pseudo header //@@ -57,6 +90,16 @@ TcpInitTcbLocal (
0x06, 0 );++ Status = TcpGetIsn (+ Tcb->LocalEnd.Ip.v4.Addr,+ sizeof (IPv4_ADDRESS),+ Tcb->LocalEnd.Port,+ Tcb->RemoteEnd.Ip.v4.Addr,+ sizeof (IPv4_ADDRESS),+ Tcb->RemoteEnd.Port,+ &Isn+ ); } else { Tcb->HeadSum = NetIp6PseudoHeadChecksum ( &Tcb->LocalEnd.Ip.v6,@@ -64,9 +107,25 @@ TcpInitTcbLocal (
0x06, 0 );++ Status = TcpGetIsn (+ Tcb->LocalEnd.Ip.v6.Addr,+ sizeof (IPv6_ADDRESS),+ Tcb->LocalEnd.Port,+ Tcb->RemoteEnd.Ip.v6.Addr,+ sizeof (IPv6_ADDRESS),+ Tcb->RemoteEnd.Port,+ &Isn+ );+ }++ if (EFI_ERROR (Status)) {+ DEBUG ((DEBUG_ERROR, "TcpInitTcbLocal: failed to get isn\n"));+ ASSERT (FALSE);+ return Status; } - Tcb->Iss = TcpGetIss ();+ Tcb->Iss = Isn; Tcb->SndUna = Tcb->Iss; Tcb->SndNxt = Tcb->Iss; @@ -82,6 +141,8 @@ TcpInitTcbLocal (
Tcb->RetxmitSeqMax = 0; Tcb->ProbeTimerOn = FALSE;++ return EFI_SUCCESS; } /**@@ -506,18 +567,162 @@ TcpCloneTcb (
} /**- Compute an ISS to be used by a new connection.+ Retrieves the Initial Sequence Number (ISN) for a TCP connection identified by local+ and remote IP addresses and ports. - @return The resulting ISS.+ This method is based on https://datatracker.ietf.org/doc/html/rfc9293#section-3.4.1+ Where the ISN is computed as follows:+ ISN = TimeStamp + MD5(LocalIP, LocalPort, RemoteIP, RemotePort, Secret)++ Otherwise:+ ISN = M + F(localip, localport, remoteip, remoteport, secretkey)++ "Here 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."++ @param[in] LocalIp A pointer to the local IP address of the TCP connection.+ @param[in] LocalIpSize The size, in bytes, of the LocalIp buffer.+ @param[in] LocalPort The local port number of the TCP connection.+ @param[in] RemoteIp A pointer to the remote IP address of the TCP connection.+ @param[in] RemoteIpSize The size, in bytes, of the RemoteIp buffer.+ @param[in] RemotePort The remote port number of the TCP connection.+ @param[out] Isn A pointer to the variable that will receive the Initial+ Sequence Number (ISN).++ @retval EFI_SUCCESS The operation completed successfully, and the ISN was+ retrieved.+ @retval EFI_INVALID_PARAMETER One or more of the input parameters are invalid.+ @retval EFI_UNSUPPORTED The operation is not supported. **/-TCP_SEQNO-TcpGetIss (- VOID+EFI_STATUS+TcpGetIsn (+ IN UINT8 *LocalIp,+ IN UINTN LocalIpSize,+ IN UINT16 LocalPort,+ IN UINT8 *RemoteIp,+ IN UINTN RemoteIpSize,+ IN UINT16 RemotePort,+ OUT TCP_SEQNO *Isn ) {- mTcpGlobalIss += TCP_ISS_INCREMENT_1;- return mTcpGlobalIss;+ EFI_STATUS Status;+ EFI_HASH2_PROTOCOL *Hash2Protocol;+ EFI_HASH2_OUTPUT HashResult;+ ISN_HASH_CTX IsnHashCtx;+ EFI_TIME TimeStamp;++ //+ // Check that the ISN pointer is valid+ //+ if (Isn == NULL) {+ return EFI_INVALID_PARAMETER;+ }++ //+ // The local ip may be a v4 or v6 address and may not be NULL+ //+ if ((LocalIp == NULL) || (LocalIpSize == 0) || (RemoteIp == NULL) || (RemoteIpSize == 0)) {+ return EFI_INVALID_PARAMETER;+ }++ //+ // the local ip may be a v4 or v6 address+ //+ if ((LocalIpSize != sizeof (EFI_IPv4_ADDRESS)) && (LocalIpSize != sizeof (EFI_IPv6_ADDRESS))) {+ return EFI_INVALID_PARAMETER;+ }++ //+ // Locate the Hash Protocol+ //+ Status = gBS->LocateProtocol (&gEfiHash2ProtocolGuid, NULL, (VOID **)&Hash2Protocol);+ if (EFI_ERROR (Status)) {+ DEBUG ((DEBUG_NET, "Failed to locate Hash Protocol: %r\n", Status));++ //+ // TcpCreateService(..) is expected to be called prior to this function+ //+ ASSERT_EFI_ERROR (Status);+ return Status;+ }++ //+ // Initialize the hash algorithm+ //+ Status = Hash2Protocol->HashInit (Hash2Protocol, &gEfiHashAlgorithmSha256Guid);+ if (EFI_ERROR (Status)) {+ DEBUG ((DEBUG_NET, "Failed to initialize sha256 hash algorithm: %r\n", Status));+ return Status;+ }++ IsnHashCtx.LocalPort = LocalPort;+ IsnHashCtx.RemotePort = RemotePort;+ IsnHashCtx.Secret = mTcpGlobalSecret;++ //+ // Check the IP address family and copy accordingly+ //+ if (LocalIpSize == sizeof (EFI_IPv4_ADDRESS)) {+ CopyMem (&IsnHashCtx.LocalAddress.IPv4, LocalIp, LocalIpSize);+ } else if (LocalIpSize == sizeof (EFI_IPv6_ADDRESS)) {+ CopyMem (&IsnHashCtx.LocalAddress.IPv6, LocalIp, LocalIpSize);+ } else {+ return EFI_INVALID_PARAMETER; // Unsupported address size+ }++ //+ // Repeat the process for the remote IP address+ //+ if (RemoteIpSize == sizeof (EFI_IPv4_ADDRESS)) {+ CopyMem (&IsnHashCtx.RemoteAddress.IPv4, RemoteIp, RemoteIpSize);+ } else if (RemoteIpSize == sizeof (EFI_IPv6_ADDRESS)) {+ CopyMem (&IsnHashCtx.RemoteAddress.IPv6, RemoteIp, RemoteIpSize);+ } else {+ return EFI_INVALID_PARAMETER; // Unsupported address size+ }++ //+ // Compute the hash+ // Update the hash with the data+ //+ Status = Hash2Protocol->HashUpdate (Hash2Protocol, (UINT8 *)&IsnHashCtx, sizeof (IsnHashCtx));+ if (EFI_ERROR (Status)) {+ DEBUG ((DEBUG_NET, "Failed to update hash: %r\n", Status));+ return Status;+ }++ //+ // Finalize the hash and retrieve the result+ //+ Status = Hash2Protocol->HashFinal (Hash2Protocol, &HashResult);+ if (EFI_ERROR (Status)) {+ DEBUG ((DEBUG_NET, "Failed to finalize hash: %r\n", Status));+ return Status;+ }++ Status = gRT->GetTime (&TimeStamp, NULL);+ if (EFI_ERROR (Status)) {+ return Status;+ }++ //+ // copy the first 4 bytes of the hash result into the ISN+ //+ CopyMem (Isn, HashResult.Md5Hash, sizeof (*Isn));++ //+ // now add the timestamp to the ISN as 4 microseconds units (1000 / 4 = 250)+ //+ *Isn += (TCP_SEQNO)TimeStamp.Nanosecond * 250;++ return Status; } /**@@ -721,17 +926,28 @@ TcpFormatNetbuf (
@param[in, out] Tcb Pointer to the TCP_CB that wants to initiate a connection. + @retval EFI_SUCCESS The operation completed successfully+ @retval others The underlying functions failed and could not complete the operation+ **/-VOID+EFI_STATUS TcpOnAppConnect ( IN OUT TCP_CB *Tcb ) {- TcpInitTcbLocal (Tcb);+ EFI_STATUS Status;++ Status = TcpInitTcbLocal (Tcb);+ if (EFI_ERROR (Status)) {+ return Status;+ }+ TcpSetState (Tcb, TCP_SYN_SENT); TcpSetTimer (Tcb, TCP_TIMER_CONNECT, Tcb->ConnectTimeout); TcpToSendData (Tcb, 1);++ return EFI_SUCCESS; } /**diff --git a/NetworkPkg/TcpDxe/TcpTimer.c b/NetworkPkg/TcpDxe/TcpTimer.c
index 5d2e124977d9..065b1bdf5feb 100644
--- a/NetworkPkg/TcpDxe/TcpTimer.c
+++ b/NetworkPkg/TcpDxe/TcpTimer.c
@@ -2,7 +2,7 @@
TCP timer related functions. Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.<BR>-+ Copyright (c) Microsoft Corporation SPDX-License-Identifier: BSD-2-Clause-Patent **/@@ -483,7 +483,6 @@ TcpTickingDpc (
INT16 Index; mTcpTick++;- mTcpGlobalIss += TCP_ISS_INCREMENT_2; // // Don't use LIST_FOR_EACH, which isn't delete safe.diff --git a/NetworkPkg/SecurityFixes.yaml b/NetworkPkg/SecurityFixes.yaml
index 20a4555019d9..4305328425d0 100644
--- a/NetworkPkg/SecurityFixes.yaml
+++ b/NetworkPkg/SecurityFixes.yaml
@@ -122,6 +122,28 @@ CVE_2023_45235:
- http://www.openwall.com/lists/oss-security/2024/01/16/2 - http://packetstormsecurity.com/files/176574/PixieFail-Proof-Of-Concepts.html - https://blog.quarkslab.com/pixiefail-nine-vulnerabilities-in-tianocores-edk-ii-ipv6-network-stack.html+CVE_2023_45236:+ commit_titles:+ - "NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236 Patch"+ cve: CVE-2023-45236+ date_reported: 2023-08-28 13:56 UTC+ description: "Bug 08 - edk2/NetworkPkg: Predictable TCP Initial Sequence Numbers"+ note:+ files_impacted:+ - NetworkPkg/Include/Library/NetLib.h+ - NetworkPkg/TcpDxe/TcpDriver.c+ - NetworkPkg/TcpDxe/TcpDxe.inf+ - NetworkPkg/TcpDxe/TcpFunc.h+ - NetworkPkg/TcpDxe/TcpInput.c+ - NetworkPkg/TcpDxe/TcpMain.h+ - NetworkPkg/TcpDxe/TcpMisc.c+ - NetworkPkg/TcpDxe/TcpTimer.c+ links:+ - https://bugzilla.tianocore.org/show_bug.cgi?id=4541+ - https://nvd.nist.gov/vuln/detail/CVE-2023-45236+ - http://www.openwall.com/lists/oss-security/2024/01/16/2+ - http://packetstormsecurity.com/files/176574/PixieFail-Proof-Of-Concepts.html+ - https://blog.quarkslab.com/pixiefail-nine-vulnerabilities-in-tianocores-edk-ii-ipv6-network-stack.html CVE_2023_45237: commit_titles: - "NetworkPkg:: SECURITY PATCH CVE 2023-45237"--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118930): https://edk2.groups.io/g/devel/message/118930
Mute This Topic: https://groups.io/mt/105996585/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2024-05-15 21:38 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-09 5:56 [edk2-devel] [PATCH v2 00/13] NetworkPkg: CVE-2023-45236 and CVE-2023-45237 Doug Flick via groups.io
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 01/13] EmulatorPkg: : Add RngDxe to EmulatorPkg Doug Flick via groups.io
2024-05-10 3:10 ` Ni, Ray
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 02/13] EmulatorPkg: : Add Hash2DxeCrypto " Doug Flick via groups.io
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 03/13] OvmfPkg:PlatformCI: Support virtio-rng-pci Doug Flick via groups.io
2024-05-09 8:45 ` Ard Biesheuvel
2024-05-09 8:45 ` Ard Biesheuvel
2024-05-09 18:21 ` Doug Flick via groups.io
2024-05-10 0:54 ` 回复: " gaoliming via groups.io
2024-05-10 17:13 ` [edk2-devel] " Doug Flick via groups.io
2024-05-11 8:40 ` Ard Biesheuvel
2024-05-13 9:22 ` Gerd Hoffmann
2024-05-13 17:24 ` Ard Biesheuvel
2024-05-17 3:27 ` Doug Flick via groups.io
2024-05-17 7:27 ` Ard Biesheuvel
2024-05-17 9:48 ` Gerd Hoffmann
2024-05-24 3:02 ` 回复: " gaoliming via groups.io
2024-05-14 19:55 ` Pedro Falcato
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 04/13] OvmfPkg: : Add Hash2DxeCrypto to OvmfPkg Doug Flick via groups.io
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 05/13] ArmVirtPkg:PlatformCI: Support virtio-rng-pci Doug Flick via groups.io
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 06/13] ArmVirtPkg: : Add Hash2DxeCrypto to ArmVirtPkg Doug Flick via groups.io
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 07/13] SecurityPkg: RngDxe: Remove incorrect limitation on GetRng Doug Flick via groups.io
2024-05-10 10:23 ` Yao, Jiewen
2024-05-10 21:12 ` Doug Flick via groups.io
2024-05-11 0:24 ` Yao, Jiewen
2024-05-13 15:53 ` PierreGondois
2024-05-11 8:26 ` Ard Biesheuvel
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 08/13] NetworkPkg:: SECURITY PATCH CVE-2023-45237 Doug Flick via groups.io
2024-05-13 14:30 ` Ard Biesheuvel
2024-05-15 19:14 ` Saloni Kasbekar
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236 Doug Flick via groups.io
2024-05-15 21:38 ` Saloni Kasbekar [this message]
2024-05-21 19:28 ` Doug Flick via groups.io
2024-05-24 1:24 ` 回复: " gaoliming via groups.io
2024-05-24 4:23 ` Saloni Kasbekar
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 10/13] MdePkg: : Add MockUefiBootServicesTableLib Doug Flick via groups.io
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 11/13] MdePkg: : Adds Protocol for MockRng Doug Flick via groups.io
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 12/13] MdePkg: Add MockHash2 Protocol for testing Doug Flick via groups.io
2024-05-09 5:56 ` [edk2-devel] [PATCH v2 13/13] NetworkPkg: Update the PxeBcDhcp6GoogleTest due to underlying changes Doug Flick via groups.io
2024-05-24 4:24 ` Saloni Kasbekar
2024-05-09 9:40 ` 回复: [edk2-devel][edk2-stable202405] [PATCH v2 00/13] NetworkPkg: CVE-2023-45236 and CVE-2023-45237 gaoliming via groups.io
2024-05-09 18:26 ` [edk2-devel] " Doug Flick via groups.io
2024-05-15 0:41 ` 回复: " gaoliming via groups.io
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SN7PR11MB82817429EEC59CA8E438DF20F1EC2@SN7PR11MB8281.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox