Skip to content

Update default pool/path in pam_zfs_key.c #1305

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eborisch
Copy link

While rpool is the default name throughout OpenZFS codebase -- in comments and tests, primarily -- this instance directly impacts the ease of use of pam_zfs_key, improving POLA.

With this change, the default value for the pam_zfs_key paramater homes=... matches what bsdinstall (https://cgit.freebsd.org/src/tree/usr.sbin/bsdinstall/scripts/zfsboot#n44) creates by default on a zfs install: zroot/home.

While 'rpool' is the default name throughout OpenZFS codebase -- in
comments and tests, primarily -- this instance directly impacts the ease
of use of pam_zfs_key, improving POLA.

With this change, the default value for the pam_zfs_key paramater
homes=... matches what bsdinstall creates by default on a zfs install:
zroot/home.
Copy link
Member

@bsdimp bsdimp left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@@ -449,7 +449,7 @@ static int
zfs_key_config_load(pam_handle_t *pamh, zfs_key_config_t *config,
int argc, const char **argv)
{
config->homes_prefix = strdup("rpool/home");
config->homes_prefix = strdup("zroot/home");
Copy link
Member

Choose a reason for hiding this comment

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

Though it's ok, it would be better to have a FreeBSD ifdef and bring in the change via upstream.

Copy link
Author

Choose a reason for hiding this comment

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

Should I just take this directly (with ifdefs) to OpenZFS, then? (Am I reading that right?) It doesn't seem super robust on FreeBSD to begin with; I can get it to mostly work with console logins, but SSH appears to be a non-starter (session complains about being unable to get password from PAM stack.)

Copy link
Member

Choose a reason for hiding this comment

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

My apologies for the tardy response.... I've been away due to some personal issues...

I think that you likely should... But I'm not familiar with all the bits that are needed to make this functionality work completely on FreeBSD.

I've not tried this feature out at all...

Copy link
Member

Choose a reason for hiding this comment

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

For upstream should we have a more extensive change to use zroot rather than rpool generally on FreeBSD?

@bsdimp bsdimp self-assigned this Oct 4, 2024
@bsdimp bsdimp added the please-upstream 3rd party software that should come in via upstream label Oct 4, 2024
@bsdimp
Copy link
Member

bsdimp commented Oct 4, 2024

Sorry for the long delay. I think the best way forward is to submit it upstream, #ifdef'd for FreeBSD, and we'll bring it into the tree that way. Once it's upstreamed, we'll figure out the best way to land it.

@bsdimp
Copy link
Member

bsdimp commented Jun 12, 2025

Ping? What's the status of this upstream?

@eborisch
Copy link
Author

Ping? What's the status of this upstream?

Sorry; out of sight, out of mind... I'll try to do this and link back.

eborisch added a commit to eborisch/zfs that referenced this pull request Aug 7, 2025
As described in freebsd/freebsd-src#1305 : FreeBSD's installer defaults to zroot/home for user home directories.

For FreeBSD only, set the default prefix for pam_zfs_key to match.
@eborisch
Copy link
Author

eborisch commented Aug 7, 2025

Fiinally got back to this and submitted an upstream PR. openzfs/zfs#17600

@bsdimp
Copy link
Member

bsdimp commented Aug 7, 2025

Fiinally got back to this and submitted an upstream PR. openzfs/zfs#17600

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please-upstream 3rd party software that should come in via upstream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants