Dev: utils: Avoid hardcoding the ssh key type as RSA #1600

liangxin1300 · 2024-11-01T02:04:04Z

Problem

The join process will fail if the existing key type is not RSA. See: crmsh has issue with the default ed25519 ssh key #1504 (comment)
If the init and join nodes are already set up for passwordless access using an ed25519 key, an RSA key pair is still generated during the init/join process.

Changes include:

Avoid hardcoding the ssh key type as RSA
Introduced a new function ssh_key.fetch_public_key_list to fetch public keys from local or remote, return as public key path list or public key content list
In KeyFileManager, use class variable to store the key type instead of hardcoding it as RSA
In bootstrap.swap_public_ssh_key function, Change the parameter name from 'add' to 'generate_key_on_remote' to make it more descriptive
In the process of join_ssh, no need to set 'generate_key_on_remote' to True, as the key is already generated on init node
Improve shell script in generate_ssh_key_pair_on_remote to avoid hardcoding the key type as RSA
Remove unused code

codecov · 2024-11-01T02:57:00Z

Codecov Report

Attention: Patch coverage is 73.17073% with 11 lines in your changes missing coverage. Please review.

Project coverage is 69.24%. Comparing base (917b0d9) to head (02ad588).
Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
crmsh/ssh_key.py	60.86%	9 Missing ⚠️
crmsh/utils.py	33.33%	2 Missing ⚠️

Additional details and impacted files

Flag	Coverage Δ
integration	`54.48% <73.17%> (-0.03%)`	⬇️
unit	`52.12% <29.26%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
crmsh/bootstrap.py	`88.63% <100.00%> (-0.04%)`	⬇️
crmsh/utils.py	`66.54% <33.33%> (-0.16%)`	⬇️
crmsh/ssh_key.py	`76.66% <60.86%> (+2.29%)`	⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

In utils.ssh_copy_id_no_raise, the ssh key type is hardcoded as RSA. Then the join process will fail if the existing key type is not RSA. Also see: ClusterLabs#1504 (comment)

In KeyFileManager, use class variable to store the key type instead of hardcoding it as RSA.

Replace remote_public_key_from as ssh_key.fetch_public_key_list

…tion - Change the parameter name from 'add' to 'generate_key_on_remote', which is more descriptive. - In the process of join_ssh, no need to set 'generate_key_on_remote' to True, as the init node already has the public key.

And remove unused code.

to avoid hardcoding the ssh key type as rsa.

nicholasyang2022 · 2024-11-05T03:51:23Z

crmsh/ssh_key.py

+    :param with_content: whether to return the content of the public key files,
+     default is False
+
+    :return: a list of public key files if with_content is False, otherwise a list of public key strings


It is a bad idea to return vaules in a same type with different meanings. Please use Key in function signature and use KeyFile and InMemoryKey correspondingly. Or separate these 2 different usage into 2 functions.

typing.List[str] is deprecated. Use list[str] instead.

nicholasyang2022 · 2024-11-06T09:29:03Z

I got a empty line of WARNING: when runing crm cluster join:

suse@ha-2-1:~> sudo crm cluster init -y -N suse@ha-2-2
......

suse@ha-2-3:~> sudo crm cluster join -c suse@ha-2-2 -y
WARNING: chronyd.service is not configured to start at system boot.
INFO: A new ssh keypair is generated for user suse.
INFO: Configuring SSH passwordless with suse@ha-2-2
(suse@ha-2-2) Password: 
WARNING: 
INFO: A new ssh keypair is generated for user hacluster.
INFO: Configuring SSH passwordless with suse@ha-2-1
(suse@ha-2-1) Password: 
INFO: Configuring csync2
INFO: Starting csync2.socket service
INFO: BEGIN csync2 syncing files in cluster
INFO: END csync2 syncing files in cluster
INFO: Merging known_hosts
Warning: Permanently added 'ha-2-3' (ED25519) to the list of known hosts.
INFO: BEGIN Probing for new partitions
INFO: END Probing for new partitions

WARNING: Hawk not installed - not configuring web management interface.
WARNING: You should change the hacluster password to something more secure!
INFO: BEGIN Waiting for cluster
..                                                                                                                                                                                                        INFO: END Waiting for cluster
WARNING: "priority" in rsc_defaults is set to 0, it was 1
INFO: BEGIN Reloading cluster configuration
INFO: END Reloading cluster configuration
INFO: Done (log saved to /var/log/crmsh/crmsh.log on ha-2-3)

liangxin1300 · 2024-11-07T07:58:12Z

I got a empty line of WARNING: when runing crm cluster join:

suse@ha-2-1:~> sudo crm cluster init -y -N suse@ha-2-2
......

suse@ha-2-3:~> sudo crm cluster join -c suse@ha-2-2 -y
WARNING: chronyd.service is not configured to start at system boot.
INFO: A new ssh keypair is generated for user suse.
INFO: Configuring SSH passwordless with suse@ha-2-2
(suse@ha-2-2) Password: 
WARNING: 
INFO: A new ssh keypair is generated for user hacluster.
INFO: Configuring SSH passwordless with suse@ha-2-1
(suse@ha-2-1) Password: 
INFO: Configuring csync2
INFO: Starting csync2.socket service
INFO: BEGIN csync2 syncing files in cluster
INFO: END csync2 syncing files in cluster
INFO: Merging known_hosts
Warning: Permanently added 'ha-2-3' (ED25519) to the list of known hosts.
INFO: BEGIN Probing for new partitions
INFO: END Probing for new partitions

WARNING: Hawk not installed - not configuring web management interface.
WARNING: You should change the hacluster password to something more secure!
INFO: BEGIN Waiting for cluster
..                                                                                                                                                                                                        INFO: END Waiting for cluster
WARNING: "priority" in rsc_defaults is set to 0, it was 1
INFO: BEGIN Reloading cluster configuration
INFO: END Reloading cluster configuration
INFO: Done (log saved to /var/log/crmsh/crmsh.log on ha-2-3)

Can't reproduce for me, can you?

suse@alp-1:~> sudo crm cluster init -y -N suse@alp-2
...
suse@alp-3:~> sudo crm cluster join -c suse@alp-2 -y
INFO: A new ssh keypair is generated for user suse.
INFO: Configuring SSH passwordless with suse@alp-2
suse@alp-2's password: 
INFO: A new ssh keypair is generated for user hacluster.
INFO: Configuring SSH passwordless with suse@alp-1
suse@alp-1's password: 
INFO: Configuring csync2

- fetch_public_key_file_list: to fetch public key file list - fetch_public_key_content_list: to fetch public key content list Since for the original fetch_public_key_list, it is a bad idea to return vaules in a same type with different meanings.

nicholasyang2022 · 2024-11-12T05:22:41Z

I got UserNotFoundError from fetch_public_key_content_list. Backtrace:

> /usr/lib64/python3.11/logging/__init__.py(1500)warning()
-> if self.isEnabledFor(WARNING):
(Pdb) bt
  /usr/sbin/crm(55)<module>()
-> rc = main.run()
  /usr/lib/python3.11/site-packages/crmsh/main.py(369)run()
-> return main_input_loop(context, user_args)
  /usr/lib/python3.11/site-packages/crmsh/main.py(246)main_input_loop()
-> rc = handle_noninteractive_use(context, user_args)
  /usr/lib/python3.11/site-packages/crmsh/main.py(203)handle_noninteractive_use()
-> if context.run(' '.join(l)):
  /usr/lib/python3.11/site-packages/crmsh/ui_context.py(93)run()
-> rv = self.execute_command() is not False
  /usr/lib/python3.11/site-packages/crmsh/ui_context.py(278)execute_command()
-> rv = self.command_info.function(*arglist)
  /usr/lib/python3.11/site-packages/crmsh/ui_cluster.py(503)do_join()
-> bootstrap.bootstrap_join(join_context)
  /usr/lib/python3.11/site-packages/crmsh/bootstrap.py(2276)bootstrap_join()
-> join_ssh(cluster_node, remote_user)
  /usr/lib/python3.11/site-packages/crmsh/bootstrap.py(1594)join_ssh()
-> return join_ssh_impl(local_user, seed_host, seed_user, keys)
  /usr/lib/python3.11/site-packages/crmsh/bootstrap.py(1618)join_ssh_impl()
-> swap_public_ssh_key(seed_host, local_user, seed_user, local_user, seed_user)
  /usr/lib/python3.11/site-packages/crmsh/bootstrap.py(1696)swap_public_ssh_key()
-> logger.warning(e)
> /usr/lib64/python3.11/logging/__init__.py(1500)warning()
-> if self.isEnabledFor(WARNING):
(Pdb) c
WARNING:

liangxin1300 · 2024-11-13T02:49:10Z

I got UserNotFoundError from fetch_public_key_content_list. Backtrace:

> /usr/lib64/python3.11/logging/__init__.py(1500)warning()
-> if self.isEnabledFor(WARNING):
(Pdb) bt
  /usr/sbin/crm(55)<module>()
-> rc = main.run()
  /usr/lib/python3.11/site-packages/crmsh/main.py(369)run()
-> return main_input_loop(context, user_args)
  /usr/lib/python3.11/site-packages/crmsh/main.py(246)main_input_loop()
-> rc = handle_noninteractive_use(context, user_args)
  /usr/lib/python3.11/site-packages/crmsh/main.py(203)handle_noninteractive_use()
-> if context.run(' '.join(l)):
  /usr/lib/python3.11/site-packages/crmsh/ui_context.py(93)run()
-> rv = self.execute_command() is not False
  /usr/lib/python3.11/site-packages/crmsh/ui_context.py(278)execute_command()
-> rv = self.command_info.function(*arglist)
  /usr/lib/python3.11/site-packages/crmsh/ui_cluster.py(503)do_join()
-> bootstrap.bootstrap_join(join_context)
  /usr/lib/python3.11/site-packages/crmsh/bootstrap.py(2276)bootstrap_join()
-> join_ssh(cluster_node, remote_user)
  /usr/lib/python3.11/site-packages/crmsh/bootstrap.py(1594)join_ssh()
-> return join_ssh_impl(local_user, seed_host, seed_user, keys)
  /usr/lib/python3.11/site-packages/crmsh/bootstrap.py(1618)join_ssh_impl()
-> swap_public_ssh_key(seed_host, local_user, seed_user, local_user, seed_user)
  /usr/lib/python3.11/site-packages/crmsh/bootstrap.py(1696)swap_public_ssh_key()
-> logger.warning(e)
> /usr/lib64/python3.11/logging/__init__.py(1500)warning()
-> if self.isEnabledFor(WARNING):
(Pdb) c
WARNING:

Seems to have to revert removed remote_public_key_from function and detect ssh key type in shell command
As the original function use LocalShell, not cluster shell, should no above UserNotFoundError error

liangxin1300 force-pushed the 20241031_key_issue_1504 branch from 1d0afbe to 75a717f Compare November 1, 2024 02:36

liangxin1300 force-pushed the 20241031_key_issue_1504 branch from 75a717f to 095dd8a Compare November 1, 2024 03:26

liangxin1300 added 2 commits November 1, 2024 20:31

Dev: utils: Avoid hardcoding the ssh key type as RSA

d60d6d4

In utils.ssh_copy_id_no_raise, the ssh key type is hardcoded as RSA. Then the join process will fail if the existing key type is not RSA. Also see: ClusterLabs#1504 (comment)

Dev: ssh_key: Avoid hardcoding the ssh key type as RSA

f4700eb

In KeyFileManager, use class variable to store the key type instead of hardcoding it as RSA.

liangxin1300 force-pushed the 20241031_key_issue_1504 branch 4 times, most recently from 0ea9007 to 1249b0b Compare November 4, 2024 06:09

liangxin1300 added 4 commits November 4, 2024 15:27

Dev: bootstrap: Avoid hardcoding the ssh key type as RSA

3299d1f

Replace remote_public_key_from as ssh_key.fetch_public_key_list

Dev: bootstrap: Adjust the docstring of configure_ssh_key function

d9b36d7

Dev: bootstrap: Reuse AuthorizedKeyManager to add key to authorized_keys

cf92da5

And remove unused code.

liangxin1300 force-pushed the 20241031_key_issue_1504 branch 2 times, most recently from 28aca1d to 919b465 Compare November 4, 2024 07:55

Dev: bootstrap: Improve shell script in generate_ssh_key_pair_on_remote

8ac3021

to avoid hardcoding the ssh key type as rsa.

liangxin1300 force-pushed the 20241031_key_issue_1504 branch from 919b465 to 8ac3021 Compare November 4, 2024 11:14

liangxin1300 marked this pull request as ready for review November 4, 2024 14:17

liangxin1300 requested a review from nicholasyang2022 November 4, 2024 14:17

nicholasyang2022 reviewed Nov 5, 2024

View reviewed changes

liangxin1300 added 2 commits November 8, 2024 11:31

Dev: unittests: Adjust unit test for previous commits

02ad588

liangxin1300 force-pushed the 20241031_key_issue_1504 branch from 4db86e4 to 02ad588 Compare November 8, 2024 03:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev: utils: Avoid hardcoding the ssh key type as RSA #1600

Dev: utils: Avoid hardcoding the ssh key type as RSA #1600

liangxin1300 commented Nov 1, 2024 •

edited

Loading

codecov bot commented Nov 1, 2024 •

edited

Loading

nicholasyang2022 Nov 5, 2024

nicholasyang2022 commented Nov 6, 2024

liangxin1300 commented Nov 7, 2024

nicholasyang2022 commented Nov 12, 2024

liangxin1300 commented Nov 13, 2024

Dev: utils: Avoid hardcoding the ssh key type as RSA #1600

Are you sure you want to change the base?

Dev: utils: Avoid hardcoding the ssh key type as RSA #1600

Conversation

liangxin1300 commented Nov 1, 2024 • edited Loading

Problem

Changes include:

codecov bot commented Nov 1, 2024 • edited Loading

Codecov Report

nicholasyang2022 Nov 5, 2024

Choose a reason for hiding this comment

nicholasyang2022 commented Nov 6, 2024

liangxin1300 commented Nov 7, 2024

nicholasyang2022 commented Nov 12, 2024

liangxin1300 commented Nov 13, 2024

liangxin1300 commented Nov 1, 2024 •

edited

Loading

codecov bot commented Nov 1, 2024 •

edited

Loading