Bug 245713 - security/doas: Add PAM option
Summary: security/doas: Add PAM option
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords: feature
Depends on: 245238
Blocks:
  Show dependency treegraph
 
Reported: 2020-04-18 01:48 UTC by Hiroo Ono
Modified: 2020-04-21 12:06 UTC (History)
1 user (show)

See Also:
koobs: maintainer-feedback? (jsmith)


Attachments
patch to Makefile and a new file in files/ (1.65 KB, patch)
2020-04-18 01:48 UTC, Hiroo Ono
no flags Details | Diff
Revised patch to Makefile and a new file in files/ (1.68 KB, patch)
2020-04-18 06:10 UTC, Hiroo Ono
no flags Details | Diff
Enable PAM support and imports PAM config file (2.03 KB, patch)
2020-04-20 23:06 UTC, jsmith
jsmith: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hiroo Ono 2020-04-18 01:48:54 UTC
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.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2020-04-18 03:00:53 UTC
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!
Comment 2 Hiroo Ono 2020-04-18 06:10:18 UTC
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 3 jsmith 2020-04-18 14:07:13 UTC
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?
Comment 4 Hiroo Ono 2020-04-18 22:03:02 UTC
(In reply to jsmith from comment #3)
The upstream currently has no pam configuration sample.
Having some is a good idea, I think.
Comment 5 jsmith 2020-04-20 23:06:31 UTC
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.
Comment 6 jsmith 2020-04-20 23:08:50 UTC
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.
Comment 7 Hiroo Ono 2020-04-21 00:29:54 UTC
(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.
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2020-04-21 02:29:33 UTC
@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
Comment 9 jsmith 2020-04-21 12:06:04 UTC
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.