| Message ID | 20210625165614.2284243-4-roberto.sassu@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Fri, Jun 25, 2021 at 06:56:05PM +0200, Roberto Sassu wrote: > --- /dev/null > +++ b/include/uapi/linux/digest_lists.h > @@ -0,0 +1,43 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +/* > + * Copyright (C) 2017-2021 Huawei Technologies Duesseldorf GmbH > + * > + * Author: Roberto Sassu <roberto.sassu@huawei.com> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation, version 2 of the > + * License. As you already have the SPDX line up there, you do not need this paragraph. Please remove it from all of the new files you have added in this series. > + * > + * File: digest_lists.h We know the filename, no need to have it here again. > + * Digest list definitions exported to user space. Now this is what probably needs more information... > + */ > + > +#ifndef _UAPI__LINUX_DIGEST_LISTS_H > +#define _UAPI__LINUX_DIGEST_LISTS_H > + > +#include <linux/types.h> > +#include <linux/hash_info.h> > + > +enum compact_types { COMPACT_KEY, COMPACT_PARSER, COMPACT_FILE, > + COMPACT_METADATA, COMPACT_DIGEST_LIST, COMPACT__LAST }; > + > +enum compact_modifiers { COMPACT_MOD_IMMUTABLE, COMPACT_MOD__LAST }; > + > +enum compact_actions { COMPACT_ACTION_IMA_MEASURED, > + COMPACT_ACTION_IMA_APPRAISED, > + COMPACT_ACTION_IMA_APPRAISED_DIGSIG, > + COMPACT_ACTION__LAST }; > + > +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL, DIGEST_LIST_OP__LAST }; > + For enums you export to userspace, you need to specify the values so that all compilers get them right. > +struct compact_list_hdr { > + __u8 version; You should never need a version, that way lies madness. > + __u8 _reserved; You better be testing this for 0, right? > + __le16 type; > + __le16 modifiers; > + __le16 algo; > + __le32 count; > + __le32 datalen; Why are user/kernel apis specified in little endian format? Why would that matter? Shouldn't they just be "native" endian? thanks, greg k-h
On Sun, Jun 27, 2021 at 12:53:47PM +0200, Greg KH wrote: > > +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL, DIGEST_LIST_OP__LAST }; > > + > > For enums you export to userspace, you need to specify the values so > that all compilers get them right. I've never heard that rule before. Where does it come from? https://en.cppreference.com/w/c/language/enum says: If enumeration-constant is not followed by = constant-expression, its value is the value one greater than the value of the previous enumerator in the same enumeration. The value of the first enumerator (if it does not use = constant-expression) is zero.
On Sun, Jun 27, 2021 at 04:23:26PM +0100, Matthew Wilcox wrote: > On Sun, Jun 27, 2021 at 12:53:47PM +0200, Greg KH wrote: > > > +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL, DIGEST_LIST_OP__LAST }; > > > + > > > > For enums you export to userspace, you need to specify the values so > > that all compilers get them right. > > I've never heard that rule before. Where does it come from? > https://en.cppreference.com/w/c/language/enum > says: > > If enumeration-constant is not followed by = constant-expression, > its value is the value one greater than the value of the previous > enumerator in the same enumeration. The value of the first enumerator > (if it does not use = constant-expression) is zero. > I thought it was in the Documentation/driver-api/ioctl.rst file, but I can't find it right now. Maybe it was something that Arnd said? Arnd, is this still an issue? For some reason I thought it was always good to have uapi .h enums be explicit as to the value in them, otherwise some C compilers might produce other values if they were not specified? thanks, greg k-h
> From: Greg KH [mailto:gregkh@linuxfoundation.org] > Sent: Sunday, June 27, 2021 12:54 PM > On Fri, Jun 25, 2021 at 06:56:05PM +0200, Roberto Sassu wrote: > > --- /dev/null > > +++ b/include/uapi/linux/digest_lists.h > > @@ -0,0 +1,43 @@ > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > > +/* > > + * Copyright (C) 2017-2021 Huawei Technologies Duesseldorf GmbH > > + * > > + * Author: Roberto Sassu <roberto.sassu@huawei.com> > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License as > > + * published by the Free Software Foundation, version 2 of the > > + * License. > > As you already have the SPDX line up there, you do not need this > paragraph. Please remove it from all of the new files you have added in > this series. Ok. > > + * > > + * File: digest_lists.h > > We know the filename, no need to have it here again. > > > + * Digest list definitions exported to user space. > > Now this is what probably needs more information... Ok. Yes, these definitions are useful to generate digest lists in user space. > > + */ > > + > > +#ifndef _UAPI__LINUX_DIGEST_LISTS_H > > +#define _UAPI__LINUX_DIGEST_LISTS_H > > + > > +#include <linux/types.h> > > +#include <linux/hash_info.h> > > + > > +enum compact_types { COMPACT_KEY, COMPACT_PARSER, > COMPACT_FILE, > > + COMPACT_METADATA, COMPACT_DIGEST_LIST, > COMPACT__LAST }; > > + > > +enum compact_modifiers { COMPACT_MOD_IMMUTABLE, > COMPACT_MOD__LAST }; > > + > > +enum compact_actions { COMPACT_ACTION_IMA_MEASURED, > > + COMPACT_ACTION_IMA_APPRAISED, > > + COMPACT_ACTION_IMA_APPRAISED_DIGSIG, > > + COMPACT_ACTION__LAST }; > > + > > +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL, DIGEST_LIST_OP__LAST }; > > + > > For enums you export to userspace, you need to specify the values so > that all compilers get them right. > > > +struct compact_list_hdr { > > + __u8 version; > > You should never need a version, that way lies madness. We wanted to have a way to switch to a new format, if necessary. > > + __u8 _reserved; > > You better be testing this for 0, right? Ok, will do. > > + __le16 type; > > + __le16 modifiers; > > + __le16 algo; > > + __le32 count; > > + __le32 datalen; > > Why are user/kernel apis specified in little endian format? Why would > that matter? Shouldn't they just be "native" endian? I thought this would make it clear that the kernel always expects the digest lists to be in little endian. Thanks Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > thanks, > > greg k-h
On Mon, Jun 28, 2021 at 08:30:32AM +0000, Roberto Sassu wrote: > > > +struct compact_list_hdr { > > > + __u8 version; > > > > You should never need a version, that way lies madness. > > We wanted to have a way to switch to a new format, if necessary. Then just add a new ioctl if you need that in the future, no need to try to cram it into this one. > > > + __le16 type; > > > + __le16 modifiers; > > > + __le16 algo; > > > + __le32 count; > > > + __le32 datalen; > > > > Why are user/kernel apis specified in little endian format? Why would > > that matter? Shouldn't they just be "native" endian? > > I thought this would make it clear that the kernel always expects the > digest lists to be in little endian. Why would a big endian system expect the data from userspace to be in little endian? Shouldn't this always just be "native" endian given that this is not something that is being sent to hardware? thanks, greg k-h
> From: Greg KH [mailto:gregkh@linuxfoundation.org] > Sent: Monday, June 28, 2021 10:46 AM > On Mon, Jun 28, 2021 at 08:30:32AM +0000, Roberto Sassu wrote: > > > > +struct compact_list_hdr { > > > > + __u8 version; > > > > > > You should never need a version, that way lies madness. > > > > We wanted to have a way to switch to a new format, if necessary. > > Then just add a new ioctl if you need that in the future, no need to try > to cram it into this one. Given that digest lists are generated elsewhere, it would be still unclear when the ioctl() would be issued. Maybe the kernel needs to parse both v1 and v2 digest lists (I expect that v1 cannot be easily converted to v2, if they are signed). It would be also unpractical if digest lists are loaded at kernel initialization time (I didn't send the patch yet). > > > > + __le16 type; > > > > + __le16 modifiers; > > > > + __le16 algo; > > > > + __le32 count; > > > > + __le32 datalen; > > > > > > Why are user/kernel apis specified in little endian format? Why would > > > that matter? Shouldn't they just be "native" endian? > > > > I thought this would make it clear that the kernel always expects the > > digest lists to be in little endian. > > Why would a big endian system expect the data from userspace to be in > little endian? Shouldn't this always just be "native" endian given that > this is not something that is being sent to hardware? The digest list might come from a system with different endianness. Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > thanks, > > greg k-h
On Mon, Jun 28, 2021 at 09:27:05AM +0000, Roberto Sassu wrote: > > From: Greg KH [mailto:gregkh@linuxfoundation.org] > > Sent: Monday, June 28, 2021 10:46 AM > > On Mon, Jun 28, 2021 at 08:30:32AM +0000, Roberto Sassu wrote: > > > > > +struct compact_list_hdr { > > > > > + __u8 version; > > > > > > > > You should never need a version, that way lies madness. > > > > > > We wanted to have a way to switch to a new format, if necessary. > > > > Then just add a new ioctl if you need that in the future, no need to try > > to cram it into this one. > > Given that digest lists are generated elsewhere, it would be still > unclear when the ioctl() would be issued. Maybe the kernel needs > to parse both v1 and v2 digest lists (I expect that v1 cannot be easily > converted to v2, if they are signed). > > It would be also unpractical if digest lists are loaded at kernel > initialization time (I didn't send the patch yet). Then that is up to your api design, I do not know. But note that "version" fields almost always never work, so be careful about assuming that this will solve any future issues. > > > > > + __le16 type; > > > > > + __le16 modifiers; > > > > > + __le16 algo; > > > > > + __le32 count; > > > > > + __le32 datalen; > > > > > > > > Why are user/kernel apis specified in little endian format? Why would > > > > that matter? Shouldn't they just be "native" endian? > > > > > > I thought this would make it clear that the kernel always expects the > > > digest lists to be in little endian. > > > > Why would a big endian system expect the data from userspace to be in > > little endian? Shouldn't this always just be "native" endian given that > > this is not something that is being sent to hardware? > > The digest list might come from a system with different endianness. Ok, I have no idea what digests really are used for then. So stick with little endian and be sure to properly convert within the kernel as needed. thanks, greg k-h
> From: Greg KH [mailto:gregkh@linuxfoundation.org] > Sent: Monday, June 28, 2021 11:32 AM > On Mon, Jun 28, 2021 at 09:27:05AM +0000, Roberto Sassu wrote: > > > From: Greg KH [mailto:gregkh@linuxfoundation.org] > > > Sent: Monday, June 28, 2021 10:46 AM > > > On Mon, Jun 28, 2021 at 08:30:32AM +0000, Roberto Sassu wrote: > > > > > > +struct compact_list_hdr { > > > > > > + __u8 version; > > > > > > > > > > You should never need a version, that way lies madness. > > > > > > > > We wanted to have a way to switch to a new format, if necessary. > > > > > > Then just add a new ioctl if you need that in the future, no need to try > > > to cram it into this one. > > > > Given that digest lists are generated elsewhere, it would be still > > unclear when the ioctl() would be issued. Maybe the kernel needs > > to parse both v1 and v2 digest lists (I expect that v1 cannot be easily > > converted to v2, if they are signed). > > > > It would be also unpractical if digest lists are loaded at kernel > > initialization time (I didn't send the patch yet). > > Then that is up to your api design, I do not know. But note that > "version" fields almost always never work, so be careful about assuming > that this will solve any future issues. > > > > > > > + __le16 type; > > > > > > + __le16 modifiers; > > > > > > + __le16 algo; > > > > > > + __le32 count; > > > > > > + __le32 datalen; > > > > > > > > > > Why are user/kernel apis specified in little endian format? Why would > > > > > that matter? Shouldn't they just be "native" endian? > > > > > > > > I thought this would make it clear that the kernel always expects the > > > > digest lists to be in little endian. > > > > > > Why would a big endian system expect the data from userspace to be in > > > little endian? Shouldn't this always just be "native" endian given that > > > this is not something that is being sent to hardware? > > > > The digest list might come from a system with different endianness. > > Ok, I have no idea what digests really are used for then. So stick with > little endian and be sure to properly convert within the kernel as > needed. The most intuitive use case is to extend secure boot to the OS. The kernel might be configured to accept only digest lists signed with a trusted key. Once the database is populated, execution and mmap can be denied if the calculated file or metadata digest is not found in the database (the file has not been released by the vendor). Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > thanks, > > greg k-h
diff --git a/Documentation/security/digest_lists.rst b/Documentation/security/digest_lists.rst index 8980be7836f8..995260294783 100644 --- a/Documentation/security/digest_lists.rst +++ b/Documentation/security/digest_lists.rst @@ -226,3 +226,122 @@ the digest list) (step 5). Finally, digests can be searched from user space through a securityfs file (step 6) or by the kernel itself. + + +Implementation +============== + +This section describes the implementation of Huawei Digest Lists. + + +Basic Definitions +----------------- + +This section introduces the basic definitions required to use digest lists. + + +Compact Digest List Format +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Compact digest lists consist of one or multiple headers defined as: + +:: + + struct compact_list_hdr { + __u8 version; + __u8 _reserved; + __le16 type; + __le16 modifiers; + __le16 algo; + __le32 count; + __le32 datalen; + } __packed; + +which characterize the subsequent block of concatenated digests. + +The ``algo`` field specifies the algorithm used to calculate the digest. + +The ``count`` field specifies how many digests are stored in the subsequent +block of digests. + +The ``datalen`` field specifies the length of the subsequent block of +digests (it is redundant, it is the same as +``hash_digest_size[algo] * count``). + + +Compact Types +............. + +Digests can be of different types: + +- ``COMPACT_PARSER``: digests of executables which are given the ability to + parse digest lists not in the compact format and to upload to the kernel + the digest list converted to the compact format; +- ``COMPACT_FILE``: digests of regular files; +- ``COMPACT_METADATA``: digests of file metadata (e.g. the digest + calculated by EVM to verify a portable signature); +- ``COMPACT_DIGEST_LIST``: digests of digest lists (only used internally by + the kernel). + +Different users of Huawei Digest Lists might query digests with different +compact types. For example, IMA would be interested in COMPACT_FILE, as it +deals with regular files, while EVM would be interested in +COMPACT_METADATA, as it verifies file metadata. + + +Compact Modifiers +................. + +Digests can also have specific attributes called modifiers (bit position): + +- ``COMPACT_MOD_IMMUTABLE``: file content or metadata should not be + modifiable. + +IMA might use this information to deny open for writing, or EVM to deny +setxattr operations. + + +Actions +....... + +This section defines a set of possible actions that have been executed on +the digest lists (bit position): + +- ``COMPACT_ACTION_IMA_MEASURED``: the digest list has been measured by + IMA; +- ``COMPACT_ACTION_IMA_APPRAISED``: the digest list has been successfully + appraised by IMA; +- ``COMPACT_ACTION_IMA_APPRAISED_DIGSIG``: the digest list has been + successfully appraised by IMA by verifying a digital signature. + +This information might help users of Huawei Digest Lists to decide whether +to use the result of a queried digest. + +For example, if a digest belongs to a digest list that was not measured +before, IMA should ignore the result of the query as the measurement list +sent to remote verifiers would lack how the database was populated. + + +Compact Digest List Example +........................... + +:: + + version: 1, type: 2, modifiers: 0 algo: 4, count: 3, datalen: 96 + <SHA256 digest1><SHA256 digest2><SHA256 digest3> + version: 1, type: 3, modifiers: 1 algo: 6, count: 2, datalen: 128 + <SHA512 digest1><SHA512 digest2> + +This digest list consists of two blocks. The first block contains three +SHA256 digests of regular files. The second block contains two SHA512 +digests of immutable metadata. + + +Compact Digest List Operations +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Finally, this section defines the possible operations that can be performed +with digest lists: + +- ``DIGEST_LIST_ADD``: the digest list is being added; +- ``DIGEST_LIST_DEL``: the digest list is being deleted. diff --git a/MAINTAINERS b/MAINTAINERS index cba3d82fee43..ccf555862673 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8387,6 +8387,7 @@ L: linux-integrity@vger.kernel.org S: Supported T: git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git F: Documentation/security/digest_lists.rst +F: uapi/linux/digest_lists.h HUAWEI ETHERNET DRIVER M: Bin Luo <luobin9@huawei.com> diff --git a/include/uapi/linux/digest_lists.h b/include/uapi/linux/digest_lists.h new file mode 100644 index 000000000000..9545a8aaa231 --- /dev/null +++ b/include/uapi/linux/digest_lists.h @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +/* + * Copyright (C) 2017-2021 Huawei Technologies Duesseldorf GmbH + * + * Author: Roberto Sassu <roberto.sassu@huawei.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, version 2 of the + * License. + * + * File: digest_lists.h + * Digest list definitions exported to user space. + */ + +#ifndef _UAPI__LINUX_DIGEST_LISTS_H +#define _UAPI__LINUX_DIGEST_LISTS_H + +#include <linux/types.h> +#include <linux/hash_info.h> + +enum compact_types { COMPACT_KEY, COMPACT_PARSER, COMPACT_FILE, + COMPACT_METADATA, COMPACT_DIGEST_LIST, COMPACT__LAST }; + +enum compact_modifiers { COMPACT_MOD_IMMUTABLE, COMPACT_MOD__LAST }; + +enum compact_actions { COMPACT_ACTION_IMA_MEASURED, + COMPACT_ACTION_IMA_APPRAISED, + COMPACT_ACTION_IMA_APPRAISED_DIGSIG, + COMPACT_ACTION__LAST }; + +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL, DIGEST_LIST_OP__LAST }; + +struct compact_list_hdr { + __u8 version; + __u8 _reserved; + __le16 type; + __le16 modifiers; + __le16 algo; + __le32 count; + __le32 datalen; +} __packed; +#endif
This patch introduces the basic definitions, exported to user space, to use digest lists. The definitions, added to include/uapi/linux/digest_lists.h, are documented in Documentation/security/digest_lists.rst. Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> --- Documentation/security/digest_lists.rst | 119 ++++++++++++++++++++++++ MAINTAINERS | 1 + include/uapi/linux/digest_lists.h | 43 +++++++++ 3 files changed, 163 insertions(+) create mode 100644 include/uapi/linux/digest_lists.h