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

check file permissions as authenticating user #107

Closed
wants to merge 1 commit into from

Conversation

cdragan
Copy link

@cdragan cdragan commented Jul 8, 2020

It's necessary to check permissions of authorized_keys file with the same euid
as opening the file, otherwise the permissions check may fail even if the
following fopen would succeed. This can happen e.g. when user's directory
is mounted from an NFS share with root squash.

It's necessary to check permissions of authorized_keys file with the same euid
as opening the file, otherwise the permissions check may fail even if the
following fopen would succeed.  This can happen e.g. when user's directory
is mounted from an NFS share with squashroot.
@cdragan
Copy link
Author

cdragan commented Jul 8, 2020

Hello, I found this problem with 2019.78 and it still exists with 2020.80. When the user's directory is mounted through NFS with root squash option (on the server), then checkpubkeyperms() would fail. The code after the invocation of checkpubkeyperms() already has a fix for this, the permissions are being correctly apply for fopen(), but even if fopen() would pass, checkpubkeyperms() fails because it's running with root privileges.

This is one possible to fix this issue.

mkj added a commit that referenced this pull request Mar 30, 2022
This is necessary on NFS with squash root.
Based on work from Chris Dragan
This commit also tidies some trailing whitespace.

Fixes github pull #107
@mkj
Copy link
Owner

mkj commented Mar 30, 2022

Thanks Chris, I've applied a similar change in 2f68f66

@mkj mkj closed this Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants