SSH Cleanup (#370)

* feat: allow user input verification for install

Depending on the method of installing waveshell, it may be desired to
pop up a modal for user verification. This is a first pass at handling
these special cases. The focus is on installing while previously
connected and auto installing while connecting.

* chore: update mshell to waveshell in error msg

* fix: run waveshell remotely with chosen shell

This ensures that the appropriate shell is used to run the waveshell
command remotely. It hasn't made a difference in my experience but is
desired in order to match the local launch.

* chore: simplify command to run waveshell remotely

This change removes the extra check for a directory and just tries to
run the command instead. It pipes the usual error to null and prints an
init packet instead.

* fix: prevent wavesrv crash during bad connection

The waveshell launch can fail in two different ways. If it has a
recoverable failure, it will attempt to reinstall waveshell. If not, it
is supposed to print an error. The unrecoverable case was causing a
segfault due to a misnamed variable. This change corrects it.

* fix: correct auto install user input modal

The previous combination of flags to catch auto install did not work
properly. This corrects them.

* chore: add "s" to countdown for user input timer

Makes it clear that the countdown is seconds.

* fix: remove auto password entry for sudo remote

The auto password entry for sudo remotes printed an error that was not
in response to the user input. To avoid this confusion, it has been
removed entirely.

* feat: add auto focus to user input modal

This automatically moves the cursor to the text box when the modal pops
up.

* feat: handle enter/escape keys for password entry

The password modal previously had to have buttons clicked to close it.
This change allows the user to close it with whatever is bound to escape
and to submit with whatever is bound to enter.

* chore: update an any type to correct type

* fix: correct keyboard event type from last commit

* fix: check identity files are readable early

Previously, an invalid identity file would send a dummy signer if the
file didn't exist. This resulted in extra sign in attempts that have no
chance of success. This could cause someone to get locked out of a
connection because of too many failed attempts. By performing the check
early, we no longer have to make these extra attempts.

* fix: only check global known hosts as root

The root user should not be able to write to a local known_hosts file.
If it does, it risks overwriting the default global behavior for only
the root user. This problem would only occur if waveterm was launched as
root, but we should protect against it just in case.

* feat: add remote name for remote password prompt

This change clarifies the remote name for password and keyboard
interactive prompts. It displays a message that authentication has been
requested from <hostname>. It is not added to publickey passphrase since
those phrases are specific to the key and not the remote.

* revert "simplify cmd to run waveshell remotely"

This reverts commit 4e5eea51b6.
This commit is contained in:
Sylvie Crowe 2024-03-04 11:56:20 -08:00 committed by GitHub
parent 438d17b933
commit 50953839b1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 156 additions and 66 deletions

View File

@ -42,7 +42,7 @@ class PasswordField extends TextField {
}
render() {
const { decoration, className, placeholder, maxLength, label } = this.props;
const { decoration, className, placeholder, maxLength, label, autoFocus } = this.props;
const { focused, internalValue, error, passwordVisible } = this.state;
const inputValue = this.props.value ?? internalValue;
@ -55,6 +55,8 @@ class PasswordField extends TextField {
onChange: this.handleInputChange,
onFocus: this.handleFocus,
onBlur: this.handleBlur,
onKeyDown: this.props.onKeyDown,
autoFocus: autoFocus,
placeholder: placeholder,
maxLength: maxLength,
};

View File

@ -2,6 +2,7 @@ import * as React from "react";
import { GlobalModel } from "@/models";
import { Choose, When, If } from "tsx-control-statements/components";
import { Modal, PasswordField, Markdown } from "@/elements";
import { checkKeyPressed, adaptFromReactOrNativeKeyEvent } from "@/util/keyutil";
import "./userinput.less";
@ -9,7 +10,7 @@ export const UserInputModal = (userInputRequest: UserInputRequest) => {
const [responseText, setResponseText] = React.useState("");
const [countdown, setCountdown] = React.useState(Math.floor(userInputRequest.timeoutms / 1000));
const closeModal = React.useCallback(() => {
const handleSendCancel = React.useCallback(() => {
GlobalModel.sendUserInput({
type: "userinputresp",
requestid: userInputRequest.requestid,
@ -39,6 +40,19 @@ export const UserInputModal = (userInputRequest: UserInputRequest) => {
[userInputRequest]
);
function handleTextKeyDown(e: React.KeyboardEvent<HTMLInputElement>) {
let waveEvent = adaptFromReactOrNativeKeyEvent(e);
if (checkKeyPressed(waveEvent, "Enter")) {
e.preventDefault();
e.stopPropagation();
handleSendText();
} else if (checkKeyPressed(waveEvent, "Escape")) {
e.preventDefault();
e.stopPropagation();
handleSendCancel();
}
}
React.useEffect(() => {
let timeout: ReturnType<typeof setTimeout>;
if (countdown == 0) {
@ -55,7 +69,7 @@ export const UserInputModal = (userInputRequest: UserInputRequest) => {
return (
<Modal className="userinput-modal">
<Modal.Header onClose={closeModal} title={userInputRequest.title + ` (${countdown})`} />
<Modal.Header onClose={handleSendCancel} title={userInputRequest.title + ` (${countdown}s)`} />
<div className="wave-modal-body">
<div className="userinput-query">
<If condition={userInputRequest.markdown}>
@ -65,13 +79,19 @@ export const UserInputModal = (userInputRequest: UserInputRequest) => {
</div>
<Choose>
<When condition={userInputRequest.responsetype == "text"}>
<PasswordField onChange={setResponseText} value={responseText} maxLength={400} />
<PasswordField
onChange={setResponseText}
value={responseText}
maxLength={400}
autoFocus={true}
onKeyDown={(e) => handleTextKeyDown(e)}
/>
</When>
</Choose>
</div>
<Choose>
<When condition={userInputRequest.responsetype == "text"}>
<Modal.Footer onCancel={closeModal} onOk={handleSendText} okLabel="Continue" />
<Modal.Footer onCancel={handleSendCancel} onOk={handleSendText} okLabel="Continue" />
</When>
<When condition={userInputRequest.responsetype == "confirm"}>
<Modal.Footer

View File

@ -1689,7 +1689,7 @@ func RemoteInstallCommand(ctx context.Context, pk *scpacket.FeCommandPacketType)
return nil, err
}
mshell := ids.Remote.MShell
go mshell.RunInstall()
go mshell.RunInstall(false)
return createRemoteViewRemoteIdUpdate(ids.Remote.RemotePtr.RemoteId), nil
}

View File

@ -534,7 +534,7 @@ func (msh *MShellProc) tryAutoInstall() {
return
}
msh.writeToPtyBuffer_nolock("trying auto-install\n")
go msh.RunInstall()
go msh.RunInstall(true)
}
// if msh.IsConnected() then GetShellPref() should return a valid shell
@ -1089,31 +1089,6 @@ func (msh *MShellProc) WaitAndSendPasswordNew(pw string) {
requiresPassword := make(chan bool, 1)
ctx, cancelFn := context.WithTimeout(context.Background(), 60*time.Second)
defer cancelFn()
if pw != "" {
// do an extra check with the saved password if it is provided
go msh.CheckPasswordRequested(ctx, requiresPassword)
select {
case <-ctx.Done():
err := ctx.Err()
var errMsg error
if err == context.Canceled {
errMsg = fmt.Errorf("canceled by the user: %v", err)
} else {
errMsg = fmt.Errorf("timed out waiting for password prompt: %v", err)
}
msh.WriteToPtyBuffer("*error, %s\n", errMsg.Error())
msh.setErrorStatus(errMsg)
return
case required := <-requiresPassword:
if !required {
// we don't need user input in this case, so we exit early
return
}
}
msh.SendPassword(pw)
}
// ask for user input once
go msh.CheckPasswordRequested(ctx, requiresPassword)
select {
case <-ctx.Done():
@ -1221,16 +1196,67 @@ func (msh *MShellProc) WaitAndSendPassword(pw string) {
}
}
func (msh *MShellProc) RunInstall() {
func (msh *MShellProc) RunInstall(autoInstall bool) {
remoteCopy := msh.GetRemoteCopy()
if remoteCopy.Archived {
msh.WriteToPtyBuffer("*error: cannot install on archived remote\n")
return
}
if autoInstall {
request := &userinput.UserInputRequestType{
ResponseType: "confirm",
QueryText: "Waveshell must be reinstalled on the connection to continue. Would you like to install it?",
Title: "Install Waveshell",
}
ctx, cancelFn := context.WithTimeout(context.Background(), 60*time.Second)
defer cancelFn()
response, err := userinput.GetUserInput(ctx, scbus.MainRpcBus, request)
if err != nil {
var errMsg error
if err == context.Canceled {
errMsg = fmt.Errorf("installation canceled by user")
} else {
errMsg = fmt.Errorf("timed out waiting for user input")
}
msh.WithLock(func() {
msh.Client = nil
})
msh.WriteToPtyBuffer("*error, %s\n", errMsg)
msh.setErrorStatus(errMsg)
return
}
if !response.Confirm {
errMsg := fmt.Errorf("installation canceled by user")
msh.WriteToPtyBuffer("*error, %s\n", errMsg.Error())
msh.setErrorStatus(err)
msh.WithLock(func() {
msh.Client = nil
})
return
}
}
baseStatus := msh.GetStatus()
if baseStatus == StatusConnecting || baseStatus == StatusConnected {
msh.WriteToPtyBuffer("*error: cannot install on remote that is connected/connecting, disconnect to install\n")
return
if baseStatus == StatusConnected {
request := &userinput.UserInputRequestType{
ResponseType: "confirm",
QueryText: "Waveshell is running on your connection and must be restarted to re-install. Would you like to continue?",
Title: "Restart Waveshell",
}
ctx, cancelFn := context.WithTimeout(context.Background(), 60*time.Second)
defer cancelFn()
response, err := userinput.GetUserInput(ctx, scbus.MainRpcBus, request)
if err != nil {
if err == context.Canceled {
msh.WriteToPtyBuffer("installation canceled by user\n")
} else {
msh.WriteToPtyBuffer("timed out waiting for user input\n")
}
return
}
if !response.Confirm {
msh.WriteToPtyBuffer("installation canceled by user\n")
return
}
}
curStatus := msh.GetInstallStatus()
if curStatus == StatusConnecting {
@ -1247,7 +1273,8 @@ func (msh *MShellProc) RunInstall() {
return
}
if msh.Client == nil {
client, err := ConnectToClient(remoteCopy.SSHOpts)
remoteDisplayName := fmt.Sprintf("%s [%s]", remoteCopy.RemoteAlias, remoteCopy.RemoteCanonicalName)
client, err := ConnectToClient(remoteCopy.SSHOpts, remoteDisplayName)
if err != nil {
statusErr := fmt.Errorf("ssh cannot connect to client: %w", err)
msh.setInstallErrorStatus(statusErr)
@ -1301,8 +1328,8 @@ func (msh *MShellProc) RunInstall() {
})
msh.WriteToPtyBuffer("successfully installed waveshell %s to ~/.mshell\n", scbase.MShellVersion)
go msh.NotifyRemoteUpdate()
if connectMode == sstore.ConnectModeStartup || connectMode == sstore.ConnectModeAuto {
// the install was successful, and we don't have a manual connect mode, try to connect
if connectMode == sstore.ConnectModeStartup || connectMode == sstore.ConnectModeAuto || autoInstall {
// the install was successful, and we didn't click the install button with manual connect mode, try to connect
go msh.Launch(true)
}
}
@ -1470,19 +1497,20 @@ func (msh *MShellProc) createWaveshellSession(remoteCopy sstore.RemoteType) (she
if remoteCopy.SSHOpts.SSHHost == "" && remoteCopy.Local {
cmdStr, err := MakeLocalMShellCommandStr(remoteCopy.IsSudo())
if err != nil {
return nil, fmt.Errorf("cannot find local mshell binary: %v", err)
return nil, fmt.Errorf("cannot find local waveshell binary: %v", err)
}
ecmd := shexec.MakeLocalExecCmd(cmdStr, sapi)
var cmdPty *os.File
cmdPty, err = msh.addControllingTty(ecmd)
if err != nil {
return nil, fmt.Errorf("cannot attach controlling tty to mshell command: %v", err)
return nil, fmt.Errorf("cannot attach controlling tty to waveshell command: %v", err)
}
go msh.RunPtyReadLoop(cmdPty)
go msh.WaitAndSendPasswordNew(remoteCopy.SSHOpts.SSHPassword)
wsSession = shexec.CmdWrap{Cmd: ecmd}
} else if msh.Client == nil {
client, err := ConnectToClient(remoteCopy.SSHOpts)
remoteDisplayName := fmt.Sprintf("%s [%s]", remoteCopy.RemoteAlias, remoteCopy.RemoteCanonicalName)
client, err := ConnectToClient(remoteCopy.SSHOpts, remoteDisplayName)
if err != nil {
return nil, fmt.Errorf("ssh cannot connect to client: %w", err)
}
@ -1493,13 +1521,15 @@ func (msh *MShellProc) createWaveshellSession(remoteCopy sstore.RemoteType) (she
if err != nil {
return nil, fmt.Errorf("ssh cannot create session: %w", err)
}
wsSession = shexec.SessionWrap{Session: session, StartCmd: MakeServerCommandStr()}
cmd := fmt.Sprintf("%s -c %s", sapi.GetLocalShellPath(), shellescape.Quote(MakeServerCommandStr()))
wsSession = shexec.SessionWrap{Session: session, StartCmd: cmd}
} else {
session, err := msh.Client.NewSession()
if err != nil {
return nil, fmt.Errorf("ssh cannot create session: %w", err)
}
wsSession = shexec.SessionWrap{Session: session, StartCmd: MakeServerCommandStr()}
cmd := fmt.Sprintf(`%s -c %s`, sapi.GetLocalShellPath(), shellescape.Quote(MakeServerCommandStr()))
wsSession = shexec.SessionWrap{Session: session, StartCmd: cmd}
}
return wsSession, nil
}
@ -1582,7 +1612,7 @@ func (NewLauncher) Launch(msh *MShellProc, interactive bool) {
go msh.tryAutoInstall()
return
} else if err != nil {
msh.WriteToPtyBuffer("*error, %s\n", serr.Error())
msh.WriteToPtyBuffer("*error, %s\n", err.Error())
msh.setErrorStatus(err)
msh.WithLock(func() {
msh.Client = nil

View File

@ -11,6 +11,7 @@ import (
"crypto/x509"
"encoding/base64"
"fmt"
"log"
"net"
"os"
"os/user"
@ -65,19 +66,32 @@ func createDummySigner() ([]ssh.Signer, error) {
// keys from being attempted. But if there's an error because of a dummy
// file, the library can still try again with a new key.
func createPublicKeyCallback(sshKeywords *SshKeywords, passphrase string) func() ([]ssh.Signer, error) {
identityFiles := make([]string, len(sshKeywords.IdentityFile))
copy(identityFiles, sshKeywords.IdentityFile)
var identityFiles []string
existingKeys := make(map[string][]byte)
// checking the file early prevents us from needing to send a
// dummy signer if there's a problem with the signer
for _, identityFile := range sshKeywords.IdentityFile {
privateKey, err := os.ReadFile(base.ExpandHomeDir(identityFile))
if err != nil {
// skip this key and try with the next
continue
}
existingKeys[identityFile] = privateKey
identityFiles = append(identityFiles, identityFile)
}
// require pointer to modify list in closure
identityFilesPtr := &identityFiles
return func() ([]ssh.Signer, error) {
if len(*identityFilesPtr) == 0 {
// skip this key and try with the next
return createDummySigner()
return nil, fmt.Errorf("no identity files remaining")
}
identityFile := (*identityFilesPtr)[0]
*identityFilesPtr = (*identityFilesPtr)[1:]
privateKey, err := os.ReadFile(base.ExpandHomeDir(identityFile))
if err != nil {
privateKey, ok := existingKeys[identityFile]
if !ok {
log.Printf("error with existingKeys, this should never happen")
// skip this key and try with the next
return createDummySigner()
}
@ -136,15 +150,20 @@ func createDefaultPasswordCallbackPrompt(password string) func() (secret string,
}
}
func createInteractivePasswordCallbackPrompt() func() (secret string, err error) {
func createInteractivePasswordCallbackPrompt(remoteDisplayName string) func() (secret string, err error) {
return func() (secret string, err error) {
// limited to 15 seconds for some reason. this should be investigated more
// in the future
ctx, cancelFn := context.WithTimeout(context.Background(), 60*time.Second)
defer cancelFn()
queryText := fmt.Sprintf(
"Password Authentication requested from connection \n"+
"%s\n\n"+
"Password:", remoteDisplayName)
request := &userinput.UserInputRequestType{
ResponseType: "text",
QueryText: "Password:",
QueryText: queryText,
Markdown: true,
Title: "Password Authentication",
}
response, err := userinput.GetUserInput(ctx, scbus.MainRpcBus, request)
@ -155,13 +174,13 @@ func createInteractivePasswordCallbackPrompt() func() (secret string, err error)
}
}
func createCombinedPasswordCallbackPrompt(password string) func() (secret string, err error) {
func createCombinedPasswordCallbackPrompt(password string, remoteDisplayName string) func() (secret string, err error) {
var once sync.Once
return func() (secret string, err error) {
var prompt func() (secret string, err error)
once.Do(func() { prompt = createDefaultPasswordCallbackPrompt(password) })
if prompt == nil {
prompt = createInteractivePasswordCallbackPrompt()
prompt = createInteractivePasswordCallbackPrompt(remoteDisplayName)
}
return prompt()
}
@ -180,14 +199,14 @@ func createNaiveKbdInteractiveChallenge(password string) func(name, instruction
}
}
func createInteractiveKbdInteractiveChallenge() func(name, instruction string, questions []string, echos []bool) (answers []string, err error) {
func createInteractiveKbdInteractiveChallenge(remoteName string) func(name, instruction string, questions []string, echos []bool) (answers []string, err error) {
return func(name, instruction string, questions []string, echos []bool) (answers []string, err error) {
if len(questions) != len(echos) {
return nil, fmt.Errorf("bad response from server: questions has len %d, echos has len %d", len(questions), len(echos))
}
for i, question := range questions {
echo := echos[i]
answer, err := promptChallengeQuestion(question, echo)
answer, err := promptChallengeQuestion(question, echo, remoteName)
if err != nil {
return nil, err
}
@ -197,14 +216,19 @@ func createInteractiveKbdInteractiveChallenge() func(name, instruction string, q
}
}
func promptChallengeQuestion(question string, echo bool) (answer string, err error) {
func promptChallengeQuestion(question string, echo bool, remoteName string) (answer string, err error) {
// limited to 15 seconds for some reason. this should be investigated more
// in the future
ctx, cancelFn := context.WithTimeout(context.Background(), 60*time.Second)
defer cancelFn()
queryText := fmt.Sprintf(
"Keyboard Interactive Authentication requested from connection \n"+
"%s\n\n"+
"%s", remoteName, question)
request := &userinput.UserInputRequestType{
ResponseType: "text",
QueryText: question,
QueryText: queryText,
Markdown: true,
Title: "Keyboard Interactive Authentication",
}
response, err := userinput.GetUserInput(ctx, scbus.MainRpcBus, request)
@ -214,13 +238,13 @@ func promptChallengeQuestion(question string, echo bool) (answer string, err err
return response.Text, nil
}
func createCombinedKbdInteractiveChallenge(password string) ssh.KeyboardInteractiveChallenge {
func createCombinedKbdInteractiveChallenge(password string, remoteName string) ssh.KeyboardInteractiveChallenge {
var once sync.Once
return func(name, instruction string, questions []string, echos []bool) (answers []string, err error) {
var challenge ssh.KeyboardInteractiveChallenge
once.Do(func() { challenge = createNaiveKbdInteractiveChallenge(password) })
if challenge == nil {
challenge = createInteractiveKbdInteractiveChallenge()
challenge = createInteractiveKbdInteractiveChallenge(remoteName)
}
return challenge(name, instruction, questions, echos)
}
@ -264,7 +288,7 @@ func writeToKnownHosts(knownHostsFile string, newLine string, getUserVerificatio
}
if !response.Confirm {
f.Close()
return UserInputCancelError{Err: fmt.Errorf("Canceled by the user")}
return UserInputCancelError{Err: fmt.Errorf("canceled by the user")}
}
_, err = f.WriteString(newLine)
@ -336,7 +360,18 @@ func createHostKeyCallback(opts *sstore.SSHOpts) (ssh.HostKeyCallback, error) {
userKnownHostsFiles := strings.Fields(rawUserKnownHostsFiles) // TODO - smarter splitting escaped spaces and quotes
rawGlobalKnownHostsFiles, _ := ssh_config.GetStrict(opts.SSHHost, "GlobalKnownHostsFile")
globalKnownHostsFiles := strings.Fields(rawGlobalKnownHostsFiles) // TODO - smarter splitting escaped spaces and quotes
unexpandedKnownHostsFiles := append(userKnownHostsFiles, globalKnownHostsFiles...)
osUser, err := user.Current()
if err != nil {
return nil, err
}
var unexpandedKnownHostsFiles []string
if osUser.Username == "root" {
unexpandedKnownHostsFiles = globalKnownHostsFiles
} else {
unexpandedKnownHostsFiles = append(userKnownHostsFiles, globalKnownHostsFiles...)
}
var knownHostsFiles []string
for _, filename := range unexpandedKnownHostsFiles {
knownHostsFiles = append(knownHostsFiles, base.ExpandHomeDir(filename))
@ -470,7 +505,7 @@ func createHostKeyCallback(opts *sstore.SSHOpts) (ssh.HostKeyCallback, error) {
return waveHostKeyCallback, nil
}
func ConnectToClient(opts *sstore.SSHOpts) (*ssh.Client, error) {
func ConnectToClient(opts *sstore.SSHOpts, remoteDisplayName string) (*ssh.Client, error) {
sshConfigKeywords, err := findSshConfigKeywords(opts.SSHHost)
if err != nil {
return nil, err
@ -482,8 +517,8 @@ func ConnectToClient(opts *sstore.SSHOpts) (*ssh.Client, error) {
}
publicKeyCallback := ssh.PublicKeysCallback(createPublicKeyCallback(sshKeywords, opts.SSHPassword))
keyboardInteractive := ssh.KeyboardInteractive(createCombinedKbdInteractiveChallenge(opts.SSHPassword))
passwordCallback := ssh.PasswordCallback(createCombinedPasswordCallbackPrompt(opts.SSHPassword))
keyboardInteractive := ssh.KeyboardInteractive(createCombinedKbdInteractiveChallenge(opts.SSHPassword, remoteDisplayName))
passwordCallback := ssh.PasswordCallback(createCombinedPasswordCallbackPrompt(opts.SSHPassword, remoteDisplayName))
// batch mode turns off interactive input. this means the number of
// attemtps must drop to 1 with this setup
@ -644,6 +679,9 @@ func findSshConfigKeywords(hostPattern string) (*SshKeywords, error) {
// these are parsed as a single string and must be separated
// these are case sensitive in openssh so they are here too
preferredAuthenticationsRaw, err := ssh_config.GetStrict(hostPattern, "PreferredAuthentications")
if err != nil {
return nil, err
}
sshKeywords.PreferredAuthentications = strings.Split(preferredAuthenticationsRaw, ",")
return sshKeywords, nil