On 06/23/15 10:43, Gary Ching-Pang Lin wrote:
> On Tue, Jun 23, 2015 at 10:07:36AM +0200, Laszlo Ersek wrote:
>> On 06/23/15 04:25, Gary Ching-Pang Lin wrote:
>>> On Mon, Jun 22, 2015 at 02:24:55PM -0400, Peter Jones wrote:
>>>> On Sat, Jun 20, 2015 at 03:01:17PM +0200, Ard Biesheuvel wrote:
>>>>
>>>>> I wonder what is going on here. My AArch64 boot tests work fine
>>>>> with these patches applied, but they don't use shim. (They do use
>>>>> GRUB as an intermediate loader calling LoadImage() to boot a
>>>>> signed kernel).
>>>>>
>>>>> Are there any plans or patches yet to move shim to a more recent
>>>>> OpenSSL version? It shouldn't be affecting things like this, but
>>>>> it would allow a quick check if someone has patches already.
>>>>
>>>> Yes, there's a plan to do so - Gary Lin has had a patch in progress
>>>> and was waiting for this patch to hit before sending it to me. I
>>>> expect to see it any time. (I would not be surprised if he's
>>>> trying to debug an analog to this exact same issue...)
>>>>
>>> I'm currently busy with other things so the update in shim may be
>>> delayed for a while.
>>>
>>>> That said, it's unclear to me how shim being on a prior openssl
>>>> version would cause the problem Laszlo is seeing - there's no
>>>> cross-linkage of any kind between the two openssl builds in memory.
>>>>
>>> shim and grub2 are using the openssl lib independent from the one in
>>> firmware, so it surprised me the openssl update patches broke the
>>> bootloaders. I just tested OVMF R17650 with openSUSE 13.2 and
>>> everything went well. The shim we use in openSUSE 13.2 is 0.7 + a
>>> series of patches (most of them are upstream patches). Hope this
>>> could narrow down the issue.
>>
>> Huh. R17650 is past the openssl-1.0.2c update.
>>
>> Can you please give me (or link for me) the install media for
>> openSUSE 13.2? I'd like to try it.
>>
> openSUSE 13.2 is available in:
> https://software.opensuse.org/132/
>
> Maybe you could download the shim first and test it in a vfat dir
> (ex: -hda fat:hda-contents/)? Just put shim.efi and MokManager.efi
> in the same directory, and shim will load MokManager when there is
> no grub2. If it works, then put grub.efi in the same directory and
> see whether the grub console shows or not. It would be easier to
> identify the problem.
>
> The openSUSE shim is available in
> https://build.opensuse.org/package/binaries/openSUSE:13.2/shim?repository=standard
Thanks, I tested this. It fails the same way for me as my earlier test.
Luckily though, I happened to have the serial console of the VM open at
this point, and I got the following exception dump there:
FS1:\opensuse\usr\lib64\efi\> shim.efi
!!!! X64 Exception Type - 000000000000000D CPU Apic ID - 00000000 !!!!
RIP - 000000001FDEFDF3, CS - 0000000000000028, RFLAGS - 0000000000010206
ExceptionData - 0000000000000000
RAX - 1FF5FF7400000000, RCX - 1FF5FF7400000000, RDX - 0000000000000004
RBX - 000000001FDF430E, RSP - 000000001FF5FEB0, RBP - 000000001FF5FEE0
RSI - 0000000000000000, RDI - 0000000000000051
R8 - 000000001EE66458, R9 - 000000001EE66458, R10 - 00000000FFFFFFFF
R11 - 0000000000000000, R12 - 0000000000000000, R13 - 0000000000000000
R14 - 0000000000000000, R15 - 0000000000000000
DS - 0000000000000008, ES - 0000000000000008, FS - 0000000000000008
GS - 0000000000000008, SS - 0000000000000008
CR0 - 0000000080000033, CR2 - 0000000000000000, CR3 - 000000001FEFF000
CR4 - 0000000000000668, CR8 - 0000000000000000
DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
GDTR - 000000001FEED7D8 000000000000003F, LDTR - 0000000000000000
IDTR - 000000001F712018 0000000000000FFF, TR - 0000000000000000
FXSAVE_STATE - 000000001FF5FB10
!!!! Find PE image
.../Build/OvmfX64/DEBUG_GCC48/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.dll
(ImageBase=000000001FDEA000, EntryPoint=000000001FDEA2AF) !!!!
I quickly repeated my original, Fedora-based test (now looking at the
serial console too), and it fails with the same exception.
The crash itself seems to be inside AsciiStrLen() -- based on RIP,
ImageBase, EntryPoint --, but I don't see the call stack that leads up
to it.
...
Okay, after a few more hours of struggle, I found the bug. It's the
usual VA_START <-> EFIAPI problem.
As discussed earlier (on several occasions), in practice *all* code in
edk2 that uses VA_START() *must* be EFIAPI. Refer to the third
VA_START() macro definition in "MdePkg/Include/Base.h". That macro
definition *depends* on the EFIAPI calling convention. This is violated
by the openssl library.
The call stack that I reconstructed (with manual debug messages) goes
like this, when edk2 tries to validate the shim binary:
Security2StubAuthenticate()
[MdeModulePkg/Universal/SecurityStubDxe/SecurityStub.c]
ExecuteSecurity2Handlers()
[MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.c]
// via function pointer
// "Security2Handler"
DxeImageVerificationHandler
[SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c]
IsAllowedByDb()
[SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c]
AuthenticodeVerify()
[CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c]
Pkcs7Verify()
[CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c]
PKCS7_verify()
[CryptoPkg/Library/OpensslLib/openssl-1.0.2c/crypto/pkcs7/pk7_smime.c]
ERR_add_error_data()
[CryptoPkg/Library/OpensslLib/openssl-1.0.2c/crypto/err/err.c]
ERR_add_error_vdata()
[CryptoPkg/Library/OpensslLib/openssl-1.0.2c/crypto/err/err.c]
strlen() BOOM
More closely, look at the following in the PKCS7_verify() function:
323 i = X509_verify_cert(&cert_ctx);
324 if (i <= 0)
325 j = X509_STORE_CTX_get_error(&cert_ctx);
326 X509_STORE_CTX_cleanup(&cert_ctx);
327 if (i <= 0) {
328 PKCS7err(PKCS7_F_PKCS7_VERIFY,
329 PKCS7_R_CERTIFICATE_VERIFY_ERROR);
330 ERR_add_error_data(2, "Verify error:",
331 X509_verify_cert_error_string(j));
332 sk_X509_free(signers);
333 return 0;
334 }
The verification step on line 323 fails. (I don't know why, but I guess
shim.efi could still be accepted by other means -- we are only inside
IsAllowedByDb(), and there are other possibilities for a binary to pass
the check.)
In any case, the variable "j" is assigned. The error message that "j"
corresponds to is:
unable to get local issuer certificate
I printed this error string separately, and it is all right per se. The
function call that blows up is on line 330, the ERR_add_error_data()
one.
Namely:
1075 void ERR_add_error_data(int num, ...)
1076 {
1077 va_list args;
1078 va_start(args, num);
1079 ERR_add_error_vdata(num, args);
1080 va_end(args);
1081 }
This function uses va_start(), which, according to
"CryptoPkg/Include/OpenSslSupport.h", gets turned into VA_START().
However, the function does not apply the EFIAPI calling convention, and
that causes it to blow up.
In PKCS7_verify(), before the call to ERR_add_error_data(), I extracted
the string "Verify error:" as:
STATIC CONST CHAR8 err1[] = "Verify error:";
and then I printed the *address* of err1, before passing err1 to
ERR_add_error_data(). It was: 0x3FE6CC98, a valid address.
Inside ERR_add_error_data() --> ERR_add_error_vdata(), we have:
1083 void ERR_add_error_vdata(int num, va_list args)
1084 {
1085 int i, n, s;
1086 char *str, *p, *a;
1087
1088 s = 80;
1089 str = OPENSSL_malloc(s + 1);
1090 if (str == NULL)
1091 return;
1092 str[0] = '\0';
1093
1094 n = 0;
1095 for (i = 0; i < num; i++) {
1096 a = va_arg(args, char *);
1097 /* ignore NULLs, thanks to Bob Beck <[email protected]> */
1098 if (a != NULL) {
1099 n += strlen(a);
The function retrieves the address of the string, and calls strlen() on
it -- which corresponds to AsciiStrLen(), again according to
OpenSslSupport.h.
Now, just before the call to strlen(), I printed the address stored "a"
*again* -- and it was 0x3FF6_0474_0000_0000.
That address is consistent with the exception dump I'm getting right
after on the serial console; the address is visible in both RAX and RCX.
Plus, the call stack that I tracked down is consistent with the
exception RIP identifying AsciiStrLen().
Summary: while storing the error message from the X509 certificate
verification, things blow up, because ERR_add_error_data() is a varargs
function, but lacks EFIAPI, hence turns its arguments into garbage.
Now, why did this use to work with 0.9.8zf? Easy; just compare the
following two files:
- CryptoPkg/Library/OpensslLib/openssl-0.9.8zf/crypto/err/err.c
- CryptoPkg/Library/OpensslLib/openssl-1.0.2c/crypto/err/err.c
In 0.9.8zf, the prototype of the ERR_add_error_data() function looks
like this:
324 /* Add EFIAPI for UEFI version. */
325 #if defined(OPENSSL_SYS_UEFI)
326 void EFIAPI ERR_add_error_data(int num, ...)
327 #else
328 void ERR_add_error_data(int num, ...)
329 #endif
330 {
331 va_list args;
332 int i, n, s;
333 char *str, *p, *a;
And that's gone in 1.0.2c.
The bug is in:
commit f93f78ea70f14f0ca17e122588b85f947adfa569
Author: Qin Long <[email protected]>
Date: Tue Jun 16 00:52:17 2015 +0000
CryptoPkg: Update openssl patch file from 0.9.8zf to 1.0.2c
This patch adds a patch file for openssl-1.0.2c, and removes
the patch file for openssl-0.9.8zf.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@17633
6f19259b-4bc3-4df7-8a09-765794883524
Because
CryptoPkg/Library/OpensslLib/EDKII_openssl-0.9.8zf.patch
used to have
a hunk that added the EFIAPI declspec to ERR_add_error_data(), but
CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2c.patch
dropped that hunk.
I'll post a patch soon.
Thanks
Laszlo
------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors
network devices and physical & virtual servers, alerts via email & sms
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
