On Sat, Nov 5, 2022 at 1:09 PM Christian Göttsche
<cgzones@xxxxxxxxxxxxxx> wrote:
>
> SELinux policies contain a lot of different identifiers, like users,
> roles, types, attributes, booleans, level aliases, classes and
> permissions (non-exhaustive list). In the frontend compilers, like
> checkpolicy(8) and secilc(8), those are quite limited with regard to
> the supported character set, mainly `[A-Za-z][A-Za-z0-9-_]*`. For the
> vast majority of policies, and in particular the Reference and Fedora
> one, this range of possible identifiers should be sufficient. I'd
> like to discuss two shortcomings I currently see:
>
> I. Length of identifiers
>
> There seems to be no identifier length limit in checkpolicy(8) and a
> quite high one of 2048 in secilc(8). On the other hand netfilter
> secmark supports only (whole) security contexts with a length of 256
> (NFT_SECMARK_CTX_MAXLEN), and all selinuxfs operations are limited
> around the PAGE_SIZE (4096).
>
The limit in checkpolicy is YYLMAX which is 8192.
There definitely can be no problem with setting the limit at 4096.
I think it would be reasonable to set the identifier limit to 2048 and
the max context length to 4096. That shouldn't cause any problems and
it would still limit what the fuzzer needs to check. It would probably
be reasonable to set the identifier limit even lower.
It makes me wonder how long the character string representing the
categories in a context can get with 1024 categories.
> II. Binary Policies
>
> Parsing identifiers from a binary policy, either in userspace via
> libsepol or in the kernel (see str_read()), does neither impose any
> constraints related to character set or length. Binary policies
> should generally be trusted, especialy when loading them into the
> kernel, but this might affect future additions of SELinux namespaces
> or debugging foreign policies in userspace.
>
>
> More strict constraints of identifiers would ensure crafted
> identifiers (unicode, control characters, etc.) would be invalid and
> need no special treating in userspace or the kernel. Also the fuzzing
> libsepol would benefit, as the fuzzer is currently limited to an
> identifier limit of 65536 to avoid OOM failures.
>
> WDYT?
>From checkpolicy an identifier must be:
{letter}({alnum}|[_\-])*([\.]?({alnum}|[_\-]))* { return(IDENTIFIER); }
In seilc:
symbols can be almost anything.
spec_char [\[\]\.\@\=\/\*\-\_\$\%\+\-\!\|\&\^\:\~\`\#\{\}\'\<\>\?\,]
symbol ({digit}|{alpha}|{spec_char})+
But identifiers are checked more closely
In cil_verify_name(), there is a check that the first character is a
letter, and the rest of the characters are alphanumeric, "_", "-", or
".".
I am definitely ok with stricter checking when reading in a binary policy.
Jim
