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 path security checks can fail when the client is in a container #51

Closed
paulhowardarm opened this issue Oct 4, 2020 · 4 comments · Fixed by #56
Closed

Socket path security checks can fail when the client is in a container #51

paulhowardarm opened this issue Oct 4, 2020 · 4 comments · Fixed by #56
Assignees
Labels
bug Something isn't working multitenancy security

Comments

@paulhowardarm
Copy link
Contributor

paulhowardarm commented Oct 4, 2020

Summary

Client-side socket path security checks can (and most likely will) fail if client is running in a Docker container.

Repro

On any Linux system, create a secure deployment of Parsec according to these documented steps.

Use the following numeric UIDs and GIDs (or else change the examples used in this repro recipe for different values)
2000 for the parsec user
3000 for the parsec-clients group
2001 for the parsec-client-1 example client user

Start the Parsec service as the parsec user.

Clone and build the parsec-tool. Use cargo build to build the default set of features. This will include the rust client with the socket folder permission checks.

Install Docker.

Change directory to where parsec-tool is checked out.

Create a Dockerfile with the following contents:

FROM debian
ADD target/debug/* /
CMD ["/parsec-tool", "ping"]

From the same directory run docker build --tag parsec-ping .

A docker image should be created. Run the image as follows:

docker run -v /run/parsec:/run/parsec -u 2001:3000 parsec-ping

EXPECTED: The docker container should execute the parsec-tool ping command running as user 2001 in group 3000 (which is parsec-client-1 in group parsec-clients). The output should be a successful ping of the service, reporting the supported wire protocol version.

OBSERVED: The container image runs, but the ping fails with an error saying Socket permission checks failed.

Root Cause

The issue is caused by the rust client checking the folder permissions by name and group name rather than by uid and gid respectively. The parsec and parsec-clients names are known to the host, but not known within the container, hence the permission checks fail.

Required Fix

We either need to relax the restrictions on the socket folder, or do the checks based on numeric ids rather than names. For the latter, we would need to document well-known numeric IDs for the parsec user and parsec-clients group.

@paulhowardarm paulhowardarm added bug Something isn't working multitenancy security labels Oct 4, 2020
@hug-dev
Copy link
Member

hug-dev commented Oct 5, 2020

Part of the resolution of this issue would have to be done with this one on Parsec.

I have just realised that our checks in the client were useless now that the socket is in /run/parsec.sock. That is because only an administrator can create the /run/parsec folder.
The same way we trusted the administrator to create the parsec user who owns the socket folder, we can now trust them to create the /run/parsec folder. I believe that is exactly the same level of trust and would allow us to remove all the checks in the client. Also, our threat model says that all users with privileges are trusted.

If we decide for now that Parsec will only be deployed with one authenticator there are two options:

  1. Deployment with Direct Authentication. If the socket is not visible to the client, it means they are not in the parsec-clients group. If it visible, clients can either trust the administrator to have set the correct group permissions or do the check themselves. Those checks will not work under containers as they are now, unles we fix the parsec-clients GID. In the parsec side, we can make sure the administrator did things right and add checks for this.
  2. Deployment with Unix Peer Credentials Authentication. Everybody should be able to see the socket and clients don't have to do any checks. Clients can be sure their keys can not be accessed with direct authentication because of both/either: keys are partitioned with authentication type and only one authenticator can be used by Parsec.

@ionut-arm
Copy link
Member

I'll have a look at what changes we need to make in the threat model with the new and improved filesystem locations, and the changes required for this issue - with a focus on making the TM easier to extend for new authenticators.

@paulhowardarm
Copy link
Contributor Author

Probably worth noting that I actually did this experiment with a build of Parsec that was still using /tmp rather than /run, but I updated the issue description to reflect the new path.

@ionut-arm
Copy link
Member

So the overall conclusion is that we don't need the checks and can just drop them, it seems. The threat model updates should cover our backsides

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working multitenancy security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants