Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vic-machine debug --rootpw enables SSH #6402

Open
stuclem opened this issue Sep 21, 2017 · 2 comments
Open

vic-machine debug --rootpw enables SSH #6402

stuclem opened this issue Sep 21, 2017 · 2 comments
Labels
area/security Management of security functionality and other issues that impact security component/install kind/defect Behavior that is inconsistent with what's intended priority/p2 team/lifecycle triage/proposed-1.5

Comments

@stuclem
Copy link
Contributor

stuclem commented Sep 21, 2017

@stuclem commented on Wed Sep 06 2017

From Slack:

Eduardo Meirelles [7:47 PM]
BTW… not sure if you guys have a chance to look at --rootpw behavior…when I ran debug --rootpw it also enabled SSH access, even though I did not specify --enable-ssh option.

Matt Williamson [8:00 PM]
i just confirmed that using the rootpw flag in 1.1.1 also enabled ssh access.

[8:00]
the docs should be updated to reflect that rootpw enables SSH AND changes the default password

[8:00]
where enable_ssh turns it on with the default password


@stuclem commented on Mon Sep 18 2017

@mdubya66 and @emeirell if I remember correctly the discussions that I had with @hickeng at the time that he added these options, this is actually a bug in the implementation of --rootpw.

I believe that the intention is for vic-machine debug to work as documented, i.e. --rootpw activates Shell access only, and then if used in combination with --enable-ssh, it enables shell and SSH access, using the same password. I can't fully remember why this separation was necessary, but @hickeng and I did go around the houses a few times when I wrote up these topics (with substantial contribution from @hickeng, IIRC).

Of course, even if the docs do present the desired behaviour, they do not present the actual behaviour. So, we have two options:

  • Make the current behaviour the official behaviour and rewrite the docs accordingly.
  • Fix the implementation of --rootpw in the product, and in the meantime, document the fact that --rootpw also enables SSH as a Known Issue in the release notes.

Which do you prefer?


@emeirell commented on Mon Sep 18 2017

Fixing the implementation of --rootpw seems the right thing to do.
This security granularity control is greatly appreciated within customers.


@stuclem commented on Thu Sep 21 2017

In the meantime, I'll add this as a Known Issue in the release notes.


@stuclem commented on Thu Sep 21 2017

Moving this to the vic repo, as this is an engineering issue rather than a doc issue. I did not find an existing issue about --rootpw enabling SSH.

@stuclem stuclem changed the title Document that debug --rootpw enables SSH and changes the default password vic-machine create --rootpw enables SSH Sep 21, 2017
@stuclem stuclem added component/install kind/defect Behavior that is inconsistent with what's intended impact/doc/note Requires creation of or changes to an official release note labels Sep 21, 2017
@hickeng hickeng added area/security Management of security functionality and other issues that impact security team/lifecycle priority/p2 labels Oct 10, 2017
@hickeng
Copy link
Member

hickeng commented Oct 10, 2017

Concur - we should just fix the behaviour to work as described. The conflation of ssh with setting of password appears to be in the Dispatcher call for DebugVCH (lib/install/management/debug.go).

We should also update cmd/vic-init/toolbox.go to use the recently added LaunchUtility instead of working around the lack of exitCodes in the presence of a child reaper.

@stuclem stuclem changed the title vic-machine create --rootpw enables SSH vic-machine debug --rootpw enables SSH Oct 16, 2017
@stuclem
Copy link
Contributor Author

stuclem commented Oct 16, 2017

Added to https://github.com/vmware/vic/releases/tag/v1.2.1#knownissues. Leaving the doc as they are currently, as they describe the correct behavior.

@stuclem stuclem removed the impact/doc/note Requires creation of or changes to an official release note label Oct 16, 2017
@zjs zjs added status/needs-estimation The issue needs to be estimated by the team triage/proposed-1.4 labels Dec 1, 2017
@andrewtchin andrewtchin removed the status/needs-estimation The issue needs to be estimated by the team label Dec 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security Management of security functionality and other issues that impact security component/install kind/defect Behavior that is inconsistent with what's intended priority/p2 team/lifecycle triage/proposed-1.5
Projects
None yet
Development

No branches or pull requests

4 participants