Hi Yao, Sorry for the late response - my team and I are at UEFI Plugfest in Redmond this week. Thanks for reviewing our patches, we will start addressing your comments right after the event.
Yes, the original big patch was split into several smaller ones per feature and per affected package, and also applied feedback received from community so far. I sent 4 small patches and we have the GS/RTC patch almost ready to submit, so you can disregard the original big patch. Our new GS/RTC patch currently supports MSVC and GCC. We (or others) can iteratively add the other compilers, e.g. we were looking to add RVCT next, but I think we need to establish some baseline, so adding compilers is easy. Regards ek -----Original Message----- From: Yao, Jiewen [mailto:jiewen....@intel.com] Sent: Saturday, July 14, 2012 4:48 PM To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] [PATCH] NX/ASLR/GS/RTCs Security Features Hi Eugene Thanks a lot for your patch. It seems big feature, so it takes me some time to review. I have read all you patch you sent separately later. Below is some of my personal comment: (Combine them together) 1) GS/RTC This feature is compiler specific one. E.g. GCC/ICC has another compile option -fstack-protector, and we need provide stub for __stack_chk_guard, __stack_chk_fail(). So basically, I suggest we create a standalone Pkg, or at least standalone Library, like CompilerStackCheckLib, with NULL library class. Do we have official document on implementation for each compiler? 2.1) ASLR - RandomLib It seems we need a RandomLib. So the best way I think, is to create another RandomLib library class. So we can have openssl instance, ISAAC instance, or any other. I do not suggest to modify and update BaseLib. Per definition BaseLib provides string functions, linked list functions, math functions, synchronization, functions, and CPU architecture-specific functions, while RandomLib is cryptography related. People may want to use his/her own crypto library. I attach a RandomLib.h file for review. 2.2) ASLR - Randomization in core. In the patch, I found it randomized the PE image loading. While I thought it might be better if we also randomize the data, such as page table, IDT entry. So my suggest is that we also update AllocatePages() implementation to allocate more random pages, and free used ones later. BTW: A quick question: have you validated S4 resume for your patch? I am not sure if we are OK to randomize the runtime service code or runtime service data for OS S4 resume? 3.1) NX - PageTableLib That is probably the most difficult part. Your patch seems a good start. I hope to create page table lib for multiple purpose and usage, for example, code can use page table lib to create identitymapping page table (needed by ACPI, S3, DxeIpl, SMM), code can use page table lib to set/clear P/RW/NX memory, or cache attribute (needed by NX feature later by CPU driver, and SMM driver), code can use page table lib to manage add/remove page (needed by SMM driver) Based on those, I draft a new PageTableLib.h attached for review. 3.2) NX - Set NX in core Basically, I think core does not need page table information or link page table lib, because it can use CPU protocol to SetMemoryAttribute(). Currently DXE has CPU_ARCH protocol, I created one for SMM. (consumed by SMM_CORE, and produced by SMM_CPU driver). I am not sure if we need set NX at PEI phase. Probably not, because it should be all OEM code. If we do need one, we can create another one for PEI to let PEI_CORE consume, and PEI_CPU produce. Yes, there is gap between Core initialization and CPU protocol ready, but I think the gap is small. Core can register a notify to set all protection when CPU protocol ready later. Thank you Yao Jiewen "The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter." -----Original Message----- From: Eugene Khoruzhenko [mailto:eugene_khoruzhe...@phoenix.com] Sent: Thursday, May 17, 2012 10:35 AM To: edk2-devel@lists.sourceforge.net Subject: [edk2] [PATCH] NX/ASLR/GS/RTCs Security Features Dear EDK2 MdeModulePkg maintainer and community, Please find the attached patch for the NX/ASLR/GS/RTCs features. Adding these features provides blanket security protection for latent vulnerabilities. The NX feature uses page tables and DXE memory management to mark pages containing data (or that do not contain code) as No Execute, causing a page fault if there is any attempt to execute code from those pages. This is to prevent code that exploits buffer overruns from including the code to be executed directly in the buffer overrun; for example, NX prevents code on the stack from being executed. This feature is implemented as a pair of libraries under MdePkg, one of which is BasePageTableLib stub library, and the other is a full implementation of the page table library for IA32E - BasePageTableLibIA32E. Integration involves changes to DxeCore and DxeIplPeim, as well as a bunch of changes to platform and silicon code to enable NXP in AP processors and SMM (not explicitly included with this patch). The ASLR feature causes PE images that are loaded to RAM to be loaded at randomized addresses. The intent is to prevent code that exploits stack buffer overruns from being able to use return oriented code from exploiting code loaded at known or fixed locations. This feature is implemented as a library that provides a randomization function called BaseBinSecurityLib. Integration involves changes to PeiCore, DxeIplPeim, DxeCore and SmmCore. GS and RTCs are to support VS2010 build with /GS and /RTCs switches enabled. Note that the /GS switch is only secure when ASLR is enabled, as we leverage ASLR's randomizing of the address of loaded code to automatically initialize the security cookie. Rather than setting the security cookie randomly in the program entrypoint code, we let PE loader set the security cookie value to the address of an arbitrarily selected function within BaseBinSecurityLib, and that address is random as a side effect of ASLR. This way, we don't have to link the full randomization code into every single driver or application. Regards, Eugene Khoruzhenko Principal Software Architect Phoenix Technologies Ltd. (425) 443-3883 ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
