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

Set emergency login for core user with random password #3755

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

praveenkumar
Copy link
Member

This PR allow user to set the emergency login for core user with a random generated password. It is helpful when user lost the ssh access and use VMM to debug the issue which require password for core user.

Generated password is printed on stdout if user use cli to start the VM and also part of ~/.crc/crc.log in case want to access it later. In case of VM is started by api endpoint then password would be part of ~/.crc.crcd.log.

$ crc config set enable-emergency-login true
$ crc config view
- enable-emergency-login                : true
[...]
$ crc start
[...]
INFO CRC VM is running
INFO Emergency login password for core user is: Yf9OwP7y
INFO Updating authorized keys...
[...]

@openshift-ci openshift-ci bot requested review from cfergeau and gbraad July 13, 2023 08:20
@praveenkumar
Copy link
Member Author

This superseed #3689 and incorporate the suggestion in that PR.

@gbraad
Copy link
Contributor

gbraad commented Jul 13, 2023

In case of VM is started by api endpoint then password would be part of ~/.crc.crcd.log.

Wouldn't this 'rotate' away ?
In case of desktop usage, you will never see this prompt. And the likelihood logs rotate this away before you need it is quite high

better leave it in a dedicated file or part of the instance crc.json

for i := range b {
b[i] = charset[rand.Intn(len(charset))] //nolint
}
logging.Infof("Emergency login password for core user is: %s", b)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will disappear from the log file when it is gets 'cleaned'/rotated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gbraad updated.

@praveenkumar praveenkumar force-pushed the pr/3689 branch 2 times, most recently from 938e8bd to 0c5331c Compare July 13, 2023 10:00
for i := range b {
b[i] = charset[rand.Intn(len(charset))] //nolint
}
if err := os.WriteFile(constants.PasswdFilePath, b, 0600); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove on stop and when the setting has been disabled

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gbraad On the stop we shouldnt' remove it otherwise we need to also remove core user password on stop, IMO we just need to delete it during crc delete not crc stop action.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it get a new password on restart?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reiterated and now when stop => start happen and user unset enable-emergency-login setting then core user password is locked and from shell not able to login.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't regenerate the ssh key on start/stop/start, I don't think the user password needs to behave differently, so as long as enable-emergency-login is set, we can keep the same password until the VM is deleted.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't think this should happen, but it is the behaviour that existed in the PR: "wouldn't it get a new password on restart?" was not something I implied being happy with, but rather an observation of the current suggestion.

}

func lockCoreUserPasswd(sshRunner *crcssh.Runner) error {
_, _, err := sshRunner.RunPrivileged("delete core user password", "passwd", "--lock", "core")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd name this disableEmergencyLogin for symmetry with enableEmergencyLogin, and the RunPrivileged string could be disable core user password

if err := lockCoreUserPasswd(sshRunner); err != nil {
return nil, errors.Wrap(err, "Error deleting the password for core user")
}
// VM started and SSH available, so we can enable the emergency login
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this comment is needed.

@@ -417,6 +418,16 @@ func (client *client) Start(ctx context.Context, startConfig types.StartConfig)
}
logging.Info("CRC VM is running")

if err := lockCoreUserPasswd(sshRunner); err != nil {
return nil, errors.Wrap(err, "Error deleting the password for core user")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be moved to an else in if startConfig.EmergencyLogin { as both operations exclusive

Copy link
Contributor

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from da66c4e#r1277594992 this looks good to me.

This PR allow user to set the emergency login for core user with a
random generated password. It is helpful when user lost the ssh access
and use VMM to debug the issue which require password for `core` user.

Generated password is stored in `~/.crc/machine/crc/passwd` file which
user can consume in case of debugging. When user unset the
`enable-emergency-login` setting then password is locked for `core`
user.

```
$ crc config set enable-emergency-login true
$ crc config view
- enable-emergency-login                : true
[...]
$ crc start
[...]
INFO CRC VM is running
INFO Emergency login password for core user is stored to /home/prkumar/.crc/machines/crc/passwd
INFO Updating authorized keys...
[...]
```
@openshift-ci
Copy link

openshift-ci bot commented Aug 7, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from cfergeau. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link

openshift-ci bot commented Aug 7, 2023

@praveenkumar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/integration-crc f4b61ec link true /test integration-crc

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@praveenkumar
Copy link
Member Author

/retest

@praveenkumar praveenkumar merged commit c7dca60 into crc-org:main Aug 9, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants