Re: [edk2-devel] [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236
No other concerns. Reviewed-by: Saloni Kasbekar From: gaoliming Sent: Thursday, May 23, 2024 6:24 PM To: devel@edk2.groups.io; dougfl...@microsoft.com; Kasbekar, Saloni Subject: 回复: [edk2-devel] [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236 Saloni: Have you any other comments for this patch? Thanks Liming 发件人: devel@edk2.groups.io<mailto:devel@edk2.groups.io> mailto:devel@edk2.groups.io>> 代表 Doug Flick via groups.io 发送时间: 2024年5月22日 3:29 收件人: Saloni Kasbekar mailto:saloni.kasbe...@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> 主题: Re: [edk2-devel] [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236 This was more of a design decision. Both Hash2Protocol and HashLib serve similar purposes. The goal was to use Hash2Protocol to decouple and provide greater modularity and flexibility over HashLib. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119199): https://edk2.groups.io/g/devel/message/119199 Mute This Topic: https://groups.io/mt/106276051/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236
Saloni: Have you any other comments for this patch? Thanks Liming 发件人: devel@edk2.groups.io 代表 Doug Flick via groups.io 发送时间: 2024年5月22日 3:29 收件人: Saloni Kasbekar ; devel@edk2.groups.io 主题: Re: [edk2-devel] [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236 This was more of a design decision. Both Hash2Protocol and HashLib serve similar purposes. The goal was to use Hash2Protocol to decouple and provide greater modularity and flexibility over HashLib. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119169): https://edk2.groups.io/g/devel/message/119169 Mute This Topic: https://groups.io/mt/106274103/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236
This was more of a design decision. Both Hash2Protocol and HashLib serve similar purposes. The goal was to use Hash2Protocol to decouple and provide greater modularity and flexibility over HashLib. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119106): https://edk2.groups.io/g/devel/message/119106 Mute This Topic: https://groups.io/mt/105996585/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236
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 Sent: Wednesday, May 8, 2024 10:56 PM To: devel@edk2.groups.io Cc: Kasbekar, Saloni ; Clark-williams, Zachary Subject: [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236 From: Doug Flick 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 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 | 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.+# 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.-+ 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 oth
[edk2-devel] [PATCH v2 09/13] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236
From: Doug Flick 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 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 | 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. +# 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] gEfiHash2ServiceBindingProtocolGuid diff --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. - + 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 - ); - /