Permalink
Commits on Apr 15, 2015
  1. Explicitly request sysv-style ELF hash sections

    We depend on there being a .hash section in the binary, and that's not
    the case on distributions that default to building with gnu-style ELF
    hashes. Explicitly request sysv-style hashes in order to avoid building
    broken binaries.
    
    Signed-off-by: Matthew Garrett <mjg59@coreos.com>
    committed with vathpela Apr 15, 2015
Commits on Apr 13, 2015
  1. gcc 5.0 changes some include bits, so copy what arm does on x86.

    Basically they messed around with stdarg some and now we need to do it
    the other way.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Apr 7, 2015
  2. Make lib/ use the right CFLAGS.

    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Feb 25, 2015
  3. Make lib/ build right with the cflags it should be using...

    ... but isn't.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Feb 25, 2015
  4. Fix length of allocated buffer for boot option comparison.

    The following commit:
    
      commit 4aac8a1
      Author: Gary Ching-Pang Lin <glin@suse.com>
      Date:   Thu Mar 6 10:57:02 2014 +0800
    
        [fallback] Fix the data size for boot option comparison
    
    corrected the data size used for comparison, but also reduced the
    allocation so it doesn't include the trailing UTF16LE '\0\0' at the
    end of the string, with the result that the trailer of the buffer
    containing the string is overwritten, which OVMF detects as memory
    corruption.
    
    Increase the size of the storage buffer in a few places to correct
    this problem.
    
    Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
    Cc: Laszlo Ersek <lersek@redhat.com>
    Cc: Gary Ching-Pang Lin <glin@suse.com>
    lersek committed with vathpela Feb 25, 2015
  5. fallback: Fix comparison between signed and unsigned in debugging code.

    fallback.c: In function ‘update_boot_order’:
    fallback.c:334:17: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
      for (j = 0 ; j < size / sizeof (CHAR16); j++)
                       ^
    fallback.c: In function ‘add_to_boot_list’:
    fallback.c:402:16: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
      for (i = 0; i < s; i++) {
                      ^
    
    Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
    rwmjones committed with vathpela Feb 25, 2015
  6. Don't install our protocols if we're not in secure mode.

    System services haven't been hooked if we're not in secure mode, so
    do_exit() will never be called.  In this case shim never gets control
    once grub exits, which means if booting fails and the firmware tries
    another boot option, it'll attempt to talk to the shim protocol we
    installed.
    
    This is wrong, because it is allowed to have been cleared from ram at
    this time, since the task it's under has exited.
    
    So just don't install the protocols when we're not enforcing.
    
    This version also has a message and a 2-second stall after calling
    start_image(), so that we can tell if we are on the expected return path
    of our execution flow.
    vathpela committed Nov 25, 2014
  7. Align the sections we're loading, and check for validity /after/ disc…

    …arding.
    
    Turns out a) the codegen on aarch64 generates code that has real
    alignment needs, and b) if we check the length of discardable sections
    before discarding them, we error for no reason.
    
    So do the error checking in the right order, and always enforce some
    alignment because we know we have to.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Feb 6, 2015
Commits on Dec 11, 2014
  1. Add nostdinc to the CFLAGS for lib

    We don't need the headers from the standard include path.
    
    Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
    lcp committed with vathpela Nov 10, 2014
Commits on Oct 13, 2014
  1. Bump version to 0.8

    vathpela committed Oct 13, 2014
Commits on Oct 2, 2014
  1. Correctly reject bad tftp addresses earlier, rather than later.

    This check is for end == NULL but was meant to be *end == '\0'.  Without
    this change, we'll pass a plausibly bad address (i.e. one with no ']' at
    the end) to Mtftp(... READ_FILE ...), which should fail correctly, but
    our error messaging will be inconsistent.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 21, 2014
  2. Use -Werror=sign-compare .

    I'm going to have to fix any errors that have this anyway, so may as
    well do it here properly.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 21, 2014
  3. Make another integer compare be signed/unsigned safe as well.

    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 21, 2014
  4. Another testplan error.

    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Oct 2, 2014
  5. Cryptlib: remove the unused files

    I mistakenly added CryptPkcs7VerifyNull.c which may make Pkcs7Verify
    always return FALSE. Besides CryptPkcs7VerifyNull.c, there are some
    functions we would never use. This commit removes those files to
    avoid any potential trouble.
    
    Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
    lcp committed with vathpela Sep 30, 2014
  6. Don't verify images with the empty build key

    We replaced the build key with an empty file while compiling shim
    for our distro. Skip the verification with the empty build key
    since this makes no sense.
    
    Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
    lcp committed with vathpela Sep 30, 2014
  7. Fix some minor testplan errors.

    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Oct 2, 2014
  8. Don't append an empty cert list to MokListRT if vendor_cert_size is 0.

    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Oct 2, 2014
Commits on Oct 1, 2014
  1. Actually find the relocations correctly and process them that way.

    Find the relocations based on the *file* address in the old binary,
    because it's only the same as the virtual address some of the time.
    
    Also perform some extra validation before processing it, and don't bail
    out in /error/ if both ReloceBase and RelocEnd are null - that condition
    is fine.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 30, 2014
  2. Revert header changes

    Revert "Do the same for ia32..."
    and "Generate a sane PE header on shim, fallback, and MokManager."
    This reverts commit 6744a7e.
    and commit 0e7ba59.
    
    These are premature and I can do this without such drastic measures.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Oct 1, 2014
Commits on Sep 21, 2014
  1. Make list_keys() index variables all be signed.

    We build with -Werror=signed-compare in fedora/rhel rpms, and this
    showed up.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 21, 2014
  2. Do the same for ia32...

    Once again, on ia32 this time, we see:
    
    00000120  47 84 00 00 0a 00 00 00  00 00 00 00 00 00 00 00 |G...............|
    
    Which is where the pointer on ia32 for the Base Relocation Table should
    be.  It points to 0x8447, which isn't a particularly reasonable address as
    numbers go, and happens to have this data there:
    
    00008440  6f 00 6e 00 66 00 69 00  67 00 75 00 72 00 65 00 |o.n.f.i.g.u.r.e.|
    00008450  00 00 49 00 50 00 76 00  36 00 28 00 00 00 2c 00 |..I.P.v.6.(...,.|
    00008460  25 00 73 00 2c 00 00 00  29 00 00 00 25 00 64 00 |%.s.,...)...%.d.|
    00008470  2e 00 25 00 64 00 2e 00  25 00 64 00 2e 00 25 00 |..%.d...%.d...%.|
    00008480  64 00 00 00 44 00 48 00  43 00 50 00 00 00 49 00 |d...D.H.C.P...I.|
    00008490  50 00 76 00 34 00 28 00  00 00 2c 00 25 00 73 00 |P.v.4.(...,.%.s.|
    
    And so that table is, in theory, this part:
    
    00008447                       00  67 00 75 00 72 00 65 00 |       .g.u.r.e.|
    00008450  00                                               |.               |
    
    Which is pretty clearly not a pointer table of any kind.
    
    So give ia32 the same treatment as x86_64, and now all arches work basically
    the same.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 21, 2014
  3. Generate a sane PE header on shim, fallback, and MokManager.

    It turns out a7249a6 was masking a second problem - on some binaries,
    when we actually don't have any base relocations at all, binutils'
    "objcopy --target efi-app-x86_64" is generating a PE header with a base
    relocations pointer that happily points into the middle of our text
    section.  So with shim processing base relocations correctly, it refuses
    to load those binaries.
    
    For example, on one binary I just built:
    
    00000130  00 a0 00 00 0a 00 00 00  00 00 00 00 00 00 00 00 |................|
    
    which says there's a Base Relocation Table at 0xa000 that's 0xa bytes long.
    That's here:
    
    0000a000  58 00 29 00 00 00 00 00  48 00 44 00 28 00 50 00 |X.).....H.D.(.P.|
    0000a010  61 00 72 00 74 00 25 00  64 00 2c 00 53 00 69 00 |a.r.t.%.d.,.S.i.|
    0000a020  67 00 25 00 67 00 29 00  00 00 00 00 00 00 00 00 |g.%.g.).........|
    0000a030  48 00 44 00 28 00 50 00  61 00 72 00 74 00 25 00 |H.D.(.P.a.r.t.%.|
    
    So the table is:
    
    0000a000  58 00 29 00 00 00 00 00  48 00                   |X.).....H.      |
    
    That wouldn't be so bad, except those binaries are MokManager.efi,
    fallback.efi, and shim.efi, and sometimes they're .reloc, which we're
    actually trying to handle correctly now because grub builds with a real
    and valid .reloc table.  So though I didn't think there was any hair
    left on this yak, more shaving ensues.
    
    With this change, instead of letting objcopy do whatever it likes, we
    switch to "-O binary" and merely link in a header that's appropriate for
    our binaries.  This is the same method Ard wrote for aarch64, and it
    seems to work fine in either place (modulo some minor changes.)
    
    At some point this should be merged into gnu-efi instead of carrying our
    own crt0-efi-x86_64.S, but that's a less immediate problem.
    
    I did not need this problem.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 20, 2014
  4. Fix our "in_protocol" printing.

    When I merged 4bfb13d and fixed the conflicts, I managed to make the
    in_protocol test exactly backwards, so that's why we don't currently see
    error messages.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 19, 2014
  5. Don't call AuthenticodeVerify if vendor_cert_size is 0.

    Actually check the size of our vendor cert quite early, so that there's
    no confusion as to what's going on.
    
    This isn't strictly necessary, in that in all cases if vendor_cert_size
    is 0, then AuthenticodeVerify -> Pkcs7Verify() -> d2i_X509() will result
    in a NULL "Cert", and it will return FALSE, and we'll reject the
    signature, but better to avoid all that code in the first place.  Belt
    and suspenders and whatnot.
    
    Based on a patch from https://github.com/TBOpen .
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 19, 2014
  6. Validate computed hash bases/hash sizes more thoroughly.

    I screwed one of these up when working on 750584c, and it's a real pain
    to figure out, so that means we should be validating them.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 20, 2014
  7. Make 64-on-32 maybe work on x86_64.

    This is mostly based on a patch (https://github.com/mjg59/shim/issues/30)
    from https://github.com/TBOpen , which refactors our __LP64__
    tests to be tests of the header magic instead.  I've simplified things
    by using what we've pre-loaded into "context" and making some helper
    functions so the conditionals in most of the code say what they do,
    instead of how they work.
    
    Note that we're only allowing that from in_protocol's loader - that is,
    we'll let 64-bit grub load a 32-bit kernel or 32-bit grub load a 64-bit
    kernel, but 32-bit shim isn't loading a 64-bit grub.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 19, 2014
Commits on Sep 19, 2014
  1. Actually refer to the base relocation table of our loaded image.

    Currently when we process base relocations, we get the correct Data
    Directory pointer from the headers (context->RelocDir), and that header
    has been copied into our pristine allocated image when we copied up to
    SizeOfHeaders.  But the data it points to has not been mirrored in to
    the new image, so it is whatever data AllocPool() gave us.
    
    This patch changes relocate_coff() to refer to the base relocation table
    from the image we loaded from disk, but apply the fixups to the new
    copy.
    
    I have no idea how x86_64 worked without this, but I can't make aarch64
    work without it.  I also don't know how Ard or Leif have seen aarch64
    work.  Maybe they haven't?  Leif indicated on irc that they may have
    only tested shim with simple "hello world" applications from gnu-efi;
    they are certainly much less complex than grub.efi, and are generated
    through a different linking process.
    
    My only theory is that we're getting recycled data there pretty reliably
    that just makes us /not/ process any relocations, but since our
    ImageBase is 0, and I don't think we ever load grub with 0 as its base
    virtual address, that doesn't follow.  I'm open to any other ideas
    anybody has.
    
    I do know that on x86_64 (and presumably aarch64 as well), we don't
    actually start seeing *symptoms* of this bug until the first chunk[0] of
    94c9a77 is applied[1].  Once that is applied, relocate_coff() starts
    seeing zero[2] for both RelocBase->VirtualAddress and
    RelocBase->SizeOfBlock, because RelocBase is a (generated, relative)
    pointer that only makes sense in the context of the original binary, not
    our partial copy.  Since RelocBase->SizeOfBlock is tested first,
    relocate_base() gives us "Reloc block size is invalid"[3] and returns
    EFI_UNSUPPORTED.  At that point shim exits with an error.
    
    [0] The second chunk of 94c9a77 patch makes no difference on this
        issue.
    [1] I don't see why at all.
    [2] Which could really be any value since it's AllocatePool() and not
        AllocateZeroPool() results, but 0 is all I've observed; I think
        AllocatePool() has simply never recycled any memory in my test
        cases.
    [3] which is silent because perror() tries to avoid talking because that
        has caused much crashing in the past; work needs to go in to 0.9 for
        this.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Sep 18, 2014
Commits on Aug 27, 2014
  1. Make sure we don't try to load a binary from a different arch.

    Since in theory you could, for example, get an x86_64 binary signed that
    also behaves as an ARM executable, we should be checking this before
    people build on other architectures.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Aug 27, 2014
  2. Don't name something exit().

    On aarch64 due to some terrifying include chain we wind up with
    Cryptlib's definition of exit here.  I'm not a glutton for punishment,
    so I'm just changing the name so it's not coliding.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Aug 27, 2014
  3. Handle empty .reloc section in PE/COFF loader

    On archs where no EFI aware objcopy is available, the generated PE/COFF
    header contains a .reloc section which is completely empty. Handle this by
    - returning early from relocate_coff() with EFI_SUCCESS,
    - ignoring discardable sections in the section loader.
    
    Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
    ardbiesheuvel committed with vathpela Aug 13, 2014
  4. Fix typo from Ard's old tree 32-bit ARM patch.

    We don't need to .data entries; the second one should be .data*.  He's
    since fixed this in his tree, but I'd already pulled it and pushed to
    master.
    
    Signed-off-by: Peter Jones <pjones@redhat.com>
    vathpela committed Aug 27, 2014
Commits on Aug 19, 2014
  1. Update openssl to 0.9.8zb

    Also update to Tiano Cryptlib r15802 and remove the execute mode
    bits from the C and header files of openssl
    lcp committed with vathpela Aug 19, 2014