Skip to content

Add pam module to read cached password from systemd-cryptsetup.#1550

Open
h0cheung wants to merge 2 commits intosddm:developfrom
h0cheung:pam_sddm
Open

Add pam module to read cached password from systemd-cryptsetup.#1550
h0cheung wants to merge 2 commits intosddm:developfrom
h0cheung:pam_sddm

Conversation

@h0cheung
Copy link
Copy Markdown

This solves #930.
Use the source file pam_gdm.c in gdm, build it as pam_sddm.so to avoid conflicting.
Then edit pam config and systemd units for it to work.

Copy link
Copy Markdown
Contributor

@davidedmundson davidedmundson left a comment

Choose a reason for hiding this comment

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

Overall it makes sense. I'm always happy to steal things from Gnome.

A few cmake issues

# KEYUTILS_INCLUDE_DIR - the keyutils include directories
# KEYUTILS_LIBRARIES - link these to use keyutils

find_path(KEYUTILS_INCLUDE_DIR keyutils.h PATHS
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

keyutils (on my system at least) has a pkgconfig file.
We should wrap that rather than manual calls where possible, it's more future proof

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pkg_check_modules(Keyutils IMPORTED_TARGET libkeyutils) would suffice.

include(KDEInstallDirs)

find_package(PAM REQUIRED)
find_package(keyutils REQUIRED)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

find_package(keyutils REQUIRED)
add_definitions(-DHAVE_KEYUTILS)

something here is off. We're making this required, yet have a code path to make it optional

int argc,
const char **argv)
{
#ifdef HAVE_KEYUTILS
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason we build this pam module at all if we don't have keyutils?

If not, we could move this to a cmake check above the add_subdirectory

@Vogtinator
Copy link
Copy Markdown
Contributor

The commit messages need a lot more explanation

@jinliu
Copy link
Copy Markdown

jinliu commented Nov 12, 2023

A similar PAM module was added in systemd 255, so we don't need another module. Just add this line to /etc/pam.d/sddm-autologin:
-auth optional pam_systemd_loadkey.so
or if the LUKS passphrase is bound to TPM:
-auth optional pam_systemd_loadkey.so keyname=luks2-pin

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.

5 participants