rhboot / shim

UEFI shim loader
Other
848 stars 290 forks source link

Disable exporting vendor-dbx to MokListXRT #514

Closed dennis-tseng99 closed 1 year ago

dennis-tseng99 commented 1 year ago

As the vendor-dbx grows, it caused some problems when writing such a large variable. Some firmwares lie the avaiable space(1) , and some even crash(2) for no good reason after the writing of MokListXRT. Both shim and kernel don't rely on MokListXRT to block anything, so we just stop exporting vendor-dbx to MokListXRT to avoid the potential hassles.

(1) https://bugzilla.suse.com/show_bug.cgi?id=1185261 (2) https://github.com/rhboot/shim/pull/369#issuecomment-855275115

Signed-off-by: Gary Lin glin@suse.com Signed-off-by: Dennis Tseng dennis.tseng@suse.com

dennis-tseng99 commented 1 year ago

Thanks for your comments indeed. Yes, I should explain more details. The reason I set a configuration option is actually I'm not sure whether other vendors with their machines will have the same issue or not. If yes, I will remove this option and the line 122 & 123 directly.

dennis-tseng99 commented 1 year ago

A configuration option is added in mok.c to stop exporting. For those vendors who do not have such issue will feel nothing changed. On the other hand, if vendors want to stop exporting, then the DISABLE_EXPORT_DBX must be defined in Makefile and shim.spec command-line respectively. For example: In Makefile: CFLAGS += -DDISABLE_EXPORT_DBX In shim.spec: make RELEASE=0 ... DISABLE_EXPORT_DBX=1 ... shim.efi

A README.compile will be added soon.

julian-klode commented 1 year ago

Basically anyone with a larger vendor dbx needs this. We've been shipping this without a config option in Ubuntu for a while and I think we gotta continue despite us switching from 20 KB of hashes to like 9KB of certs - it's still to big.

The Linux kernel can revoke via certificate hashes, which would significantly reduce the size requirement, but I'm not sure how one would represent that.

vathpela commented 1 year ago

I don't really like the implementation here - I'd rather see a mok_state_variable flag to say to mirror it using config table space but not variable space, rather than simply not adding the data we have.