Compare commits
10 Commits
db413a8c0c
...
7ad5863296
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7ad5863296 | ||
|
|
c3a776afa8 | ||
|
|
879cf2319a | ||
|
|
8a57ffe89f | ||
|
|
05ce7f4994 | ||
|
|
b78a0b7ffc | ||
|
|
29cd7f28f3 | ||
|
|
59507c0953 | ||
|
|
532bf782ac | ||
|
|
506636cbbd |
File diff suppressed because it is too large
Load Diff
@ -0,0 +1,87 @@
|
||||
From b95e87d6764eb59e0e0814da5f33c4900cab57d4 Mon Sep 17 00:00:00 2001
|
||||
From: ShenYage <shenyage1@huawei.com>
|
||||
Date: Fri, 28 Feb 2025 16:04:22 +0800
|
||||
Subject: [PATCH 1/2] NetworkPkg: TcpDxe: SECURITY PATCH CVE-2023-45236 Relared
|
||||
Patch
|
||||
|
||||
BUG: Tianocore's EDK2 TCP implementation generates ISNs using fixed
|
||||
increments from a fixed base value and thus is uceptible to TCP session injection
|
||||
and session hijack attacks.
|
||||
|
||||
This commit is a patch for CVE-2023-45236. Generates ISNs using RngLib to get a high-quality random number.
|
||||
|
||||
Signed-off-by: ShenYage <shenyage1@huawei.com>
|
||||
---
|
||||
NetworkPkg/TcpDxe/TcpDxe.inf | 1 +
|
||||
NetworkPkg/TcpDxe/TcpMain.h | 1 +
|
||||
NetworkPkg/TcpDxe/TcpMisc.c | 9 ++++++++-
|
||||
NetworkPkg/TcpDxe/TcpTimer.c | 7 ++++++-
|
||||
4 files changed, 16 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/NetworkPkg/TcpDxe/TcpDxe.inf b/NetworkPkg/TcpDxe/TcpDxe.inf
|
||||
index c0acbdca..9281f908 100644
|
||||
--- a/NetworkPkg/TcpDxe/TcpDxe.inf
|
||||
+++ b/NetworkPkg/TcpDxe/TcpDxe.inf
|
||||
@@ -67,6 +67,7 @@
|
||||
DpcLib
|
||||
NetLib
|
||||
IpIoLib
|
||||
+ RngLib
|
||||
|
||||
|
||||
[Protocols]
|
||||
diff --git a/NetworkPkg/TcpDxe/TcpMain.h b/NetworkPkg/TcpDxe/TcpMain.h
|
||||
index c0c9b7f4..fc66c00e 100644
|
||||
--- a/NetworkPkg/TcpDxe/TcpMain.h
|
||||
+++ b/NetworkPkg/TcpDxe/TcpMain.h
|
||||
@@ -16,6 +16,7 @@
|
||||
#include <Library/IpIoLib.h>
|
||||
#include <Library/DevicePathLib.h>
|
||||
#include <Library/PrintLib.h>
|
||||
+#include <Library/RngLib.h>
|
||||
|
||||
#include "Socket.h"
|
||||
#include "TcpProto.h"
|
||||
diff --git a/NetworkPkg/TcpDxe/TcpMisc.c b/NetworkPkg/TcpDxe/TcpMisc.c
|
||||
index c93212d4..b5e6120d 100644
|
||||
--- a/NetworkPkg/TcpDxe/TcpMisc.c
|
||||
+++ b/NetworkPkg/TcpDxe/TcpMisc.c
|
||||
@@ -516,7 +516,14 @@ TcpGetIss (
|
||||
VOID
|
||||
)
|
||||
{
|
||||
- mTcpGlobalIss += TCP_ISS_INCREMENT_1;
|
||||
+ UINT32 RandomVal;
|
||||
+
|
||||
+ if (GetRandomNumber32(&RandomVal)) {
|
||||
+ mTcpGlobalIss += RandomVal;
|
||||
+ } else {
|
||||
+ mTcpGlobalIss += TCP_ISS_INCREMENT_1;
|
||||
+ }
|
||||
+
|
||||
return mTcpGlobalIss;
|
||||
}
|
||||
|
||||
diff --git a/NetworkPkg/TcpDxe/TcpTimer.c b/NetworkPkg/TcpDxe/TcpTimer.c
|
||||
index 5d2e1249..5c2ba1a1 100644
|
||||
--- a/NetworkPkg/TcpDxe/TcpTimer.c
|
||||
+++ b/NetworkPkg/TcpDxe/TcpTimer.c
|
||||
@@ -481,9 +481,14 @@ TcpTickingDpc (
|
||||
LIST_ENTRY *Next;
|
||||
TCP_CB *Tcb;
|
||||
INT16 Index;
|
||||
+ UINT32 RandomVal;
|
||||
|
||||
mTcpTick++;
|
||||
- mTcpGlobalIss += TCP_ISS_INCREMENT_2;
|
||||
+ if (GetRandomNumber32(&RandomVal)) {
|
||||
+ mTcpGlobalIss += RandomVal;
|
||||
+ } else {
|
||||
+ mTcpGlobalIss += TCP_ISS_INCREMENT_2;
|
||||
+ }
|
||||
|
||||
//
|
||||
// Don't use LIST_FOR_EACH, which isn't delete safe.
|
||||
--
|
||||
2.33.0
|
||||
|
||||
@ -0,0 +1,67 @@
|
||||
From e0bdb75c67290d6851a4d2509fcfafaf9ef0e696 Mon Sep 17 00:00:00 2001
|
||||
From: ShenYage <shenyage1@huawei.com>
|
||||
Date: Fri, 28 Feb 2025 16:18:39 +0800
|
||||
Subject: [PATCH 2/2] NetworkPkg: DxeNetLib: SECURITY PATCH CVE-2023-45237
|
||||
Relared Patch
|
||||
|
||||
This commit is a patch for CVE-2023-45237. Using RngLib to generate a stronger pseudoRandom number for NetRandomInitSeed().
|
||||
|
||||
Signed-off-by: ShenYage <shenyage1@huawei.com>
|
||||
---
|
||||
NetworkPkg/Library/DxeNetLib/DxeNetLib.c | 18 ++++++++++++------
|
||||
NetworkPkg/Library/DxeNetLib/DxeNetLib.inf | 1 +
|
||||
2 files changed, 13 insertions(+), 6 deletions(-)
|
||||
|
||||
diff --git a/NetworkPkg/Library/DxeNetLib/DxeNetLib.c b/NetworkPkg/Library/DxeNetLib/DxeNetLib.c
|
||||
index fd4a9e15..d24038e8 100644
|
||||
--- a/NetworkPkg/Library/DxeNetLib/DxeNetLib.c
|
||||
+++ b/NetworkPkg/Library/DxeNetLib/DxeNetLib.c
|
||||
@@ -31,6 +31,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
|
||||
#include <Library/DevicePathLib.h>
|
||||
#include <Library/PrintLib.h>
|
||||
#include <Library/UefiLib.h>
|
||||
+#include <Library/RngLib.h>
|
||||
|
||||
#define NIC_ITEM_CONFIG_SIZE (sizeof (NIC_IP4_CONFIG_INFO) + sizeof (EFI_IP4_ROUTE_TABLE) * MAX_IP4_CONFIG_IN_VARIABLE)
|
||||
#define DEFAULT_ZERO_START ((UINTN) ~0)
|
||||
@@ -902,14 +903,19 @@ NetRandomInitSeed (
|
||||
EFI_TIME Time;
|
||||
UINT32 Seed;
|
||||
UINT64 MonotonicCount;
|
||||
+ UINT32 RandomVal;
|
||||
|
||||
- gRT->GetTime (&Time, NULL);
|
||||
- Seed = (Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | Time.Second);
|
||||
- Seed ^= Time.Nanosecond;
|
||||
- Seed ^= Time.Year << 7;
|
||||
+ if (GetRandomNumber32(&RandomVal)) {
|
||||
+ Seed = RandomVal;
|
||||
+ } else {
|
||||
+ gRT->GetTime (&Time, NULL);
|
||||
+ Seed = (Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | Time.Second);
|
||||
+ Seed ^= Time.Nanosecond;
|
||||
+ Seed ^= Time.Year << 7;
|
||||
|
||||
- gBS->GetNextMonotonicCount (&MonotonicCount);
|
||||
- Seed += (UINT32)MonotonicCount;
|
||||
+ gBS->GetNextMonotonicCount (&MonotonicCount);
|
||||
+ Seed += (UINT32)MonotonicCount;
|
||||
+ }
|
||||
|
||||
return Seed;
|
||||
}
|
||||
diff --git a/NetworkPkg/Library/DxeNetLib/DxeNetLib.inf b/NetworkPkg/Library/DxeNetLib/DxeNetLib.inf
|
||||
index 8145d256..ce90aa5e 100644
|
||||
--- a/NetworkPkg/Library/DxeNetLib/DxeNetLib.inf
|
||||
+++ b/NetworkPkg/Library/DxeNetLib/DxeNetLib.inf
|
||||
@@ -43,6 +43,7 @@
|
||||
MemoryAllocationLib
|
||||
DevicePathLib
|
||||
PrintLib
|
||||
+ RngLib
|
||||
|
||||
|
||||
[Guids]
|
||||
--
|
||||
2.33.0
|
||||
|
||||
@ -1,823 +0,0 @@
|
||||
From bb6d7763998a29ac05144d382966fe9fd5b7ef78 Mon Sep 17 00:00:00 2001
|
||||
From: Doug Flick <dougflick@microsoft.com>
|
||||
Date: Wed, 8 May 2024 22:56:29 -0700
|
||||
Subject: [PATCH 2/2] NetworkPkg TcpDxe: SECURITY PATCH CVE-2023-45236
|
||||
|
||||
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>
|
||||
Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
|
||||
---
|
||||
NetworkPkg/SecurityFixes.yaml | 22 +++
|
||||
NetworkPkg/TcpDxe/TcpDriver.c | 92 ++++++++++++-
|
||||
NetworkPkg/TcpDxe/TcpDxe.inf | 8 +-
|
||||
NetworkPkg/TcpDxe/TcpFunc.h | 23 ++--
|
||||
NetworkPkg/TcpDxe/TcpInput.c | 13 +-
|
||||
NetworkPkg/TcpDxe/TcpMain.h | 59 ++++++--
|
||||
NetworkPkg/TcpDxe/TcpMisc.c | 244 ++++++++++++++++++++++++++++++++--
|
||||
NetworkPkg/TcpDxe/TcpTimer.c | 3 +-
|
||||
8 files changed, 415 insertions(+), 49 deletions(-)
|
||||
|
||||
diff --git a/NetworkPkg/SecurityFixes.yaml b/NetworkPkg/SecurityFixes.yaml
|
||||
index a44cfc4..00ebacb 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"
|
||||
diff --git a/NetworkPkg/TcpDxe/TcpDriver.c b/NetworkPkg/TcpDxe/TcpDriver.c
|
||||
index f5d10c6..32cff88 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/TcpDxe.inf b/NetworkPkg/TcpDxe/TcpDxe.inf
|
||||
index 1b30980..dc08f76 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 @@
|
||||
NetLib
|
||||
IpIoLib
|
||||
|
||||
-
|
||||
[Protocols]
|
||||
## SOMETIMES_CONSUMES
|
||||
## SOMETIMES_PRODUCES
|
||||
@@ -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
|
||||
diff --git a/NetworkPkg/TcpDxe/TcpFunc.h b/NetworkPkg/TcpDxe/TcpFunc.h
|
||||
index a7af01f..35ea55d 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/TcpInput.c b/NetworkPkg/TcpDxe/TcpInput.c
|
||||
index 7b329be..63fd03a 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/TcpMain.h b/NetworkPkg/TcpDxe/TcpMain.h
|
||||
index c0c9b7f..dbc1da2 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
|
||||
+ );
|
||||
+
|
||||
#endif
|
||||
diff --git a/NetworkPkg/TcpDxe/TcpMisc.c b/NetworkPkg/TcpDxe/TcpMisc.c
|
||||
index c93212d..753dec5 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.
|
||||
-
|
||||
- @return The resulting ISS.
|
||||
+ 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.
|
||||
|
||||
**/
|
||||
-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 5d2e124..f45d4fb 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.
|
||||
--
|
||||
2.33.0
|
||||
|
||||
121
0083-Fix-timing-side-channel-CVE-2024-13176.patch
Normal file
121
0083-Fix-timing-side-channel-CVE-2024-13176.patch
Normal file
@ -0,0 +1,121 @@
|
||||
From ccdf50988462e9889f3553cbefbe81bba3e41e1f Mon Sep 17 00:00:00 2001
|
||||
From: hy <12444214+dhjgty@user.noreply.gitee.com>
|
||||
Date: Tue, 25 Feb 2025 23:29:26 +0800
|
||||
Subject: [PATCH] Fix timing side-channel in ECDSA signature computation
|
||||
There is a timing signal of around 300 nanoseconds when the top word of
|
||||
the inverted ECDSA nonce value is zero. This can happen with significant
|
||||
probability only for some of the supported elliptic curves. In particular
|
||||
the NIST P-521 curve is affected. To be able to measure this leak, the
|
||||
attacker process must either be located in the same physical computer or
|
||||
must have a very fast network connection with low latency.
|
||||
|
||||
Attacks on ECDSA nonce are also known as Minerva attack.
|
||||
|
||||
Fixes CVE-2024-13176
|
||||
|
||||
Reviewed-by: Tim Hudson <tjh@openssl.org>
|
||||
Reviewed-by: Neil Horman <nhorman@openssl.org>
|
||||
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
|
||||
|
||||
---
|
||||
.../OpensslLib/openssl/crypto/bn/bn_exp.c | 21 +++++++++++++------
|
||||
.../OpensslLib/openssl/crypto/ec/ec_lib.c | 7 ++++---
|
||||
.../OpensslLib/openssl/include/crypto/bn.h | 3 +++
|
||||
3 files changed, 22 insertions(+), 9 deletions(-)
|
||||
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/crypto/bn/bn_exp.c b/CryptoPkg/Library/OpensslLib/openssl/crypto/bn/bn_exp.c
|
||||
index 4e169ae1..a161e580 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/crypto/bn/bn_exp.c
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/crypto/bn/bn_exp.c
|
||||
@@ -598,7 +598,7 @@ static int MOD_EXP_CTIME_COPY_FROM_PREBUF(BIGNUM *b, int top,
|
||||
* out by Colin Percival,
|
||||
* http://www.daemonology.net/hyperthreading-considered-harmful/)
|
||||
*/
|
||||
-int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
|
||||
+int bn_mod_exp_mont_fixed_top(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
|
||||
const BIGNUM *m, BN_CTX *ctx,
|
||||
BN_MONT_CTX *in_mont)
|
||||
{
|
||||
@@ -615,10 +615,6 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
|
||||
unsigned int t4 = 0;
|
||||
#endif
|
||||
|
||||
- bn_check_top(a);
|
||||
- bn_check_top(p);
|
||||
- bn_check_top(m);
|
||||
-
|
||||
if (!BN_is_odd(m)) {
|
||||
ERR_raise(ERR_LIB_BN, BN_R_CALLED_WITH_EVEN_MODULUS);
|
||||
return 0;
|
||||
@@ -1138,7 +1134,7 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
|
||||
goto err;
|
||||
} else
|
||||
#endif
|
||||
- if (!BN_from_montgomery(rr, &tmp, mont, ctx))
|
||||
+ if (!bn_from_mont_fixed_top(rr, &tmp, mont, ctx))
|
||||
goto err;
|
||||
ret = 1;
|
||||
err:
|
||||
@@ -1152,6 +1148,19 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
|
||||
return ret;
|
||||
}
|
||||
|
||||
+int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
|
||||
+ const BIGNUM *m, BN_CTX *ctx,
|
||||
+ BN_MONT_CTX *in_mont)
|
||||
+{
|
||||
+ bn_check_top(a);
|
||||
+ bn_check_top(p);
|
||||
+ bn_check_top(m);
|
||||
+ if (!bn_mod_exp_mont_fixed_top(rr, a, p, m, ctx, in_mont))
|
||||
+ return 0;
|
||||
+ bn_correct_top(rr);
|
||||
+ return 1;
|
||||
+}
|
||||
+
|
||||
int BN_mod_exp_mont_word(BIGNUM *rr, BN_ULONG a, const BIGNUM *p,
|
||||
const BIGNUM *m, BN_CTX *ctx, BN_MONT_CTX *in_mont)
|
||||
{
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/crypto/ec/ec_lib.c b/CryptoPkg/Library/OpensslLib/openssl/crypto/ec/ec_lib.c
|
||||
index b1696d93..1f0bf1ec 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/crypto/ec/ec_lib.c
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/crypto/ec/ec_lib.c
|
||||
@@ -20,6 +20,7 @@
|
||||
#include <openssl/err.h>
|
||||
#include <openssl/opensslv.h>
|
||||
#include "crypto/ec.h"
|
||||
+#include "crypto/bn.h"
|
||||
#include "internal/nelem.h"
|
||||
#include "ec_local.h"
|
||||
|
||||
@@ -1262,10 +1263,10 @@ static int ec_field_inverse_mod_ord(const EC_GROUP *group, BIGNUM *r,
|
||||
if (!BN_sub(e, group->order, e))
|
||||
goto err;
|
||||
/*-
|
||||
- * Exponent e is public.
|
||||
- * No need for scatter-gather or BN_FLG_CONSTTIME.
|
||||
+ * Although the exponent is public we want the result to be
|
||||
+ * fixed top.
|
||||
*/
|
||||
- if (!BN_mod_exp_mont(r, x, e, group->order, ctx, group->mont_data))
|
||||
+ if (!bn_mod_exp_mont_fixed_top(r, x, e, group->order, ctx, group->mont_data))
|
||||
goto err;
|
||||
|
||||
ret = 1;
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/include/crypto/bn.h b/CryptoPkg/Library/OpensslLib/openssl/include/crypto/bn.h
|
||||
index fd1c09d9..ba50bca2 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/include/crypto/bn.h
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/include/crypto/bn.h
|
||||
@@ -73,6 +73,9 @@ int bn_set_words(BIGNUM *a, const BN_ULONG *words, int num_words);
|
||||
*/
|
||||
int bn_mul_mont_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
|
||||
BN_MONT_CTX *mont, BN_CTX *ctx);
|
||||
+int bn_mod_exp_mont_fixed_top(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
|
||||
+ const BIGNUM *m, BN_CTX *ctx,
|
||||
+ BN_MONT_CTX *in_mont);
|
||||
int bn_to_mont_fixed_top(BIGNUM *r, const BIGNUM *a, BN_MONT_CTX *mont,
|
||||
BN_CTX *ctx);
|
||||
int bn_from_mont_fixed_top(BIGNUM *r, const BIGNUM *a, BN_MONT_CTX *mont,
|
||||
--
|
||||
2.33.0
|
||||
|
||||
70
0084-Free-the-read-buffers-CVE-2024-4741.patch
Normal file
70
0084-Free-the-read-buffers-CVE-2024-4741.patch
Normal file
@ -0,0 +1,70 @@
|
||||
From 4487afc6c7ca5024ff8556ab76907449769b660d Mon Sep 17 00:00:00 2001
|
||||
From: hy <12444214+dhjgty@user.noreply.gitee.com>
|
||||
Date: Mon, 24 Feb 2025 22:50:29 +0800
|
||||
Subject: [PATCH] fix CVE-2024-4741
|
||||
|
||||
Only free the read buffers if we're not using them
|
||||
If we're part way through processing a record, or the application has
|
||||
not released all the records then we should not free our buffer because
|
||||
they are still needed.
|
||||
|
||||
CVE-2024-4741
|
||||
|
||||
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
||||
Reviewed-by: Neil Horman <nhorman@openssl.org>
|
||||
Reviewed-by: Matt Caswell <matt@openssl.org>
|
||||
---
|
||||
.../Library/OpensslLib/openssl/ssl/record/rec_layer_s3.c | 9 +++++++++
|
||||
CryptoPkg/Library/OpensslLib/openssl/ssl/record/record.h | 1 +
|
||||
CryptoPkg/Library/OpensslLib/openssl/ssl/ssl_lib.c | 3 +++
|
||||
3 files changed, 13 insertions(+)
|
||||
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/ssl/record/rec_layer_s3.c b/CryptoPkg/Library/OpensslLib/openssl/ssl/record/rec_layer_s3.c
|
||||
index 3baf8207..99602b6b 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/ssl/record/rec_layer_s3.c
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/ssl/record/rec_layer_s3.c
|
||||
@@ -81,6 +81,15 @@ int RECORD_LAYER_read_pending(const RECORD_LAYER *rl)
|
||||
return SSL3_BUFFER_get_left(&rl->rbuf) != 0;
|
||||
}
|
||||
|
||||
+int RECORD_LAYER_data_present(const RECORD_LAYER *rl)
|
||||
+{
|
||||
+ if (rl->rstate == SSL_ST_READ_BODY)
|
||||
+ return 1;
|
||||
+ if (RECORD_LAYER_processed_read_pending(rl))
|
||||
+ return 1;
|
||||
+ return 0;
|
||||
+}
|
||||
+
|
||||
/* Checks if we have decrypted unread record data pending */
|
||||
int RECORD_LAYER_processed_read_pending(const RECORD_LAYER *rl)
|
||||
{
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/ssl/record/record.h b/CryptoPkg/Library/OpensslLib/openssl/ssl/record/record.h
|
||||
index 234656bf..b60f71c8 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/ssl/record/record.h
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/ssl/record/record.h
|
||||
@@ -205,6 +205,7 @@ void RECORD_LAYER_release(RECORD_LAYER *rl);
|
||||
int RECORD_LAYER_read_pending(const RECORD_LAYER *rl);
|
||||
int RECORD_LAYER_processed_read_pending(const RECORD_LAYER *rl);
|
||||
int RECORD_LAYER_write_pending(const RECORD_LAYER *rl);
|
||||
+int RECORD_LAYER_data_present(const RECORD_LAYER *rl);
|
||||
void RECORD_LAYER_reset_read_sequence(RECORD_LAYER *rl);
|
||||
void RECORD_LAYER_reset_write_sequence(RECORD_LAYER *rl);
|
||||
int RECORD_LAYER_is_sslv2_record(RECORD_LAYER *rl);
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/ssl/ssl_lib.c b/CryptoPkg/Library/OpensslLib/openssl/ssl/ssl_lib.c
|
||||
index 5d57f5d2..ac4ae41e 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/ssl/ssl_lib.c
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/ssl/ssl_lib.c
|
||||
@@ -5489,6 +5489,9 @@ int SSL_free_buffers(SSL *ssl)
|
||||
if (RECORD_LAYER_read_pending(rl) || RECORD_LAYER_write_pending(rl))
|
||||
return 0;
|
||||
|
||||
+ if (RECORD_LAYER_data_present(rl))
|
||||
+ return 0;
|
||||
+
|
||||
RECORD_LAYER_release(rl);
|
||||
return 1;
|
||||
}
|
||||
--
|
||||
2.33.0
|
||||
|
||||
79
0085-Process-key-length-CVE-2023-5363.patch
Normal file
79
0085-Process-key-length-CVE-2023-5363.patch
Normal file
@ -0,0 +1,79 @@
|
||||
From d691ea7823f1a139ac7b859709be84c74ea6653f Mon Sep 17 00:00:00 2001
|
||||
From: hy <12444214+dhjgty@user.noreply.gitee.com>
|
||||
Date: Mon, 24 Feb 2025 23:05:55 +0800
|
||||
Subject: [PATCH] evp: process key length and iv length early if present
|
||||
evp_cipher_init_internal() takes a params array argument and this is
|
||||
processed late in the initialisation process for some ciphers (AEAD ones).
|
||||
|
||||
This means that changing the IV length as a parameter will either truncate the
|
||||
IV (very bad if SP 800-38d section 8.2.1 is used) or grab extra uninitialised
|
||||
bytes.
|
||||
|
||||
Truncation is very bad if SP 800-38d section 8.2.1 is being used to
|
||||
contruct a deterministic IV. This leads to an instant loss of confidentiality.
|
||||
|
||||
Grabbing extra bytes isn't so serious, it will most likely result in a bad
|
||||
decryption.
|
||||
|
||||
Problem reported by Tony Battersby of Cybernetics.com but earlier discovered
|
||||
and raised as issue #19822.
|
||||
|
||||
Fixes CVE-2023-5363
|
||||
Fixes #19822
|
||||
|
||||
Reviewed-by: Hugo Landau <hlandau@openssl.org>
|
||||
Reviewed-by: Matt Caswell <matt@openssl.org>
|
||||
---
|
||||
.../OpensslLib/openssl/crypto/evp/evp_enc.c | 36 +++++++++++++++++++
|
||||
1 file changed, 36 insertions(+)
|
||||
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/crypto/evp/evp_enc.c b/CryptoPkg/Library/OpensslLib/openssl/crypto/evp/evp_enc.c
|
||||
index b178d108..2dff3e66 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/crypto/evp/evp_enc.c
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/crypto/evp/evp_enc.c
|
||||
@@ -218,6 +218,42 @@ static int evp_cipher_init_internal(EVP_CIPHER_CTX *ctx,
|
||||
return 0;
|
||||
}
|
||||
|
||||
+#ifndef FIPS_MODULE
|
||||
+ /*
|
||||
+ * Fix for CVE-2023-5363
|
||||
+ * Passing in a size as part of the init call takes effect late
|
||||
+ * so, force such to occur before the initialisation.
|
||||
+ *
|
||||
+ * The FIPS provider's internal library context is used in a manner
|
||||
+ * such that this is not an issue.
|
||||
+ */
|
||||
+ if (params != NULL) {
|
||||
+ OSSL_PARAM param_lens[3] = { OSSL_PARAM_END, OSSL_PARAM_END,
|
||||
+ OSSL_PARAM_END };
|
||||
+ OSSL_PARAM *q = param_lens;
|
||||
+ const OSSL_PARAM *p;
|
||||
+
|
||||
+ p = OSSL_PARAM_locate_const(params, OSSL_CIPHER_PARAM_KEYLEN);
|
||||
+ if (p != NULL)
|
||||
+ memcpy(q++, p, sizeof(*q));
|
||||
+
|
||||
+ /*
|
||||
+ * Note that OSSL_CIPHER_PARAM_AEAD_IVLEN is a synomym for
|
||||
+ * OSSL_CIPHER_PARAM_IVLEN so both are covered here.
|
||||
+ */
|
||||
+ p = OSSL_PARAM_locate_const(params, OSSL_CIPHER_PARAM_IVLEN);
|
||||
+ if (p != NULL)
|
||||
+ memcpy(q++, p, sizeof(*q));
|
||||
+
|
||||
+ if (q != param_lens) {
|
||||
+ if (!EVP_CIPHER_CTX_set_params(ctx, param_lens)) {
|
||||
+ ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_LENGTH);
|
||||
+ return 0;
|
||||
+ }
|
||||
+ }
|
||||
+ }
|
||||
+#endif
|
||||
+
|
||||
if (enc) {
|
||||
if (ctx->cipher->einit == NULL) {
|
||||
ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
|
||||
--
|
||||
2.33.0
|
||||
|
||||
198
0086-Check-DSA-parameters.patch
Normal file
198
0086-Check-DSA-parameters.patch
Normal file
@ -0,0 +1,198 @@
|
||||
From 164f617509519076015b4ec63fb3e11cbcad3028 Mon Sep 17 00:00:00 2001
|
||||
From: hy <12444214+dhjgty@user.noreply.gitee.com>
|
||||
Date: Sun, 16 Mar 2025 16:30:29 +0800
|
||||
Subject: [PATCH] Check DSA parameters for excessive sizes before validating
|
||||
This avoids overly long computation of various validation checks.
|
||||
|
||||
Fixes CVE-2024-4603
|
||||
|
||||
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
|
||||
Reviewed-by: Matt Caswell <matt@openssl.org>
|
||||
Reviewed-by: Neil Horman <nhorman@openssl.org>
|
||||
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
|
||||
---
|
||||
.../Library/OpensslLib/openssl/CHANGES.md | 17 ++++++
|
||||
.../OpensslLib/openssl/crypto/dsa/dsa_check.c | 44 ++++++++++++--
|
||||
.../invalid/p10240_q256_too_big.pem | 57 +++++++++++++++++++
|
||||
3 files changed, 114 insertions(+), 4 deletions(-)
|
||||
create mode 100644 CryptoPkg/Library/OpensslLib/openssl/test/recipes/15-test_dsaparam_data/invalid/p10240_q256_too_big.pem
|
||||
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/CHANGES.md b/CryptoPkg/Library/OpensslLib/openssl/CHANGES.md
|
||||
index 0fb1eb1f..2209e0ce 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/CHANGES.md
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/CHANGES.md
|
||||
@@ -30,6 +30,23 @@ breaking changes, and mappings for the large list of deprecated functions.
|
||||
|
||||
### Changes between 3.0.8 and 3.0.9 [30 May 2023]
|
||||
|
||||
+ * Fixed an issue where checking excessively long DSA keys or parameters may
|
||||
+ be very slow.
|
||||
+
|
||||
+ Applications that use the functions EVP_PKEY_param_check() or
|
||||
+ EVP_PKEY_public_check() to check a DSA public key or DSA parameters may
|
||||
+ experience long delays. Where the key or parameters that are being checked
|
||||
+ have been obtained from an untrusted source this may lead to a Denial of
|
||||
+ Service.
|
||||
+
|
||||
+ To resolve this issue DSA keys larger than OPENSSL_DSA_MAX_MODULUS_BITS
|
||||
+ will now fail the check immediately with a DSA_R_MODULUS_TOO_LARGE error
|
||||
+ reason.
|
||||
+
|
||||
+ ([CVE-2024-4603])
|
||||
+
|
||||
+ *Tomáš Mráz*
|
||||
+
|
||||
* Mitigate for the time it takes for `OBJ_obj2txt` to translate gigantic
|
||||
OBJECT IDENTIFIER sub-identifiers to canonical numeric text form.
|
||||
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/crypto/dsa/dsa_check.c b/CryptoPkg/Library/OpensslLib/openssl/crypto/dsa/dsa_check.c
|
||||
index 7ee914a4..ed01ea8f 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/crypto/dsa/dsa_check.c
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/crypto/dsa/dsa_check.c
|
||||
@@ -19,8 +19,34 @@
|
||||
#include "dsa_local.h"
|
||||
#include "crypto/dsa.h"
|
||||
|
||||
+static int dsa_precheck_params(const DSA *dsa, int *ret)
|
||||
+ {
|
||||
+ if (dsa->params.p == NULL || dsa->params.q == NULL) {
|
||||
+ ERR_raise(ERR_LIB_DSA, DSA_R_BAD_FFC_PARAMETERS);
|
||||
+ *ret = FFC_CHECK_INVALID_PQ;
|
||||
+ return 0;
|
||||
+ }
|
||||
+
|
||||
+ if (BN_num_bits(dsa->params.p) > OPENSSL_DSA_MAX_MODULUS_BITS) {
|
||||
+ ERR_raise(ERR_LIB_DSA, DSA_R_MODULUS_TOO_LARGE);
|
||||
+ *ret = FFC_CHECK_INVALID_PQ;
|
||||
+ return 0;
|
||||
+ }
|
||||
+
|
||||
+ if (BN_num_bits(dsa->params.q) >= BN_num_bits(dsa->params.p)) {
|
||||
+ ERR_raise(ERR_LIB_DSA, DSA_R_BAD_Q_VALUE);
|
||||
+ *ret = FFC_CHECK_INVALID_PQ;
|
||||
+ return 0;
|
||||
+ }
|
||||
+
|
||||
+ return 1;
|
||||
+ }
|
||||
+
|
||||
int ossl_dsa_check_params(const DSA *dsa, int checktype, int *ret)
|
||||
{
|
||||
+ if (!dsa_precheck_params(dsa, ret))
|
||||
+ return 0;
|
||||
+
|
||||
if (checktype == OSSL_KEYMGMT_VALIDATE_QUICK_CHECK)
|
||||
return ossl_ffc_params_simple_validate(dsa->libctx, &dsa->params,
|
||||
FFC_PARAM_TYPE_DSA, ret);
|
||||
@@ -39,6 +65,9 @@ int ossl_dsa_check_params(const DSA *dsa, int checktype, int *ret)
|
||||
*/
|
||||
int ossl_dsa_check_pub_key(const DSA *dsa, const BIGNUM *pub_key, int *ret)
|
||||
{
|
||||
+ if (!dsa_precheck_params(dsa, ret))
|
||||
+ return 0;
|
||||
+
|
||||
return ossl_ffc_validate_public_key(&dsa->params, pub_key, ret);
|
||||
}
|
||||
|
||||
@@ -49,6 +78,9 @@ int ossl_dsa_check_pub_key(const DSA *dsa, const BIGNUM *pub_key, int *ret)
|
||||
*/
|
||||
int ossl_dsa_check_pub_key_partial(const DSA *dsa, const BIGNUM *pub_key, int *ret)
|
||||
{
|
||||
+ if (!dsa_precheck_params(dsa, ret))
|
||||
+ return 0;
|
||||
+
|
||||
return ossl_ffc_validate_public_key_partial(&dsa->params, pub_key, ret);
|
||||
}
|
||||
|
||||
@@ -56,8 +88,10 @@ int ossl_dsa_check_priv_key(const DSA *dsa, const BIGNUM *priv_key, int *ret)
|
||||
{
|
||||
*ret = 0;
|
||||
|
||||
- return (dsa->params.q != NULL
|
||||
- && ossl_ffc_validate_private_key(dsa->params.q, priv_key, ret));
|
||||
+ if (!dsa_precheck_params(dsa, ret))
|
||||
+ return 0;
|
||||
+
|
||||
+ return ossl_ffc_validate_private_key(dsa->params.q, priv_key, ret);
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -70,8 +104,10 @@ int ossl_dsa_check_pairwise(const DSA *dsa)
|
||||
BN_CTX *ctx = NULL;
|
||||
BIGNUM *pub_key = NULL;
|
||||
|
||||
- if (dsa->params.p == NULL
|
||||
- || dsa->params.g == NULL
|
||||
+ if (!dsa_precheck_params(dsa, &ret))
|
||||
+ return 0;
|
||||
+
|
||||
+ if (dsa->params.g == NULL
|
||||
|| dsa->priv_key == NULL
|
||||
|| dsa->pub_key == NULL)
|
||||
return 0;
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/test/recipes/15-test_dsaparam_data/invalid/p10240_q256_too_big.pem b/CryptoPkg/Library/OpensslLib/openssl/test/recipes/15-test_dsaparam_data/invalid/p10240_q256_too_big.pem
|
||||
new file mode 100644
|
||||
index 00000000..162be8a8
|
||||
--- /dev/null
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/test/recipes/15-test_dsaparam_data/invalid/p10240_q256_too_big.pem
|
||||
@@ -0,0 +1,57 @@
|
||||
+-----BEGIN DSA PARAMETERS-----
|
||||
+ MIIKLAKCBQEAym47LzPFZdbz16WvjczLKuzLtsP8yRk/exxL4bBthJhP1qOwctja
|
||||
+ p1586SF7gDxCMn7yWVEYdfRbFefGoq0gj1XOE917XqlbnkmZhMgxut2KbNJo/xil
|
||||
+ XNFUjGvKs3F413U9rAodC8f07cWHP1iTcWL+vPe6u2yilKWYYfnLWHQH+Z6aPrrF
|
||||
+ x/R08LI6DZ6nEsIo+hxaQnEtx+iqNTJC6Q1RIjWDqxQkFVTkJ0Y7miRDXmRdneWk
|
||||
+ oLrMZRpaXr5l5tSjEghh1pBgJcdyOv0lh4dlDy/alAiqE2Qlb667yHl6A9dDPlpW
|
||||
+ dAntpffy4LwOxfbuEhISvKjjQoBwIvYE4TBPqL0Q6bC6HgQ4+tqd9b44pQjdIQjb
|
||||
+ Xcjc6azheITSnPEex3OdKtKoQeRq01qCeLBpMXu1c+CTf4ApKArZvT3vZSg0hM1O
|
||||
+ pR71bRZrEEegDj0LH2HCgI5W6H3blOS9A0kUTddCoQXr2lsVdiPtRbPKH1gcd9FQ
|
||||
+ P8cGrvbakpTiC0dCczOMDaCteM1QNILlkM7ZoV6VghsKvDnFPxFsiIr5GgjasXP5
|
||||
+ hhbn3g7sDoq1LiTEo+IKQY28pBWx7etSOSRuXW/spnvCkivZla7lSEGljoy9QlQ2
|
||||
+ UZmsEQI9G3YyzgpxHvKZBK1CiZVTywdYKTZ4TYCxvqzhYhjv2bqbpjI12HRFLojB
|
||||
+ koyEmMSp53lldCzp158PrIanqSp2rksMR8SmmCL3FwfAp2OjqFMEglG9DT8x0WaN
|
||||
+ TLSkjGC6t2csMte7WyU1ekNoFDKfMjDSAz0+xIx21DEmZtYqFOg1DNPK1xYLS0pl
|
||||
+ RSMRRkJVN2mk/G7/1oxlB8Wb9wgi3GKUqqCYT11SnBjzq0NdoJ3E4GMedp5Lx3AZ
|
||||
+ 4mFuRPUd4iV86tE0XDSHSFE7Y3ZkrOjD7Q/26/L53L/UH5z4HW6CHP5os7QERJjg
|
||||
+ c1S3x87wXWo9QXbB9b2xmf+c+aWwAAr1cviw38tru58jF3/IGyduj9H8claKQqBG
|
||||
+ cIOUF4aNe1hK2K3ArAOApUxr4KE+tCvrltRfiTmVFip0g9Jt1CPY3Zu7Bd4Z2ZkE
|
||||
+ DtSztpwa49HrWF5E9xpquvBL2U8jQ68E7Xd8Wp4orI/TIChriamBmdkgRz3H2LvN
|
||||
+ Ozb6+hsnEGrz3sp2RVAToSqA9ysa6nHZdfufPNtMEbQdO/k1ehmGRb0ljBRsO6b2
|
||||
+ rsG2eYuC8tg8eCrIkua0TGRI7g6a4K32AJdzaX6NsISaaIW+OYJuoDSscvD3oOg8
|
||||
+ PPEhU+zM7xJskTA+jxvPlikKx8V7MNHOCQECldJlUBwzJvqp40JvwfnDsF+8VYwd
|
||||
+ UaiieR3pzMzyTjpReXRmZbnRPusRcsVzxb2OhB79wmuy4UPjjQBX+7eD0rs8xxvW
|
||||
+ 5a5q1Cjq4AvbwmmcA/wDrHDOjcbD/zodad2O1QtBWa/R4xyWea4zKsflgACE1zY9
|
||||
+ wW2br7+YQFekcrXkkkEzgxd6zxv8KVEDpXRZjmAM1cI5LvkoN64To4GedN8Qe/G7
|
||||
+ R9SZh9gnS17PTP64hK+aYqhFafMdu87q/+qLfxaSux727qE5hiW01u4nnWhACf9s
|
||||
+ xuOozowKqxZxkolMIyZv6Lddwy1Zv5qjCyd0DvM/1skpXWkb9kfabYC+OhjsjVhs
|
||||
+ 0Ktfs6a5B3eixiw5x94hhIcTEcS4hmvhGUL72FiTca6ZeSERTKmNBy8CIQC9/ZUN
|
||||
+ uU/V5JTcnYyUGHzm7+XcZBjyGBagBj9rCmW3SQKCBQAJ/k9rb39f1cO+/3XDEMjy
|
||||
+ 9bIEXSuS48g5RAc1UGd5nrrBQwuDxGWFyz0yvAY7LgyidZuJS21+MAp9EY7AOMmx
|
||||
+ TDttifNaBJYt4GZ8of166PcqTKkHQwq5uBpxeSDv/ZE8YbYfaCtLTcUC8KlO+l36
|
||||
+ gjJHSkdkflSsGy1yObSNDQDfVAAwQs//TjDMnuEtvlNXZllsTvFFBceXVETn10K2
|
||||
+ ZMmdSIJNfLnjReUKEN6PfeGqv7F4xoyGwUybEfRE4u5RmXrqCODaIjY3SNMrOq8B
|
||||
+ R3Ata/cCozsM1jIdIW2z+OybDJH+BYsYm2nkSZQjZS6javTYClLrntEKG/hAQwL8
|
||||
+ F16YLOQXpHhgiAaWnTZzANtLppB2+5qCVy5ElzKongOwT8JTjTFXOaRnqe/ngm9W
|
||||
+ SSbrxfDaoWUOyK9XD8Cydzpv3n4Y8nWNGayi7/yAFCU36Ri040ufgv/TZLuKacnl
|
||||
+ +3ga3ZUpRlSigzx0kb1+KjTSWeQ8vE/psdWjvBukVEbzdUauMLyRLo/6znSVvvPX
|
||||
+ UGhviThE5uhrsUg+wEPFINriSHfF7JDKVhDcJnLBdaXvfN52pkF/naLBF5Rt3Gvq
|
||||
+ fjCxjx0Sy9Lag1hDN4dor7dzuO7wmwOS01DJW1PtNLuuH0Bbqh1kYSaQkmyXBZWX
|
||||
+ qo8K3nkoDM0niOtJJubOhTNrGmSaZpNXkK3Mcy9rBbdvEs5O0Jmqaax/eOdU0Yot
|
||||
+ B3lX+3ddOseT2ZEFjzObqTtkWuFBeBxuYNcRTsu3qMdIBsEb8URQdsTtjoIja2fK
|
||||
+ hreVgjK36GW70KXEl8V/vq5qjQulmqkBEjmilcDuiREKqQuyeagUOnhQaBplqVco
|
||||
+ 4xznh5DMBMRbpGb5lHxKv4cPNi+uNAJ5i98zWUM1JRt6aXnRCuWcll1z8fRZ+5kD
|
||||
+ vK9FaZU3VRMK/eknEG49cGr8OuJ6ZRSaC+tKwV1y+amkSZpKPWnk2bUnQI3ApJv3
|
||||
+ k1e1EToeECpMUkLMDgNbpKBoz4nqMEvAAlYgw9xKNbLlQlahqTVEAmaJHh4yDMDy
|
||||
+ i7IZ9Wrn47IGoR7s3cvhDHUpRPeW4nsmgzj+tf5EAxemI61STZJTTWo0iaPGJxct
|
||||
+ 9nhOOhw1I38Mvm4vkAbFH7YJ0B6QrjjYL2MbOTp5JiIh4vdOeWwNo9/y4ffyaN5+
|
||||
+ ADpxuuIAmcbdr6GPOhkOFFixRJa0B2eP1i032HESlLs8RB9oYtdTXdXQotnIgJGd
|
||||
+ Y8tSKOa1zjzeLHn3AVpRZTUW++/BxmApV3GKIeG8fsUjg/df0QRrBcdC/1uccdaG
|
||||
+ KKlAOwlywVn5jUlwHkTmDiTM9w5AqVVGHZ2b+4ZgQW8jnPKN0SrKf6U555D+zp7E
|
||||
+ x4uXoE8ojN9y8m8UKf0cTLnujH2XgZorjPfuMOt5VZEhQFMS2QaljSeni5CJJ8gk
|
||||
+ XtztNqfBlAtWR4V5iAHeQOfIB2YaOy8GESda89tyKraKeaez41VblpTVHTeq9IIF
|
||||
+ YB4cQA2PfuNaGVRGLMAgT3Dvl+mxxxeJyxnGAiUcETU/jJJt9QombiuszBlYGQ5d
|
||||
+ ELOSm/eQSRARV9zNSt5jaQlMSjMBqenIEM09BzYqa7jDwqoztFxNdO8bcuQPuKwa
|
||||
+ 4z3bBZ1yYm63WFdNbQqqGEwc0OYmqg1raJ0zltgHyjFyw8IGu4g/wETs+nVQcH7D
|
||||
+ vKuje86bePD6kD/LH3wmkA==
|
||||
+ -----END DSA PARAMETERS-----
|
||||
--
|
||||
2.33.0
|
||||
|
||||
187
0087-Harden-BN_GF2m_poly2arr-against-misuse.patch
Normal file
187
0087-Harden-BN_GF2m_poly2arr-against-misuse.patch
Normal file
@ -0,0 +1,187 @@
|
||||
From 2a0fa58af18f2ab5435ee2cefa6a02cacfb18818 Mon Sep 17 00:00:00 2001
|
||||
From: hy <941973499@qq.com>
|
||||
Date: Fri, 28 Mar 2025 22:48:57 +0800
|
||||
Subject: [PATCH] Harden BN_GF2m_poly2arr against misuse. The
|
||||
BN_GF2m_poly2arr() function converts characteristic-2 field (GF_{2^m}) Galois
|
||||
polynomials from a representation as a BIGNUM bitmask, to a compact array
|
||||
with just the exponents of the non-zero terms.
|
||||
|
||||
These polynomials are then used in BN_GF2m_mod_arr() to perform modular
|
||||
reduction. A precondition of calling BN_GF2m_mod_arr() is that the
|
||||
polynomial must have a non-zero constant term (i.e. the array has `0` as
|
||||
its final element).
|
||||
|
||||
Internally, callers of BN_GF2m_poly2arr() did not verify that
|
||||
precondition, and binary EC curve parameters with an invalid polynomial
|
||||
could lead to out of bounds memory reads and writes in BN_GF2m_mod_arr().
|
||||
|
||||
The precondition is always true for polynomials that arise from the
|
||||
standard form of EC parameters for characteristic-two fields (X9.62).
|
||||
See the "Finite Field Identification" section of:
|
||||
|
||||
https://www.itu.int/ITU-T/formal-language/itu-t/x/x894/2018-cor1/ANSI-X9-62.html
|
||||
|
||||
The OpenSSL GF(2^m) code supports only the trinomial and pentanomial
|
||||
basis X9.62 forms.
|
||||
|
||||
This commit updates BN_GF2m_poly2arr() to return `0` (failure) when
|
||||
the constant term is zero (i.e. the input bitmask BIGNUM is not odd).
|
||||
|
||||
Additionally, the return value is made unambiguous when there is not
|
||||
enough space to also pad the array with a final `-1` sentinel value.
|
||||
The return value is now always the number of elements (including the
|
||||
final `-1`) that would be filled when the output array is sufficiently
|
||||
large. Previously the same count was returned both when the array has
|
||||
just enough room for the final `-1` and when it had only enough space
|
||||
for non-sentinel values.
|
||||
|
||||
Finally, BN_GF2m_poly2arr() is updated to reject polynomials whose
|
||||
degree exceeds `OPENSSL_ECC_MAX_FIELD_BITS`, this guards against
|
||||
CPU exhausition attacks via excessively large inputs.
|
||||
|
||||
The above issues do not arise in processing X.509 certificates. These
|
||||
generally have EC keys from "named curves", and RFC5840 (Section 2.1.1)
|
||||
disallows explicit EC parameters. The TLS code in OpenSSL enforces this
|
||||
constraint only after the certificate is decoded, but, even if explicit
|
||||
parameters are specified, they are in X9.62 form, which cannot represent
|
||||
problem values as noted above.
|
||||
|
||||
Initially reported as oss-fuzz issue 71623.
|
||||
---
|
||||
.../OpensslLib/openssl/crypto/bn/bn_gf2m.c | 28 +++++++---
|
||||
.../openssl/test/ec_internal_test.c | 51 +++++++++++++++++++
|
||||
2 files changed, 71 insertions(+), 8 deletions(-)
|
||||
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/crypto/bn/bn_gf2m.c b/CryptoPkg/Library/OpensslLib/openssl/crypto/bn/bn_gf2m.c
|
||||
index 304c2ea0..65e9958c 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/crypto/bn/bn_gf2m.c
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/crypto/bn/bn_gf2m.c
|
||||
@@ -15,6 +15,7 @@
|
||||
#include "bn_local.h"
|
||||
|
||||
#ifndef OPENSSL_NO_EC2M
|
||||
+# include <openssl/ec.h>
|
||||
|
||||
/*
|
||||
* Maximum number of iterations before BN_GF2m_mod_solve_quad_arr should
|
||||
@@ -1134,16 +1135,26 @@ int BN_GF2m_mod_solve_quad(BIGNUM *r, const BIGNUM *a, const BIGNUM *p,
|
||||
/*
|
||||
* Convert the bit-string representation of a polynomial ( \sum_{i=0}^n a_i *
|
||||
* x^i) into an array of integers corresponding to the bits with non-zero
|
||||
- * coefficient. Array is terminated with -1. Up to max elements of the array
|
||||
- * will be filled. Return value is total number of array elements that would
|
||||
- * be filled if array was large enough.
|
||||
+ * coefficient. The array is intended to be suitable for use with
|
||||
+ * `BN_GF2m_mod_arr()`, and so the constant term of the polynomial must not be
|
||||
+ * zero. This translates to a requirement that the input BIGNUM `a` is odd.
|
||||
+ *
|
||||
+ * Given sufficient room, the array is terminated with -1. Up to max elements
|
||||
+ * of the array will be filled.
|
||||
+ *
|
||||
+ * The return value is total number of array elements that would be filled if
|
||||
+ * array was large enough, including the terminating `-1`. It is `0` when `a`
|
||||
+ * is not odd or the constant term is zero contrary to requirement.
|
||||
+ *
|
||||
+ * The return value is also `0` when the leading exponent exceeds
|
||||
+ * `OPENSSL_ECC_MAX_FIELD_BITS`, this guards against CPU exhaustion attacks,
|
||||
*/
|
||||
int BN_GF2m_poly2arr(const BIGNUM *a, int p[], int max)
|
||||
{
|
||||
int i, j, k = 0;
|
||||
BN_ULONG mask;
|
||||
|
||||
- if (BN_is_zero(a))
|
||||
+ if (!BN_is_odd(a))
|
||||
return 0;
|
||||
|
||||
for (i = a->top - 1; i >= 0; i--) {
|
||||
@@ -1161,12 +1172,13 @@ int BN_GF2m_poly2arr(const BIGNUM *a, int p[], int max)
|
||||
}
|
||||
}
|
||||
|
||||
- if (k < max) {
|
||||
+ if (k > 0 && p[0] > OPENSSL_ECC_MAX_FIELD_BITS)
|
||||
+ return 0;
|
||||
+
|
||||
+ if (k < max)
|
||||
p[k] = -1;
|
||||
- k++;
|
||||
- }
|
||||
|
||||
- return k;
|
||||
+ return k + 1;
|
||||
}
|
||||
|
||||
/*
|
||||
diff --git a/CryptoPkg/Library/OpensslLib/openssl/test/ec_internal_test.c b/CryptoPkg/Library/OpensslLib/openssl/test/ec_internal_test.c
|
||||
index 8c2cd056..484cbb2a 100644
|
||||
--- a/CryptoPkg/Library/OpensslLib/openssl/test/ec_internal_test.c
|
||||
+++ b/CryptoPkg/Library/OpensslLib/openssl/test/ec_internal_test.c
|
||||
@@ -155,6 +155,56 @@ static int field_tests_ecp_mont(void)
|
||||
}
|
||||
|
||||
#ifndef OPENSSL_NO_EC2M
|
||||
+/* Test that decoding of invalid GF2m field parameters fails. */
|
||||
+ static int ec2m_field_sanity(void)
|
||||
+ {
|
||||
+ int ret = 0;
|
||||
+ BN_CTX *ctx = BN_CTX_new();
|
||||
+ BIGNUM *p, *a, *b;
|
||||
+ EC_GROUP *group1 = NULL, *group2 = NULL, *group3 = NULL;
|
||||
+
|
||||
+ TEST_info("Testing GF2m hardening\n");
|
||||
+
|
||||
+ BN_CTX_start(ctx);
|
||||
+ p = BN_CTX_get(ctx);
|
||||
+ a = BN_CTX_get(ctx);
|
||||
+ if (!TEST_ptr(b = BN_CTX_get(ctx))
|
||||
+ || !TEST_true(BN_one(a))
|
||||
+ || !TEST_true(BN_one(b)))
|
||||
+ goto out;
|
||||
+
|
||||
+ /* Even pentanomial value should be rejected */
|
||||
+ if (!TEST_true(BN_set_word(p, 0xf2)))
|
||||
+ goto out;
|
||||
+ if (!TEST_ptr_null(group1 = EC_GROUP_new_curve_GF2m(p, a, b, ctx)))
|
||||
+ TEST_error("Zero constant term accepted in GF2m polynomial");
|
||||
+
|
||||
+ /* Odd hexanomial should also be rejected */
|
||||
+ if (!TEST_true(BN_set_word(p, 0xf3)))
|
||||
+ goto out;
|
||||
+ if (!TEST_ptr_null(group2 = EC_GROUP_new_curve_GF2m(p, a, b, ctx)))
|
||||
+ TEST_error("Hexanomial accepted as GF2m polynomial");
|
||||
+
|
||||
+ /* Excessive polynomial degree should also be rejected */
|
||||
+ if (!TEST_true(BN_set_word(p, 0x71))
|
||||
+ || !TEST_true(BN_set_bit(p, OPENSSL_ECC_MAX_FIELD_BITS + 1)))
|
||||
+ goto out;
|
||||
+ if (!TEST_ptr_null(group3 = EC_GROUP_new_curve_GF2m(p, a, b, ctx)))
|
||||
+ TEST_error("GF2m polynomial degree > %d accepted",
|
||||
+ OPENSSL_ECC_MAX_FIELD_BITS);
|
||||
+
|
||||
+ ret = group1 == NULL && group2 == NULL && group3 == NULL;
|
||||
+
|
||||
+ out:
|
||||
+ EC_GROUP_free(group1);
|
||||
+ EC_GROUP_free(group2);
|
||||
+ EC_GROUP_free(group3);
|
||||
+ BN_CTX_end(ctx);
|
||||
+ BN_CTX_free(ctx);
|
||||
+
|
||||
+ return ret;
|
||||
+ }
|
||||
+
|
||||
/* test EC_GF2m_simple_method directly */
|
||||
static int field_tests_ec2_simple(void)
|
||||
{
|
||||
@@ -443,6 +493,7 @@ int setup_tests(void)
|
||||
ADD_TEST(field_tests_ecp_simple);
|
||||
ADD_TEST(field_tests_ecp_mont);
|
||||
#ifndef OPENSSL_NO_EC2M
|
||||
+ ADD_TEST(ec2m_field_sanity);
|
||||
ADD_TEST(field_tests_ec2_simple);
|
||||
#endif
|
||||
ADD_ALL_TESTS(field_tests_default, crv_len);
|
||||
--
|
||||
2.33.0
|
||||
|
||||
185
0088-SecurityPkg-Out-of-bound-read-in-HashPeImageByType.patch
Normal file
185
0088-SecurityPkg-Out-of-bound-read-in-HashPeImageByType.patch
Normal file
@ -0,0 +1,185 @@
|
||||
From 6460d06c6f028154088ea7db4a44821ffabfe9e6 Mon Sep 17 00:00:00 2001
|
||||
From: hy <941973499@qq.com>
|
||||
Date: Sat, 26 Apr 2025 23:38:23 +0800
|
||||
Subject: [PATCH] SecurityPkg: Out of bound read in HashPeImageByType() In
|
||||
HashPeImageByType(), the hash of PE/COFF image is calculated. This function
|
||||
may get untrusted input.
|
||||
|
||||
Inside this function, the following code verifies the loaded image has
|
||||
the correct format, by reading the second byte of the buffer.
|
||||
|
||||
```c
|
||||
if ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) {
|
||||
...
|
||||
}
|
||||
```
|
||||
|
||||
The input image is not trusted and that may not have the second byte to
|
||||
read. So this poses an out of bound read error.
|
||||
|
||||
With below fix we are assuring that we don't do out of bound read. i.e,
|
||||
we make sure that AuthDataSize is greater than 1.
|
||||
|
||||
```c
|
||||
if (AuthDataSize > 1
|
||||
&& (*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE){
|
||||
...
|
||||
}
|
||||
```
|
||||
|
||||
AuthDataSize size is verified before reading the second byte.
|
||||
So if AuthDataSize is less than 2, the second byte will not be read, and
|
||||
the out of bound read situation won't occur.
|
||||
|
||||
Tested the patch on real platform with and without TPM connected and
|
||||
verified image is booting fine.
|
||||
|
||||
Authored-by: Raj AlwinX Selvaraj <Alw...@intel.com>
|
||||
Signed-off-by: Doug Flick <DougFlick@microsoft.com>
|
||||
---
|
||||
.../DxeImageVerificationLib.c | 37 ++++++++++---------
|
||||
SecurityPkg/SecurityFixes.yaml | 15 ++++++++
|
||||
.../SecureBootConfigImpl.c | 37 +++++++++++--------
|
||||
3 files changed, 55 insertions(+), 34 deletions(-)
|
||||
|
||||
diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
|
||||
index 5d8dbd54..157318b1 100644
|
||||
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
|
||||
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
|
||||
@@ -618,6 +618,7 @@ Done:
|
||||
@param[in] AuthDataSize Size of the Authenticode Signature in bytes.
|
||||
|
||||
@retval EFI_UNSUPPORTED Hash algorithm is not supported.
|
||||
+ @retval EFI_BAD_BUFFER_SIZE AuthData provided is invalid size.
|
||||
@retval EFI_SUCCESS Hash successfully.
|
||||
|
||||
**/
|
||||
@@ -629,28 +630,28 @@ HashPeImageByType (
|
||||
{
|
||||
UINT8 Index;
|
||||
|
||||
- for (Index = 0; Index < HASHALG_MAX; Index++) {
|
||||
+ //
|
||||
+ // Check the Hash algorithm in PE/COFF Authenticode.
|
||||
+ // According to PKCS#7 Definition:
|
||||
+ // SignedData ::= SEQUENCE {
|
||||
+ // version Version,
|
||||
+ // digestAlgorithms DigestAlgorithmIdentifiers,
|
||||
+ // contentInfo ContentInfo,
|
||||
+ // .... }
|
||||
+ // The DigestAlgorithmIdentifiers can be used to determine the hash algorithm in PE/COFF hashing
|
||||
+ // This field has the fixed offset (+32) in final Authenticode ASN.1 data.
|
||||
+ // Fixed offset (+32) is calculated based on two bytes of length encoding.
|
||||
+ //
|
||||
+ if ((AuthDataSize > 1) && ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE)) {
|
||||
//
|
||||
- // Check the Hash algorithm in PE/COFF Authenticode.
|
||||
- // According to PKCS#7 Definition:
|
||||
- // SignedData ::= SEQUENCE {
|
||||
- // version Version,
|
||||
- // digestAlgorithms DigestAlgorithmIdentifiers,
|
||||
- // contentInfo ContentInfo,
|
||||
- // .... }
|
||||
- // The DigestAlgorithmIdentifiers can be used to determine the hash algorithm in PE/COFF hashing
|
||||
- // This field has the fixed offset (+32) in final Authenticode ASN.1 data.
|
||||
- // Fixed offset (+32) is calculated based on two bytes of length encoding.
|
||||
+ // Only support two bytes of Long Form of Length Encoding.
|
||||
//
|
||||
- if ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) {
|
||||
- //
|
||||
- // Only support two bytes of Long Form of Length Encoding.
|
||||
- //
|
||||
- continue;
|
||||
- }
|
||||
+ return EFI_BAD_BUFFER_SIZE;
|
||||
+ }
|
||||
|
||||
+ for (Index = 0; Index < HASHALG_MAX; Index++) {
|
||||
if (AuthDataSize < 32 + mHash[Index].OidLength) {
|
||||
- return EFI_UNSUPPORTED;
|
||||
+ continue;
|
||||
}
|
||||
|
||||
if (CompareMem (AuthData + 32, mHash[Index].OidValue, mHash[Index].OidLength) == 0) {
|
||||
diff --git a/SecurityPkg/SecurityFixes.yaml b/SecurityPkg/SecurityFixes.yaml
|
||||
index ceaaa256..0b24844d 100644
|
||||
--- a/SecurityPkg/SecurityFixes.yaml
|
||||
+++ b/SecurityPkg/SecurityFixes.yaml
|
||||
@@ -34,3 +34,18 @@ CVE_2022_36764:
|
||||
- Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c
|
||||
links:
|
||||
- https://bugzilla.tianocore.org/show_bug.cgi?id=4118
|
||||
+CVE_2024_38797:
|
||||
+ commit-titles:
|
||||
+ - "SecurityPkg: Out of bound read in HashPeImageByType()"
|
||||
+ - "SecurityPkg: Improving HashPeImageByType () logic"
|
||||
+ - "SecurityPkg: Improving SecureBootConfigImpl:HashPeImageByType () logic"
|
||||
+ cve: CVE-2024-38797
|
||||
+ date_reported: 2024-06-04 12:00 UTC
|
||||
+ description: Out of bound read in HashPeImageByType()
|
||||
+ note:
|
||||
+ files_impacted:
|
||||
+ - SecurityPkg\Library\DxeImageVerificationLib\DxeImageVerificationLib.c
|
||||
+ - SecurityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigImpl.c
|
||||
+ links:
|
||||
+ - https://bugzilla.tianocore.org/show_bug.cgi?id=2214
|
||||
+ - https://github.com/tianocore/edk2/security/advisories/GHSA-4wjw-6xmf-44xf
|
||||
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
|
||||
index 0e31502b..02aa142b 100644
|
||||
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
|
||||
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
|
||||
@@ -2079,30 +2079,35 @@ HashPeImageByType (
|
||||
{
|
||||
UINT8 Index;
|
||||
WIN_CERTIFICATE_EFI_PKCS *PkcsCertData;
|
||||
+ UINT32 PkcsCertSize;
|
||||
|
||||
PkcsCertData = (WIN_CERTIFICATE_EFI_PKCS *)(mImageBase + mSecDataDir->Offset);
|
||||
+ PkcsCertSize = mSecDataDir->SizeOfCert;
|
||||
|
||||
- for (Index = 0; Index < HASHALG_MAX; Index++) {
|
||||
+ //
|
||||
+ // Check the Hash algorithm in PE/COFF Authenticode.
|
||||
+ // According to PKCS#7 Definition:
|
||||
+ // SignedData ::= SEQUENCE {
|
||||
+ // version Version,
|
||||
+ // digestAlgorithms DigestAlgorithmIdentifiers,
|
||||
+ // contentInfo ContentInfo,
|
||||
+ // .... }
|
||||
+ // The DigestAlgorithmIdentifiers can be used to determine the hash algorithm in PE/COFF hashing
|
||||
+ // This field has the fixed offset (+32) in final Authenticode ASN.1 data.
|
||||
+ // Fixed offset (+32) is calculated based on two bytes of length encoding.
|
||||
+ //
|
||||
+ if ((PkcsCertSize > 1) && ((*(PkcsCertData->CertData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE)) {
|
||||
//
|
||||
- // Check the Hash algorithm in PE/COFF Authenticode.
|
||||
- // According to PKCS#7 Definition:
|
||||
- // SignedData ::= SEQUENCE {
|
||||
- // version Version,
|
||||
- // digestAlgorithms DigestAlgorithmIdentifiers,
|
||||
- // contentInfo ContentInfo,
|
||||
- // .... }
|
||||
- // The DigestAlgorithmIdentifiers can be used to determine the hash algorithm in PE/COFF hashing
|
||||
- // This field has the fixed offset (+32) in final Authenticode ASN.1 data.
|
||||
- // Fixed offset (+32) is calculated based on two bytes of length encoding.
|
||||
+ // Only support two bytes of Long Form of Length Encoding.
|
||||
//
|
||||
- if ((*(PkcsCertData->CertData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) {
|
||||
- //
|
||||
- // Only support two bytes of Long Form of Length Encoding.
|
||||
- //
|
||||
+ return EFI_BAD_BUFFER_SIZE;
|
||||
+ }
|
||||
+
|
||||
+ for (Index = 0; Index < HASHALG_MAX; Index++) {
|
||||
+ if (PkcsCertSize < 32 + mHash[Index].OidLength) {
|
||||
continue;
|
||||
}
|
||||
|
||||
- //
|
||||
if (CompareMem (PkcsCertData->CertData + 32, mHash[Index].OidValue, mHash[Index].OidLength) == 0) {
|
||||
break;
|
||||
}
|
||||
--
|
||||
2.33.0
|
||||
|
||||
35
edk2.spec
35
edk2.spec
@ -7,7 +7,7 @@
|
||||
|
||||
Name: edk2
|
||||
Version: %{stable_date}
|
||||
Release: 17
|
||||
Release: 22
|
||||
Summary: EFI Development Kit II
|
||||
License: BSD-2-Clause-Patent and OpenSSL and MIT
|
||||
URL: https://github.com/tianocore/edk2
|
||||
@ -94,8 +94,8 @@ patch53: 0053-relax_edk2_gcc14.patch
|
||||
patch54: 0054-MdePkg-Fix-overflow-issue-in-BasePeCoffLib.patch
|
||||
|
||||
# Fix CVE-2023-45236、CVE-2023-45237
|
||||
patch55: 0055-NetworkPkg-SECURITY-PATCH-CVE-2023-45237.patch
|
||||
patch56: 0056-NetworkPkg-TcpDxe-SECURITY-PATCH-CVE-2023-45236.patch
|
||||
patch55: 0055-NetworkPkg-TcpDxe-SECURITY-PATCH-CVE-2023-45236-Rela.patch
|
||||
patch56: 0056-NetworkPkg-DxeNetLib-SECURITY-PATCH-CVE-2023-45237-R.patch
|
||||
|
||||
# Support Hygon CSV3
|
||||
patch57: 0057-MdePkg-Add-StandardSignatureIsHygonGenuine-in-BaseCp.patch
|
||||
@ -132,6 +132,20 @@ patch80: 0080-Platform-Loongson-Remove-minimium-memory-size-limita.patch
|
||||
patch81: 0081-Platform-Loongson-Modify-loongarch-uefi-firmware-siz.patch
|
||||
patch82: 0082-fixup-fdt-parse-error.patch
|
||||
|
||||
# Fix CVE-2024-13176、CVE-2024-4741、CVE-2023-5363
|
||||
patch83: 0083-Fix-timing-side-channel-CVE-2024-13176.patch
|
||||
patch84: 0084-Free-the-read-buffers-CVE-2024-4741.patch
|
||||
patch85: 0085-Process-key-length-CVE-2023-5363.patch
|
||||
|
||||
# Fix CVE-2024-4603
|
||||
patch86: 0086-Check-DSA-parameters.patch
|
||||
|
||||
# Fix CVE-2024-9143
|
||||
patch87: 0087-Harden-BN_GF2m_poly2arr-against-misuse.patch
|
||||
|
||||
# Fix CVE-2024-38797
|
||||
patch88: 0088-SecurityPkg-Out-of-bound-read-in-HashPeImageByType.patch
|
||||
|
||||
BuildRequires: acpica-tools gcc gcc-c++ libuuid-devel python3 bc nasm python3-unversioned-command isl
|
||||
|
||||
%description
|
||||
@ -401,6 +415,21 @@ chmod +x %{buildroot}%{_bindir}/Rsa2048Sha256GenerateKeys
|
||||
%endif
|
||||
|
||||
%changelog
|
||||
* Sun Apr 27 2025 huyu<huyu70@h-partners.com> - 202308-22
|
||||
- fix CVE-2024-38797
|
||||
|
||||
* Fri Mar 28 2025 huyu<huyu70@h-partners.com> - 202308-21
|
||||
- fix CVE-2024-9143
|
||||
|
||||
* Sun Mar 16 2025 huyu<huyu70@h-partners.com> - 202308-20
|
||||
- fix CVE-2024-4603
|
||||
|
||||
* Sun Mar 9 2025 shenyage<shenyage1@huawei.com> - 202308-19
|
||||
- fix bugs for CVE-2023-45236、CVE-2023-45237
|
||||
|
||||
* Sun Feb 23 2025 huyu<huyu70@h-partners.com> - 202308-18
|
||||
- fix CVE-2024-13176、CVE-2024-4741、CVE-2023-5363
|
||||
|
||||
* Tue Dec 17 2024 Xiaotian Wu <wuxiaotian@loongson.cn> - 202308-17
|
||||
- Update LoongArch virtual machine
|
||||
- 0080-Platform-Loongson-Remove-minimium-memory-size-limita.patch
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user