CLOUDSTACK-10381: Fix password reset / reset ssh key with ConfigDrive#2705
CLOUDSTACK-10381: Fix password reset / reset ssh key with ConfigDrive#2705PaulAngus merged 1 commit intoapache:4.11apache/cloudstack:4.11from nuagenetworks:bugfix/CLOUDSTACK-10381nuagenetworks/cloudstack:bugfix/CLOUDSTACK-10381Copy head branch name to clipboard
Conversation
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
| final boolean canHandle = canHandle(network.getTrafficType()); | ||
|
|
||
| if (canHandle) { | ||
| storePasswordInVmDetails(vm); |
There was a problem hiding this comment.
are we storing passwd on saveSSHKey, @fmaximus ?
There was a problem hiding this comment.
I may be wrong, but when I tested it I could see different passwords in the isos before shutdown and post-reset password and startup of the VM @fmaximus. Have you tested this against a normal config drive enabled network such as a L2 network?
There was a problem hiding this comment.
SaveSshKey implicitly also resets the password. So I'm also saving it in this case.
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2111 |
|
From what i read, it looks to me like you're using the standard encryption algorithm to store the users' password - I would be quite strongly opposed to storing these passwords (and i think that people would have compliance issues) outside of a proper 'vault'. I'd also ask if the password is removed from the config drive once a VM has booted. |
| /** | ||
| * Store password in vm details so it can be picked up during VM start. | ||
| */ | ||
| private void storePasswordInVmDetails(VirtualMachineProfile vm) { |
There was a problem hiding this comment.
Would you mind creating a unit test for this method?
| final boolean canHandle = canHandle(network.getTrafficType()); | ||
|
|
||
| if (canHandle) { | ||
| storePasswordInVmDetails(vm); |
There was a problem hiding this comment.
I may be wrong, but when I tested it I could see different passwords in the isos before shutdown and post-reset password and startup of the VM @fmaximus. Have you tested this against a normal config drive enabled network such as a L2 network?
|
I've used the same code as is used in VRElement, when VR is stopped. We've looked at removing the password from the config drive, it soon becomes complex.
|
|
@fmaximus yes, it is a requirement that VM is stopped for doing password reset and this also ensures that a new ISO will be built when VM is started (if config drive is enabled/available). Per the apidocs, similar to the reset SSH key API I've added a check that will not allow reset password API to work if VM is not stopped. /cc @PaulAngus @DaanHoogland |
yadvr
left a comment
There was a problem hiding this comment.
LGTM, if password is not saved/seen in the config drive post-reset API and start of VM.
|
LGTM - tested by; |
|
@rhtyd you approve and say LGTM but then say "if..." effectively -1ing the PR. the password will remain available in the configdrive until it is rereated, won't it? @fmaximus am I seeing this wrong? |
Description
Let ConfigDriveElement remember the new password,
so it can be put on the ConfigDrive iso while vm is starting.
Types of changes
How Has This Been Tested?
Checklist:
Testing