RFC: Add check for wide directory path file contexts

[cgzones]

Using a file context /var/lib/myapp(/.*)? gen_context(... is common, but it matches non-directory entries /var/lib/myapp.

One might want to specify two file contexts:

    /var/lib/myapp     -d gen_context(...
    /var/lib/myapp/.*     gen_context(...

Due do this type of file contexts being a common practice, it is debatable if such check should be added, and if so if it should be enabled by default.

[dburgener]

Do you want this in for 1.2? My thought is to get #189 rebased and merged and then cut 1.2, and then look at this for after. Is that reasonable?

[cgzones]

Do you want this in for 1.2?

No, Dominick mentioned this file context usage on IRC a couple times so I gave it a try.

[dburgener]

Sounds good. I'll wait to take a look at this until 1.2 is out the door then. Hoping for any day now. Just need to find the time.

[stevedlawrence]

I'm not sure I see the advantage to suggesting that we split this very common filecontext statement in two. What's the problem if it matches non-directories? If a file context says the directory /var/lib/myapp and everything below it should have a certain label, I imagine most users would also expect the file /var/lib/myapp to also have that same label. The regex is maybe excessive, but not in a way that changes the security properties of the system. If anything, the excessive regex is better--if /var/lib/myapp was accidentally a file it would still be labeled as one expects rather than getting a default like var_lib_t that you would get by splitting the filecontext out, which could affect the security of the system.

If feels like what is maybe needed instead is detecting the file/dir types on the actual system and warning if they don't match a filecontext entry. For example:

/var/lib/myapp is a file, but a filecontext regex ends with (/.*)? implying it is a directory.

or

/var/lib/myapp is a directory, but the file context only matches a file

But that requires knowledge about the system, which selint might not normally have since I assume in most cases it's not running on the target machine.

[dburgener]

I haven't read the code yet, but I'm inclined to agree with Steve on the idea of the check. The (/.*)? pattern is very common, and I don't know that I see a lot wrong with it.

That said, I support the general idea of including checks that some people want even if they aren't generally applicable, particularly under the category of "conventions". The whole idea of a "convention" is that there's no right or wrong on this topic, but there's value in having a common standard in a particular policy. This seems to me like it pretty much fits that definition.

I think if we do take this, we should set it to off in the default config since it's probably recommended for most use cases. Even better would be if this could be a configuration option on something more generally helpful, although I"m not sure what that would be.

@cgzones is this something that you would want to enable in your use case? I assume Dominick is not using SELint, since he focuses on CIL policy, but did it sound like this is a convention he would support using when he mentioned it on IRC?

[cgzones]

Closing as (/.*)? is a common pattern and one can argue whether complying to this check improves or degrades the policy.