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

More secure file permissions for global YubiKey mapping file #147

Open
kbabioch opened this issue Apr 20, 2018 · 0 comments
Open

More secure file permissions for global YubiKey mapping file #147

kbabioch opened this issue Apr 20, 2018 · 0 comments

Comments

@kbabioch
Copy link
Contributor

This issue is about the file permissions of a global YubiKey mapping file (as opposed to a system-wide challenge-response setup as discussed in #146). This is described, for instance, in README:

yubico-pam/README

Lines 266 to 280 in f567af6

=== Central authorization mapping
Create a `/etc/yubikey_mappings`, the file must contain a user name and the
Yubikey token ID separated by colons (same format as the passwd file) for
each user you want to allow onto the system using a Yubikey.
The mappings should look like this, one per line:
<first user name>:<Yubikey token ID1>:<Yubikey token ID2>:….
<second user name>:<Yubikey token ID3>:<Yubikey token ID4>:….
Now add `authfile=/etc/yubikey_mappings` to your PAM configuration line, so it
looks like:
auth sufficient pam_yubico.so id=[Your API Client ID] authfile=/etc/yubikey_mappings

It is up to the system administrator to make sure the file has the correct ownership and permission.

The code is only doing some very basic sanity checking in regard to this file:

yubico-pam/util.c

Lines 112 to 131 in f567af6

fd = open(authfile, O_RDONLY | O_CLOEXEC, 0);
if (fd < 0) {
if(verbose)
D (debug_file, "Cannot open file: %s (%s)", authfile, strerror(errno));
return retval;
}
if (fstat(fd, &st) < 0) {
if(verbose)
D (debug_file, "Cannot stat file: %s (%s)", authfile, strerror(errno));
close(fd);
return retval;
}
if (!S_ISREG(st.st_mode)) {
if(verbose)
D (debug_file, "%s is not a regular file", authfile);
close(fd);
return retval;
}

There are no file ownership and/or permission checks in place, so this file could be group and/or world-writable, leading to insecure setups.

At the very least, there should be some sort of warning, when the ownership and permissions are wrong. This could be implemented as warning in debug mode or, as suggested by the PAM module writer's guide, using syslog(3) with LOG_AUTHPRIV as facility and LOG_ERR as level might be appropriate.

Personally, I would prefer to simply refuse to work in case of wrong permissions and ownership, just like OpenSSH does when the authorized_keys file is setup wrong, since it is inherently dangerous when the directory and file permissions are set up wrongly. Another possibility might be to inform the user about this using the functionality provided by pam_conv(3), although this might make an user aware of the security issue in the first place.

I'm happy to provide appropriate pull requests, but wanted to give this some discussion first, since I don't know how aggressive we want to be about this and which approach we should take.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant