From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id DFCB5D80047 for ; Fri, 24 May 2024 05:45:29 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=4wjO1JBuF3D/KH5XN1ttWDZbb25JsHu0s3E633RcXnM=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20240206; t=1716529528; v=1; b=kLLWK87lDbAF/14Arn1o4qh1tNxcy4Ucac9SgnLPg4i0KGrI4Zy5U9uaCMnKva9PJv+QmB4X t+Fc8sYzlRTK2Vgrxl6kNPw/5rHy+ZmYKLlW7rfgLTJdzKu7N1iBIixi21hMls+a+XfjGjbeN9b iFNh+zSZsaTPVend6YBuZ2imnURhHuXEuRb+kRWziVq84UMwLiRgmxw4m1Hn6iNjOi+ZXxlPcKC Hnk3//M8B5TK3us9HA7ID+MsmtT73hOglGKjTJFuioOyvONEcqeyiNtB8+1Yw8y21J9ncfo5y7a qUhziGIyA7yyIW6jq7FJiX1O/BZ9YjfBTbLS7K+ADaMJQ== X-Received: by 127.0.0.2 with SMTP id Ad26YY7687511xk7yccoei7O; Thu, 23 May 2024 22:45:28 -0700 X-Received: from mail-oa1-f54.google.com (mail-oa1-f54.google.com [209.85.160.54]) by mx.groups.io with SMTP id smtpd.web11.9353.1716529526133372161 for ; Thu, 23 May 2024 22:45:26 -0700 X-Received: by mail-oa1-f54.google.com with SMTP id 586e51a60fabf-24c0dbd2866so2466445fac.0 for ; Thu, 23 May 2024 22:45:26 -0700 (PDT) X-Gm-Message-State: GkiHPJljpmFRE4Up9sv9ow3Ox7686176AA= X-Google-Smtp-Source: AGHT+IGgQ4VUyBvJHx94jzfdA6xxUk9gvMwACRiQF6dJaElOnSJJzS1kFOkTGEmOPOxJmtejiNUs7A== X-Received: by 2002:a05:6871:419a:b0:22e:8ca0:36ba with SMTP id 586e51a60fabf-24ca11f4325mr1372809fac.22.1716529524828; Thu, 23 May 2024 22:45:24 -0700 (PDT) X-Received: from localhost.localdomain (c-67-160-15-86.hsd1.wa.comcast.net. [67.160.15.86]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-6f8fcfe64a4sm471919b3a.158.2024.05.23.22.45.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 May 2024 22:45:24 -0700 (PDT) From: "Doug Flick via groups.io" X-Google-Original-From: Flickdm To: devel@edk2.groups.io Cc: Saloni Kasbekar , Zachary Clark-williams Subject: [edk2-devel] [PATCH v3 09/20] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236 Date: Thu, 23 May 2024 22:45:01 -0700 Message-Id: <20240524054512.523329-10-douglas.flick@microsoft.com> In-Reply-To: <20240524054512.523329-1-douglas.flick@microsoft.com> References: <20240524054512.523329-1-douglas.flick@microsoft.com> MIME-Version: 1.0 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Thu, 23 May 2024 22:45:26 -0700 Resent-From: dougflick@microsoft.com Reply-To: devel@edk2.groups.io,dougflick@microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=kLLWK87l; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=pass (policy=none) header.from=groups.io From: Doug Flick REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4541 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 =3D 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 Cc: Zachary Clark-williams Signed-off-by: Doug Flick [MSFT] --- 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 | 244 ++++++++++++++++++-- NetworkPkg/TcpDxe/TcpTimer.c | 3 +- NetworkPkg/SecurityFixes.yaml | 22 ++ 8 files changed, 415 insertions(+), 49 deletions(-) diff --git a/NetworkPkg/TcpDxe/TcpDxe.inf b/NetworkPkg/TcpDxe/TcpDxe.inf index cf5423f4c5..76de4cf9ec 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 IPv= 6 network stack.=0D #=0D # Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
= =0D +# Copyright (c) Microsoft Corporation=0D #=0D # SPDX-License-Identifier: BSD-2-Clause-Patent=0D #=0D @@ -68,7 +69,6 @@ NetLib=0D IpIoLib=0D =0D -=0D [Protocols]=0D ## SOMETIMES_CONSUMES=0D ## SOMETIMES_PRODUCES=0D @@ -81,6 +81,12 @@ gEfiIp6ServiceBindingProtocolGuid ## TO_START=0D gEfiTcp6ProtocolGuid ## BY_START=0D gEfiTcp6ServiceBindingProtocolGuid ## BY_START=0D + gEfiHash2ProtocolGuid ## BY_START=0D + gEfiHash2ServiceBindingProtocolGuid ## BY_START=0D +=0D +[Guids]=0D + gEfiHashAlgorithmMD5Guid ## CONSUMES=0D + gEfiHashAlgorithmSha256Guid ## CONSUMES=0D =0D [Depex]=0D gEfiHash2ServiceBindingProtocolGuid=0D diff --git a/NetworkPkg/TcpDxe/TcpFunc.h b/NetworkPkg/TcpDxe/TcpFunc.h index a7af01fff2..c707bee3e5 100644 --- a/NetworkPkg/TcpDxe/TcpFunc.h +++ b/NetworkPkg/TcpDxe/TcpFunc.h @@ -2,7 +2,7 @@ Declaration of external functions shared in TCP driver.=0D =0D Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
=0D -=0D + Copyright (c) Microsoft Corporation=0D SPDX-License-Identifier: BSD-2-Clause-Patent=0D =0D **/=0D @@ -36,8 +36,11 @@ VOID =0D @param[in, out] Tcb Pointer to the TCP_CB of this TCP ins= tance.=0D =0D + @retval EFI_SUCCESS The operation completed successfully=0D + @retval others The underlying functions failed and coul= d not complete the operation=0D +=0D **/=0D -VOID=0D +EFI_STATUS=0D TcpInitTcbLocal (=0D IN OUT TCP_CB *Tcb=0D );=0D @@ -128,17 +131,6 @@ TcpCloneTcb ( IN TCP_CB *Tcb=0D );=0D =0D -/**=0D - Compute an ISS to be used by a new connection.=0D -=0D - @return The result ISS.=0D -=0D -**/=0D -TCP_SEQNO=0D -TcpGetIss (=0D - VOID=0D - );=0D -=0D /**=0D Get the local mss.=0D =0D @@ -202,8 +194,11 @@ TcpFormatNetbuf ( @param[in, out] Tcb Pointer to the TCP_CB that wants to initia= te a=0D connection.=0D =0D + @retval EFI_SUCCESS The operation completed successfully=0D + @retval others The underlying functions failed and coul= d not complete the operation=0D +=0D **/=0D -VOID=0D +EFI_STATUS=0D TcpOnAppConnect (=0D IN OUT TCP_CB *Tcb=0D );=0D diff --git a/NetworkPkg/TcpDxe/TcpMain.h b/NetworkPkg/TcpDxe/TcpMain.h index c0c9b7f46e..4d5566ab93 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.=0D =0D Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
=0D -=0D + Copyright (c) Microsoft Corporation=0D SPDX-License-Identifier: BSD-2-Clause-Patent=0D =0D **/=0D @@ -13,6 +13,7 @@ =0D #include =0D #include =0D +#include =0D #include =0D #include =0D #include =0D @@ -31,7 +32,7 @@ extern EFI_UNICODE_STRING_TABLE *gTcpControllerNameT= able; =0D extern LIST_ENTRY mTcpRunQue;=0D extern LIST_ENTRY mTcpListenQue;=0D -extern TCP_SEQNO mTcpGlobalIss;=0D +extern TCP_SEQNO mTcpGlobalSecret;=0D extern UINT32 mTcpTick;=0D =0D ///=0D @@ -45,14 +46,6 @@ extern UINT32 mTcpTick; =0D #define TCP_EXPIRE_TIME 65535=0D =0D -///=0D -/// The implementation selects the initial send sequence number and the un= it to=0D -/// be added when it is increased.=0D -///=0D -#define TCP_BASE_ISS 0x4d7e980b=0D -#define TCP_ISS_INCREMENT_1 2048=0D -#define TCP_ISS_INCREMENT_2 100=0D -=0D typedef union {=0D EFI_TCP4_CONFIG_DATA Tcp4CfgData;=0D EFI_TCP6_CONFIG_DATA Tcp6CfgData;=0D @@ -774,4 +767,50 @@ Tcp6Poll ( IN EFI_TCP6_PROTOCOL *This=0D );=0D =0D +/**=0D + Retrieves the Initial Sequence Number (ISN) for a TCP connection identif= ied by local=0D + and remote IP addresses and ports.=0D +=0D + This method is based on https://datatracker.ietf.org/doc/html/rfc9293#se= ction-3.4.1=0D + Where the ISN is computed as follows:=0D + ISN =3D TimeStamp + MD5(LocalIP, LocalPort, RemoteIP, RemotePort, Secr= et)=0D +=0D + Otherwise:=0D + ISN =3D M + F(localip, localport, remoteip, remoteport, secretkey)=0D +=0D + "Here M is the 4 microsecond timer, and F() is a pseudorandom function= (PRF) of the=0D + connection's identifying parameters ("localip, localport, remoteip, re= moteport")=0D + and a secret key ("secretkey") (SHLD-1). F() MUST NOT be computable fr= om the=0D + outside (MUST-9), or an attacker could still guess at sequence numbers= from the=0D + ISN used for some other connection. The PRF could be implemented as a= =0D + cryptographic hash of the concatenation of the TCP connection paramete= rs and some=0D + secret data. For discussion of the selection of a specific hash algori= thm and=0D + management of the secret key data."=0D +=0D + @param[in] LocalIp A pointer to the local IP address of the= TCP connection.=0D + @param[in] LocalIpSize The size, in bytes, of the LocalIp buffe= r.=0D + @param[in] LocalPort The local port number of the TCP connect= ion.=0D + @param[in] RemoteIp A pointer to the remote IP address of th= e TCP connection.=0D + @param[in] RemoteIpSize The size, in bytes, of the RemoteIp buff= er.=0D + @param[in] RemotePort The remote port number of the TCP connec= tion.=0D + @param[out] Isn A pointer to the variable that will rece= ive the Initial=0D + Sequence Number (ISN).=0D +=0D + @retval EFI_SUCCESS The operation completed successfully, an= d the ISN was=0D + retrieved.=0D + @retval EFI_INVALID_PARAMETER One or more of the input parameters are = invalid.=0D + @retval EFI_UNSUPPORTED The operation is not supported.=0D +=0D +**/=0D +EFI_STATUS=0D +TcpGetIsn (=0D + IN UINT8 *LocalIp,=0D + IN UINTN LocalIpSize,=0D + IN UINT16 LocalPort,=0D + IN UINT8 *RemoteIp,=0D + IN UINTN RemoteIpSize,=0D + IN UINT16 RemotePort,=0D + OUT TCP_SEQNO *Isn=0D + );=0D +=0D #endif=0D diff --git a/NetworkPkg/TcpDxe/TcpDriver.c b/NetworkPkg/TcpDxe/TcpDriver.c index 8fe6badd68..40bba4080c 100644 --- a/NetworkPkg/TcpDxe/TcpDriver.c +++ b/NetworkPkg/TcpDxe/TcpDriver.c @@ -83,6 +83,12 @@ EFI_SERVICE_BINDING_PROTOCOL gTcpServiceBinding =3D { TcpServiceBindingDestroyChild=0D };=0D =0D +//=0D +// This is the handle for the Hash2ServiceBinding Protocol instance this d= river produces=0D +// if the platform does not provide one.=0D +//=0D +EFI_HANDLE mHash2ServiceHandle =3D NULL;=0D +=0D /**=0D Create and start the heartbeat timer for the TCP driver.=0D =0D @@ -165,6 +171,23 @@ TcpDriverEntryPoint ( EFI_STATUS Status;=0D UINT32 Random;=0D =0D + //=0D + // Initialize the Secret used for hashing TCP sequence numbers=0D + //=0D + // Normally this should be regenerated periodically, but since=0D + // this is only used for UEFI networking and not a general purpose=0D + // operating system, it is not necessary to regenerate it.=0D + //=0D + Status =3D PseudoRandomU32 (&mTcpGlobalSecret);=0D + if (EFI_ERROR (Status)) {=0D + DEBUG ((DEBUG_ERROR, "%a failed to generate random number: %r\n", __fu= nc__, Status));=0D + return Status;=0D + }=0D +=0D + //=0D + // Get a random number used to generate a random port number=0D + // Intentionally not linking this to mTcpGlobalSecret to avoid leaking i= nformation about the secret=0D + //=0D Status =3D PseudoRandomU32 (&Random);=0D if (EFI_ERROR (Status)) {=0D DEBUG ((DEBUG_ERROR, "%a Failed to generate random number: %r\n", __fu= nc__, Status));=0D @@ -207,9 +230,8 @@ TcpDriverEntryPoint ( }=0D =0D //=0D - // Initialize ISS and random port.=0D + // Initialize the random port.=0D //=0D - mTcpGlobalIss =3D Random % mTcpGlobalIss;=0D mTcp4RandomPort =3D (UINT16)(TCP_PORT_KNOWN + (Random % TCP_PORT_KNOWN))= ;=0D mTcp6RandomPort =3D mTcp4RandomPort;=0D =0D @@ -224,6 +246,8 @@ TcpDriverEntryPoint ( @param[in] IpVersion IP_VERSION_4 or IP_VERSION_6.=0D =0D @retval EFI_OUT_OF_RESOURCES Failed to allocate some resources.=0D + @retval EFI_UNSUPPORTED Service Binding Protocols are unavailable= .=0D + @retval EFI_ALREADY_STARTED The TCP driver is already started on the = controller.=0D @retval EFI_SUCCESS A new IP6 service binding private was cre= ated.=0D =0D **/=0D @@ -234,11 +258,13 @@ TcpCreateService ( IN UINT8 IpVersion=0D )=0D {=0D - EFI_STATUS Status;=0D - EFI_GUID *IpServiceBindingGuid;=0D - EFI_GUID *TcpServiceBindingGuid;=0D - TCP_SERVICE_DATA *TcpServiceData;=0D - IP_IO_OPEN_DATA OpenData;=0D + EFI_STATUS Status;=0D + EFI_GUID *IpServiceBindingGuid;=0D + EFI_GUID *TcpServiceBindingGuid;=0D + TCP_SERVICE_DATA *TcpServiceData;=0D + IP_IO_OPEN_DATA OpenData;=0D + EFI_SERVICE_BINDING_PROTOCOL *Hash2ServiceBinding;=0D + EFI_HASH2_PROTOCOL *Hash2Protocol;=0D =0D if (IpVersion =3D=3D IP_VERSION_4) {=0D IpServiceBindingGuid =3D &gEfiIp4ServiceBindingProtocolGuid;=0D @@ -272,6 +298,33 @@ TcpCreateService ( return EFI_UNSUPPORTED;=0D }=0D =0D + Status =3D gBS->LocateProtocol (&gEfiHash2ProtocolGuid, NULL, (VOID **)&= Hash2Protocol);=0D + if (EFI_ERROR (Status)) {=0D + //=0D + // If we can't find the Hashing protocol, then we need to create one.= =0D + //=0D +=0D + //=0D + // Platform is expected to publish the hash service binding protocol t= o support TCP.=0D + //=0D + Status =3D gBS->LocateProtocol (=0D + &gEfiHash2ServiceBindingProtocolGuid,=0D + NULL,=0D + (VOID **)&Hash2ServiceBinding=0D + );=0D + if (EFI_ERROR (Status) || (Hash2ServiceBinding =3D=3D NULL) || (Hash2S= erviceBinding->CreateChild =3D=3D NULL)) {=0D + return EFI_UNSUPPORTED;=0D + }=0D +=0D + //=0D + // Create an instance of the hash protocol for this controller.=0D + //=0D + Status =3D Hash2ServiceBinding->CreateChild (Hash2ServiceBinding, &mHa= sh2ServiceHandle);=0D + if (EFI_ERROR (Status)) {=0D + return EFI_UNSUPPORTED;=0D + }=0D + }=0D +=0D //=0D // Create the TCP service data.=0D //=0D @@ -423,6 +476,7 @@ TcpDestroyService ( EFI_STATUS Status;=0D LIST_ENTRY *List;=0D TCP_DESTROY_CHILD_IN_HANDLE_BUF_CONTEXT Context;=0D + EFI_SERVICE_BINDING_PROTOCOL *Hash2ServiceBinding;=0D =0D ASSERT ((IpVersion =3D=3D IP_VERSION_4) || (IpVersion =3D=3D IP_VERSION_= 6));=0D =0D @@ -439,6 +493,30 @@ TcpDestroyService ( return EFI_SUCCESS;=0D }=0D =0D + //=0D + // Destroy the Hash2ServiceBinding instance if it is created by Tcp driv= er.=0D + //=0D + if (mHash2ServiceHandle !=3D NULL) {=0D + Status =3D gBS->LocateProtocol (=0D + &gEfiHash2ServiceBindingProtocolGuid,=0D + NULL,=0D + (VOID **)&Hash2ServiceBinding=0D + );=0D + if (EFI_ERROR (Status) || (Hash2ServiceBinding =3D=3D NULL) || (Hash2S= erviceBinding->DestroyChild =3D=3D NULL)) {=0D + return EFI_UNSUPPORTED;=0D + }=0D +=0D + //=0D + // Destroy the instance of the hashing protocol for this controller.=0D + //=0D + Status =3D Hash2ServiceBinding->DestroyChild (Hash2ServiceBinding, &mH= ash2ServiceHandle);=0D + if (EFI_ERROR (Status)) {=0D + return EFI_UNSUPPORTED;=0D + }=0D +=0D + mHash2ServiceHandle =3D NULL;=0D + }=0D +=0D Status =3D gBS->OpenProtocol (=0D NicHandle,=0D ServiceBindingGuid,=0D diff --git a/NetworkPkg/TcpDxe/TcpInput.c b/NetworkPkg/TcpDxe/TcpInput.c index 97633a3908..a5d575ccaf 100644 --- a/NetworkPkg/TcpDxe/TcpInput.c +++ b/NetworkPkg/TcpDxe/TcpInput.c @@ -724,6 +724,7 @@ TcpInput ( TCP_SEQNO Urg;=0D UINT16 Checksum;=0D INT32 Usable;=0D + EFI_STATUS Status;=0D =0D ASSERT ((Version =3D=3D IP_VERSION_4) || (Version =3D=3D IP_VERSION_6));= =0D =0D @@ -872,7 +873,17 @@ TcpInput ( Tcb->LocalEnd.Port =3D Head->DstPort;=0D Tcb->RemoteEnd.Port =3D Head->SrcPort;=0D =0D - TcpInitTcbLocal (Tcb);=0D + Status =3D TcpInitTcbLocal (Tcb);=0D + if (EFI_ERROR (Status)) {=0D + DEBUG (=0D + (DEBUG_ERROR,=0D + "TcpInput: discard a segment because failed to init local end f= or TCB %p\n",=0D + Tcb)=0D + );=0D +=0D + goto DISCARD;=0D + }=0D +=0D TcpInitTcbPeer (Tcb, Seg, &Option);=0D =0D TcpSetState (Tcb, TCP_SYN_RCVD);=0D diff --git a/NetworkPkg/TcpDxe/TcpMisc.c b/NetworkPkg/TcpDxe/TcpMisc.c index c93212d47d..3310306f63 100644 --- a/NetworkPkg/TcpDxe/TcpMisc.c +++ b/NetworkPkg/TcpDxe/TcpMisc.c @@ -3,7 +3,7 @@ =0D (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
=0D Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
=0D -=0D + Copyright (c) Microsoft Corporation=0D SPDX-License-Identifier: BSD-2-Clause-Patent=0D =0D **/=0D @@ -20,7 +20,34 @@ LIST_ENTRY mTcpListenQue =3D { &mTcpListenQue=0D };=0D =0D -TCP_SEQNO mTcpGlobalIss =3D TCP_BASE_ISS;=0D +//=0D +// The Session secret=0D +// This must be initialized to a random value at boot time=0D +//=0D +TCP_SEQNO mTcpGlobalSecret;=0D +=0D +//=0D +// Union to hold either an IPv4 or IPv6 address=0D +// This is used to simplify the ISN hash computation=0D +//=0D +typedef union {=0D + UINT8 IPv4[4];=0D + UINT8 IPv6[16];=0D +} NETWORK_ADDRESS;=0D +=0D +//=0D +// The ISN is computed by hashing this structure=0D +// It is initialized with the local and remote IP addresses and ports=0D +// and the secret=0D +//=0D +//=0D +typedef struct {=0D + UINT16 LocalPort;=0D + UINT16 RemotePort;=0D + NETWORK_ADDRESS LocalAddress;=0D + NETWORK_ADDRESS RemoteAddress;=0D + TCP_SEQNO Secret;=0D +} ISN_HASH_CTX;=0D =0D CHAR16 *mTcpStateName[] =3D {=0D L"TCP_CLOSED",=0D @@ -41,12 +68,18 @@ CHAR16 *mTcpStateName[] =3D { =0D @param[in, out] Tcb Pointer to the TCP_CB of this TCP ins= tance.=0D =0D + @retval EFI_SUCCESS The operation completed successfully=0D + @retval others The underlying functions failed and coul= d not complete the operation=0D +=0D **/=0D -VOID=0D +EFI_STATUS=0D TcpInitTcbLocal (=0D IN OUT TCP_CB *Tcb=0D )=0D {=0D + TCP_SEQNO Isn;=0D + EFI_STATUS Status;=0D +=0D //=0D // Compute the checksum of the fixed parts of pseudo header=0D //=0D @@ -57,6 +90,16 @@ TcpInitTcbLocal ( 0x06,=0D 0=0D );=0D +=0D + Status =3D TcpGetIsn (=0D + Tcb->LocalEnd.Ip.v4.Addr,=0D + sizeof (IPv4_ADDRESS),=0D + Tcb->LocalEnd.Port,=0D + Tcb->RemoteEnd.Ip.v4.Addr,=0D + sizeof (IPv4_ADDRESS),=0D + Tcb->RemoteEnd.Port,=0D + &Isn=0D + );=0D } else {=0D Tcb->HeadSum =3D NetIp6PseudoHeadChecksum (=0D &Tcb->LocalEnd.Ip.v6,=0D @@ -64,9 +107,25 @@ TcpInitTcbLocal ( 0x06,=0D 0=0D );=0D +=0D + Status =3D TcpGetIsn (=0D + Tcb->LocalEnd.Ip.v6.Addr,=0D + sizeof (IPv6_ADDRESS),=0D + Tcb->LocalEnd.Port,=0D + Tcb->RemoteEnd.Ip.v6.Addr,=0D + sizeof (IPv6_ADDRESS),=0D + Tcb->RemoteEnd.Port,=0D + &Isn=0D + );=0D + }=0D +=0D + if (EFI_ERROR (Status)) {=0D + DEBUG ((DEBUG_ERROR, "TcpInitTcbLocal: failed to get isn\n"));=0D + ASSERT (FALSE);=0D + return Status;=0D }=0D =0D - Tcb->Iss =3D TcpGetIss ();=0D + Tcb->Iss =3D Isn;=0D Tcb->SndUna =3D Tcb->Iss;=0D Tcb->SndNxt =3D Tcb->Iss;=0D =0D @@ -82,6 +141,8 @@ TcpInitTcbLocal ( Tcb->RetxmitSeqMax =3D 0;=0D =0D Tcb->ProbeTimerOn =3D FALSE;=0D +=0D + return EFI_SUCCESS;=0D }=0D =0D /**=0D @@ -506,18 +567,162 @@ TcpCloneTcb ( }=0D =0D /**=0D - Compute an ISS to be used by a new connection.=0D -=0D - @return The resulting ISS.=0D + Retrieves the Initial Sequence Number (ISN) for a TCP connection identif= ied by local=0D + and remote IP addresses and ports.=0D +=0D + This method is based on https://datatracker.ietf.org/doc/html/rfc9293#se= ction-3.4.1=0D + Where the ISN is computed as follows:=0D + ISN =3D TimeStamp + MD5(LocalIP, LocalPort, RemoteIP, RemotePort, Secr= et)=0D +=0D + Otherwise:=0D + ISN =3D M + F(localip, localport, remoteip, remoteport, secretkey)=0D +=0D + "Here M is the 4 microsecond timer, and F() is a pseudorandom function= (PRF) of the=0D + connection's identifying parameters ("localip, localport, remoteip, re= moteport")=0D + and a secret key ("secretkey") (SHLD-1). F() MUST NOT be computable fr= om the=0D + outside (MUST-9), or an attacker could still guess at sequence numbers= from the=0D + ISN used for some other connection. The PRF could be implemented as a= =0D + cryptographic hash of the concatenation of the TCP connection paramete= rs and some=0D + secret data. For discussion of the selection of a specific hash algori= thm and=0D + management of the secret key data."=0D +=0D + @param[in] LocalIp A pointer to the local IP address of the= TCP connection.=0D + @param[in] LocalIpSize The size, in bytes, of the LocalIp buffe= r.=0D + @param[in] LocalPort The local port number of the TCP connect= ion.=0D + @param[in] RemoteIp A pointer to the remote IP address of th= e TCP connection.=0D + @param[in] RemoteIpSize The size, in bytes, of the RemoteIp buff= er.=0D + @param[in] RemotePort The remote port number of the TCP connec= tion.=0D + @param[out] Isn A pointer to the variable that will rece= ive the Initial=0D + Sequence Number (ISN).=0D +=0D + @retval EFI_SUCCESS The operation completed successfully, an= d the ISN was=0D + retrieved.=0D + @retval EFI_INVALID_PARAMETER One or more of the input parameters are = invalid.=0D + @retval EFI_UNSUPPORTED The operation is not supported.=0D =0D **/=0D -TCP_SEQNO=0D -TcpGetIss (=0D - VOID=0D +EFI_STATUS=0D +TcpGetIsn (=0D + IN UINT8 *LocalIp,=0D + IN UINTN LocalIpSize,=0D + IN UINT16 LocalPort,=0D + IN UINT8 *RemoteIp,=0D + IN UINTN RemoteIpSize,=0D + IN UINT16 RemotePort,=0D + OUT TCP_SEQNO *Isn=0D )=0D {=0D - mTcpGlobalIss +=3D TCP_ISS_INCREMENT_1;=0D - return mTcpGlobalIss;=0D + EFI_STATUS Status;=0D + EFI_HASH2_PROTOCOL *Hash2Protocol;=0D + EFI_HASH2_OUTPUT HashResult;=0D + ISN_HASH_CTX IsnHashCtx;=0D + EFI_TIME TimeStamp;=0D +=0D + //=0D + // Check that the ISN pointer is valid=0D + //=0D + if (Isn =3D=3D NULL) {=0D + return EFI_INVALID_PARAMETER;=0D + }=0D +=0D + //=0D + // The local ip may be a v4 or v6 address and may not be NULL=0D + //=0D + if ((LocalIp =3D=3D NULL) || (LocalIpSize =3D=3D 0) || (RemoteIp =3D=3D = NULL) || (RemoteIpSize =3D=3D 0)) {=0D + return EFI_INVALID_PARAMETER;=0D + }=0D +=0D + //=0D + // the local ip may be a v4 or v6 address=0D + //=0D + if ((LocalIpSize !=3D sizeof (EFI_IPv4_ADDRESS)) && (LocalIpSize !=3D si= zeof (EFI_IPv6_ADDRESS))) {=0D + return EFI_INVALID_PARAMETER;=0D + }=0D +=0D + //=0D + // Locate the Hash Protocol=0D + //=0D + Status =3D gBS->LocateProtocol (&gEfiHash2ProtocolGuid, NULL, (VOID **)&= Hash2Protocol);=0D + if (EFI_ERROR (Status)) {=0D + DEBUG ((DEBUG_NET, "Failed to locate Hash Protocol: %r\n", Status));=0D +=0D + //=0D + // TcpCreateService(..) is expected to be called prior to this functio= n=0D + //=0D + ASSERT_EFI_ERROR (Status);=0D + return Status;=0D + }=0D +=0D + //=0D + // Initialize the hash algorithm=0D + //=0D + Status =3D Hash2Protocol->HashInit (Hash2Protocol, &gEfiHashAlgorithmSha= 256Guid);=0D + if (EFI_ERROR (Status)) {=0D + DEBUG ((DEBUG_NET, "Failed to initialize sha256 hash algorithm: %r\n",= Status));=0D + return Status;=0D + }=0D +=0D + IsnHashCtx.LocalPort =3D LocalPort;=0D + IsnHashCtx.RemotePort =3D RemotePort;=0D + IsnHashCtx.Secret =3D mTcpGlobalSecret;=0D +=0D + //=0D + // Check the IP address family and copy accordingly=0D + //=0D + if (LocalIpSize =3D=3D sizeof (EFI_IPv4_ADDRESS)) {=0D + CopyMem (&IsnHashCtx.LocalAddress.IPv4, LocalIp, LocalIpSize);=0D + } else if (LocalIpSize =3D=3D sizeof (EFI_IPv6_ADDRESS)) {=0D + CopyMem (&IsnHashCtx.LocalAddress.IPv6, LocalIp, LocalIpSize);=0D + } else {=0D + return EFI_INVALID_PARAMETER; // Unsupported address size=0D + }=0D +=0D + //=0D + // Repeat the process for the remote IP address=0D + //=0D + if (RemoteIpSize =3D=3D sizeof (EFI_IPv4_ADDRESS)) {=0D + CopyMem (&IsnHashCtx.RemoteAddress.IPv4, RemoteIp, RemoteIpSize);=0D + } else if (RemoteIpSize =3D=3D sizeof (EFI_IPv6_ADDRESS)) {=0D + CopyMem (&IsnHashCtx.RemoteAddress.IPv6, RemoteIp, RemoteIpSize);=0D + } else {=0D + return EFI_INVALID_PARAMETER; // Unsupported address size=0D + }=0D +=0D + //=0D + // Compute the hash=0D + // Update the hash with the data=0D + //=0D + Status =3D Hash2Protocol->HashUpdate (Hash2Protocol, (UINT8 *)&IsnHashCt= x, sizeof (IsnHashCtx));=0D + if (EFI_ERROR (Status)) {=0D + DEBUG ((DEBUG_NET, "Failed to update hash: %r\n", Status));=0D + return Status;=0D + }=0D +=0D + //=0D + // Finalize the hash and retrieve the result=0D + //=0D + Status =3D Hash2Protocol->HashFinal (Hash2Protocol, &HashResult);=0D + if (EFI_ERROR (Status)) {=0D + DEBUG ((DEBUG_NET, "Failed to finalize hash: %r\n", Status));=0D + return Status;=0D + }=0D +=0D + Status =3D gRT->GetTime (&TimeStamp, NULL);=0D + if (EFI_ERROR (Status)) {=0D + return Status;=0D + }=0D +=0D + //=0D + // copy the first 4 bytes of the hash result into the ISN=0D + //=0D + CopyMem (Isn, HashResult.Md5Hash, sizeof (*Isn));=0D +=0D + //=0D + // now add the timestamp to the ISN as 4 microseconds units (1000 / 4 = =3D 250)=0D + //=0D + *Isn +=3D (TCP_SEQNO)TimeStamp.Nanosecond * 250;=0D +=0D + return Status;=0D }=0D =0D /**=0D @@ -721,17 +926,28 @@ TcpFormatNetbuf ( @param[in, out] Tcb Pointer to the TCP_CB that wants to initia= te a=0D connection.=0D =0D + @retval EFI_SUCCESS The operation completed successfully=0D + @retval others The underlying functions failed and coul= d not complete the operation=0D +=0D **/=0D -VOID=0D +EFI_STATUS=0D TcpOnAppConnect (=0D IN OUT TCP_CB *Tcb=0D )=0D {=0D - TcpInitTcbLocal (Tcb);=0D + EFI_STATUS Status;=0D +=0D + Status =3D TcpInitTcbLocal (Tcb);=0D + if (EFI_ERROR (Status)) {=0D + return Status;=0D + }=0D +=0D TcpSetState (Tcb, TCP_SYN_SENT);=0D =0D TcpSetTimer (Tcb, TCP_TIMER_CONNECT, Tcb->ConnectTimeout);=0D TcpToSendData (Tcb, 1);=0D +=0D + return EFI_SUCCESS;=0D }=0D =0D /**=0D diff --git a/NetworkPkg/TcpDxe/TcpTimer.c b/NetworkPkg/TcpDxe/TcpTimer.c index 5d2e124977..065b1bdf5f 100644 --- a/NetworkPkg/TcpDxe/TcpTimer.c +++ b/NetworkPkg/TcpDxe/TcpTimer.c @@ -2,7 +2,7 @@ TCP timer related functions.=0D =0D Copyright (c) 2009 - 2010, Intel Corporation. All rights reserved.
=0D -=0D + Copyright (c) Microsoft Corporation=0D SPDX-License-Identifier: BSD-2-Clause-Patent=0D =0D **/=0D @@ -483,7 +483,6 @@ TcpTickingDpc ( INT16 Index;=0D =0D mTcpTick++;=0D - mTcpGlobalIss +=3D TCP_ISS_INCREMENT_2;=0D =0D //=0D // Don't use LIST_FOR_EACH, which isn't delete safe.=0D diff --git a/NetworkPkg/SecurityFixes.yaml b/NetworkPkg/SecurityFixes.yaml index 20a4555019..4305328425 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=0D - http://packetstormsecurity.com/files/176574/PixieFail-Proof-Of-Conce= pts.html=0D - https://blog.quarkslab.com/pixiefail-nine-vulnerabilities-in-tianoco= res-edk-ii-ipv6-network-stack.html=0D +CVE_2023_45236:=0D + commit_titles:=0D + - "NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236 Patch"=0D + cve: CVE-2023-45236=0D + date_reported: 2023-08-28 13:56 UTC=0D + description: "Bug 08 - edk2/NetworkPkg: Predictable TCP Initial Sequence= Numbers"=0D + note:=0D + files_impacted:=0D + - NetworkPkg/Include/Library/NetLib.h=0D + - NetworkPkg/TcpDxe/TcpDriver.c=0D + - NetworkPkg/TcpDxe/TcpDxe.inf=0D + - NetworkPkg/TcpDxe/TcpFunc.h=0D + - NetworkPkg/TcpDxe/TcpInput.c=0D + - NetworkPkg/TcpDxe/TcpMain.h=0D + - NetworkPkg/TcpDxe/TcpMisc.c=0D + - NetworkPkg/TcpDxe/TcpTimer.c=0D + links:=0D + - https://bugzilla.tianocore.org/show_bug.cgi?id=3D4541=0D + - https://nvd.nist.gov/vuln/detail/CVE-2023-45236=0D + - http://www.openwall.com/lists/oss-security/2024/01/16/2=0D + - http://packetstormsecurity.com/files/176574/PixieFail-Proof-Of-Conce= pts.html=0D + - https://blog.quarkslab.com/pixiefail-nine-vulnerabilities-in-tianoco= res-edk-ii-ipv6-network-stack.html=0D CVE_2023_45237:=0D commit_titles:=0D - "NetworkPkg:: SECURITY PATCH CVE 2023-45237"=0D --=20 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119236): https://edk2.groups.io/g/devel/message/119236 Mute This Topic: https://groups.io/mt/106276861/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-