Skip to content

Commit

Permalink
fdpass: send stderr to tbot for error diagnostics (#43586)
Browse files Browse the repository at this point in the history
  • Loading branch information
espadolini authored Jun 27, 2024
1 parent 74c8b6b commit 031cf44
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 9 deletions.
49 changes: 46 additions & 3 deletions lib/tbot/service_ssh_multiplexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import (
"github.com/gravitational/teleport/lib/tbot/identity"
"github.com/gravitational/teleport/lib/tbot/ssh"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/teleport/lib/utils/uds"
)

const (
Expand Down Expand Up @@ -539,12 +540,47 @@ func (s *SSHMultiplexerService) handleConn(
}
}()

var stderr *os.File
defer func() {
if stderr == nil {
return
}
defer stderr.Close()
if err != nil {
fmt.Fprintln(stderr, err)
}
}()

var req string
// here we try receiving a file descriptor to use for error output, which
// should be mapped to OpenSSH's own stderr (or /dev/null)
if un, _ := downstream.(*net.UnixConn); un != nil {
b := make([]byte, 1)
fds := make([]*os.File, 1)

// TODO(espadolini): get rid of [uds.Conn]
n, fdn, err := (&uds.Conn{UnixConn: un}).ReadWithFDs(b, fds)
if err != nil {
return trace.Wrap(err, "reading request")
}
if fdn > 0 {
s.log.DebugContext(ctx, "Received stderr file descriptor from client for error reporting")
stderr = fds[0]
}
// this approach works because we know that req must be at least one
// byte at the end (as it must end with a NUL)
req = string(b[:n])
}

// The first thing downstream will send is the multiplexing request which is
// the "[host]:[port]\x00" format.
buf := bufio.NewReader(downstream)
req, err := buf.ReadString('\x00')
if err != nil {
return trace.Wrap(err, "reading request")
if !strings.HasSuffix(req, "\x00") {
r, err := buf.ReadString('\x00')
if err != nil {
return trace.Wrap(err, "reading request")
}
req += r
}
req = req[:len(req)-1] // Drop the NUL.
host, port, err := utils.SplitHostPort(req)
Expand Down Expand Up @@ -647,6 +683,13 @@ func (s *SSHMultiplexerService) handleConn(
log.InfoContext(ctx, "Proxying connection for multiplexing request")
startedProxying := time.Now()

// once the connection is actually started we should stop writing to the
// client's stderr
if stderr != nil {
_ = stderr.Close()
stderr = nil
}

errCh := make(chan error, 2)
go func() {
defer upstream.Close()
Expand Down
39 changes: 33 additions & 6 deletions tool/fdpass-teleport/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ fn main() -> Result<()> {
}

let mux_path = args.nth(1).ok_or_eyre("missing mux path")?;
let mut target = args.next().ok_or_eyre("missing connection target")?;
target.push("\0");
let target = args.next().ok_or_eyre("missing connection target")?;

// in OpenSSH ProxyCommand+ProxyUseFdPass the program is executed with a
// unix domain socket as stdout, which we can check with a getsockname();
Expand All @@ -79,9 +78,35 @@ fn main() -> Result<()> {
}

let mut mux_conn = UnixStream::connect(mux_path).wrap_err("could not connect to mux")?;
// we added a final NUL to target above

let mut target = target;
target.push("\0");
let mut buf = target.as_bytes();

// pass the current stderr to tbot, so it can be used to output dial errors
loop {
match socket::sendmsg::<()>(
mux_conn.as_raw_fd(),
&[IoSlice::new(buf)],
&[ControlMessage::ScmRights(&[libc::STDERR_FILENO])],
MsgFlags::empty(),
None,
) {
Err(Errno::EINTR) => continue,
Ok(0) => eyre::bail!("unexpected sendmsg return value 0"),
Ok(n) => {
buf = &buf[n..];
break;
}
Err(e) => {
return Err(e).wrap_err("could not send connection target to mux");
}
};
}
// in all likelyhood the target fit in the socket buffer already, but we
// can't assume that, so here we send the rest of the unsent target (if any)
mux_conn
.write_all(target.as_bytes())
.write_all(buf)
.wrap_err("could not send connection target to mux")?;

// we can now pass the connection to OpenSSH (or whoever launched us) over
Expand All @@ -96,10 +121,12 @@ fn main() -> Result<()> {
MsgFlags::empty(),
None,
) {
Err(Errno::EINTR) => continue,
Ok(1) => break,
Ok(s) => eyre::bail!("unexpected sendmsg return value {s}"),
Err(Errno::EAGAIN | Errno::EINTR) => continue,
Err(e) => Err(e).wrap_err("could not pass connection to stdout")?,
Err(e) => {
return Err(e).wrap_err("could not pass connection to stdout");
}
};
}

Expand Down

0 comments on commit 031cf44

Please sign in to comment.