Run hb_report under hacluster #742

liangxin1300 · 2021-02-05T14:22:11Z

Problem

It didn't work if run hb_report or crm cluster copy under hacluster user, which was required from Hawk

Solution

Change function bootstrap.configure_local_ssh_key and bootstrap.swap_public_ssh_key to accept non-root user parameter; That is, for common user, bootstrap process also setup password-less among cluster nodes
Assign /usr/bin/sh to hacluster's login shell, otherwise, it's impossible to run command remotely
On interactive mode, behavior about changing hacluster's login shell and setup ssh access will be asked for confirm
For runtime existing cluster, need to use ssh stage to setup password-less access for hacluster
- on init node, run crm cluster init ssh -y
- on join node, run crm cluster join ssh -c <init node> -y
For runtime existing cluster, /etc/corosync/corosync.conf and /etc/sysconfig/sbd might has 400 mod, which will cause Permission denied exceptions when running hb_report under hacluster, Dev: utils: change default file mod as 644 for str2file function #747 try to resolve this, but not works for runtime existing cluster, to avoid that, before running hb_report under hacluster:
- Run chmod 644 /etc/corosync/corosync.conf
- Run chmod 644 /etc/sysconfig/sbd

Test rpm link: https://build.opensuse.org/package/show/home:XinLiang:branches:crmsh_testing7/crmsh

MalloZup

seems good to me but not familiar with crmsh codebase

arbulu89

@liangxin1300 Added some comments about the code.
About the purpose of the changes, I cannot really judge without knowing what are the consequences of changing the hacluster user bash option

arbulu89 · 2021-02-09T08:10:53Z

crmsh/bootstrap.py

+    private_key = "{}/.ssh/id_rsa".format(home_dir)
+    public_key = "{}/.ssh/id_rsa.pub".format(home_dir)
+    authorized_file = "{}/.ssh/authorized_keys".format(home_dir)
+    return private_key, public_key, authorized_file


I wouldn't return a 3 elements list here. This is a potential issue if for some reason you check the return data, as you will need to change all the places where the method is used.
Better to return a dictionary or specific object, so you can use just the needed elements

yop valid point

Thanks @arbulu89 , already changed!

arbulu89 · 2021-02-09T08:11:22Z

crmsh/bootstrap.py

@@ -1134,22 +1133,59 @@ def init_ssh():
    Configure passwordless SSH.
    """
    utils.start_service("sshd.service", enable=True)
-    configure_local_ssh_key()
+    for user in ["root", "hacluster"]:


This ["root", "hacluster"] list looks something that should go in a constant, as it is repeated later

arbulu89 · 2021-02-09T08:14:34Z

crmsh/bootstrap.py

+    if not os.path.exists(private_key):
+        status("Generating SSH key for {}".format(user))
+        cmd = "ssh-keygen -q -f {} -C 'Cluster Internal on {}' -N ''".format(private_key, utils.this_node())
+        if user != "root":


Maybe adding the next code in a method is a good idea:

if user != "root": cmd = utils.add_su(cmd, user)

It is reused in different places

Already put it into add_su function

gao-yan

It generally looks good to me. Thanks for the nice work, @liangxin1300!

Only one nitpick.

crmsh/bootstrap.py

gao-yan · 2021-02-10T13:27:36Z

For runtime existing cluster, need to use ssh stage to setup password-less access for hacluster

on init node, run crm cluster init ssh -y

on join node, run crm cluster join ssh -c <init node> -y

Probably it could be a further improvement. To make it as friendly as possible, given that there's an existing cluster, it'd be great to only require running such an ssh stage on any of the nodes, which takes care of all the nodes.

For runtime existing cluster, /etc/corosync/corosync.conf and /etc/sysconfig/sbd might has 400 mod, which will cause Permission denied exceptions when running hb_report under hacluster, Dev: utils: change default file mod as 644 for str2file function #747 try to resolve this, but not works for runtime existing cluster, to avoid that, before running hb_report under hacluster:

Run chmod 644 /etc/corosync/corosync.conf

Run chmod 644 /etc/sysconfig/sbd

Also to make it as easy as possible for users, I'm thinking probably ssh stage running for an existing cluster should as well fix the permissions of any theses existing files?

liangxin1300 · 2021-02-17T14:11:54Z

For runtime existing cluster, need to use ssh stage to setup password-less access for hacluster

on init node, run crm cluster init ssh -y

on join node, run crm cluster join ssh -c <init node> -y

Probably it could be a further improvement. To make it as friendly as possible, given that there's an existing cluster, it'd be great to only require running such an ssh stage on any of the nodes, which takes care of all the nodes.

Good suggestion! Thanks @gao-yan

For runtime existing cluster, /etc/corosync/corosync.conf and /etc/sysconfig/sbd might has 400 mod, which will cause Permission denied exceptions when running hb_report under hacluster, Dev: utils: change default file mod as 644 for str2file function #747 try to resolve this, but not works for runtime existing cluster, to avoid that, before running hb_report under hacluster:

Run chmod 644 /etc/corosync/corosync.conf

Run chmod 644 /etc/sysconfig/sbd

Also to make it as easy as possible for users, I'm thinking probably ssh stage running for an existing cluster should as well fix the permissions of any theses existing files?

I agree that will be more convenient for this whole issue.
But ssh key and the permissions changing are two kinds of things for crmsh, is that proper to put them together in ssh stage?

gao-yan · 2021-02-17T14:33:47Z

For runtime existing cluster, /etc/corosync/corosync.conf and /etc/sysconfig/sbd might has 400 mod, which will cause Permission denied exceptions when running hb_report under hacluster, Dev: utils: change default file mod as 644 for str2file function #747 try to resolve this, but not works for runtime existing cluster, to avoid that, before running hb_report under hacluster:

Run chmod 644 /etc/corosync/corosync.conf

Run chmod 644 /etc/sysconfig/sbd

Also to make it as easy as possible for users, I'm thinking probably ssh stage running for an existing cluster should as well fix the permissions of any theses existing files?

I agree that will be more convenient for this whole issue.
But ssh key and the permissions changing are two kinds of things for crmsh, is that proper to put them together in ssh stage?

Technically, no :-) As we can see, the "breaking" changes of hawk will bring users much inconvenience requiring them to re-run ssh stage. I was thinking combing this into ssh stage would make their lives a bit easier :-) But of course, we could either add this step also into update note / TID ...

liangxin1300 · 2021-02-18T01:31:55Z

For runtime existing cluster, /etc/corosync/corosync.conf and /etc/sysconfig/sbd might has 400 mod, which will cause Permission denied exceptions when running hb_report under hacluster, Dev: utils: change default file mod as 644 for str2file function #747 try to resolve this, but not works for runtime existing cluster, to avoid that, before running hb_report under hacluster:

Run chmod 644 /etc/corosync/corosync.conf

Run chmod 644 /etc/sysconfig/sbd

Also to make it as easy as possible for users, I'm thinking probably ssh stage running for an existing cluster should as well fix the permissions of any theses existing files?

I agree that will be more convenient for this whole issue.
But ssh key and the permissions changing are two kinds of things for crmsh, is that proper to put them together in ssh stage?

Technically, no :-) As we can see, the "breaking" changes of hawk will bring users much inconvenience requiring them to re-run ssh stage. I was thinking combing this into ssh stage would make their lives a bit easier :-) But of course, we could either add this step also into update note / TID ...

@arbulu89 , How do you think about this?

liangxin1300 · 2021-02-18T01:36:29Z

For runtime existing cluster, need to use ssh stage to setup password-less access for hacluster

on init node, run crm cluster init ssh -y

on join node, run crm cluster join ssh -c <init node> -y

Probably it could be a further improvement. To make it as friendly as possible, given that there's an existing cluster, it'd be great to only require running such an ssh stage on any of the nodes, which takes care of all the nodes.

Good suggestion! Thanks @gao-yan

@gao-yan After estimating, I think this change might be big and might break compatibility and bring regression.
So I suggest keep this unchanged and try to clarify these steps in update note

arbulu89

@liangxin1300
I see correct the code changes. But I'm talking about python code itself.
As commented, I don't have that much context about the change, so I would wait until @gao-yan approval to move forward

arbulu89 · 2021-02-19T08:09:40Z

crmsh/bootstrap.py

+        return re.search("{}:.*:/sbin/nologin".format(user), f.read())
+
+
+def configure_local_ssh_key(user="root"):


I think this configure_local_ssh_key method has become too big and complex.
We could split the logic in different submethods

Thanks @arbulu89
I moved the change shell codes into change_user_shell function
And I will adjust UT codes later

, bsc#1179999; CVE-2021-3020, bsc#1180571)

…20-35459, bsc#1179999; CVE-2021-3020, bsc#1180571)

liangxin1300 · 2022-10-09T06:52:07Z

Loop #1033 here

liangxin1300 force-pushed the 20210205_hb_report branch 3 times, most recently from 3ce64f9 to 6e5b30e Compare February 7, 2021 08:27

liangxin1300 changed the title ~~[WIP] 20210205 hb report~~ Run hb_report under hacluster Feb 7, 2021

MalloZup reviewed Feb 8, 2021

View reviewed changes

liangxin1300 force-pushed the 20210205_hb_report branch from 1e7f618 to 4666e31 Compare February 8, 2021 15:42

liangxin1300 requested review from arbulu89 and gao-yan February 8, 2021 15:53

This was referenced Feb 9, 2021

[crmsh-4.1] Run hb_report under hacluster #745

Merged

[crmsh-3.0] Run hb_report under hacluster #746

Merged

liangxin1300 force-pushed the 20210205_hb_report branch from 4666e31 to c967970 Compare February 9, 2021 06:37

arbulu89 reviewed Feb 9, 2021

View reviewed changes

liangxin1300 force-pushed the 20210205_hb_report branch 2 times, most recently from c5acd09 to 01e3bbc Compare February 10, 2021 03:34

MalloZup approved these changes Feb 10, 2021

View reviewed changes

gao-yan requested changes Feb 10, 2021

View reviewed changes

crmsh/bootstrap.py Outdated Show resolved Hide resolved

This was referenced Feb 15, 2021

runas pure sle12 ClusterLabs/hawk#237

Merged

Hawk invoke sle15 ClusterLabs/hawk#238

Merged

liangxin1300 force-pushed the 20210205_hb_report branch from 7e96d03 to 3804c75 Compare February 17, 2021 14:13

arbulu89 approved these changes Feb 19, 2021

View reviewed changes

gao-yan approved these changes Feb 19, 2021

View reviewed changes

Fix: bootstrap: setup authorized ssh access for hacluster(CVE-2020-35459

c538024

, bsc#1179999; CVE-2021-3020, bsc#1180571)

liangxin1300 force-pushed the 20210205_hb_report branch from ef4f3c1 to 52c0352 Compare February 19, 2021 15:00

liangxin1300 added 2 commits February 19, 2021 23:06

Dev: unittest: add unit test for authorized ssh access for hacluster

1736c94

Fix: hb_report: walk through hb_report process under hacluster(CVE-20…

7f6f8d5

…20-35459, bsc#1179999; CVE-2021-3020, bsc#1180571)

liangxin1300 force-pushed the 20210205_hb_report branch from 52c0352 to 7f6f8d5 Compare February 19, 2021 15:06

liangxin1300 merged commit 5d1bf03 into ClusterLabs:master Feb 19, 2021

liangxin1300 mentioned this pull request Oct 9, 2022

Fix: ha-cluster-join fails to add nodes to a existing cluster after the version update (bsc#1201785) #1033

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run hb_report under hacluster #742

Run hb_report under hacluster #742

liangxin1300 commented Feb 5, 2021 •

edited

Loading

MalloZup left a comment

arbulu89 left a comment

arbulu89 Feb 9, 2021

MalloZup Feb 9, 2021

liangxin1300 Feb 10, 2021

arbulu89 Feb 9, 2021

liangxin1300 Feb 10, 2021

arbulu89 Feb 9, 2021

liangxin1300 Feb 10, 2021

gao-yan left a comment

gao-yan commented Feb 10, 2021 •

edited

Loading

liangxin1300 commented Feb 17, 2021

gao-yan commented Feb 17, 2021

liangxin1300 commented Feb 18, 2021

liangxin1300 commented Feb 18, 2021 •

edited

Loading

arbulu89 left a comment

arbulu89 Feb 19, 2021

liangxin1300 Feb 19, 2021

liangxin1300 commented Oct 9, 2022

		return re.search("{}:.*:/sbin/nologin".format(user), f.read())


		def configure_local_ssh_key(user="root"):

Run hb_report under hacluster #742

Run hb_report under hacluster #742

Conversation

liangxin1300 commented Feb 5, 2021 • edited Loading

Problem

Solution

MalloZup left a comment

Choose a reason for hiding this comment

arbulu89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gao-yan left a comment

Choose a reason for hiding this comment

gao-yan commented Feb 10, 2021 • edited Loading

liangxin1300 commented Feb 17, 2021

gao-yan commented Feb 17, 2021

liangxin1300 commented Feb 18, 2021

liangxin1300 commented Feb 18, 2021 • edited Loading

arbulu89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liangxin1300 commented Oct 9, 2022

liangxin1300 commented Feb 5, 2021 •

edited

Loading

gao-yan commented Feb 10, 2021 •

edited

Loading

liangxin1300 commented Feb 18, 2021 •

edited

Loading