irods / irods_auth_plugin_pam_interactive

BSD 3-Clause "New" or "Revised" License
2 stars 5 forks source link

Authentication loops forever with `pam_password`-like flow while connected to catalog service consumer #47

Closed alanking closed 2 months ago

alanking commented 2 months ago

While connected to a catalog service consumer, I tried to authenticate using the pam_password-like flow documented in the README and found that I could not. It looks like the PAM conversation gets stuck in a loop:

$ iinit
Password:  
Password: [******] 
Password: [******] 
Password: [******] 
Password: [******] 
Password: [******] 

It doesn't matter if I type the password correctly or incorrectly - it just loops until you ctrl+C out of it. Despite the [******]'s appearing in the prompts, the password is never written down to a file, so that's good. I also confirmed that the enforcement of insecure_mode on the redirect is working as intended.

I created the PAM user on both the catalog service provider and catalog service consumer with the same name and password. I tried authenticating from a catalog service consumer with pam_password proper and everything is working as expected.

I think this has to do with the redirecting to the catalog service provider in this plugin and the way the PAM conversation works. I think that the PAM conversation is not being advanced in the connected server agent in the catalog service consumer. As you can see here, the agent response method just returns when the step completes in the redirected connection to the catalog service provider: https://github.com/irods/irods_auth_plugin_pam_interactive/blob/aa843bb04c1c1c45ba572d17e81a8021041c7645/plugin/src/main.cpp#L578-L599

In the else case, the state of the PAM conversation is updated for the connected agent in the catalog service consumer, but that will never happen for this case when we redirect to the catalog service provider and then return. However, the JSON response coming from the catalog service provider should be using all the same keys, so I'm not sure why it wouldn't advance to the next step.

Please investigate.

Bonus note: I was able to authenticate using this flow on the catalog service provider, even if I didn't have an "irods" PAM stack. Not sure if that's good or bad.

alanking commented 2 months ago

We could also just remove the requirement that we must redirect to a catalog service provider. There is only one case where it is required that we be connected to a catalog service provider and it is here: https://github.com/irods/irods_auth_plugin_pam_interactive/blob/aa843bb04c1c1c45ba572d17e81a8021041c7645/plugin/src/main.cpp#L627-L631

The rest of the time, we only require being connected to the catalog service provider as a matter of convenience for the administrator so that PAM only needs to be configured on one machine. Changing this could possibly be a negative for that reason, so we should probably get some input from the working group.

alanking commented 2 months ago

From team discussion...

If connected to a catalog service consumer, the server could check for a configuration which would have one of three values:

  1. Reject authentication attempt
  2. Authenticate with PAM locally
  3. Send provider (or... other... configured hostname?) connection information back to client-side plugin to redirect (automatically)

(1) would solve this problem (that is, the looping). (2) would "open up" the PAM conversation for all servers (or at least all the servers configured with (2)). Is this a security concern? Don't know. (3) can happen later as it would be an enhancement.

alanking commented 2 months ago

After more team discussion...

It seems that redirecting to the catalog service provider (if there is one catalog service provider in the PRIMARY_RCAT role) will reuse the same connection every time. This was discovered when the switch_user API was being developed. See: https://github.com/irods/irods/blob/bc2a9bf0d8d3587d1aad8bf6a9b3db650e0d6944/plugins/api/src/switch_user.cpp#L281-L284

If we remove the call to disconnect here, the agent will keep its state and pam_interactive is able to function while connected to the catalog service consumer: https://github.com/irods/irods_auth_plugin_pam_interactive/blob/d760282b7416000272a6e71ce1d8484f30adb423/plugin/src/main.cpp#L583-L588

We should investigate a bit to make sure we're not leaking connections on redirection to the catalog service provider, but I'm pretty sure this is okay because many of our APIs have this redirect behavior without calling rcDisconnect at the end (so that other redirects can use the same connections, I think). So, the behavior will either be like our existing redirect implementations (if there are any problems there, there will be identical problems here), or this is okay. Here is an example of an API which redirects to the catalog service provider without disconnecting at the end: https://github.com/irods/irods/blob/bc2a9bf0d8d3587d1aad8bf6a9b3db650e0d6944/plugins/api/src/atomic_apply_metadata_operations.cpp#L447-L459

One question which remains: What happens when there are multiple catalog service providers in a zone? Can multiple catalog service providers be advertised as PRIMARY_RCAT? Should we be using PRIMARY_RCAT in the redirection for this plugin?

Finally, I think this removes the need for the configuration described in my previous comment. This plugin (and pam_password) have always redirected to the catalog service provider out of necessity, and we cannot perform the PAM authentication on the local server without opening a security hole. So, the only option we would be exposing would be to disallow authentication while connected to a catalog service consumer (that is, reject any redirection attempts). This is easily implemented as a rule in the server(s) in question, so we do not need to expose this as a configuration for the plugin.