PAM_SUCCESS returned for non-duo users instead of PAM_IGNORE about duo_unix HOT 8 OPEN

@st0rmi This seems like a reasonable idea. The tricky part is that if we release with this change, it could alter the behavior of existing PAM stacks, possibly even opening security vulnerabilities if people aren't expecting it.

Off the top of my head, the safest way to deal with that problem feels like we should keep the old behavior as default but allow (probably by config) an opt-in to the new behavior. I'm worried that hardly anyone will opt in to the new behavior though, somewhat undermining the effectiveness of updating it...

from duo_unix.

@st0rmi Just to make sure I'm looking at the right place, you are talking about the return value at https://github.com/duosecurity/duo_unix/blob/master/pam_duo/pam_duo.c#L221? Where if the user doesn't match any groups we return PAM_SUCCESS?

from duo_unix.

@AaronAtDuo Yup, that's the line that I believe is the culprit. I am not 100% sure, but I believe a PAM_IGNORE might be better here.

from duo_unix.

@AaronAtDuo That such a change might break existing PAM configurations was a concern I had as well. I think handling this behavior via config is probably the safest option. It might also be an option to take the "fail safe"/"fail secure" config that already exists for other behaviors into account.

from duo_unix.

We do some shenanigans on our site to ensure we don't go through DUO for non-DUO users at all, specifically because of this problem. So +1 from here.

from duo_unix.

It could also be an option to initially make this a non-default configuration (i.e. user has to manually set this behavior in the config file) and then change it to be the default behavior during the next major version update.

I could go ahead and create a PR for this, unless you need some more time to discuss this change internally first. What are your thoughts on this?

from duo_unix.

Interesting - looks like about 8 years ago we had the same thought, but reverted it
3eb3e35

from duo_unix.

By the PAM semantics, however, the commit AaronAtDuo mentions above was correct prior to the change. I note this makes it difficult to manage with the DUO config instead of inside PAM (it's certainly possible to bypass based on group with PAM, because that's what I'm doing now, but I acknowledge it makes the config more complicated).

Can I second a config option, please? I understand why the default behaviour is designed to support your DUO configuration, but for those people who have multiple MFA stacks it matters. There are complex reasons why we have multiple MFA stacks and it's not an option to use DUO for certain users. At the moment I have to put all DUO users into a posix group and detect that group, otherwise bypass DUO.

The alternative to a config option is to change your documentation so that people who want to use the DUO config have to use [ ignore=ok ]. That should result in the same behaviour that you have now (as [success=ok] in a standard PAM stack). Then you can return ignore. This does require a config change on the client side and would need more communication of the change.

from duo_unix.