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
[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
- );
-
