Skip to content

CLOUDSTACK-5822: keep user-added sshkeys in authorized_keys#1044

Merged
asfgit merged 1 commit into
apache:masterfrom
ustcweizhou:keep-sshkey
Jan 27, 2016
Merged

CLOUDSTACK-5822: keep user-added sshkeys in authorized_keys#1044
asfgit merged 1 commit into
apache:masterfrom
ustcweizhou:keep-sshkey

Conversation

@ustcweizhou
Copy link
Copy Markdown
Contributor

For now, if we add the ssh key inside the vm (not on cloudstack UI), the sshkey will be removed if we reset the sshkey on cloudstack UI.

After this commit, the sshkey (added by cloudstack) will end with cloudstack@apache.org.
We will only control the sshkeys with cloudstack@apache.org.

This will be used for multiple sshkey support for vm in the future.

@DaanHoogland
Copy link
Copy Markdown
Contributor

makes sense and looks good. @ustcweizhou I assume you have tested this in production already, no?

@NuxRo
Copy link
Copy Markdown
Contributor

NuxRo commented Nov 7, 2015

Makes sense and looks good, but I am surprised you are still using these scripts instead of cloud-init.
On the same note, worth having a look at how cloud-init behaves, I'll give it a try when I get some time.

@wido
Copy link
Copy Markdown
Contributor

wido commented Nov 9, 2015

Looks good, sane commit.

I agree with @NuxRo that cloud-init is imho the way forward. We probably want to ditch these legacy scripts at some point.

@wilderrodrigues
Copy link
Copy Markdown
Contributor

@ustcweizhou

How did you test it?

Ping @remibergsma @DaanHoogland @ustcweizhou @karuturi @borisroman @Runseb @wido @NuxRo @bhaisaab @miguelaferreira

I think we should stick to a LGTM being given only if tests have been done and steps, on how to test, have been made clear.

Cheers,
Wilder

@NuxRo
Copy link
Copy Markdown
Contributor

NuxRo commented Nov 11, 2015

On a second thought - and something worth pondering on - this could have some security implications.

Imagine you have a private cloud, a developer/employee leaves and you want to remove his key from the instances because "security". People used to the old behaviour might think they're safe when they are in fact not.
Thoughts?

Now, multi-key support, that'd be terrific. :-)

@DaanHoogland
Copy link
Copy Markdown
Contributor

@NuxRo valid point but isn't this unexpected behavior instead of expected? The key was not added by the UI (or API) but will be removed by it. If we need this a seperate API, resetAllSshKeysInVm should be made.

An angry employee having keys on a vm (out of band) is a real and present danger, indeed.

@NuxRo
Copy link
Copy Markdown
Contributor

NuxRo commented Nov 11, 2015

Good point as well with the "unexpected". I definitely see where Wei is coming from, but I think it could be misleading. Perhaps a better way to do this is mark the ACS key and only reset that one. i.e.
When we add the key append a "# added by Cloudstack" and when we issue a reset, just delete that one.

Am I overcomplicating this? I might be, especially as these scripts are being slowly phased out.

@DaanHoogland
Copy link
Copy Markdown
Contributor

@NuxRo @ustcweizhou is using "cloudstack@apache.org$" for this.

@NuxRo
Copy link
Copy Markdown
Contributor

NuxRo commented Nov 11, 2015

Ah, right, checking the code is important. :-D
I'll go back to my corner.

@DaanHoogland
Copy link
Copy Markdown
Contributor

regression tests executed:
1044.network.results.txt
1044.vpc.results.txt
LGTM

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jan 27, 2016

LGTM
Merging based on 2+LGTMs, test results shared in comments

@asfgit asfgit merged commit 64ef4fa into apache:master Jan 27, 2016
asfgit pushed a commit that referenced this pull request Jan 27, 2016
CLOUDSTACK-5822: keep user-added sshkeys in authorized_keysFor now, if we add the ssh key inside the vm (not on cloudstack UI), the sshkey will be removed if we reset the sshkey on cloudstack UI.

After this commit, the sshkey (added by cloudstack) will end with cloudstack@apache.org.
We will only control the sshkeys with cloudstack@apache.org.

This will be used for multiple sshkey support for vm in the future.

* pr/1044:
  CLOUDSTACK-5822: keep user-added sshkeys in authorized_keys

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants