On Feb 7, 2020, at 7:51 AM, Mimi Zohar [off-list ref] wrote:

On Thu, 2020-02-06 at 14:40 -0700, Eric Snowberg wrote:

<snip>

quoted

Currently the upstream code will fail if the module is uncompressed.
If you compress the same module it will load with the current
upstream code.

quoted

Lastly, there is nothing in these patches that indicate that the

kernel modules being compressed/uncompressed is related to the
signature verification.

quoted

Basically if you have the following setup:

Kernel built with CONFIG_IMA_ARCH_POLICY or kernel booted with
module.sig_enforce=1 along with the following ima policy:

appraise func=MODULE_CHECK appraise_type=imasig|modsig

Enabling CONFIG_IMA_ARCH_POLICY or module.sig_enforce=1 behave totally
differently.  CONFIG_IMA_ARCH_POLICY coordinates between the IMA
signature verification and the original module_sig_check()
verification.  Either one signature verification method is enabled or
the other, but not both.

The existing IMA x86 arch policy has not been updated to support
appended signatures.

That is not what I’m seeing.  Appended signatures mostly work.  They just
don’t work thru the finit_module system call.

To understand what is happening, we need to analyze each scenario
separately.

- If CONFIG_MODULE_SIG is configured or enabled on the boot command
line ("module.sig_enforce = 1"), then the IMA arch x86 policy WILL NOT
require an IMA signature.

All tests below are without my change
x86 booted with module.sig_enforce=1

empty ima policy
$ cat /sys/kernel/security/ima/policy
$ insmod ./foo.ko.xz   <— loads ok
$ rmmod foo
$ unxz ./foo.ko.xz
$ insmod ./foo.ko      <— loads ok
$ rmmod foo

add in module appraisal 
$ echo "appraise func=MODULE_CHECK appraise_type=imasig|modsig" > /sys/kernel/security/ima/policy

$ insmod ./foo.ko.xz   <— loads ok
$ rmmod foo

$ insmod ./foo.ko
insmod: ERROR: could not insert module ./foo.ko: Permission denied

last entry from audit log:
type=INTEGRITY_DATA msg=audit(1581089373.076:83): pid=2874 uid=0 auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=appraise_data cause=invalid-signature comm="insmod" name="/root/keys/modules/foo.ko" dev="dm-0" ino=10918365 res=0^]UID="root" AUID=“root"

This is because modsig_verify() will be called from within ima_appraise_measurement(), 
since try_modsig is true.  Then modsig_verify() will return INTEGRITY_FAIL.

If I build with CONFIG_IMA_ARCH_POLICY & CONFIG_MODULE_SIG all tests work the same above,
I just don’t have to add module.sig_enforce=1 when I boot.

Adding my change will allow foo.ko to load above when “|modsig” is added, since it will now evaluate 
the module. Without my change the “imsig|modsig” is true for compressed, but the policy is really 
“imasig&modsig” for uncompressed.

- If CONFIG_MODULE_SIG is NOT configured or enabled on the boot
command line, then the IMA arch x86 policy WILL require an IMA
signature.

Agreed

- If CONFIG_MODULE_SIG is configured or enabled on the boot command
line, the IMA arch x86 policy is not configured, and the above policy
rule is defined, an appended signature will be verified by both IMA
and module_sig_check().

I think this is the same as what I have done above?

quoted

If you have a module foo.ko that contains a valid appended signature
but is not ima signed, it will fail to load.

That would only happen in the second scenario or in the last scenario
if the key is not found.

quoted

Now if the end user simply compresses the same foo.ko, making it
foo.ko.xz.  The module will load.

This implies that CONFIG_MODULE_SIG is configured or enabled on the
boot command line, like the first scenario described above, or in the
last scenario and the key is found.

quoted

Modules can be loaded thru two different syscalls, finit_module and
init_module.  The changes added in [1] work if you use the
init_module syscall.  My change adds support when the finit_module
syscall gets used instead.

With the IMA arch x86 policy, without CONFIG_MODULE_SIG configured or
enabled on the boot command line, IMA will prevent the init_module()
syscall.  This is intentional.

Agreed

Your second patch (2/2) changes the arch x86 policy rule to allow
appended signatures.  The reason for any other changes needs to be
clearer.  I suggest you look at the audit log and kernel messages, as
well as the kexec selftests, to better understand what is happening.

I can add more details.  I’m just trying to make it so the end user has the same 
experience when using the default secure_boot ima policy. I don’t see a point in
forcing someone to compress a module to get around security, especially when they
have a policy that contains “|modsig”.  

Let me know how you would like me to move forward.  Are you ok with the actual code in 
my patches, assuming I add a lot more details? Or do you want more analysis here first?