From 637d57ddfd2e908f9508ac9a822d09063f0c146e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 26 Apr 2023 21:45:35 +0200 Subject: [PATCH] image-policy: split out code that "extends" underspecified partition policy flags When encoding partition policy flags we allow parts of the flags to be "unspecified" (i.e. entirely zeros), which when actually checking the policy we'll automatically consider equivalent to "any" (i.e. entirely ones). This "extension" of the flags was so far done as part of partition_policy_normalized_flags(). Let's split this logic out into a new function partition_policy_flags_extend() that simply sets all bits in a specific part of the flags field if they were entirely zeroes so far. When comparing policy objects for equivalence we so far used partition_policy_normalized_flags() to compare the per-designator flags, which thus meant that "underspecified" flags, and fully specified ones that are set to "any" were considered equivalent. Which is great. However, we forgot to do that for the fallback policy flags, the flags that apply to all partitions for which no explicit policy flags are specified. Let's use the new partition_policy_flags_extend() call to compare them in extended form, so that there two we can hide the difference between "underspecified" and "any" flags. --- src/shared/image-policy.c | 40 +++++++++++++++++++++++------------- src/shared/image-policy.h | 2 ++ src/test/test-image-policy.c | 9 ++++++++ 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/shared/image-policy.c b/src/shared/image-policy.c index 2d7538ee115..a831d22a04c 100644 --- a/src/shared/image-policy.c +++ b/src/shared/image-policy.c @@ -33,6 +33,23 @@ static PartitionPolicy* image_policy_bsearch(const ImagePolicy *policy, Partitio partition_policy_compare); } +PartitionPolicyFlags partition_policy_flags_extend(PartitionPolicyFlags flags) { + /* If some parts of a flags field are left unspecified, let's fill in all options. */ + + /* If no protection flag is set, then this means all are set */ + if ((flags & _PARTITION_POLICY_USE_MASK) == 0) + flags |= PARTITION_POLICY_OPEN; + + /* If the gpt flags bits are not specified, set both options for each */ + if ((flags & _PARTITION_POLICY_READ_ONLY_MASK) == 0) + flags |= PARTITION_POLICY_READ_ONLY_ON|PARTITION_POLICY_READ_ONLY_OFF; + + if ((flags & _PARTITION_POLICY_GROWFS_MASK) == 0) + flags |= PARTITION_POLICY_GROWFS_ON|PARTITION_POLICY_GROWFS_OFF; + + return flags; +} + static PartitionPolicyFlags partition_policy_normalized_flags(const PartitionPolicy *policy) { PartitionPolicyFlags flags = ASSERT_PTR(policy)->flags; @@ -40,9 +57,7 @@ static PartitionPolicyFlags partition_policy_normalized_flags(const PartitionPol * unspecified, we'll fill in the appropriate "dontcare" policy instead. We'll also mask out bits * that do not make any sense for specific partition types. */ - /* If no protection flag is set, then this means all are set */ - if ((flags & _PARTITION_POLICY_USE_MASK) == 0) - flags |= PARTITION_POLICY_OPEN; + flags = partition_policy_flags_extend(flags); /* If this is a verity or verity signature designator, then mask off all protection bits, this after * all needs no protection, because it *is* the protection */ @@ -54,16 +69,9 @@ static PartitionPolicyFlags partition_policy_normalized_flags(const PartitionPol if (partition_verity_of(policy->designator) < 0) flags &= ~(PARTITION_POLICY_VERITY|PARTITION_POLICY_SIGNED); + /* If the partition must be absent, then the gpt flags don't matter */ if ((flags & _PARTITION_POLICY_USE_MASK) == PARTITION_POLICY_ABSENT) - /* If the partition must be absent, then the gpt flags don't matter */ flags &= ~(_PARTITION_POLICY_READ_ONLY_MASK|_PARTITION_POLICY_GROWFS_MASK); - else { - /* If the gpt flags bits are not specified, set both options for each */ - if ((flags & _PARTITION_POLICY_READ_ONLY_MASK) == 0) - flags |= PARTITION_POLICY_READ_ONLY_ON|PARTITION_POLICY_READ_ONLY_OFF; - if ((flags & _PARTITION_POLICY_GROWFS_MASK) == 0) - flags |= PARTITION_POLICY_GROWFS_ON|PARTITION_POLICY_GROWFS_OFF; - } return flags; } @@ -427,12 +435,16 @@ int partition_policy_flags_to_string(PartitionPolicyFlags flags, bool simplify, return 0; } +static bool partition_policy_flags_extended_equal(PartitionPolicyFlags a, PartitionPolicyFlags b) { + return partition_policy_flags_extend(a) == partition_policy_flags_extend(b); +} + static int image_policy_flags_all_match(const ImagePolicy *policy, PartitionPolicyFlags expected) { if (expected < 0) return -EINVAL; - if (image_policy_default(policy) != expected) + if (!partition_policy_flags_extended_equal(image_policy_default(policy), expected)) return false; for (PartitionDesignator d = 0; d < _PARTITION_DESIGNATOR_MAX; d++) { @@ -532,7 +544,7 @@ int image_policy_to_string(const ImagePolicy *policy, bool simplify, char **ret) return -ENOMEM; } - if (!simplify || image_policy_default(policy) != PARTITION_POLICY_IGNORE) { + if (!simplify || !partition_policy_flags_extended_equal(image_policy_default(policy), PARTITION_POLICY_IGNORE)) { _cleanup_free_ char *df = NULL; r = partition_policy_flags_to_string(image_policy_default(policy), simplify, &df); @@ -580,7 +592,7 @@ int image_policy_equivalent(const ImagePolicy *a, const ImagePolicy *b) { * redundant, and will be recognized as such by image_policy_equivalent() but not by * image_policy_equal()- */ - if (image_policy_default(a) != image_policy_default(b)) + if (!partition_policy_flags_extended_equal(image_policy_default(a), image_policy_default(b))) return false; for (PartitionDesignator d = 0; d < _PARTITION_DESIGNATOR_MAX; d++) { diff --git a/src/shared/image-policy.h b/src/shared/image-policy.h index 1b3d068c721..675b061f548 100644 --- a/src/shared/image-policy.h +++ b/src/shared/image-policy.h @@ -78,6 +78,8 @@ static inline size_t image_policy_n_entries(const ImagePolicy *policy) { return policy ? policy->n_policies : 0; } +PartitionPolicyFlags partition_policy_flags_extend(PartitionPolicyFlags flags); + PartitionPolicyFlags partition_policy_flags_from_string(const char *s); int partition_policy_flags_to_string(PartitionPolicyFlags flags, bool simplify, char **ret); diff --git a/src/test/test-image-policy.c b/src/test/test-image-policy.c index 41941704d42..f2eba949614 100644 --- a/src/test/test-image-policy.c +++ b/src/test/test-image-policy.c @@ -119,4 +119,13 @@ TEST_RET(test_image_policy_to_string) { return 0; } +TEST(extend) { + assert_se(partition_policy_flags_extend(0) == _PARTITION_POLICY_MASK); + assert_se(partition_policy_flags_extend(_PARTITION_POLICY_MASK) == _PARTITION_POLICY_MASK); + assert_se(partition_policy_flags_extend(PARTITION_POLICY_UNPROTECTED) == (PARTITION_POLICY_UNPROTECTED|_PARTITION_POLICY_PFLAGS_MASK)); + assert_se(partition_policy_flags_extend(PARTITION_POLICY_UNPROTECTED|PARTITION_POLICY_READ_ONLY_ON) == (PARTITION_POLICY_UNPROTECTED|PARTITION_POLICY_READ_ONLY_ON|_PARTITION_POLICY_GROWFS_MASK)); + assert_se(partition_policy_flags_extend(PARTITION_POLICY_UNPROTECTED|PARTITION_POLICY_READ_ONLY_ON|PARTITION_POLICY_GROWFS_OFF) == (PARTITION_POLICY_UNPROTECTED|PARTITION_POLICY_READ_ONLY_ON|PARTITION_POLICY_GROWFS_OFF)); + assert_se(partition_policy_flags_extend(PARTITION_POLICY_GROWFS_ON) == (PARTITION_POLICY_GROWFS_ON|_PARTITION_POLICY_USE_MASK|_PARTITION_POLICY_READ_ONLY_MASK)); +} + DEFINE_TEST_MAIN(LOG_INFO);