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

SSH Permission Changes #185

Merged
merged 3 commits into from
Oct 10, 2024
Merged

Conversation

markagonzales
Copy link
Contributor

@markagonzales markagonzales commented Sep 1, 2024

linuxserver.io


  • I have read the contributing guideline and understand that I have made the correct modifications

Description:

This PR changes how code-server manages files in the /config/.ssh directory on startup. Three scoped find commands replace the existing chown and enforces the correct file permission for directories, private keys, and public keys that may be present.

Benefits of this PR and context:

The .ssh can have other files in that directory which the current. Although it addresses unsafe permissions and satisfies the client, the current behavior over corrects what those permissions should be and potentially disrupt how that directory is organized.

How Has This Been Tested?

A nod to the recommendation to NOT mount into an existing mount path but for testing purposes...

mkdir -p temp-ssh/hi
touch temp-ssh/{hello,key.pub}
chmod 777 temp-ssh/*  
ls -alth temp-ssh
# this will need to be stopped after it completes its startup
docker run --rm -v "$PWD/temp-ssh:/config/.ssh" -e "PUID=$(id -u)" -e "PGID=$(id -g)" lscr.io/linuxserver/code-server:latest
ls -alth temp-ssh

Subdirectories should remain user at least 700, public keys should have 644 permissions, and private keys 600.

Source / References:

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this pull request! Be sure to follow the pull request template!

@LinuxServer-CI
Copy link
Collaborator

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/code-server/4.92.2-pkg-be24e528-dev-c0aa0f7fc4f823a51eedd6e89ed462aadf915066-pr-185/index.html
https://ci-tests.linuxserver.io/lspipepr/code-server/4.92.2-pkg-be24e528-dev-c0aa0f7fc4f823a51eedd6e89ed462aadf915066-pr-185/shellcheck-result.xml

Tag Passed
amd64-4.92.2-pkg-be24e528-dev-c0aa0f7fc4f823a51eedd6e89ed462aadf915066-pr-185
arm64v8-4.92.2-pkg-be24e528-dev-c0aa0f7fc4f823a51eedd6e89ed462aadf915066-pr-185

@LinuxServer-CI
Copy link
Collaborator

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/code-server/4.92.2-pkg-978f6c7a-dev-60b285ea654ef13d427fe32f7a2d901ab3b9e7b7-pr-185/index.html
https://ci-tests.linuxserver.io/lspipepr/code-server/4.92.2-pkg-978f6c7a-dev-60b285ea654ef13d427fe32f7a2d901ab3b9e7b7-pr-185/shellcheck-result.xml

Tag Passed
amd64-4.92.2-pkg-978f6c7a-dev-60b285ea654ef13d427fe32f7a2d901ab3b9e7b7-pr-185
arm64v8-4.92.2-pkg-978f6c7a-dev-60b285ea654ef13d427fe32f7a2d901ab3b9e7b7-pr-185

@LinuxServer-CI
Copy link
Collaborator

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/code-server/4.93.1-pkg-3d426902-dev-35f551fad7483df777ad0fca51b0abc070bd90d7-pr-185/index.html
https://ci-tests.linuxserver.io/lspipepr/code-server/4.93.1-pkg-3d426902-dev-35f551fad7483df777ad0fca51b0abc070bd90d7-pr-185/shellcheck-result.xml

Tag Passed
amd64-4.93.1-pkg-3d426902-dev-35f551fad7483df777ad0fca51b0abc070bd90d7-pr-185
arm64v8-4.93.1-pkg-3d426902-dev-35f551fad7483df777ad0fca51b0abc070bd90d7-pr-185

@chessmango
Copy link

PR looks excellent, thanks @markagonzales - just run into this issue myself and I'm currently working around it with custom-cont-init.d, but it'd be good to get this into a better state.
@aptalca or @thespad (probably?) - could you review this? :D

Thanks!

@thespad
Copy link
Member

thespad commented Oct 10, 2024

Could you update the changelog date and then I'm happy to merge this

@thespad thespad self-assigned this Oct 10, 2024
@LinuxServer-CI
Copy link
Collaborator

I am a bot, here are the test results for this PR:
https://ci-tests.linuxserver.io/lspipepr/code-server/4.93.1-pkg-3d426902-dev-d95bcfcc582e8b71c6a0c5790b9fc500d17d7a07-pr-185/index.html
https://ci-tests.linuxserver.io/lspipepr/code-server/4.93.1-pkg-3d426902-dev-d95bcfcc582e8b71c6a0c5790b9fc500d17d7a07-pr-185/shellcheck-result.xml

Tag Passed
amd64-4.93.1-pkg-3d426902-dev-d95bcfcc582e8b71c6a0c5790b9fc500d17d7a07-pr-185
arm64v8-4.93.1-pkg-3d426902-dev-d95bcfcc582e8b71c6a0c5790b9fc500d17d7a07-pr-185

@thespad thespad merged commit 1f80e77 into linuxserver:master Oct 10, 2024
4 checks passed
@markagonzales markagonzales deleted the ssh-perm-changes branch October 10, 2024 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants