Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide barebones versions of existing domains #328

Closed
DemiMarie opened this issue Dec 20, 2020 · 25 comments
Closed

Provide barebones versions of existing domains #328

DemiMarie opened this issue Dec 20, 2020 · 25 comments
Labels
stale Issue/PR has not had any recent activity.

Comments

@DemiMarie
Copy link

Currently, domain adds more permissions than are actually needed.

It would be nice to have a type (barebones_domain?) that includes exactly the permissions needed to run a program that does nothing, and no more. Similarly, there would be a barebones_daemon with the minimum permissions for a daemon that just immediately exits, and so on.

This would be useful for those who want a strict default-deny policy, since it ensures that access is not accidentally granted.

@pebenito
Copy link
Member

What permissions allowed on domain do you feel are excessive?

There use to be a 'base domain" that didn't have any of the permissions added on domains, but only kernel threads used it, and we didn't feel the distinction provided real value.

@DemiMarie
Copy link
Author

Mostly ioctl and socket permissions, but I honestly want to be able to say, “this program gets exactly what it needs and nothing else.” I want to lock down a process so tightly that if I removed even a single permission, the process would not be able to perform its intended task.

daemon adds even more permissions, not all of which are needed by all daemons. If a daemon needs more than the bare minimum permissions, that should be specified in the daemon’s policy.

It’s also worth noting that I tend to avoid using interfaces, as they often grant unnecessary permissions. I prefer to use audit2allow -bx and examine the AVC denials myself.

@pebenito
Copy link
Member

It does not seem that Refpolicy is a good choice for your needs. What you're stating as excessive are considered reasonable tradeoffs to improve readability and maintainability in refpolicy. Interfaces allowing a process that needs to write a file to also getattr, a typical operation to determine file existence, and to append, a subset of write access, fits the dictionary definition of excessive, but it is not considered excessive in refpolicy. This type of tradeoff is made throughout the policy.

If you have specific criticisms for rules that are applied to domain or daemon, I'll be happy to consider them, but I'm not yet convinced a base domain needs to be brought back.

@ghost
Copy link

ghost commented Dec 23, 2020 via email

@DemiMarie
Copy link
Author

It does not seem that Refpolicy is a good choice for your needs. What you're stating as excessive are considered reasonable tradeoffs to improve readability and maintainability in refpolicy. Interfaces allowing a process that needs to write a file to also getattr, a typical operation to determine file existence, and to append, a subset of write access, fits the dictionary definition of excessive, but it is not considered excessive in refpolicy. This type of tradeoff is made throughout the policy.

Those are fine. The main permission I don’t like is ioctl, as it is a significantly larger attack surface.

@DemiMarie
Copy link
Author

DemiMarie commented Dec 25, 2020

It does not seem that Refpolicy is a good choice for your needs. What you're stating as excessive are considered reasonable tradeoffs to improve readability and maintainability in refpolicy. Interfaces allowing a process that needs to write a file to also getattr, a typical operation to determine file existence, and to append, a subset of write access, fits the dictionary definition of excessive, but it is not considered excessive in refpolicy. This type of tradeoff is made throughout the policy. If you have specific criticisms for rules that are applied to domain or daemon, I'll be happy to consider them, but I'm not yet convinced a base domain needs to be brought back.

The issue I have with domain versus for example other low level attributes like file_type are the neverallow rules associated with domain making domain essentially mandatory for processes. The allow rules referencing domain as a source are relatively few but there are also allow rules that reference domain as a target and one might want to be able to override those. That said, I do agree that its all about compromising. Implementing this with CIL is much easier and therefore more compelling.

There are several other limitations I have noticed:

  • init_t is distinct from unconfined_t, but there is no such distinct type for user systemd instances. Since confined users are allowed to talk to the system instance of systemd (via nsswitch_domain), this means that D-Bus enforcement is all that prevents an escalation.
  • Confined user support often regresses.

@pebenito
Copy link
Member

pebenito commented Jan 5, 2021

The issue I have with domain versus for example other low level attributes like file_type are the neverallow rules associated with domain making domain essentially mandatory for processes.

This is intentional, at a minimum so you don't have a domain transitioning to a non-domain.

  • init_t is distinct from unconfined_t, but there is no such distinct type for user systemd instances.

If you're referring to systemd --user instances, this is inaccurate. There are domains for each confined domain, e.g. staff_systemd_t. For unconfined_t, the systemd --user will run as unconfined_t.

  • Confined user support often regresses.

Can you provide examples? I don't remember anyone reporting regressions.

@pebenito
Copy link
Member

pebenito commented Jan 5, 2021

One further note, the systemd --user functionality is rudimentary, so it isn't likely fully functional, but the domains are there.

@ghost
Copy link

ghost commented Jan 6, 2021 via email

@DemiMarie
Copy link
Author

@doverride exactly. If I want to transition to a type with absolutely no privileges, I should be able to.

@pebenito
Copy link
Member

pebenito commented Jan 6, 2021

These are the ioctls that are provided by domain, ignoring the ioctls on non-device classes:

allow domain devtty_t:chr_file { append getattr ioctl lock open read write };
allow domain null_device_t:chr_file { append getattr ioctl lock open read write };
allow domain urandom_device_t:chr_file { getattr ioctl lock open read }; [ global_ssp ]:True
allow domain zero_device_t:chr_file { append getattr ioctl lock open read write };

@pebenito pebenito closed this as completed Jan 6, 2021
@pebenito pebenito reopened this Jan 6, 2021
@pebenito
Copy link
Member

pebenito commented Jan 6, 2021

Other than maybe revoking the ioctl on urandom, I'm still not convinced for a base domain need.

@ghost
Copy link

ghost commented Jan 6, 2021 via email

@pebenito
Copy link
Member

pebenito commented Jan 6, 2021

And so what if it has that access? Why add more policy complexity to stop a container from accessing /dev/null, for example?

@dburgener
Copy link
Member

I know that this is intentional but in my view that aspect could warrant
a base domain. So that the neverallow rule can be associated with the
base domain but not any other "optional" rules common to domain.

This seems like a fairly sensible argument in my view. However, I'm somewhat concerned about the usability issues where now rules of the form "allow foo domain:process { some_perms };" don't actually apply to all processes on the system.

It seems to me as though there are two potential goals here:

  1. A domain that can only do a bare minimum amount of functionality
  2. A domain that has only specifically opted in reachability from other domains.

I understand the desire to be strictly least privilege, but it seems like in the current architecture, you'd have usability problems around point 2 above. My inclination would be that if there's some sort of expansion, we might want separate attributes for each of those two cases depending on the security goals, but then you're adding even more complexity.

If I'm understanding the original request correctly, it's really more about point 1 I believe. In that scenario, I'd think you'd want all existing access with target domain to still apply to this bare domain. That's something that would be a lot easier to implement in CIL than in existing refpolicy I think.

@ghost
Copy link

ghost commented Jan 6, 2021 via email

@ghost
Copy link

ghost commented Jan 6, 2021

I know that this is intentional but in my view that aspect could warrant
a base domain. So that the neverallow rule can be associated with the
base domain but not any other "optional" rules common to domain.

This seems like a fairly sensible argument in my view. However, I'm somewhat concerned about the usability issues where now rules of the form "allow foo domain:process { some_perms };" don't actually apply to all processes on the system.

True, but that is also the point. It is the ability to by-pass and the bare domain should only be used where you really want the process to be treated unlike common processes (that is also why i don't use it much but i still like having the option)

It seems to me as though there are two potential goals here:

1. A domain that can only do a bare minimum amount of functionality

In my policy I have "subj" and "common subj" where "subj" is used to just associate the neverallow so that the policy will not build there is a transition rule associated with a type that is not either subj or common subj.

common subj is a superset of subj and in my policy i associate various rules with that type that apply to common processes. So i use it a bit more extensively. For example using generic libraries is something i allow on this low level for efficiency.

So i admit that my use case isnt completely comparable to refpolicy.

2. A domain that has only specifically opted in reachability from other domains.

I understand the desire to be strictly least privilege, but it seems like in the current architecture, you'd have usability problems around point 2 above. My inclination would be that if there's some sort of expansion, we might want separate attributes for each of those two cases depending on the security goals, but then you're adding even more complexity.

If I'm understanding the original request correctly, it's really more about point 1 I believe. In that scenario, I'd think you'd want all existing access with target domain to still apply to this bare domain. That's something that would be a lot easier to implement in CIL than in existing refpolicy I think.

I think so to and i do understand the concerns. I just think that one might be able to squeeze more out of the low level domain concept. but if not then its not a big deal since domain is currently almost nothing. There are probably more rules referencing domain as a target than there are rules referencing domain as a source

@DemiMarie
Copy link
Author

These are the ioctls that are provided by domain, ignoring the ioctls on non-device classes:

allow domain devtty_t:chr_file { append getattr ioctl lock open read write };
allow domain null_device_t:chr_file { append getattr ioctl lock open read write };
allow domain urandom_device_t:chr_file { getattr ioctl lock open read }; [ global_ssp ]:True
allow domain zero_device_t:chr_file { append getattr ioctl lock open read write };

Actually, it is the ioctls on regular files and sockets that I am more worried about. Those have a much larger attack surface, and many of them are quite obscure so I suspect they do not see much testing. In particular, there have been quite a few cases where Android was saved because SELinux blocked an obscure and vulnerable socket ioctl.

Personally, I would like to have these classes:

  • base_domain: This contains the bare minimum needed for a do-nothing, statically-lined assembler program that just calls exit_group. It also is the target of rules such as allow init_t base_domain:process signal;, where every potential process must be a potential target. The main use I can think of for base_domain is containers, which do not necessarily need access to the host’s libc. A process with a non-base_domain label is itself a vulnerability, since init won’t be able to kill it.
  • libc_domain: This includes the bare minimum for a C program dynamically linked to libc. It should have only what is needed by /bin/true.
  • domain: This provides access to resources that a program can reasonably expect to have. For instance, it includes read and mmap (but not ioctl) access to system libraries, as well as access to the system cryptographic pseudorandom number generator. It is meant as a good starting point for one’s own policies.

@pebenito
Copy link
Member

pebenito commented Jan 7, 2021

Actually, it is the ioctls on regular files and sockets that I am more worried about. Those have a much larger attack surface, and many of them are quite obscure so I suspect they do not see much testing.

domain by itself does not allow any ioctls on any sockets. If you can provide references where ioctls on regular files and dirs was a vulnerability, I'd appreciate that, as that warrants a review of the ioctls on the entire policy.

@github-actions
Copy link

This issue has not had any recent activity. It will be closed in 7 days if it makes no further progress.

@github-actions github-actions bot added the stale Issue/PR has not had any recent activity. label Apr 21, 2021
@DemiMarie
Copy link
Author

Actually, it is the ioctls on regular files and sockets that I am more worried about. Those have a much larger attack surface, and many of them are quite obscure so I suspect they do not see much testing.

domain by itself does not allow any ioctls on any sockets. If you can provide references where ioctls on regular files and dirs was a vulnerability, I'd appreciate that, as that warrants a review of the ioctls on the entire policy.

IIRC there was a (now fixed) vulnerability in an f2fs ioctl found by syzbot a while back. I suggest starting with ioctls that are filesystem specific, as I expect most applications have no need to know what filesystem they are running on.

@github-actions github-actions bot removed the stale Issue/PR has not had any recent activity. label Apr 22, 2021
@pebenito
Copy link
Member

pebenito commented May 4, 2021

Ok, I'm only marginally convinced on this still. However, since this is a simple change I'll do it anyway. I will be equally skeptical for patches/PRs making anything in refpolicy a base domain.

@pebenito
Copy link
Member

pebenito commented May 5, 2021

I forgot that domain_base_type() still existed, so I am accepting of this change.

@DemiMarie please try the above PR and see if it meets your needs.

@github-actions
Copy link

github-actions bot commented Jul 5, 2021

This issue has not had any recent activity. It will be closed in 7 days if it makes no further progress.

@github-actions github-actions bot added the stale Issue/PR has not had any recent activity. label Jul 5, 2021
@github-actions
Copy link

Closing stale PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issue/PR has not had any recent activity.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants