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

Implement/extend transport plugin to support authentication by password while not recommending it #6642

Open
agoscinski opened this issue Nov 26, 2024 · 23 comments · May be fixed by #6761
Open

Comments

@agoscinski
Copy link
Contributor

agoscinski commented Nov 26, 2024

Is your feature request related to a problem? Please describe

A number of French HPCs only allow an authentication by password. By modern safety standards this is considered as not safe but it is current state for a lot of HPCs (merde!) so we should support it while not recommending it. Since this is a large user bases I marked it as important.

Describe the solution you'd like

Implement a transport plugin or extend the current transport to also support ssh connections authenticated by password.

@unkcpz
Copy link
Member

unkcpz commented Nov 27, 2024

I think it would be better to not put in the aiida-core but as a separate plugin and registered in aiida-register?

@khsrali
Copy link
Contributor

khsrali commented Nov 28, 2024

I can arrange this in core.ssh_async or to be unified with core.ssh
We can put it in the aiida-core, but explicitly warn them about the danger of this unsafe behavior.. the rest is user's responsibility tbh..

@unkcpz
Copy link
Member

unkcpz commented Feb 13, 2025

Was come from #6688 (comment)

Some idea, hope it is helpful. There is a keyctl which is a basic tool in linux for store keyring. @agoscinski could you check if it is also shipped with Mac?

I think this linux tool can provide what we need to build a relatively safe plugin that solve the issue.

  1. I think it should be a plugin rather than inside aiida-core (this can be hard since we don't yet have any interface to separate the authentication layer out from transport, @khsrali I guess you know more about the difficulties here? )
  2. When daemon start, this plugin should force user to provide the password and store in the memory, when daemon stop, it should be disabled immediately.
  3. when running calculation without daemon, the password is prompt every time.

@unkcpz
Copy link
Member

unkcpz commented Feb 13, 2025

After some research, I think it is doable.
The mac os does not provide keyctl which is a linux specific thing, but mac has its own tool called keychain which has parallel functionality that handle the crypt in kernel level.

For the tool side, since our user are mainly linux and apple, we can limit this functionality to these two platform at the moment.

  • From the tool, to make it work with AiiDA, we need a python implementation that wrappers C API for both linux and mac. No tool exist yet, but should be easy to do by just grab https://github.com/landhb/linux-keyutils and https://github.com/kornelski/rust-security-framework with implement python API on top. (Yes, it is 🦀 again 😄).
  • We therefore need a list of API to define what is the shared operation that required to store and setup a ephemeral key that bind to daemon. TBD.
  • From aiida-core side, this functionality should be exposed to two parts: daemon and transport. I am thinking to introduce a hook entry point to aiida-core that we not only can define the plugins for using AiiDA. But also the hook that spread around the engine part that can change the behaviour of how kernel part of AiiDA works. For this case, the hook should be put in daemon_start and daemon_stop for create and clean the key. Also the check hook should be put in transport for checking if there is valid password in the memory, otherwise raise and abort the operation.

Therefore, we need aiidateam/keymanager that provide abstraction python API to inteact with both mac/linux syscall, aiidateam/key-hook that use keymanager and provide the hook (these two thing can be just one repo, but I somehow think the keymanager can be generic tool that also needed by other python tool, TBD) and implement the hook functionality in aiida-core.

@agoscinski @khsrali what do you think?

@khsrali
Copy link
Contributor

khsrali commented Feb 15, 2025

Thanks @unkcpz . Indeed using a keymanager is the best solution for this.

P.S. Now that I read this issue again:
We have to be explicit, what the exact mechanism of those French HPCs are?
for instance if they are asking for multiplexing, something like this:
https://aiida.discourse.group/t/accessing-clusters-with-2-factor-authentication-sigma2-using-aiida/538

It's not clear if it even possible at all. No python package currently support it.

@unkcpz
Copy link
Member

unkcpz commented Feb 15, 2025

https://aiida.discourse.group/t/accessing-clusters-with-2-factor-authentication-sigma2-using-aiida/538

The OP is a different issue, and Sigma2 is a Norwegian HPC clusters. You can open another issue for that, this issue is specifically about an authentication by password.

We have to be explicit, what the exact mechanism of those French HPCs are?

@mbercx @agoscinski who is the contact person you know that has this use case? Is the description from this issue complete? Can we (do we need) get more specific use case of it? If it requires just username and password it also makes a lot sense (from the description of this issue, it is.)

@etiennemlb
Copy link

The use case is this: provided a distant ssh enabled machine, not providing key authentication, not port forwarding, make aiida work correctly. The OS is RHEL 8/9. you are not root. You can't expect whatever software to be installed on the login nodes (it's not the far west and there are security implications for more things that I would like..). You can get a feel for the environment by reading through: https://dci.dci-gitlab.cines.fr/webextranet/index.html, https://www-hpc.cea.fr/tgcc-public/en/html/tgcc-public.html

AFAIK, there are no relevant additional constraints.

@khsrali
Copy link
Contributor

khsrali commented Feb 17, 2025

@etiennemlb, Thanks for your comment. However I don't understand. Please be more descriptive and provide a clear and step by step procedure that how you are connecting to your server.

distant ssh enabled machine,

I don't understand what you mean by that, sorry. 💁

The OS is RHEL 8/9. you are not root.

You are not supposed to install aiida on the server, you should install it on your local machine, if that's what you mean.

https://dci.dci-gitlab.cines.fr/webextranet/index.html,
https://www-hpc.cea.fr/tgcc-public/en/html/tgcc-public.html

To save time, please send an exact link to the exact login procedure,

@unkcpz
Copy link
Member

unkcpz commented Feb 17, 2025

provided a distant ssh enabled machine, not providing key authentication, not port forwarding, make aiida work correctly. The OS is RHEL 8/9. you are not root.

I believe this means the remote HPC or super computer center.

To save time, please send an exact link to the exact login procedure,

I also struggle to find where in the documentation it mentioned password only login. @etiennemlb could you point to the link?

I think anyway this is a fair use case and provide such functionality would help for onboarding new user, since set up a key pair for passwordless login is not always trivial if user never did it. The difficulties for us is how to make computer setup interface consistent with the one require private key. Will discuss this on biweekly meeting.

but should be easy to do by just grab https://github.com/landhb/linux-keyutils and https://github.com/kornelski/rust-security-framework with implement python API on top.

This part is easy, I had something work already ;)

@etiennemlb
Copy link

@etiennemlb, Thanks for your comment. However I don't understand. Please be more descriptive and provide a clear and step by step procedure that how you are connecting to your server.

distant ssh enabled machine,

I don't understand what you mean by that, sorry. 💁

The OS is RHEL 8/9. you are not root.

You are not supposed to install aiida on the server, you should install it on your local machine, if that's what you mean.

https://dci.dci-gitlab.cines.fr/webextranet/index.html,
https://www-hpc.cea.fr/tgcc-public/en/html/tgcc-public.html

To save time, please send an exact link to the exact login procedure,

Distant, meaning the machine is not somewhere in your garage, you do not have access to it, so no assumption about your ability to fix it should be taken. A very small amount of requirement (of which I do not know exactly the extent) should be made about the target machine (assume RHEL8/9).

The point is not that you should install Aiida or not, the point is that you can't, again, change things on the machine. You can touch the configuration of the ssh daemon for instance. You can't enable ssh port forwarding if not already enabled etc etc.

For the login procedure, it's very classical, except that no ssh key is usable for authentication:
https://dci.dci-gitlab.cines.fr/webextranet/quick_start/index.html#connecting-to-adastra
https://dci.dci-gitlab.cines.fr/webextranet/user_support/index.html#connecting
https://dci.dci-gitlab.cines.fr/webextranet/other/faq_and_known_issues.html#faq (see Can I use ssh keys for my connections to Adastra ?)

@khsrali
Copy link
Contributor

khsrali commented Feb 17, 2025

@etiennemlb ok, then all the information that you are actually trying to "add" to this issue is:
"On has to enter their password manually, although you are not aware of the exact mechanism, you assumed they just simply deactivated login via SSH key pairs"

No nasty language is required to say something as simple as that.

Basics about AiiDA:
It installs absolutely nothing, and it configures absolutely nothing on your server or the garage computer.

@agoscinski
Copy link
Contributor Author

I did some prototyping using keyring to identify challenges:

  • How do we not store the password using a keychain? The transport is initialized with the AuthInfo which will contain all the user configuration. So we need to prevent the storage of the password even before the transport plugin is created. I think we can create a dedicated orm password node that does not store any value in the database but has the hook to store and load it to the keychain.
  • On macOS using keyring within a ipython shell works, but for some reason when I try to use it in the transport plugin I get this error keyring.errors.PasswordSetError: Can't store password on keychain: (-25308, 'Unknown Error'). Because I could not find anything useful looking for the error message, I thought trying this out on Linux might be the be more fruitful to understand what is wrong. I saw some people having this issue when the keychain was locked, but I am not sure why it is locked.
  • Can we inherit from (async) ssh to implement? So far the changes I did are quite minimal so inheritance would be nice to reduce massive duplication

Technical details to keep in mind for implementation:

  • mark password as hide_input, click argument to not show typing in prompt
  • Use for tests ssh -o PubkeyAuthentication=no -o PreferredAuthentications=password to avoid that the sshkey is used, maybe command needs to be monkeypatched?

@unkcpz
Copy link
Member

unkcpz commented Feb 17, 2025

thanks for checking @agoscinski

I did some prototyping using keyring to identify challenges

I aware of keyring as well, but for macOS it has memory leak issue not yet solved (the issue you see I am not sure it is related). For the linux, I think we want to always fallback to keyutil which do not require dbus service.

I get this error keyring.errors.PasswordSetError: Can't store password on keychain: (-25308, 'Unknown Error')

I public the draft repo where I wrap keyring-rs that has keyctl support and seems not have macOS memory issue. Could you give it a try on your mac? https://pypi.org/project/keyringrs/

How do we not store the password using a keychain? The transport is initialized with the AuthInfo which will contain all the user configuration. So we need to prevent the storage of the password even before the transport plugin is created.

I was thinking this should be a prompt when setup computer and at the same time provide a command to setup the password separately. Can be verdi computer configure I think.
As I mentioned above, we need also decide the lifetime of the password in the keyring, we can make it persistent store in the keyring, or we can set a TTL to the password (keyring not support the TTL, we need a finer control if we really need it).

Can we inherit from (async) ssh to implement?

Why not ;) I am not always against inheritance, especially it is just prototype.
But it has to be a separated transport plugin or we can put some injection to pass password to paramiko call safely? This I am not sure and require more investigation.

@agoscinski
Copy link
Contributor Author

I public the draft repo where I wrap keyring-rs that has keyctl support and seems not have macOS memory issue. Could you give it a try on your mac? https://pypi.org/project/keyringrs/

I just understood my issue. It was because I used two different terminal panes and in the one I tested the verdi command I was ssh'ed into localhost. When you are ssh'ed you do not have permissions to access the keychain. The keyringrs also works.

Why not ;) I am not always against inheritance, especially it is just prototype. But it has to be a separated transport plugin or we can put some injection to pass password to paramiko call safely? This I am not sure and require more investigation.

I mean paramiko is supporting passwords, so we can pass the password to the client.connect, if we trust it to handle the private keys, I am not sure where a problem could be with the passwords. For the gotcomputer command we can use sshpass.

I am not sure if it should be separate transport or extend ssh, both would be possible and seems to be only an UI question. It depends what the goal ssh_async with respect to ssh and ssh_auto.

@unkcpz
Copy link
Member

unkcpz commented Feb 17, 2025

so we can pass the password to the client.connect, if we trust it to handle the private keys, I am not sure where a problem could be with the passwords.

both would be possible and seems to be only an UI question.

Sure we can trust it to handle password. The problem I don't have answer in head is how to pass password to client.connet, how we support both password and private key? How we prompt to ask user to give input and how we update orm model to store the key name, and what is the lifetime of the password.

It is only a ui question but to me it is a difficult one.

For the gotcomputer command we can use sshpass.

Good point, I never think about gotocomputer command. But I don't get why sshpass is needed?

@agoscinski
Copy link
Contributor Author

agoscinski commented Feb 18, 2025

The problem I don't have answer in head is how to pass password to client.connet

paramiko supports it https://docs.paramiko.org/en/latest/api/client.html Worked when I tried.

how we support both password and private key

Yes this is the open UI question to me. If we want to merge it into one or keep it separate. We should discuss with @khsrali

How we prompt to ask user to give input

In click you can specify hide_input, then the input is not shown in the terminal. One can just add to the transport _valid_connect_options. This worked for me

            'password',
            {
                'type': str,
                'prompt': 'Password',
                'hide_input': True,
                'help': 'Login password for the remote machine.',
                'non_interactive_default': True,
            },

orm model to store the key name

I am not sure if this covers all cases, but we could hook into the auth params in AuthInfo to set/get the password to/from the keychain and overwrite before writing to database

    def get_auth_params(self) -> Dict[str, Any]:
        auth_params = self._backend_entity.get_auth_params()
        if password := auth_params.get("password", None):
            from keyrings import Entry
            from aiida.manage import get_manager
            username = auth_params['username']
            profile = get_manager().get_profile().name
            auth_params['password'] = Entry(f"aiida-{profile}", f"{username}").get_password()
        return auth_params


    def set_auth_params(self, auth_params: Dict[str, Any]) -> None:
        if password := auth_params.get("password", None):
            from keyrings import Entry
            from aiida.manage import get_manager
            username = auth_params['username']
            profile = get_manager().get_profile().name
            Entry(f"aiida-{profile}", f"{username}").set_password(auth_params['password'])
            auth_params['password'] = True 
        self._backend_entity.set_auth_params(auth_params)

what is the lifetime of the password

Until the computer or profile is deleted.

Good point, I never think about gotocomputer command. But I don't get why sshpass is needed?

With ssh you cannot pass the password in the command so a prompt will appear. I guess that would be okay for that command, but if we could just use the stored password in a save way, it would be more convenient for the user. I guess if we do sshpass -pMY_PASSWORD ssh user@machine the password might appear in some history logs. So maybe we rather write it to a temporary file and use sshpass -f filename. One could be worried that the file is not correctly deleted if the program crashes, but if only the user has read rights to it, I don't see a problem if the tmp file by accident survives. Eventually the system will delete it since it is in the tmp directory.

@agoscinski
Copy link
Contributor Author

agoscinski commented Feb 18, 2025

@unkcpz @khsrali had a small discussion. These are the main points:

  • We decided to extend ssh_async with the functionality since this is the only ssh transport plugin we want to support in the future

  • We need to think about gotocomputer because passing the plain password into a file is unsafer than using keychain that keeps the password encrypted, maybe there are tools that help us to pass passwords of the keychain to the ssh command.

  • Keep in mind relabel and delete of computer needs to also take care of the password in the keychain

  • For the entry in the keychain it seems smarter to just use the label of the transport instance label Entry("aiida", f"{computer_label}")

@agoscinski
Copy link
Contributor Author

agoscinski commented Feb 18, 2025

I think the gotocomputer problem could be solved with sshpass -p "$(security find-generic-password -s my-service -a my-name -w)" ssh user@machine for macOS. For Linux the command needs to be adapted. The user will be prompted to unlock the keychain so it can be accessed, this is the user account password typically. One can set up to always allow access for this password to avoid these prompt. As I understand, the user is requested with a prompt for a password if a different application than the one that set it up is trying to access it. In this case, I created the password in python and wanted to access it with bash so I need to supply the password. I am a bit worried because in my keychain the application is just python. So any other python application now has access to it. I am also not sure how one would solve this problem, ideally only the aiida python package would have access to it. Right now I can only think of recommending the user to use aiida in a separate environment without anything else, to be sure that only this python environment has access to the keychain stored by aiida.

@unkcpz
Copy link
Member

unkcpz commented Feb 18, 2025

For Linux the command needs to be adapted. The user will be prompted to unlock the keychain so it can be accessed, this is the user account password typically.

Good to know, I think this is a good way for gotocomputer command.

In this case, I created the password in python and wanted to access it with bash so I need to supply the password. I am a bit worried because in my keychain the application is just python.

This is exact reason I was investigating the keyutil in linux in the first place. It provide the fine control for the keys that can be only store and accessed by a kernel process. In macOS, the keychain also support similar setup for a process space access. By default the keyring library from python didn't provide the way to set it, let alone it provide support for keyutil in Linux.

But I slightly change my perspective and think the process space key accessing is not necessary. If the key is set to user space it is fine. If the user installed any malicious app that will try to explore the keychains, the ssh password is just one of many important keys the hacker want to get.

@khsrali
Copy link
Contributor

khsrali commented Feb 18, 2025

I am a bit worried because in my keychain the application is just python. So any other python application now has access to it

that's a good point, actually 🤔
limiting the python environment seems not ideal, tbh.

@unkcpz can you update keyringrs/ so that it would verify the client as well?
One can do this, e.g. by storing the hash of the permitted script as a passphrase.

password = entry.set_password(<PASS>, ['transport.py'])

The only downside is that when a user updates their aiida installation, they may need to reauthorize, or re-configure their aiida computer.

@unkcpz
Copy link
Member

unkcpz commented Feb 18, 2025

limiting the python environment seems not ideal, tbh.

And I think it is not able to solve the problem, when using keyring (no matter the python one or the one I wrapped), the keys are set in the user level by default.

@mbercx
Copy link
Member

mbercx commented Feb 19, 2025

@mbercx @agoscinski who is the contact person you know that has this use case?

Not sure if it is still relevant, but I believe it was (French) Gabriel from EPFL who is now working at a French institution. Let me know if I should reach out to ask more details.

@unkcpz
Copy link
Member

unkcpz commented Feb 20, 2025

Thanks @mbercx. I think soon @agoscinski will have something work and ready for test, we can reach out then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants