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

Socket file location is not according to FHS #250

Closed
puiterwijk opened this issue Sep 17, 2020 · 18 comments
Closed

Socket file location is not according to FHS #250

puiterwijk opened this issue Sep 17, 2020 · 18 comments
Labels
question Further information is requested
Projects

Comments

@puiterwijk
Copy link
Contributor

puiterwijk commented Sep 17, 2020

static SOCKET_PATH: &str = "/tmp/parsec/parsec.sock";
specifies the socket to be in /tmp/parsec/parsec.sock.
However, according to the Filesystem Hierarchy Standard v2.3, the correct location for pid files and socket files is /var/run/ (System programs that maintain transient UNIX-domain sockets must place them in this directory.), with possibly an application-level folder if it has multiple files.

Could this perhaps get fixed, and maybe made configurable?

@hug-dev
Copy link
Member

hug-dev commented Sep 17, 2020

Hello!

The first reason we choosed /tmp/parsec is for the service, which is run as an user daemon, to be able to create the socket there without administrator privileges.
However I looked at my /run/ folder (which seemed to have replaced /var/run in FHS 3.0) and saw that some applications had folder there which was owned by the application user.
So we could do something similar and have the administrator create /run/parsec with correct rights, according to our requirements:

sudo mkdir /run/parsec
sudo groupadd parsec-clients
sudo chown :parsec-clients /run/parsec
sudo chmod 750 /run/parsec

I believe that would work? That would need changes in parsec, parsec-client-rs and our docs.

The path to the socket is not configurable and is hardcoded so that clients can always expect the socket to be at that location. That may change in the future.

I have some questions for you 😃 :

  1. (probably noobie question) What are the advantages of following this standard as opposed as putting the socket in /tmp? As in, what would we gain from it?
  2. We store other kind of persistent data and configuration in the home folder of the parsec user. Is that still fine you think or we should also aim to storing it inside one of those more standard folders?

@hug-dev hug-dev added the question Further information is requested label Sep 17, 2020
@hug-dev hug-dev added this to All issues in Parsec via automation Sep 17, 2020
@ionut-arm
Copy link
Member

Hi,

I'll chip in a question as well:
3. If we do make the path configurable, how would you suggest we advertise the path to clients? Through environment variables?

@anta5010
Copy link
Collaborator

I raised similar issues when I worked on the parsec Yocto layer. But, I cannot find how this discussion ended. (BTW in the yocto layer parsec service init script I used /etc/parsec directory for config files). Using /tmp directory is just strange from Linux administration point of view. Using standard directories would simply help sysadmins to quicker find details and troubleshoot issues if necessary.
I think to align parse with standard linux services we should:

  1. Use /run/parsec directory for the socket file
  2. Use /etc/parsec directory for config files in parsec service init scripts. We can still use current directory for manual tests and CI testing of course
  3. To simplify clients configuration the socket path and name should be hard-coded and properly documented. Documenting /run/parsec path would not cause raised eyebrows as /tmp/parsec would do.

@puiterwijk
Copy link
Contributor Author

puiterwijk commented Sep 18, 2020

Right, sorry for linking to the FHS 2.3, that's just the one the Fedora packaging documentation points to and that's always the way I find it 😄.

  1. Yep, /run/parsec would probably be best, and just defaulting that everywhere. One of the main reasons you don't want to use /tmp is because that is getting more and more separated these days.
  2. Config should be in /etc/parsec (given that you have just one file, /etc/parsec.conf seems reasonable to me?). Persistent data should go into /var/lib/parsec. But based on what I saw, I think it's using the current working dir, and not the homedir? For example ./mappings is stored in the current working dir. If you use cwd for storage, that should be fine, given that the systemd unit file could be made to just set the WorkingDir somewhere else.
  3. I do think it'd be nice to make it possible to override, probably with an environment variable PARSEC_SOCKET in case you want to run multiple for tests or other reasons (e.g. I might want to run one parsec instance against a smartcard for specific usecases, and on the same system have a parsec with TPM for different things).

@nullr0ute
Copy link

In dealing with it in Fedora I put the binary in /usr/libexec, set the home to /var/lib/parsec, the config to /etc/parsec

@hug-dev
Copy link
Member

hug-dev commented Sep 18, 2020

Thanks for the explanation of why /tmp why not be a good future-proof idea!

I would agree putting the socket in /run/parsec and the configuration in /etc/parsec. Might be better to have a directory there just in case, in the future, we have more than one configuration file.

/var/lib/parsec sounds also good for any persistent data that Parsec modifies. As far as I know those are the mappings and the keys stored by the Mbed Crypto provider. At the moment those are expected to be at the current working directory and making WorkingDir point to that seems to be a good solution 💯

Using /usr/libexec seems also like the standard location for the binary. That would actually remove the need of the parsec user having a home folder then.

If we go ahead with this, this would need change in Parsec repo and in the documentation (updating the secure deployment guide mainly). As @anta5010 we might need ways to have config options for local testing/CI as well.

Regarding the PARSEC_SOCKET env var for configuration I am not so sure, I think that one strong goal of Parsec is that there would only be one on your system abstracting for all crypto providers that you might have.

@nullr0ute
Copy link

That all makes sense. I think we need to make a decision now and change it before things are widely deployed.

@joechrisellis
Copy link
Contributor

These suggestions look good to me. 😄

Another point against using /tmp is that some distributions/systems periodically wipe the directory, which could result in the socket file being deleted. This has happened to me before with tmux; my sessions can be (temporarily) lost because the socket file inside the /tmp directory can be unexpectedly wiped. I'm unsure of the precise mechanisms around clearing /tmp but it would be nice to avoid this problem altogether with Parsec. 🙂

I think using /etc/parsec as a config directory, and /etc/parsec/parsec.conf as the config file is probably a more future-proof approach, unless we are really sure that we want to have a single configuration file for all of Parsec for the long-term future.

Just my $0.02! 😄

@paulhowardarm
Copy link
Collaborator

Looks like we should switch to /run/parsec as the default location for the socket. But it is inevitable at some point that this will need to be configurable on the client side. Having a hard-coded default is fine, but having it be the only permitted value will not work in a future where a single Parsec service might have multiple endpoints and/or endpoints that are based on transports other than UDS. I have previously advocated for environment variables for this purpose, see: parallaxsecond/parsec-client-rust#37

In the interests of keeping all issues and PR's "small", I would suggest just changing the default for now. We'll need to announce the change and check that it won't cause issues for people, assuming that we're just going to do it as a "lockstep" change (ie. just change the code in the service and the client to use a different path, and not try to cater for situations where the service and the client are updated at different times such that only one is aware of the change).

@anta5010
Copy link
Collaborator

I would agree putting the socket in /run/parsec and the configuration in /etc/parsec. Might be better to have a directory there just in case, in the future, we have more than one configuration file.

/var/lib/parsec sounds also good for any persistent data that Parsec modifies. As far as I know those are the mappings and the keys stored by the Mbed Crypto provider. At the moment those are expected to be at the current working directory and making WorkingDir point to that seems to be a good solution 💯

Keeping persistent data in /var/lib/parsec is reasonable. But, data used only while running (in addition to the socket file) would be better kept in /var/run/parsec instead of the current working directory I think.

@hug-dev
Copy link
Member

hug-dev commented Sep 22, 2020

data used only while running (in addition to the socket file) would be better kept in /var/run/parsec instead of the current working directory I think.

I believe that, except for the socket file, all the data that we use while running in Parsec needs to be persisted.

Sometimes though data is a little bit of both: for example you can create a key and destroy it before Parsec is shut down, in that case would it be better to save information for that key in /var/lib/parsec or /run/parsec?
We could set the current working directory to be /run/parsec and only transfer the mappings and other files that need to be persisted, just before shutdown, to /var/lib/parsec but that would need deeper changes in the code.

If we decide to go with the changes described in my previous comment, the changes are not that big, it is mostly about:

  • changing the socket location (cfg with a feature) to /run/parsec/parsec.sock and similar in the client
  • modifying the installation instructions in the book

(Also note that with #200 the Unix Peer Credentials authenticator is now in and similar changes to related to its deployment needs to be addressed via #249. If we wanted, we could address all those issues together, or, depending on the urgency of this, just address this present one.)

@anta5010
Copy link
Collaborator

I think data like created keys should be kept in /var/lib/parsec even if keys could be deleted before parsec is stopped.
Why don't we:

  1. Change the current working directory to /var/lib/parsec and create mappings/keys/etc there
  2. Store the socket file in /run/parsec
    I think this approach would simplify the changes required.

@paulhowardarm
Copy link
Collaborator

I would campaign for addressing just this specific issue for now. I would also campaign for a very precise specification for the behavioural change, so that the impact can be clearly understood. For instance:

  • When the service restarts, it will start listening on the new socket only. Any client processes that continue to use the previous socket will fail to connect.
  • Any existing clients that were built to statically link the C or Rust libraries would have to be rebuilt and updated in order to get the change.
  • If any clients are updated without the corresponding service update/restart, they will likewise fail to connect.
  • The service will not automatically delete the old socket after the update. That will have to be done manually.
  • All previous recommendations for secure deployment continue to hold for the new socket path. It is only the path that is changing.

Is that a correct and complete statement?

@hug-dev
Copy link
Member

hug-dev commented Sep 23, 2020

I believe this is all correct.

I think this point:

The service will not automatically delete the old socket after the update. That will have to be done manually.

can be expanded into:

  • The service will need to be stopped and manually uninstalled. New recommendations for secure deployment will have to be followed to install the service after the update.

We can provide instructions on how to uninstall in the same book page where the new instructions are written.

@hug-dev
Copy link
Member

hug-dev commented Sep 25, 2020

I have done most of the changes with the PR linked above. This is passing in CI and I tested the secure installation steps on my Fedora 32 machine.

To all interested parties: I would appreciate your review on the following specific changes:

  • the systemd unit file changes here
  • the configuration changes here and the new defaults
  • the new installation instructions rendered here

For the mappings folder, I have kept the current working directory in /home/parsec but set the new default location for the mappings to /var/lib/parsec/mappings.

The installation instructions are very verbose and would definitely gain some scripting. However, I would prefer to do that as a later step as I don't think it will be as easy as it looks to produce a good quality installation script.

@puiterwijk
Copy link
Contributor Author

👍 On these changes, thanks!

@nullr0ute
Copy link

LGTM, thanks.

@hug-dev
Copy link
Member

hug-dev commented Oct 2, 2020

Great, thanks!

Just published new versions: parsec-service 0.5.0 and parsec-client 0.10.0. Either of those two projects will not be compatible with lower versions of the other one 🚀

@hug-dev hug-dev closed this as completed Oct 2, 2020
Parsec automation moved this from In progress to Done Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
Development

No branches or pull requests

7 participants