| Age | Commit message (Collapse) | Author |
|---|
|
Everything was going just fine until I made a vendor_db with 17kB of
sha256 sums in it. And then the same source tree that had worked fine
without that threw errors and failed all over the place. I wrote some
code to diagnose the problem, and of course it was a failure in
mirroring MokList to MokListRT.
As Patrick noted in 741c61abba7, some systems have obnoxiously low
amounts of variable storage available:
mok.c:550:import_mok_state() BS+RT variable info:
MaximumVariableStorageSize:0x000000000000DFE4
RemainingVariableStorageSize:0x000000000000D21C
MaximumVariableSize:0x0000000000001FC4
The most annoying part is that on at least this edk2 build,
SetVariable() /does actually appear to set the variable/, but it returns
EFI_INVALID_PARAMETER. I'm not planning on relying on that behavior.
So... yeah, the largest *volatile* (i.e. RAM only) variable this edk2
build will let you create is less than two pages. It's only got 7.9G
free, so I guess it's feeling like space is a little tight.
We're also not quite preserving that return code well enough for his
workaround to work.
New plan. We try to create variables the normal way, but we don't
consider not having enough space to be fatal. In that case, we create
an EFI_SECURITY_LIST with one sha256sum in it, with a value of all 0,
and try to add that so we're sure there's /something/ there that's
innocuous. On systems where the first SetVariable() /
QueryVariableInfo() lied to us, the correct variable should be there,
otherwise the one with the zero-hash will be.
We then also build a config table to hold this info and install that.
The config table is a packed array of this struct:
struct mok_variable_config_entry {
CHAR8 name[256];
UINT64 data_size;
UINT8 data[];
};
There will be N+1 entries, and the last entry is all 0 for name and
data_size. The total allocation size will always be a multiple of 4096.
In the typical RHEL 7.9 case that means it'll be around 5 pages.
It's installed with this guid:
c451ed2b-9694-45d3-baba-ed9f8988a389
Anything that can go wrong will.
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: not yet, I don't want people to read this before Wednesday.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Potential new signing strategies ( for example signing grub, fwupdate
and vmlinuz with separate certificates ) require shim to support a
vendor provided bundle of trusted certificates and hashes, which allows
shim to trust EFI binaries matching either certificate by signature or
hash in the vendor_db. Functionality is similar to vendor_dbx.
This also improves the mirroring quite a bit.
Upstream: pr#206
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
Upstream: pr#212
|
|
There are lots of hi-dpi laptops nowadays, as doing mok enrollment, the font
is too small to see.
https://bugs.launchpad.net/ubuntu/+source/shim/+bug/1822043
This patch checks if the resolution is larger than Full HD (1920x1080) and
current console output columns and rows is in a good mode. Then swith the
console output to a better mode.
Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
Upstream-commit-id: cf05af6d899
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
The manual merge of the "console: Do not set EFI console to textmode until
something is printed" patch has lead to a bunch of tabs being replaced
with 7 spaces. This commit fixes this.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
Remove the setup_console(1) calls from shim and instead make lib/console.c
make that call when necessary. This avoids shim forcing the EFI console to
switch to text-mode if nothing is printed.
This commit also modifies MokManager to work the same way for consistency,
even though MokManager will always print something.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
This is a preparation patch for making setup_console() private.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
This is a preparation commit for removing the setup_console(1) calls from
MokManager and shim so that we don't force the EFI console to switch to
text-mode.
This commit replaces all direct calls to Print / PrintAt with calls to
the new helpers (no functional changes) so that we can delay calling
setup_console(1) till the first Print call in a follow-up patch.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
I'm pretty done with typing uefi_call_wrapper() and counting arguments
every time. Instead, just make the compiler error if we don't have
ms_abi. Also, make it so nothing can use uefi_call_wrapper() directly.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Also consistently name our status variable "efi_status" unless there's a
good reason not to, such as already having another one of those.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
clang-analyze believes the following:
311 EFI_STATUS
312 variable_enroll_hash(CHAR16 *var, EFI_GUID owner,
313 UINT8 hash[SHA256_DIGEST_SIZE])
314 {
315 EFI_STATUS efi_status;
316
317 efi_status = find_in_variable_esl(var, owner, hash, SHA256_DIGEST_SIZE);
> Calling 'find_in_variable_esl' →
260 EFI_STATUS
261 find_in_variable_esl(CHAR16* var, EFI_GUID owner, UINT8 *key, UINTN keylen)
262 {
263 UINTN DataSize;
264 UINT8 *Data;
> ← 'Data' declared without an initial value →
265 EFI_STATUS efi_status;
266
267 efi_status = get_variable(var, &Data, &DataSize, owner);
> ← Calling 'get_variable' →
237 EFI_STATUS
238 get_variable(CHAR16 *var, UINT8 **data, UINTN *len, EFI_GUID owner)
239 {
240 return get_variable_attr(var, data, len, owner, NULL);
> ← Calling 'get_variable_attr' →
213 EFI_STATUS
214 get_variable_attr(CHAR16 *var, UINT8 **data, UINTN *len, EFI_GUID owner,
215 UINT32 *attributes)
216 {
217 EFI_STATUS efi_status;
218
219 *len = 0;
220
221 efi_status = GetVariable(var, &owner, NULL, len, NULL);
> ← Calling 'GetVariable' →
> ← Returning from 'GetVariable' →
222 if (efi_status != EFI_BUFFER_TOO_SMALL)
> ← Assuming the condition is true →
> ← Taking true branch →
223 return efi_status;
224
225 *data = AllocateZeroPool(*len);
226 if (!*data)
227 return EFI_OUT_OF_RESOURCES;
228
229 efi_status = GetVariable(var, &owner, attributes, len, *data);
230 if (EFI_ERROR(efi_status)) {
231 FreePool(*data);
232 *data = NULL;
233 }
234 return efi_status;
235 }
And it can't figure out that the first GetVariable() call will, in fact,
always return EFI_BUFFER_TOO_SMALL, and that AllocateZeroPool() will
then *correctly* clobber the two variables we never assigned the value
from. It also then believes that efi_status might have been returned
/without/ being an error, and thinks that means we'll use the
uninitialized pointer.
This won't happen, but hey, let's make the code better express to the
checker what is intended.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
clang-analyzer correctly believes this:
465 int i;
466
467 i = StrLen(name) - 1;
^ Value stored to 'i' is never read
468
469 for (i = StrLen(name); i > 0; --i) {
470 if (name[i] == '\\')
471 break;
472 }
And it's right; that's completely dead code.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Because they don't believe code should be defensive against future
changes, covscan believes:
520 out_free:
521 FreePool(dmp);
CID 182824 (#1 of 1): Dereference before null check
(REVERSE_INULL)check_after_deref: Null-checking entries suggests that
it may be null, but it has already been dereferenced on all paths
leading to the check.
522 if (entries) {
523 free_entries(entries, count);
524 FreePool(entries);
525 }
526 out_free_name:
527 FreePool(name);
528}
Which is technically correct, but still kind of dumb. So this patch
combines the two error out paths into just being out_free, so that the
first path there is before entries is allocated. (It also initializes
dmp to NULL and checks that before freeing it.)
I also Lindent-ed that function.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Lindent gets confused by these, and they're hard to read anyway.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Obviously, these are not correct. Most of them are just useless; one
can be changed to a more useful test.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
This lets you do:
mkdir build-x64 build-ia32
cd build-x64
make TOPDIR=.. -f ../Makefile
cd ../build-ia32
setarch i686 -B make ARCH=ia32 TOPDIR=.. -f ../Makefile
And not worry about generated sources and headers mixing and matching.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Allow to overwrite certain settings from the external.
Signed-off-by: Lans Zhang <jia.zhang@windriver.com>
|
|
When compiling with -Werror security policy can't be compiled because of
type errors. This commit fixes this problem.
|
|
Woops.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
When we made lib build with the correct CFLAGS, it inherited
-Werror=sign-compare, and I fixed up some parameters on
console_print_box() and console_print_box_at() to avoid sign comparison
errors.
The fixups were *completely wrong*, as some behavior relies on negative
values. So this fixes them in a completely different way, by casting
appropriately to signed types where we're doing comparisons.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
... but isn't.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
We don't need the headers from the standard include path.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
This patch cleans up and refactors the Makefiles to better allow new
architectures to be added:
- remove unused Makefile definitions
- import Makefile definitions from top level rather than redefining
- move x86 specific CFLAGS to inside ifeq() blocks
- remove x86 inline asm
- allow $(FORMAT) to be overridden: this is necessary as there exists no
EFI or PE/COFF aware objcopy for ARM
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
|
|
If "SecureBoot" exists but "SetupMode" does not, assume "SetupMode" says
we're not in Setup Mode.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
There are functions defined in lib to check the secure variables.
Use the functions to shun the duplicate code.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
Conflicts:
shim.c
|
|
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
|
|
On some machines, even though the key event was signaled, ReadKeyStroke
still got EFI_NOT_READY. This commit handles the error status to avoid
console_get_keystroke from returning unexpected keys.
Signed-off-by: Gary Ching-Pang Lin <glin@suse.com>
Conflicts:
MokManager.c
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Coverity scan noticed that entries is uninitialized when we pass its
location to another function.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
CHAR16* vs CHAR16**, so the result is the same on all platforms.
Detected by coverity.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Also removed unused LIB_PATH from some Makefiles.
Change-Id: I7d28d18f7531b51b6121a2ffb88bcaedec57c467
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
I feel dirty.
|
|
Sorry about that.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Make OVERRIDE_SECURITY_POLICY a build option.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
Since these are topically the same thing, they can live together.
Signed-off-by: Peter Jones <pjones@redhat.com>
|
