adelton / mod_authnz_pam

Apache module to run PAM authorization on result of other module's authentication; also full Basic Auth PAM provider.
https://www.adelton.com/apache/mod_authnz_pam/
Apache License 2.0
14 stars 9 forks source link

why auth stack would require a account stack check #5

Closed cxfcxf closed 4 years ago

cxfcxf commented 6 years ago

i learned this by write using it with my own ldap module so the code will try to do an account stack operation even you only ask it to do auth

say: you have pam-service file as

auth    required   pam_ldap_go.so

you do not want it to do an account stack operation but it will do a full _PAM_STEP_ALL as default for auth stack

in another case you have

auth    required   pam_ldap_go.so
account required   pam_ldap_go.so

in auth stack, it will do full _PAM_STEP_ALL including a pam_acct_mgmt call then it will do a _PAM_STEP_ACCOUNT, which is already done in first auth call

adelton commented 6 years ago

If you just want the PAM to do auth and always pass the account step for the Basic Auth (AuthBasicProvider PAM) use case, you should be able to use

account required pam_permit.so

or something similar.

The reason we do the auth + account and not just auth is to get 401 HTTP status, to allow for easy fall-back to another authentication provider.

cxfcxf commented 6 years ago

thanks for explain the reason behind. may i suggest modify the document to only include auth stack call, and mention this?

auth required sss.so

according to following from document

The PAM service needs to be configured. For the above shown
tlwiki example, file /etc/pam.d/tlwiki could be created with content

    auth    required   pam_sss.so
    account required   pam_sss.so

to authenticate against sssd.

this would do two calls, one to auth and one to account (not necessary) it may confuse people since auth already did the account stack call.

just a suggestion since it might help with people using it later

adelton commented 6 years ago

What is configured in /etc/pam.d/tlwiki does not imply what calls will be made. It's up to the code to make the call.

In the README, we show the basic auth with Require valid-user, which is basically r->user is set. We might change it to Require pam-account tlwiki to be explicit about using the account module as well, and apply the proposed patch.

But that might break (left without checks) existing installations which assume that the account check is being done against PAM stack for basic auth with valid-user.

I'm not rejecting the idea just yet but I need to find some time to think about and test all the implications.

cxfcxf commented 6 years ago

thanks for the consideration.

adelton commented 4 years ago

I've now amended README to clarify that it is not needed to do Require pam-account ... for the Basic authentication situation, and I've added tests to actually check that behaviour, as we as the behaviour of having different PAM service (and thus account check run) configured with AuthPAMService and different one with Require pam-account.

I was also considering adding an option to skip the account step in the Basic authentication operation but account required pam_permit.so should achieve that.