Skip to content

Commit

Permalink
NetworkPkg TcpDxe: SECURITY PATCH CVE-2023-45236
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Cc: Zachary Clark-williams <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Saloni Kasbekar <[email protected]>
  • Loading branch information
@Flickdm @mergify
Flickdm authored and mergify[bot] committed May 24, 2024
1 parent 4c4ceb2 commit 1904a64
Show file tree
Hide file tree
Showing 8 changed files with 415 additions and 49 deletions.
22 changes: 22 additions & 0 deletions NetworkPkg/SecurityFixes.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
92 changes: 85 additions & 7 deletions NetworkPkg/TcpDxe/TcpDriver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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;

Expand All @@ -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.
**/
Expand All @@ -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;
Expand Down Expand Up @@ -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.
//
Expand Down Expand Up @@ -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));

Expand All @@ -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,
Expand Down
8 changes: 7 additions & 1 deletion NetworkPkg/TcpDxe/TcpDxe.inf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down Expand Up @@ -68,7 +69,6 @@
NetLib
IpIoLib


[Protocols]
## SOMETIMES_CONSUMES
## SOMETIMES_PRODUCES
Expand All @@ -81,6 +81,12 @@
gEfiIp6ServiceBindingProtocolGuid ## TO_START
gEfiTcp6ProtocolGuid ## BY_START
gEfiTcp6ServiceBindingProtocolGuid ## BY_START
gEfiHash2ProtocolGuid ## BY_START
gEfiHash2ServiceBindingProtocolGuid ## BY_START

[Guids]
gEfiHashAlgorithmMD5Guid ## CONSUMES
gEfiHashAlgorithmSha256Guid ## CONSUMES

[Depex]
gEfiHash2ServiceBindingProtocolGuid
Expand Down
23 changes: 9 additions & 14 deletions NetworkPkg/TcpDxe/TcpFunc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
**/
Expand Down Expand Up @@ -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
);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
);
Expand Down
13 changes: 12 additions & 1 deletion NetworkPkg/TcpDxe/TcpInput.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,7 @@ TcpInput (
TCP_SEQNO Urg;
UINT16 Checksum;
INT32 Usable;
EFI_STATUS Status;

ASSERT ((Version == IP_VERSION_4) || (Version == IP_VERSION_6));

Expand Down Expand Up @@ -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);
Expand Down
59 changes: 49 additions & 10 deletions NetworkPkg/TcpDxe/TcpMain.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
**/
Expand All @@ -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>
Expand All @@ -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;

///
Expand All @@ -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;
Expand Down Expand Up @@ -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
);

#endif
Loading

0 comments on commit 1904a64

Please sign in to comment.