On Feb 5, 2021, at 3:27 AM, Mickaël Salaün [off-list ref] wrote:


On 05/02/2021 01:24, Eric Snowberg wrote:

quoted

quoted

On Feb 4, 2021, at 1:26 AM, Mickaël Salaün [off-list ref] wrote:


On 04/02/2021 04:53, Eric Snowberg wrote:

quoted

quoted

On Feb 3, 2021, at 11:49 AM, Mickaël Salaün [off-list ref] wrote:

This looks good to me, and it still works for my use case. Eric's
patchset only looks for asymmetric keys in the blacklist keyring, so
even if we use the same keyring we don't look for the same key types. My
patchset only allows blacklist keys (i.e. hashes, not asymmetric keys)
to be added by user space (if authenticated), but because Eric's
asymmetric keys are loaded with KEY_ALLOC_BYPASS_RESTRICTION, it should
be OK for his use case.  There should be no interference between the two
new features, but I find it a bit confusing to have such distinct use of
keys from the same keyring depending on their type.

I agree, it is a bit confusing.  What is the thought of having a dbx 
keyring, similar to how the platform keyring works?

https://www.spinics.net/lists/linux-security-module/msg40262.html

quoted

On 03/02/2021 17:26, David Howells wrote:

quoted

Eric Snowberg [off-list ref] wrote:

quoted

This is the fifth patch series for adding support for 
EFI_CERT_X509_GUID entries [1].  It has been expanded to not only include
dbx entries but also entries in the mokx.  Additionally my series to
preload these certificate [2] has also been included.

Okay, I've tentatively applied this to my keys-next branch.  However, it
conflicts minorly with Mickaël Salaün's patches that I've previously merged on
the same branch.  Can you have a look at the merge commit

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=keys-next&id=fdbbe7ceeb95090d09c33ce0497e0394c82aa33d

	(the top patch of my keys-next branch)

to see if that is okay by both of you?  If so, can you give it a whirl?

I’m seeing a build error within blacklist_hashes_checked with
one of my configs.

The config is as follows:

$ grep CONFIG_SYSTEM_BLACKLIST_HASH_LIST .config
CONFIG_SYSTEM_BLACKLIST_HASH_LIST=“revocation_list"

$ cat certs/revocation_list
"tbs:1e125ea4f38acb7b29b0c495fd8e7602c2c3353b913811a9da3a2fb505c08a32”

make[1]: *** No rule to make target 'revocation_list', needed by 'certs/blacklist_hashes_checked'.  Stop.

It requires an absolute path.

Ok, if I use an absolute path now with CONFIG_SYSTEM_BLACKLIST_HASH_LIST 
it works.

quoted

This is to align with other variables
using the config_filename macro: CONFIG_SYSTEM_TRUSTED_KEYS,
CONFIG_MODULE_SIG_KEY and now CONFIG_SYSTEM_REVOCATION_KEYS.

I just did a quick test with CONFIG_SYSTEM_TRUSTED_KEYS. It looks like we 
can use either a relative or absolute path with CONFIG_SYSTEM_TRUSTED_KEYS. 
Shouldn’t this be consistent?

CONFIG_SYSTEM_TRUSTED_KEYS (and similar config) works with relative path
to $(srctree) not $(srctree)/certs as in your example.

Correct, I had "certs" in my relative path.

After applying this patch, CONFIG_SYSTEM_BLACKLIST_HASH_LIST now works
like the other filename macros.  It seems like this would be a good
addition.

I have done some additional testing, I am seeing a regression. The blacklist 
keyring is no longer picking up any of the hashes from the dbx during boot. 
I backed out the merge with my changes  (fdbbe7ceeb95090d09c33ce0497e0394c82aa33d) 
and still see the regression.  I then backed out Mickaël merge
(5bf1adccf5c41dbdd51d1f4de220d335d9548598) and it fixes the regression.

On a x86 with the updated dbx from uefi.org, I’d expect to see 234 bin hash entries
in the blacklist keyring.  With the current merged code, there is none.