Re: [RFC PATCH v2 22/22] selinux: restrict policy strings

[Daniel Burgener <dburgener@linux.microsoft.com>]

On 1/7/2025 9:04 AM, Christian Göttsche wrote:
> On Mon, 6 Jan 2025 at 00:26, Joe Nall <joenall@gmail.com> wrote:
>>
>>
>>
>>> On Jan 3, 2025, at 2:12 PM, Stephen Smalley <stephen.smalley.work@gmail.com> wrote:
>>>
>>> On Mon, Dec 16, 2024 at 11:42 AM Christian Göttsche
>>> <cgoettsche@seltendoof.de> wrote:
>>>>
>>>> From: Christian Göttsche <cgzones@googlemail.com>
>>>>
>>>> Validate the characters and the lengths of strings parsed from binary
>>>> policies.
>> Excellent idea.
>>
>>>>   * Disallow control characters
>> Fine here.
>>
>>>>   * Limit characters of identifiers to alphanumeric, underscore, dash,
>>>>     and dot
>> Fine again.
>>
>>>>   * Limit identifiers in length to 128,
>> Fine again, our longest
>>   - type is 51 characters
>>   - attribute is 31
>>   - boolean is 46
>>   - role is 12
>>
>>>> expect types to 1024 and
>> I don’t understand what this means.
> 
> Similar to your list of the length in you policy boolean, role, user,
> class, and permission identifiers are limited to 128 charatcers (not
> including NUL), types (and attributes, which are just special types)
> are limited to 1024 characters, and individual sensitivities and
> categories are limited to 32 characters.
> 
>>
>>>>     categories to 32, characters (excluding NUL-terminator)
>> One category or the whole category string? A single category longer than 7 characters seems pretty unlikely and 32 is wildly short for the whole string.
> 
> This only affects individual sensitivities and categories, like "s9"
> or "c1023", not whole MCS/MLS parts.
> 
>> Joe
>>
>>> One option if we are concerned about breaking backward compatibility
>>> with policies in the wild would be to make these restrictions
>>> conditional on whether the policy is being loaded into a non-init
>>> SELinux namespace, similar to:
>>> https://lore.kernel.org/selinux/20250102164509.25606-38-stephen.smalley.work@gmail.com/T/#u
>>>
>>> That said, it seems hard to imagine real-world policies that would
>>> exceed these limits, and likely could make them even smaller.
>>> But as Daniel said, we should make them consistently enforced in both
>>> userspace and kernel, and potentially these should all be #define'd
>>> symbols in a uapi header that can be referenced by both.
>>> Looks like you left the type limit at 1024 despite Daniel's
>>> observation that CIL uses 2048 as the limit, but as you noted, given
>>> the page size limit on the entire context by various kernel
>>> interfaces,
>>> this is likely fine.
> 
> I interpreted Daniels comment more like a assessment what CIL
> currently constrains, not as a request for a change, maybe I
> misunderstood?

That is what I intended, yes.  My related request was "I would think 
that we'd want to end up in a situation where the kernel is either 
equally restrictive or less restrictive than CIL".  In isolation, my 
opinion is that the 1024 limit is fine, but I've been hoping James would 
chime in about the feasibility of dropping the CIL limit at some point 
to get them in sync.

FWIW we have a few generated type names internally that subjectively 
feel long to humans, but are still under 100 characters.  So 1024 is 
plenty of extra margin in my opinion.

-Daniel

> 
> Exporting the limits via a public headers seems reasonable.
> 
> Maybe for a smooth transition one could introduce a build time
> configuration (CONFIG_SELINUX_STRICT_IDENTIFIERS?).
> This configuration can be disabled by default, leading to identifiers
> not being rejected only logged.
> Than after two releases the default can change to reject instead of log.
> And after the next LTS release the configuration can be dropped again.
> 
>>>
>>