RE: [RFC PATCH v7 00/16] Integrity Policy Enforcement (IPE)

RE: [RFC PATCH v7 00/16] Integrity Policy Enforcement (IPE)

[Roberto Sassu]

> From: deven.desai@linux.microsoft.com
> [mailto:deven.desai@linux.microsoft.com]
> From: Deven Bowers <deven.desai@linux.microsoft.com>
> 
> Overview:
> ---------
> 
> IPE is a Linux Security Module which takes a complimentary approach to
> access control. Whereas existing systems approach use labels or paths
> which control access to a resource, IPE controls access to a resource
> based on the system's trust of said resource.

To me, it does not give a particularly precise idea of what IPE is about.

It would have been more clear, assuming that I understood it correctly,
if you have said:

Whereas existing mandatory access control mechanisms base their
decisions on labels and paths, IPE instead determines whether or not
an operation should be allowed based on immutable security properties
of the system component the operation is being performed on.

IPE itself does not mandate how the security property should be
evaluated, but relies on an extensible set of external property providers
to evaluate the component. IPE makes its decision based on reference
values for the selected properties, specified in the IPE policy.

The reference values represent the value that the policy writer and the
local system administrator (based on the policy signature) trust for the
system to accomplish the desired tasks.

One such provider is for example dm-verity, which is able to represent
the integrity property of a partition (its immutable state) with a digest.

> Trust requirements are established via IPE's policy, sourcing multiple
> different implementations within the kernel to build a cohesive trust
> model, based on how the system was built.
> 
> Trust, with respect to computing, is a concept that designates a set
> of entities who will endorse a set of resources as non-malicious.
> Traditionally, this is done via signatures, which is the act of endorsing
> a resource.
> 
> Integrity, on the other hand, is the concept of ensuring that a resource
> has not been modified since a point of time. This is typically done through
> cryptographic hashes or signatures.
> 
> Trust and integrity are very closely tied together concepts, as integrity
> is the way you can prove trust for a resource; otherwise it could have
> been modified by an entity who is untrusted.
> 
> IPE provides a way for a user to express trust requirements of resources,
> by using pre-existing systems which provide the integrity half of the
> equation.
> 
> IPE is compiled under CONFIG_SECURITY_IPE.
> 
> Use Cases
> ---------
> 
> IPE works best in fixed-function devices: Devices in which their purpose
> is clearly defined and not supposed to be changed (e.g. network firewall
> device in a data center, an IoT device, etcetera), where all software and
> configuration is built and provisioned by the system owner.
> 
> IPE is a long-way off for use in general-purpose computing:
> the Linux community as a whole tends to follow a decentralized trust
> model, known as the Web of Trust, which IPE has no support for as of yet.
> Instead, IPE supports the PKI Trust Model, which generally designates a
> set of entities that provide a measure absolute trust.

It is true that packages are signed with PGP, which is decentralized,
but there is a special case where Linux distribution vendors trust
their own keys. This, at least, would allow to trust the software built
by a particular vendor (I ported David Howells's work on PGP keys and
signature to the current kernel).

> Additionally, while most packages are signed today, the files inside
> the packages (for instance, the executables), tend to be unsigned. This
> makes it difficult to utilize IPE in systems where a package manager is
> expected to be functional, without major changes to the package manager
> and ecosystem behind it.

Yes, RPMs don't have per file signatures but have a signature of the
list of file digests, which is equivalent. They could have also the fsverity
digests (instead of the fsverity signatures) to reduce size overhead.

Given that the authenticity of RPMs headers can be verified, if the
PGP key of the vendor is included in the primary keyring of the kernel,
being able to protect file or fsverity digests against tampering by
user space and being able to query them (e.g. with DIGLIM) extends
the applicability of IPE to general purpose OSes.

> Policy:
> -------
> 
> IPE policy is a plain-text [#]_ policy composed of multiple statements
> over several lines. There is one required line, at the top of the
> policy, indicating the policy name, and the policy version, for
> instance:
> 
>   policy_name="Ex Policy" policy_version=0.0.0
> 
> The policy version indicates the current version of the policy (NOT the
> policy syntax version). This is used to prevent roll-back of policy to
> potentially insecure previous versions of the policy.
> 
> The next portion of IPE policy, are rules. Rules are formed by key=value
> pairs, known as properties. IPE rules require two properties: "action",

Better:

IPE rules require two keys:

> which determines what IPE does when it encounters a match against the
> policy, and "op", which determines when that rule should be evaluated.
> Thus, a minimal rule is:
> 
>   op=EXECUTE action=ALLOW
> 
> This example will allow any execution. Additional properties are used to
> restrict attributes about the files being evaluated. These properties are
> intended to be deterministic attributes that are resident in the kernel.
> Available properties for IPE described in the documentation patch of this
> series.
> 
> A rule is required to have the "op" property as the first token of a rule,
> and the "action" as the last token of the rule. Rules are evaluated
> top-to-bottom. As a result, any revocation rules, or denies should be
> placed early in the file to ensure that these rules are evaluated before
> a rule with "action=ALLOW" is hit.
> 
> Any unknown syntax in IPE policy will result in a fatal error to parse
> the policy. User mode can interrogate the kernel to understand what
> properties and the associated versions through the securityfs node,
> $securityfs/ipe/config, which will return a string of form:
> 
>   key1=version1
>   key2=version2
>   .
>   .
>   .
>   keyN=versionN
> 
> User-mode should correlate these versions with the supported values
> identified in the documentation to determine whether a policy should
> be accepted by the system without actually trying to deploy the policy.
> 
> Additionally, a DEFAULT operation must be set for all understood
> operations within IPE. For policies to remain completely forwards
> compatible, it is recommended that users add a "DEFAULT action=ALLOW"
> and override the defaults on a per-operation basis.
> 
> For more information about the policy syntax, the kernel documentation
> page.
> 
> Early Usermode Protection:
> --------------------------
> 
> IPE can be provided with a policy at startup to load and enforce.
> This is intended to be a minimal policy to get the system to a state
> where userland is setup and ready to receive commands, at which
> point a policy can be deployed via securityfs. This "boot policy" can be
> specified via the config, SECURITY_IPE_BOOT_POLICY, which accepts a path
> to a plain-text version of the IPE policy to apply. This policy will be
> compiled into the kernel. If not specified, IPE will be disabled until a
> policy is deployed and activated through the method above.
> 
> Policy Examples:
> ----------------
> 
> Allow all:
> 
>   policy_name="Allow All" policy_version=0.0.0
>   DEFAULT action=ALLOW
> 
> Allow only initial superblock:
> 
>   policy_name="Allow All Initial SB" policy_version=0.0.0
>   DEFAULT action=DENY
> 
>   op=EXECUTE boot_verified=TRUE action=ALLOW
> 
> Allow any signed dm-verity volume and the initial superblock:
> 
>   policy_name="AllowSignedAndInitial" policy_version=0.0.0
>   DEFAULT action=DENY
> 
>   op=EXECUTE boot_verified=TRUE action=ALLOW
>   op=EXECUTE dmverity_signature=TRUE action=ALLOW
> 
> Prohibit execution from a specific dm-verity volume:
> 
>   policy_name="AllowSignedAndInitial" policy_version=0.0.0
>   DEFAULT action=DENY
> 
>   op=EXECUTE
> dmverity_roothash=401fcec5944823ae12f62726e8184407a5fa9599783f030dec
> 146938 action=DENY
>   op=EXECUTE boot_verified=TRUE action=ALLOW
>   op=EXECUTE dmverity_signature=TRUE action=ALLOW
> 
> Allow only a specific dm-verity volume:
> 
>   policy_name="AllowSignedAndInitial" policy_version=0.0.0
>   DEFAULT action=DENY
> 
>   op=EXECUTE
> dmverity_roothash=401fcec5944823ae12f62726e8184407a5fa9599783f030dec
> 146938 action=ALLOW
> 
> Deploying Policies:
> -------------------
> 
> First sign a plain text policy, with a certificate that is present in
> the SYSTEM_TRUSTED_KEYRING of your test machine. Through openssl, the
> signing can be done via:
> 
>   openssl smime -sign -in "$MY_POLICY" -signer "$MY_CERTIFICATE" \
>     -inkey "$MY_PRIVATE_KEY" -binary -outform der -noattr -nodetach \
>     -out "$MY_POLICY.p7s"
> 
> Then, simply cat the file into the IPE's "new_policy" securityfs node:
> 
>   cat "$MY_POLICY.p7s" > /sys/kernel/security/ipe/new_policy
> 
> The policy should now be present under the policies/ subdirectory, under
> its "policy_name" attribute.
> 
> The policy is now present in the kernel and can be marked as active,
> via the securityfs node:
> 
>   echo "1" > "/sys/kernel/security/ipe/$MY_POLICY_NAME/active"
> 
> This will now mark the policy as active and the system will be enforcing
> $MY_POLICY_NAME.
> 
> There is one requirement when marking a policy as active, the policy_version
> attribute must either increase, or remain the same as the currently running
> policy.
> 
> Policies can be updated via:
> 
>   cat "$MY_UPDATED_POLICY.p7s" > \
>     "/sys/kernel/security/ipe/policies/$MY_POLICY_NAME/update"
> 
> Additionally, policies can be deleted via the "delete" securityfs
> node. Simply write "1" to the corresponding node in the policy folder:
> 
>   echo "1" > "/sys/kernel/security/ipe/policies/$MY_POLICY_NAME/delete"
> 
> There is only one requirement to delete policies, the policy being
> deleted must not be the active policy.
> 
> NOTE: The securityfs commands will require CAP_MAC_ADMIN.
> 
> Integrations:
> -------------
> 
> This patch series adds support for fsverity via digest and signature
> (fsverity_signature and fsverity_digest), dm-verity by digest and
> signature (dmverity_signature and dmverity_roothash), and trust for
> the initramfs (boot_verified).

Verifying the initial ram disk looks like a big problem. On general
purpose OSes, having a reference value for it would be very hard.

Instead, we would still be able to use per file reference values.
Executable and shared libraries in the initial ram disk are copied
from the main OS. Without fsverity support in tmpfs, I wonder
if it would be still possible to mark the file as immutable and do
an on the fly calculation of the root digest.

As an alternative, the IMA approach of calculating the file digest
could be used (or IPE could get the file digest as a property from
the integrity subsystem).

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> Please see the documentation patch for more information about the
> integrations available.
> 
> Testing:
> --------
> 
> KUnit Tests are available. Recommended kunitconfig:
> 
>     CONFIG_KUNIT=y
>     CONFIG_SECURITY=y
>     CONFIG_SECURITYFS=y
>     CONFIG_PKCS7_MESSAGE_PARSER=y
>     CONFIG_SYSTEM_DATA_VERIFICATION=y
>     CONFIG_FS_VERITY=y
>     CONFIG_FS_VERITY_BUILTIN_SIGNATURES=y
>     CONFIG_BLOCK=y
>     CONFIG_MD=y
>     CONFIG_BLK_DEV_DM=y
>     CONFIG_DM_VERITY=y
>     CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG=y
> 
>     CONFIG_SECURITY_IPE=y
>     CONFIG_SECURITY_IPE_KUNIT_TEST=y
>     CONFIG_IPE_PROP_BOOT_VERIFIED=y
>     CONFIG_IPE_PROP_DM_VERITY_SIGNATURE=y
>     CONFIG_IPE_PROP_DM_VERITY_ROOTHASH=y
>     CONFIG_IPE_PROP_FS_VERITY_SIGNATURE=y
>     CONFIG_IPE_PROP_FS_VERITY_DIGEST=y
> 
> Simply run:
> 
>     make ARCH=um mrproper
>     ./tools/testing/kunit/kunit.py run --kunitconfig <path/to/config>
> 
> And the tests will execute and report the result. For more indepth testing,
> it will require you to create and mount a dm-verity volume or fs-verity
> enabled file.
> 
> Documentation:
> --------------
> 
> There is both documentation available on github at
> https://microsoft.github.io/ipe, and Documentation in this patch series,
> to be added in-tree. This includes architectural block diagrams.
> 
> Known Gaps:
> -----------
> 
> IPE has two known gaps:
> 
> 1. IPE cannot verify the integrity of anonymous executable memory, such as
>   the trampolines created by gcc closures and libffi (<3.4.2), or JIT'd code.
>   Unfortunately, as this is dynamically generated code, there is no way
>   for IPE to ensure the integrity of this code to form a trust basis. In all
>   cases, the return result for these operations will be whatever the admin
>   configures the DEFAULT action for "EXECUTE".
> 
> 2. IPE cannot verify the integrity of interpreted languages' programs when
>   these scripts invoked via ``<interpreter> <file>``. This is because the
>   way interpreters execute these files, the scripts themselves are not
>   evaluated as executable code through one of IPE's hooks. Interpreters
>   can be enlightened to the usage of IPE by trying to mmap a file into
>   executable memory (+X), after opening the file and responding to the
>   error code appropriately. This also applies to included files, or high
>   value files, such as configuration files of critical system components.
>   However, there is a patchset that is looking to address this gap [1].
> 
> Appendix:
> ---------
> 
> A. IPE Github Repository: https://github.com/microsoft/ipe
> B. IPE Users' Guide: Documentation/admin-guide/LSM/ipe.rst
> 
> References:
> -----------
> 
> [1] https://lore.kernel.org/all/20211012192410.2356090-1-mic@digikod.net/
> 
> FAQ:
> ----
> 
> Q: What's the difference between other LSMs which provide trust-based
>   access control, for instance, IMA?
> 
> A: IMA is a fantastic option when needing measurement in addition to the
>   trust-based access model. All of IMA is centered around their measurement
>   hashes, so you save time when doing both actions. IPE, on the other hand,
>   is a highly performant system that does not rely (and explicitly prohibits),
>   generating its own integrity mechanisms - separating measurement and access
>   control. Simply put, IPE provides only the enforcement of trust, while other
>   subsystems provide the integrity guarantee that IPE needs to determine the
>   trust of a resource. IMA provides both the integrity guarantee, the
>   enforcement of trust, and a whole host of other features that may not be
>   needed.
> 
> Changelog:
> ----------
> 
> Changes since v1:
>   Split the second patch of the previous series into two.
>   Minor corrections in the cover-letter and documentation
>   comments regarding CAP_MAC_ADMIN checks in IPE.
> 
> Changes since v2:
>   Address various comments by Jann Horn. Highlights:
>     Switch various audit allocators to GFP_KERNEL.
>     Utilize rcu_access_pointer() in various locations.
>     Strip out the caching system for properties
>     Strip comments from headers
>     Move functions around in patches
>     Remove kernel command line parameters
>     Reconcile the race condition on the delete node for policy by
>       expanding the policy critical section.
> 
>   Address a few comments by Jonathan Corbet around the documentation
>     pages for IPE.
> 
>   Fix an issue with the initialization of IPE policy with a "-0"
>     version, caused by not initializing the hlist entries before
>     freeing.
> 
> Changes since v3:
>   Address a concern around IPE's behavior with unknown syntax.
>     Specifically, make any unknown syntax a fatal error instead of a
>     warning, as suggested by Mickaël Salaün.
>   Introduce a new securityfs node, $securityfs/ipe/property_config,
>     which provides a listing of what properties are enabled by the
>     kernel and their versions. This allows usermode to predict what
>     policies should be allowed.
>   Strip some comments from c files that I missed.
>   Clarify some documentation comments around 'boot_verified'.
>     While this currently does not functionally change the property
>     itself, the distinction is important when IPE can enforce verified
>     reads. Additionally, 'KERNEL_READ' was omitted from the documentation.
>     This has been corrected.
>   Change SecurityFS and SHA1 to a reverse dependency.
>   Update the cover-letter with the updated behavior of unknown syntax.
>   Remove all sysctls, making an equivalent function in securityfs.
>   Rework the active/delete mechanism to be a node under the policy in
>     $securityfs/ipe/policies.
>   The kernel command line parameters ipe.enforce and ipe.success_audit
>     have returned as this functionality is no longer exposed through
>     sysfs.
> 
> Changes since v4:
>   Correct some grammatical errors reported by Randy Dunlap.
>   Fix some warnings reported by kernel test bot.
>   Change convention around security_bdev_setsecurity. -ENOSYS
>     is now expected if an LSM does not implement a particular @name,
>     as suggested by Casey Schaufler.
>   Minor string corrections related to the move from sysfs to securityfs
>   Correct a spelling of an #ifdef for the permissive argument.
>   Add the kernel parameters re-added to the documentation.
>   Fix a minor bug where the mode being audited on permissive switch
>     was the original mode, not the mode being swapped to.
>   Cleanup doc comments, fix some whitespace alignment issues.
> 
> Changes since v5:
>   Change if statement condition in security_bdev_setsecurity to be
>     more concise, as suggested by Casey Schaufler and Al Viro
>   Drop the 6th patch in the series, "dm-verity move signature check..."
>     due to numerous issues, and it ultimately providing no real value.
>   Fix the patch tree - the previous iteration appears to have been in a
>     torn state (patches 8+9 were merged). This has since been corrected.
> 
> Changes since v6:
>   * Reword cover letter to more accurate convey IPE's purpose
>     and latest updates.
>   * Refactor series to:
>       1. Support a context structure, enabling:
>           1. Easier Testing via KUNIT
>           2. A better architecture for future designs
>       2. Make parser code cleaner
>   * Move patch 01/12 to [14/16] of the series
>   * Split up patch 02/12 into four parts:
>       1. context creation [01/16]
>       2. audit [07/16]
>       3. evaluation loop [03/16]
>       4. access control hooks [05/16]
>       5. permissive mode [08/16]
>   * Split up patch 03/12 into two parts:
>       1. parser [02/16]
>       2. userspace interface [04/16]
>   * Reword and refactor patch 04/12 to [09/16]
>   * Squash patch 05/12, 07/12, 09/12 to [10/16]
>   * Squash patch 08/12, 10/12 to [11/16]
>   * Change audit records to MAC region (14XX) from Integrity region (18XX)
>   * Add FSVerity Support
>   * Interface changes:
>       1. "raw" was renamed to "pkcs7" and made read only
>       2. "raw"'s write functionality (update a policy) moved to "update"
>       3. introduced "version", "policy_name" nodes.
>       4. "content" renamed to "policy"
>       5. The boot policy can now be updated like any other policy.
>   * Add additional developer-level documentation
>   * Update admin-guide docs to reflect changes.
>   * Kunit tests
>   * Dropped CONFIG_SECURITY_IPE_PERMISSIVE_SWITCH - functionality can
>     easily come later with a small patch.
>   * Use partition0 for block_device for dm-verity patch
> 
> Deven Bowers (14):
>   security: add ipe lsm & initial context creation
>   ipe: add policy parser
>   ipe: add evaluation loop
>   ipe: add userspace interface
>   ipe: add LSM hooks on execution and kernel read
>   uapi|audit: add trust audit message definitions
>   ipe: add auditing support
>   ipe: add permissive toggle
>   ipe: introduce 'boot_verified' as a trust provider
>   fs|dm-verity: add block_dev LSM blob and submit dm-verity data
>   ipe: add support for dm-verity as a trust provider
>   scripts: add boot policy generation program
>   ipe: kunit tests
>   documentation: add ipe documentation
> 
> Fan Wu (2):
>   fsverity|security: add security hooks to fsverity digest and signature
>   ipe: enable support for fs-verity as a trust provider
> 
>  Documentation/admin-guide/LSM/index.rst       |    1 +
>  Documentation/admin-guide/LSM/ipe.rst         |  587 ++++++++++
>  .../admin-guide/kernel-parameters.txt         |   12 +
>  Documentation/security/index.rst              |    1 +
>  Documentation/security/ipe.rst                |  339 ++++++
>  MAINTAINERS                                   |    9 +
>  block/bdev.c                                  |    7 +
>  drivers/md/dm-verity-target.c                 |   20 +-
>  drivers/md/dm-verity-verify-sig.c             |   16 +-
>  drivers/md/dm-verity-verify-sig.h             |   10 +-
>  fs/verity/open.c                              |   12 +
>  fs/verity/signature.c                         |    5 +-
>  include/asm-generic/vmlinux.lds.h             |   16 +
>  include/linux/blk_types.h                     |    1 +
>  include/linux/device-mapper.h                 |    3 +
>  include/linux/fsverity.h                      |    3 +
>  include/linux/lsm_hook_defs.h                 |    5 +
>  include/linux/lsm_hooks.h                     |   12 +
>  include/linux/security.h                      |   22 +
>  include/uapi/linux/audit.h                    |    4 +
>  scripts/Makefile                              |    1 +
>  scripts/ipe/Makefile                          |    2 +
>  scripts/ipe/polgen/.gitignore                 |    1 +
>  scripts/ipe/polgen/Makefile                   |    6 +
>  scripts/ipe/polgen/polgen.c                   |  145 +++
>  security/Kconfig                              |   11 +-
>  security/Makefile                             |    1 +
>  security/ipe/.gitignore                       |    1 +
>  security/ipe/Kconfig                          |  100 ++
>  security/ipe/Makefile                         |   39 +
>  security/ipe/audit.c                          |  304 +++++
>  security/ipe/audit.h                          |   41 +
>  security/ipe/ctx.c                            |  381 ++++++
>  security/ipe/ctx.h                            |   43 +
>  security/ipe/ctx_test.c                       |  732 ++++++++++++
>  security/ipe/eval.c                           |  237 ++++
>  security/ipe/eval.h                           |   57 +
>  security/ipe/fs.c                             |  327 ++++++
>  security/ipe/fs.h                             |   13 +
>  security/ipe/hooks.c                          |  328 ++++++
>  security/ipe/hooks.h                          |   56 +
>  security/ipe/ipe.c                            |  143 +++
>  security/ipe/ipe.h                            |   27 +
>  security/ipe/ipe_parser.h                     |   59 +
>  security/ipe/modules.c                        |  134 +++
>  security/ipe/modules.h                        |   17 +
>  security/ipe/modules/Kconfig                  |   66 ++
>  security/ipe/modules/Makefile                 |   12 +
>  security/ipe/modules/boot_verified.c          |   24 +
>  security/ipe/modules/dmverity_roothash.c      |   80 ++
>  security/ipe/modules/dmverity_signature.c     |   25 +
>  security/ipe/modules/fsverity_digest.c        |   80 ++
>  security/ipe/modules/fsverity_signature.c     |   33 +
>  security/ipe/modules/ipe_module.h             |   40 +
>  security/ipe/parsers.c                        |  139 +++
>  security/ipe/parsers/Makefile                 |   12 +
>  security/ipe/parsers/default.c                |  106 ++
>  security/ipe/parsers/policy_header.c          |  126 ++
>  security/ipe/policy.c                         | 1037 +++++++++++++++++
>  security/ipe/policy.h                         |  113 ++
>  security/ipe/policy_parser_tests.c            |  299 +++++
>  security/ipe/policyfs.c                       |  528 +++++++++
>  security/security.c                           |   76 +-
>  63 files changed, 7069 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/admin-guide/LSM/ipe.rst
>  create mode 100644 Documentation/security/ipe.rst
>  create mode 100644 scripts/ipe/Makefile
>  create mode 100644 scripts/ipe/polgen/.gitignore
>  create mode 100644 scripts/ipe/polgen/Makefile
>  create mode 100644 scripts/ipe/polgen/polgen.c
>  create mode 100644 security/ipe/.gitignore
>  create mode 100644 security/ipe/Kconfig
>  create mode 100644 security/ipe/Makefile
>  create mode 100644 security/ipe/audit.c
>  create mode 100644 security/ipe/audit.h
>  create mode 100644 security/ipe/ctx.c
>  create mode 100644 security/ipe/ctx.h
>  create mode 100644 security/ipe/ctx_test.c
>  create mode 100644 security/ipe/eval.c
>  create mode 100644 security/ipe/eval.h
>  create mode 100644 security/ipe/fs.c
>  create mode 100644 security/ipe/fs.h
>  create mode 100644 security/ipe/hooks.c
>  create mode 100644 security/ipe/hooks.h
>  create mode 100644 security/ipe/ipe.c
>  create mode 100644 security/ipe/ipe.h
>  create mode 100644 security/ipe/ipe_parser.h
>  create mode 100644 security/ipe/modules.c
>  create mode 100644 security/ipe/modules.h
>  create mode 100644 security/ipe/modules/Kconfig
>  create mode 100644 security/ipe/modules/Makefile
>  create mode 100644 security/ipe/modules/boot_verified.c
>  create mode 100644 security/ipe/modules/dmverity_roothash.c
>  create mode 100644 security/ipe/modules/dmverity_signature.c
>  create mode 100644 security/ipe/modules/fsverity_digest.c
>  create mode 100644 security/ipe/modules/fsverity_signature.c
>  create mode 100644 security/ipe/modules/ipe_module.h
>  create mode 100644 security/ipe/parsers.c
>  create mode 100644 security/ipe/parsers/Makefile
>  create mode 100644 security/ipe/parsers/default.c
>  create mode 100644 security/ipe/parsers/policy_header.c
>  create mode 100644 security/ipe/policy.c
>  create mode 100644 security/ipe/policy.h
>  create mode 100644 security/ipe/policy_parser_tests.c
>  create mode 100644 security/ipe/policyfs.c
> 
> --
> 2.33.0

Re: [RFC PATCH v7 00/16] Integrity Policy Enforcement (IPE)

[Deven Bowers]

On 10/25/2021 4:30 AM, Roberto Sassu wrote:
>> From:deven.desai@linux.microsoft.com
>> [mailto:deven.desai@linux.microsoft.com]
>> From: Deven Bowers<deven.desai@linux.microsoft.com>
>>
>> Overview:
>> ---------
>>
>> IPE is a Linux Security Module which takes a complimentary approach to
>> access control. Whereas existing systems approach use labels or paths
>> which control access to a resource, IPE controls access to a resource
>> based on the system's trust of said resource.
> To me, it does not give a particularly precise idea of what IPE is about.
>
> It would have been more clear, assuming that I understood it correctly,
> if you have said:
>
> Whereas existing mandatory access control mechanisms base their
> decisions on labels and paths, IPE instead determines whether or not
> an operation should be allowed based on immutable security properties
> of the system component the operation is being performed on.
>
> IPE itself does not mandate how the security property should be
> evaluated, but relies on an extensible set of external property providers
> to evaluate the component. IPE makes its decision based on reference
> values for the selected properties, specified in the IPE policy.
>
> The reference values represent the value that the policy writer and the
> local system administrator (based on the policy signature) trust for the
> system to accomplish the desired tasks.
>
> One such provider is for example dm-verity, which is able to represent
> the integrity property of a partition (its immutable state) with a digest.
You understood it perfectly, and managed to word in a much more clear
way than I did. I'll apply these changes in the next posting! Thanks.
>> Trust requirements are established via IPE's policy, sourcing multiple
>> different implementations within the kernel to build a cohesive trust
>> model, based on how the system was built.
>>
>> Trust, with respect to computing, is a concept that designates a set
>> of entities who will endorse a set of resources as non-malicious.
>> Traditionally, this is done via signatures, which is the act of endorsing
>> a resource.
>>
>> Integrity, on the other hand, is the concept of ensuring that a resource
>> has not been modified since a point of time. This is typically done through
>> cryptographic hashes or signatures.
>>
>> Trust and integrity are very closely tied together concepts, as integrity
>> is the way you can prove trust for a resource; otherwise it could have
>> been modified by an entity who is untrusted.
>>
>> IPE provides a way for a user to express trust requirements of resources,
>> by using pre-existing systems which provide the integrity half of the
>> equation.
>>
>> IPE is compiled under CONFIG_SECURITY_IPE.
>>
>> Use Cases
>> ---------
>>
>> IPE works best in fixed-function devices: Devices in which their purpose
>> is clearly defined and not supposed to be changed (e.g. network firewall
>> device in a data center, an IoT device, etcetera), where all software and
>> configuration is built and provisioned by the system owner.
>>
>> IPE is a long-way off for use in general-purpose computing:
>> the Linux community as a whole tends to follow a decentralized trust
>> model, known as the Web of Trust, which IPE has no support for as of yet.
>> Instead, IPE supports the PKI Trust Model, which generally designates a
>> set of entities that provide a measure absolute trust.
> It is true that packages are signed with PGP, which is decentralized,
> but there is a special case where Linux distribution vendors trust
> their own keys. This, at least, would allow to trust the software built
> by a particular vendor (I ported David Howells's work on PGP keys and
> signature to the current kernel).
Yes, that is true. I figured that this scenario was somewhat obvious,
as it is, at a high level, similar to PKI but I can certainly add it
explicitly.
>> Additionally, while most packages are signed today, the files inside
>> the packages (for instance, the executables), tend to be unsigned. This
>> makes it difficult to utilize IPE in systems where a package manager is
>> expected to be functional, without major changes to the package manager
>> and ecosystem behind it.
> Yes, RPMs don't have per file signatures but have a signature of the
> list of file digests, which is equivalent. They could have also the fsverity
> digests (instead of the fsverity signatures) to reduce size overhead.
>
> Given that the authenticity of RPMs headers can be verified, if the
> PGP key of the vendor is included in the primary keyring of the kernel,
> being able to protect file or fsverity digests against tampering by
> user space and being able to query them (e.g. with DIGLIM) extends
> the applicability of IPE to general purpose OSes.
Agreed. With these two functionalities, it does appear that IPE + DIGLIM
can be used for general purpose RPM-based OSes. I'll add a reference to
your recent posting (v3?) as a way to extend the functionality to general
purposes OSes in the next revision.
>> Policy:
>> -------
>>
>> IPE policy is a plain-text [#]_ policy composed of multiple statements
>> over several lines. There is one required line, at the top of the
>> policy, indicating the policy name, and the policy version, for
>> instance:
>>
>>    policy_name="Ex Policy" policy_version=0.0.0
>>
>> The policy version indicates the current version of the policy (NOT the
>> policy syntax version). This is used to prevent roll-back of policy to
>> potentially insecure previous versions of the policy.
>>
>> The next portion of IPE policy, are rules. Rules are formed by key=value
>> pairs, known as properties. IPE rules require two properties: "action",
> Better:
>
> IPE rules require two keys:
Ack.
>> which determines what IPE does when it encounters a match against the
>> policy, and "op", which determines when that rule should be evaluated.
>> Thus, a minimal rule is:
>>
>>    op=EXECUTE action=ALLOW
>>
>> This example will allow any execution. Additional properties are used to
>> restrict attributes about the files being evaluated. These properties are
>> intended to be deterministic attributes that are resident in the kernel.
>> Available properties for IPE described in the documentation patch of this
>> series.
>>
>> A rule is required to have the "op" property as the first token of a rule,
>> and the "action" as the last token of the rule. Rules are evaluated
>> top-to-bottom. As a result, any revocation rules, or denies should be
>> placed early in the file to ensure that these rules are evaluated before
>> a rule with "action=ALLOW" is hit.
>>
>> Any unknown syntax in IPE policy will result in a fatal error to parse
>> the policy. User mode can interrogate the kernel to understand what
>> properties and the associated versions through the securityfs node,
>> $securityfs/ipe/config, which will return a string of form:
>>
>>    key1=version1
>>    key2=version2
>>    .
>>    .
>>    .
>>    keyN=versionN
>>
>> User-mode should correlate these versions with the supported values
>> identified in the documentation to determine whether a policy should
>> be accepted by the system without actually trying to deploy the policy.
>>
>> Additionally, a DEFAULT operation must be set for all understood
>> operations within IPE. For policies to remain completely forwards
>> compatible, it is recommended that users add a "DEFAULT action=ALLOW"
>> and override the defaults on a per-operation basis.
>>
>> For more information about the policy syntax, the kernel documentation
>> page.
>>
>> Early Usermode Protection:
>> --------------------------
>>
>> IPE can be provided with a policy at startup to load and enforce.
>> This is intended to be a minimal policy to get the system to a state
>> where userland is setup and ready to receive commands, at which
>> point a policy can be deployed via securityfs. This "boot policy" can be
>> specified via the config, SECURITY_IPE_BOOT_POLICY, which accepts a path
>> to a plain-text version of the IPE policy to apply. This policy will be
>> compiled into the kernel. If not specified, IPE will be disabled until a
>> policy is deployed and activated through the method above.
>>
>> Policy Examples:
>> ----------------
>>
>> Allow all:
>>
>>    policy_name="Allow All" policy_version=0.0.0
>>    DEFAULT action=ALLOW
>>
>> Allow only initial superblock:
>>
>>    policy_name="Allow All Initial SB" policy_version=0.0.0
>>    DEFAULT action=DENY
>>
>>    op=EXECUTE boot_verified=TRUE action=ALLOW
>>
>> Allow any signed dm-verity volume and the initial superblock:
>>
>>    policy_name="AllowSignedAndInitial" policy_version=0.0.0
>>    DEFAULT action=DENY
>>
>>    op=EXECUTE boot_verified=TRUE action=ALLOW
>>    op=EXECUTE dmverity_signature=TRUE action=ALLOW
>>
>> Prohibit execution from a specific dm-verity volume:
>>
>>    policy_name="AllowSignedAndInitial" policy_version=0.0.0
>>    DEFAULT action=DENY
>>
>>    op=EXECUTE
>> dmverity_roothash=401fcec5944823ae12f62726e8184407a5fa9599783f030dec
>> 146938 action=DENY
>>    op=EXECUTE boot_verified=TRUE action=ALLOW
>>    op=EXECUTE dmverity_signature=TRUE action=ALLOW
>>
>> Allow only a specific dm-verity volume:
>>
>>    policy_name="AllowSignedAndInitial" policy_version=0.0.0
>>    DEFAULT action=DENY
>>
>>    op=EXECUTE
>> dmverity_roothash=401fcec5944823ae12f62726e8184407a5fa9599783f030dec
>> 146938 action=ALLOW
>>
>> Deploying Policies:
>> -------------------
>>
>> First sign a plain text policy, with a certificate that is present in
>> the SYSTEM_TRUSTED_KEYRING of your test machine. Through openssl, the
>> signing can be done via:
>>
>>    openssl smime -sign -in "$MY_POLICY" -signer "$MY_CERTIFICATE" \
>>      -inkey "$MY_PRIVATE_KEY" -binary -outform der -noattr -nodetach \
>>      -out "$MY_POLICY.p7s"
>>
>> Then, simply cat the file into the IPE's "new_policy" securityfs node:
>>
>>    cat "$MY_POLICY.p7s" > /sys/kernel/security/ipe/new_policy
>>
>> The policy should now be present under the policies/ subdirectory, under
>> its "policy_name" attribute.
>>
>> The policy is now present in the kernel and can be marked as active,
>> via the securityfs node:
>>
>>    echo "1" > "/sys/kernel/security/ipe/$MY_POLICY_NAME/active"
>>
>> This will now mark the policy as active and the system will be enforcing
>> $MY_POLICY_NAME.
>>
>> There is one requirement when marking a policy as active, the policy_version
>> attribute must either increase, or remain the same as the currently running
>> policy.
>>
>> Policies can be updated via:
>>
>>    cat "$MY_UPDATED_POLICY.p7s" > \
>>      "/sys/kernel/security/ipe/policies/$MY_POLICY_NAME/update"
>>
>> Additionally, policies can be deleted via the "delete" securityfs
>> node. Simply write "1" to the corresponding node in the policy folder:
>>
>>    echo "1" > "/sys/kernel/security/ipe/policies/$MY_POLICY_NAME/delete"
>>
>> There is only one requirement to delete policies, the policy being
>> deleted must not be the active policy.
>>
>> NOTE: The securityfs commands will require CAP_MAC_ADMIN.
>>
>> Integrations:
>> -------------
>>
>> This patch series adds support for fsverity via digest and signature
>> (fsverity_signature and fsverity_digest), dm-verity by digest and
>> signature (dmverity_signature and dmverity_roothash), and trust for
>> the initramfs (boot_verified).
> Verifying the initial ram disk looks like a big problem. On general
> purpose OSes, having a reference value for it would be very hard.
>
> Instead, we would still be able to use per file reference values.
> Executable and shared libraries in the initial ram disk are copied
> from the main OS. Without fsverity support in tmpfs, I wonder
> if it would be still possible to mark the file as immutable and do
> an on the fly calculation of the root digest.
Yes, verifying the initial ramdisk is very difficult. "boot_verified",
is largely an assumption of trust as all the warning shows in the
documentation; it assumes the boot stack verified the initramfs somehow
(i.e. u-boot verified boot with it in the fitImage), and 'pins' (similar
to loadpin) the superblock to allow execution from that superblock.
> As an alternative, the IMA approach of calculating the file digest
> could be used (or IPE could get the file digest as a property from
> the integrity subsystem).
In general, I would like to keep as much of the implementation of the
integrity mechanisms out of IPE as much as possible - there are likely
much better layers to implement new ways of providing integrity /
authenticity claims than at the lsm layer within IPE.
> Roberto
>
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
>
>> Please see the documentation patch for more information about the
>> integrations available.
>>
>> Testing:
>> --------
>>
>> KUnit Tests are available. Recommended kunitconfig:
>>
>>      CONFIG_KUNIT=y
>>      CONFIG_SECURITY=y
>>      CONFIG_SECURITYFS=y
>>      CONFIG_PKCS7_MESSAGE_PARSER=y
>>      CONFIG_SYSTEM_DATA_VERIFICATION=y
>>      CONFIG_FS_VERITY=y
>>      CONFIG_FS_VERITY_BUILTIN_SIGNATURES=y
>>      CONFIG_BLOCK=y
>>      CONFIG_MD=y
>>      CONFIG_BLK_DEV_DM=y
>>      CONFIG_DM_VERITY=y
>>      CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG=y
>>
>>      CONFIG_SECURITY_IPE=y
>>      CONFIG_SECURITY_IPE_KUNIT_TEST=y
>>      CONFIG_IPE_PROP_BOOT_VERIFIED=y
>>      CONFIG_IPE_PROP_DM_VERITY_SIGNATURE=y
>>      CONFIG_IPE_PROP_DM_VERITY_ROOTHASH=y
>>      CONFIG_IPE_PROP_FS_VERITY_SIGNATURE=y
>>      CONFIG_IPE_PROP_FS_VERITY_DIGEST=y
>>
>> Simply run:
>>
>>      make ARCH=um mrproper
>>      ./tools/testing/kunit/kunit.py run --kunitconfig <path/to/config>
>>
>> And the tests will execute and report the result. For more indepth testing,
>> it will require you to create and mount a dm-verity volume or fs-verity
>> enabled file.
>>
>> Documentation:
>> --------------
>>
>> There is both documentation available on github at
>> https://microsoft.github.io/ipe, and Documentation in this patch series,
>> to be added in-tree. This includes architectural block diagrams.
>>
>> Known Gaps:
>> -----------
>>
>> IPE has two known gaps:
>>
>> 1. IPE cannot verify the integrity of anonymous executable memory, such as
>>    the trampolines created by gcc closures and libffi (<3.4.2), or JIT'd code.
>>    Unfortunately, as this is dynamically generated code, there is no way
>>    for IPE to ensure the integrity of this code to form a trust basis. In all
>>    cases, the return result for these operations will be whatever the admin
>>    configures the DEFAULT action for "EXECUTE".
>>
>> 2. IPE cannot verify the integrity of interpreted languages' programs when
>>    these scripts invoked via ``<interpreter> <file>``. This is because the
>>    way interpreters execute these files, the scripts themselves are not
>>    evaluated as executable code through one of IPE's hooks. Interpreters
>>    can be enlightened to the usage of IPE by trying to mmap a file into
>>    executable memory (+X), after opening the file and responding to the
>>    error code appropriately. This also applies to included files, or high
>>    value files, such as configuration files of critical system components.
>>    However, there is a patchset that is looking to address this gap [1].
>>
>> Appendix:
>> ---------
>>
>> A. IPE Github Repository:https://github.com/microsoft/ipe
>> B. IPE Users' Guide: Documentation/admin-guide/LSM/ipe.rst
>>
>> References:
>> -----------
>>
>> [1]https://lore.kernel.org/all/20211012192410.2356090-1-mic@digikod.net/
>>
>> FAQ:
>> ----
>>
>> Q: What's the difference between other LSMs which provide trust-based
>>    access control, for instance, IMA?
>>
>> A: IMA is a fantastic option when needing measurement in addition to the
>>    trust-based access model. All of IMA is centered around their measurement
>>    hashes, so you save time when doing both actions. IPE, on the other hand,
>>    is a highly performant system that does not rely (and explicitly prohibits),
>>    generating its own integrity mechanisms - separating measurement and access
>>    control. Simply put, IPE provides only the enforcement of trust, while other
>>    subsystems provide the integrity guarantee that IPE needs to determine the
>>    trust of a resource. IMA provides both the integrity guarantee, the
>>    enforcement of trust, and a whole host of other features that may not be
>>    needed.
>>
>> Changelog:
>> ----------
>>
>> Changes since v1:
>>    Split the second patch of the previous series into two.
>>    Minor corrections in the cover-letter and documentation
>>    comments regarding CAP_MAC_ADMIN checks in IPE.
>>
>> Changes since v2:
>>    Address various comments by Jann Horn. Highlights:
>>      Switch various audit allocators to GFP_KERNEL.
>>      Utilize rcu_access_pointer() in various locations.
>>      Strip out the caching system for properties
>>      Strip comments from headers
>>      Move functions around in patches
>>      Remove kernel command line parameters
>>      Reconcile the race condition on the delete node for policy by
>>        expanding the policy critical section.
>>
>>    Address a few comments by Jonathan Corbet around the documentation
>>      pages for IPE.
>>
>>    Fix an issue with the initialization of IPE policy with a "-0"
>>      version, caused by not initializing the hlist entries before
>>      freeing.
>>
>> Changes since v3:
>>    Address a concern around IPE's behavior with unknown syntax.
>>      Specifically, make any unknown syntax a fatal error instead of a
>>      warning, as suggested by Mickaël Salaün.
>>    Introduce a new securityfs node, $securityfs/ipe/property_config,
>>      which provides a listing of what properties are enabled by the
>>      kernel and their versions. This allows usermode to predict what
>>      policies should be allowed.
>>    Strip some comments from c files that I missed.
>>    Clarify some documentation comments around 'boot_verified'.
>>      While this currently does not functionally change the property
>>      itself, the distinction is important when IPE can enforce verified
>>      reads. Additionally, 'KERNEL_READ' was omitted from the documentation.
>>      This has been corrected.
>>    Change SecurityFS and SHA1 to a reverse dependency.
>>    Update the cover-letter with the updated behavior of unknown syntax.
>>    Remove all sysctls, making an equivalent function in securityfs.
>>    Rework the active/delete mechanism to be a node under the policy in
>>      $securityfs/ipe/policies.
>>    The kernel command line parameters ipe.enforce and ipe.success_audit
>>      have returned as this functionality is no longer exposed through
>>      sysfs.
>>
>> Changes since v4:
>>    Correct some grammatical errors reported by Randy Dunlap.
>>    Fix some warnings reported by kernel test bot.
>>    Change convention around security_bdev_setsecurity. -ENOSYS
>>      is now expected if an LSM does not implement a particular @name,
>>      as suggested by Casey Schaufler.
>>    Minor string corrections related to the move from sysfs to securityfs
>>    Correct a spelling of an #ifdef for the permissive argument.
>>    Add the kernel parameters re-added to the documentation.
>>    Fix a minor bug where the mode being audited on permissive switch
>>      was the original mode, not the mode being swapped to.
>>    Cleanup doc comments, fix some whitespace alignment issues.
>>
>> Changes since v5:
>>    Change if statement condition in security_bdev_setsecurity to be
>>      more concise, as suggested by Casey Schaufler and Al Viro
>>    Drop the 6th patch in the series, "dm-verity move signature check..."
>>      due to numerous issues, and it ultimately providing no real value.
>>    Fix the patch tree - the previous iteration appears to have been in a
>>      torn state (patches 8+9 were merged). This has since been corrected.
>>
>> Changes since v6:
>>    * Reword cover letter to more accurate convey IPE's purpose
>>      and latest updates.
>>    * Refactor series to:
>>        1. Support a context structure, enabling:
>>            1. Easier Testing via KUNIT
>>            2. A better architecture for future designs
>>        2. Make parser code cleaner
>>    * Move patch 01/12 to [14/16] of the series
>>    * Split up patch 02/12 into four parts:
>>        1. context creation [01/16]
>>        2. audit [07/16]
>>        3. evaluation loop [03/16]
>>        4. access control hooks [05/16]
>>        5. permissive mode [08/16]
>>    * Split up patch 03/12 into two parts:
>>        1. parser [02/16]
>>        2. userspace interface [04/16]
>>    * Reword and refactor patch 04/12 to [09/16]
>>    * Squash patch 05/12, 07/12, 09/12 to [10/16]
>>    * Squash patch 08/12, 10/12 to [11/16]
>>    * Change audit records to MAC region (14XX) from Integrity region (18XX)
>>    * Add FSVerity Support
>>    * Interface changes:
>>        1. "raw" was renamed to "pkcs7" and made read only
>>        2. "raw"'s write functionality (update a policy) moved to "update"
>>        3. introduced "version", "policy_name" nodes.
>>        4. "content" renamed to "policy"
>>        5. The boot policy can now be updated like any other policy.
>>    * Add additional developer-level documentation
>>    * Update admin-guide docs to reflect changes.
>>    * Kunit tests
>>    * Dropped CONFIG_SECURITY_IPE_PERMISSIVE_SWITCH - functionality can
>>      easily come later with a small patch.
>>    * Use partition0 for block_device for dm-verity patch
>>
>> Deven Bowers (14):
>>    security: add ipe lsm & initial context creation
>>    ipe: add policy parser
>>    ipe: add evaluation loop
>>    ipe: add userspace interface
>>    ipe: add LSM hooks on execution and kernel read
>>    uapi|audit: add trust audit message definitions
>>    ipe: add auditing support
>>    ipe: add permissive toggle
>>    ipe: introduce 'boot_verified' as a trust provider
>>    fs|dm-verity: add block_dev LSM blob and submit dm-verity data
>>    ipe: add support for dm-verity as a trust provider
>>    scripts: add boot policy generation program
>>    ipe: kunit tests
>>    documentation: add ipe documentation
>>
>> Fan Wu (2):
>>    fsverity|security: add security hooks to fsverity digest and signature
>>    ipe: enable support for fs-verity as a trust provider
>>
>>   Documentation/admin-guide/LSM/index.rst       |    1 +
>>   Documentation/admin-guide/LSM/ipe.rst         |  587 ++++++++++
>>   .../admin-guide/kernel-parameters.txt         |   12 +
>>   Documentation/security/index.rst              |    1 +
>>   Documentation/security/ipe.rst                |  339 ++++++
>>   MAINTAINERS                                   |    9 +
>>   block/bdev.c                                  |    7 +
>>   drivers/md/dm-verity-target.c                 |   20 +-
>>   drivers/md/dm-verity-verify-sig.c             |   16 +-
>>   drivers/md/dm-verity-verify-sig.h             |   10 +-
>>   fs/verity/open.c                              |   12 +
>>   fs/verity/signature.c                         |    5 +-
>>   include/asm-generic/vmlinux.lds.h             |   16 +
>>   include/linux/blk_types.h                     |    1 +
>>   include/linux/device-mapper.h                 |    3 +
>>   include/linux/fsverity.h                      |    3 +
>>   include/linux/lsm_hook_defs.h                 |    5 +
>>   include/linux/lsm_hooks.h                     |   12 +
>>   include/linux/security.h                      |   22 +
>>   include/uapi/linux/audit.h                    |    4 +
>>   scripts/Makefile                              |    1 +
>>   scripts/ipe/Makefile                          |    2 +
>>   scripts/ipe/polgen/.gitignore                 |    1 +
>>   scripts/ipe/polgen/Makefile                   |    6 +
>>   scripts/ipe/polgen/polgen.c                   |  145 +++
>>   security/Kconfig                              |   11 +-
>>   security/Makefile                             |    1 +
>>   security/ipe/.gitignore                       |    1 +
>>   security/ipe/Kconfig                          |  100 ++
>>   security/ipe/Makefile                         |   39 +
>>   security/ipe/audit.c                          |  304 +++++
>>   security/ipe/audit.h                          |   41 +
>>   security/ipe/ctx.c                            |  381 ++++++
>>   security/ipe/ctx.h                            |   43 +
>>   security/ipe/ctx_test.c                       |  732 ++++++++++++
>>   security/ipe/eval.c                           |  237 ++++
>>   security/ipe/eval.h                           |   57 +
>>   security/ipe/fs.c                             |  327 ++++++
>>   security/ipe/fs.h                             |   13 +
>>   security/ipe/hooks.c                          |  328 ++++++
>>   security/ipe/hooks.h                          |   56 +
>>   security/ipe/ipe.c                            |  143 +++
>>   security/ipe/ipe.h                            |   27 +
>>   security/ipe/ipe_parser.h                     |   59 +
>>   security/ipe/modules.c                        |  134 +++
>>   security/ipe/modules.h                        |   17 +
>>   security/ipe/modules/Kconfig                  |   66 ++
>>   security/ipe/modules/Makefile                 |   12 +
>>   security/ipe/modules/boot_verified.c          |   24 +
>>   security/ipe/modules/dmverity_roothash.c      |   80 ++
>>   security/ipe/modules/dmverity_signature.c     |   25 +
>>   security/ipe/modules/fsverity_digest.c        |   80 ++
>>   security/ipe/modules/fsverity_signature.c     |   33 +
>>   security/ipe/modules/ipe_module.h             |   40 +
>>   security/ipe/parsers.c                        |  139 +++
>>   security/ipe/parsers/Makefile                 |   12 +
>>   security/ipe/parsers/default.c                |  106 ++
>>   security/ipe/parsers/policy_header.c          |  126 ++
>>   security/ipe/policy.c                         | 1037 +++++++++++++++++
>>   security/ipe/policy.h                         |  113 ++
>>   security/ipe/policy_parser_tests.c            |  299 +++++
>>   security/ipe/policyfs.c                       |  528 +++++++++
>>   security/security.c                           |   76 +-
>>   63 files changed, 7069 insertions(+), 18 deletions(-)
>>   create mode 100644 Documentation/admin-guide/LSM/ipe.rst
>>   create mode 100644 Documentation/security/ipe.rst
>>   create mode 100644 scripts/ipe/Makefile
>>   create mode 100644 scripts/ipe/polgen/.gitignore
>>   create mode 100644 scripts/ipe/polgen/Makefile
>>   create mode 100644 scripts/ipe/polgen/polgen.c
>>   create mode 100644 security/ipe/.gitignore
>>   create mode 100644 security/ipe/Kconfig
>>   create mode 100644 security/ipe/Makefile
>>   create mode 100644 security/ipe/audit.c
>>   create mode 100644 security/ipe/audit.h
>>   create mode 100644 security/ipe/ctx.c
>>   create mode 100644 security/ipe/ctx.h
>>   create mode 100644 security/ipe/ctx_test.c
>>   create mode 100644 security/ipe/eval.c
>>   create mode 100644 security/ipe/eval.h
>>   create mode 100644 security/ipe/fs.c
>>   create mode 100644 security/ipe/fs.h
>>   create mode 100644 security/ipe/hooks.c
>>   create mode 100644 security/ipe/hooks.h
>>   create mode 100644 security/ipe/ipe.c
>>   create mode 100644 security/ipe/ipe.h
>>   create mode 100644 security/ipe/ipe_parser.h
>>   create mode 100644 security/ipe/modules.c
>>   create mode 100644 security/ipe/modules.h
>>   create mode 100644 security/ipe/modules/Kconfig
>>   create mode 100644 security/ipe/modules/Makefile
>>   create mode 100644 security/ipe/modules/boot_verified.c
>>   create mode 100644 security/ipe/modules/dmverity_roothash.c
>>   create mode 100644 security/ipe/modules/dmverity_signature.c
>>   create mode 100644 security/ipe/modules/fsverity_digest.c
>>   create mode 100644 security/ipe/modules/fsverity_signature.c
>>   create mode 100644 security/ipe/modules/ipe_module.h
>>   create mode 100644 security/ipe/parsers.c
>>   create mode 100644 security/ipe/parsers/Makefile
>>   create mode 100644 security/ipe/parsers/default.c
>>   create mode 100644 security/ipe/parsers/policy_header.c
>>   create mode 100644 security/ipe/policy.c
>>   create mode 100644 security/ipe/policy.h
>>   create mode 100644 security/ipe/policy_parser_tests.c
>>   create mode 100644 security/ipe/policyfs.c
>>
>> --
>> 2.33.0

RE: [RFC PATCH v7 00/16] Integrity Policy Enforcement (IPE)

[Roberto Sassu]

> From: Deven Bowers [mailto:deven.desai@linux.microsoft.com]
> Sent: Tuesday, October 26, 2021 9:04 PM
> On 10/25/2021 4:30 AM, Roberto Sassu wrote:
> >> From:deven.desai@linux.microsoft.com
> >> [mailto:deven.desai@linux.microsoft.com]
> >> From: Deven Bowers<deven.desai@linux.microsoft.com>
> >>
> >> Overview:
> >> ---------
> >>
> >> IPE is a Linux Security Module which takes a complimentary approach to
> >> access control. Whereas existing systems approach use labels or paths
> >> which control access to a resource, IPE controls access to a resource
> >> based on the system's trust of said resource.
> > To me, it does not give a particularly precise idea of what IPE is about.
> >
> > It would have been more clear, assuming that I understood it correctly,
> > if you have said:
> >
> > Whereas existing mandatory access control mechanisms base their
> > decisions on labels and paths, IPE instead determines whether or not
> > an operation should be allowed based on immutable security properties
> > of the system component the operation is being performed on.
> >
> > IPE itself does not mandate how the security property should be
> > evaluated, but relies on an extensible set of external property providers
> > to evaluate the component. IPE makes its decision based on reference
> > values for the selected properties, specified in the IPE policy.
> >
> > The reference values represent the value that the policy writer and the
> > local system administrator (based on the policy signature) trust for the
> > system to accomplish the desired tasks.
> >
> > One such provider is for example dm-verity, which is able to represent
> > the integrity property of a partition (its immutable state) with a digest.
> You understood it perfectly, and managed to word in a much more clear
> way than I did. I'll apply these changes in the next posting! Thanks.

Welcome.

> >> Trust requirements are established via IPE's policy, sourcing multiple
> >> different implementations within the kernel to build a cohesive trust
> >> model, based on how the system was built.
> >>
> >> Trust, with respect to computing, is a concept that designates a set
> >> of entities who will endorse a set of resources as non-malicious.
> >> Traditionally, this is done via signatures, which is the act of endorsing
> >> a resource.
> >>
> >> Integrity, on the other hand, is the concept of ensuring that a resource
> >> has not been modified since a point of time. This is typically done through
> >> cryptographic hashes or signatures.
> >>
> >> Trust and integrity are very closely tied together concepts, as integrity
> >> is the way you can prove trust for a resource; otherwise it could have
> >> been modified by an entity who is untrusted.
> >>
> >> IPE provides a way for a user to express trust requirements of resources,
> >> by using pre-existing systems which provide the integrity half of the
> >> equation.
> >>
> >> IPE is compiled under CONFIG_SECURITY_IPE.
> >>
> >> Use Cases
> >> ---------
> >>
> >> IPE works best in fixed-function devices: Devices in which their purpose
> >> is clearly defined and not supposed to be changed (e.g. network firewall
> >> device in a data center, an IoT device, etcetera), where all software and
> >> configuration is built and provisioned by the system owner.
> >>
> >> IPE is a long-way off for use in general-purpose computing:
> >> the Linux community as a whole tends to follow a decentralized trust
> >> model, known as the Web of Trust, which IPE has no support for as of yet.
> >> Instead, IPE supports the PKI Trust Model, which generally designates a
> >> set of entities that provide a measure absolute trust.
> > It is true that packages are signed with PGP, which is decentralized,
> > but there is a special case where Linux distribution vendors trust
> > their own keys. This, at least, would allow to trust the software built
> > by a particular vendor (I ported David Howells's work on PGP keys and
> > signature to the current kernel).
> Yes, that is true. I figured that this scenario was somewhat obvious,
> as it is, at a high level, similar to PKI but I can certainly add it
> explicitly.

Perfect.

> >> Additionally, while most packages are signed today, the files inside
> >> the packages (for instance, the executables), tend to be unsigned. This
> >> makes it difficult to utilize IPE in systems where a package manager is
> >> expected to be functional, without major changes to the package manager
> >> and ecosystem behind it.
> > Yes, RPMs don't have per file signatures but have a signature of the
> > list of file digests, which is equivalent. They could have also the fsverity
> > digests (instead of the fsverity signatures) to reduce size overhead.
> >
> > Given that the authenticity of RPMs headers can be verified, if the
> > PGP key of the vendor is included in the primary keyring of the kernel,
> > being able to protect file or fsverity digests against tampering by
> > user space and being able to query them (e.g. with DIGLIM) extends
> > the applicability of IPE to general purpose OSes.
> Agreed. With these two functionalities, it does appear that IPE + DIGLIM
> can be used for general purpose RPM-based OSes. I'll add a reference to
> your recent posting (v3?) as a way to extend the functionality to general
> purposes OSes in the next revision.

Ok. Yes, v3 is the latest.

> >> Policy:
> >> -------
> >>
> >> IPE policy is a plain-text [#]_ policy composed of multiple statements
> >> over several lines. There is one required line, at the top of the
> >> policy, indicating the policy name, and the policy version, for
> >> instance:
> >>
> >>    policy_name="Ex Policy" policy_version=0.0.0
> >>
> >> The policy version indicates the current version of the policy (NOT the
> >> policy syntax version). This is used to prevent roll-back of policy to
> >> potentially insecure previous versions of the policy.
> >>
> >> The next portion of IPE policy, are rules. Rules are formed by key=value
> >> pairs, known as properties. IPE rules require two properties: "action",
> > Better:
> >
> > IPE rules require two keys:
> Ack.
> >> which determines what IPE does when it encounters a match against the
> >> policy, and "op", which determines when that rule should be evaluated.
> >> Thus, a minimal rule is:
> >>
> >>    op=EXECUTE action=ALLOW
> >>
> >> This example will allow any execution. Additional properties are used to
> >> restrict attributes about the files being evaluated. These properties are
> >> intended to be deterministic attributes that are resident in the kernel.
> >> Available properties for IPE described in the documentation patch of this
> >> series.
> >>
> >> A rule is required to have the "op" property as the first token of a rule,
> >> and the "action" as the last token of the rule. Rules are evaluated
> >> top-to-bottom. As a result, any revocation rules, or denies should be
> >> placed early in the file to ensure that these rules are evaluated before
> >> a rule with "action=ALLOW" is hit.
> >>
> >> Any unknown syntax in IPE policy will result in a fatal error to parse
> >> the policy. User mode can interrogate the kernel to understand what
> >> properties and the associated versions through the securityfs node,
> >> $securityfs/ipe/config, which will return a string of form:
> >>
> >>    key1=version1
> >>    key2=version2
> >>    .
> >>    .
> >>    .
> >>    keyN=versionN
> >>
> >> User-mode should correlate these versions with the supported values
> >> identified in the documentation to determine whether a policy should
> >> be accepted by the system without actually trying to deploy the policy.
> >>
> >> Additionally, a DEFAULT operation must be set for all understood
> >> operations within IPE. For policies to remain completely forwards
> >> compatible, it is recommended that users add a "DEFAULT action=ALLOW"
> >> and override the defaults on a per-operation basis.
> >>
> >> For more information about the policy syntax, the kernel documentation
> >> page.
> >>
> >> Early Usermode Protection:
> >> --------------------------
> >>
> >> IPE can be provided with a policy at startup to load and enforce.
> >> This is intended to be a minimal policy to get the system to a state
> >> where userland is setup and ready to receive commands, at which
> >> point a policy can be deployed via securityfs. This "boot policy" can be
> >> specified via the config, SECURITY_IPE_BOOT_POLICY, which accepts a path
> >> to a plain-text version of the IPE policy to apply. This policy will be
> >> compiled into the kernel. If not specified, IPE will be disabled until a
> >> policy is deployed and activated through the method above.
> >>
> >> Policy Examples:
> >> ----------------
> >>
> >> Allow all:
> >>
> >>    policy_name="Allow All" policy_version=0.0.0
> >>    DEFAULT action=ALLOW
> >>
> >> Allow only initial superblock:
> >>
> >>    policy_name="Allow All Initial SB" policy_version=0.0.0
> >>    DEFAULT action=DENY
> >>
> >>    op=EXECUTE boot_verified=TRUE action=ALLOW
> >>
> >> Allow any signed dm-verity volume and the initial superblock:
> >>
> >>    policy_name="AllowSignedAndInitial" policy_version=0.0.0
> >>    DEFAULT action=DENY
> >>
> >>    op=EXECUTE boot_verified=TRUE action=ALLOW
> >>    op=EXECUTE dmverity_signature=TRUE action=ALLOW
> >>
> >> Prohibit execution from a specific dm-verity volume:
> >>
> >>    policy_name="AllowSignedAndInitial" policy_version=0.0.0
> >>    DEFAULT action=DENY
> >>
> >>    op=EXECUTE
> >>
> dmverity_roothash=401fcec5944823ae12f62726e8184407a5fa9599783f030dec
> >> 146938 action=DENY
> >>    op=EXECUTE boot_verified=TRUE action=ALLOW
> >>    op=EXECUTE dmverity_signature=TRUE action=ALLOW
> >>
> >> Allow only a specific dm-verity volume:
> >>
> >>    policy_name="AllowSignedAndInitial" policy_version=0.0.0
> >>    DEFAULT action=DENY
> >>
> >>    op=EXECUTE
> >>
> dmverity_roothash=401fcec5944823ae12f62726e8184407a5fa9599783f030dec
> >> 146938 action=ALLOW
> >>
> >> Deploying Policies:
> >> -------------------
> >>
> >> First sign a plain text policy, with a certificate that is present in
> >> the SYSTEM_TRUSTED_KEYRING of your test machine. Through openssl, the
> >> signing can be done via:
> >>
> >>    openssl smime -sign -in "$MY_POLICY" -signer "$MY_CERTIFICATE" \
> >>      -inkey "$MY_PRIVATE_KEY" -binary -outform der -noattr -nodetach \
> >>      -out "$MY_POLICY.p7s"
> >>
> >> Then, simply cat the file into the IPE's "new_policy" securityfs node:
> >>
> >>    cat "$MY_POLICY.p7s" > /sys/kernel/security/ipe/new_policy
> >>
> >> The policy should now be present under the policies/ subdirectory, under
> >> its "policy_name" attribute.
> >>
> >> The policy is now present in the kernel and can be marked as active,
> >> via the securityfs node:
> >>
> >>    echo "1" > "/sys/kernel/security/ipe/$MY_POLICY_NAME/active"
> >>
> >> This will now mark the policy as active and the system will be enforcing
> >> $MY_POLICY_NAME.
> >>
> >> There is one requirement when marking a policy as active, the policy_version
> >> attribute must either increase, or remain the same as the currently running
> >> policy.
> >>
> >> Policies can be updated via:
> >>
> >>    cat "$MY_UPDATED_POLICY.p7s" > \
> >>      "/sys/kernel/security/ipe/policies/$MY_POLICY_NAME/update"
> >>
> >> Additionally, policies can be deleted via the "delete" securityfs
> >> node. Simply write "1" to the corresponding node in the policy folder:
> >>
> >>    echo "1" > "/sys/kernel/security/ipe/policies/$MY_POLICY_NAME/delete"
> >>
> >> There is only one requirement to delete policies, the policy being
> >> deleted must not be the active policy.
> >>
> >> NOTE: The securityfs commands will require CAP_MAC_ADMIN.
> >>
> >> Integrations:
> >> -------------
> >>
> >> This patch series adds support for fsverity via digest and signature
> >> (fsverity_signature and fsverity_digest), dm-verity by digest and
> >> signature (dmverity_signature and dmverity_roothash), and trust for
> >> the initramfs (boot_verified).
> > Verifying the initial ram disk looks like a big problem. On general
> > purpose OSes, having a reference value for it would be very hard.
> >
> > Instead, we would still be able to use per file reference values.
> > Executable and shared libraries in the initial ram disk are copied
> > from the main OS. Without fsverity support in tmpfs, I wonder
> > if it would be still possible to mark the file as immutable and do
> > an on the fly calculation of the root digest.
> Yes, verifying the initial ramdisk is very difficult. "boot_verified",
> is largely an assumption of trust as all the warning shows in the
> documentation; it assumes the boot stack verified the initramfs somehow
> (i.e. u-boot verified boot with it in the fitImage), and 'pins' (similar
> to loadpin) the superblock to allow execution from that superblock.
> > As an alternative, the IMA approach of calculating the file digest
> > could be used (or IPE could get the file digest as a property from
> > the integrity subsystem).
> In general, I would like to keep as much of the implementation of the
> integrity mechanisms out of IPE as much as possible - there are likely
> much better layers to implement new ways of providing integrity /
> authenticity claims than at the lsm layer within IPE.

That would be still the case. The integrity subsystem will be still
responsible to calculate the file digest and maintain it in a per
inode metadata. Then, IPE could evaluate the file digest as the
same as for the fsverity digest:

op=EXECUTE integrity_digest=<hex> action=ALLOW

integrity_digest will be handled by a separate IPE module which
communicates with the integrity subsystem.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> > Roberto
> >
> > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> > Managing Director: Li Peng, Zhong Ronghua
> >
> >> Please see the documentation patch for more information about the
> >> integrations available.
> >>
> >> Testing:
> >> --------
> >>
> >> KUnit Tests are available. Recommended kunitconfig:
> >>
> >>      CONFIG_KUNIT=y
> >>      CONFIG_SECURITY=y
> >>      CONFIG_SECURITYFS=y
> >>      CONFIG_PKCS7_MESSAGE_PARSER=y
> >>      CONFIG_SYSTEM_DATA_VERIFICATION=y
> >>      CONFIG_FS_VERITY=y
> >>      CONFIG_FS_VERITY_BUILTIN_SIGNATURES=y
> >>      CONFIG_BLOCK=y
> >>      CONFIG_MD=y
> >>      CONFIG_BLK_DEV_DM=y
> >>      CONFIG_DM_VERITY=y
> >>      CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG=y
> >>
> >>      CONFIG_SECURITY_IPE=y
> >>      CONFIG_SECURITY_IPE_KUNIT_TEST=y
> >>      CONFIG_IPE_PROP_BOOT_VERIFIED=y
> >>      CONFIG_IPE_PROP_DM_VERITY_SIGNATURE=y
> >>      CONFIG_IPE_PROP_DM_VERITY_ROOTHASH=y
> >>      CONFIG_IPE_PROP_FS_VERITY_SIGNATURE=y
> >>      CONFIG_IPE_PROP_FS_VERITY_DIGEST=y
> >>
> >> Simply run:
> >>
> >>      make ARCH=um mrproper
> >>      ./tools/testing/kunit/kunit.py run --kunitconfig <path/to/config>
> >>
> >> And the tests will execute and report the result. For more indepth testing,
> >> it will require you to create and mount a dm-verity volume or fs-verity
> >> enabled file.
> >>
> >> Documentation:
> >> --------------
> >>
> >> There is both documentation available on github at
> >> https://microsoft.github.io/ipe, and Documentation in this patch series,
> >> to be added in-tree. This includes architectural block diagrams.
> >>
> >> Known Gaps:
> >> -----------
> >>
> >> IPE has two known gaps:
> >>
> >> 1. IPE cannot verify the integrity of anonymous executable memory, such as
> >>    the trampolines created by gcc closures and libffi (<3.4.2), or JIT'd code.
> >>    Unfortunately, as this is dynamically generated code, there is no way
> >>    for IPE to ensure the integrity of this code to form a trust basis. In all
> >>    cases, the return result for these operations will be whatever the admin
> >>    configures the DEFAULT action for "EXECUTE".
> >>
> >> 2. IPE cannot verify the integrity of interpreted languages' programs when
> >>    these scripts invoked via ``<interpreter> <file>``. This is because the
> >>    way interpreters execute these files, the scripts themselves are not
> >>    evaluated as executable code through one of IPE's hooks. Interpreters
> >>    can be enlightened to the usage of IPE by trying to mmap a file into
> >>    executable memory (+X), after opening the file and responding to the
> >>    error code appropriately. This also applies to included files, or high
> >>    value files, such as configuration files of critical system components.
> >>    However, there is a patchset that is looking to address this gap [1].
> >>
> >> Appendix:
> >> ---------
> >>
> >> A. IPE Github Repository:https://github.com/microsoft/ipe
> >> B. IPE Users' Guide: Documentation/admin-guide/LSM/ipe.rst
> >>
> >> References:
> >> -----------
> >>
> >> [1]https://lore.kernel.org/all/20211012192410.2356090-1-mic@digikod.net/
> >>
> >> FAQ:
> >> ----
> >>
> >> Q: What's the difference between other LSMs which provide trust-based
> >>    access control, for instance, IMA?
> >>
> >> A: IMA is a fantastic option when needing measurement in addition to the
> >>    trust-based access model. All of IMA is centered around their measurement
> >>    hashes, so you save time when doing both actions. IPE, on the other hand,
> >>    is a highly performant system that does not rely (and explicitly prohibits),
> >>    generating its own integrity mechanisms - separating measurement and
> access
> >>    control. Simply put, IPE provides only the enforcement of trust, while other
> >>    subsystems provide the integrity guarantee that IPE needs to determine the
> >>    trust of a resource. IMA provides both the integrity guarantee, the
> >>    enforcement of trust, and a whole host of other features that may not be
> >>    needed.
> >>
> >> Changelog:
> >> ----------
> >>
> >> Changes since v1:
> >>    Split the second patch of the previous series into two.
> >>    Minor corrections in the cover-letter and documentation
> >>    comments regarding CAP_MAC_ADMIN checks in IPE.
> >>
> >> Changes since v2:
> >>    Address various comments by Jann Horn. Highlights:
> >>      Switch various audit allocators to GFP_KERNEL.
> >>      Utilize rcu_access_pointer() in various locations.
> >>      Strip out the caching system for properties
> >>      Strip comments from headers
> >>      Move functions around in patches
> >>      Remove kernel command line parameters
> >>      Reconcile the race condition on the delete node for policy by
> >>        expanding the policy critical section.
> >>
> >>    Address a few comments by Jonathan Corbet around the documentation
> >>      pages for IPE.
> >>
> >>    Fix an issue with the initialization of IPE policy with a "-0"
> >>      version, caused by not initializing the hlist entries before
> >>      freeing.
> >>
> >> Changes since v3:
> >>    Address a concern around IPE's behavior with unknown syntax.
> >>      Specifically, make any unknown syntax a fatal error instead of a
> >>      warning, as suggested by Mickaël Salaün.
> >>    Introduce a new securityfs node, $securityfs/ipe/property_config,
> >>      which provides a listing of what properties are enabled by the
> >>      kernel and their versions. This allows usermode to predict what
> >>      policies should be allowed.
> >>    Strip some comments from c files that I missed.
> >>    Clarify some documentation comments around 'boot_verified'.
> >>      While this currently does not functionally change the property
> >>      itself, the distinction is important when IPE can enforce verified
> >>      reads. Additionally, 'KERNEL_READ' was omitted from the documentation.
> >>      This has been corrected.
> >>    Change SecurityFS and SHA1 to a reverse dependency.
> >>    Update the cover-letter with the updated behavior of unknown syntax.
> >>    Remove all sysctls, making an equivalent function in securityfs.
> >>    Rework the active/delete mechanism to be a node under the policy in
> >>      $securityfs/ipe/policies.
> >>    The kernel command line parameters ipe.enforce and ipe.success_audit
> >>      have returned as this functionality is no longer exposed through
> >>      sysfs.
> >>
> >> Changes since v4:
> >>    Correct some grammatical errors reported by Randy Dunlap.
> >>    Fix some warnings reported by kernel test bot.
> >>    Change convention around security_bdev_setsecurity. -ENOSYS
> >>      is now expected if an LSM does not implement a particular @name,
> >>      as suggested by Casey Schaufler.
> >>    Minor string corrections related to the move from sysfs to securityfs
> >>    Correct a spelling of an #ifdef for the permissive argument.
> >>    Add the kernel parameters re-added to the documentation.
> >>    Fix a minor bug where the mode being audited on permissive switch
> >>      was the original mode, not the mode being swapped to.
> >>    Cleanup doc comments, fix some whitespace alignment issues.
> >>
> >> Changes since v5:
> >>    Change if statement condition in security_bdev_setsecurity to be
> >>      more concise, as suggested by Casey Schaufler and Al Viro
> >>    Drop the 6th patch in the series, "dm-verity move signature check..."
> >>      due to numerous issues, and it ultimately providing no real value.
> >>    Fix the patch tree - the previous iteration appears to have been in a
> >>      torn state (patches 8+9 were merged). This has since been corrected.
> >>
> >> Changes since v6:
> >>    * Reword cover letter to more accurate convey IPE's purpose
> >>      and latest updates.
> >>    * Refactor series to:
> >>        1. Support a context structure, enabling:
> >>            1. Easier Testing via KUNIT
> >>            2. A better architecture for future designs
> >>        2. Make parser code cleaner
> >>    * Move patch 01/12 to [14/16] of the series
> >>    * Split up patch 02/12 into four parts:
> >>        1. context creation [01/16]
> >>        2. audit [07/16]
> >>        3. evaluation loop [03/16]
> >>        4. access control hooks [05/16]
> >>        5. permissive mode [08/16]
> >>    * Split up patch 03/12 into two parts:
> >>        1. parser [02/16]
> >>        2. userspace interface [04/16]
> >>    * Reword and refactor patch 04/12 to [09/16]
> >>    * Squash patch 05/12, 07/12, 09/12 to [10/16]
> >>    * Squash patch 08/12, 10/12 to [11/16]
> >>    * Change audit records to MAC region (14XX) from Integrity region (18XX)
> >>    * Add FSVerity Support
> >>    * Interface changes:
> >>        1. "raw" was renamed to "pkcs7" and made read only
> >>        2. "raw"'s write functionality (update a policy) moved to "update"
> >>        3. introduced "version", "policy_name" nodes.
> >>        4. "content" renamed to "policy"
> >>        5. The boot policy can now be updated like any other policy.
> >>    * Add additional developer-level documentation
> >>    * Update admin-guide docs to reflect changes.
> >>    * Kunit tests
> >>    * Dropped CONFIG_SECURITY_IPE_PERMISSIVE_SWITCH - functionality can
> >>      easily come later with a small patch.
> >>    * Use partition0 for block_device for dm-verity patch
> >>
> >> Deven Bowers (14):
> >>    security: add ipe lsm & initial context creation
> >>    ipe: add policy parser
> >>    ipe: add evaluation loop
> >>    ipe: add userspace interface
> >>    ipe: add LSM hooks on execution and kernel read
> >>    uapi|audit: add trust audit message definitions
> >>    ipe: add auditing support
> >>    ipe: add permissive toggle
> >>    ipe: introduce 'boot_verified' as a trust provider
> >>    fs|dm-verity: add block_dev LSM blob and submit dm-verity data
> >>    ipe: add support for dm-verity as a trust provider
> >>    scripts: add boot policy generation program
> >>    ipe: kunit tests
> >>    documentation: add ipe documentation
> >>
> >> Fan Wu (2):
> >>    fsverity|security: add security hooks to fsverity digest and signature
> >>    ipe: enable support for fs-verity as a trust provider
> >>
> >>   Documentation/admin-guide/LSM/index.rst       |    1 +
> >>   Documentation/admin-guide/LSM/ipe.rst         |  587 ++++++++++
> >>   .../admin-guide/kernel-parameters.txt         |   12 +
> >>   Documentation/security/index.rst              |    1 +
> >>   Documentation/security/ipe.rst                |  339 ++++++
> >>   MAINTAINERS                                   |    9 +
> >>   block/bdev.c                                  |    7 +
> >>   drivers/md/dm-verity-target.c                 |   20 +-
> >>   drivers/md/dm-verity-verify-sig.c             |   16 +-
> >>   drivers/md/dm-verity-verify-sig.h             |   10 +-
> >>   fs/verity/open.c                              |   12 +
> >>   fs/verity/signature.c                         |    5 +-
> >>   include/asm-generic/vmlinux.lds.h             |   16 +
> >>   include/linux/blk_types.h                     |    1 +
> >>   include/linux/device-mapper.h                 |    3 +
> >>   include/linux/fsverity.h                      |    3 +
> >>   include/linux/lsm_hook_defs.h                 |    5 +
> >>   include/linux/lsm_hooks.h                     |   12 +
> >>   include/linux/security.h                      |   22 +
> >>   include/uapi/linux/audit.h                    |    4 +
> >>   scripts/Makefile                              |    1 +
> >>   scripts/ipe/Makefile                          |    2 +
> >>   scripts/ipe/polgen/.gitignore                 |    1 +
> >>   scripts/ipe/polgen/Makefile                   |    6 +
> >>   scripts/ipe/polgen/polgen.c                   |  145 +++
> >>   security/Kconfig                              |   11 +-
> >>   security/Makefile                             |    1 +
> >>   security/ipe/.gitignore                       |    1 +
> >>   security/ipe/Kconfig                          |  100 ++
> >>   security/ipe/Makefile                         |   39 +
> >>   security/ipe/audit.c                          |  304 +++++
> >>   security/ipe/audit.h                          |   41 +
> >>   security/ipe/ctx.c                            |  381 ++++++
> >>   security/ipe/ctx.h                            |   43 +
> >>   security/ipe/ctx_test.c                       |  732 ++++++++++++
> >>   security/ipe/eval.c                           |  237 ++++
> >>   security/ipe/eval.h                           |   57 +
> >>   security/ipe/fs.c                             |  327 ++++++
> >>   security/ipe/fs.h                             |   13 +
> >>   security/ipe/hooks.c                          |  328 ++++++
> >>   security/ipe/hooks.h                          |   56 +
> >>   security/ipe/ipe.c                            |  143 +++
> >>   security/ipe/ipe.h                            |   27 +
> >>   security/ipe/ipe_parser.h                     |   59 +
> >>   security/ipe/modules.c                        |  134 +++
> >>   security/ipe/modules.h                        |   17 +
> >>   security/ipe/modules/Kconfig                  |   66 ++
> >>   security/ipe/modules/Makefile                 |   12 +
> >>   security/ipe/modules/boot_verified.c          |   24 +
> >>   security/ipe/modules/dmverity_roothash.c      |   80 ++
> >>   security/ipe/modules/dmverity_signature.c     |   25 +
> >>   security/ipe/modules/fsverity_digest.c        |   80 ++
> >>   security/ipe/modules/fsverity_signature.c     |   33 +
> >>   security/ipe/modules/ipe_module.h             |   40 +
> >>   security/ipe/parsers.c                        |  139 +++
> >>   security/ipe/parsers/Makefile                 |   12 +
> >>   security/ipe/parsers/default.c                |  106 ++
> >>   security/ipe/parsers/policy_header.c          |  126 ++
> >>   security/ipe/policy.c                         | 1037 +++++++++++++++++
> >>   security/ipe/policy.h                         |  113 ++
> >>   security/ipe/policy_parser_tests.c            |  299 +++++
> >>   security/ipe/policyfs.c                       |  528 +++++++++
> >>   security/security.c                           |   76 +-
> >>   63 files changed, 7069 insertions(+), 18 deletions(-)
> >>   create mode 100644 Documentation/admin-guide/LSM/ipe.rst
> >>   create mode 100644 Documentation/security/ipe.rst
> >>   create mode 100644 scripts/ipe/Makefile
> >>   create mode 100644 scripts/ipe/polgen/.gitignore
> >>   create mode 100644 scripts/ipe/polgen/Makefile
> >>   create mode 100644 scripts/ipe/polgen/polgen.c
> >>   create mode 100644 security/ipe/.gitignore
> >>   create mode 100644 security/ipe/Kconfig
> >>   create mode 100644 security/ipe/Makefile
> >>   create mode 100644 security/ipe/audit.c
> >>   create mode 100644 security/ipe/audit.h
> >>   create mode 100644 security/ipe/ctx.c
> >>   create mode 100644 security/ipe/ctx.h
> >>   create mode 100644 security/ipe/ctx_test.c
> >>   create mode 100644 security/ipe/eval.c
> >>   create mode 100644 security/ipe/eval.h
> >>   create mode 100644 security/ipe/fs.c
> >>   create mode 100644 security/ipe/fs.h
> >>   create mode 100644 security/ipe/hooks.c
> >>   create mode 100644 security/ipe/hooks.h
> >>   create mode 100644 security/ipe/ipe.c
> >>   create mode 100644 security/ipe/ipe.h
> >>   create mode 100644 security/ipe/ipe_parser.h
> >>   create mode 100644 security/ipe/modules.c
> >>   create mode 100644 security/ipe/modules.h
> >>   create mode 100644 security/ipe/modules/Kconfig
> >>   create mode 100644 security/ipe/modules/Makefile
> >>   create mode 100644 security/ipe/modules/boot_verified.c
> >>   create mode 100644 security/ipe/modules/dmverity_roothash.c
> >>   create mode 100644 security/ipe/modules/dmverity_signature.c
> >>   create mode 100644 security/ipe/modules/fsverity_digest.c
> >>   create mode 100644 security/ipe/modules/fsverity_signature.c
> >>   create mode 100644 security/ipe/modules/ipe_module.h
> >>   create mode 100644 security/ipe/parsers.c
> >>   create mode 100644 security/ipe/parsers/Makefile
> >>   create mode 100644 security/ipe/parsers/default.c
> >>   create mode 100644 security/ipe/parsers/policy_header.c
> >>   create mode 100644 security/ipe/policy.c
> >>   create mode 100644 security/ipe/policy.h
> >>   create mode 100644 security/ipe/policy_parser_tests.c
> >>   create mode 100644 security/ipe/policyfs.c
> >>
> >> --
> >> 2.33.0

Re: [RFC PATCH v7 00/16] Integrity Policy Enforcement (IPE)

[Deven Bowers]

On 10/27/2021 1:26 AM, Roberto Sassu wrote:
>> From: Deven Bowers [mailto:deven.desai@linux.microsoft.com]
>> Sent: Tuesday, October 26, 2021 9:04 PM
>> On 10/25/2021 4:30 AM, Roberto Sassu wrote:
>>>> From:deven.desai@linux.microsoft.com
>>>> [mailto:deven.desai@linux.microsoft.com]
>>>> From: Deven Bowers<deven.desai@linux.microsoft.com>
>>>>
>>>> Overview:
>>>> ---------
>>>>
>>>> IPE is a Linux Security Module which takes a complimentary approach to
>>>> access control. Whereas existing systems approach use labels or paths
>>>> which control access to a resource, IPE controls access to a resource
>>>> based on the system's trust of said resource.
>>> To me, it does not give a particularly precise idea of what IPE is about.
>>>
>>> It would have been more clear, assuming that I understood it correctly,
>>> if you have said:
>>>
>>> Whereas existing mandatory access control mechanisms base their
>>> decisions on labels and paths, IPE instead determines whether or not
>>> an operation should be allowed based on immutable security properties
>>> of the system component the operation is being performed on.
>>>
>>> IPE itself does not mandate how the security property should be
>>> evaluated, but relies on an extensible set of external property providers
>>> to evaluate the component. IPE makes its decision based on reference
>>> values for the selected properties, specified in the IPE policy.
>>>
>>> The reference values represent the value that the policy writer and the
>>> local system administrator (based on the policy signature) trust for the
>>> system to accomplish the desired tasks.
>>>
>>> One such provider is for example dm-verity, which is able to represent
>>> the integrity property of a partition (its immutable state) with a digest.
>> You understood it perfectly, and managed to word in a much more clear
>> way than I did. I'll apply these changes in the next posting! Thanks.
> Welcome.
>
>>>> Trust requirements are established via IPE's policy, sourcing multiple
>>>> different implementations within the kernel to build a cohesive trust
>>>> model, based on how the system was built.
>>>>
>>>> Trust, with respect to computing, is a concept that designates a set
>>>> of entities who will endorse a set of resources as non-malicious.
>>>> Traditionally, this is done via signatures, which is the act of endorsing
>>>> a resource.
>>>>
>>>> Integrity, on the other hand, is the concept of ensuring that a resource
>>>> has not been modified since a point of time. This is typically done through
>>>> cryptographic hashes or signatures.
>>>>
>>>> Trust and integrity are very closely tied together concepts, as integrity
>>>> is the way you can prove trust for a resource; otherwise it could have
>>>> been modified by an entity who is untrusted.
>>>>
>>>> IPE provides a way for a user to express trust requirements of resources,
>>>> by using pre-existing systems which provide the integrity half of the
>>>> equation.
>>>>
>>>> IPE is compiled under CONFIG_SECURITY_IPE.
>>>>
>>>> Use Cases
>>>> ---------
>>>>
>>>> IPE works best in fixed-function devices: Devices in which their purpose
>>>> is clearly defined and not supposed to be changed (e.g. network firewall
>>>> device in a data center, an IoT device, etcetera), where all software and
>>>> configuration is built and provisioned by the system owner.
>>>>
>>>> IPE is a long-way off for use in general-purpose computing:
>>>> the Linux community as a whole tends to follow a decentralized trust
>>>> model, known as the Web of Trust, which IPE has no support for as of yet.
>>>> Instead, IPE supports the PKI Trust Model, which generally designates a
>>>> set of entities that provide a measure absolute trust.
>>> It is true that packages are signed with PGP, which is decentralized,
>>> but there is a special case where Linux distribution vendors trust
>>> their own keys. This, at least, would allow to trust the software built
>>> by a particular vendor (I ported David Howells's work on PGP keys and
>>> signature to the current kernel).
>> Yes, that is true. I figured that this scenario was somewhat obvious,
>> as it is, at a high level, similar to PKI but I can certainly add it
>> explicitly.
> Perfect.
>
>>>> Additionally, while most packages are signed today, the files inside
>>>> the packages (for instance, the executables), tend to be unsigned. This
>>>> makes it difficult to utilize IPE in systems where a package manager is
>>>> expected to be functional, without major changes to the package manager
>>>> and ecosystem behind it.
>>> Yes, RPMs don't have per file signatures but have a signature of the
>>> list of file digests, which is equivalent. They could have also the fsverity
>>> digests (instead of the fsverity signatures) to reduce size overhead.
>>>
>>> Given that the authenticity of RPMs headers can be verified, if the
>>> PGP key of the vendor is included in the primary keyring of the kernel,
>>> being able to protect file or fsverity digests against tampering by
>>> user space and being able to query them (e.g. with DIGLIM) extends
>>> the applicability of IPE to general purpose OSes.
>> Agreed. With these two functionalities, it does appear that IPE + DIGLIM
>> can be used for general purpose RPM-based OSes. I'll add a reference to
>> your recent posting (v3?) as a way to extend the functionality to general
>> purposes OSes in the next revision.
> Ok. Yes, v3 is the latest.
>
>>>> Policy:
>>>> -------
>>>>
>>>> IPE policy is a plain-text [#]_ policy composed of multiple statements
>>>> over several lines. There is one required line, at the top of the
>>>> policy, indicating the policy name, and the policy version, for
>>>> instance:
>>>>
>>>>     policy_name="Ex Policy" policy_version=0.0.0
>>>>
>>>> The policy version indicates the current version of the policy (NOT the
>>>> policy syntax version). This is used to prevent roll-back of policy to
>>>> potentially insecure previous versions of the policy.
>>>>
>>>> The next portion of IPE policy, are rules. Rules are formed by key=value
>>>> pairs, known as properties. IPE rules require two properties: "action",
>>> Better:
>>>
>>> IPE rules require two keys:
>> Ack.
>>>> which determines what IPE does when it encounters a match against the
>>>> policy, and "op", which determines when that rule should be evaluated.
>>>> Thus, a minimal rule is:
>>>>
>>>>     op=EXECUTE action=ALLOW
>>>>
>>>> This example will allow any execution. Additional properties are used to
>>>> restrict attributes about the files being evaluated. These properties are
>>>> intended to be deterministic attributes that are resident in the kernel.
>>>> Available properties for IPE described in the documentation patch of this
>>>> series.
>>>>
>>>> A rule is required to have the "op" property as the first token of a rule,
>>>> and the "action" as the last token of the rule. Rules are evaluated
>>>> top-to-bottom. As a result, any revocation rules, or denies should be
>>>> placed early in the file to ensure that these rules are evaluated before
>>>> a rule with "action=ALLOW" is hit.
>>>>
>>>> Any unknown syntax in IPE policy will result in a fatal error to parse
>>>> the policy. User mode can interrogate the kernel to understand what
>>>> properties and the associated versions through the securityfs node,
>>>> $securityfs/ipe/config, which will return a string of form:
>>>>
>>>>     key1=version1
>>>>     key2=version2
>>>>     .
>>>>     .
>>>>     .
>>>>     keyN=versionN
>>>>
>>>> User-mode should correlate these versions with the supported values
>>>> identified in the documentation to determine whether a policy should
>>>> be accepted by the system without actually trying to deploy the policy.
>>>>
>>>> Additionally, a DEFAULT operation must be set for all understood
>>>> operations within IPE. For policies to remain completely forwards
>>>> compatible, it is recommended that users add a "DEFAULT action=ALLOW"
>>>> and override the defaults on a per-operation basis.
>>>>
>>>> For more information about the policy syntax, the kernel documentation
>>>> page.
>>>>
>>>> Early Usermode Protection:
>>>> --------------------------
>>>>
>>>> IPE can be provided with a policy at startup to load and enforce.
>>>> This is intended to be a minimal policy to get the system to a state
>>>> where userland is setup and ready to receive commands, at which
>>>> point a policy can be deployed via securityfs. This "boot policy" can be
>>>> specified via the config, SECURITY_IPE_BOOT_POLICY, which accepts a path
>>>> to a plain-text version of the IPE policy to apply. This policy will be
>>>> compiled into the kernel. If not specified, IPE will be disabled until a
>>>> policy is deployed and activated through the method above.
>>>>
>>>> Policy Examples:
>>>> ----------------
>>>>
>>>> Allow all:
>>>>
>>>>     policy_name="Allow All" policy_version=0.0.0
>>>>     DEFAULT action=ALLOW
>>>>
>>>> Allow only initial superblock:
>>>>
>>>>     policy_name="Allow All Initial SB" policy_version=0.0.0
>>>>     DEFAULT action=DENY
>>>>
>>>>     op=EXECUTE boot_verified=TRUE action=ALLOW
>>>>
>>>> Allow any signed dm-verity volume and the initial superblock:
>>>>
>>>>     policy_name="AllowSignedAndInitial" policy_version=0.0.0
>>>>     DEFAULT action=DENY
>>>>
>>>>     op=EXECUTE boot_verified=TRUE action=ALLOW
>>>>     op=EXECUTE dmverity_signature=TRUE action=ALLOW
>>>>
>>>> Prohibit execution from a specific dm-verity volume:
>>>>
>>>>     policy_name="AllowSignedAndInitial" policy_version=0.0.0
>>>>     DEFAULT action=DENY
>>>>
>>>>     op=EXECUTE
>>>>
>> dmverity_roothash=401fcec5944823ae12f62726e8184407a5fa9599783f030dec
>>>> 146938 action=DENY
>>>>     op=EXECUTE boot_verified=TRUE action=ALLOW
>>>>     op=EXECUTE dmverity_signature=TRUE action=ALLOW
>>>>
>>>> Allow only a specific dm-verity volume:
>>>>
>>>>     policy_name="AllowSignedAndInitial" policy_version=0.0.0
>>>>     DEFAULT action=DENY
>>>>
>>>>     op=EXECUTE
>>>>
>> dmverity_roothash=401fcec5944823ae12f62726e8184407a5fa9599783f030dec
>>>> 146938 action=ALLOW
>>>>
>>>> Deploying Policies:
>>>> -------------------
>>>>
>>>> First sign a plain text policy, with a certificate that is present in
>>>> the SYSTEM_TRUSTED_KEYRING of your test machine. Through openssl, the
>>>> signing can be done via:
>>>>
>>>>     openssl smime -sign -in "$MY_POLICY" -signer "$MY_CERTIFICATE" \
>>>>       -inkey "$MY_PRIVATE_KEY" -binary -outform der -noattr -nodetach \
>>>>       -out "$MY_POLICY.p7s"
>>>>
>>>> Then, simply cat the file into the IPE's "new_policy" securityfs node:
>>>>
>>>>     cat "$MY_POLICY.p7s" > /sys/kernel/security/ipe/new_policy
>>>>
>>>> The policy should now be present under the policies/ subdirectory, under
>>>> its "policy_name" attribute.
>>>>
>>>> The policy is now present in the kernel and can be marked as active,
>>>> via the securityfs node:
>>>>
>>>>     echo "1" > "/sys/kernel/security/ipe/$MY_POLICY_NAME/active"
>>>>
>>>> This will now mark the policy as active and the system will be enforcing
>>>> $MY_POLICY_NAME.
>>>>
>>>> There is one requirement when marking a policy as active, the policy_version
>>>> attribute must either increase, or remain the same as the currently running
>>>> policy.
>>>>
>>>> Policies can be updated via:
>>>>
>>>>     cat "$MY_UPDATED_POLICY.p7s" > \
>>>>       "/sys/kernel/security/ipe/policies/$MY_POLICY_NAME/update"
>>>>
>>>> Additionally, policies can be deleted via the "delete" securityfs
>>>> node. Simply write "1" to the corresponding node in the policy folder:
>>>>
>>>>     echo "1" > "/sys/kernel/security/ipe/policies/$MY_POLICY_NAME/delete"
>>>>
>>>> There is only one requirement to delete policies, the policy being
>>>> deleted must not be the active policy.
>>>>
>>>> NOTE: The securityfs commands will require CAP_MAC_ADMIN.
>>>>
>>>> Integrations:
>>>> -------------
>>>>
>>>> This patch series adds support for fsverity via digest and signature
>>>> (fsverity_signature and fsverity_digest), dm-verity by digest and
>>>> signature (dmverity_signature and dmverity_roothash), and trust for
>>>> the initramfs (boot_verified).
>>> Verifying the initial ram disk looks like a big problem. On general
>>> purpose OSes, having a reference value for it would be very hard.
>>>
>>> Instead, we would still be able to use per file reference values.
>>> Executable and shared libraries in the initial ram disk are copied
>>> from the main OS. Without fsverity support in tmpfs, I wonder
>>> if it would be still possible to mark the file as immutable and do
>>> an on the fly calculation of the root digest.
>> Yes, verifying the initial ramdisk is very difficult. "boot_verified",
>> is largely an assumption of trust as all the warning shows in the
>> documentation; it assumes the boot stack verified the initramfs somehow
>> (i.e. u-boot verified boot with it in the fitImage), and 'pins' (similar
>> to loadpin) the superblock to allow execution from that superblock.
>>> As an alternative, the IMA approach of calculating the file digest
>>> could be used (or IPE could get the file digest as a property from
>>> the integrity subsystem).
>> In general, I would like to keep as much of the implementation of the
>> integrity mechanisms out of IPE as much as possible - there are likely
>> much better layers to implement new ways of providing integrity /
>> authenticity claims than at the lsm layer within IPE.
> That would be still the case. The integrity subsystem will be still
> responsible to calculate the file digest and maintain it in a per
> inode metadata. Then, IPE could evaluate the file digest as the
> same as for the fsverity digest:
>
> op=EXECUTE integrity_digest=<hex> action=ALLOW
>
> integrity_digest will be handled by a separate IPE module which
> communicates with the integrity subsystem.

Sure, I'm happy with this. My comment was originally to the first half
of your response ("the IMA approach of calculating the file digest
could be used"); I don't see that as part of IPE's purpose.

I wanted to draw as rough boundary between what I find acceptable
as an IPE extension and what isn't.

RE: [RFC PATCH v7 04/16] ipe: add userspace interface

[Roberto Sassu]

> From: deven.desai@linux.microsoft.com
> [mailto:deven.desai@linux.microsoft.com]
> From: Deven Bowers <deven.desai@linux.microsoft.com>
> 
> As is typical with LSMs, IPE uses securityfs as its interface with
> userspace. for a complete list of the interfaces and the respective
> inputs/outputs, please see the documentation under
> admin-guide/LSM/ipe.rst
> 
> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> ---
> 
> Relevant changes since v6:
>   * Refactor series to:
>       1. Support a context structure, enabling easier testing
>   * Split up patch 03/12 into two parts:
>       1. parser [02/16]
>       2. userspace interface [04/16] (this patch)
>   * Interface changes:
>       1. "raw" was renamed to "pkcs7" and made read only
>       2. "raw"'s write functionality (update a policy) moved to "update"
>       3. introduced "version", "policy_name" nodes.
>       4. "content" renamed to "policy"
>       5. The boot policy can now be updated like any other policy.
> 
> ---
>  security/ipe/Makefile   |   2 +
>  security/ipe/ctx.c      | 121 +++++++++
>  security/ipe/ctx.h      |   6 +
>  security/ipe/fs.c       | 170 +++++++++++++
>  security/ipe/fs.h       |  13 +
>  security/ipe/policy.c   |  41 ++++
>  security/ipe/policy.h   |   4 +
>  security/ipe/policyfs.c | 528 ++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 885 insertions(+)
>  create mode 100644 security/ipe/fs.c
>  create mode 100644 security/ipe/fs.h
>  create mode 100644 security/ipe/policyfs.c
> 
> diff --git a/security/ipe/Makefile b/security/ipe/Makefile
> index 0db69f13e82a..d5660a17364c 100644
> --- a/security/ipe/Makefile
> +++ b/security/ipe/Makefile
> @@ -10,9 +10,11 @@ ccflags-y := -I$(srctree)/security/ipe/modules
>  obj-$(CONFIG_SECURITY_IPE) += \
>  	ctx.o \
>  	eval.o \
> +	fs.o \
>  	hooks.o \
>  	ipe.o \
>  	modules.o \
>  	parsers/ \
>  	parsers.o \
>  	policy.o \
> +	policyfs.o \
> diff --git a/security/ipe/ctx.c b/security/ipe/ctx.c
> index 9274e51eff52..664c671a4f9c 100644
> --- a/security/ipe/ctx.c
> +++ b/security/ipe/ctx.c
> @@ -13,6 +13,29 @@
>  #include <linux/refcount.h>
>  #include <linux/spinlock.h>
> 
> +/**
> + * ver_to_u64: convert an internal ipe_policy_version to a u64
> + * @p: Policy to extract the version from
> + *
> + * Bits (LSB is index 0):
> + *	[48,32] -> Major
> + *	[32,16] -> Minor
> + *	[16, 0] -> Revision
> + *
> + * Return:
> + * u64 version of the embedded version structure.
> + */
> +static inline u64 ver_to_u64(const struct ipe_policy *const p)
> +{
> +	u64 r = 0;
> +
> +	r = (((u64)p->parsed->version.major) << 32)
> +	  | (((u64)p->parsed->version.minor) << 16)
> +	  | ((u64)(p->parsed->version.rev));
> +
> +	return r;
> +}
> +
>  /**
>   * ipe_current_ctx: Helper to retrieve the ipe_context for the current task.
>   *
> @@ -96,6 +119,7 @@ static void free_ctx_work(struct work_struct *const
> work)
>  	list_for_each_entry(p, &ctx->policies, next)
>  		ipe_put_policy(p);
> 
> +	securityfs_remove(ctx->policy_root);
>  	kfree(ctx);
>  }
> 
> @@ -160,6 +184,9 @@ void ipe_remove_policy(struct ipe_policy *p)
>   * ipe_add_policy: Associate @p with @ctx
>   * @ctx: Supplies a pointer to the ipe_context structure to associate @p with.
>   * @p: Supplies a pointer to the ipe_policy structure to associate.
> + *
> + * This will increase @p's reference count by one.
> + *
>   */
>  void ipe_add_policy(struct ipe_context *ctx, struct ipe_policy *p)
>  {
> @@ -168,7 +195,101 @@ void ipe_add_policy(struct ipe_context *ctx, struct
> ipe_policy *p)
>  	list_add_tail(&p->next, &ctx->policies);
>  	refcount_inc(&p->refcount);
>  	spin_unlock(&ctx->lock);
> +}
> +
> +/**
> + * ipe_replace_policy: Replace @old with @new in the list of policies in @ctx
> + * @ctx: Supplies the context object to manipulate.
> + * @old: Supplies a pointer to the ipe_policy to replace with @new
> + * @new: Supplies a pointer to the ipe_policy structure to replace @old with
> + */
> +int ipe_replace_policy(struct ipe_policy *old, struct ipe_policy *new)
> +{
> +	int rc = -EINVAL;
> +	struct ipe_context *ctx;
> +	struct ipe_policy *cursor;
> +	struct ipe_policy *p = NULL;
> +
> +	ctx = ipe_get_ctx_rcu(old->ctx);
> +	if (!ctx)
> +		return -ENOENT;
> +
> +	spin_lock(&ctx->lock);
> +	list_for_each_entry(cursor, &ctx->policies, next) {
> +		if (!strcmp(old->parsed->name, cursor->parsed->name)) {
> +			if (ipe_is_policy_active(old)) {
> +				if (ver_to_u64(old) > ver_to_u64(new))
> +					break;
> +				rcu_assign_pointer(ctx->active_policy, new);
> +			}
> +			list_replace_init(&cursor->next, &new->next);
> +			refcount_inc(&new->refcount);
> +			rcu_assign_pointer(new->ctx, old->ctx);
> +			p = cursor;
> +			rc = 0;
> +			break;
> +		}
> +	}
> +	spin_unlock(&ctx->lock);
> +	synchronize_rcu();
> +
> +	ipe_put_policy(p);
> +	ipe_put_ctx(ctx);
> +	return rc;
> +}
> +
> +/**
> + * ipe_set_active_pol: Make @p the active policy.
> + * @p: Supplies a pointer to the policy to make active.
> + */
> +int ipe_set_active_pol(const struct ipe_policy *p)
> +{
> +	int rc = 0;
> +	struct ipe_policy *ap = NULL;
> +	struct ipe_context *ctx = NULL;
> +
> +	ctx = ipe_get_ctx_rcu(p->ctx);
> +	if (!ctx) {
> +		rc = -ENOENT;
> +		goto out;
> +	}
> +
> +	ap = ipe_get_policy_rcu(ctx->active_policy);
> +	if (ap && ver_to_u64(ap) > ver_to_u64(p)) {
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +
> +	spin_lock(&ctx->lock);
> +	rcu_assign_pointer(ctx->active_policy, p);
> +	spin_unlock(&ctx->lock);
>  	synchronize_rcu();
> +
> +out:
> +	ipe_put_policy(ap);
> +	ipe_put_ctx(ctx);
> +	return rc;
> +}
> +
> +/**
> + * ipe_is_policy_active: Determine wehther @p is the active policy
> + * @p: Supplies a pointer to the policy to check.
> + *
> + * Return:
> + * true - @p is the active policy of @ctx
> + * false - @p is not the active policy of @ctx
> + */
> +bool ipe_is_policy_active(const struct ipe_policy *p)
> +{
> +	bool rv;
> +	struct ipe_context *ctx;
> +
> +	rcu_read_lock();
> +	ctx = rcu_dereference(p->ctx);
> +	rv = !IS_ERR_OR_NULL(ctx) && rcu_access_pointer(ctx->active_policy)
> == p;
> +	rcu_read_unlock();
> +
> +	return rv;
>  }
> 
>  /**
> diff --git a/security/ipe/ctx.h b/security/ipe/ctx.h
> index a0da92da818c..fe11fb767788 100644
> --- a/security/ipe/ctx.h
> +++ b/security/ipe/ctx.h
> @@ -7,6 +7,7 @@
> 
>  #include <linux/sched.h>
>  #include <linux/types.h>
> +#include <linux/dcache.h>
>  #include <linux/refcount.h>
>  #include <linux/spinlock.h>
>  #include <linux/workqueue.h>
> @@ -20,6 +21,8 @@ struct ipe_context {
> 
>  	struct list_head policies; /* type: ipe_policy */
> 
> +	struct dentry *policy_root;
> +
>  	struct work_struct free_work;
>  };
> 
> @@ -30,5 +33,8 @@ struct ipe_context *ipe_get_ctx_rcu(struct ipe_context
> __rcu *ctx);
>  void ipe_put_ctx(struct ipe_context *ctx);
>  void ipe_add_policy(struct ipe_context *ctx, struct ipe_policy *p);
>  void ipe_remove_policy(struct ipe_policy *p);
> +int ipe_replace_policy(struct ipe_policy *old, struct ipe_policy *new);
> +int ipe_set_active_pol(const struct ipe_policy *p);
> +bool ipe_is_policy_active(const struct ipe_policy *p);
> 
>  #endif /* IPE_CONTEXT_H */
> diff --git a/security/ipe/fs.c b/security/ipe/fs.c
> new file mode 100644
> index 000000000000..10ad23f8bf92
> --- /dev/null
> +++ b/security/ipe/fs.c
> @@ -0,0 +1,170 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) Microsoft Corporation. All rights reserved.
> + */
> +#include "ipe.h"
> +#include "fs.h"
> +#include "policy.h"
> +
> +#include <linux/dcache.h>
> +#include <linux/security.h>
> +
> +static struct dentry *np __ro_after_init;
> +static struct dentry *root __ro_after_init;
> +static struct dentry *config __ro_after_init;
> +
> +/**
> + * new_policy: Write handler for the securityfs node, "ipe/new_policy"
> + * @f: Supplies a file structure representing the securityfs node.
> + * @data: Suppleis a buffer passed to the write syscall
> + * @len: Supplies the length of @data
> + * @offset: unused.
> + *
> + * Return:
> + * >0 - Success, Length of buffer written
> + * <0 - Error
> + */
> +static ssize_t new_policy(struct file *f, const char __user *data,
> +			  size_t len, loff_t *offset)
> +{
> +	int rc = 0;
> +	char *copy = NULL;
> +	struct ipe_policy *p = NULL;
> +	struct ipe_context *ctx = NULL;
> +
> +	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
> +		return -EPERM;
> +
> +	ctx = ipe_current_ctx();
> +
> +	copy = memdup_user_nul(data, len);
> +	if (IS_ERR(copy)) {
> +		rc = PTR_ERR(copy);
> +		goto err;
> +	}
> +
> +	p = ipe_new_policy(NULL, 0, copy, len);
> +	if (IS_ERR(p)) {
> +		rc = PTR_ERR(p);
> +		goto err;
> +	}
> +
> +	rc = ipe_new_policyfs_node(ctx, p);
> +	if (rc)
> +		goto err;
> +
> +	ipe_add_policy(ctx, p);
> +err:
> +	ipe_put_policy(p);
> +	ipe_put_ctx(ctx);
> +	return (rc < 0) ? rc : len;
> +}
> +
> +/**
> + * get_config: Read handler for the securityfs node, "ipe/config"
> + * @f: Supplies a file structure representing the securityfs node.
> + * @data: Supplies a buffer passed to the read syscall
> + * @len: Supplies the length of @data
> + * @offset: unused.
> + *
> + * Return:
> + * >0 - Success, Length of buffer written
> + * <0 - Error
> + */
> +static ssize_t get_config(struct file *f, char __user *data, size_t len,
> +			  loff_t *offset)
> +{
> +	int rc = 0;
> +	char *buf = NULL;
> +	size_t buflen = 0;
> +	char tmp[30] = { 0 };
> +	struct ipe_parser *p = NULL;
> +	struct ipe_module *m = NULL;
> +
> +	for (p = __start_ipe_parsers; p < __end_ipe_parsers; ++p)
> +		buflen += snprintf(NULL, 0, "%s=%d\n", p->first_token, p-
> >version);
> +	for (m = __start_ipe_modules; m < __end_ipe_modules; ++m)
> +		buflen += snprintf(NULL, 0, "%s=%d\n", m->name, m->version);
> +
> +	++buflen;
> +	buf = kzalloc(buflen, GFP_KERNEL);
> +	if (!buf) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
> +	for (p = __start_ipe_parsers; p < __end_ipe_parsers; ++p) {
> +		memset(tmp, 0x0, ARRAY_SIZE(tmp));
> +		scnprintf(tmp, ARRAY_SIZE(tmp), "%s=%d\n", p->first_token, p-
> >version);
> +		strcat(buf, tmp);
> +	}
> +
> +	for (m = __start_ipe_modules; m < __end_ipe_modules; ++m) {
> +		memset(tmp, 0x0, ARRAY_SIZE(tmp));
> +		scnprintf(tmp, ARRAY_SIZE(tmp), "%s=%d\n", m->name, m-
> >version);
> +		strcat(buf, tmp);
> +	}
> +
> +	rc = simple_read_from_buffer(data, len, offset, buf, buflen);
> +out:
> +	kfree(buf);
> +	return rc;
> +}
> +
> +static const struct file_operations cfg_fops = {
> +	.read = get_config,
> +};
> +
> +static const struct file_operations np_fops = {
> +	.write = new_policy,
> +};
> +
> +/**
> + * ipe_init_securityfs: Initialize IPE's securityfs tree at fsinit
> + *
> + * Return:
> + * !0 - Error
> + * 0 - OK
> + */
> +static int __init ipe_init_securityfs(void)
> +{
> +	int rc = 0;
> +	struct ipe_context *ctx = NULL;
> +
> +	ctx = ipe_current_ctx();

Hi Deven

the instruction above should be executed only if IPE LSM is
enabled. Otherwise, the kernel panics due to the illegal access
to the security blob of the task.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> +
> +	root = securityfs_create_dir("ipe", NULL);
> +	if (IS_ERR(root)) {
> +		rc = PTR_ERR(root);
> +		goto err;
> +	}
> +
> +	np = securityfs_create_file("new_policy", 0200, root, NULL, &np_fops);
> +	if (IS_ERR(np)) {
> +		rc = PTR_ERR(np);
> +		goto err;
> +	}
> +
> +	config = securityfs_create_file("config", 0400, root, NULL,
> +					&cfg_fops);
> +	if (IS_ERR(config)) {
> +		rc = PTR_ERR(config);
> +		goto err;
> +	}
> +
> +	ctx->policy_root = securityfs_create_dir("policies", root);
> +	if (IS_ERR(ctx->policy_root)) {
> +		rc = PTR_ERR(ctx->policy_root);
> +		goto err;
> +	}
> +
> +	return 0;
> +err:
> +	securityfs_remove(np);
> +	securityfs_remove(root);
> +	securityfs_remove(config);
> +	securityfs_remove(ctx->policy_root);
> +	return rc;
> +}
> +
> +fs_initcall(ipe_init_securityfs);
> diff --git a/security/ipe/fs.h b/security/ipe/fs.h
> new file mode 100644
> index 000000000000..4ab2f4e8c454
> --- /dev/null
> +++ b/security/ipe/fs.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) Microsoft Corporation. All rights reserved.
> + */
> +
> +#ifndef IPE_FS_H
> +#define IPE_FS_H
> +
> +void ipe_soft_del_policyfs(struct ipe_policy *p);
> +int ipe_new_policyfs_node(struct ipe_context *ctx, struct ipe_policy *p);
> +void ipe_del_policyfs_node(struct ipe_policy *p);
> +
> +#endif /* IPE_FS_H */
> diff --git a/security/ipe/policy.c b/security/ipe/policy.c
> index 8970f96453d6..b766824cc08f 100644
> --- a/security/ipe/policy.c
> +++ b/security/ipe/policy.c
> @@ -4,6 +4,7 @@
>   */
> 
>  #include "ipe.h"
> +#include "fs.h"
>  #include "policy.h"
>  #include "ipe_parser.h"
>  #include "modules.h"
> @@ -867,6 +868,8 @@ void ipe_put_policy(struct ipe_policy *p)
>  	if (IS_ERR_OR_NULL(p) || !refcount_dec_and_test(&p->refcount))
>  		return;
> 
> +	ipe_del_policyfs_node(p);
> +	securityfs_remove(p->policyfs);
>  	free_parsed_policy(p->parsed);
>  	if (!p->pkcs7)
>  		kfree(p->text);
> @@ -911,6 +914,44 @@ static int set_pkcs7_data(void *ctx, const void *data,
> size_t len,
>  	return 0;
>  }
> 
> +/**
> + * ipe_update_policy: parse a new policy and replace @old with it.
> + * @old: Supplies a pointer to the policy to replace
> + * @text: Supplies a pointer to the plain text policy
> + * @textlen: Supplies the length of @text
> + * @pkcs7: Supplies a pointer to a buffer containing a pkcs7 message.
> + * @pkcs7len: Supplies the length of @pkcs7len
> + *
> + * @text/@textlen is mutually exclusive with @pkcs7/@pkcs7len - see
> + * ipe_new_policy.
> + *
> + * Return:
> + * !IS_ERR - OK
> + */
> +struct ipe_policy *ipe_update_policy(struct ipe_policy *old,
> +				     const char *text, size_t textlen,
> +				     const char *pkcs7, size_t pkcs7len)
> +{
> +	int rc = 0;
> +	struct ipe_policy *new;
> +
> +	new = ipe_new_policy(text, textlen, pkcs7, pkcs7len);
> +	if (IS_ERR(new)) {
> +		rc = PTR_ERR(new);
> +		goto err;
> +	}
> +
> +	if (strcmp(new->parsed->name, old->parsed->name)) {
> +		rc = -EINVAL;
> +		goto err;
> +	}
> +
> +	rc = ipe_replace_policy(old, new);
> +err:
> +	ipe_put_policy(new);
> +	return (rc < 0) ? ERR_PTR(rc) : new;
> +}
> +
>  /**
>   * ipe_new_policy: allocate and parse an ipe_policy structure.
>   *
> diff --git a/security/ipe/policy.h b/security/ipe/policy.h
> index 2b5041c5a75a..6818f6405dd0 100644
> --- a/security/ipe/policy.h
> +++ b/security/ipe/policy.h
> @@ -88,12 +88,16 @@ struct ipe_policy {
> 
>  	refcount_t	refcount;
> 
> +	struct dentry *policyfs;
>  	struct list_head next;		/* type: ipe_policy */
>  	struct ipe_context __rcu *ctx;
>  };
> 
>  struct ipe_policy *ipe_new_policy(const char *text, size_t textlen,
>  				  const char *pkcs7, size_t pkcs7len);
> +struct ipe_policy *ipe_update_policy(struct ipe_policy *old, const char *text,
> +				     size_t textlen, const char *pkcs7,
> +				     size_t pkcs7len);
>  void ipe_put_policy(struct ipe_policy *pol);
>  bool ipe_is_op_alias(int op, const enum ipe_operation **map, size_t *size);
>  struct ipe_policy *ipe_get_policy_rcu(struct ipe_policy __rcu *p);
> diff --git a/security/ipe/policyfs.c b/security/ipe/policyfs.c
> new file mode 100644
> index 000000000000..d34c22e99225
> --- /dev/null
> +++ b/security/ipe/policyfs.c
> @@ -0,0 +1,528 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) Microsoft Corporation. All rights reserved.
> + */
> +#include "ipe.h"
> +#include "policy.h"
> +#include "fs.h"
> +
> +#include <linux/fs.h>
> +#include <linux/namei.h>
> +#include <linux/types.h>
> +#include <linux/dcache.h>
> +#include <linux/security.h>
> +
> +#define MAX_VERSION_SIZE ARRAY_SIZE("65535.65535.65535")
> +
> +/**
> + * find_policy: Follow the i_private field of a dentry, returning the address
> + *		of the resulting policy structure.
> + * @f: Securityfs object that contains a link to the dentry containing the
> + *     policy structure.
> + *
> + * Return:
> + * Always-Valid Address Pointer
> + */
> +static inline struct ipe_policy __rcu **find_policy(struct file *f)
> +{
> +	struct dentry *link;
> +
> +	link = d_inode(f->f_path.dentry)->i_private;
> +
> +	return (struct ipe_policy __rcu **)&(d_inode(link)->i_private);
> +}
> +
> +/**
> + * ipefs_file: defines a file in securityfs
> + */
> +struct ipefs_file {
> +	const char	*name;
> +	umode_t		access;
> +	const struct	file_operations *fops;
> +};
> +
> +/**
> + * read_pkcs7: Read handler for the securityfs node,
> + *	       "ipe/policies/$name/pkcs7"
> + * @f: Supplies a file structure representing the securityfs node.
> + * @data: Suppleis a buffer passed to the write syscall
> + * @len: Supplies the length of @data
> + * @offset: unused.
> + *
> + * @data will be populated with the pkcs7 blob representing the policy
> + * on success. If the policy is unsigned (like the boot policy), this
> + * will return -ENOENT.
> + *
> + * Return:
> + * >0 - Success, Length of buffer written
> + * <0 - Error
> + */
> +static ssize_t read_pkcs7(struct file *f, char __user *data,
> +			  size_t len, loff_t *offset)
> +{
> +	int rc = 0;
> +	struct ipe_policy *p = NULL;
> +
> +	p = ipe_get_policy_rcu(*find_policy(f));
> +	if (!p)
> +		return -ENOENT;
> +
> +	if (!p->pkcs7) {
> +		rc = -ENOENT;
> +		goto out;
> +	}
> +
> +	rc = simple_read_from_buffer(data, len, offset, p->pkcs7, p->pkcs7len);
> +
> +out:
> +	ipe_put_policy(p);
> +	return rc;
> +}
> +
> +/**
> + * read_policy: Read handler for the securityfs node,
> + *		"ipe/policies/$name/policy"
> + * @f: Supplies a file structure representing the securityfs node.
> + * @data: Suppleis a buffer passed to the write syscall
> + * @len: Supplies the length of @data
> + * @offset: unused.
> + *
> + * @data will be populated with the plain-text version of the policy
> + * on success.
> + *
> + * Return:
> + * >0 - Success, Length of buffer written
> + * <0 - Error
> + */
> +static ssize_t read_policy(struct file *f, char __user *data,
> +			   size_t len, loff_t *offset)
> +{
> +	int rc = 0;
> +	struct ipe_policy *p = NULL;
> +
> +	p = ipe_get_policy_rcu(*find_policy(f));
> +	if (!p)
> +		return -ENOENT;
> +
> +	rc = simple_read_from_buffer(data, len, offset, p->text, p->textlen);
> +
> +	ipe_put_policy(p);
> +	return rc;
> +}
> +
> +/**
> + * read_name: Read handler for the securityfs node,
> + *	      "ipe/policies/$name/name"
> + * @f: Supplies a file structure representing the securityfs node.
> + * @data: Suppleis a buffer passed to the write syscall
> + * @len: Supplies the length of @data
> + * @offset: unused.
> + *
> + * @data will be populated with the policy_name attribute on success
> + *
> + * Return:
> + * >0 - Success, Length of buffer written
> + * <0 - Error
> + */
> +static ssize_t read_name(struct file *f, char __user *data,
> +			 size_t len, loff_t *offset)
> +{
> +	int rc = 0;
> +	struct ipe_policy *p = NULL;
> +
> +	p = ipe_get_policy_rcu(*find_policy(f));
> +	if (!p)
> +		return -ENOENT;
> +
> +	rc = simple_read_from_buffer(data, len, offset, p->parsed->name,
> +				     strlen(p->parsed->name));
> +
> +	ipe_put_policy(p);
> +	return rc;
> +}
> +
> +/**
> + * read_version: Read handler for the securityfs node,
> + *		 "ipe/policies/$name/version"
> + * @f: Supplies a file structure representing the securityfs node.
> + * @data: Suppleis a buffer passed to the write syscall
> + * @len: Supplies the length of @data
> + * @offset: unused.
> + *
> + * @data will be populated with the version string on success.
> + *
> + * Return:
> + * >0 - Success, Length of buffer written
> + * <0 - Error
> + */
> +static ssize_t read_version(struct file *f, char __user *data,
> +			    size_t len, loff_t *offset)
> +{
> +	ssize_t rc = 0;
> +	size_t bufsize = 0;
> +	struct ipe_policy *p = NULL;
> +	char buffer[MAX_VERSION_SIZE] = { 0 };
> +
> +	p = ipe_get_policy_rcu(*find_policy(f));
> +	if (!p)
> +		return -ENOENT;
> +
> +	bufsize = scnprintf(buffer, ARRAY_SIZE(buffer), "%hu.%hu.%hu",
> +			    p->parsed->version.major, p->parsed->version.minor,
> +			    p->parsed->version.rev);
> +
> +	rc = simple_read_from_buffer(data, len, offset, buffer, bufsize);
> +
> +	ipe_put_policy(p);
> +	return rc;
> +}
> +
> +/**
> + * setactive: Write handler for the securityfs node,
> + *	      "ipe/policies/$name/active"
> + * @f: Supplies a file structure representing the securityfs node.
> + * @data: Supplies a buffer passed to the write syscall
> + * @len: Supplies the length of @data
> + * @offset: unused.
> + *
> + * Return:
> + * >0 - Success, Length of buffer written
> + * <0 - Error
> + */
> +static ssize_t setactive(struct file *f, const char __user *data,
> +			 size_t len, loff_t *offset)
> +{
> +	int rc = 0;
> +	bool value = false;
> +	struct ipe_policy *p = NULL;
> +	struct ipe_context *ctx = NULL;
> +
> +	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
> +		return -EPERM;
> +
> +	rc = kstrtobool_from_user(data, len, &value);
> +	if (rc)
> +		goto out;
> +
> +	if (!value) {
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +
> +	p = ipe_get_policy_rcu(*find_policy(f));
> +	if (!p) {
> +		rc = -ENOENT;
> +		goto out;
> +	}
> +
> +	ctx = ipe_get_ctx_rcu(p->ctx);
> +	if (!ctx) {
> +		rc = -ENOENT;
> +		goto out;
> +	}
> +
> +	rc = ipe_set_active_pol(p);
> +
> +out:
> +	ipe_put_ctx(ctx);
> +	ipe_put_policy(p);
> +	return (rc < 0) ? rc : len;
> +}
> +
> +/**
> + * getactive: Read handler for the securityfs node,
> + *	      "ipe/policies/$name/active"
> + * @f: Supplies a file structure representing the securityfs node.
> + * @data: Suppleis a buffer passed to the write syscall
> + * @len: Supplies the length of @data
> + * @offset: unused.
> + *
> + * @data will be populated with the 1 or 0 depending on if the
> + * corresponding policy is active.
> + *
> + * Return:
> + * >0 - Success, Length of buffer written
> + * <0 - Error
> + */
> +static ssize_t getactive(struct file *f, char __user *data,
> +			 size_t len, loff_t *offset)
> +{
> +	int rc = 0;
> +	const char *str;
> +	struct ipe_policy *p = NULL;
> +
> +	p = ipe_get_policy_rcu(*find_policy(f));
> +	if (!p) {
> +		rc = -ENOENT;
> +		goto out;
> +	}
> +
> +	str = ipe_is_policy_active(p) ? "1" : "0";
> +	rc = simple_read_from_buffer(data, len, offset, str, 2);
> +
> +out:
> +	ipe_put_policy(p);
> +	return rc;
> +}
> +
> +/**
> + * update_policy: Write handler for the securityfs node,
> + *		  "ipe/policies/$name/active"
> + * @f: Supplies a file structure representing the securityfs node.
> + * @data: Supplies a buffer passed to the write syscall
> + * @len: Supplies the length of @data
> + * @offset: unused.
> + *
> + * On success this updates the policy represented by $name,
> + * in-place.
> + *
> + * Return:
> + * >0 - Success, Length of buffer written
> + * <0 - Error
> + */
> +static ssize_t update_policy(struct file *f, const char __user *data,
> +			     size_t len, loff_t *offset)
> +{
> +	int rc = 0;
> +	char *copy = NULL;
> +	struct ipe_policy *new = NULL;
> +	struct ipe_policy *old = NULL;
> +	struct ipe_context *ctx = NULL;
> +	struct ipe_policy __rcu **addr = NULL;
> +
> +	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
> +		return -EPERM;
> +
> +	ctx = ipe_current_ctx();
> +	if (!ctx)
> +		return -ENOENT;
> +
> +	addr = find_policy(f);
> +	old = ipe_get_policy_rcu(*addr);
> +	if (!old) {
> +		rc = -ENOENT;
> +		goto err;
> +	}
> +
> +	copy = memdup_user(data, len);
> +	if (IS_ERR(copy)) {
> +		rc = PTR_ERR(copy);
> +		goto err;
> +	}
> +
> +	new = ipe_update_policy(old, NULL, 0, copy, len);
> +	if (IS_ERR(new)) {
> +		rc = PTR_ERR(new);
> +		goto err;
> +	}
> +
> +	spin_lock(&ctx->lock);
> +	rcu_assign_pointer(*addr, new);
> +	spin_unlock(&ctx->lock);
> +	synchronize_rcu();
> +
> +	swap(new->policyfs, old->policyfs);
> +
> +	kfree(copy);
> +	ipe_put_ctx(ctx);
> +	ipe_put_policy(old);
> +	return len;
> +err:
> +	kfree(copy);
> +	ipe_put_ctx(ctx);
> +	ipe_put_policy(new);
> +	ipe_put_policy(old);
> +	return rc;
> +}
> +
> +/**
> + * delete_policy: write handler for securityfs dir, "ipe/policies/$name/delete"
> + * @f: Supplies a file structure representing the securityfs node.
> + * @data: Supplies a buffer passed to the write syscall
> + * @len: Supplies the length of @data
> + * @offset: unused.
> + *
> + * On success this deletes the policy represented by $name.
> + *
> + * Return:
> + * >0 - Success, Length of buffer written
> + * <0 - Error
> + */
> +static ssize_t delete_policy(struct file *f, const char __user *data,
> +			     size_t len, loff_t *offset)
> +{
> +	int rc = 0;
> +	bool value = false;
> +	struct ipe_policy *p = NULL;
> +	struct ipe_context *ctx = NULL;
> +
> +	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
> +		return -EPERM;
> +
> +	rc = kstrtobool_from_user(data, len, &value);
> +	if (rc)
> +		goto out;
> +
> +	if (!value) {
> +		rc = -EINVAL;
> +		goto out;
> +	}
> +
> +	p = ipe_get_policy_rcu(*find_policy(f));
> +	if (!p) {
> +		rc = -ENOENT;
> +		goto out;
> +	}
> +
> +	if (ipe_is_policy_active(p)) {
> +		rc = -EPERM;
> +		goto out;
> +	}
> +
> +	ctx = ipe_get_ctx_rcu(p->ctx);
> +	if (!ctx) {
> +		rc = -ENOENT;
> +		goto out;
> +	}
> +
> +	ipe_remove_policy(p);
> +out:
> +	ipe_put_ctx(ctx);
> +	ipe_put_policy(p);
> +	return (rc < 0) ? rc : len;
> +}
> +
> +static const struct file_operations content_fops = {
> +	.read = read_policy,
> +};
> +
> +static const struct file_operations pkcs7_fops = {
> +	.read = read_pkcs7,
> +};
> +
> +static const struct file_operations name_fops = {
> +	.read = read_name,
> +};
> +
> +static const struct file_operations ver_fops = {
> +	.read = read_version,
> +};
> +
> +static const struct file_operations active_fops = {
> +	.write = setactive,
> +	.read = getactive,
> +};
> +
> +static const struct file_operations update_fops = {
> +	.write = update_policy,
> +};
> +
> +static const struct file_operations delete_fops = {
> +	.write = delete_policy,
> +};
> +
> +/**
> + * policy_subdir: files under a policy subdirectory
> + */
> +static const struct ipefs_file policy_subdir[] = {
> +	{ "pkcs7", 0444, &pkcs7_fops },
> +	{ "policy", 0444, &content_fops },
> +	{ "name", 0444, &name_fops },
> +	{ "version", 0444, &ver_fops },
> +	{ "active", 0600, &active_fops },
> +	{ "update", 0200, &update_fops },
> +	{ "delete", 0200, &delete_fops },
> +};
> +
> +/**
> + * soft_del_policyfs - soft delete the policyfs tree, preventing new
> + *		       accesses to the interfaces for this policy.
> + * @p - Policy to soft delete the tree for.
> + */
> +static void soft_del_policyfs(struct ipe_policy *p)
> +{
> +	struct inode *ino = NULL;
> +	struct ipe_policy __rcu **addr = NULL;
> +
> +	ino = d_inode(p->policyfs);
> +	addr = (struct ipe_policy __rcu **)&ino->i_private;
> +
> +	inode_lock(ino);
> +	rcu_assign_pointer(*addr, NULL);
> +	inode_unlock(ino);
> +	synchronize_rcu();
> +}
> +
> +/**
> + * ipe_del_policyfs_node: Delete a securityfs entry for @p
> + * @p: Supplies a pointer to the policy to delete a securityfs entry for.
> + */
> +void ipe_del_policyfs_node(struct ipe_policy *p)
> +{
> +	size_t i = 0;
> +	struct dentry *d = NULL;
> +	const struct ipefs_file *f = NULL;
> +
> +	if (IS_ERR_OR_NULL(p->policyfs))
> +		return;
> +
> +	soft_del_policyfs(p);
> +
> +	for (i = 0; i < ARRAY_SIZE(policy_subdir); ++i) {
> +		f = &policy_subdir[i];
> +
> +		d = lookup_positive_unlocked(f->name, p->policyfs,
> +					     strlen(f->name));
> +		if (IS_ERR(d))
> +			continue;
> +
> +		securityfs_remove(d);
> +		dput(d);
> +	}
> +
> +	securityfs_remove(p->policyfs);
> +}
> +
> +/**
> + * ipe_new_policyfs_node: Create a securityfs entry for @p
> + * @ctx: Supplies a pointer to a context structure that contains the root of
> + *	 the policy tree.
> + * @p: Supplies a pointer to the policy to create a securityfs entry for.
> + *
> + * Return:
> + * 0 - OK
> + * !0 - Error
> + */
> +int ipe_new_policyfs_node(struct ipe_context *ctx, struct ipe_policy *p)
> +{
> +	int rc = 0;
> +	size_t i = 0;
> +	struct dentry *d = NULL;
> +	struct ipe_policy **addr = NULL;
> +	const struct ipefs_file *f = NULL;
> +
> +	p->policyfs = securityfs_create_dir(p->parsed->name, ctx->policy_root);
> +	if (IS_ERR(p->policyfs)) {
> +		rc = PTR_ERR(p->policyfs);
> +		goto err;
> +	}
> +
> +	addr = (struct ipe_policy **)&(d_inode(p->policyfs)->i_private);
> +	*addr = p;
> +
> +	for (i = 0; i < ARRAY_SIZE(policy_subdir); ++i) {
> +		f = &policy_subdir[i];
> +
> +		d = securityfs_create_file(f->name, f->access, p->policyfs, p-
> >policyfs,
> +					   f->fops);
> +		if (IS_ERR(d)) {
> +			rc = PTR_ERR(d);
> +			goto err;
> +		}
> +	}
> +
> +	return 0;
> +err:
> +	ipe_del_policyfs_node(p);
> +	return rc;
> +}
> --
> 2.33.0

Re: [RFC PATCH v7 04/16] ipe: add userspace interface

[Deven Bowers]

On 11/3/2021 2:42 AM, Roberto Sassu wrote:
>>
>> +
>> +/**
>> + * ipe_init_securityfs: Initialize IPE's securityfs tree at fsinit
>> + *
>> + * Return:
>> + * !0 - Error
>> + * 0 - OK
>> + */
>> +static int __init ipe_init_securityfs(void)
>> +{
>> +	int rc = 0;
>> +	struct ipe_context *ctx = NULL;
>> +
>> +	ctx = ipe_current_ctx();
> Hi Deven
>
> the instruction above should be executed only if IPE LSM is
> enabled. Otherwise, the kernel panics due to the illegal access
> to the security blob of the task.

I see. I mistakenly assumed that failure in the LSM init would cause
a kernel panic (as the system is now booting without a potentially
required security component) as opposed to just disabling the LSM
and emitting a warning.

Easy fix for v8.

Thanks for pointing it out.

RE: [RFC PATCH v7 12/16] fsverity|security: add security hooks to fsverity digest and signature

[Roberto Sassu]

> From: Roberto Sassu [mailto:roberto.sassu@huawei.com]
> Sent: Wednesday, October 20, 2021 5:09 PM
> > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > Sent: Friday, October 15, 2021 10:11 PM
> > On Fri, Oct 15, 2021 at 12:25:53PM -0700, Deven Bowers wrote:
> > >
> > > On 10/13/2021 12:24 PM, Eric Biggers wrote:
> > > > On Wed, Oct 13, 2021 at 12:06:31PM -0700,
> > deven.desai@linux.microsoft.com wrote:
> > > > > From: Fan Wu <wufan@linux.microsoft.com>
> > > > >
> > > > > Add security_inode_setsecurity to fsverity signature verification.
> > > > > This can let LSMs save the signature data and digest hashes provided
> > > > > by fsverity.
> > > > Can you elaborate on why LSMs need this information?
> > >
> > > The proposed LSM (IPE) of this series will be the only one to need
> > > this information at the  moment. IPE’s goal is to have provide
> > > trust-based access control. Trust and Integrity are tied together,
> > > as you cannot prove trust without proving integrity.
> >
> > I think you mean authenticity, not integrity?
> >
> > Also how does this differ from IMA?  I know that IMA doesn't support fs-verity
> > file hashes, but that could be changed.  Why not extend IMA to cover your use
> > case(s)?
> >
> > > IPE needs the digest information to be able to compare a digest
> > > provided by the policy author, against the digest calculated by
> > > fsverity to make a decision on whether that specific file, represented
> > > by the digest is authorized for the actions specified in the policy.
> > >
> > > A more concrete example, if an IPE policy author writes:
> > >
> > >     op=EXECUTE fsverity_digest=<HexDigest > action=DENY
> > >
> > > IPE takes the digest provided by this security hook, stores it
> > > in IPE's security blob on the inode. If this file is later
> > > executed, IPE compares the digest stored in the LSM blob,
> > > provided by this hook, against <HexDigest> in the policy, if
> > > it matches, it denies the access, performing a revocation
> > > of that file.
> >
> > Do you have a better example?  This one is pretty useless since one can get
> > around it just by executing a file that doesn't have fs-verity enabled.
> 
> I was wondering if the following use case can be supported:
> allow the execution of files protected with fsverity if the root
> digest is found among reference values (instead of providing
> them one by one in the policy).
> 
> Something like:
> 
> op=EXECUTE fsverity_digest=diglim action=ALLOW

Looks like it works. I modified IPE to query the root digest
of an fsverity-protected file in DIGLIM.

# cat ipe-policy
policy_name="AllowFSVerityKmodules" policy_version=0.0.1
DEFAULT action=ALLOW
DEFAULT op=KMODULE action=DENY
op=KMODULE fsverity_digest=diglim action=ALLOW

IPE setup:
# cat ipe-policy.p7s > /sys/kernel/security/ipe/new_policy
# echo -n 1 >  /sys/kernel/security/ipe/policies/AllowFSVerityKmodules/active
# echo 1 > /sys/kernel/security/ipe/enforce

IPE denies loading of kernel modules not protected by fsverity:
# insmod  /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko
insmod: ERROR: could not insert module /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko: Permission denied

Protect fat.ko with fsverity:
# cp /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko /fsverity
# fsverity enable /fsverity/fat.ko
# fsverity measure /fsverity/fat.ko
sha256:079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 /fsverity/fat.ko

IPE still denies the loading of fat.ko (root digest not uploaded to the kernel):
# insmod /fsverity/fat.ko
insmod: ERROR: could not insert module /fsverity/fat.ko: Permission denied

Generate a digest list with the root digest above and upload it to the kernel:
# ./compact_gen -i 079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 -a sha256 -d test -s -t file -f
# echo $PWD/test/0-file_list-compact-079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 > /sys/kernel/security/integrity/diglim/digest_list_add

IPE allows the loading of fat.ko:
# insmod /fsverity/fat.ko
#

Regarding authenticity, not shown in this demo, IPE will also
ensure that the root digest is signed (diglim_digest_get_info()
reports this information).

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> DIGLIM is a component I'm working on that generically
> stores digests. The current use case is to store file digests
> from RPMTAG_FILEDIGESTS and use them with IMA, but
> the fsverity use case could be easily supported (if the root
> digest is stored in the RPM header).
> 
> DIGLIM also tells whether or not the signature of the source
> containing file digests (or fsverity digests) is valid (the signature
> of the RPM header is taken from RPMTAG_RSAHEADER).
> 
> The memory occupation is relatively small for executables
> and shared libraries. I published a demo for Fedora and
> openSUSE some time ago:
> 
> https://lore.kernel.org/linux-
> integrity/48cd737c504d45208377daa27d625531@huawei.com/
> 
> Thanks
> 
> Roberto
> 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
> 
> > > This brings me to your next comment:
> > >
> > > > The digest isn't meaningful without knowing the hash algorithm it uses.
> > > It's available here, but you aren't passing it to this function.
> > >
> > > The digest is meaningful without the algorithm in this case.
> >
> > No, it's not.
> >
> > Digests are meaningless without knowing what algorithm they were created
> > with.
> >
> > If your security policy is something like "Trust the file with digest $foo" and
> > multiple hash algorithms are possible, then the alorithm intended to be used
> > needs to be explicitly specified.  Otherwise any algorithm with the same length
> > digest will be accepted.  That's a fatal flaw if any of these algorithms is
> > cryptographically broken or was never intended to be a cryptographic
> algorithm
> > in the first place (e.g., a non-cryptographic checksum).
> >
> > Cryptosystems always need to specify the crypto algorithm(s) used; the
> > adversary
> > must not be allowed to choose the algorithms.
> >
> > I'm not sure how these patches can be taken seriously when they're getting
> this
> > sort of thing wrong.
> >
> > > > > +
> 	FS_VERITY_SIGNATURE_SEC_NAME,
> > > > > +					signature, sig_size, 0);
> > > > This is only for fs-verity built-in signatures which aren't the only way to do
> > > > signatures with fs-verity.  Are you sure this is what you're looking for?
> > >
> > > Could you elaborate on the other signature types that can be used
> > > with fs-verity? I’m 99% sure this is what I’m looking for as this
> > > is a signature validated in the kernel against the fs-verity keyring
> > > as part of the “fsverity enable” utility.
> > >
> > > It's important that the signature is validated in the kernel, as
> > > userspace is considered untrusted until the signature is validated
> > > for this case.
> > >
> > > > Can you elaborate on your use case for fs-verity built-in signatures,
> > > Sure, signatures, like digests, also provide a way to prove integrity,
> > > and the trust component comes from the validation against the keyring,
> > > as opposed to a fixed value in IPE’s policy. The use case for fs-verity
> > > built-in signatures is that we have a rw ext4 filesystem that has some
> > > executable files, and we want to have a execution policy (through IPE)
> > > that only _trusted_ executables can run. Perf is important here, hence
> > > fs-verity.
> >
> > Most users of fs-verity built-in signatures have actually been enforcing their
> > security policy in userspace, by checking whether specific files have the
> > fs-verity bit set or not.  Such users could just store and verify signatures in
> > userspace instead, without any kernel involvement.  So that's what I've been
> > recommending (with limited success, unfortunately).
> >
> > If you really do need in-kernel signature verification, then that may be a
> > legitimate use case for the fs-verity built-in signatures, although I do wonder
> > why you aren't using IMA and its signature mechanism instead.
> >
> > - Eric

Re: [RFC PATCH v7 12/16] fsverity|security: add security hooks to fsverity digest and signature

[Deven Bowers]

On 10/22/2021 9:31 AM, Roberto Sassu wrote:
>> From: Roberto Sassu [mailto:roberto.sassu@huawei.com]
>> Sent: Wednesday, October 20, 2021 5:09 PM
>>> From: Eric Biggers [mailto:ebiggers@kernel.org]
>>> Sent: Friday, October 15, 2021 10:11 PM
>>> On Fri, Oct 15, 2021 at 12:25:53PM -0700, Deven Bowers wrote:
>>>> On 10/13/2021 12:24 PM, Eric Biggers wrote:
>>>>> On Wed, Oct 13, 2021 at 12:06:31PM -0700,
>>> deven.desai@linux.microsoft.com  wrote:
>>>>>> From: Fan Wu<wufan@linux.microsoft.com>
>>>>>>
>>>>>> Add security_inode_setsecurity to fsverity signature verification.
>>>>>> This can let LSMs save the signature data and digest hashes provided
>>>>>> by fsverity.
>>>>> Can you elaborate on why LSMs need this information?
>>>> The proposed LSM (IPE) of this series will be the only one to need
>>>> this information at the  moment. IPE’s goal is to have provide
>>>> trust-based access control. Trust and Integrity are tied together,
>>>> as you cannot prove trust without proving integrity.
>>> I think you mean authenticity, not integrity?
>>>
>>> Also how does this differ from IMA?  I know that IMA doesn't support fs-verity
>>> file hashes, but that could be changed.  Why not extend IMA to cover your use
>>> case(s)?
>>>
>>>> IPE needs the digest information to be able to compare a digest
>>>> provided by the policy author, against the digest calculated by
>>>> fsverity to make a decision on whether that specific file, represented
>>>> by the digest is authorized for the actions specified in the policy.
>>>>
>>>> A more concrete example, if an IPE policy author writes:
>>>>
>>>>      op=EXECUTE fsverity_digest=<HexDigest > action=DENY
>>>>
>>>> IPE takes the digest provided by this security hook, stores it
>>>> in IPE's security blob on the inode. If this file is later
>>>> executed, IPE compares the digest stored in the LSM blob,
>>>> provided by this hook, against <HexDigest> in the policy, if
>>>> it matches, it denies the access, performing a revocation
>>>> of that file.
>>> Do you have a better example?  This one is pretty useless since one can get
>>> around it just by executing a file that doesn't have fs-verity enabled.
>> I was wondering if the following use case can be supported:
>> allow the execution of files protected with fsverity if the root
>> digest is found among reference values (instead of providing
>> them one by one in the policy).
>>
>> Something like:
>>
>> op=EXECUTE fsverity_digest=diglim action=ALLOW
> Looks like it works. I modified IPE to query the root digest
> of an fsverity-protected file in DIGLIM.
>
> # cat ipe-policy
> policy_name="AllowFSVerityKmodules" policy_version=0.0.1
> DEFAULT action=ALLOW
> DEFAULT op=KMODULE action=DENY
> op=KMODULE fsverity_digest=diglim action=ALLOW
>
> IPE setup:
> # cat ipe-policy.p7s > /sys/kernel/security/ipe/new_policy
> # echo -n 1 >  /sys/kernel/security/ipe/policies/AllowFSVerityKmodules/active
> # echo 1 > /sys/kernel/security/ipe/enforce
>
> IPE denies loading of kernel modules not protected by fsverity:
> # insmod  /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko
> insmod: ERROR: could not insert module /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko: Permission denied
>
> Protect fat.ko with fsverity:
> # cp /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko /fsverity
> # fsverity enable /fsverity/fat.ko
> # fsverity measure /fsverity/fat.ko
> sha256:079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 /fsverity/fat.ko
>
> IPE still denies the loading of fat.ko (root digest not uploaded to the kernel):
> # insmod /fsverity/fat.ko
> insmod: ERROR: could not insert module /fsverity/fat.ko: Permission denied
>
> Generate a digest list with the root digest above and upload it to the kernel:
> # ./compact_gen -i 079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 -a sha256 -d test -s -t file -f
> # echo $PWD/test/0-file_list-compact-079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 > /sys/kernel/security/integrity/diglim/digest_list_add
>
> IPE allows the loading of fat.ko:
> # insmod /fsverity/fat.ko
> #
>
> Regarding authenticity, not shown in this demo, IPE will also
> ensure that the root digest is signed (diglim_digest_get_info()
> reports this information).

I apologize for the delay in responses, but it looks like
you've figured it out.

This kind of thing is exactly what IPE's design is supposed to
solve, you have some other system which provides the
integrity mechanism and (optionally) determine if it is trusted or
not, and IPE can provide the policy aspect very easily to
make a set of system-wide requirements around your mechanism.

I'm very supportive of adding the functionality, but I wonder
if it makes more sense to have digilm's extension be a separate
key instead of tied to the fsverity_digest key - something like

    op=EXECUTE diglim_fsverity=TRUE action=DENY

that way the condition that enables this property can depend
on digilm in the build, and it separates the two systems'
integrations in a slightly more clean way.

As an aside, did you find it difficult to extend?

> Roberto
>
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
>
>> DIGLIM is a component I'm working on that generically
>> stores digests. The current use case is to store file digests
>> from RPMTAG_FILEDIGESTS and use them with IMA, but
>> the fsverity use case could be easily supported (if the root
>> digest is stored in the RPM header).
>>
>> DIGLIM also tells whether or not the signature of the source
>> containing file digests (or fsverity digests) is valid (the signature
>> of the RPM header is taken from RPMTAG_RSAHEADER).
>>
>> The memory occupation is relatively small for executables
>> and shared libraries. I published a demo for Fedora and
>> openSUSE some time ago:
>>
>> https://lore.kernel.org/linux-
>> integrity/48cd737c504d45208377daa27d625531@huawei.com/
>>
>> Thanks
>>
>> Roberto
>>
>> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
>> Managing Director: Li Peng, Zhong Ronghua

RE: [RFC PATCH v7 12/16] fsverity|security: add security hooks to fsverity digest and signature

[Roberto Sassu]

> From: Deven Bowers [mailto:deven.desai@linux.microsoft.com]
> Sent: Tuesday, October 26, 2021 9:04 PM
> On 10/22/2021 9:31 AM, Roberto Sassu wrote:
> >> From: Roberto Sassu [mailto:roberto.sassu@huawei.com]
> >> Sent: Wednesday, October 20, 2021 5:09 PM
> >>> From: Eric Biggers [mailto:ebiggers@kernel.org]
> >>> Sent: Friday, October 15, 2021 10:11 PM
> >>> On Fri, Oct 15, 2021 at 12:25:53PM -0700, Deven Bowers wrote:
> >>>> On 10/13/2021 12:24 PM, Eric Biggers wrote:
> >>>>> On Wed, Oct 13, 2021 at 12:06:31PM -0700,
> >>> deven.desai@linux.microsoft.com  wrote:
> >>>>>> From: Fan Wu<wufan@linux.microsoft.com>
> >>>>>>
> >>>>>> Add security_inode_setsecurity to fsverity signature verification.
> >>>>>> This can let LSMs save the signature data and digest hashes provided
> >>>>>> by fsverity.
> >>>>> Can you elaborate on why LSMs need this information?
> >>>> The proposed LSM (IPE) of this series will be the only one to need
> >>>> this information at the  moment. IPE’s goal is to have provide
> >>>> trust-based access control. Trust and Integrity are tied together,
> >>>> as you cannot prove trust without proving integrity.
> >>> I think you mean authenticity, not integrity?
> >>>
> >>> Also how does this differ from IMA?  I know that IMA doesn't support fs-
> verity
> >>> file hashes, but that could be changed.  Why not extend IMA to cover your
> use
> >>> case(s)?
> >>>
> >>>> IPE needs the digest information to be able to compare a digest
> >>>> provided by the policy author, against the digest calculated by
> >>>> fsverity to make a decision on whether that specific file, represented
> >>>> by the digest is authorized for the actions specified in the policy.
> >>>>
> >>>> A more concrete example, if an IPE policy author writes:
> >>>>
> >>>>      op=EXECUTE fsverity_digest=<HexDigest > action=DENY
> >>>>
> >>>> IPE takes the digest provided by this security hook, stores it
> >>>> in IPE's security blob on the inode. If this file is later
> >>>> executed, IPE compares the digest stored in the LSM blob,
> >>>> provided by this hook, against <HexDigest> in the policy, if
> >>>> it matches, it denies the access, performing a revocation
> >>>> of that file.
> >>> Do you have a better example?  This one is pretty useless since one can get
> >>> around it just by executing a file that doesn't have fs-verity enabled.
> >> I was wondering if the following use case can be supported:
> >> allow the execution of files protected with fsverity if the root
> >> digest is found among reference values (instead of providing
> >> them one by one in the policy).
> >>
> >> Something like:
> >>
> >> op=EXECUTE fsverity_digest=diglim action=ALLOW
> > Looks like it works. I modified IPE to query the root digest
> > of an fsverity-protected file in DIGLIM.
> >
> > # cat ipe-policy
> > policy_name="AllowFSVerityKmodules" policy_version=0.0.1
> > DEFAULT action=ALLOW
> > DEFAULT op=KMODULE action=DENY
> > op=KMODULE fsverity_digest=diglim action=ALLOW
> >
> > IPE setup:
> > # cat ipe-policy.p7s > /sys/kernel/security/ipe/new_policy
> > # echo -n 1 >  /sys/kernel/security/ipe/policies/AllowFSVerityKmodules/active
> > # echo 1 > /sys/kernel/security/ipe/enforce
> >
> > IPE denies loading of kernel modules not protected by fsverity:
> > # insmod  /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko
> > insmod: ERROR: could not insert module /lib/modules/5.15.0-
> rc1+/kernel/fs/fat/fat.ko: Permission denied
> >
> > Protect fat.ko with fsverity:
> > # cp /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko /fsverity
> > # fsverity enable /fsverity/fat.ko
> > # fsverity measure /fsverity/fat.ko
> >
> sha256:079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe154
> 7803 /fsverity/fat.ko
> >
> > IPE still denies the loading of fat.ko (root digest not uploaded to the kernel):
> > # insmod /fsverity/fat.ko
> > insmod: ERROR: could not insert module /fsverity/fat.ko: Permission denied
> >
> > Generate a digest list with the root digest above and upload it to the kernel:
> > # ./compact_gen -i
> 079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 -a
> sha256 -d test -s -t file -f
> > # echo $PWD/test/0-file_list-compact-
> 079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 >
> /sys/kernel/security/integrity/diglim/digest_list_add
> >
> > IPE allows the loading of fat.ko:
> > # insmod /fsverity/fat.ko
> > #
> >
> > Regarding authenticity, not shown in this demo, IPE will also
> > ensure that the root digest is signed (diglim_digest_get_info()
> > reports this information).
> 
> I apologize for the delay in responses, but it looks like
> you've figured it out.

No problem.

> This kind of thing is exactly what IPE's design is supposed to
> solve, you have some other system which provides the
> integrity mechanism and (optionally) determine if it is trusted or
> not, and IPE can provide the policy aspect very easily to
> make a set of system-wide requirements around your mechanism.
> 
> I'm very supportive of adding the functionality, but I wonder
> if it makes more sense to have digilm's extension be a separate
> key instead of tied to the fsverity_digest key - something like
> 
>     op=EXECUTE diglim_fsverity=TRUE action=DENY
> 
> that way the condition that enables this property can depend
> on digilm in the build, and it separates the two systems'
> integrations in a slightly more clean way.

Yes, I agree. It is much more clean.

> As an aside, did you find it difficult to extend?

No, it was very straightforward.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> > Roberto
> >
> > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> > Managing Director: Li Peng, Zhong Ronghua
> >
> >> DIGLIM is a component I'm working on that generically
> >> stores digests. The current use case is to store file digests
> >> from RPMTAG_FILEDIGESTS and use them with IMA, but
> >> the fsverity use case could be easily supported (if the root
> >> digest is stored in the RPM header).
> >>
> >> DIGLIM also tells whether or not the signature of the source
> >> containing file digests (or fsverity digests) is valid (the signature
> >> of the RPM header is taken from RPMTAG_RSAHEADER).
> >>
> >> The memory occupation is relatively small for executables
> >> and shared libraries. I published a demo for Fedora and
> >> openSUSE some time ago:
> >>
> >> https://lore.kernel.org/linux-
> >> integrity/48cd737c504d45208377daa27d625531@huawei.com/
> >>
> >> Thanks
> >>
> >> Roberto
> >>
> >> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> >> Managing Director: Li Peng, Zhong Ronghua

RE: [RFC PATCH v7 12/16] fsverity|security: add security hooks to fsverity digest and signature

[Roberto Sassu]

> From: Deven Bowers [mailto:deven.desai@linux.microsoft.com]
> Sent: Friday, October 15, 2021 9:26 PM
> On 10/13/2021 12:24 PM, Eric Biggers wrote:
> > On Wed, Oct 13, 2021 at 12:06:31PM -0700,
> deven.desai@linux.microsoft.com wrote:
> >> From: Fan Wu <wufan@linux.microsoft.com>
> >>
> >> Add security_inode_setsecurity to fsverity signature verification.
> >> This can let LSMs save the signature data and digest hashes provided
> >> by fsverity.
> > Can you elaborate on why LSMs need this information?
> 
> The proposed LSM (IPE) of this series will be the only one to need
> this information at the  moment. IPE’s goal is to have provide
> trust-based access control. Trust and Integrity are tied together,
> as you cannot prove trust without proving integrity.

I wanted to go back on this question.

It seems, at least for fsverity, that you could obtain the
root digest at run-time, without storing it in a security blob.

I thought I should use fsverity_get_info() but the fsverity_info
structure is not exported (it is defined in fs/verity/fsverity_private.h).

Then, I defined a new function, fsverity_get_file_digest() to copy
the file_digest member of fsverity_info to a buffer and to pass
the associated hash algorithm.

With that, the code of evaluate() for DIGLIM becomes:

        info = fsverity_get_info(file_inode(ctx->file));
        if (info)
                ret = fsverity_get_file_digest(info, buffer, sizeof(buffer), &algo);

        if (!strcmp(expect->data, "diglim") && ret > 0) {
                ret = diglim_digest_get_info(buffer, algo, COMPACT_FILE, &modifiers, &actions);
                if (!ret)
                        return true;
        }

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> IPE needs the digest information to be able to compare a digest
> provided by the policy author, against the digest calculated by
> fsverity to make a decision on whether that specific file, represented
> by the digest is authorized for the actions specified in the policy.
> 
> A more concrete example, if an IPE policy author writes:
> 
>      op=EXECUTE fsverity_digest=<HexDigest > action=DENY
> 
> IPE takes the digest provided by this security hook, stores it
> in IPE's security blob on the inode. If this file is later
> executed, IPE compares the digest stored in the LSM blob,
> provided by this hook, against <HexDigest> in the policy, if
> it matches, it denies the access, performing a revocation
> of that file.
> 
> This brings me to your next comment:
> 
>  > The digest isn't meaningful without knowing the hash algorithm it uses.
> It's available here, but you aren't passing it to this function.
> 
> The digest is meaningful without the algorithm in this case.
> IPE does not want to recalculate a digest, that’s expensive and
> doesn’t provide any value. IPE, in this case, treats this as a
> buffer to compare the policy-provided one above to make a
> policy decision about access to the resource.
> 
> >> Also changes the implementaion inside the hook function to let
> >> multiple LSMs can add hooks.
> > Please split fs/verity/ changes and security/ changes into separate patches, if
> > possible.
> 
> Sorry, will do, not a problem.
> 
> >> @@ -177,6 +178,17 @@ struct fsverity_info *fsverity_create_info(const
> struct inode *inode,
> >>   		fsverity_err(inode, "Error %d computing file digest", err);
> >>   		goto out;
> >>   	}
> >> +
> >> +	err = security_inode_setsecurity((struct inode *)inode,
> > If a non-const inode is needed, please propagate that into the callers rather
> > than randomly casting away the const.
> >
> >> +					 FS_VERITY_DIGEST_SEC_NAME,
> >> +					 vi->file_digest,
> >> +					 vi->tree_params.hash_alg-
> >digest_size,
> >> +					 0);
> >> @@ -84,7 +85,9 @@ int fsverity_verify_signature(const struct fsverity_info
> *vi,
> >>
> >>   	pr_debug("Valid signature for file digest %s:%*phN\n",
> >>   		 hash_alg->name, hash_alg->digest_size, vi->file_digest);
> >> -	return 0;
> >> +	return security_inode_setsecurity((struct inode *)inode,
> >>
> > Likewise, please don't cast away const.
> 
> Sorry, I should've caught these myself. I'll change
> fsverity_create_info to accept the non-const inode, and
> change fsverity_verify_signature to accept an additional inode
> struct as the first arg instead of changing the fsverity_info
> structure to have a non-const inode field.
> 
> >> +					FS_VERITY_SIGNATURE_SEC_NAME,
> >> +					signature, sig_size, 0);
> > This is only for fs-verity built-in signatures which aren't the only way to do
> > signatures with fs-verity.  Are you sure this is what you're looking for?
> 
> Could you elaborate on the other signature types that can be used
> with fs-verity? I’m 99% sure this is what I’m looking for as this
> is a signature validated in the kernel against the fs-verity keyring
> as part of the “fsverity enable” utility.
> 
> It's important that the signature is validated in the kernel, as
> userspace is considered untrusted until the signature is validated
> for this case.
> 
> > Can you elaborate on your use case for fs-verity built-in signatures,
> Sure, signatures, like digests, also provide a way to prove integrity,
> and the trust component comes from the validation against the keyring,
> as opposed to a fixed value in IPE’s policy. The use case for fs-verity
> built-in signatures is that we have a rw ext4 filesystem that has some
> executable files, and we want to have a execution policy (through IPE)
> that only _trusted_ executables can run. Perf is important here, hence
> fs-verity.
> 
> > and what the LSM hook will do with them?
> 
> At the moment, this will just signal to IPE that these fs-verity files were
> enabled with a built-in signature as opposed to enabled without a signature.
> In v7, it copies the signature data into IPE's LSM blob attached to the
> inode.
> In v8+, I'm changing this to store “true” in IPE's LSM blob instead, as
> copying
> the signature data is an unnecessary waste of space and point of
> failure. This
> has a _slightly_ different functionality then fs.verity.require_signatures,
> because even if someone were to disable the require signatures option, IPE
> would still know if these files were signed or not and be able to make the
> access control decision based IPE's policy.
> 
> Very concretely, this powers this kind of rule in IPE:
> 
>    op=EXECUTE fsverity_signature=TRUE action=ALLOW
> 
> if that fsverity_signature value in IPE’s LSM blob attached to the inode is
> true, then fsverity_signature in IPE’s policy will evaluate to true and
> match
> this rule. The inverse is also applicable.

Re: [RFC PATCH v7 12/16] fsverity|security: add security hooks to fsverity digest and signature

[Deven Bowers]

On 11/3/2021 5:28 AM, Roberto Sassu wrote:
>> From: Deven Bowers [mailto:deven.desai@linux.microsoft.com]
>> Sent: Friday, October 15, 2021 9:26 PM
>> On 10/13/2021 12:24 PM, Eric Biggers wrote:
>>> On Wed, Oct 13, 2021 at 12:06:31PM -0700,
>> deven.desai@linux.microsoft.com wrote:
>>>> From: Fan Wu <wufan@linux.microsoft.com>
>>>>
>>>> Add security_inode_setsecurity to fsverity signature verification.
>>>> This can let LSMs save the signature data and digest hashes provided
>>>> by fsverity.
>>> Can you elaborate on why LSMs need this information?
>> The proposed LSM (IPE) of this series will be the only one to need
>> this information at the  moment. IPE’s goal is to have provide
>> trust-based access control. Trust and Integrity are tied together,
>> as you cannot prove trust without proving integrity.
> I wanted to go back on this question.
>
> It seems, at least for fsverity, that you could obtain the
> root digest at run-time, without storing it in a security blob.
>
> I thought I should use fsverity_get_info() but the fsverity_info
> structure is not exported (it is defined in fs/verity/fsverity_private.h).
>
> Then, I defined a new function, fsverity_get_file_digest() to copy
> the file_digest member of fsverity_info to a buffer and to pass
> the associated hash algorithm.
>
> With that, the code of evaluate() for DIGLIM becomes:
>
>          info = fsverity_get_info(file_inode(ctx->file));
>          if (info)
>                  ret = fsverity_get_file_digest(info, buffer, sizeof(buffer), &algo);
>
>          if (!strcmp(expect->data, "diglim") && ret > 0) {
>                  ret = diglim_digest_get_info(buffer, algo, COMPACT_FILE, &modifiers, &actions);
>                  if (!ret)
>                          return true;
>          }
This would work with the digest with a bit more code in fs-verity. It
also has benefits if there are other callers who want this information.

I was planning on grouping the digest and signature into 
apublic_key_signature
structure in v8 to pass the digest, digest algorithm,digest size, signature
and signature size (as opposed to defining a new structfor this purpose),
reducing the number of LSM hook calls down to one functionin fsverity.

I think both approaches have merit. fsverity_get_file_digest is more useful
if there are callers outside of LSMs that want this information. The LSM 
hook
is cleaner if only LSMs want this information.

At least, at the moment, it seems like it's only IPE who wants this 
information,
and it's not like it won't be able to change later if the need arises, 
as this
is all implementation details that wouldn't effect the end-user.

I'll defer to Eric - his opinion holds the most weight, as fsverity would be
the main code affected in either case.

RE: [RFC PATCH v7 14/16] scripts: add boot policy generation program

[Roberto Sassu]

> From: deven.desai@linux.microsoft.com
> [mailto:deven.desai@linux.microsoft.com]
> From: Deven Bowers <deven.desai@linux.microsoft.com>
> 
> Enables an IPE policy to be enforced from kernel start, enabling access
> control based on trust from kernel startup. This is accomplished by
> transforming an IPE policy indicated by CONFIG_IPE_BOOT_POLICY into a
> c-string literal that is parsed at kernel startup as an unsigned policy.
> 
> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> ---
> 
> Relevant changes since v6:
>   * Move patch 01/12 to [14/16] of the series
> 
> ---
>  MAINTAINERS                   |   1 +
>  scripts/Makefile              |   1 +
>  scripts/ipe/Makefile          |   2 +
>  scripts/ipe/polgen/.gitignore |   1 +
>  scripts/ipe/polgen/Makefile   |   6 ++
>  scripts/ipe/polgen/polgen.c   | 145 ++++++++++++++++++++++++++++++++++
>  security/ipe/.gitignore       |   1 +
>  security/ipe/Kconfig          |  10 +++
>  security/ipe/Makefile         |  13 +++
>  security/ipe/ctx.c            |  18 +++++
>  10 files changed, 198 insertions(+)
>  create mode 100644 scripts/ipe/Makefile
>  create mode 100644 scripts/ipe/polgen/.gitignore
>  create mode 100644 scripts/ipe/polgen/Makefile
>  create mode 100644 scripts/ipe/polgen/polgen.c
>  create mode 100644 security/ipe/.gitignore
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f1e76f791d47..a84ca781199b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9283,6 +9283,7 @@ INTEGRITY POLICY ENFORCEMENT (IPE)
>  M:	Deven Bowers <deven.desai@linux.microsoft.com>
>  M:	Fan Wu <wufan@linux.microsoft.com>
>  S:	Supported
> +F:	scripts/ipe/
>  F:	security/ipe/
> 
>  INTEL 810/815 FRAMEBUFFER DRIVER
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 9adb6d247818..a31da6d57a36 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -41,6 +41,7 @@ targets += module.lds
>  subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
>  subdir-$(CONFIG_MODVERSIONS) += genksyms
>  subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> +subdir-$(CONFIG_SECURITY_IPE) += ipe
> 
>  # Let clean descend into subdirs
>  subdir-	+= basic dtc gdb kconfig mod
> diff --git a/scripts/ipe/Makefile b/scripts/ipe/Makefile
> new file mode 100644
> index 000000000000..e87553fbb8d6
> --- /dev/null
> +++ b/scripts/ipe/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +subdir-y := polgen
> diff --git a/scripts/ipe/polgen/.gitignore b/scripts/ipe/polgen/.gitignore
> new file mode 100644
> index 000000000000..80f32f25d200
> --- /dev/null
> +++ b/scripts/ipe/polgen/.gitignore
> @@ -0,0 +1 @@
> +polgen
> diff --git a/scripts/ipe/polgen/Makefile b/scripts/ipe/polgen/Makefile
> new file mode 100644
> index 000000000000..066060c22b4a
> --- /dev/null
> +++ b/scripts/ipe/polgen/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +hostprogs-always-y	:= polgen
> +HOST_EXTRACFLAGS += \
> +	-I$(srctree)/include \
> +	-I$(srctree)/include/uapi \
> +
> diff --git a/scripts/ipe/polgen/polgen.c b/scripts/ipe/polgen/polgen.c
> new file mode 100644
> index 000000000000..73cf13e743f7
> --- /dev/null
> +++ b/scripts/ipe/polgen/polgen.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) Microsoft Corporation. All rights reserved.
> + */
> +
> +#include <stdlib.h>
> +#include <stddef.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <errno.h>
> +
> +static void usage(const char *const name)
> +{
> +	printf("Usage: %s OutputFile (PolicyFile)\n", name);
> +	exit(EINVAL);
> +}
> +
> +static int policy_to_buffer(const char *pathname, char **buffer, size_t *size)
> +{
> +	int rc = 0;
> +	FILE *fd;
> +	char *lbuf;
> +	size_t fsize;
> +	size_t read;
> +
> +	fd = fopen(pathname, "r");
> +	if (!fd) {
> +		rc = errno;
> +		goto out;
> +	}
> +
> +	fseek(fd, 0, SEEK_END);
> +	fsize = ftell(fd);
> +	rewind(fd);
> +
> +	lbuf = malloc(fsize);
> +	if (!lbuf) {
> +		rc = ENOMEM;
> +		goto out_close;
> +	}
> +
> +	read = fread((void *)lbuf, sizeof(*lbuf), fsize, fd);
> +	if (read != fsize) {
> +		rc = -1;
> +		goto out_free;
> +	}
> +
> +	*buffer = lbuf;
> +	*size = fsize;
> +	fclose(fd);
> +
> +	return rc;
> +
> +out_free:
> +	free(lbuf);
> +out_close:
> +	fclose(fd);
> +out:
> +	return rc;
> +}
> +
> +static int write_boot_policy(const char *pathname, const char *buf, size_t size)
> +{
> +	int rc = 0;
> +	FILE *fd;
> +	size_t i;
> +
> +	fd = fopen(pathname, "w");
> +	if (!fd) {
> +		rc = errno;
> +		goto err;
> +	}
> +
> +	fprintf(fd, "/* This file is automatically generated.");
> +	fprintf(fd, " Do not edit. */\n");
> +	fprintf(fd, "#include <stddef.h>\n");
> +	fprintf(fd, "\nextern const char *const ipe_boot_policy;\n\n");
> +	fprintf(fd, "const char *const ipe_boot_policy =\n");
> +
> +	if (!buf || size == 0) {
> +		fprintf(fd, "\tNULL;\n");
> +		fclose(fd);
> +		return 0;
> +	}
> +
> +	fprintf(fd, "\t\"");
> +
> +	for (i = 0; i < size; ++i) {
> +		switch (buf[i]) {
> +		case '"':
> +			fprintf(fd, "\\\"");
> +			break;
> +		case '\'':
> +			fprintf(fd, "'");
> +			break;
> +		case '\n':
> +			fprintf(fd, "\\n\"\n\t\"");
> +			break;
> +		case '\\':
> +			fprintf(fd, "\\\\");
> +			break;
> +		case '\t':
> +			fprintf(fd, "\\t");
> +			break;
> +		case '\?':
> +			fprintf(fd, "\\?");
> +			break;
> +		default:
> +			fprintf(fd, "%c", buf[i]);
> +		}
> +	}
> +	fprintf(fd, "\";\n");
> +	fclose(fd);
> +
> +	return 0;
> +
> +err:
> +	if (fd)
> +		fclose(fd);
> +	return rc;
> +}
> +
> +int main(int argc, const char *const argv[])
> +{
> +	int rc = 0;
> +	size_t len = 0;
> +	char *policy = NULL;
> +
> +	if (argc < 2)
> +		usage(argv[0]);
> +
> +	if (argc > 2) {
> +		rc = policy_to_buffer(argv[2], &policy, &len);
> +		if (rc != 0)
> +			goto cleanup;
> +	}
> +
> +	rc = write_boot_policy(argv[1], policy, len);
> +cleanup:
> +	if (policy)
> +		free(policy);
> +	if (rc != 0)
> +		perror("An error occurred during policy conversion: ");
> +	return rc;
> +}
> diff --git a/security/ipe/.gitignore b/security/ipe/.gitignore
> new file mode 100644
> index 000000000000..eca22ad5ed22
> --- /dev/null
> +++ b/security/ipe/.gitignore
> @@ -0,0 +1 @@
> +boot-policy.c
> \ No newline at end of file
> diff --git a/security/ipe/Kconfig b/security/ipe/Kconfig
> index fcf82a8152ec..39df680b67a2 100644
> --- a/security/ipe/Kconfig
> +++ b/security/ipe/Kconfig
> @@ -20,6 +20,16 @@ menuconfig SECURITY_IPE
> 
>  if SECURITY_IPE
> 
> +config IPE_BOOT_POLICY
> +	string "Integrity policy to apply on system startup"
> +	help
> +	  This option specifies a filepath to a IPE policy that is compiled
> +	  into the kernel. This policy will be enforced until a policy update
> +	  is deployed via the $securityfs/ipe/policies/$policy_name/active
> +	  interface.
> +
> +	  If unsure, leave blank.
> +
>  choice
>  	prompt "Hash algorithm used in auditing policies"
>  	default IPE_AUDIT_HASH_SHA1
> diff --git a/security/ipe/Makefile b/security/ipe/Makefile
> index 1e7b2d7fcd9e..89fec670f954 100644
> --- a/security/ipe/Makefile
> +++ b/security/ipe/Makefile
> @@ -7,7 +7,18 @@
> 
>  ccflags-y := -I$(srctree)/security/ipe/modules
> 
> +quiet_cmd_polgen = IPE_POL $(2)
> +      cmd_polgen = scripts/ipe/polgen/polgen security/ipe/boot-policy.c $(2)
> +
> +$(eval $(call config_filename,IPE_BOOT_POLICY))
> +
> +targets += boot-policy.c
> +
> +$(obj)/boot-policy.c: scripts/ipe/polgen/polgen
> $(IPE_BOOT_POLICY_FILENAME) FORCE
> +	$(call if_changed,polgen,$(IPE_BOOT_POLICY_FILENAME))
> +
>  obj-$(CONFIG_SECURITY_IPE) += \
> +	boot-policy.o \
>  	ctx.o \
>  	eval.o \
>  	fs.o \
> @@ -21,3 +32,5 @@ obj-$(CONFIG_SECURITY_IPE) += \
>  	policyfs.o \
> 
>  obj-$(CONFIG_AUDIT) += audit.o
> +
> +clean-files := boot-policy.c \
> diff --git a/security/ipe/ctx.c b/security/ipe/ctx.c
> index fc9b8e467bc9..879acf4ceac5 100644
> --- a/security/ipe/ctx.c
> +++ b/security/ipe/ctx.c
> @@ -15,6 +15,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/moduleparam.h>
> 
> +extern const char *const ipe_boot_policy;
>  static bool success_audit;
>  static bool enforce = true;
> 
> @@ -329,6 +330,7 @@ void ipe_put_ctx(struct ipe_context *ctx)
>  int __init ipe_init_ctx(void)
>  {
>  	int rc = 0;
> +	struct ipe_policy *p = NULL;
>  	struct ipe_context *lns = NULL;
> 
>  	lns = create_ctx();
> @@ -342,10 +344,26 @@ int __init ipe_init_ctx(void)
>  	WRITE_ONCE(lns->enforce, enforce);
>  	spin_unlock(&lns->lock);
> 
> +	if (ipe_boot_policy) {
> +		p = ipe_new_policy(ipe_boot_policy, strlen(ipe_boot_policy),
> +				   NULL, 0);
> +		if (IS_ERR(p)) {
> +			rc = PTR_ERR(lns);

This should be:

	rc = PTR_ERR(p);

> +			goto err;
> +		}
> +
> +		ipe_add_policy(lns, p);
> +		rc = ipe_set_active_pol(p);
> +		if (!rc)

Here you need to set a non-zero value, so that ipe_init()
does not enable the LSM.

I would set to 1 a new global variable, like ipe_lsm_enabled,
in ipe_init() just before security_add_hooks().

Then, I would add a check of this variable in ipe_init_securityfs()
to avoid the kernel panic.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> +			goto err;
> +	}
> +
>  	rcu_assign_pointer(*ipe_tsk_ctx(current), lns);
> +	ipe_put_policy(p);
> 
>  	return 0;
>  err:
> +	ipe_put_policy(p);
>  	ipe_put_ctx(lns);
>  	return rc;
>  }
> --
> 2.33.0

RE: [RFC PATCH v7 14/16] scripts: add boot policy generation program

[Roberto Sassu]

> From: Roberto Sassu [mailto:roberto.sassu@huawei.com]
> Sent: Wednesday, November 3, 2021 5:43 PM
> > From: deven.desai@linux.microsoft.com
> > [mailto:deven.desai@linux.microsoft.com]
> > From: Deven Bowers <deven.desai@linux.microsoft.com>
> >
> > Enables an IPE policy to be enforced from kernel start, enabling access
> > control based on trust from kernel startup. This is accomplished by
> > transforming an IPE policy indicated by CONFIG_IPE_BOOT_POLICY into a
> > c-string literal that is parsed at kernel startup as an unsigned policy.
> >
> > Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> > ---
> >
> > Relevant changes since v6:
> >   * Move patch 01/12 to [14/16] of the series
> >
> > ---
> >  MAINTAINERS                   |   1 +
> >  scripts/Makefile              |   1 +
> >  scripts/ipe/Makefile          |   2 +
> >  scripts/ipe/polgen/.gitignore |   1 +
> >  scripts/ipe/polgen/Makefile   |   6 ++
> >  scripts/ipe/polgen/polgen.c   | 145 ++++++++++++++++++++++++++++++++++
> >  security/ipe/.gitignore       |   1 +
> >  security/ipe/Kconfig          |  10 +++
> >  security/ipe/Makefile         |  13 +++
> >  security/ipe/ctx.c            |  18 +++++
> >  10 files changed, 198 insertions(+)
> >  create mode 100644 scripts/ipe/Makefile
> >  create mode 100644 scripts/ipe/polgen/.gitignore
> >  create mode 100644 scripts/ipe/polgen/Makefile
> >  create mode 100644 scripts/ipe/polgen/polgen.c
> >  create mode 100644 security/ipe/.gitignore
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f1e76f791d47..a84ca781199b 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9283,6 +9283,7 @@ INTEGRITY POLICY ENFORCEMENT (IPE)
> >  M:	Deven Bowers <deven.desai@linux.microsoft.com>
> >  M:	Fan Wu <wufan@linux.microsoft.com>
> >  S:	Supported
> > +F:	scripts/ipe/
> >  F:	security/ipe/
> >
> >  INTEL 810/815 FRAMEBUFFER DRIVER
> > diff --git a/scripts/Makefile b/scripts/Makefile
> > index 9adb6d247818..a31da6d57a36 100644
> > --- a/scripts/Makefile
> > +++ b/scripts/Makefile
> > @@ -41,6 +41,7 @@ targets += module.lds
> >  subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
> >  subdir-$(CONFIG_MODVERSIONS) += genksyms
> >  subdir-$(CONFIG_SECURITY_SELINUX) += selinux
> > +subdir-$(CONFIG_SECURITY_IPE) += ipe
> >
> >  # Let clean descend into subdirs
> >  subdir-	+= basic dtc gdb kconfig mod
> > diff --git a/scripts/ipe/Makefile b/scripts/ipe/Makefile
> > new file mode 100644
> > index 000000000000..e87553fbb8d6
> > --- /dev/null
> > +++ b/scripts/ipe/Makefile
> > @@ -0,0 +1,2 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +subdir-y := polgen
> > diff --git a/scripts/ipe/polgen/.gitignore b/scripts/ipe/polgen/.gitignore
> > new file mode 100644
> > index 000000000000..80f32f25d200
> > --- /dev/null
> > +++ b/scripts/ipe/polgen/.gitignore
> > @@ -0,0 +1 @@
> > +polgen
> > diff --git a/scripts/ipe/polgen/Makefile b/scripts/ipe/polgen/Makefile
> > new file mode 100644
> > index 000000000000..066060c22b4a
> > --- /dev/null
> > +++ b/scripts/ipe/polgen/Makefile
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +hostprogs-always-y	:= polgen
> > +HOST_EXTRACFLAGS += \
> > +	-I$(srctree)/include \
> > +	-I$(srctree)/include/uapi \
> > +
> > diff --git a/scripts/ipe/polgen/polgen.c b/scripts/ipe/polgen/polgen.c
> > new file mode 100644
> > index 000000000000..73cf13e743f7
> > --- /dev/null
> > +++ b/scripts/ipe/polgen/polgen.c
> > @@ -0,0 +1,145 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) Microsoft Corporation. All rights reserved.
> > + */
> > +
> > +#include <stdlib.h>
> > +#include <stddef.h>
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +#include <errno.h>
> > +
> > +static void usage(const char *const name)
> > +{
> > +	printf("Usage: %s OutputFile (PolicyFile)\n", name);
> > +	exit(EINVAL);
> > +}
> > +
> > +static int policy_to_buffer(const char *pathname, char **buffer, size_t *size)
> > +{
> > +	int rc = 0;
> > +	FILE *fd;
> > +	char *lbuf;
> > +	size_t fsize;
> > +	size_t read;
> > +
> > +	fd = fopen(pathname, "r");
> > +	if (!fd) {
> > +		rc = errno;
> > +		goto out;
> > +	}
> > +
> > +	fseek(fd, 0, SEEK_END);
> > +	fsize = ftell(fd);
> > +	rewind(fd);
> > +
> > +	lbuf = malloc(fsize);
> > +	if (!lbuf) {
> > +		rc = ENOMEM;
> > +		goto out_close;
> > +	}
> > +
> > +	read = fread((void *)lbuf, sizeof(*lbuf), fsize, fd);
> > +	if (read != fsize) {
> > +		rc = -1;
> > +		goto out_free;
> > +	}
> > +
> > +	*buffer = lbuf;
> > +	*size = fsize;
> > +	fclose(fd);
> > +
> > +	return rc;
> > +
> > +out_free:
> > +	free(lbuf);
> > +out_close:
> > +	fclose(fd);
> > +out:
> > +	return rc;
> > +}
> > +
> > +static int write_boot_policy(const char *pathname, const char *buf, size_t
> size)
> > +{
> > +	int rc = 0;
> > +	FILE *fd;
> > +	size_t i;
> > +
> > +	fd = fopen(pathname, "w");
> > +	if (!fd) {
> > +		rc = errno;
> > +		goto err;
> > +	}
> > +
> > +	fprintf(fd, "/* This file is automatically generated.");
> > +	fprintf(fd, " Do not edit. */\n");
> > +	fprintf(fd, "#include <stddef.h>\n");
> > +	fprintf(fd, "\nextern const char *const ipe_boot_policy;\n\n");
> > +	fprintf(fd, "const char *const ipe_boot_policy =\n");
> > +
> > +	if (!buf || size == 0) {
> > +		fprintf(fd, "\tNULL;\n");
> > +		fclose(fd);
> > +		return 0;
> > +	}
> > +
> > +	fprintf(fd, "\t\"");
> > +
> > +	for (i = 0; i < size; ++i) {
> > +		switch (buf[i]) {
> > +		case '"':
> > +			fprintf(fd, "\\\"");
> > +			break;
> > +		case '\'':
> > +			fprintf(fd, "'");
> > +			break;
> > +		case '\n':
> > +			fprintf(fd, "\\n\"\n\t\"");
> > +			break;
> > +		case '\\':
> > +			fprintf(fd, "\\\\");
> > +			break;
> > +		case '\t':
> > +			fprintf(fd, "\\t");
> > +			break;
> > +		case '\?':
> > +			fprintf(fd, "\\?");
> > +			break;
> > +		default:
> > +			fprintf(fd, "%c", buf[i]);
> > +		}
> > +	}
> > +	fprintf(fd, "\";\n");
> > +	fclose(fd);
> > +
> > +	return 0;
> > +
> > +err:
> > +	if (fd)
> > +		fclose(fd);
> > +	return rc;
> > +}
> > +
> > +int main(int argc, const char *const argv[])
> > +{
> > +	int rc = 0;
> > +	size_t len = 0;
> > +	char *policy = NULL;
> > +
> > +	if (argc < 2)
> > +		usage(argv[0]);
> > +
> > +	if (argc > 2) {
> > +		rc = policy_to_buffer(argv[2], &policy, &len);
> > +		if (rc != 0)
> > +			goto cleanup;
> > +	}
> > +
> > +	rc = write_boot_policy(argv[1], policy, len);
> > +cleanup:
> > +	if (policy)
> > +		free(policy);
> > +	if (rc != 0)
> > +		perror("An error occurred during policy conversion: ");
> > +	return rc;
> > +}
> > diff --git a/security/ipe/.gitignore b/security/ipe/.gitignore
> > new file mode 100644
> > index 000000000000..eca22ad5ed22
> > --- /dev/null
> > +++ b/security/ipe/.gitignore
> > @@ -0,0 +1 @@
> > +boot-policy.c
> > \ No newline at end of file
> > diff --git a/security/ipe/Kconfig b/security/ipe/Kconfig
> > index fcf82a8152ec..39df680b67a2 100644
> > --- a/security/ipe/Kconfig
> > +++ b/security/ipe/Kconfig
> > @@ -20,6 +20,16 @@ menuconfig SECURITY_IPE
> >
> >  if SECURITY_IPE
> >
> > +config IPE_BOOT_POLICY
> > +	string "Integrity policy to apply on system startup"
> > +	help
> > +	  This option specifies a filepath to a IPE policy that is compiled
> > +	  into the kernel. This policy will be enforced until a policy update
> > +	  is deployed via the $securityfs/ipe/policies/$policy_name/active
> > +	  interface.
> > +
> > +	  If unsure, leave blank.
> > +
> >  choice
> >  	prompt "Hash algorithm used in auditing policies"
> >  	default IPE_AUDIT_HASH_SHA1
> > diff --git a/security/ipe/Makefile b/security/ipe/Makefile
> > index 1e7b2d7fcd9e..89fec670f954 100644
> > --- a/security/ipe/Makefile
> > +++ b/security/ipe/Makefile
> > @@ -7,7 +7,18 @@
> >
> >  ccflags-y := -I$(srctree)/security/ipe/modules
> >
> > +quiet_cmd_polgen = IPE_POL $(2)
> > +      cmd_polgen = scripts/ipe/polgen/polgen security/ipe/boot-policy.c $(2)
> > +
> > +$(eval $(call config_filename,IPE_BOOT_POLICY))
> > +
> > +targets += boot-policy.c
> > +
> > +$(obj)/boot-policy.c: scripts/ipe/polgen/polgen
> > $(IPE_BOOT_POLICY_FILENAME) FORCE
> > +	$(call if_changed,polgen,$(IPE_BOOT_POLICY_FILENAME))
> > +
> >  obj-$(CONFIG_SECURITY_IPE) += \
> > +	boot-policy.o \
> >  	ctx.o \
> >  	eval.o \
> >  	fs.o \
> > @@ -21,3 +32,5 @@ obj-$(CONFIG_SECURITY_IPE) += \
> >  	policyfs.o \
> >
> >  obj-$(CONFIG_AUDIT) += audit.o
> > +
> > +clean-files := boot-policy.c \
> > diff --git a/security/ipe/ctx.c b/security/ipe/ctx.c
> > index fc9b8e467bc9..879acf4ceac5 100644
> > --- a/security/ipe/ctx.c
> > +++ b/security/ipe/ctx.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/spinlock.h>
> >  #include <linux/moduleparam.h>
> >
> > +extern const char *const ipe_boot_policy;
> >  static bool success_audit;
> >  static bool enforce = true;
> >
> > @@ -329,6 +330,7 @@ void ipe_put_ctx(struct ipe_context *ctx)
> >  int __init ipe_init_ctx(void)
> >  {
> >  	int rc = 0;
> > +	struct ipe_policy *p = NULL;
> >  	struct ipe_context *lns = NULL;
> >
> >  	lns = create_ctx();
> > @@ -342,10 +344,26 @@ int __init ipe_init_ctx(void)
> >  	WRITE_ONCE(lns->enforce, enforce);
> >  	spin_unlock(&lns->lock);
> >
> > +	if (ipe_boot_policy) {
> > +		p = ipe_new_policy(ipe_boot_policy, strlen(ipe_boot_policy),
> > +				   NULL, 0);
> > +		if (IS_ERR(p)) {
> > +			rc = PTR_ERR(lns);
> 
> This should be:
> 
> 	rc = PTR_ERR(p);
> 
> > +			goto err;
> > +		}
> > +
> > +		ipe_add_policy(lns, p);
> > +		rc = ipe_set_active_pol(p);
> > +		if (!rc)
> 
> Here you need to set a non-zero value, so that ipe_init()
> does not enable the LSM.

Actually you probably should just check that rc is not zero
and goto err.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> I would set to 1 a new global variable, like ipe_lsm_enabled,
> in ipe_init() just before security_add_hooks().
> 
> Then, I would add a check of this variable in ipe_init_securityfs()
> to avoid the kernel panic.
> 
> Roberto
> 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
> 
> > +			goto err;
> > +	}
> > +
> >  	rcu_assign_pointer(*ipe_tsk_ctx(current), lns);
> > +	ipe_put_policy(p);
> >
> >  	return 0;
> >  err:
> > +	ipe_put_policy(p);
> >  	ipe_put_ctx(lns);
> >  	return rc;
> >  }
> > --
> > 2.33.0

Re: [RFC PATCH v7 14/16] scripts: add boot policy generation program

[Deven Bowers]

On 11/3/2021 9:53 AM, Roberto Sassu wrote:
>> From: Roberto Sassu [mailto:roberto.sassu@huawei.com]
>> Sent: Wednesday, November 3, 2021 5:43 PM
>>> From: deven.desai@linux.microsoft.com
>>> [mailto:deven.desai@linux.microsoft.com]
>>> From: Deven Bowers <deven.desai@linux.microsoft.com>
>>>
>>> Enables an IPE policy to be enforced from kernel start, enabling access
>>> control based on trust from kernel startup. This is accomplished by
>>> transforming an IPE policy indicated by CONFIG_IPE_BOOT_POLICY into a
>>> c-string literal that is parsed at kernel startup as an unsigned policy.
>>>
>>> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
>>> ---
>>>
>>> Relevant changes since v6:
>>>    * Move patch 01/12 to [14/16] of the series
>>>
>>> ---
>>>   MAINTAINERS                   |   1 +
>>>   scripts/Makefile              |   1 +
>>>   scripts/ipe/Makefile          |   2 +
>>>   scripts/ipe/polgen/.gitignore |   1 +
>>>   scripts/ipe/polgen/Makefile   |   6 ++
>>>   scripts/ipe/polgen/polgen.c   | 145 ++++++++++++++++++++++++++++++++++
>>>   security/ipe/.gitignore       |   1 +
>>>   security/ipe/Kconfig          |  10 +++
>>>   security/ipe/Makefile         |  13 +++
>>>   security/ipe/ctx.c            |  18 +++++
>>>   10 files changed, 198 insertions(+)
>>>   create mode 100644 scripts/ipe/Makefile
>>>   create mode 100644 scripts/ipe/polgen/.gitignore
>>>   create mode 100644 scripts/ipe/polgen/Makefile
>>>   create mode 100644 scripts/ipe/polgen/polgen.c
>>>   create mode 100644 security/ipe/.gitignore
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index f1e76f791d47..a84ca781199b 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -9283,6 +9283,7 @@ INTEGRITY POLICY ENFORCEMENT (IPE)
>>>   M:	Deven Bowers <deven.desai@linux.microsoft.com>
>>>   M:	Fan Wu <wufan@linux.microsoft.com>
>>>   S:	Supported
>>> +F:	scripts/ipe/
>>>   F:	security/ipe/
>>>
>>>   INTEL 810/815 FRAMEBUFFER DRIVER
>>> diff --git a/scripts/Makefile b/scripts/Makefile
>>> index 9adb6d247818..a31da6d57a36 100644
>>> --- a/scripts/Makefile
>>> +++ b/scripts/Makefile
>>> @@ -41,6 +41,7 @@ targets += module.lds
>>>   subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
>>>   subdir-$(CONFIG_MODVERSIONS) += genksyms
>>>   subdir-$(CONFIG_SECURITY_SELINUX) += selinux
>>> +subdir-$(CONFIG_SECURITY_IPE) += ipe
>>>
>>>   # Let clean descend into subdirs
>>>   subdir-	+= basic dtc gdb kconfig mod
>>> diff --git a/scripts/ipe/Makefile b/scripts/ipe/Makefile
>>> new file mode 100644
>>> index 000000000000..e87553fbb8d6
>>> --- /dev/null
>>> +++ b/scripts/ipe/Makefile
>>> @@ -0,0 +1,2 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only
>>> +subdir-y := polgen
>>> diff --git a/scripts/ipe/polgen/.gitignore b/scripts/ipe/polgen/.gitignore
>>> new file mode 100644
>>> index 000000000000..80f32f25d200
>>> --- /dev/null
>>> +++ b/scripts/ipe/polgen/.gitignore
>>> @@ -0,0 +1 @@
>>> +polgen
>>> diff --git a/scripts/ipe/polgen/Makefile b/scripts/ipe/polgen/Makefile
>>> new file mode 100644
>>> index 000000000000..066060c22b4a
>>> --- /dev/null
>>> +++ b/scripts/ipe/polgen/Makefile
>>> @@ -0,0 +1,6 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +hostprogs-always-y	:= polgen
>>> +HOST_EXTRACFLAGS += \
>>> +	-I$(srctree)/include \
>>> +	-I$(srctree)/include/uapi \
>>> +
>>> diff --git a/scripts/ipe/polgen/polgen.c b/scripts/ipe/polgen/polgen.c
>>> new file mode 100644
>>> index 000000000000..73cf13e743f7
>>> --- /dev/null
>>> +++ b/scripts/ipe/polgen/polgen.c
>>> @@ -0,0 +1,145 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) Microsoft Corporation. All rights reserved.
>>> + */
>>> +
>>> +#include <stdlib.h>
>>> +#include <stddef.h>
>>> +#include <stdio.h>
>>> +#include <unistd.h>
>>> +#include <errno.h>
>>> +
>>> +static void usage(const char *const name)
>>> +{
>>> +	printf("Usage: %s OutputFile (PolicyFile)\n", name);
>>> +	exit(EINVAL);
>>> +}
>>> +
>>> +static int policy_to_buffer(const char *pathname, char **buffer, size_t *size)
>>> +{
>>> +	int rc = 0;
>>> +	FILE *fd;
>>> +	char *lbuf;
>>> +	size_t fsize;
>>> +	size_t read;
>>> +
>>> +	fd = fopen(pathname, "r");
>>> +	if (!fd) {
>>> +		rc = errno;
>>> +		goto out;
>>> +	}
>>> +
>>> +	fseek(fd, 0, SEEK_END);
>>> +	fsize = ftell(fd);
>>> +	rewind(fd);
>>> +
>>> +	lbuf = malloc(fsize);
>>> +	if (!lbuf) {
>>> +		rc = ENOMEM;
>>> +		goto out_close;
>>> +	}
>>> +
>>> +	read = fread((void *)lbuf, sizeof(*lbuf), fsize, fd);
>>> +	if (read != fsize) {
>>> +		rc = -1;
>>> +		goto out_free;
>>> +	}
>>> +
>>> +	*buffer = lbuf;
>>> +	*size = fsize;
>>> +	fclose(fd);
>>> +
>>> +	return rc;
>>> +
>>> +out_free:
>>> +	free(lbuf);
>>> +out_close:
>>> +	fclose(fd);
>>> +out:
>>> +	return rc;
>>> +}
>>> +
>>> +static int write_boot_policy(const char *pathname, const char *buf, size_t
>> size)
>>> +{
>>> +	int rc = 0;
>>> +	FILE *fd;
>>> +	size_t i;
>>> +
>>> +	fd = fopen(pathname, "w");
>>> +	if (!fd) {
>>> +		rc = errno;
>>> +		goto err;
>>> +	}
>>> +
>>> +	fprintf(fd, "/* This file is automatically generated.");
>>> +	fprintf(fd, " Do not edit. */\n");
>>> +	fprintf(fd, "#include <stddef.h>\n");
>>> +	fprintf(fd, "\nextern const char *const ipe_boot_policy;\n\n");
>>> +	fprintf(fd, "const char *const ipe_boot_policy =\n");
>>> +
>>> +	if (!buf || size == 0) {
>>> +		fprintf(fd, "\tNULL;\n");
>>> +		fclose(fd);
>>> +		return 0;
>>> +	}
>>> +
>>> +	fprintf(fd, "\t\"");
>>> +
>>> +	for (i = 0; i < size; ++i) {
>>> +		switch (buf[i]) {
>>> +		case '"':
>>> +			fprintf(fd, "\\\"");
>>> +			break;
>>> +		case '\'':
>>> +			fprintf(fd, "'");
>>> +			break;
>>> +		case '\n':
>>> +			fprintf(fd, "\\n\"\n\t\"");
>>> +			break;
>>> +		case '\\':
>>> +			fprintf(fd, "\\\\");
>>> +			break;
>>> +		case '\t':
>>> +			fprintf(fd, "\\t");
>>> +			break;
>>> +		case '\?':
>>> +			fprintf(fd, "\\?");
>>> +			break;
>>> +		default:
>>> +			fprintf(fd, "%c", buf[i]);
>>> +		}
>>> +	}
>>> +	fprintf(fd, "\";\n");
>>> +	fclose(fd);
>>> +
>>> +	return 0;
>>> +
>>> +err:
>>> +	if (fd)
>>> +		fclose(fd);
>>> +	return rc;
>>> +}
>>> +
>>> +int main(int argc, const char *const argv[])
>>> +{
>>> +	int rc = 0;
>>> +	size_t len = 0;
>>> +	char *policy = NULL;
>>> +
>>> +	if (argc < 2)
>>> +		usage(argv[0]);
>>> +
>>> +	if (argc > 2) {
>>> +		rc = policy_to_buffer(argv[2], &policy, &len);
>>> +		if (rc != 0)
>>> +			goto cleanup;
>>> +	}
>>> +
>>> +	rc = write_boot_policy(argv[1], policy, len);
>>> +cleanup:
>>> +	if (policy)
>>> +		free(policy);
>>> +	if (rc != 0)
>>> +		perror("An error occurred during policy conversion: ");
>>> +	return rc;
>>> +}
>>> diff --git a/security/ipe/.gitignore b/security/ipe/.gitignore
>>> new file mode 100644
>>> index 000000000000..eca22ad5ed22
>>> --- /dev/null
>>> +++ b/security/ipe/.gitignore
>>> @@ -0,0 +1 @@
>>> +boot-policy.c
>>> \ No newline at end of file
>>> diff --git a/security/ipe/Kconfig b/security/ipe/Kconfig
>>> index fcf82a8152ec..39df680b67a2 100644
>>> --- a/security/ipe/Kconfig
>>> +++ b/security/ipe/Kconfig
>>> @@ -20,6 +20,16 @@ menuconfig SECURITY_IPE
>>>
>>>   if SECURITY_IPE
>>>
>>> +config IPE_BOOT_POLICY
>>> +	string "Integrity policy to apply on system startup"
>>> +	help
>>> +	  This option specifies a filepath to a IPE policy that is compiled
>>> +	  into the kernel. This policy will be enforced until a policy update
>>> +	  is deployed via the $securityfs/ipe/policies/$policy_name/active
>>> +	  interface.
>>> +
>>> +	  If unsure, leave blank.
>>> +
>>>   choice
>>>   	prompt "Hash algorithm used in auditing policies"
>>>   	default IPE_AUDIT_HASH_SHA1
>>> diff --git a/security/ipe/Makefile b/security/ipe/Makefile
>>> index 1e7b2d7fcd9e..89fec670f954 100644
>>> --- a/security/ipe/Makefile
>>> +++ b/security/ipe/Makefile
>>> @@ -7,7 +7,18 @@
>>>
>>>   ccflags-y := -I$(srctree)/security/ipe/modules
>>>
>>> +quiet_cmd_polgen = IPE_POL $(2)
>>> +      cmd_polgen = scripts/ipe/polgen/polgen security/ipe/boot-policy.c $(2)
>>> +
>>> +$(eval $(call config_filename,IPE_BOOT_POLICY))
>>> +
>>> +targets += boot-policy.c
>>> +
>>> +$(obj)/boot-policy.c: scripts/ipe/polgen/polgen
>>> $(IPE_BOOT_POLICY_FILENAME) FORCE
>>> +	$(call if_changed,polgen,$(IPE_BOOT_POLICY_FILENAME))
>>> +
>>>   obj-$(CONFIG_SECURITY_IPE) += \
>>> +	boot-policy.o \
>>>   	ctx.o \
>>>   	eval.o \
>>>   	fs.o \
>>> @@ -21,3 +32,5 @@ obj-$(CONFIG_SECURITY_IPE) += \
>>>   	policyfs.o \
>>>
>>>   obj-$(CONFIG_AUDIT) += audit.o
>>> +
>>> +clean-files := boot-policy.c \
>>> diff --git a/security/ipe/ctx.c b/security/ipe/ctx.c
>>> index fc9b8e467bc9..879acf4ceac5 100644
>>> --- a/security/ipe/ctx.c
>>> +++ b/security/ipe/ctx.c
>>> @@ -15,6 +15,7 @@
>>>   #include <linux/spinlock.h>
>>>   #include <linux/moduleparam.h>
>>>
>>> +extern const char *const ipe_boot_policy;
>>>   static bool success_audit;
>>>   static bool enforce = true;
>>>
>>> @@ -329,6 +330,7 @@ void ipe_put_ctx(struct ipe_context *ctx)
>>>   int __init ipe_init_ctx(void)
>>>   {
>>>   	int rc = 0;
>>> +	struct ipe_policy *p = NULL;
>>>   	struct ipe_context *lns = NULL;
>>>
>>>   	lns = create_ctx();
>>> @@ -342,10 +344,26 @@ int __init ipe_init_ctx(void)
>>>   	WRITE_ONCE(lns->enforce, enforce);
>>>   	spin_unlock(&lns->lock);
>>>
>>> +	if (ipe_boot_policy) {
>>> +		p = ipe_new_policy(ipe_boot_policy, strlen(ipe_boot_policy),
>>> +				   NULL, 0);
>>> +		if (IS_ERR(p)) {
>>> +			rc = PTR_ERR(lns);
>> This should be:
>>
>> 	rc = PTR_ERR(p);
>>
>>> +			goto err;
>>> +		}
>>> +
>>> +		ipe_add_policy(lns, p);
>>> +		rc = ipe_set_active_pol(p);
>>> +		if (!rc)
>> Here you need to set a non-zero value, so that ipe_init()
>> does not enable the LSM.
> Actually you probably should just check that rc is not zero
> and goto err.
>
> Roberto
>
Yeah, I actually noticed these two mistakes myself fairly recently.

Changes will be applied in v8.
>
>> I would set to 1 a new global variable, like ipe_lsm_enabled,
>> in ipe_init() just before security_add_hooks().
>>
>> Then, I would add a check of this variable in ipe_init_securityfs()
>> to avoid the kernel panic.

That's my plan.

-Deven

RE: [RFC PATCH v7 11/16] ipe: add support for dm-verity as a trust provider

[Roberto Sassu]

> From: deven.desai@linux.microsoft.com
> [mailto:deven.desai@linux.microsoft.com]
> Sent: Wednesday, October 13, 2021 9:07 PM
> From: Deven Bowers <deven.desai@linux.microsoft.com>
> 
> Allows author of IPE policy to indicate trust for a singular dm-verity
> volume, identified by roothash, through "dmverity_roothash" and all
> signed dm-verity volumes, through "dmverity_signature".
> 
> Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com>
> ---
> 
> Relevant changes since v6:
>   * Squash patch 08/12, 10/12 to [11/16]
> 
> ---
>  security/ipe/eval.c                       |  5 ++
>  security/ipe/eval.h                       | 10 +++
>  security/ipe/hooks.c                      | 48 ++++++++++++++
>  security/ipe/hooks.h                      |  6 ++
>  security/ipe/ipe.c                        |  9 +++
>  security/ipe/ipe.h                        |  3 +
>  security/ipe/modules/Kconfig              | 23 +++++++
>  security/ipe/modules/Makefile             |  2 +
>  security/ipe/modules/dmverity_roothash.c  | 80 +++++++++++++++++++++++
>  security/ipe/modules/dmverity_signature.c | 25 +++++++
>  10 files changed, 211 insertions(+)
>  create mode 100644 security/ipe/modules/dmverity_roothash.c
>  create mode 100644 security/ipe/modules/dmverity_signature.c
> 
> diff --git a/security/ipe/eval.c b/security/ipe/eval.c
> index 361efccebad4..facc05c753f4 100644
> --- a/security/ipe/eval.c
> +++ b/security/ipe/eval.c
> @@ -23,6 +23,7 @@ static struct super_block *pinned_sb;
>  static DEFINE_SPINLOCK(pin_lock);
> 
>  #define FILE_SUPERBLOCK(f) ((f)->f_path.mnt->mnt_sb)
> +#define FILE_BLOCK_DEV(f) (FILE_SUPERBLOCK(f)->s_bdev)
> 
>  /**
>   * pin_sb: pin the underlying superblock of @f, marking it as trusted
> @@ -95,6 +96,10 @@ static struct ipe_eval_ctx *build_ctx(const struct file *file,
>  	ctx->hook = hook;
>  	ctx->ci_ctx = ipe_current_ctx();
>  	ctx->from_init_sb = from_pinned(file);
> +	if (file) {
> +		if (FILE_BLOCK_DEV(file))
> +			ctx->ipe_bdev = ipe_bdev(FILE_BLOCK_DEV(file));
> +	}
> 
>  	return ctx;
>  }
> diff --git a/security/ipe/eval.h b/security/ipe/eval.h
> index 42fb7fdf2599..25d2d8d55702 100644
> --- a/security/ipe/eval.h
> +++ b/security/ipe/eval.h
> @@ -13,6 +13,14 @@
>  #include "hooks.h"
>  #include "policy.h"
> 
> +struct ipe_bdev {
> +	const u8       *sigdata;
> +	size_t		siglen;
> +
> +	const u8       *hash;
> +	size_t		hashlen;
> +};
> +
>  struct ipe_eval_ctx {
>  	enum ipe_hook hook;
>  	enum ipe_operation op;
> @@ -20,6 +28,8 @@ struct ipe_eval_ctx {
>  	const struct file *file;
>  	struct ipe_context *ci_ctx;
> 
> +	const struct ipe_bdev *ipe_bdev;
> +
>  	bool from_init_sb;
>  };
> 
> diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
> index 2d4a4f0eead0..470fb48e490c 100644
> --- a/security/ipe/hooks.c
> +++ b/security/ipe/hooks.c
> @@ -13,6 +13,7 @@
>  #include <linux/types.h>
>  #include <linux/refcount.h>
>  #include <linux/rcupdate.h>
> +#include <linux/blk_types.h>
>  #include <linux/binfmts.h>
>  #include <linux/mman.h>
> 
> @@ -219,3 +220,50 @@ void ipe_sb_free_security(struct super_block *mnt_sb)
>  {
>  	ipe_invalidate_pinned_sb(mnt_sb);
>  }
> +
> +/**
> + * ipe_bdev_free_security: free nested structures within IPE's LSM blob
> + *			   in block_devices
> + * @bdev: Supplies a pointer to a block_device that contains the structure
> + *	  to free.
> + */
> +void ipe_bdev_free_security(struct block_device *bdev)
> +{
> +	struct ipe_bdev *blob = ipe_bdev(bdev);
> +
> +	kfree(blob->sigdata);
> +}
> +
> +/**
> + * ipe_bdev_setsecurity: associate some data from the block device layer
> + *			 with IPE's LSM blob.
> + * @bdev: Supplies a pointer to a block_device that contains the LSM blob.
> + * @key: Supplies the string key that uniquely identifies the value.
> + * @value: Supplies the value to store.
> + * @len: The length of @value.
> + */
> +int ipe_bdev_setsecurity(struct block_device *bdev, const char *key,
> +			 const void *value, size_t len)
> +{
> +	struct ipe_bdev *blob = ipe_bdev(bdev);
> +
> +	if (!strcmp(key, DM_VERITY_SIGNATURE_SEC_NAME)) {
> +		blob->siglen = len;
> +		blob->sigdata = kmemdup(value, len, GFP_KERNEL);
> +		if (!blob->sigdata)
> +			return -ENOMEM;
> +
> +		return 0;
> +	}
> +
> +	if (!strcmp(key, DM_VERITY_ROOTHASH_SEC_NAME)) {
> +		blob->hashlen = len;
> +		blob->hash = kmemdup(value, len, GFP_KERNEL);
> +		if (!blob->hash)
> +			return -ENOMEM;
> +
> +		return 0;
> +	}
> +
> +	return -ENOSYS;
> +}
> diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
> index e7f107ab5620..285f35187188 100644
> --- a/security/ipe/hooks.h
> +++ b/security/ipe/hooks.h
> @@ -10,6 +10,7 @@
>  #include <linux/sched.h>
>  #include <linux/binfmts.h>
>  #include <linux/security.h>
> +#include <linux/device-mapper.h>
> 
>  enum ipe_hook {
>  	ipe_hook_exec = 0,
> @@ -40,4 +41,9 @@ int ipe_on_kernel_load_data(enum kernel_load_data_id
> id, bool contents);
> 
>  void ipe_sb_free_security(struct super_block *mnt_sb);
> 
> +void ipe_bdev_free_security(struct block_device *bdev);
> +
> +int ipe_bdev_setsecurity(struct block_device *bdev, const char *key,
> +			 const void *value, size_t len);
> +
>  #endif /* IPE_HOOKS_H */
> diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
> index 1382d50078ec..215936cb4574 100644
> --- a/security/ipe/ipe.c
> +++ b/security/ipe/ipe.c
> @@ -9,6 +9,7 @@
>  #include "ipe_parser.h"
>  #include "modules/ipe_module.h"
>  #include "modules.h"
> +#include "eval.h"
> 
>  #include <linux/fs.h>
>  #include <linux/sched.h>
> @@ -20,8 +21,14 @@
> 
>  struct lsm_blob_sizes ipe_blobs __lsm_ro_after_init = {
>  	.lbs_task = sizeof(struct ipe_context __rcu *),
> +	.lbs_bdev = sizeof(struct ipe_bdev),
>  };
> 
> +struct ipe_bdev *ipe_bdev(struct block_device *b)
> +{
> +	return b->security + ipe_blobs.lbs_bdev;
> +}
> +
>  static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(task_alloc, ipe_task_alloc),
>  	LSM_HOOK_INIT(task_free, ipe_task_free),
> @@ -31,6 +38,8 @@ static struct security_hook_list ipe_hooks[]
> __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(kernel_read_file, ipe_on_kernel_read),
>  	LSM_HOOK_INIT(kernel_load_data, ipe_on_kernel_load_data),
>  	LSM_HOOK_INIT(sb_free_security, ipe_sb_free_security),
> +	LSM_HOOK_INIT(bdev_free_security, ipe_bdev_free_security),
> +	LSM_HOOK_INIT(bdev_setsecurity, ipe_bdev_setsecurity),
>  };
> 
>  /**
> diff --git a/security/ipe/ipe.h b/security/ipe/ipe.h
> index ad16d2bebfec..6b4c7e07f204 100644
> --- a/security/ipe/ipe.h
> +++ b/security/ipe/ipe.h
> @@ -14,10 +14,13 @@
> 
>  #include <linux/types.h>
>  #include <linux/sched.h>
> +#include <linux/blk_types.h>
>  #include <linux/lsm_hooks.h>
> 
>  extern struct lsm_blob_sizes ipe_blobs;
>  extern struct ipe_parser __start_ipe_parsers[], __end_ipe_parsers[];
>  extern struct ipe_module __start_ipe_modules[], __end_ipe_modules[];
> 
> +struct ipe_bdev *ipe_bdev(struct block_device *b);
> +
>  #endif /* IPE_H */
> diff --git a/security/ipe/modules/Kconfig b/security/ipe/modules/Kconfig
> index fad96ba534e2..a6ea06cf0737 100644
> --- a/security/ipe/modules/Kconfig
> +++ b/security/ipe/modules/Kconfig
> @@ -16,5 +16,28 @@ config IPE_PROP_BOOT_VERIFIED
> 
>  	  If unsure, answer N.
> 
> +config IPE_PROP_DM_VERITY_SIGNATURE
> +	bool "Enable support for signed dm-verity volumes"
> +	depends on DM_VERITY_VERIFY_ROOTHASH_SIG
> +	default Y
> +	help
> +	  This option enables the property 'dmverity_signature' in
> +	  IPE policy. This property evaluates to TRUE when a file
> +	  is evaluated against a dm-verity volume that was mounted
> +	  with a signed root-hash.
> +
> +	  If unsure, answer Y.
> +
> +config IPE_PROP_DM_VERITY_ROOTHASH
> +	bool "Enable support for dm-verity volumes"
> +	depends on DM_VERITY
> +	default Y
> +	help
> +	  This option enables the property 'dmverity_roothash' in
> +	  IPE policy. This property evaluates to TRUE when a file
> +	  is evaluated against a dm-verity volume whose root hash
> +	  matches the supplied value.
> +
> +	  If unsure, answer Y.
> 
>  endmenu
> diff --git a/security/ipe/modules/Makefile b/security/ipe/modules/Makefile
> index e0045ec65434..84fadce85193 100644
> --- a/security/ipe/modules/Makefile
> +++ b/security/ipe/modules/Makefile
> @@ -6,3 +6,5 @@
>  #
> 
>  obj-$(CONFIG_IPE_PROP_BOOT_VERIFIED) += boot_verified.o
> +obj-$(CONFIG_IPE_PROP_DM_VERITY_SIGNATURE) += dmverity_signature.o
> +obj-$(CONFIG_IPE_PROP_DM_VERITY_ROOTHASH) += dmverity_roothash.o
> diff --git a/security/ipe/modules/dmverity_roothash.c
> b/security/ipe/modules/dmverity_roothash.c
> new file mode 100644
> index 000000000000..0f82bec3b842
> --- /dev/null
> +++ b/security/ipe/modules/dmverity_roothash.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) Microsoft Corporation. All rights reserved.
> + */
> +
> +#include "ipe_module.h"
> +
> +#include <linux/fs.h>
> +#include <linux/types.h>
> +
> +struct counted_array {
> +	size_t	len;
> +	u8     *data;
> +};
> +
> +static int dvrh_parse(const char *valstr, void **value)
> +{
> +	int rv = 0;
> +	struct counted_array *arr;
> +
> +	arr = kzalloc(sizeof(*arr), GFP_KERNEL);
> +	if (!arr) {
> +		rv = -ENOMEM;
> +		goto err;
> +	}
> +
> +	arr->len = (strlen(valstr) / 2);
> +
> +	arr->data = kzalloc(arr->len, GFP_KERNEL);
> +	if (!arr->data) {
> +		rv = -ENOMEM;
> +		goto err;
> +	}
> +
> +	rv = hex2bin(arr->data, valstr, arr->len);
> +	if (rv != 0)
> +		goto err2;
> +
> +	*value = arr;
> +	return rv;
> +err2:
> +	kfree(arr->data);
> +err:
> +	kfree(arr);
> +	return rv;
> +}
> +
> +static bool dvrh_eval(const struct ipe_eval_ctx *ctx, const void *val)
> +{
> +	const u8 *src;
> +	struct counted_array *expect = (struct counted_array *)val;
> +
> +	if (!ctx->ipe_bdev)
> +		return false;
> +
> +	if (ctx->ipe_bdev->hashlen != expect->len)
> +		return false;
> +
> +	src = ctx->ipe_bdev->hash;
> +
> +	return !memcmp(expect->data, src, expect->len);

Hi Deven

I was curious to see if determining the property at run-time
could apply also to dm-verity. It seems it could be done
(I omit some checks, I also keep the expected value in hex
format):

---
        md = dm_get_md(file_inode(ctx->file)->i_sb->s_dev);
        table = dm_get_live_table(md, &srcu_idx);
        num_targets = dm_table_get_num_targets(table);

        for (i = 0; i < num_targets; i++) {
                struct dm_target *ti = dm_table_get_target(table, i);

                if (strcmp(ti->type->name, "verity"))
                        continue;

                ti->type->status(ti, STATUSTYPE_IMA, 0, result, sizeof(result));
        }

        dm_put_live_table(md, srcu_idx);
        dm_put(md);

        root_digest_ptr = strstr(result, "root_digest=");
        return !strncmp(expect->data, root_digest_ptr + 12, expect->len);
---

Only dm_table_get_target() is not exported yet, but I guess it could
be. dm_table_get_num_targets() is exported.

With this code, you would not have to manage security blobs
outside IPE. Maybe you could add a blob for the super block, so
that you verify the dm-verity property just once per filesystem.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> +}
> +
> +static int dvrh_free(void **val)
> +{
> +	struct counted_array *expect = (struct counted_array *)val;
> +
> +	kfree(expect->data);
> +	kfree(expect);
> +
> +	return 0;
> +}
> +
> +IPE_MODULE(dvrh) = {
> +	.name = "dmverity_roothash",
> +	.version = 1,
> +	.parse = dvrh_parse,
> +	.free = dvrh_free,
> +	.eval = dvrh_eval,
> +};
> diff --git a/security/ipe/modules/dmverity_signature.c
> b/security/ipe/modules/dmverity_signature.c
> new file mode 100644
> index 000000000000..08746fcbcb3e
> --- /dev/null
> +++ b/security/ipe/modules/dmverity_signature.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) Microsoft Corporation. All rights reserved.
> + */
> +
> +#include "ipe_module.h"
> +
> +#include <linux/fs.h>
> +#include <linux/types.h>
> +
> +static bool dvv_eval(const struct ipe_eval_ctx *ctx, const void *val)
> +{
> +	bool expect = (bool)val;
> +	bool eval = ctx->ipe_bdev && (!!ctx->ipe_bdev->sigdata);
> +
> +	return expect == eval;
> +}
> +
> +IPE_MODULE(dvv) = {
> +	.name = "dmverity_signature",
> +	.version = 1,
> +	.parse = ipe_bool_parse,
> +	.free = NULL,
> +	.eval = dvv_eval,
> +};
> --
> 2.33.0

Re: [RFC PATCH v7 11/16] ipe: add support for dm-verity as a trust provider

[Deven Bowers]

On 11/25/2021 1:37 AM, Roberto Sassu wrote:
>> From: deven.desai@linux.microsoft.com
>> [mailto:deven.desai@linux.microsoft.com]
>> Sent: Wednesday, October 13, 2021 9:07 PM
>> From: Deven Bowers <deven.desai@linux.microsoft.com>

..snip

>> diff --git a/security/ipe/modules/Makefile b/security/ipe/modules/Makefile
>> index e0045ec65434..84fadce85193 100644
>> --- a/security/ipe/modules/Makefile
>> +++ b/security/ipe/modules/Makefile
>> @@ -6,3 +6,5 @@
>>   #
>>
>>   obj-$(CONFIG_IPE_PROP_BOOT_VERIFIED) += boot_verified.o
>> +obj-$(CONFIG_IPE_PROP_DM_VERITY_SIGNATURE) += dmverity_signature.o
>> +obj-$(CONFIG_IPE_PROP_DM_VERITY_ROOTHASH) += dmverity_roothash.o
>> diff --git a/security/ipe/modules/dmverity_roothash.c
>> b/security/ipe/modules/dmverity_roothash.c
>> new file mode 100644
>> index 000000000000..0f82bec3b842
>> --- /dev/null
>> +++ b/security/ipe/modules/dmverity_roothash.c
>> @@ -0,0 +1,80 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) Microsoft Corporation. All rights reserved.
>> + */
>> +
>> +#include "ipe_module.h"
>> +
>> +#include <linux/fs.h>
>> +#include <linux/types.h>
>> +
>> +struct counted_array {
>> +	size_t	len;
>> +	u8     *data;
>> +};
>> +
>> +static int dvrh_parse(const char *valstr, void **value)
>> +{
>> +	int rv = 0;
>> +	struct counted_array *arr;
>> +
>> +	arr = kzalloc(sizeof(*arr), GFP_KERNEL);
>> +	if (!arr) {
>> +		rv = -ENOMEM;
>> +		goto err;
>> +	}
>> +
>> +	arr->len = (strlen(valstr) / 2);
>> +
>> +	arr->data = kzalloc(arr->len, GFP_KERNEL);
>> +	if (!arr->data) {
>> +		rv = -ENOMEM;
>> +		goto err;
>> +	}
>> +
>> +	rv = hex2bin(arr->data, valstr, arr->len);
>> +	if (rv != 0)
>> +		goto err2;
>> +
>> +	*value = arr;
>> +	return rv;
>> +err2:
>> +	kfree(arr->data);
>> +err:
>> +	kfree(arr);
>> +	return rv;
>> +}
>> +
>> +static bool dvrh_eval(const struct ipe_eval_ctx *ctx, const void *val)
>> +{
>> +	const u8 *src;
>> +	struct counted_array *expect = (struct counted_array *)val;
>> +
>> +	if (!ctx->ipe_bdev)
>> +		return false;
>> +
>> +	if (ctx->ipe_bdev->hashlen != expect->len)
>> +		return false;
>> +
>> +	src = ctx->ipe_bdev->hash;
>> +
>> +	return !memcmp(expect->data, src, expect->len);
> Hi Deven
>
> I was curious to see if determining the property at run-time
> could apply also to dm-verity. It seems it could be done
> (I omit some checks, I also keep the expected value in hex
> format):
>
> ---
>          md = dm_get_md(file_inode(ctx->file)->i_sb->s_dev);
>          table = dm_get_live_table(md, &srcu_idx);
>          num_targets = dm_table_get_num_targets(table);
>
>          for (i = 0; i < num_targets; i++) {
>                  struct dm_target *ti = dm_table_get_target(table, i);
>
>                  if (strcmp(ti->type->name, "verity"))
>                          continue;
>
>                  ti->type->status(ti, STATUSTYPE_IMA, 0, result, sizeof(result));
>          }
>
>          dm_put_live_table(md, srcu_idx);
>          dm_put(md);
>
>          root_digest_ptr = strstr(result, "root_digest=");
>          return !strncmp(expect->data, root_digest_ptr + 12, expect->len);
> ---
>
> Only dm_table_get_target() is not exported yet, but I guess it could
> be. dm_table_get_num_targets() is exported.

I had tried something similar in a very early draft of IPE. The issue
that comes with this is that when you compile device-mapper as a module
(CONFIG_BLK_DEV_DM=m) you start to get linking errors with this
approach.

Obviously, we can fix this in the IPE's module Kconfig by setting the
dependency to be =y, but it's something to highlight. My general
preference is to support the =m configuration by using these blobs.

The runtime approach does work with fs-verity, because fs-verity is a
file-system level feature that cannot be compiled as a module.

> With this code, you would not have to manage security blobs
> outside IPE. Maybe you could add a blob for the super block, so
> that you verify the dm-verity property just once per filesystem.
>
> Roberto
>
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
>
>> +}
>> +
>> +static int dvrh_free(void **val)
>> +{
>> +	struct counted_array *expect = (struct counted_array *)val;
>> +
>> +	kfree(expect->data);
>> +	kfree(expect);
>> +
>> +	return 0;
>> +}
>> +
>> +IPE_MODULE(dvrh) = {
>> +	.name = "dmverity_roothash",
>> +	.version = 1,
>> +	.parse = dvrh_parse,
>> +	.free = dvrh_free,
>> +	.eval = dvrh_eval,
>> +};
>> diff --git a/security/ipe/modules/dmverity_signature.c
>> b/security/ipe/modules/dmverity_signature.c
>> new file mode 100644
>> index 000000000000..08746fcbcb3e
>> --- /dev/null
>> +++ b/security/ipe/modules/dmverity_signature.c
>> @@ -0,0 +1,25 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) Microsoft Corporation. All rights reserved.
>> + */
>> +
>> +#include "ipe_module.h"
>> +
>> +#include <linux/fs.h>
>> +#include <linux/types.h>
>> +
>> +static bool dvv_eval(const struct ipe_eval_ctx *ctx, const void *val)
>> +{
>> +	bool expect = (bool)val;
>> +	bool eval = ctx->ipe_bdev && (!!ctx->ipe_bdev->sigdata);
>> +
>> +	return expect == eval;
>> +}
>> +
>> +IPE_MODULE(dvv) = {
>> +	.name = "dmverity_signature",
>> +	.version = 1,
>> +	.parse = ipe_bool_parse,
>> +	.free = NULL,
>> +	.eval = dvv_eval,
>> +};
>> --
>> 2.33.0

[RFC][PATCH] device mapper: Add builtin function dm_get_status()

[Roberto Sassu]

Users of the device mapper driver might want to obtain a device status,
with status types defined in the status_type_t enumerator.

If a function to get the status is exported by the device mapper, when
compiled as a module, it is not suitable to use by callers compiled builtin
in the kernel.

Introduce the real function to get the status, _dm_get_status(), in the
device mapper module, and add the stub dm_get_status() in dm-builtin.c, so
that it can be invoked by builtin callers.

The stub calls the real function if the device mapper is compiled builtin
or the module has been loaded. Calls to the real function are safely
disabled if the module is unloaded. The race condition is avoided by
incrementing the reference count of the module.

_dm_get_status() invokes the status() method for each device mapper table,
which writes a string to the supplied buffer as output. The buffer might
contain multiple strings concatenated together. If there is not enough
space available, the string is truncated and a termination character is put
at the end.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 drivers/md/dm-builtin.c       | 13 +++++++
 drivers/md/dm-core.h          |  5 +++
 drivers/md/dm.c               | 71 +++++++++++++++++++++++++++++++++++
 include/linux/device-mapper.h |  3 ++
 4 files changed, 92 insertions(+)

diff --git a/drivers/md/dm-builtin.c b/drivers/md/dm-builtin.c
index 8eb52e425141..cc1e9c27ab41 100644
--- a/drivers/md/dm-builtin.c
+++ b/drivers/md/dm-builtin.c
@@ -47,3 +47,16 @@ void dm_kobject_release(struct kobject *kobj)
 }
 
 EXPORT_SYMBOL(dm_kobject_release);
+
+dm_get_status_fn status_fn;
+EXPORT_SYMBOL(status_fn);
+
+ssize_t dm_get_status(dev_t dev, status_type_t type, const char *target_name,
+		      u8 *buf, size_t buf_len)
+{
+	if (status_fn)
+		return status_fn(dev, type, target_name, buf, buf_len);
+
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(dm_get_status);
diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
index b855fef4f38a..6600ec260558 100644
--- a/drivers/md/dm-core.h
+++ b/drivers/md/dm-core.h
@@ -259,4 +259,9 @@ extern atomic_t dm_global_event_nr;
 extern wait_queue_head_t dm_global_eventq;
 void dm_issue_global_event(void);
 
+typedef ssize_t (*dm_get_status_fn)(dev_t dev, status_type_t type,
+				    const char *target_name, u8 *buf,
+				    size_t buf_len);
+
+extern dm_get_status_fn status_fn;
 #endif
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 662742a310cb..55e59a4e3661 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -192,6 +192,74 @@ static unsigned dm_get_numa_node(void)
 					 DM_NUMA_NODE, num_online_nodes() - 1);
 }
 
+static ssize_t _dm_get_status(dev_t dev, status_type_t type,
+			      const char *target_name, u8 *buf, size_t buf_len)
+{
+	struct mapped_device *md;
+	struct dm_table *table;
+	u8 *buf_ptr = buf;
+	ssize_t len, res = 0;
+	int srcu_idx, num_targets, i;
+
+	if (buf_len > INT_MAX)
+		return -EINVAL;
+
+	if (!buf_len)
+		return buf_len;
+
+	if (!try_module_get(THIS_MODULE))
+		return -EBUSY;
+
+	md = dm_get_md(dev);
+	if (!md) {
+		res = -ENOENT;
+		goto out_module;
+	}
+
+	table = dm_get_live_table(md, &srcu_idx);
+	if (!table) {
+		res = -ENOENT;
+		goto out_md;
+	}
+
+	memset(buf, 0, buf_len);
+
+	num_targets = dm_table_get_num_targets(table);
+
+	for (i = 0; i < num_targets; i++) {
+		struct dm_target *ti = dm_table_get_target(table, i);
+
+		if (!ti)
+			continue;
+
+		if (target_name && strcmp(ti->type->name, target_name))
+			continue;
+
+		if (!ti->type->status)
+			continue;
+
+		ti->type->status(ti, type, 0, buf_ptr, buf + buf_len - buf_ptr);
+
+		if (!*buf_ptr)
+			continue;
+
+		len = strlen(buf_ptr);
+		buf_ptr += len + 1;
+
+		if (buf_ptr == buf + buf_len)
+			break;
+
+		res += len + 1;
+	}
+
+	dm_put_live_table(md, srcu_idx);
+out_md:
+	dm_put(md);
+out_module:
+	module_put(THIS_MODULE);
+	return res;
+}
+
 static int __init local_init(void)
 {
 	int r;
@@ -275,6 +343,7 @@ static int __init dm_init(void)
 			goto bad;
 	}
 
+	status_fn = _dm_get_status;
 	return 0;
 bad:
 	while (i--)
@@ -287,6 +356,8 @@ static void __exit dm_exit(void)
 {
 	int i = ARRAY_SIZE(_exits);
 
+	status_fn = NULL;
+
 	while (i--)
 		_exits[i]();
 
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index a7df155ea49b..d97b296d3104 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -487,6 +487,9 @@ int dm_report_zones(struct block_device *bdev, sector_t start, sector_t sector,
 		    struct dm_report_zones_args *args, unsigned int nr_zones);
 #endif /* CONFIG_BLK_DEV_ZONED */
 
+ssize_t dm_get_status(dev_t dev, status_type_t type, const char *target_name,
+		      u8 *buf, size_t buf_len);
+
 /*
  * Device mapper functions to parse and create devices specified by the
  * parameter "dm-mod.create="
-- 
2.32.0

RE: [RFC][PATCH] device mapper: Add builtin function dm_get_status()

[Roberto Sassu]

> From: Roberto Sassu
> Sent: Wednesday, December 1, 2021 5:37 PM
> Users of the device mapper driver might want to obtain a device status,
> with status types defined in the status_type_t enumerator.
> 
> If a function to get the status is exported by the device mapper, when
> compiled as a module, it is not suitable to use by callers compiled builtin
> in the kernel.
> 
> Introduce the real function to get the status, _dm_get_status(), in the
> device mapper module, and add the stub dm_get_status() in dm-builtin.c, so
> that it can be invoked by builtin callers.
> 
> The stub calls the real function if the device mapper is compiled builtin
> or the module has been loaded. Calls to the real function are safely
> disabled if the module is unloaded. The race condition is avoided by
> incrementing the reference count of the module.
> 
> _dm_get_status() invokes the status() method for each device mapper table,
> which writes a string to the supplied buffer as output. The buffer might
> contain multiple strings concatenated together. If there is not enough
> space available, the string is truncated and a termination character is put
> at the end.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  drivers/md/dm-builtin.c       | 13 +++++++
>  drivers/md/dm-core.h          |  5 +++
>  drivers/md/dm.c               | 71 +++++++++++++++++++++++++++++++++++
>  include/linux/device-mapper.h |  3 ++
>  4 files changed, 92 insertions(+)
> 
> diff --git a/drivers/md/dm-builtin.c b/drivers/md/dm-builtin.c
> index 8eb52e425141..cc1e9c27ab41 100644
> --- a/drivers/md/dm-builtin.c
> +++ b/drivers/md/dm-builtin.c
> @@ -47,3 +47,16 @@ void dm_kobject_release(struct kobject *kobj)
>  }
> 
>  EXPORT_SYMBOL(dm_kobject_release);
> +
> +dm_get_status_fn status_fn;
> +EXPORT_SYMBOL(status_fn);
> +
> +ssize_t dm_get_status(dev_t dev, status_type_t type, const char
> *target_name,
> +		      u8 *buf, size_t buf_len)
> +{
> +	if (status_fn)
> +		return status_fn(dev, type, target_name, buf, buf_len);
> +
> +	return -EOPNOTSUPP;
> +}
> +EXPORT_SYMBOL(dm_get_status);
> diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> index b855fef4f38a..6600ec260558 100644
> --- a/drivers/md/dm-core.h
> +++ b/drivers/md/dm-core.h
> @@ -259,4 +259,9 @@ extern atomic_t dm_global_event_nr;
>  extern wait_queue_head_t dm_global_eventq;
>  void dm_issue_global_event(void);
> 
> +typedef ssize_t (*dm_get_status_fn)(dev_t dev, status_type_t type,
> +				    const char *target_name, u8 *buf,
> +				    size_t buf_len);
> +
> +extern dm_get_status_fn status_fn;
>  #endif
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 662742a310cb..55e59a4e3661 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -192,6 +192,74 @@ static unsigned dm_get_numa_node(void)
>  					 DM_NUMA_NODE,
> num_online_nodes() - 1);
>  }
> 
> +static ssize_t _dm_get_status(dev_t dev, status_type_t type,
> +			      const char *target_name, u8 *buf, size_t buf_len)
> +{
> +	struct mapped_device *md;
> +	struct dm_table *table;
> +	u8 *buf_ptr = buf;
> +	ssize_t len, res = 0;
> +	int srcu_idx, num_targets, i;
> +
> +	if (buf_len > INT_MAX)
> +		return -EINVAL;
> +
> +	if (!buf_len)
> +		return buf_len;
> +
> +	if (!try_module_get(THIS_MODULE))
> +		return -EBUSY;
> +
> +	md = dm_get_md(dev);
> +	if (!md) {
> +		res = -ENOENT;
> +		goto out_module;
> +	}
> +
> +	table = dm_get_live_table(md, &srcu_idx);
> +	if (!table) {
> +		res = -ENOENT;
> +		goto out_md;
> +	}
> +
> +	memset(buf, 0, buf_len);
> +
> +	num_targets = dm_table_get_num_targets(table);
> +
> +	for (i = 0; i < num_targets; i++) {
> +		struct dm_target *ti = dm_table_get_target(table, i);
> +
> +		if (!ti)
> +			continue;
> +
> +		if (target_name && strcmp(ti->type->name, target_name))
> +			continue;
> +
> +		if (!ti->type->status)
> +			continue;
> +
> +		ti->type->status(ti, type, 0, buf_ptr, buf + buf_len - buf_ptr);
> +
> +		if (!*buf_ptr)
> +			continue;
> +
> +		len = strlen(buf_ptr);
> +		buf_ptr += len + 1;
> +
> +		if (buf_ptr == buf + buf_len)
> +			break;
> +
> +		res += len + 1;

The line above before the check.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> +	}
> +
> +	dm_put_live_table(md, srcu_idx);
> +out_md:
> +	dm_put(md);
> +out_module:
> +	module_put(THIS_MODULE);
> +	return res;
> +}
> +
>  static int __init local_init(void)
>  {
>  	int r;
> @@ -275,6 +343,7 @@ static int __init dm_init(void)
>  			goto bad;
>  	}
> 
> +	status_fn = _dm_get_status;
>  	return 0;
>  bad:
>  	while (i--)
> @@ -287,6 +356,8 @@ static void __exit dm_exit(void)
>  {
>  	int i = ARRAY_SIZE(_exits);
> 
> +	status_fn = NULL;
> +
>  	while (i--)
>  		_exits[i]();
> 
> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index a7df155ea49b..d97b296d3104 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -487,6 +487,9 @@ int dm_report_zones(struct block_device *bdev,
> sector_t start, sector_t sector,
>  		    struct dm_report_zones_args *args, unsigned int nr_zones);
>  #endif /* CONFIG_BLK_DEV_ZONED */
> 
> +ssize_t dm_get_status(dev_t dev, status_type_t type, const char
> *target_name,
> +		      u8 *buf, size_t buf_len);
> +
>  /*
>   * Device mapper functions to parse and create devices specified by the
>   * parameter "dm-mod.create="
> --
> 2.32.0

Re: [RFC][PATCH] device mapper: Add builtin function dm_get_status()

[Christoph Hellwig]

On Wed, Dec 01, 2021 at 05:37:08PM +0100, Roberto Sassu wrote:
> Users of the device mapper driver might want to obtain a device status,
> with status types defined in the status_type_t enumerator.

The patch looks really odd.  And without the corresponding user of your
new functionality it is entirely unreviewable anyway.

RE: [RFC][PATCH] device mapper: Add builtin function dm_get_status()

[Roberto Sassu]

> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, December 2, 2021 8:21 AM
> On Wed, Dec 01, 2021 at 05:37:08PM +0100, Roberto Sassu wrote:
> > Users of the device mapper driver might want to obtain a device status,
> > with status types defined in the status_type_t enumerator.
> 
> The patch looks really odd.  And without the corresponding user of your
> new functionality it is entirely unreviewable anyway.

Hi Christoph

ok, I will send it together with a patch for a not yet accepted
software, Integrity Policy Enforcement (IPE), that will be
the primary user of the introduced functionality.

Regarding the patch itself, could you please provide a more
detailed explanation?

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

Re: [RFC][PATCH] device mapper: Add builtin function dm_get_status()

[Christoph Hellwig]

On Thu, Dec 02, 2021 at 07:59:38AM +0000, Roberto Sassu wrote:
> ok, I will send it together with a patch for a not yet accepted
> software, Integrity Policy Enforcement (IPE), that will be
> the primary user of the introduced functionality.
> 
> Regarding the patch itself, could you please provide a more
> detailed explanation?

We don't build things into the kernel just as hooks.  So in doubt you
need to restructured the code.  And that a security module pokes into
a random block driver is a big hint that whatever you're trying to do is
completely broken.

RE: [RFC][PATCH] device mapper: Add builtin function dm_get_status()

[Roberto Sassu]

> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, December 2, 2021 9:44 AM
> On Thu, Dec 02, 2021 at 07:59:38AM +0000, Roberto Sassu wrote:
> > ok, I will send it together with a patch for a not yet accepted
> > software, Integrity Policy Enforcement (IPE), that will be
> > the primary user of the introduced functionality.
> >
> > Regarding the patch itself, could you please provide a more
> > detailed explanation?
> 
> We don't build things into the kernel just as hooks.  So in doubt you
> need to restructured the code.  And that a security module pokes into
> a random block driver is a big hint that whatever you're trying to do is
> completely broken.

I will add more context to the discussion.

The problem being solved is how to grant access to files
which satisfy a property defined in the policy.

For example, a policy enforced by IPE could be:

policy_name="AllowDMVerityKmodules" policy_version=0.0.1
DEFAULT action=ALLOW
DEFAULT op=KMODULE action=DENY
op=KMODULE dmverity_roothash=3c64aae64ae5e8ca781df4d1fbff7c02cb78c4f18a79198263db192cc7f7ba11 action=ALLOW

This would require IPE to obtain somehow this property.

So far, there are two different approaches. The first one,
proposed by the IPE authors was to define a new LSM hook
for block devices, to append a security blob for those devices,
and to store the dm-verity root digest as soon as this information
can be determined. IPE will then access the security blob at
run-time and will match the blob content with the property
value in the policy.

The second one I'm proposing is to directly retrieve the
information at run-time, when files are accessed, and to
possibly cache the result of the evaluation per filesystem.
This would avoid to the introduction of a new LSM hook
and to append a security blob for the purpose of passing
information from the device mapper driver to IPE.

Security blobs are usually used to store LSM-specific
information such as a label (or a reference of it). Sometimes,
when the label must be stored persistently, the subsystem
responsible for this task, such as the VFS, uses subsystem-defined
methods to retrieve the label from the storage and copy it to
the security blob.

In this case, it is not an LSM-specific information but rather
an existing property of another subsystem IPE is interested in.
Since LSMs need anyway to inspect the object before making
the security decision, they could directly retrieve the information
that is already available, instead of making it redundant.

Even if I would prefer the second option, it would be fine for
me if the first is adopted.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

Re: [RFC][PATCH] device mapper: Add builtin function dm_get_status()

[Christoph Hellwig]

On Thu, Dec 02, 2021 at 09:29:52AM +0000, Roberto Sassu wrote:
> The problem being solved is how to grant access to files
> which satisfy a property defined in the policy.

If you have want to enforce access to files in the block layer using
a specific stacking block driver you don't just have one layering
violation but a bunch of them.  Please go back to the drawing board.

RE: [RFC][PATCH] device mapper: Add builtin function dm_get_status()

[Roberto Sassu]

> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Friday, December 3, 2021 7:52 AM
> On Thu, Dec 02, 2021 at 09:29:52AM +0000, Roberto Sassu wrote:
> > The problem being solved is how to grant access to files
> > which satisfy a property defined in the policy.
> 
> If you have want to enforce access to files in the block layer using
> a specific stacking block driver you don't just have one layering
> violation but a bunch of them.  Please go back to the drawing board.

Ok. I write my thoughts here, so that it is easier to align.

dm-verity provides block-level integrity, which means that
the block layer itself is responsible to not pass data to the
upper layer, the filesystem, if a block is found corrupted.

The dm-verity root digest represents the immutable state
of the block device. dm-verity is still responsible to enforce
accesses to the block device according to the root digest
passed at device setup time. Nothing changes, the block
layer still detects data corruption against the passed
reference value.

The task of the security layer is to decide whether or not
the root digest passed at device setup time is acceptable,
e.g. it represents a device containing genuine files coming
from a software vendor.

The mandatory policy can be enforced at different layers,
depending on whether the security controls are placed.
A possibility would be to deny mounting block devices that
don't satisfy the mandatory policy.

However, if the mandatory policy wants only to restrict
execution of approved files and allowing the rest, making
the decision at the block layer is too coarse and restrictive.
It would force the user to mount only approved block
devices. The security layer must operate on files to enforce
this policy.

Now probably there is the part where there is no agreement.

The integrity property of a block device applies also to the
files on the filesystem mounted from that device. User space
programs cannot access files in that filesystem coming from a
device with a different dm-verity root digest, or files stored
in a corrupted block device.

If what I wrote is correct, that the integrity property is preserved
across the layers, this would give enough flexibility to enforce
policies at a higher layer, although that property is guaranteed
by a lower layer.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

RE: [RFC][PATCH] device mapper: Add builtin function dm_get_status()

[Roberto Sassu]

> From: Roberto Sassu [mailto:roberto.sassu@huawei.com]
> Sent: Friday, December 3, 2021 11:20 AM
> > From: Christoph Hellwig [mailto:hch@infradead.org]
> > Sent: Friday, December 3, 2021 7:52 AM
> > On Thu, Dec 02, 2021 at 09:29:52AM +0000, Roberto Sassu wrote:
> > > The problem being solved is how to grant access to files
> > > which satisfy a property defined in the policy.
> >
> > If you have want to enforce access to files in the block layer using
> > a specific stacking block driver you don't just have one layering
> > violation but a bunch of them.  Please go back to the drawing board.
> 
> Ok. I write my thoughts here, so that it is easier to align.
> 
> dm-verity provides block-level integrity, which means that
> the block layer itself is responsible to not pass data to the
> upper layer, the filesystem, if a block is found corrupted.
> 
> The dm-verity root digest represents the immutable state
> of the block device. dm-verity is still responsible to enforce
> accesses to the block device according to the root digest
> passed at device setup time. Nothing changes, the block
> layer still detects data corruption against the passed
> reference value.
> 
> The task of the security layer is to decide whether or not
> the root digest passed at device setup time is acceptable,
> e.g. it represents a device containing genuine files coming
> from a software vendor.
> 
> The mandatory policy can be enforced at different layers,
> depending on whether the security controls are placed.
> A possibility would be to deny mounting block devices that
> don't satisfy the mandatory policy.
> 
> However, if the mandatory policy wants only to restrict
> execution of approved files and allowing the rest, making
> the decision at the block layer is too coarse and restrictive.
> It would force the user to mount only approved block
> devices. The security layer must operate on files to enforce
> this policy.
> 
> Now probably there is the part where there is no agreement.
> 
> The integrity property of a block device applies also to the
> files on the filesystem mounted from that device. User space
> programs cannot access files in that filesystem coming from a
> device with a different dm-verity root digest, or files stored
> in a corrupted block device.
> 
> If what I wrote is correct, that the integrity property is preserved
> across the layers, this would give enough flexibility to enforce
> policies at a higher layer, although that property is guaranteed
> by a lower layer.

Hi Christoph

did I address your concerns? If yes, I could send the new patch
set, including the patch that uses the new functionality.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua