Created attachment 213515 [details] patch to Makefile and a new file in files/ security/doas has PAM support in its source, but it is not enabled. Add option PAM to enable this. As I use winbind to user authentication, PAM support is critical for me.
Thank you for the report and patch Hiroo Could you please update the patch to include PAM as an OPTIONS_DEFAULT for package users. Thanks!
Created attachment 213526 [details] Revised patch to Makefile and a new file in files/ Thank you. Added OPTIONS_DEFAULT=PAM. by the way, bug #245238 is also a patch to security/doas, which is maintainer update.
Comment on attachment 213526 [details] Revised patch to Makefile and a new file in files/ I have tested this on a FreeBSD server. Can confirm it builds and authenticates properly. Am approving and recommending the patch. On a side note, how does everyone feel about moving some of this, like the files/pam.conf addition, upstream? I'd prefer to have it there as opposed to as a downstream patch. Maybe we can call it something like "pam.conf.freebsd" in the upstream repo?
(In reply to jsmith from comment #3) The upstream currently has no pam configuration sample. Having some is a good idea, I think.
Created attachment 213618 [details] Enable PAM support and imports PAM config file Enables PAM support and imports new PAM configuration file from upstream source rather than generating it at build time.
I have copied the provides PAM configuration sample file into the upstream source. It will be available in doas 6.2p5 and newer. I've posted a patch which just implements the original patch, but uses the config file from upstream rather than generating it when the port is built. Hopefully this will do the same thing as the original patch, just pull in the config sample from upstream. Please give it a test run and confirm this works.
(In reply to jsmith from comment #6) poudriere testport succeeded with and without PAM option. Verified that doas authentication fails without PAM option (which is an expected behavior on my PC), and succeeds with PAM option. Your patch works well.
@Maintainer any reason this issue depends on bug 245238? If the man page issue is also in quarterly, we'll want to merge that change, which explains bug 245238 going first, as we don't normally add merge feature/functional changes, which adding the PAM options is
I do not think this issue relies on bug 245238 in any way. Bug 245238 is a minor update to fix an error in the manual page and does not adjust or add any features. This issue with PAM support (and the patch I put together) can be applied independently of the 245238 issue. In other words, I think the patch for 245238 should be applied as a bug fix. While this issue can wait and be applied later as a new feature in the next quarterly update.
Since this code has been applied upstream and I believe is now available in the FreeBSD port, we can close this report.
Thank you for reporting the patch upstream.