Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document how to store passwords in .irodsA file #596

Closed
alanking opened this issue Jul 30, 2024 · 7 comments
Closed

Document how to store passwords in .irodsA file #596

alanking opened this issue Jul 30, 2024 · 7 comments
Assignees
Milestone

Comments

@alanking
Copy link
Contributor

It seems this is possible:

# Store new password in .irodsA if requested.
if self.account._auth_file and cfg.legacy_auth.pam.store_password_to_environment:
with open(self.account._auth_file,'w') as f:
f.write(obf.encode(auth_out.result_))
logger.debug('new PAM pw write succeeded')

However, this is not documented anywhere, and it's not clear (at least to me) whether this option is available outside of PAM authentication (this code is under _pam_login) or how to use it. Let's add documentation around how this works and how to use it.

@alanking alanking added this to the 2.1.0 milestone Jul 30, 2024
@d-w-moore
Copy link
Collaborator

d-w-moore commented Jul 30, 2024

I think it's already documented here in the list item that describes the setting legacy_auth.pam.store_password_to_environment. Do we need more detail in that section, or perhaps (more likely) elsewhere?

@alanking
Copy link
Contributor Author

Oh, I see. I was not looking for that.

This step is only performed while auto-renewing PAM authenticated sessions.

If this is true, that means that iRODS native authentication cannot use this feature. Is that true?

I feel like having a dedicated how-to section for this would be helpful. Or perhaps a mention of this option in the section about creating a connection (https://github.com/irods/python-irodsclient?tab=readme-ov-file#establishing-a-secure-connection) or one of the sections following that. Does that seem like a good idea? Maybe what we have is enough...

@korydraughn
Copy link
Contributor

A new section makes sense given it wasn't obvious where to look.

That could be either a FAQ, Troubleshooting, or How-To section.

@d-w-moore
Copy link
Collaborator

d-w-moore commented Jul 31, 2024

This step is only performed while auto-renewing PAM authenticated sessions.

If this is true, that means that iRODS native authentication cannot use this feature. Is that true?

Native authentication doesn't execute this particular section of code , since it's located within _login_pam(), but know that PRC does - like iinit - store the encoded native encoded password data in the environment's auth file, .irodsA, in particular when a user changes their native password. I remember implementing that one, too, some time ago. But when it's the iRODS native password that's changing, no configuration setting is needed to approve the overwriting of the .irodsA. Which seems appropriate, because native auth is the most direct and straightforward form of authentication in iRODS.

What would be more accurate to say regarding PAM is that PAM authentication in PRC currently needs explicit approval to overwrite .irodsA with its encoded password token, and that approval is granted by setting legacy_auth.pam.store_password_to_environment to True. I think my mindset was that I wanted the designer of an application that leverages PRC to be explicit about (thus provably conscious of) the decision of making PAM the active authentication method, rather than just let PRC possibly accidentally overwrite a current copy of .irodsA that might contain a user's native login data. So, ... accountability, in other words. I may have been misguided or inconsistent with other clients in taking that approach, however. After all iinit just goes ahead and overwrites it with whatever the current auth scheme dictates, right?

This all arose from trying to give the PRC some iinit-like behavior with respect to reading & writing PAM password data in .irodsA, which it never could do until a very recent release (v2.0.0). And implementing things the way I did seems to have made sense as opposed to PRC shelling out to iinit when for example a PAM password was found to have expired.

The consequence I guess is that iinit-type behavior is more arbitrarily distributed throughout the PRC in its present form than it should be, ideally.

Background: Yes, it's true PRC has long had the capability of authenticating through PAM, but until that recent release it wasn't using stored .irodsA data or persisting such data back into .irodsA for the next PAM session when the current password was found to have expired. Rather, PRC was actually authenticating each new session by generating the login tokens anew, via calling the PAM api instead of respecting what iinit might have stored in .irodsA. Examine the _login_pam method in v1.1.9, and you'll see that _login_native is not called preemptively to query the environment-stored password before deciding whether to have the entire PAM-api exchange with the server.

Whereas the iCommands' methodology was always to read out the PAM token that existed already in .irodsA . So this just brings PRC up to where the iCommands had been.

@alanking alanking modified the milestones: 2.1.0, 2.1.1 Aug 1, 2024
@d-w-moore
Copy link
Collaborator

d-w-moore commented Sep 18, 2024

There's now a commit in pr #620 documenting how users may employ PRC in place of iinit to store pam passwords.

It isn't clear to me which route to go, in furnishing the iinit-like capability to create an .irodsA in PRC for native logins as well. Is that something we want / need ?

If so, how do we prefer to proceed?

  • by using a similar configuration option, say legacy_auth.native.password, that enables writing the native .irodsA , or
  • by providing a free function in the PRC, something of this sort for anyone who wants to call it:
def write_native_credentials( irods_password ):
   # open .irodsA path
   # write encoded irods_password

@korydraughn
Copy link
Contributor

Providing a free function feels the most correct and enables flexibility.

I don't think the PRC should automatically write any files due to its design. That feels like a decision the user/dev should make. It's also not clear when the PRC would write that file.

@korydraughn
Copy link
Contributor

Adding this as a discussion item.

d-w-moore added a commit to d-w-moore/python-irodsclient that referenced this issue Oct 11, 2024
… native authentication.

This commit introduces iinit-like capability to generate the .irodsA file, when not
previously existing, for the pam_password authentication scheme.  Also, free functions
are introduced which create the .irodsA file from a cleartext password value in the native and
pam_password authentication schemes.
alanking pushed a commit that referenced this issue Oct 11, 2024
…ication.

This commit introduces iinit-like capability to generate the .irodsA file, when not
previously existing, for the pam_password authentication scheme.  Also, free functions
are introduced which create the .irodsA file from a cleartext password value in the native and
pam_password authentication schemes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants