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

x/crypto/ssh: Signal method doesn't work #16597

Open
purpleidea opened this Issue Aug 4, 2016 · 10 comments

Comments

Projects
None yet
7 participants
@purpleidea
Copy link

purpleidea commented Aug 4, 2016

Starting a process over SSH using the /x/crypto/ssh library and then trying to send a signal to it, seems to have no effect. I've found a few musings on the internet about users with the same problem.

I've attached a full reproducer, including the crappy workaround.

package main

import (
    "bytes"
    "fmt"
    "golang.org/x/crypto/ssh"
    "time"
)

func main() {

    // An SSH client is represented with a ClientConn.
    //
    // To authenticate with the remote server you must pass at least one
    // implementation of AuthMethod via the Auth field in ClientConfig.
    config := &ssh.ClientConfig{
        User: "someuser", // XXX
        Auth: []ssh.AuthMethod{
            ssh.Password("somepass"), // XXX
        },
    }
    client, err := ssh.Dial("tcp", "localhost:22", config)
    if err != nil {
        panic("Failed to dial: " + err.Error())
    }

    // Each ClientConn can support multiple interactive sessions,
    // represented by a Session.
    session, err := client.NewSession()
    if err != nil {
        panic("Failed to create session: " + err.Error())
    }
    defer session.Close()

    // Once a Session is created, you can execute a single command on
    // the remote side using the Run method.
    var b bytes.Buffer
    session.Stdout = &b

    go func() {
        time.Sleep(2 * time.Second)
        // XXX none of these signals work! :(
        //      fmt.Println("Running signal!")
        //      session.Signal(ssh.SIGINT)
        //      session.Signal(ssh.SIGKILL)
        //      session.Signal(ssh.SIGQUIT)
        //      session.Signal(ssh.SIGHUP)
        //      session.Signal(ssh.SIGPIPE)
        //      fmt.Println("Done running signal!")

        s2, err := client.NewSession()
        if err != nil {
            panic("Failed to create session: " + err.Error())
        }
        defer s2.Close()
        var c bytes.Buffer
        s2.Stdout = &c
        if err := s2.Run("echo start && pidof sleep && killall sleep"); err != nil {
            fmt.Println("s2 error!")
        }
        fmt.Println("Sig run done!")
        fmt.Println(c.String())

    }()

    if err := session.Run("echo $0 && /usr/bin/sleep 10s"); err != nil {

        if e, ok := err.(*ssh.ExitError); ok {
            fmt.Printf("Remote: Exit msg: %s", e.Waitmsg.Msg()) // XXX (does this ever return anything useful?)
            fmt.Printf("Remote: Exit signal: %s", e.Waitmsg.Signal())
            fmt.Printf("Remote: Error: Output...\n%s", b.String())
            fmt.Printf("Exited (%d) with: %s", e.Waitmsg.ExitStatus(), e.Error())

        } else if e, ok := err.(*ssh.ExitMissingError); ok {
            fmt.Printf("Exit code missing: %s", e.Error())
        }

        panic("Failed to run: " + err.Error())
    }
    fmt.Println("Done!")
    fmt.Println(b.String())
}

Uncomment the signal you want, and you'll see that they all do nothing to kill the running sleep command.

Tested with go version: 1.5.4 (but I have no reason to expect this is fixed with newer versions!) on Fedora GNU/Linux 24 as the SSH server, running OpenSSH.

Thanks!

@quentinmit quentinmit changed the title The /x/crypto/ssh library signal method doesn't work. x/crypto/ssh: Signal method doesn't work. Aug 4, 2016

@quentinmit quentinmit added this to the Unreleased milestone Aug 4, 2016

@purpleidea

This comment has been minimized.

Copy link
Author

purpleidea commented Aug 5, 2016

This might be related to #4115 in any case this should be fixed :)

@hanwen

This comment has been minimized.

Copy link
Contributor

hanwen commented Aug 8, 2016

ssh signal just sends a request with the signal name to the other side, so there is little that can break on the cliet side. Can you provide proof that signals do work in other client implementations with this server?

@purpleidea

This comment has been minimized.

Copy link
Author

purpleidea commented Aug 8, 2016

On Mon, Aug 8, 2016 at 5:55 AM, Han-Wen Nienhuys notifications@github.com
wrote:

ssh signal just sends a request with the signal name to the other side, so
there is little that can break on the cliet side. Can you provide proof
that signals do work in other client implementations with this server?

It's a good question, but I don't know the answer. I do know that running
an Exec killall -SIGNALNAME over ssh does work, which is the (ugly)
workaround I've been employing. So either this should either be fixed, or
documented that it doesn't work with the most common ssh server out there
or something, because this definitely doesn't fit with the principle of
least surprise. Maybe even in the worst case scenario that this doesn't
work with openssh that we should fall back to running that killall command
for the user behind the scenes. If that's the answer, I'll write the patch
if someone can help me to know how to detect the server capabilities and
the pid of the previously run command.

@hanwen

This comment has been minimized.

Copy link
Contributor

hanwen commented Aug 8, 2016

"exec killall" doesn't work when there is a restrictions on running commands (eg. ProgramName critical option in certs), so I don't think we should offer this is as a default workaround.

It would be great if you could get to the bottom of this, so we understand the problem fully before we document anything. I suspect openssh disables signals by default, and that they have some good reason for it.

@spudlyo

This comment has been minimized.

Copy link

spudlyo commented Sep 26, 2016

It looks to me like OpenSSH doesn't support this on the server side, it's in the protocol documentation, but unimplemented.

darkstar:~/src/openssh-7.3p1$ find . -type f | xargs grep \"signal\"
./mux.c: *   - If we ever support the "signal" channel request, send signals on

https://bugzilla.mindrot.org/show_bug.cgi?id=1424

@purpleidea

This comment has been minimized.

Copy link
Author

purpleidea commented Sep 26, 2016

@spudlyo Great find, thanks. Anyone have friends upstream there that can get that merged?

@purpleidea

This comment has been minimized.

Copy link
Author

purpleidea commented Apr 11, 2018

Ping? Anybody know who the maintainer for this is?

@jayschwa

This comment has been minimized.

Copy link
Contributor

jayschwa commented Apr 11, 2018

I have asked about the open ticket a couple times in the last few months and never got a reply.

https://marc.info/?l=openssh-unix-dev&m=151872213119286&w=2
https://marc.info/?l=openssh-unix-dev&m=152104163506271&w=2

@ALTree ALTree changed the title x/crypto/ssh: Signal method doesn't work. x/crypto/ssh: Signal method doesn't work Sep 22, 2018

@richard-mauri

This comment has been minimized.

Copy link

richard-mauri commented Dec 8, 2018

I'm hitting this too.
I'd even be happy with ssh client workarounds.
I'm using to go crypto/ssh package to run interactive sessions and am facing problems with signal handling not propagating and other terminal related hell like getting vi to work on the server side session.

@nrailgun

This comment has been minimized.

Copy link

nrailgun commented Jan 3, 2019

God I though that was my code's problem...
This lib is awesome, hope this problem can be fixed soon. thank you.
I guess i have to hack this problem for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.