cephadm: Allow users to provide ssh keys during bootstrap by ricardoasmarques · Pull Request #35195 · ceph/ceph

ricardoasmarques · 2020-05-22T09:12:27Z

With this PR user will be able to provide their own SSH keys:

ceph cephadm set-priv-key -i /root/.ssh/secret
ceph cephadm set-pub-key -i /root/.ssh/secret.pub

Even for the bootstrap:

cephadm boostrap \
    ...
    --ssh-private-key /root/.ssh/secret \
    --ssh-public-key /root/.ssh/secret.pub

Fixes: https://tracker.ceph.com/issues/45629

Signed-off-by: Ricardo Marques rimarques@suse.com

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard backend
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

ricardoasmarques · 2020-05-22T15:28:41Z

@sebastian-philipp Can we assume that cephadm only works with keys that are not protected with passwords?

If yes, then we can have a single command ceph cephadm set-key <private_key> and extract the public key from the private key by doing something like ssh-keygen -y -f <private_key> internally.

Or maybe it's better to explicitly set both anyway, thoughts?

epuertat · 2020-05-25T08:04:56Z

src/cephadm/cephadm

+            mounts = {}
+            mounts[pathify(args.ssh_private_key)] = '/tmp/cephadm-ssh-key:z'
+            mounts[pathify(args.ssh_public_key)] = '/tmp/cephadm-ssh-key.pub:z'


Suggested change

mounts = {}

mounts[pathify(args.ssh_private_key)] = '/tmp/cephadm-ssh-key:z'

mounts[pathify(args.ssh_public_key)] = '/tmp/cephadm-ssh-key.pub:z'

mounts = {

pathify(args.ssh_private_key): '/tmp/cephadm-ssh-key:z'

pathify(args.ssh_public_key): '/tmp/cephadm-ssh-key.pub:z'

}

epuertat · 2020-05-25T08:15:53Z

src/cephadm/cephadm

+    parser_bootstrap.add_argument(
+        '--ssh-private-key',
+        help='SSH private key')
+    parser_bootstrap.add_argument(
+        '--ssh-public-key',
+        help='SSH public key')


?

Suggested change

parser_bootstrap.add_argument(

'--ssh-private-key',

help='SSH private key')

parser_bootstrap.add_argument(

'--ssh-public-key',

help='SSH public key')

parser_bootstrap.add_argument(

'--ssh-private-key',

type=argparse.FileType('r'),

help='SSH private key')

parser_bootstrap.add_argument(

'--ssh-public-key',

type=argparse.FileType('r'),

help='SSH public key')

FileType also checks whether the file exists, is readable and can be opened, and opens these files, so the filename will be stored in args.ssh_*_key.name.

epuertat · 2020-05-25T09:32:09Z

src/pybind/mgr/cephadm/module.py

+    def _set_priv_key(self, inbuf=None):
+        if inbuf is None or len(inbuf) == 0:
+            return -errno.EINVAL, "", "empty private ssh key provided"
+        self.set_store("ssh_identity_key", inbuf)


Is there any plan to remediate this? Storing private SSH keys in the clear (Is this somehow printed to the logs? Is this mon key-value store encrypted?) sounds like a serious attack vector: Dashboard's CVE-2020-1699 (or any other user-facing component breach) would expose the filesystem under ceph user, but with this data there everything else would be compromised, right?

@epuertat AFAIK, it's not possible to encrypt "config-keys". Note that this security issue is not being introduced in this PR, it's already present for the "generated SSH key" - https://github.com/ceph/ceph/blob/master/src/pybind/mgr/cephadm/module.py#L745

@sebastian-philipp was this security issue already discussed? Should we have a separate issue for that?

FYI, a similar discussion happened here ceph/ceph-iscsi#173 (comment)

Thanks for the context, @ricardoasmarques!

I think, we need a proper and secure way to store secrets in the MONs. config-key and config are both not designed to store passwords etc. @jecluis any opinion? @travisn maybe something in the direction of k8s secrets might be an idea?

@sebastian-philipp Rook certainly relies on K8s secrets, but not sure how cephadm could leverage something similar.

sebastian-philipp · 2020-05-26T14:53:15Z

can we also set the ssh config?

epuertat

Thanks for addressing my comments @ricardoasmarques !

ricardoasmarques · 2020-05-27T08:26:05Z

can we also set the ssh config?

@sebastian-philipp I've pushed a new commit that will alow users to provide ssh config during bootstrap (--ssh-config option).

sebastian-philipp · 2020-05-27T08:31:08Z

Big thanks! Do you also want to amend the man page? https://docs.ceph.com/docs/master/man/8/cephadm/

Fixes: https://tracker.ceph.com/issues/45629 Signed-off-by: Ricardo Marques <rimarques@suse.com>

Custom ssh config can be provided by using the '--ssh-config' option Signed-off-by: Ricardo Marques <rimarques@suse.com>

ricardoasmarques · 2020-05-27T11:02:54Z

Big thanks! Do you also want to amend the man page? https://docs.ceph.com/docs/master/man/8/cephadm/

@sebastian-philipp Good catch. Man page is now updated.

sebastian-philipp · 2020-05-27T16:00:39Z

QA passed: http://pulpito.ceph.com/swagner-2020-05-27_13:56:17-rados:cephadm-wip-swagner-testing-2020-05-27-1352-distro-basic-smithi/

ricardoasmarques added the cephadm label May 22, 2020

ricardoasmarques requested a review from sebastian-philipp May 22, 2020 09:12

ricardoasmarques requested a review from a team as a code owner May 22, 2020 09:12

epuertat reviewed May 25, 2020

View reviewed changes

This was referenced May 25, 2020

cephadm: Validate bootstrap "--dashboard-{key|crt}" path #35229

Merged

Rely on "bootstrap" to configure MGR module (and more...) ceph/ceph-salt#250

Closed

ricardoasmarques force-pushed the allow-specify-ssh branch from be01589 to 3d36ebe Compare May 26, 2020 14:08

epuertat approved these changes May 26, 2020

View reviewed changes

sebastian-philipp added the needs-qa label May 27, 2020

ricardoasmarques added 2 commits May 27, 2020 11:57

cephadm: Allow users to provide ssh keys during bootstrap

ad75937

Fixes: https://tracker.ceph.com/issues/45629 Signed-off-by: Ricardo Marques <rimarques@suse.com>

cephadm: Alow users to provide ssh config during bootstrap

84c390f

Custom ssh config can be provided by using the '--ssh-config' option Signed-off-by: Ricardo Marques <rimarques@suse.com>

ricardoasmarques force-pushed the allow-specify-ssh branch from c1ac09f to 84c390f Compare May 27, 2020 11:01

sebastian-philipp added the wip-swagner-testing My Teuthology tests label May 27, 2020

sebastian-philipp merged commit 571b868 into ceph:master May 27, 2020

Daniel-Pivonka mentioned this pull request May 28, 2020

mgr/cephadm: cephadm bootstrap add --apply-spec <cluster.yaml> #34879

Merged

sebastian-philipp mentioned this pull request Jun 2, 2020

octopus: cephadm: batch backport June (1) #35347

Merged

This was referenced Jun 15, 2020

Use ceph orch status as check for configuring ssh orchestrator ceph/ceph-salt#267

Closed

Rely on "bootstrap" to configure MGR module ceph/ceph-salt#270

Merged

Conversation

ricardoasmarques commented May 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

ricardoasmarques commented May 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebastian-philipp commented May 26, 2020

Uh oh!

epuertat left a comment

Choose a reason for hiding this comment

Uh oh!

ricardoasmarques commented May 27, 2020

Uh oh!

sebastian-philipp commented May 27, 2020

Uh oh!

ricardoasmarques commented May 27, 2020

Uh oh!

sebastian-philipp commented May 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ricardoasmarques commented May 22, 2020 •

edited

Loading

ricardoasmarques commented May 22, 2020 •

edited

Loading