[PATCH 1/2] Add support for GPG signature enforcement on booted
Timothy Pearson
tpearson at raptorengineering.com
Wed Aug 3 13:00:42 AEST 2016
On 08/01/2016 11:00 PM, Samuel Mendoza-Jonas wrote:
> On Mon, 2016-08-01 at 12:09 -0500, Timothy Pearson wrote:
>> kernels and related blobs
>>
>> This can be used to implement a form of organization-controlled secure boot,
>> whereby kernels may be loaded from a variety of sources but they will only
>> boot if a valid signature file is found for each component, and only if the
>> signature is listed in the /etc/pb-lockdown file.
>
> Cool! This all looks pretty interesting - I have some general comments
> lower down, but in particular:
>
> - Some work with Secure Boot is also being done by Nayna Jain, but with
> a different approach. I don't think either version steps on the other's
> toes but something to look at.
> (http://patchwork.ozlabs.org/patch/629157/ etc)
Is there a more relevant link? The patch linked to does not seem
related to secure boot.
> - It looks like you've based your changes on and older/separate version
> of Petitboot. Current upstream (and especially for OpenPOWER) can be
> found here: http://git.ozlabs.org/?p=petitboot, and also at
> https://github.com/open-power/petitboot
I will update this once the general approach / implementation is
approved. When this patchset was created I was unaware that upstream
was hosted outside of the Linux kernel GIT.
> It's not immediately clear to me why the boot files need to be copied to
> tmp before verifying them. Is there some quirk of gpgme that requires it?
A previous poster is correct -- this is done to prevent timing attacks
on the signed files.
> Do you mean to return here?
Definitely. These arrors (and others) have been corrected in the V2
patches sent just now.
> Any particular reason for this size?
No, it's mostly a first-pass balance between slow read / write and
memory use.
> '0' or '\0'?
Corrected.
> I think there's officially enough code depending on libgpgme that you
> could justify moving it into it's own .c file instead of interspersing
> it within boot.c. Then instead from wrapping everything in #if defines,
> you could decide somewhere further up whether to boot normally or with
> signatures (eg. by stat-ing /etc/pb-lockdown). Thoughts?
I removed some of the #if guards that weren't needed in the final
version. Is this something you would need done before a merge?
> I noticed in a few places increasing numbers are returned for errors, but
> it doesn't look like any callers check for specific numbers - do these
> have any particular meaning?
None whatsoever. They were used during development to determine the
exact bailout point of a failure.
> Style - we order variable declarations in decreasing size for clarity, eg:
>
> gpgme_signature_t verification_signatures;
> gpgme_verify_result_t verification_result;
> gpgme_engine_info_t enginfo;
> etc...
Done.
> return 1; ?
Fixed.
> Use talloc_strndup() instead of strdup()
Done.
> In other spots too, but keep to an 80-character limit
Fixed.
> The problem here is that while the "Exit to shell" button is replaced,
> users can still exit with 'x' or potentially some other key combination
> that causes ncurses to exit.
> Additionally it looks like the "Reboot" button just calls the exit
> callback anyway :)
Good point. The script we were launching petitboot from issued a hard
machine reset if petitboot-nc exited for any reason and as a result this
was overlooked.
Version 2 of this patch implements the following additional features:
* Linux reboot on UI termination
* Signature verification of Linux arguments
--
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645 (direct line)
+1 (512) 690-0200 (switchboard)
https://www.raptorengineering.com
More information about the Petitboot
mailing list