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.

Sign up for GitHub

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

Jump to bottom

Non-root ownership of state dirs #6

Closed
leenaars opened this issue Jan 31, 2018 · 5 comments
Closed

Non-root ownership of state dirs #6

leenaars opened this issue Jan 31, 2018 · 5 comments

Comments

@leenaars
Copy link
Contributor

leenaars commented Jan 31, 2018 •
edited
Loading

By default, the state directories are owned by root.
This seems wrong, these should be owned by the owner of the process that is supposed to write in those directories.

**Proposed solution: **
At creation of state directory, assign rights to the right owner. Use setgid.
@qknight
Copy link
Member

qknight commented Jan 31, 2018

your assumption is right. looking into this.

@qknight
Copy link
Member

qknight commented Feb 2, 2018

@leenaars can't reproduce this with the leaps service. how can i reproduce your problem? are you using something else than the leaps service like the /root/nixcloud.monitoring-server/pkgs/nixcloud-webservices/lib/make-webservice.nix mechanism?

@aszlig
Copy link
Collaborator

aszlig commented Feb 5, 2018

I'd suggest a more generic solution where within instances you have a createDirectories option that is responsible for creating all sorts of directories in one place, especially because creating directories is something that's quite common for a lot of services.

@qknight
Copy link
Member

qknight commented Feb 6, 2018 •
edited
Loading

@leenaars 2aszlig and me are currently looking into alternatives to the current concept of using the nixos abstraction to create the homedir. we consider two options:

  • createDirectories: which uses chmod g+s /some/dir
  • createDirectories: which uses POSIX ACL commands

@qknight
Copy link
Member

qknight commented Feb 9, 2018

@aszlig irgend ein zeitlicher plan hier?

aszlig added a commit that referenced this issue Feb 12, 2018
@aszlig
Add a module for creating dirs and setting perms
af62fb6 
So the first step is doing this at the top-level, so that we can pass
through a subset of the per-instance options (which will be introduced
later) to our mechanism.

The mechanism in itself is based on a directories option which consists
of an attribute set of paths and their corresponding owner/group and
additional permissions to set during creation and also during fixup.

While the fixup phase is run whenever the directory already exists the
creation phase is run whenever the directory does not exist.

Right now the implementation is pretty much work in progress, but I'm
already committing this straight to master because whenever the
directories option is not used the module doesn't do anything.

Also, we have several limitations left to solve before this is
production-ready:

 * The creation and fixup units need to have a conflict on each other so
   that we don't run into a race condition between those two.
 * Currently the permissions.recursive option doesn't do anything.
 * More tests need to be implement to make sure we properly propagate
   default ACLs.
 * We need to fix up the generated systemd services so that they get
   properly ordered and don't start in parallel with the services that
   require them.
 * Make sure we don't open up ways for privilege escalation. For example
   if the fixup service first sets the permissions and the owner/group
   later, this could lead to elevated permissions of the previous
   owner/group and/or the allowed users/groups in the ACL.

In order to implement the permissions.recursive option, we need to take
care of nested directories, so let's say we have a directory /a/b and
/a/b/c, we need to stop recursing during fixup of /a/b once we hit
/a/b/c so that we don't override the permissions from that subdirectory.

Signed-off-by: aszlig <aszlig@nix.build>
Issue: #6
aszlig added a commit that referenced this issue Feb 14, 2018
@aszlig
directories: Implement recursive fixup of perms
05dcf20 
This is one of the main features of this module and we really need to
take care to not overwrite permissions of subdirectories that are also
defined via the same module option.

In order to avoid this, we filter all the subdirectories from the
directory definitions and exclude them using "find" arguments (-path and
-prune).

The rest of the find arguments is to ensure that directories and files
are treated differently, because files usually shouldn't be executable.

I've also added a few tests to ensure that recursive fixup of
permissions is working correctly and whether changing of the owner/group
works as well.

Signed-off-by: aszlig <aszlig@nix.build>
Issue: #6
@aszlig aszlig closed this as completed in e578cb5 Feb 16, 2018
aszlig added a commit that referenced this issue Feb 16, 2018
@aszlig
modules/web: Convert createHome to directories
54e457d 
This of course only applies if the user's home directory is within
stateDir, but whenever this is the case a directory definition is
created for it and createHome is passed as false to the top-level
users.users option.

Passing the users home directory to the directories option also has the
side effect that it overrides existing directory options, so in order to
prevent this, we pass those values with a priority of 200 so that other
defined options by the user will have precedence.

I've also added a test which ensures the ownership of the stateDir in
the leaps web service.

Signed-off-by: aszlig <aszlig@nix.build>
Issue: #6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@qknight @aszlig @leenaars