[Bug]: MfciDxe: ValidateBlobWithXdrCertificates - XDR certificate handling assumes the PublicKeyDataXdr buffer is 4-byte aligned #814

Is there an existing issue for this?

• I have searched existing issues

Current Behavior

The current implementation for XDR certificate handling assumes the PublicKeyDataXdr buffer is 4-byte aligned. This is not always the case. For example, if PublicKeyDataXdr=0x02, and the length is 0x05, the correct offset should be 0x2 + align_up(0x5, 4) -> 0x2 + 0x8 = 0xA. The current logic produces align_up(0x2 + 0x5, 4) -> align_up(0x7, 4) = 0x8.

RFC XDR reference on 4-byte alignment. https://www.rfc-editor.org/rfc/rfc4506#page-16

   (4) Why is the XDR unit four bytes wide?

   There is a tradeoff in choosing the XDR unit size.  Choosing a small
   size, such as two, makes the encoded data small, but causes alignment
   problems for machines that aren't aligned on these boundaries.  A
   large size, such as eight, means the data will be aligned on
   virtually every machine, but causes the encoded data to grow too big.
   We chose four as a compromise.  Four is big enough to support most
   architectures efficiently, except for rare machines such as the
   eight-byte-aligned Cray*.  Four is also small enough to keep the
   encoded data restricted to a reasonable size.

Expected Behavior

The XDR certificate parsing is able to properly analyze an XDR certificate buffer with multiple certificates.

Steps To Reproduce

1. Attempt to utilize MFCI feature
2. The second certificate is the certificate needed for validation
3. XDR verify fails.
   00:00:07.009 : [DXE ] [ERR ] ValidateBlobWithXdrCertificates: Certificate size extends beyond end of PCD, skipping it.
   00:00:07.010 : [DXE ] [ERR ] VerifyPolicyAndChange - Target blob validation failed - Aborted.

Build Environment

- OS(s): Windows, Ubuntu 24.02
- Tool Chain(s): CLANGPDB
- Targets Impacted: RELEASE, DEBUG

Version Information

latest release/202502 commit.
f84d42d04cec07b8f76c585d55825e5b01b1d813

Urgency

Low

Are you going to fix this?

I will fix it

Do you need maintainer feedback?

No maintainer feedback needed

Anything else?

For example, here's the current logic on a simplified design.

PublicKeyDataXdr += PublicKeyDataLength;
PublicKeyDataXdr  = (UINT8 *)ALIGN_POINTER (PublicKeyDataXdr, sizeof (UINT32));

Assume the pointer is set to an unaligned value 0x02 and the data length is 0x05.

PublicKeyDataXdr = 0x02;
PublicKeyDataLength = 0x05;

PublicKeyDataXdr = 0x02 + 0x05; // => 0x07
PublicKeyDataXdr = (UINT8 *)ALIGN_POINTER (0x07, 4); // => 0x08

This design is incorrect, as the expected value for the end of the certificate should be 0xA.
PublicKeyDataXdr += ALIGN_VALUE (PublicKeyDataLength, 4);

PublicKeyDataXdr = 0x02;
PublicKeyDataLength = 0x05;

PublicKeyDataXdr = 0x02 + ALIGN_VALUE (0x05, 4); // => 0x02 + 0x08 = 0x0A

The initial value of PublicKeyDataXdr is not being correctly accounted for in the original math, since the pointer alignment loses it as part of the alignment.