linux-ima-user — Q&A for users of IMA

Re: [Linux-ima-user] EVM: Using a presigned image

[Martin T. <mto...@gm...>]

On Fri, Oct 13, 2017 at 5:38 PM, Martin Townsend
<mto...@gm...> wrote:
> Hi,
>
> On Wed, Oct 11, 2017 at 6:23 PM, Mimi Zohar <zo...@li...> wrote:
>> On Wed, 2017-10-11 at 18:12 +0100, Martin Townsend wrote:
>>> Hi,
>>>
>>> I want to sign an root filesystem off line using the same private key
>>> for both IMA and EVM, ie using evmctl sign --imasig
>>>
>>> This image is read-only and is on an embedded product.  The kernel
>>> automatically loads the public key as I have
>>> CONFIG_IMA_TRUSTED_KEYRING=y
>>> CONFIG_IMA_LOAD_X509=y
>>> CONFIG_IMA_X509_PATH="/etc/keys/ima_x509.der"
>>>
>>> set in the kernel configuration
>>>
>>> I can't see how to enable this key for verifying EVM as soon as the
>>> kernel passes control to the init process.  Is this possible? Do I
>>> have to write my own init processes which could be a script to load
>>> the /etc/keys/ima_x509.der into the evm keyring, enable evm and then
>>> pass control to systemd?
>>
>> There is a separate CONFIG_EVM_X509_PATH option for EVM.  You can
>> specify the same x509 certificate pathname.
>>
>> Mimi
>>
> I upgraded to 4.9 kernel and I tried using the same key pathname and
> get the following errors
>
> integrity: Problem loading X.509 certificate (-126): /etc/keys/ima_x509.der
> integrity: Problem loading X.509 certificate (-126): /etc/keys/ima_x509.der
> integrity: Request for unknown key 'id:399171f9' err -11
> Starting init: /sbin/init exists but couldn't execute it (error -13)
>
> I've checked and the key is there and is used by IMA.  After debugging
> it fails in
> restrict_link_by_signature when it calls find_asymmetric_key.
>
> If I use the same key with 4.1 it works fine.  Is there something
> special that I need to do with the 4.9 kernel?
>
> Here's some of the kernel configuration in case it helps
>
> #
> # Security options
> #
> CONFIG_KEYS=y
> # CONFIG_PERSISTENT_KEYRINGS is not set
> # CONFIG_BIG_KEYS is not set
> # CONFIG_TRUSTED_KEYS is not set
> CONFIG_ENCRYPTED_KEYS=y
> # CONFIG_KEY_DH_OPERATIONS is not set
> # CONFIG_SECURITY_DMESG_RESTRICT is not set
> CONFIG_SECURITY=y
> CONFIG_SECURITYFS=y
> CONFIG_SECURITY_NETWORK=y
> # CONFIG_SECURITY_NETWORK_XFRM is not set
> # CONFIG_SECURITY_PATH is not set
> CONFIG_HAVE_HARDENED_USERCOPY_ALLOCATOR=y
> CONFIG_HAVE_ARCH_HARDENED_USERCOPY=y
> # CONFIG_HARDENED_USERCOPY is not set
> # CONFIG_SECURITY_SELINUX is not set
> CONFIG_SECURITY_SMACK=y
> # CONFIG_SECURITY_SMACK_BRINGUP is not set
> # CONFIG_SECURITY_SMACK_APPEND_SIGNALS is not set
> # CONFIG_SECURITY_TOMOYO is not set
> # CONFIG_SECURITY_APPARMOR is not set
> # CONFIG_SECURITY_LOADPIN is not set
> # CONFIG_SECURITY_YAMA is not set
> CONFIG_INTEGRITY=y
> CONFIG_INTEGRITY_SIGNATURE=y
> CONFIG_INTEGRITY_ASYMMETRIC_KEYS=y
> CONFIG_INTEGRITY_TRUSTED_KEYRING=y
> CONFIG_INTEGRITY_AUDIT=y
> CONFIG_IMA=y
> CONFIG_IMA_MEASURE_PCR_IDX=10
> CONFIG_IMA_LSM_RULES=y
> # CONFIG_IMA_TEMPLATE is not set
> CONFIG_IMA_NG_TEMPLATE=y
> # CONFIG_IMA_SIG_TEMPLATE is not set
> CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
> CONFIG_IMA_DEFAULT_HASH_SHA1=y
> # CONFIG_IMA_DEFAULT_HASH_SHA256 is not set
> # CONFIG_IMA_DEFAULT_HASH_SHA512 is not set
> # CONFIG_IMA_DEFAULT_HASH_WP512 is not set
> CONFIG_IMA_DEFAULT_HASH="sha1"
> # CONFIG_IMA_WRITE_POLICY is not set
> # CONFIG_IMA_READ_POLICY is not set
> CONFIG_IMA_APPRAISE=y
> CONFIG_IMA_TRUSTED_KEYRING=y
> CONFIG_IMA_BLACKLIST_KEYRING=y
> CONFIG_IMA_LOAD_X509=y
> CONFIG_IMA_X509_PATH="/etc/keys/ima-x509.der"
> CONFIG_IMA_APPRAISE_SIGNED_INIT=y
> CONFIG_EVM=y
> # CONFIG_EVM_ATTR_FSUUID is not set
> CONFIG_EVM_EXTRA_SMACK_XATTRS=y
> CONFIG_EVM_LOAD_X509=y
> CONFIG_EVM_X509_PATH="/etc/keys/ima-x509.der"
> # CONFIG_DEFAULT_SECURITY_SMACK is not set
> CONFIG_DEFAULT_SECURITY_DAC=y
> CONFIG_DEFAULT_SECURITY=""
> CONFIG_CRYPTO=y
>
> Many Thanks,
> Martin.

After reading through the code I could see that I needed to build the
kernel with the root CA certificate using CONFIG_SYSTEM_TRUSTED_KEYS
which is great as now we have a trusted root certificate in the kernel
and the kernel is signed and secured via freescale HAB.

The intermediate keys are now successfully loaded but sadly it hangs.
I know there are 2 firmware files it has to load sdma and brcmfmac so
maybe the problem is there. I see SDMA fails, Here's the trace in case
it helps.

UBIFS (ubi0:0): reserved for root: 0 bytes (0 KiB)
UBIFS (ubi0:0): media format: w4/r0 (latest is w4/r0), UUID
724DF819-1381-41CE-A1C6-5597F29F1067, small LPT model
VFS: Mounted root (ubifs filesystem) readonly on device 0:14.
devtmpfs: mounted
integrity: Loaded X.509 cert 'IMA Certificate Authority:
f1ca9f5d8e7302b74a277d1d09a6ce0c399171f9': /etc/keys/ima-x509.der
integrity: Loaded X.509 cert 'IMA Certificate Authority:
f1ca9f5d8e7302b74a277d1d09a6ce0c399171f9': /etc/keys/ima-x509.der
Freeing unused kernel memory: 1024K
imx-sdma 20ec000.sdma: external firmware not found, using ROM firmware

INFO: task kworker/u2:1:126 blocked for more than 120 seconds.
      Not tainted 4.9.44-fslc+g8f876e1 #2
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/u2:1    D    0   126    105 0x00000000
[<808425a4>] (__schedule) from [<80842a34>] (schedule+0x4c/0xac)
[<80842a34>] (schedule) from [<808451c8>] (rwsem_down_write_failed+0xf8/0x27c)
[<808451c8>] (rwsem_down_write_failed) from [<803c4c40>]
(process_measurement+0xe0/0x420)
[<803c4c40>] (process_measurement) from [<803c4fa8>] (ima_file_check+0x28/0x30)
[<803c4fa8>] (ima_file_check) from [<802365f4>] (path_openat+0x2a8/0x11c4)
[<802365f4>] (path_openat) from [<802387b0>] (do_filp_open+0x5c/0xc0)
[<802387b0>] (do_filp_open) from [<8022dad8>] (do_open_execat+0x60/0x160)
[<8022dad8>] (do_open_execat) from [<8022fb34>] (do_execveat_common+0x188/0x71c)
[<8022fb34>] (do_execveat_common) from [<802300ec>] (do_execve+0x24/0x2c)
[<802300ec>] (do_execve) from [<80131478>]
(call_usermodehelper_exec_async+0x124/0x1b0)
[<80131478>] (call_usermodehelper_exec_async) from [<80107718>]
(ret_from_fork+0x14/0x3c)
Kernel panic - not syncing: hung_task: blocked tasks
CPU: 0 PID: 14 Comm: khungtaskd Not tainted 4.9.44-fslc+g8f876e1 #2
Hardware name: Freescale i.MX6 Ultralite (Device Tree)
[<8010db5c>] (unwind_backtrace) from [<8010b718>] (show_stack+0x10/0x14)
[<8010b718>] (show_stack) from [<801d8028>] (panic+0xd0/0x244)
[<801d8028>] (panic) from [<801a1298>] (watchdog+0x320/0x3c4)
[<801a1298>] (watchdog) from [<8013a640>] (kthread+0xf4/0x10c)
[<8013a640>] (kthread) from [<80107718>] (ret_from_fork+0x14/0x3c)

I'll try and debug some more at the weekend.

-Martin.

Re: [Linux-ima-user] EVM: Using a presigned image

[Martin T. <mto...@gm...>]

Hi,

On Wed, Oct 11, 2017 at 6:23 PM, Mimi Zohar <zo...@li...> wrote:
> On Wed, 2017-10-11 at 18:12 +0100, Martin Townsend wrote:
>> Hi,
>>
>> I want to sign an root filesystem off line using the same private key
>> for both IMA and EVM, ie using evmctl sign --imasig
>>
>> This image is read-only and is on an embedded product.  The kernel
>> automatically loads the public key as I have
>> CONFIG_IMA_TRUSTED_KEYRING=y
>> CONFIG_IMA_LOAD_X509=y
>> CONFIG_IMA_X509_PATH="/etc/keys/ima_x509.der"
>>
>> set in the kernel configuration
>>
>> I can't see how to enable this key for verifying EVM as soon as the
>> kernel passes control to the init process.  Is this possible? Do I
>> have to write my own init processes which could be a script to load
>> the /etc/keys/ima_x509.der into the evm keyring, enable evm and then
>> pass control to systemd?
>
> There is a separate CONFIG_EVM_X509_PATH option for EVM.  You can
> specify the same x509 certificate pathname.
>
> Mimi
>
I upgraded to 4.9 kernel and I tried using the same key pathname and
get the following errors

integrity: Problem loading X.509 certificate (-126): /etc/keys/ima_x509.der
integrity: Problem loading X.509 certificate (-126): /etc/keys/ima_x509.der
integrity: Request for unknown key 'id:399171f9' err -11
Starting init: /sbin/init exists but couldn't execute it (error -13)

I've checked and the key is there and is used by IMA.  After debugging
it fails in
restrict_link_by_signature when it calls find_asymmetric_key.

If I use the same key with 4.1 it works fine.  Is there something
special that I need to do with the 4.9 kernel?

Here's some of the kernel configuration in case it helps

#
# Security options
#
CONFIG_KEYS=y
# CONFIG_PERSISTENT_KEYRINGS is not set
# CONFIG_BIG_KEYS is not set
# CONFIG_TRUSTED_KEYS is not set
CONFIG_ENCRYPTED_KEYS=y
# CONFIG_KEY_DH_OPERATIONS is not set
# CONFIG_SECURITY_DMESG_RESTRICT is not set
CONFIG_SECURITY=y
CONFIG_SECURITYFS=y
CONFIG_SECURITY_NETWORK=y
# CONFIG_SECURITY_NETWORK_XFRM is not set
# CONFIG_SECURITY_PATH is not set
CONFIG_HAVE_HARDENED_USERCOPY_ALLOCATOR=y
CONFIG_HAVE_ARCH_HARDENED_USERCOPY=y
# CONFIG_HARDENED_USERCOPY is not set
# CONFIG_SECURITY_SELINUX is not set
CONFIG_SECURITY_SMACK=y
# CONFIG_SECURITY_SMACK_BRINGUP is not set
# CONFIG_SECURITY_SMACK_APPEND_SIGNALS is not set
# CONFIG_SECURITY_TOMOYO is not set
# CONFIG_SECURITY_APPARMOR is not set
# CONFIG_SECURITY_LOADPIN is not set
# CONFIG_SECURITY_YAMA is not set
CONFIG_INTEGRITY=y
CONFIG_INTEGRITY_SIGNATURE=y
CONFIG_INTEGRITY_ASYMMETRIC_KEYS=y
CONFIG_INTEGRITY_TRUSTED_KEYRING=y
CONFIG_INTEGRITY_AUDIT=y
CONFIG_IMA=y
CONFIG_IMA_MEASURE_PCR_IDX=10
CONFIG_IMA_LSM_RULES=y
# CONFIG_IMA_TEMPLATE is not set
CONFIG_IMA_NG_TEMPLATE=y
# CONFIG_IMA_SIG_TEMPLATE is not set
CONFIG_IMA_DEFAULT_TEMPLATE="ima-ng"
CONFIG_IMA_DEFAULT_HASH_SHA1=y
# CONFIG_IMA_DEFAULT_HASH_SHA256 is not set
# CONFIG_IMA_DEFAULT_HASH_SHA512 is not set
# CONFIG_IMA_DEFAULT_HASH_WP512 is not set
CONFIG_IMA_DEFAULT_HASH="sha1"
# CONFIG_IMA_WRITE_POLICY is not set
# CONFIG_IMA_READ_POLICY is not set
CONFIG_IMA_APPRAISE=y
CONFIG_IMA_TRUSTED_KEYRING=y
CONFIG_IMA_BLACKLIST_KEYRING=y
CONFIG_IMA_LOAD_X509=y
CONFIG_IMA_X509_PATH="/etc/keys/ima-x509.der"
CONFIG_IMA_APPRAISE_SIGNED_INIT=y
CONFIG_EVM=y
# CONFIG_EVM_ATTR_FSUUID is not set
CONFIG_EVM_EXTRA_SMACK_XATTRS=y
CONFIG_EVM_LOAD_X509=y
CONFIG_EVM_X509_PATH="/etc/keys/ima-x509.der"
# CONFIG_DEFAULT_SECURITY_SMACK is not set
CONFIG_DEFAULT_SECURITY_DAC=y
CONFIG_DEFAULT_SECURITY=""
CONFIG_CRYPTO=y

Many Thanks,
Martin.

Re: [Linux-ima-user] Offline image creation: evm signing files problem with ubifs.

[Martin T. <mto...@gm...>]

Hi Daniel,

Thank you for the reply and a great idea and one I would have no doubt
tried.  But after reading through the mkfs.ubifs source code it dawned
on me that the inode numbers should be the same if I copy the files
across, sign the image once and recreate the ubifs, then reload this
back into the NAND simulator and sign again and recreate the ubifs. On
the second time the inode numbering should stay the same as no new
inodes have been created (ie no new extended attributes or new key
files).

This worked (haven't tried it on the HW yet but evmctl verify reports
success).  It does double the time it takes but a small price to pay
considering the amount of time it's going to save in the factory where
the alternative is to sign all the files on the target hardware during
production.

Many Thanks,
Martin.

On Wed, Oct 11, 2017 at 8:04 PM, Daniel Glöckner <dg...@em...> wrote:
> Hello Martin,
>
> Am 10/11/17 um 19:50 schrieb Martin Townsend:
>> I will also look at how mkfs.ubifs works with regards to inode numbers
>> to see if anything can be done here.  If anyone has any ideas they
>> would be greatly appreciated.
>
> I'd try to extract the UBI volume from /dev/ubiX_Y or the nandsim data
> instead of running mkfs.ubifs a second time. You can cut off all blocks
> at the end that contain only 0xff. The image is probably a bit bigger
> than with mkfs.ubifs because of the old data that has not yet been
> garbage collected.
>
> Best regards,
>
>   Daniel
>
> --
> Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
> Fon +49 551 30664-0, Fax +49 551 30664-11,
> Bertha-von-Suttner-Straße 9, 37085 Göttingen, Germany
> Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
> Geschäftsführung: Heike Jordan, Dr. Uwe Kracke
> Ust-IdNr.: DE 205 198 055
>
> emlix - your embedded linux partner

Re: [Linux-ima-user] Offline image creation: evm signing files problem with ubifs.

[Daniel G. <dg...@em...>]

Hello Martin,

Am 10/11/17 um 19:50 schrieb Martin Townsend:
> I will also look at how mkfs.ubifs works with regards to inode numbers
> to see if anything can be done here.  If anyone has any ideas they
> would be greatly appreciated.

I'd try to extract the UBI volume from /dev/ubiX_Y or the nandsim data
instead of running mkfs.ubifs a second time. You can cut off all blocks
at the end that contain only 0xff. The image is probably a bit bigger
than with mkfs.ubifs because of the old data that has not yet been
garbage collected.

Best regards,

  Daniel

-- 
Dipl.-Math. Daniel Glöckner, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11,
Bertha-von-Suttner-Straße 9, 37085 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke
Ust-IdNr.: DE 205 198 055

emlix - your embedded linux partner

Re: [Linux-ima-user] Offline image creation: evm signing files problem with ubifs.

[Martin T. <mto...@gm...>]

Hi Mimi,


On Wed, Oct 11, 2017 at 6:21 PM, Mimi Zohar <zo...@li...> wrote:
> On Wed, 2017-10-11 at 13:45 +0100, Martin Townsend wrote:
>> Hi,
>>
>> I'm trying to take a ubifs image load it into nandsim sign all the
>> files and then recreate the ubifs image on a host PC and then flash
>> onto the real target.
>>
>> So far I can IMA sign all the files and recreate the ubifs image and
>> then flash the target and all is well.  The problem is with EVM; once
>> the image is loaded into nandsim and mounted to /mnt/ubi I sign with:
>>
>> evmctl sign --imasig  --generation 0 --uuid --smack --key
>> /mnt/ubi/path/to/private_key /mnt/ubi/path/to/file
>>
>> and then verify with
>>
>> evmctl verify --generation 0 --uuid --smack --key
>> /mnt/ubi/path/to/public_key /mnt/ubi/path/to/file
>>
>> And this is fine the hashes match.  Once I then recreate the ubifs
>> with mkfs.ubifs and then reload into the nand simulator the same
>> verify command fails with
>> Verification failed: 35
>> errno: No data available (61)
>>
>> I stepped through the code and I think it's because the hashes now do not match.
>>
>> After reading a few things about inodes in offline images I'm
>> wondering if I have to use the --ino option, if so what do I have to
>> pass to this argument and if someone could explain what the problem is
>> with inodes?
>
> We're working on making EVM signatures portable (and immutable).
>  Please look at the recent discussions on the new linux-integrity
> mailing list - https://www.spinics.net/lists/linux-integrity/.
>
> Mimi
>

Sounds very promising and is probably exactly what I require.  Sadly I
need something with the kernel we have which will be 4.9 by the time
we release.

I checked the inode with the stat command before recreating the image
with mkfs.ubifs in nand simulator and the inode on the real target and
they matched.  After recreating the image and reloading the new ubifs
the inode was different so mkfs.ubifs is re-assigning inode numbers
I'm guessing.  We are adding keys to the rootfs in the mounted image
before recreating the ubifs so maybe this is upsetting all the inode
numbers, I'm not really sure how they get assigned by mkfs.ubifs.

I see there is a patch:
https://sourceforge.net/p/linux-ima/mailman/message/3464
that uses user.image-inode-number extended attribute and requires
evmctl, mkfs.ubifs being called a few times. I will investigate this
further.

I will also look at how mkfs.ubifs works with regards to inode numbers
to see if anything can be done here.  If anyone has any ideas they
would be greatly appreciated.

Many Thanks,
Martin.

Re: [Linux-ima-user] EVM: Using a presigned image

[Martin T. <mto...@gm...>]

Thanks Mimi,

A bit of searching in LXR seems to indicate that this went into the
v4.5 kernel, is this correct?

We are currently using 4.1 but will be upgrading to 4.9 LTSI in the
near future so the CONFIG_EVM_X509_PATH looks perfect :)

Many Thanks,
Martin.


On Wed, Oct 11, 2017 at 6:23 PM, Mimi Zohar <zo...@li...> wrote:
> On Wed, 2017-10-11 at 18:12 +0100, Martin Townsend wrote:
>> Hi,
>>
>> I want to sign an root filesystem off line using the same private key
>> for both IMA and EVM, ie using evmctl sign --imasig
>>
>> This image is read-only and is on an embedded product.  The kernel
>> automatically loads the public key as I have
>> CONFIG_IMA_TRUSTED_KEYRING=y
>> CONFIG_IMA_LOAD_X509=y
>> CONFIG_IMA_X509_PATH="/etc/keys/ima_x509.der"
>>
>> set in the kernel configuration
>>
>> I can't see how to enable this key for verifying EVM as soon as the
>> kernel passes control to the init process.  Is this possible? Do I
>> have to write my own init processes which could be a script to load
>> the /etc/keys/ima_x509.der into the evm keyring, enable evm and then
>> pass control to systemd?
>
> There is a separate CONFIG_EVM_X509_PATH option for EVM.  You can
> specify the same x509 certificate pathname.
>
> Mimi
>

Re: [Linux-ima-user] EVM: Using a presigned image

[Mimi Z. <zo...@li...>]

On Wed, 2017-10-11 at 18:12 +0100, Martin Townsend wrote:
> Hi,
> 
> I want to sign an root filesystem off line using the same private key
> for both IMA and EVM, ie using evmctl sign --imasig
> 
> This image is read-only and is on an embedded product.  The kernel
> automatically loads the public key as I have
> CONFIG_IMA_TRUSTED_KEYRING=y
> CONFIG_IMA_LOAD_X509=y
> CONFIG_IMA_X509_PATH="/etc/keys/ima_x509.der"
> 
> set in the kernel configuration
> 
> I can't see how to enable this key for verifying EVM as soon as the
> kernel passes control to the init process.  Is this possible? Do I
> have to write my own init processes which could be a script to load
> the /etc/keys/ima_x509.der into the evm keyring, enable evm and then
> pass control to systemd?

There is a separate CONFIG_EVM_X509_PATH option for EVM.  You can
specify the same x509 certificate pathname.

Mimi

Re: [Linux-ima-user] Offline image creation: evm signing files problem with ubifs.

[Mimi Z. <zo...@li...>]

On Wed, 2017-10-11 at 13:45 +0100, Martin Townsend wrote:
> Hi,
> 
> I'm trying to take a ubifs image load it into nandsim sign all the
> files and then recreate the ubifs image on a host PC and then flash
> onto the real target.
> 
> So far I can IMA sign all the files and recreate the ubifs image and
> then flash the target and all is well.  The problem is with EVM; once
> the image is loaded into nandsim and mounted to /mnt/ubi I sign with:
> 
> evmctl sign --imasig  --generation 0 --uuid --smack --key
> /mnt/ubi/path/to/private_key /mnt/ubi/path/to/file
> 
> and then verify with
> 
> evmctl verify --generation 0 --uuid --smack --key
> /mnt/ubi/path/to/public_key /mnt/ubi/path/to/file
> 
> And this is fine the hashes match.  Once I then recreate the ubifs
> with mkfs.ubifs and then reload into the nand simulator the same
> verify command fails with
> Verification failed: 35
> errno: No data available (61)
> 
> I stepped through the code and I think it's because the hashes now do not match.
> 
> After reading a few things about inodes in offline images I'm
> wondering if I have to use the --ino option, if so what do I have to
> pass to this argument and if someone could explain what the problem is
> with inodes?

We're working on making EVM signatures portable (and immutable).
 Please look at the recent discussions on the new linux-integrity
mailing list - https://www.spinics.net/lists/linux-integrity/.

Mimi

[Linux-ima-user] EVM: Using a presigned image

[Martin T. <mto...@gm...>]

Hi,

I want to sign an root filesystem off line using the same private key
for both IMA and EVM, ie using evmctl sign --imasig

This image is read-only and is on an embedded product.  The kernel
automatically loads the public key as I have
CONFIG_IMA_TRUSTED_KEYRING=y
CONFIG_IMA_LOAD_X509=y
CONFIG_IMA_X509_PATH="/etc/keys/ima_x509.der"

set in the kernel configuration

I can't see how to enable this key for verifying EVM as soon as the
kernel passes control to the init process.  Is this possible? Do I
have to write my own init processes which could be a script to load
the /etc/keys/ima_x509.der into the evm keyring, enable evm and then
pass control to systemd?

Many Thanks in advnace,
Martin.

[Linux-ima-user] Offline image creation: evm signing files problem with ubifs.

[Martin T. <mto...@gm...>]

Hi,

I'm trying to take a ubifs image load it into nandsim sign all the
files and then recreate the ubifs image on a host PC and then flash
onto the real target.

So far I can IMA sign all the files and recreate the ubifs image and
then flash the target and all is well.  The problem is with EVM; once
the image is loaded into nandsim and mounted to /mnt/ubi I sign with:

evmctl sign --imasig  --generation 0 --uuid --smack --key
/mnt/ubi/path/to/private_key /mnt/ubi/path/to/file

and then verify with

evmctl verify --generation 0 --uuid --smack --key
/mnt/ubi/path/to/public_key /mnt/ubi/path/to/file

And this is fine the hashes match.  Once I then recreate the ubifs
with mkfs.ubifs and then reload into the nand simulator the same
verify command fails with
Verification failed: 35
errno: No data available (61)

I stepped through the code and I think it's because the hashes now do not match.

After reading a few things about inodes in offline images I'm
wondering if I have to use the --ino option, if so what do I have to
pass to this argument and if someone could explain what the problem is
with inodes?

Many Thanks in advance,
Martin.

Re: [Linux-ima-user] [PATCH v8 01/20] crypto: change transient busy return code to -EAGAIN

[Herbert Xu <he...@go...>]

On Sat, Oct 07, 2017 at 10:51:42AM +0300, Gilad Ben-Yossef wrote:
> On Sat, Oct 7, 2017 at 6:05 AM, Herbert Xu <he...@go...> wrote:
> > On Tue, Sep 05, 2017 at 03:38:40PM +0300, Gilad Ben-Yossef wrote:
> >>
> >> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> >> index 5e92bd2..3b3c154 100644
> >> --- a/crypto/algif_hash.c
> >> +++ b/crypto/algif_hash.c
> >> @@ -39,6 +39,20 @@ struct algif_hash_tfm {
> >>       bool has_key;
> >>  };
> >>
> >> +/* Previous versions of crypto_* ops used to return -EBUSY
> >> + * rather than -EAGAIN to indicate being tied up. The in
> >> + * kernel API changed but we don't want to break the user
> >> + * space API. As only the hash user interface exposed this
> >> + * error ever to the user, do the translation here.
> >> + */
> >> +static inline int crypto_user_err(int err)
> >> +{
> >> +     if (err == -EAGAIN)
> >> +             return -EBUSY;
> >> +
> >> +     return err;
> >
> > I don't see the need to carry along this baggage.  Does anyone
> > in user-space actually rely on EBUSY?
> 
> 
> I am not aware of anyone who does. I was just trying to avoid
> changing the user ABI.
> 
> Shall I roll a new revision without this patch?

Yes please.  I'd rather not carry this around for eternity unless
it was actually required.

Thanks,
-- 
Email: Herbert Xu <he...@go...>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[Linux-ima-user] [PATCH] ima: Fix bool initialization/comparison

[Thomas M. <th...@m3...>]

Bool initializations should use true and false. Bool tests don't need
comparisons.

Signed-off-by: Thomas Meyer <th...@m3...>
---

diff -u -p a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -32,7 +32,7 @@ bool ima_canonical_fmt;
 static int __init default_canonical_fmt_setup(char *str)
 {
 #ifdef __BIG_ENDIAN
-	ima_canonical_fmt = 1;
+	ima_canonical_fmt = true;
 #endif
 	return 1;
 }
diff -u -p a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -196,9 +196,9 @@ static int __init policy_setup(char *str
 		if ((strcmp(p, "tcb") == 0) && !ima_policy)
 			ima_policy = DEFAULT_TCB;
 		else if (strcmp(p, "appraise_tcb") == 0)
-			ima_use_appraise_tcb = 1;
+			ima_use_appraise_tcb = true;
 		else if (strcmp(p, "secure_boot") == 0)
-			ima_use_secure_boot = 1;
+			ima_use_secure_boot = true;
 	}
 
 	return 1;
@@ -207,7 +207,7 @@ __setup("ima_policy=", policy_setup);
 
 static int __init default_appraise_policy_setup(char *str)
 {
-	ima_use_appraise_tcb = 1;
+	ima_use_appraise_tcb = true;
 	return 1;
 }
 __setup("ima_appraise_tcb", default_appraise_policy_setup);

Re: [Linux-ima-user] [PATCH v8 01/20] crypto: change transient busy return code to -EAGAIN

[Gilad Ben-Y. <gi...@be...>]

On Sat, Oct 7, 2017 at 6:05 AM, Herbert Xu <he...@go...> wrote:
> On Tue, Sep 05, 2017 at 03:38:40PM +0300, Gilad Ben-Yossef wrote:
>>
>> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
>> index 5e92bd2..3b3c154 100644
>> --- a/crypto/algif_hash.c
>> +++ b/crypto/algif_hash.c
>> @@ -39,6 +39,20 @@ struct algif_hash_tfm {
>>       bool has_key;
>>  };
>>
>> +/* Previous versions of crypto_* ops used to return -EBUSY
>> + * rather than -EAGAIN to indicate being tied up. The in
>> + * kernel API changed but we don't want to break the user
>> + * space API. As only the hash user interface exposed this
>> + * error ever to the user, do the translation here.
>> + */
>> +static inline int crypto_user_err(int err)
>> +{
>> +     if (err == -EAGAIN)
>> +             return -EBUSY;
>> +
>> +     return err;
>
> I don't see the need to carry along this baggage.  Does anyone
> in user-space actually rely on EBUSY?


I am not aware of anyone who does. I was just trying to avoid
changing the user ABI.

Shall I roll a new revision without this patch?

Thanks,
Gilad

>
> Cheers,
> --
> Email: Herbert Xu <he...@go...>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

Re: [Linux-ima-user] [PATCH v8 01/20] crypto: change transient busy return code to -EAGAIN

[Herbert Xu <he...@go...>]

On Tue, Sep 05, 2017 at 03:38:40PM +0300, Gilad Ben-Yossef wrote:
>
> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
> index 5e92bd2..3b3c154 100644
> --- a/crypto/algif_hash.c
> +++ b/crypto/algif_hash.c
> @@ -39,6 +39,20 @@ struct algif_hash_tfm {
>  	bool has_key;
>  };
>  
> +/* Previous versions of crypto_* ops used to return -EBUSY
> + * rather than -EAGAIN to indicate being tied up. The in
> + * kernel API changed but we don't want to break the user
> + * space API. As only the hash user interface exposed this
> + * error ever to the user, do the translation here.
> + */
> +static inline int crypto_user_err(int err)
> +{
> +	if (err == -EAGAIN)
> +		return -EBUSY;
> +
> +	return err;

I don't see the need to carry along this baggage.  Does anyone
in user-space actually rely on EBUSY?

Cheers,
-- 
Email: Herbert Xu <he...@go...>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[Linux-ima-user] How to prevent the no signature files be executed.

[HC T. <fp...@gm...>]

Hi everyone,

In Ubuntu 16.04 I do the following steps to digitally signed.

@Enable EVM
echo "1" > /sys/kernel/security/evm

@Create keys folder
$ su -c 'mkdir -p /etc/keys'

@To create and save the kernel master key (user type):
$ su -c 'modprobe trusted encrypted'
$ su -c 'keyctl add user kmk-user "`dd if=/dev/urandom bs=1 count=32
2>/dev/null`" @u'
$ su -c 'keyctl pipe `keyctl search @u user kmk-user` >
/etc/keys/kmk-user.blob'

@Create the EVM encrypted key
$ su -c 'keyctl add encrypted evm-key "new user:kmk-user 32" @u'
$ su -c 'keyctl pipe `keyctl search @u encrypted evm-key`
>/etc/keys/evm-user.blob'

@generate unencrypted private key
openssl genrsa -out privkey_evm.pem 1024

@Image Labeling : whole file-system
sudo find / \( -fstype rootfs -o -fstype ext3 -o -fstype ext4 \) ! -path
"/lib/modules/*" -type f -uid 0 -exec evmctl sign --imahash '{}' \;

sudo find / \( -fstype rootfs -o -fstype ext3 -o -fstype ext4 \) ! -path
"/lib/modules/*" -type f -uid 0 -exec evmctl ima_sign --imahash '{}' -print
\;

@check security.ima content
evmctl ima_sign --hashalgo sha256 --rsa --key /etc/keys/privkey_evm.pem -t
f /home/my/test.sh
getfattr -e hex -m security -d /home/my/test.sh

In the last step, i can see the signature for tset.sh file.
However, i have no idea how to enable system verification function to
prevent the no signature files be executed.
Does anyone can help it?
THX

Re: [Linux-ima-user] Why ascii_runtime_measurements only has one entry

[Mimi Z. <zo...@li...>]

On Mon, 2017-09-11 at 16:02 +0800, shijun zhao wrote:
> Hi everyone,
> 
> I want to use IMA to measure my system, but I find that the
> ascii_runtime_measurements only has one entry.
> I'm using Ubuntu 16.04, Linux kernel is 4.4.62, and I don't have hardware
> TPM chip.
> 
> The entry in ascii_runtime_measurements is as follows:
> 10 719de8e521439498e9b77f6ed41e230b9821111e ima
> 0000000000000000000000000000000000000000 boot_aggregate

The TPM is needed for quoting the PCRs.  Without a TPM, IMA goes into
bypass mode. Try specifying "ima_policy=tcb" on the boot command line.

Mimi

[Linux-ima-user] Why ascii_runtime_measurements only has one entry

[shijun z. <zq...@gm...>]

Hi everyone,

I want to use IMA to measure my system, but I find that the
ascii_runtime_measurements only has one entry.
I'm using Ubuntu 16.04, Linux kernel is 4.4.62, and I don't have hardware
TPM chip.

The entry in ascii_runtime_measurements is as follows:
10 719de8e521439498e9b77f6ed41e230b9821111e ima
0000000000000000000000000000000000000000 boot_aggregate

Best Regards,
Shijun

[Linux-ima-user] [PATCH v8 20/20] crypto: adapt api sample to use async. op wait

[Gilad Ben-Y. <gi...@be...>]

The code sample is waiting for an async. crypto op completion.
Adapt sample to use the new generic infrastructure to do the same.

This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
---
 Documentation/crypto/api-samples.rst | 52 +++++++-----------------------------
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/Documentation/crypto/api-samples.rst b/Documentation/crypto/api-samples.rst
index 2531948..006827e 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -7,59 +7,27 @@ Code Example For Symmetric Key Cipher Operation
 ::
 
 
-    struct tcrypt_result {
-        struct completion completion;
-        int err;
-    };
-
     /* tie all data structures together */
     struct skcipher_def {
         struct scatterlist sg;
         struct crypto_skcipher *tfm;
         struct skcipher_request *req;
-        struct tcrypt_result result;
+        struct crypto_wait wait;
     };
 
-    /* Callback function */
-    static void test_skcipher_cb(struct crypto_async_request *req, int error)
-    {
-        struct tcrypt_result *result = req->data;
-
-        if (error == -EINPROGRESS)
-            return;
-        result->err = error;
-        complete(&result->completion);
-        pr_info("Encryption finished successfully\n");
-    }
-
     /* Perform cipher operation */
     static unsigned int test_skcipher_encdec(struct skcipher_def *sk,
                          int enc)
     {
-        int rc = 0;
+        int rc;
 
         if (enc)
-            rc = crypto_skcipher_encrypt(sk->req);
+            rc = crypto_wait_req(crypto_skcipher_encrypt(sk->req), &sk->wait);
         else
-            rc = crypto_skcipher_decrypt(sk->req);
-
-        switch (rc) {
-        case 0:
-            break;
-        case -EINPROGRESS:
-        case -EBUSY:
-            rc = wait_for_completion_interruptible(
-                &sk->result.completion);
-            if (!rc && !sk->result.err) {
-                reinit_completion(&sk->result.completion);
-                break;
-            }
-        default:
-            pr_info("skcipher encrypt returned with %d result %d\n",
-                rc, sk->result.err);
-            break;
-        }
-        init_completion(&sk->result.completion);
+            rc = crypto_wait_req(crypto_skcipher_decrypt(sk->req), &sk->wait);
+
+	if (rc)
+		pr_info("skcipher encrypt returned with result %d\n", rc);
 
         return rc;
     }
@@ -89,8 +57,8 @@ Code Example For Symmetric Key Cipher Operation
         }
 
         skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-                          test_skcipher_cb,
-                          &sk.result);
+                          crypto_req_done,
+                          &sk.wait);
 
         /* AES 256 with random key */
         get_random_bytes(&key, 32);
@@ -122,7 +90,7 @@ Code Example For Symmetric Key Cipher Operation
         /* We encrypt one block */
         sg_init_one(&sk.sg, scratchpad, 16);
         skcipher_request_set_crypt(req, &sk.sg, &sk.sg, 16, ivdata);
-        init_completion(&sk.result.completion);
+        crypto_init_wait(&sk.wait);
 
         /* encrypt data */
         ret = test_skcipher_encdec(&sk, 1);
-- 
2.1.4

[Linux-ima-user] [PATCH v8 19/20] crypto: mediatek: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

The mediatek driver starts several async crypto ops and waits for their
completions. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
Acked-by: Ryder Lee <ryd...@me...>
---
 drivers/crypto/mediatek/mtk-aes.c | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/crypto/mediatek/mtk-aes.c b/drivers/crypto/mediatek/mtk-aes.c
index 9e845e8..e2c7c95 100644
--- a/drivers/crypto/mediatek/mtk-aes.c
+++ b/drivers/crypto/mediatek/mtk-aes.c
@@ -137,11 +137,6 @@ struct mtk_aes_gcm_ctx {
 	struct crypto_skcipher *ctr;
 };
 
-struct mtk_aes_gcm_setkey_result {
-	int err;
-	struct completion completion;
-};
-
 struct mtk_aes_drv {
 	struct list_head dev_list;
 	/* Device list lock */
@@ -936,17 +931,6 @@ static int mtk_aes_gcm_crypt(struct aead_request *req, u64 mode)
 				    &req->base);
 }
 
-static void mtk_gcm_setkey_done(struct crypto_async_request *req, int err)
-{
-	struct mtk_aes_gcm_setkey_result *result = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	result->err = err;
-	complete(&result->completion);
-}
-
 /*
  * Because of the hardware limitation, we need to pre-calculate key(H)
  * for the GHASH operation. The result of the encryption operation
@@ -962,7 +946,7 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 		u32 hash[4];
 		u8 iv[8];
 
-		struct mtk_aes_gcm_setkey_result result;
+		struct crypto_wait wait;
 
 		struct scatterlist sg[1];
 		struct skcipher_request req;
@@ -1002,22 +986,17 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 	if (!data)
 		return -ENOMEM;
 
-	init_completion(&data->result.completion);
+	crypto_init_wait(&data->wait);
 	sg_init_one(data->sg, &data->hash, AES_BLOCK_SIZE);
 	skcipher_request_set_tfm(&data->req, ctr);
 	skcipher_request_set_callback(&data->req, CRYPTO_TFM_REQ_MAY_SLEEP |
 				      CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      mtk_gcm_setkey_done, &data->result);
+				      crypto_req_done, &data->wait);
 	skcipher_request_set_crypt(&data->req, data->sg, data->sg,
 				   AES_BLOCK_SIZE, data->iv);
 
-	err = crypto_skcipher_encrypt(&data->req);
-	if (err == -EINPROGRESS || err == -EBUSY) {
-		err = wait_for_completion_interruptible(
-			&data->result.completion);
-		if (!err)
-			err = data->result.err;
-	}
+	err = crypto_wait_req(crypto_skcipher_encrypt(&data->req),
+			      &data->wait);
 	if (err)
 		goto out;
 
-- 
2.1.4

[Linux-ima-user] [PATCH v8 18/20] crypto: qce: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

The qce driver starts several async crypto ops and  waits for their
completions. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
---
 drivers/crypto/qce/sha.c | 30 ++++--------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c
index 47e114a..53227d7 100644
--- a/drivers/crypto/qce/sha.c
+++ b/drivers/crypto/qce/sha.c
@@ -349,28 +349,12 @@ static int qce_ahash_digest(struct ahash_request *req)
 	return qce->async_req_enqueue(tmpl->qce, &req->base);
 }
 
-struct qce_ahash_result {
-	struct completion completion;
-	int error;
-};
-
-static void qce_digest_complete(struct crypto_async_request *req, int error)
-{
-	struct qce_ahash_result *result = req->data;
-
-	if (error == -EINPROGRESS)
-		return;
-
-	result->error = error;
-	complete(&result->completion);
-}
-
 static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 				 unsigned int keylen)
 {
 	unsigned int digestsize = crypto_ahash_digestsize(tfm);
 	struct qce_sha_ctx *ctx = crypto_tfm_ctx(&tfm->base);
-	struct qce_ahash_result result;
+	struct crypto_wait wait;
 	struct ahash_request *req;
 	struct scatterlist sg;
 	unsigned int blocksize;
@@ -405,9 +389,9 @@ static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 		goto err_free_ahash;
 	}
 
-	init_completion(&result.completion);
+	crypto_init_wait(&wait);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				   qce_digest_complete, &result);
+				   crypto_req_done, &wait);
 	crypto_ahash_clear_flags(ahash_tfm, ~0);
 
 	buf = kzalloc(keylen + QCE_MAX_ALIGN_SIZE, GFP_KERNEL);
@@ -420,13 +404,7 @@ static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 	sg_init_one(&sg, buf, keylen);
 	ahash_request_set_crypt(req, &sg, ctx->authkey, keylen);
 
-	ret = crypto_ahash_digest(req);
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
-		ret = wait_for_completion_interruptible(&result.completion);
-		if (!ret)
-			ret = result.error;
-	}
-
+	ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
 	if (ret)
 		crypto_ahash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
 
-- 
2.1.4

[Linux-ima-user] [PATCH v8 17/20] crypto: talitos: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

The talitos driver starts several async crypto ops and  waits for their
completions. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
---
 drivers/crypto/talitos.c | 38 +++++---------------------------------
 1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 79791c6..194a307 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2037,22 +2037,6 @@ static int ahash_import(struct ahash_request *areq, const void *in)
 	return 0;
 }
 
-struct keyhash_result {
-	struct completion completion;
-	int err;
-};
-
-static void keyhash_complete(struct crypto_async_request *req, int err)
-{
-	struct keyhash_result *res = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	res->err = err;
-	complete(&res->completion);
-}
-
 static int keyhash(struct crypto_ahash *tfm, const u8 *key, unsigned int keylen,
 		   u8 *hash)
 {
@@ -2060,10 +2044,10 @@ static int keyhash(struct crypto_ahash *tfm, const u8 *key, unsigned int keylen,
 
 	struct scatterlist sg[1];
 	struct ahash_request *req;
-	struct keyhash_result hresult;
+	struct crypto_wait wait;
 	int ret;
 
-	init_completion(&hresult.completion);
+	crypto_init_wait(&wait);
 
 	req = ahash_request_alloc(tfm, GFP_KERNEL);
 	if (!req)
@@ -2072,25 +2056,13 @@ static int keyhash(struct crypto_ahash *tfm, const u8 *key, unsigned int keylen,
 	/* Keep tfm keylen == 0 during hash of the long key */
 	ctx->keylen = 0;
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				   keyhash_complete, &hresult);
+				   crypto_req_done, &wait);
 
 	sg_init_one(&sg[0], key, keylen);
 
 	ahash_request_set_crypt(req, sg, hash, keylen);
-	ret = crypto_ahash_digest(req);
-	switch (ret) {
-	case 0:
-		break;
-	case -EINPROGRESS:
-	case -EBUSY:
-		ret = wait_for_completion_interruptible(
-			&hresult.completion);
-		if (!ret)
-			ret = hresult.err;
-		break;
-	default:
-		break;
-	}
+	ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
+
 	ahash_request_free(req);
 
 	return ret;
-- 
2.1.4

[Linux-ima-user] [PATCH v8 16/20] crypto: tcrypt: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

tcrypt starts several async crypto ops and  waits for their completions.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
---
 crypto/tcrypt.c | 84 +++++++++++++++++----------------------------------------
 1 file changed, 25 insertions(+), 59 deletions(-)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 0022a18..802aa81 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -79,34 +79,11 @@ static char *check[] = {
 	NULL
 };
 
-struct tcrypt_result {
-	struct completion completion;
-	int err;
-};
-
-static void tcrypt_complete(struct crypto_async_request *req, int err)
-{
-	struct tcrypt_result *res = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	res->err = err;
-	complete(&res->completion);
-}
-
 static inline int do_one_aead_op(struct aead_request *req, int ret)
 {
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
-		struct tcrypt_result *tr = req->base.data;
+	struct crypto_wait *wait = req->base.data;
 
-		ret = wait_for_completion_interruptible(&tr->completion);
-		if (!ret)
-			ret = tr->err;
-		reinit_completion(&tr->completion);
-	}
-
-	return ret;
+	return crypto_wait_req(ret, wait);
 }
 
 static int test_aead_jiffies(struct aead_request *req, int enc,
@@ -248,7 +225,7 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs,
 	char *axbuf[XBUFSIZE];
 	unsigned int *b_size;
 	unsigned int iv_len;
-	struct tcrypt_result result;
+	struct crypto_wait wait;
 
 	iv = kzalloc(MAX_IVLEN, GFP_KERNEL);
 	if (!iv)
@@ -284,7 +261,7 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs,
 		goto out_notfm;
 	}
 
-	init_completion(&result.completion);
+	crypto_init_wait(&wait);
 	printk(KERN_INFO "\ntesting speed of %s (%s) %s\n", algo,
 			get_driver_name(crypto_aead, tfm), e);
 
@@ -296,7 +273,7 @@ static void test_aead_speed(const char *algo, int enc, unsigned int secs,
 	}
 
 	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				  tcrypt_complete, &result);
+				  crypto_req_done, &wait);
 
 	i = 0;
 	do {
@@ -397,21 +374,16 @@ static void test_hash_sg_init(struct scatterlist *sg)
 
 static inline int do_one_ahash_op(struct ahash_request *req, int ret)
 {
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
-		struct tcrypt_result *tr = req->base.data;
+	struct crypto_wait *wait = req->base.data;
 
-		wait_for_completion(&tr->completion);
-		reinit_completion(&tr->completion);
-		ret = tr->err;
-	}
-	return ret;
+	return crypto_wait_req(ret, wait);
 }
 
 struct test_mb_ahash_data {
 	struct scatterlist sg[TVMEMSIZE];
 	char result[64];
 	struct ahash_request *req;
-	struct tcrypt_result tresult;
+	struct crypto_wait wait;
 	char *xbuf[XBUFSIZE];
 };
 
@@ -440,7 +412,7 @@ static void test_mb_ahash_speed(const char *algo, unsigned int sec,
 		if (testmgr_alloc_buf(data[i].xbuf))
 			goto out;
 
-		init_completion(&data[i].tresult.completion);
+		crypto_init_wait(&data[i].wait);
 
 		data[i].req = ahash_request_alloc(tfm, GFP_KERNEL);
 		if (!data[i].req) {
@@ -449,8 +421,8 @@ static void test_mb_ahash_speed(const char *algo, unsigned int sec,
 			goto out;
 		}
 
-		ahash_request_set_callback(data[i].req, 0,
-					   tcrypt_complete, &data[i].tresult);
+		ahash_request_set_callback(data[i].req, 0, crypto_req_done,
+					   &data[i].wait);
 		test_hash_sg_init(data[i].sg);
 	}
 
@@ -492,16 +464,16 @@ static void test_mb_ahash_speed(const char *algo, unsigned int sec,
 			if (ret)
 				break;
 
-			complete(&data[k].tresult.completion);
-			data[k].tresult.err = 0;
+			crypto_req_done(&data[k].req->base, 0);
 		}
 
 		for (j = 0; j < k; j++) {
-			struct tcrypt_result *tr = &data[j].tresult;
+			struct crypto_wait *wait = &data[j].wait;
+			int wait_ret;
 
-			wait_for_completion(&tr->completion);
-			if (tr->err)
-				ret = tr->err;
+			wait_ret = crypto_wait_req(-EINPROGRESS, wait);
+			if (wait_ret)
+				ret = wait_ret;
 		}
 
 		end = get_cycles();
@@ -679,7 +651,7 @@ static void test_ahash_speed_common(const char *algo, unsigned int secs,
 				    struct hash_speed *speed, unsigned mask)
 {
 	struct scatterlist sg[TVMEMSIZE];
-	struct tcrypt_result tresult;
+	struct crypto_wait wait;
 	struct ahash_request *req;
 	struct crypto_ahash *tfm;
 	char *output;
@@ -708,9 +680,9 @@ static void test_ahash_speed_common(const char *algo, unsigned int secs,
 		goto out;
 	}
 
-	init_completion(&tresult.completion);
+	crypto_init_wait(&wait);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				   tcrypt_complete, &tresult);
+				   crypto_req_done, &wait);
 
 	output = kmalloc(MAX_DIGEST_SIZE, GFP_KERNEL);
 	if (!output)
@@ -765,15 +737,9 @@ static void test_hash_speed(const char *algo, unsigned int secs,
 
 static inline int do_one_acipher_op(struct skcipher_request *req, int ret)
 {
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
-		struct tcrypt_result *tr = req->base.data;
-
-		wait_for_completion(&tr->completion);
-		reinit_completion(&tr->completion);
-		ret = tr->err;
-	}
+	struct crypto_wait *wait = req->base.data;
 
-	return ret;
+	return crypto_wait_req(ret, wait);
 }
 
 static int test_acipher_jiffies(struct skcipher_request *req, int enc,
@@ -853,7 +819,7 @@ static void test_skcipher_speed(const char *algo, int enc, unsigned int secs,
 				unsigned int tcount, u8 *keysize, bool async)
 {
 	unsigned int ret, i, j, k, iv_len;
-	struct tcrypt_result tresult;
+	struct crypto_wait wait;
 	const char *key;
 	char iv[128];
 	struct skcipher_request *req;
@@ -866,7 +832,7 @@ static void test_skcipher_speed(const char *algo, int enc, unsigned int secs,
 	else
 		e = "decryption";
 
-	init_completion(&tresult.completion);
+	crypto_init_wait(&wait);
 
 	tfm = crypto_alloc_skcipher(algo, 0, async ? 0 : CRYPTO_ALG_ASYNC);
 
@@ -887,7 +853,7 @@ static void test_skcipher_speed(const char *algo, int enc, unsigned int secs,
 	}
 
 	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      tcrypt_complete, &tresult);
+				      crypto_req_done, &wait);
 
 	i = 0;
 	do {
-- 
2.1.4

[Linux-ima-user] [PATCH v8 15/20] ima: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

ima starts several async crypto ops and  waits for their completions.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
Acked-by: Mimi Zohar <zo...@li...>
---
 security/integrity/ima/ima_crypto.c | 56 +++++++++++--------------------------
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index a856d8c..9057b16 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -27,11 +27,6 @@
 
 #include "ima.h"
 
-struct ahash_completion {
-	struct completion completion;
-	int err;
-};
-
 /* minimum file size for ahash use */
 static unsigned long ima_ahash_minsize;
 module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644);
@@ -196,30 +191,13 @@ static void ima_free_atfm(struct crypto_ahash *tfm)
 		crypto_free_ahash(tfm);
 }
 
-static void ahash_complete(struct crypto_async_request *req, int err)
+static inline int ahash_wait(int err, struct crypto_wait *wait)
 {
-	struct ahash_completion *res = req->data;
 
-	if (err == -EINPROGRESS)
-		return;
-	res->err = err;
-	complete(&res->completion);
-}
+	err = crypto_wait_req(err, wait);
 
-static int ahash_wait(int err, struct ahash_completion *res)
-{
-	switch (err) {
-	case 0:
-		break;
-	case -EINPROGRESS:
-	case -EBUSY:
-		wait_for_completion(&res->completion);
-		reinit_completion(&res->completion);
-		err = res->err;
-		/* fall through */
-	default:
+	if (err)
 		pr_crit_ratelimited("ahash calculation failed: err: %d\n", err);
-	}
 
 	return err;
 }
@@ -233,7 +211,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 	int rc, read = 0, rbuf_len, active = 0, ahash_rc = 0;
 	struct ahash_request *req;
 	struct scatterlist sg[1];
-	struct ahash_completion res;
+	struct crypto_wait wait;
 	size_t rbuf_size[2];
 
 	hash->length = crypto_ahash_digestsize(tfm);
@@ -242,12 +220,12 @@ static int ima_calc_file_hash_atfm(struct file *file,
 	if (!req)
 		return -ENOMEM;
 
-	init_completion(&res.completion);
+	crypto_init_wait(&wait);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
 				   CRYPTO_TFM_REQ_MAY_SLEEP,
-				   ahash_complete, &res);
+				   crypto_req_done, &wait);
 
-	rc = ahash_wait(crypto_ahash_init(req), &res);
+	rc = ahash_wait(crypto_ahash_init(req), &wait);
 	if (rc)
 		goto out1;
 
@@ -288,7 +266,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 			 * read/request, wait for the completion of the
 			 * previous ahash_update() request.
 			 */
-			rc = ahash_wait(ahash_rc, &res);
+			rc = ahash_wait(ahash_rc, &wait);
 			if (rc)
 				goto out3;
 		}
@@ -304,7 +282,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 			 * read/request, wait for the completion of the
 			 * previous ahash_update() request.
 			 */
-			rc = ahash_wait(ahash_rc, &res);
+			rc = ahash_wait(ahash_rc, &wait);
 			if (rc)
 				goto out3;
 		}
@@ -318,7 +296,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 			active = !active; /* swap buffers, if we use two */
 	}
 	/* wait for the last update request to complete */
-	rc = ahash_wait(ahash_rc, &res);
+	rc = ahash_wait(ahash_rc, &wait);
 out3:
 	if (read)
 		file->f_mode &= ~FMODE_READ;
@@ -327,7 +305,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 out2:
 	if (!rc) {
 		ahash_request_set_crypt(req, NULL, hash->digest, 0);
-		rc = ahash_wait(crypto_ahash_final(req), &res);
+		rc = ahash_wait(crypto_ahash_final(req), &wait);
 	}
 out1:
 	ahash_request_free(req);
@@ -537,7 +515,7 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
 {
 	struct ahash_request *req;
 	struct scatterlist sg;
-	struct ahash_completion res;
+	struct crypto_wait wait;
 	int rc, ahash_rc = 0;
 
 	hash->length = crypto_ahash_digestsize(tfm);
@@ -546,12 +524,12 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
 	if (!req)
 		return -ENOMEM;
 
-	init_completion(&res.completion);
+	crypto_init_wait(&wait);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
 				   CRYPTO_TFM_REQ_MAY_SLEEP,
-				   ahash_complete, &res);
+				   crypto_req_done, &wait);
 
-	rc = ahash_wait(crypto_ahash_init(req), &res);
+	rc = ahash_wait(crypto_ahash_init(req), &wait);
 	if (rc)
 		goto out;
 
@@ -561,10 +539,10 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
 	ahash_rc = crypto_ahash_update(req);
 
 	/* wait for the update request to complete */
-	rc = ahash_wait(ahash_rc, &res);
+	rc = ahash_wait(ahash_rc, &wait);
 	if (!rc) {
 		ahash_request_set_crypt(req, NULL, hash->digest, 0);
-		rc = ahash_wait(crypto_ahash_final(req), &res);
+		rc = ahash_wait(crypto_ahash_final(req), &wait);
 	}
 out:
 	ahash_request_free(req);
-- 
2.1.4

[Linux-ima-user] [PATCH v8 14/20] cifs: move to generic async completion

[Gilad Ben-Y. <gi...@be...>]

cifs starts an async. crypto op and waits for their completion.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
Acked-by: Pavel Shilovsky <ps...@mi...>
---
 fs/cifs/smb2ops.c | 30 ++++--------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index fb2934b..982b39d 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2066,22 +2066,6 @@ init_sg(struct smb_rqst *rqst, u8 *sign)
 	return sg;
 }
 
-struct cifs_crypt_result {
-	int err;
-	struct completion completion;
-};
-
-static void cifs_crypt_complete(struct crypto_async_request *req, int err)
-{
-	struct cifs_crypt_result *res = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	res->err = err;
-	complete(&res->completion);
-}
-
 static int
 smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key)
 {
@@ -2122,12 +2106,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
 	struct aead_request *req;
 	char *iv;
 	unsigned int iv_len;
-	struct cifs_crypt_result result = {0, };
+	DECLARE_CRYPTO_WAIT(wait);
 	struct crypto_aead *tfm;
 	unsigned int crypt_len = le32_to_cpu(tr_hdr->OriginalMessageSize);
 
-	init_completion(&result.completion);
-
 	rc = smb2_get_enc_key(server, tr_hdr->SessionId, enc, key);
 	if (rc) {
 		cifs_dbg(VFS, "%s: Could not get %scryption key\n", __func__,
@@ -2187,14 +2169,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
 	aead_request_set_ad(req, assoc_data_len);
 
 	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				  cifs_crypt_complete, &result);
+				  crypto_req_done, &wait);
 
-	rc = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
-
-	if (rc == -EINPROGRESS || rc == -EBUSY) {
-		wait_for_completion(&result.completion);
-		rc = result.err;
-	}
+	rc = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+				: crypto_aead_decrypt(req), &wait);
 
 	if (!rc && enc)
 		memcpy(&tr_hdr->Signature, sign, SMB2_SIGNATURE_SIZE);
-- 
2.1.4

[Linux-ima-user] [PATCH v8 13/20] dm: move dm-verity to generic async completion

[Gilad Ben-Y. <gi...@be...>]

dm-verity is starting async. crypto ops and waiting for them to complete.
Move it over to generic code doing the same.

This also avoids a future potential data coruption bug created
by the use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing, should this code ever move to a context
where signals are not masked.

Signed-off-by: Gilad Ben-Yossef <gi...@be...>
CC: Mikulas Patocka <mpa...@re...>
---
 drivers/md/dm-verity-target.c | 81 +++++++++++--------------------------------
 drivers/md/dm-verity.h        |  5 ---
 2 files changed, 20 insertions(+), 66 deletions(-)

diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index bda3cac..811ad28 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -92,74 +92,33 @@ static sector_t verity_position_at_level(struct dm_verity *v, sector_t block,
 	return block >> (level * v->hash_per_block_bits);
 }
 
-/*
- * Callback function for asynchrnous crypto API completion notification
- */
-static void verity_op_done(struct crypto_async_request *base, int err)
-{
-	struct verity_result *res = (struct verity_result *)base->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	res->err = err;
-	complete(&res->completion);
-}
-
-/*
- * Wait for async crypto API callback
- */
-static inline int verity_complete_op(struct verity_result *res, int ret)
-{
-	switch (ret) {
-	case 0:
-		break;
-
-	case -EINPROGRESS:
-	case -EBUSY:
-		ret = wait_for_completion_interruptible(&res->completion);
-		if (!ret)
-			ret = res->err;
-		reinit_completion(&res->completion);
-		break;
-
-	default:
-		DMERR("verity_wait_hash: crypto op submission failed: %d", ret);
-	}
-
-	if (unlikely(ret < 0))
-		DMERR("verity_wait_hash: crypto op failed: %d", ret);
-
-	return ret;
-}
-
 static int verity_hash_update(struct dm_verity *v, struct ahash_request *req,
 				const u8 *data, size_t len,
-				struct verity_result *res)
+				struct crypto_wait *wait)
 {
 	struct scatterlist sg;
 
 	sg_init_one(&sg, data, len);
 	ahash_request_set_crypt(req, &sg, NULL, len);
 
-	return verity_complete_op(res, crypto_ahash_update(req));
+	return crypto_wait_req(crypto_ahash_update(req), wait);
 }
 
 /*
  * Wrapper for crypto_ahash_init, which handles verity salting.
  */
 static int verity_hash_init(struct dm_verity *v, struct ahash_request *req,
-				struct verity_result *res)
+				struct crypto_wait *wait)
 {
 	int r;
 
 	ahash_request_set_tfm(req, v->tfm);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP |
 					CRYPTO_TFM_REQ_MAY_BACKLOG,
-					verity_op_done, (void *)res);
-	init_completion(&res->completion);
+					crypto_req_done, (void *)wait);
+	crypto_init_wait(wait);
 
-	r = verity_complete_op(res, crypto_ahash_init(req));
+	r = crypto_wait_req(crypto_ahash_init(req), wait);
 
 	if (unlikely(r < 0)) {
 		DMERR("crypto_ahash_init failed: %d", r);
@@ -167,18 +126,18 @@ static int verity_hash_init(struct dm_verity *v, struct ahash_request *req,
 	}
 
 	if (likely(v->salt_size && (v->version >= 1)))
-		r = verity_hash_update(v, req, v->salt, v->salt_size, res);
+		r = verity_hash_update(v, req, v->salt, v->salt_size, wait);
 
 	return r;
 }
 
 static int verity_hash_final(struct dm_verity *v, struct ahash_request *req,
-			     u8 *digest, struct verity_result *res)
+			     u8 *digest, struct crypto_wait *wait)
 {
 	int r;
 
 	if (unlikely(v->salt_size && (!v->version))) {
-		r = verity_hash_update(v, req, v->salt, v->salt_size, res);
+		r = verity_hash_update(v, req, v->salt, v->salt_size, wait);
 
 		if (r < 0) {
 			DMERR("verity_hash_final failed updating salt: %d", r);
@@ -187,7 +146,7 @@ static int verity_hash_final(struct dm_verity *v, struct ahash_request *req,
 	}
 
 	ahash_request_set_crypt(req, NULL, digest, 0);
-	r = verity_complete_op(res, crypto_ahash_final(req));
+	r = crypto_wait_req(crypto_ahash_final(req), wait);
 out:
 	return r;
 }
@@ -196,17 +155,17 @@ int verity_hash(struct dm_verity *v, struct ahash_request *req,
 		const u8 *data, size_t len, u8 *digest)
 {
 	int r;
-	struct verity_result res;
+	struct crypto_wait wait;
 
-	r = verity_hash_init(v, req, &res);
+	r = verity_hash_init(v, req, &wait);
 	if (unlikely(r < 0))
 		goto out;
 
-	r = verity_hash_update(v, req, data, len, &res);
+	r = verity_hash_update(v, req, data, len, &wait);
 	if (unlikely(r < 0))
 		goto out;
 
-	r = verity_hash_final(v, req, digest, &res);
+	r = verity_hash_final(v, req, digest, &wait);
 
 out:
 	return r;
@@ -389,7 +348,7 @@ int verity_hash_for_block(struct dm_verity *v, struct dm_verity_io *io,
  * Calculates the digest for the given bio
  */
 int verity_for_io_block(struct dm_verity *v, struct dm_verity_io *io,
-			struct bvec_iter *iter, struct verity_result *res)
+			struct bvec_iter *iter, struct crypto_wait *wait)
 {
 	unsigned int todo = 1 << v->data_dev_block_bits;
 	struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size);
@@ -414,7 +373,7 @@ int verity_for_io_block(struct dm_verity *v, struct dm_verity_io *io,
 		 */
 		sg_set_page(&sg, bv.bv_page, len, bv.bv_offset);
 		ahash_request_set_crypt(req, &sg, NULL, len);
-		r = verity_complete_op(res, crypto_ahash_update(req));
+		r = crypto_wait_req(crypto_ahash_update(req), wait);
 
 		if (unlikely(r < 0)) {
 			DMERR("verity_for_io_block crypto op failed: %d", r);
@@ -482,7 +441,7 @@ static int verity_verify_io(struct dm_verity_io *io)
 	struct dm_verity *v = io->v;
 	struct bvec_iter start;
 	unsigned b;
-	struct verity_result res;
+	struct crypto_wait wait;
 
 	for (b = 0; b < io->n_blocks; b++) {
 		int r;
@@ -507,17 +466,17 @@ static int verity_verify_io(struct dm_verity_io *io)
 			continue;
 		}
 
-		r = verity_hash_init(v, req, &res);
+		r = verity_hash_init(v, req, &wait);
 		if (unlikely(r < 0))
 			return r;
 
 		start = io->iter;
-		r = verity_for_io_block(v, io, &io->iter, &res);
+		r = verity_for_io_block(v, io, &io->iter, &wait);
 		if (unlikely(r < 0))
 			return r;
 
 		r = verity_hash_final(v, req, verity_io_real_digest(v, io),
-					&res);
+					&wait);
 		if (unlikely(r < 0))
 			return r;
 
diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
index a59e0ad..b675bc0 100644
--- a/drivers/md/dm-verity.h
+++ b/drivers/md/dm-verity.h
@@ -90,11 +90,6 @@ struct dm_verity_io {
 	 */
 };
 
-struct verity_result {
-	struct completion completion;
-	int err;
-};
-
 static inline struct ahash_request *verity_io_hash_req(struct dm_verity *v,
 						     struct dm_verity_io *io)
 {
-- 
2.1.4