Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PixieFail #8 and #9 TCBZ4541 and TCBZ4542 #5582

Merged

Conversation

Flickdm
Copy link
Contributor

@Flickdm Flickdm commented Apr 23, 2024

No description provided.

@Flickdm Flickdm force-pushed the fix/NetworkPkg/bugzilla/4541_and_4542 branch 25 times, most recently from 52fd2d8 to b29d245 Compare April 26, 2024 19:45
@Flickdm Flickdm force-pushed the fix/NetworkPkg/bugzilla/4541_and_4542 branch 5 times, most recently from bad659e to 0b67c6b Compare May 6, 2024 19:17
@Flickdm Flickdm force-pushed the fix/NetworkPkg/bugzilla/4541_and_4542 branch 7 times, most recently from d8a7871 to 02c64ff Compare May 24, 2024 06:56
Flickdm and others added 20 commits May 24, 2024 22:46
This patch adds RngDxe to EmulatorPkg. The RngDxe is used to provide
random number generation services to the UEFI firmware.

Cc: Andrew Fish <[email protected]>
Cc: Ray Ni <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ray Ni <[email protected]>
This patch adds Hash2DxeCrypto to EmulatorPkg. The Hash2DxeCrypto is
used to provide the hashing protocol services.

Cc: Andrew Fish <[email protected]>
Cc: Ray Ni <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ray Ni <[email protected]>
This patch adds "virtio-rng-pci" to the PlatformBuildLib.py
This adds Rng services to the guest VM

Cc: Ard Biesheuvel <[email protected]>
Cc: Jiewen Yao <[email protected]>
Cc: Gerd Hoffmann <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Tested-by: Gerd Hoffmann <[email protected]>
Acked-by: Gerd Hoffmann <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
This patch adds Hash2DxeCrypto to OvmfPkg. The Hash2DxeCrypto is
used to provide the hashing protocol services.

Cc: Ard Biesheuvel <[email protected]>
Cc: Jiewen Yao <[email protected]>
Cc: Gerd Hoffmann <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Tested-by: Gerd Hoffmann <[email protected]>
Acked-by: Gerd Hoffmann <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
This patch adds "virtio-rng-pci" to the PlatformBuildLib.py
This adds Rng services to the guest VM

Cc: Ard Biesheuvel <[email protected]>
Cc: Leif Lindholm <[email protected]>
Cc: Sami Mujawar <[email protected]>
Cc: Gerd Hoffmann <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
This patch adds Hash2DxeCrypto to ArmVirtPkg. The Hash2DxeCrypto is
used to provide the hashing protocol services.

Cc: Ard Biesheuvel <[email protected]>
Cc: Leif Lindholm <[email protected]>
Cc: Sami Mujawar <[email protected]>
Cc: Gerd Hoffmann <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
Removed from gEfiRngAlgorithmRaw an incorrect assumption that
Raw cannot return less than 256 bits. The DRNG Algorithms
should always use a 256 bit seed as per nist standards
however a caller is free to request less than 256 bits.
>
>     //
>    // When a DRBG is used on the output of a entropy source,
>    // its security level must be at least 256 bits according to UEFI
Spec.
>    //
>    if (RNGValueLength < 32) {
>      return EFI_INVALID_PARAMETER;
>    }
>

AARCH64 platforms do not have this limitation and this brings both
implementations into alignment with each other and the spec.

Cc: Jiewen Yao <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
Reviewed-by: Pierre Gondois <[email protected]>
Acked-by: Jiewe Yao <[email protected]>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4542

Bug Overview:
PixieFail Bug tianocore#9
CVE-2023-45237
CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:N/A:N
CWE-338 Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)

Use of a Weak PseudoRandom Number Generator

Change Overview:

Updates all Instances of NET_RANDOM (NetRandomInitSeed ()) to either

>
> EFI_STATUS
> EFIAPI
> PseudoRandomU32 (
>  OUT UINT32  *Output
>  );
>

or (depending on the use case)

>
> EFI_STATUS
> EFIAPI
> PseudoRandom (
>  OUT  VOID   *Output,
>  IN   UINTN  OutputLength
>  );
>

This is because the use of

Example:

The following code snippet PseudoRandomU32 () function is used:

>
> UINT32         Random;
>
> Status = PseudoRandomU32 (&Random);
> if (EFI_ERROR (Status)) {
>   DEBUG ((DEBUG_ERROR, "%a failed to generate random number: %r\n",
__func__, Status));
>   return Status;
> }
>

This also introduces a new PCD to enable/disable the use of the
secure implementation of algorithms for PseudoRandom () and
instead depend on the default implementation. This may be required for
some platforms where the UEFI Spec defined algorithms are not available.

>
> PcdEnforceSecureRngAlgorithms
>

If the platform does not have any one of the UEFI defined
secure RNG algorithms then the driver will assert.

Cc: Saloni Kasbekar <[email protected]>
Cc: Zachary Clark-williams <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Saloni Kasbekar <[email protected]>
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 tianocore#8
CVE-2023-45236
CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:C/C:L/I:N/A:N
CWE-200 Exposure of Sensitive Information to an Unauthorized Actor

Updates TCP ISN generation to use a cryptographic hash of the
connection's identifying parameters and a secret key.
This prevents an attacker from guessing the ISN used for some other
connection.

This is follows the guidance in RFC 1948, RFC 6528, and RFC 9293.

RFC: 9293 Section 3.4.1.  Initial Sequence Number Selection

   A TCP implementation MUST use the above type of "clock" for clock-
   driven selection of initial sequence numbers (MUST-8), and SHOULD
   generate its initial sequence numbers with the expression:

   ISN = M + F(localip, localport, remoteip, remoteport, secretkey)

   where M is the 4 microsecond timer, and F() is a pseudorandom
   function (PRF) of the connection's identifying parameters ("localip,
   localport, remoteip, remoteport") and a secret key ("secretkey")
   (SHLD-1).  F() MUST NOT be computable from the outside (MUST-9), or
   an attacker could still guess at sequence numbers from the ISN used
   for some other connection.  The PRF could be implemented as a
   cryptographic hash of the concatenation of the TCP connection
   parameters and some secret data.  For discussion of the selection of
   a specific hash algorithm and management of the secret key data,
   please see Section 3 of [42].

   For each connection there is a send sequence number and a receive
   sequence number.  The initial send sequence number (ISS) is chosen by
   the data sending TCP peer, and the initial receive sequence number
   (IRS) is learned during the connection-establishing procedure.

   For a connection to be established or initialized, the two TCP peers
   must synchronize on each other's initial sequence numbers.  This is
   done in an exchange of connection-establishing segments carrying a
   control bit called "SYN" (for synchronize) and the initial sequence
   numbers.  As a shorthand, segments carrying the SYN bit are also
   called "SYNs".  Hence, the solution requires a suitable mechanism for
   picking an initial sequence number and a slightly involved handshake
   to exchange the ISNs.

Cc: Saloni Kasbekar <[email protected]>
Cc: Zachary Clark-williams <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Saloni Kasbekar <[email protected]>
This commit adds a mock library for UefiBootServicesTableLib.

Cc: Michael D Kinney <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Zhiguang Liu <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Liming Gao <[email protected]>
This patch adds a protocol for MockRng. This protocol is used to
mock the Rng protocol for testing purposes.

Cc: Michael D Kinney <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Zhiguang Liu <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Liming Gao <[email protected]>
This commit adds a new MockHash2 protocol to the MdePkg. This allows
the unit tests to pick up the new protocol and use it for testing.

Cc: Michael D Kinney <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Zhiguang Liu <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Liming Gao <[email protected]>
This patch updates the PxeBcDhcp6GoogleTest due to the changes in the
underlying code. The changes are as follows:
 - Random now comes from the RngLib Protocol
 - The TCP ISN is now generated by the hash function

Cc: Saloni Kasbekar <[email protected]>
Cc: Zachary Clark-williams <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Saloni Kasbekar <[email protected]>
ArmVirtQemu may execute at EL2, in which case monitor calls are
generally made using SMC instructions instead of HVC instructions.

Whether or not this is the case can only be decided at runtime, and so
the associated PCD needs to be settable at runtime, if the platform
definition chooses so. This implies a boolean PCD, given that a feature
PCD is build-time configurable only.

Cc: Leif Lindholm <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Sami Mujawar <[email protected]>

Committed-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
This moves the PcdMonitorConduitHvc from PcdsFeatureFlag.Common to
PcdsFixedAtBuild.Common

This is a follow on to the previous commit:
ArmPkg: Allow SMC/HVC monitor conduit to be specified at runtime

ArmVirtQemu may execute at EL2, in which case monitor calls are
generally made using SMC instructions instead of HVC instructions.

Whether or not this is the case can only be decided at runtime, and so
the associated PCD needs to be settable at runtime, if the platform
definition chooses so. This implies a boolean PCD, given that a feature
PCD is build-time configurable only.

Cc: Leif Lindholm <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Sami Mujawar <[email protected]>

Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
BaseRngLib on AARCH64 will discover whether or not RNDR instructions are
supported, by inspecting the ISAR0 identification register, and setting
a global boolean accordingly. This boolean is used in subsequent
execution to decide whether or not to issue the instruction.

The same discovery code also ASSERT()s that RNDR instructions are
implemented, which is unnecessary, and breaks execution on systems that
incorporate the library but don't implement the instruction (or fail to
expose it to the exception level that the firmware executes at).

So drop the ASSERT().

Cc: Michael D Kinney <[email protected]>
Cc: Liming Gao <[email protected]>
Cc: Zhiguang Liu <[email protected]>

Committed-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Liming Gao <[email protected]>
Currently, only TPM2 builds enable the PCD PEIM, which is a prerequisite
for being able to use dynamic PCDs already at the PEI stage. This
facility will be used for other reasons too so move those pieces out of
code block that are conditional on TPM2_ENABLE

Cc: Ard Biesheuvel <[email protected]>
Cc: Leif Lindholm <[email protected]>
Cc: Sami Mujawar <[email protected]>
Cc: Gerd Hoffmann <[email protected]>

Committed-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
On ARM systems, whether SMC or HVC instructions need to be used to issue
monitor calls is typically dependent on the exception level, but there
are also cases where EL1 might use SMC instructions, so there is no hard
and fast rule.

For ArmVirtQemu, this does depend strictly on the exception level, so
set the default to HVC (for EL1 execution) and override it to SMC when
booted at EL2.

Cc: Ard Biesheuvel <[email protected]>
Cc: Leif Lindholm <[email protected]>
Cc: Sami Mujawar <[email protected]>
Cc: Gerd Hoffmann <[email protected]>

Committed-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
MdeLibs.inc sets default library class resolutions which are much more
general than the ones that might be specified in ArmVirt.dsc.inc. So the
latter should be included *after* MdeLibs.inc to ensure that its
definitions take precedence.

Cc: Ard Biesheuvel <[email protected]>
Cc: Leif Lindholm <[email protected]>
Cc: Sami Mujawar <[email protected]>
Cc: Gerd Hoffmann <[email protected]>

Committed-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
Add the RngDxe driver to the build, backed by either RNDR or TRNG, one
of which is expected to be available in most cases:
- RNDR is implemented by the 'max' CPU that QEMU implements in TCG mode
- TRNG is implemented by the KVM hypervisor, which backs QEMU's 'host'
  CPU

Other TCG modes (e.g., the 'cortex-a*' CPUs) implement neither, which
should prevent the RngDxe driver from dispatching entirely, resulting
in the same situation as before.

Cc: Ard Biesheuvel <[email protected]>
Cc: Leif Lindholm <[email protected]>
Cc: Sami Mujawar <[email protected]>
Cc: Gerd Hoffmann <[email protected]>

Committed-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Doug Flick [MSFT] <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
@lgao4 lgao4 force-pushed the fix/NetworkPkg/bugzilla/4541_and_4542 branch from 02c64ff to 30b237f Compare May 24, 2024 14:50
@lgao4 lgao4 added the push Auto push patch series in PR if all checks pass label May 24, 2024
@mergify mergify bot merged commit 3e72240 into tianocore:master May 24, 2024
125 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
push Auto push patch series in PR if all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants