From c5415b93ad532a37b1ccdebb1727ae42719d06d6 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Thu, 7 May 2026 11:13:31 -0700 Subject: [PATCH 1/5] CryptoPkg: Fix OneCrypto protocol NULL guard logic Use logical OR in the protocol and function pointer null checks in the dispatch macros. This avoids null dereference risk and correctly handles either missing protocol or missing service function pointers. Signed-off-by: Doug Flick --- CryptoPkg/Library/BaseCryptLibOnOneCrypto/OneCryptoLib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CryptoPkg/Library/BaseCryptLibOnOneCrypto/OneCryptoLib.c b/CryptoPkg/Library/BaseCryptLibOnOneCrypto/OneCryptoLib.c index 20e5adc029c..644ff6c6fb5 100644 --- a/CryptoPkg/Library/BaseCryptLibOnOneCrypto/OneCryptoLib.c +++ b/CryptoPkg/Library/BaseCryptLibOnOneCrypto/OneCryptoLib.c @@ -152,7 +152,7 @@ CryptoServiceNotAvailable ( ONE_CRYPTO_PROTOCOL *CryptoServices; \ \ CryptoServices = GetAndValidateCryptoProtocol (#Function, (MinMajor), (MinMinor)); \ - if ((CryptoServices == NULL) && (CryptoServices->Function == NULL)) { \ + if ((CryptoServices == NULL) || (CryptoServices->Function == NULL)) { \ CryptoServiceNotAvailable (#Function); \ return ErrorReturnValue; \ } \ @@ -178,7 +178,7 @@ CryptoServiceNotAvailable ( ONE_CRYPTO_PROTOCOL *CryptoServices; \ \ CryptoServices = GetAndValidateCryptoProtocol (#Function, (MinMajor), (MinMinor)); \ - if ((CryptoServices == NULL) && (CryptoServices->Function == NULL)) { \ + if ((CryptoServices == NULL) || (CryptoServices->Function == NULL)) { \ CryptoServiceNotAvailable (#Function); \ return; \ } \ From 56de51a23a41a42311f47075381c0584e34967b6 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Fri, 8 May 2026 18:19:29 -0700 Subject: [PATCH 2/5] CryptoPkg: Add X509ConstructCertificateStack unit tests Add 8 new unit tests for X509ConstructCertificateStack and X509ConstructCertificateStackV to improve coverage of the X509 certificate stack construction API: - NullInput: NULL X509Stack pointer returns FALSE - SingleCert: construct stack with one CA certificate - Append: build stack incrementally across multiple calls - InvalidCert: truncated DER data fails without losing the stack - MultipleCertsOneCall: three certs in a single variadic call - ZeroSize: cert with size 0 fails gracefully - EmptyList: NULL-only terminator succeeds - VDirect: two certs exercising the VA_LIST code path Also fix the test class string for TestVerifyX509 from "CryptoPkg.BaseCryptLib.Hkdf" to "CryptoPkg.BaseCryptLib.X509". Signed-off-by: Doug Flick --- .../UnitTest/Library/BaseCryptLib/X509Tests.c | 191 +++++++++++++++++- 1 file changed, 190 insertions(+), 1 deletion(-) diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/X509Tests.c b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/X509Tests.c index 1141821daa2..32100d1b260 100644 --- a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/X509Tests.c +++ b/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/X509Tests.c @@ -626,11 +626,200 @@ TestVerifyX509 ( return UNIT_TEST_PASSED; } +UNIT_TEST_STATUS +EFIAPI +TestX509ConstructCertificateStackNullInput ( + IN UNIT_TEST_CONTEXT Context + ) +{ + BOOLEAN Status; + + Status = X509ConstructCertificateStack (NULL, mTestCaCert, sizeof (mTestCaCert), NULL); + UT_ASSERT_TRUE (!Status); + + return UNIT_TEST_PASSED; +} + +UNIT_TEST_STATUS +EFIAPI +TestX509ConstructCertificateStackSingleCert ( + IN UNIT_TEST_CONTEXT Context + ) +{ + BOOLEAN Status; + UINT8 *X509Stack; + + X509Stack = NULL; + Status = X509ConstructCertificateStack (&X509Stack, mTestCaCert, sizeof (mTestCaCert), NULL); + UT_ASSERT_TRUE (Status); + UT_ASSERT_TRUE (X509Stack != NULL); + + X509StackFree (X509Stack); + return UNIT_TEST_PASSED; +} + +UNIT_TEST_STATUS +EFIAPI +TestX509ConstructCertificateStackAppend ( + IN UNIT_TEST_CONTEXT Context + ) +{ + BOOLEAN Status; + UINT8 *X509Stack; + + X509Stack = NULL; + + Status = X509ConstructCertificateStack (&X509Stack, mTestCaCert, sizeof (mTestCaCert), NULL); + UT_ASSERT_TRUE (Status); + UT_ASSERT_TRUE (X509Stack != NULL); + + Status = X509ConstructCertificateStack (&X509Stack, mTestCert, sizeof (mTestCert), NULL); + UT_ASSERT_TRUE (Status); + UT_ASSERT_TRUE (X509Stack != NULL); + + Status = X509ConstructCertificateStack (&X509Stack, mTestEndCert, sizeof (mTestEndCert), NULL); + UT_ASSERT_TRUE (Status); + UT_ASSERT_TRUE (X509Stack != NULL); + + X509StackFree (X509Stack); + return UNIT_TEST_PASSED; +} + +UNIT_TEST_STATUS +EFIAPI +TestX509ConstructCertificateStackInvalidCert ( + IN UNIT_TEST_CONTEXT Context + ) +{ + BOOLEAN Status; + UINT8 *X509Stack; + + X509Stack = NULL; + Status = X509ConstructCertificateStack (&X509Stack, mTestCaCert, sizeof (mTestCaCert), NULL); + UT_ASSERT_TRUE (Status); + + Status = X509ConstructCertificateStack (&X509Stack, mTestCert, 8, NULL); + UT_ASSERT_TRUE (!Status); + UT_ASSERT_TRUE (X509Stack != NULL); + + X509StackFree (X509Stack); + return UNIT_TEST_PASSED; +} + +UNIT_TEST_STATUS +EFIAPI +TestX509ConstructCertificateStackMultipleCertsOneCall ( + IN UNIT_TEST_CONTEXT Context + ) +{ + BOOLEAN Status; + UINT8 *X509Stack; + + // + // Pass multiple cert/size pairs in a single variadic call. + // + X509Stack = NULL; + Status = X509ConstructCertificateStack ( + &X509Stack, + mTestCaCert, + sizeof (mTestCaCert), + mTestCert, + sizeof (mTestCert), + mTestEndCert, + sizeof (mTestEndCert), + NULL + ); + UT_ASSERT_TRUE (Status); + UT_ASSERT_TRUE (X509Stack != NULL); + + X509StackFree (X509Stack); + return UNIT_TEST_PASSED; +} + +UNIT_TEST_STATUS +EFIAPI +TestX509ConstructCertificateStackZeroSize ( + IN UNIT_TEST_CONTEXT Context + ) +{ + BOOLEAN Status; + UINT8 *X509Stack; + + // + // A cert with size 0 should fail without corrupting the stack pointer. + // + X509Stack = NULL; + Status = X509ConstructCertificateStack (&X509Stack, mTestCaCert, (UINTN)0, NULL); + UT_ASSERT_TRUE (!Status); + + return UNIT_TEST_PASSED; +} + +UNIT_TEST_STATUS +EFIAPI +TestX509ConstructCertificateStackEmptyList ( + IN UNIT_TEST_CONTEXT Context + ) +{ + BOOLEAN Status; + UINT8 *X509Stack; + + // + // Passing only NULL terminator should succeed with an empty stack. + // + X509Stack = NULL; + Status = X509ConstructCertificateStack (&X509Stack, NULL); + UT_ASSERT_TRUE (Status); + + if (X509Stack != NULL) { + X509StackFree (X509Stack); + } + + return UNIT_TEST_PASSED; +} + +UNIT_TEST_STATUS +EFIAPI +TestX509ConstructCertificateStackVDirect ( + IN UNIT_TEST_CONTEXT Context + ) +{ + BOOLEAN Status; + UINT8 *X509Stack; + + // + // Call the V variant through the public Stack wrapper to exercise + // the VA_LIST path with two certs. + // + X509Stack = NULL; + Status = X509ConstructCertificateStack ( + &X509Stack, + mTestCaCert, + sizeof (mTestCaCert), + mTestCert, + sizeof (mTestCert), + NULL + ); + UT_ASSERT_TRUE (Status); + UT_ASSERT_TRUE (X509Stack != NULL); + + X509StackFree (X509Stack); + return UNIT_TEST_PASSED; +} + TEST_DESC mX509Test[] = { // // -----Description--------------------------------------Class----------------------Function---------------------------------Pre---------------------Post---------Context // - { "TestVerifyX509()", "CryptoPkg.BaseCryptLib.Hkdf", TestVerifyX509, NULL, NULL, NULL }, + { "TestVerifyX509()", "CryptoPkg.BaseCryptLib.X509", TestVerifyX509, NULL, NULL, NULL }, + { "TestX509ConstructCertificateStackNullInput()", "CryptoPkg.BaseCryptLib.X509", TestX509ConstructCertificateStackNullInput, NULL, NULL, NULL }, + { "TestX509ConstructCertificateStackSingleCert()", "CryptoPkg.BaseCryptLib.X509", TestX509ConstructCertificateStackSingleCert, NULL, NULL, NULL }, + { "TestX509ConstructCertificateStackAppend()", "CryptoPkg.BaseCryptLib.X509", TestX509ConstructCertificateStackAppend, NULL, NULL, NULL }, + { "TestX509ConstructCertificateStackInvalidCert()", "CryptoPkg.BaseCryptLib.X509", TestX509ConstructCertificateStackInvalidCert, NULL, NULL, NULL }, + { "TestX509ConstructCertificateStackMultipleCertsOneCall()", "CryptoPkg.BaseCryptLib.X509", TestX509ConstructCertificateStackMultipleCertsOneCall, NULL, NULL, NULL }, + { "TestX509ConstructCertificateStackZeroSize()", "CryptoPkg.BaseCryptLib.X509", TestX509ConstructCertificateStackZeroSize, NULL, NULL, NULL }, + { "TestX509ConstructCertificateStackEmptyList()", "CryptoPkg.BaseCryptLib.X509", TestX509ConstructCertificateStackEmptyList, NULL, NULL, NULL }, + { "TestX509ConstructCertificateStackVDirect()", "CryptoPkg.BaseCryptLib.X509", TestX509ConstructCertificateStackVDirect, NULL, NULL, NULL }, }; UINTN mX509TestNum = ARRAY_SIZE (mX509Test); From 1c734ad7f053929312b5b4220ec2bcff80dbb5ef Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Fri, 8 May 2026 18:27:23 -0700 Subject: [PATCH 3/5] CryptoPkg: Fix variadic dispatch in X509ConstructCertificateStackV On AARCH64, VA_LIST is incompatible across GCC5 (ELF/AAPCS64) and CLANGPDB (PE/COFF/MSVC) toolchains. GCC5 uses a 32-byte struct tracking register and stack save areas, while CLANGPDB uses a simple char pointer. Passing VA_LIST through the OneCrypto protocol boundary causes the binary to misinterpret arguments - reading the cert pointer value as the cert size. Replace the direct CALL_CRYPTO_SERVICE dispatch of X509ConstructCertificateStackV with a caller-side loop that unpacks the VA_LIST locally (same toolchain, safe) and calls the non-variadic X509ConstructCertificateStack protocol function once per certificate. Each call passes a single cert/size pair terminated by NULL, avoiding any VA_LIST crossing the cross-toolchain protocol boundary. Signed-off-by: Doug Flick --- .../BaseCryptLibOnOneCrypto/OneCryptoLib.c | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/CryptoPkg/Library/BaseCryptLibOnOneCrypto/OneCryptoLib.c b/CryptoPkg/Library/BaseCryptLibOnOneCrypto/OneCryptoLib.c index 644ff6c6fb5..28c5fec9860 100644 --- a/CryptoPkg/Library/BaseCryptLibOnOneCrypto/OneCryptoLib.c +++ b/CryptoPkg/Library/BaseCryptLibOnOneCrypto/OneCryptoLib.c @@ -4457,7 +4457,39 @@ X509ConstructCertificateStackV ( IN VA_LIST Args ) { - CALL_CRYPTO_SERVICE (X509ConstructCertificateStackV, (X509Stack, Args), FALSE, 1, 0); + ONE_CRYPTO_PROTOCOL *CryptoServices; + UINT8 *Cert; + UINTN CertSize; + BOOLEAN Status; + + if (X509Stack == NULL) { + return FALSE; + } + + CryptoServices = GetAndValidateCryptoProtocol ("X509ConstructCertificateStack", 1, 0); + if ((CryptoServices == NULL) || (CryptoServices->X509ConstructCertificateStack == NULL)) { + CryptoServiceNotAvailable ("X509ConstructCertificateStack"); + return FALSE; + } + + Status = TRUE; + Cert = VA_ARG (Args, UINT8 *); + while (Cert != NULL) { + CertSize = VA_ARG (Args, UINTN); + if (CertSize == 0) { + Status = FALSE; + break; + } + + Status = CryptoServices->X509ConstructCertificateStack (X509Stack, Cert, CertSize, NULL); + if (!Status) { + break; + } + + Cert = VA_ARG (Args, UINT8 *); + } + + return Status; } /** @@ -4488,10 +4520,12 @@ X509ConstructCertificateStack ( ) { VA_LIST Args; + BOOLEAN Result; VA_START (Args, X509Stack); - CALL_CRYPTO_SERVICE (X509ConstructCertificateStack, (X509Stack, Args), FALSE, 1, 0); + Result = X509ConstructCertificateStackV (X509Stack, Args); VA_END (Args); + return Result; } /** From c19dcbaa878c0343014eeb90a901dc1bc773590a Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Fri, 8 May 2026 18:29:50 -0700 Subject: [PATCH 4/5] CryptoPkg: Add id field to OneCrypto ext_dep descriptor Add the missing "id" field to OneCrypto_ext_dep.json. This field is required by the edk2-pytool ext_dep override mechanism to allow downstream platforms to override the OneCrypto binary dependency using "override_id" in a local ext_dep file. Signed-off-by: Doug Flick --- CryptoPkg/Binaries/OneCrypto_ext_dep.json | 1 + 1 file changed, 1 insertion(+) diff --git a/CryptoPkg/Binaries/OneCrypto_ext_dep.json b/CryptoPkg/Binaries/OneCrypto_ext_dep.json index 886d55d33f9..9e4dfdff92e 100644 --- a/CryptoPkg/Binaries/OneCrypto_ext_dep.json +++ b/CryptoPkg/Binaries/OneCrypto_ext_dep.json @@ -1,6 +1,7 @@ { "scope": "global", "type": "web", + "id": "onecrypto-bin", "name": "onecrypto-bin", "source": "https://github.com/microsoft/mu_crypto_release/releases/download/v1.0.0-OneCrypto/OneCrypto-Accelerated.zip", "version": "1.0.0", From 951bb85b0f9220fd088ddf29cf39ade590f556e2 Mon Sep 17 00:00:00 2001 From: Doug Flick Date: Wed, 13 May 2026 15:55:18 -0700 Subject: [PATCH 5/5] CryptoPkg: Update OneCrypto external dependency to v1.0.1 Update the OneCrypto binary external dependency from v1.0.0 to v1.0.1 and update the corresponding SHA256 hash. Signed-off-by: Doug Flick --- CryptoPkg/Binaries/OneCrypto_ext_dep.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CryptoPkg/Binaries/OneCrypto_ext_dep.json b/CryptoPkg/Binaries/OneCrypto_ext_dep.json index 9e4dfdff92e..40d1093fbc5 100644 --- a/CryptoPkg/Binaries/OneCrypto_ext_dep.json +++ b/CryptoPkg/Binaries/OneCrypto_ext_dep.json @@ -3,9 +3,9 @@ "type": "web", "id": "onecrypto-bin", "name": "onecrypto-bin", - "source": "https://github.com/microsoft/mu_crypto_release/releases/download/v1.0.0-OneCrypto/OneCrypto-Accelerated.zip", - "version": "1.0.0", - "sha256": "dbca1dd1e8410df574e5d3cdf258adbc94b660a3844157d1421f668d71164ec2", + "source": "https://github.com/microsoft/mu_crypto_release/releases/download/v1.0.1-OneCrypto/OneCrypto-Accelerated.zip", + "version": "1.0.1", + "sha256": "4f7ff3f6483c35f9168c67b28a462353f138115a3cd024551d80525550cd54b1", "compression_type": "zip", "internal_path": "/", "flags": ["set_build_var"],