When detaching a signature, we need to know the size of the
non-signature data. So, add a data_size member to struct image, and
populate it when we iterate through the section table.
When writing the image, use data_size rather than size, so we don't
unnecessarily add the (now unused) signature data.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Rather than only calling image_find_regions when we want to sign or
verify image, call it when the image is loaded. We'll want to use the
parse data later, which will require it to be present on all instances
of an image.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
make distcheck was failing due to a missing efivars.h in the dist
tarball. Add it to common_SOURCES to include it.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Since we can sign i386 PE/COFF images, run the tests on both x86-64 and
i386 binaries.
We do this by moving test.pecoff to test-<arch>.pecoff, and using
automake's parallel-test option to add a wrapper to each test execution.
This wrapper calls each test once per arch (as defined in TEST_ARCHES),
and checks for failures in any invocation.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Replace struct image->aouthdr with a union of the 32- and 64-bit a.out
header definitions, and abstract the relevant parsing code into the
image_pecoff_parse_{32,64} functions.
We also move all references of data in the a.out header to these
functions, so we don't need to lookup the machine types elsewhere.
Based on a patch by Maxim Kammerer <mk@dee.su>.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
If we use IMAGE_FILE_MACHINE_AMD64 instead of AMD64MAGIC, we can avoid
including the arch-specific coff/x86_64 header.
Based on a patch from Maxim Kammerer <mk@dee.su>.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
padlen variable in image_write() cannot be used uninitialized,
but compiler is unable to determine that.
Signed-off-by: Maxim Kammerer <mk@dee.su>
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
KEK, db and dbx updates need to be written as EFI_SIGNATURE_LIST
structures, so create a simple tool to create them.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
We do whole-file reads in a few places, so unify to a fileio_read_file()
function.
To do this, we change the type of struct image->buf to a uint8_t *.
Where we do pointer manipulation on the image buffer, we need a
temporary void * variable.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Rather than duplicating the key & certificate loading in each tool,
unify it in a fileio object.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Rather than setting ->sigbuf directly, add two functions to handle image
signature addition and removal:
image_add_signature(image, sig, sigsize);
image_remove_signature(image);
And warn when a signature is to be overwritten.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
sbattach --detach isn't working, as we're not properly setting sigbuf in
image_pecoff parse.
This change ensures we populate sigbuf when we find a valid cert table.
Also, add a test case for this.
Bug report & initial patch from from Steve Langasek.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
sbattach.c was generating a warning on compile:
../sbattach.c: In function ‘main’:
../sbattach.c:247:2: warning: implicit declaration of function ‘OpenSSL_add_all_digests’ [-Wimplicit-function-declaration]
OpenSSL_add_all_digests is defined in evp.h, so add the #include.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
We're using OpenSSL, so need to grant binary distrbutors permission to
link with it.
Cleared with current contributors.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
The current gnu efi generation tools insist on leaving junk at the end
of the binary. According to the authenticode spec, we have to include
this in the hash otherwise signature verification fails, so add the end
junk to the calculation of the hash.
I've verified that with this fix (and another one to get objcopy to
align the sections correctly) we can now sign gnu tools generated efi
code with tianocore r13466
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Add a test to check the validity of the certificate table header,
ensuring that parsing the header gives us the valid certificate.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
The sbsign tools appear to assume that WIN_CERTIFICATE.dwLength is the
length of the signature. It's not, it's the length of the signature
plus the length of the WIN_CERTIFICATE header. UEFI Version 2.3.1,
Errata A explicitly states this in section 27.2.5 (Code Definitions).
I found this because I've been playing around with the tianocore secure
boot UEFI images and I couldn't get efi binaries signed with your tools
to verify. When you apply the fix, I've got the binaries to verify (at
least with X509 KEK signatures).
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
The current incarnation of sbsign doesn't all all ciphers to OpenSSL
meaning that if the private key is encrypted, it can't decrypt it and
instead it returns the unhelpful error message
error reading private key ../certs/PK.key
Fix this by adding all ciphers before trying to read the private key.
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
We need bfd.h for the image object, so add a check for it in
configure.ac.
Signed-off-by: Ivan Hu <ivanh.hu@canonical.com>
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Add a test for the invalid PKCS7 signature attaching. This test
generates 1K of zero bytes as an invalid signature to attach.
Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Check detached signatures to ensure that we're attaching a valid PKCS7
object. If no, show a warning message and skip the attach action.
Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Rather than requiring an explicit image_pecoff_parse, do it
unconditionally in image_load. We don't have any instances where we need
to do this separately.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Currently, sbverify will segfault when it can't load an image file, as
the image is used unconditionally. This change adds a check to ensure we
continue with a valid image pointer.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Instead of executing in the current (build) directory, create a
temporary directory and change into it before running any tests. This
ensures that tests aren't relying on left-overs from previous test runs.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
`make distcheck` fails with the following error:
ERROR: files left in build directory after distclean:
./docs/sbverify.1
./docs/sbsign.1
This change adds a CLEANFILES rule for the generated manpages.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Add a few tests for the sign, verify, attach and detach code. These
require some additional infrastructure to create a sample PE/COFF
executable, plus a key & cert for testing.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Change image_write_signed to image_write, and conditionally write the
signature if one is present.
This will allow us to write unsigned images when detaching a sig from an
image.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
The optstrings for sbsign and sbverify are out of sync with the long
options, this change brings them up to date.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
We'd like to read detached signatures too, so split the
signature-buffer-reading code into a separate function.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Fix a few warnings:
idc.c: In function ‘IDC_get’:
idc.c:248:12: warning: ‘idclen’ may be used uninitialised in this function [-Wuninitialized]
image.c: In function ‘image_load’:
image.c:37:15: warning: unused variable ‘bytes_read’ [-Wunused-variable]
Plus, a bunch of strict-aliasing warnings:
image.c:101:2: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
[ similar warnings trimmed ]
when compiling image.c. Since struct external_PEI_DOS_hdr uses char[]
types for all members, we need to use accessors here.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Add an option (--detached) to sbsign, which creates a detached
signature, rather than embedding it in the PE/COFF signature table.
Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
