Support pty or interaction handlers

[robd]

I know there has been a lot of confusion and misunderstanding around the Netssh.config.pty option with a lot of people (myself included - sorry!) thinking that interactive sessions are supported in SSHKit. My understanding is that full interactivity support is not a direction SSHKit should go in (I agree). So this is an attempt at implementing a minimal patch to allow users to write their own programmatic, rules-based handlers to support interactive workflows. ie 'When the server responds with x', 'send y'.

This PR adds support for an enhanced :pty option on Command in addition to the existing global level Netssh.config.pty option. This :pty option still supports values of true and false/nil as before, but also now supports passing a handler object which defines on_stdout(data, channel) and/or on_sterr(data, channel). If specified, this handler is called once per line from the server and provides the channel so that data can be written in response to particular server responses.

I do not propose to add any specific handlers to SSHKit - these could be provided by one or more external gems, but below is an example of a handler to support interactive sudo based on the working implementation in ssh-sudo:

# The following would not be included in SSHKit, 
# but here is an example of a pty handler which supports interactive sudo

class InteractiveSudo
  def on_stdout(data, ch)
    if data =~ /Sorry.*\stry\sagain/
      @cached_password = nil
    end
    if data =~ /password.*:/
      pass = @cached_password
      unless pass
        pass = $stdin.noecho(&:gets)
        @cached_password = pass
      end
      ch.send_data(pass)
    end
  end
end

INTERACTIVE_SUDO = InteractiveSudo.new()

# Client code:

execute(:sudo, 'Some command', :pty => INTERACTIVE_SUDO)

As part of this work, I also formalised the mutations allowed on the @stdout, @stderr, @full_stdout and @full_stderr attributes on Command, and removed the writers. I have some questions around this and ideas for a bit of further cleanup / tests here too.

The test suite passes, but I didn't want to spend time adding more tests until I had some indication that this PR is desirable. As well as tests, I'd also add comprehensive docs about the interactivity support, and try to add comments to all the issues / SO questions to steer people in the right direction.

There are a number of outstanding questions, especially around local server support, but we can get into those if you're happy with the basic idea. This work was based on discussions with @kentaroi on his ssh-sudo project.

EDIT: It turns out, the changes to pty are not necessary to support the basic use case of interacting with the server, so I reworked this PR see comments below for subsequent developments

[leehambley]

Excellent looking PR @robd - I wholly support the idea of this (I'm a bit fan of expect… and like the fuzzy feelings of nostalgia that this PR gives me!) I'll need a bit more time to review it, but sshkit-* seems like a fantastic direction to go in (and, echoes what we did with Capistrano)

[robd]

OK great - thanks vm for the quick reply @leehambley. I will work this up with more examples and tests. I'll more carefully audit the other issues and PRs around this and try to make sure that all use cases are covered with this minimal change.

[robd]

@leehambley OK, I reworked this PR to add documentation and tests. I also added a MappingInteractionHandler to support the common use case - one-to-one mapping from server out lines to input lines. Unexpectedly, I found that this worked without needing a pty, so I removed the pty related changes from the PR. I expect a per-command pty option would be desirable but I want to wait until I have a use case for this before I implement it.

If possible, I would like this to be merged before #239 since I rely on capture in the new tests in this PR, and #239 will break them. I will fix up #239 so it applies cleanly on top of this PR after it's merged.

I also made some changes to the way logging works so I could log from the MappingInteractionHandler.

Note: I removed the trace log level, since I couldn't get this to work with the Pretty formatter when I tried to add a test for it, even when I reimplemented the level method to handle the Logger::TRACE level value of -1

[robd]

@leehambley If it's OK with you, I'd like to rebase this PR on latest master - there are a couple of things that need fixing I think. Is that OK?

[leehambley]

Absolutely Rob, no history is sacred until merged 😈

[robd]

OK, great - this is done. I fixed up some trailing whitespace and updated a new test which was failing due to changes to capture behaviour from #239

[leehambley]

I'll need a little more time to review this sorry @robd, don't want to take the wind out of your sails :) but I'm a bit crammed on time, and I want to make sure we get it right(ish) first time.

[leehambley]

Maybe write this in 1.9 syntax?

[leehambley]

Just completed a preliminary skimming through the code, and related new docs. I'll do it again when I have more time later, a couple of tiny notes, but I'll respond in depth later. Thanks for the stellar work @robd

[robd]

Yeah no probs. I agree this would benefit from careful review, so do not
feel you have to rush. Sorry for inundating you with so many PRs - I had a
block of free time the last few days, so I wanted to try and get this in
while I was fresh in the code. I'm travelling at the moment, so am on 1G
mobile signal, but I will be able to discuss more easily later on.

On 27 April 2015 at 14:59, Lee Hambley [email protected] wrote:

Just completed a preliminary skimming through the code, and related new
docs. I'll do it again when I have more time later, a couple of tiny notes,
but I'll respond in depth later. Thanks for the stellar work @robd
https://github.com/robd

—
Reply to this email directly or view it on GitHub
#234 (comment).

[vivid-inc]

Great PR -- Otsukaresama deshita! If it is indeed the case that requesting a PTY for a session means that the session is then associated with a pseudo-terminal for the remainder of its life and not just "for the next command I execute only," (RFC 4254?) I feel it's a service to forewarn otherwise-unsuspecting SSHKit users so that they can be wary of the attendant pitfalls.

[robd]

Hi @vivid-inc

Great PR -- Otsukaresama deshita!

I had to look this up! And thanks!

In the end, I don't request a pty at all when you specify the :interaction_handler option.

TBH, I don't understand whether pty would normally be needed or not, but this test passes without a pty. I hope to be able to test in real world scenarios tomorrow and see whether pty is needed. I know for some historic linux distributions, tty is required for sudo, but I believe in recent linux distributions (ubuntu 14.04 at least) the requiretty option in sudoers defaults to off.

My understanding is that Netssh opens a new ssh connection for every command, so hopefully, if a per-command pty option is needed, it will only apply to a single command, not multiple commands. I will add documentation if I add support for such an option.

However, at the moment, pty can only be configured globally for all commands.

[vivid-inc]

Hi Rob,

Well, I'll let you know how it goes for us when we get to the implementation.
Looking forward to it!

Thanks --

Ty C
Vivid Inc. Atlassian Support
[email protected]

On 2015 Apr 28, at 09:45, Rob Dupuis [email protected] wrote:

Hi @vivid-inc

Great PR -- Otsukaresama deshita!

I had to look this up! And thanks!

In the end, I don't request a pty at all when you specify the :interaction_handler option.

TBH, I don't understand whether pty would normally be needed or not, but this test passes without a pty. I hope to be able to test in real world scenarios tomorrow and see whether pty is needed. I know for some historic linux distributions, tty is required for sudo, but I believe in recent linux distributions (ubuntu 14.04 at least) the requiretty option in sudoers defaults to off.

My understanding is that Netssh opens a new ssh connection for every command, so hopefully, if a per-command pty option is needed, it will only apply to a single command, not multiple commands. I will add documentation if I add support for such an option.

However, at the moment, pty can only be configured globally for all commands.

—
Reply to this email directly or view it on GitHub.

[leehambley]

I don't understand whether pty would normally be needed or not.

Assigning a "pty" essentially tells the process "you are attached to a [pseudo] terminal", since we don't really have real ttys anymore. If a program thinks it is being controlled by a terminal, usually the following are expected to be true:

• Can move a cursor.
• Can prompt for input.
• Can (should?) use colours.
• Should print output in a line-based, rather than \0 terminated.
• etc, etc

A more complete example might be the following:

cat | grep -v hello world | tr '[:lower:]' '[:upper:]' | sort

Here, only and sort thinks it's attached to one end of the pty|tty master/slave (I'm not sure honestly which), and cat, grep and tr are not connected to a pty|tty at all. (I think, caveat always is that "this shit is complicated"), here's an example I threw together:

$  cat ./ispty.rb
#!/usr/bin/ruby
Kernel.warn [$stdin.tty?, $stdout.tty?].join(',') # Kernel.warn writes to $stderr
$ ./ispty.rb | ./ispty.rb | ./ispty.rb | ./ispty.rb
true,false
false,false
false,false
false,true

(Note: the output is very prone to racing, but this is the output one should expect)

http://capistranorb.com/documentation/advanced-features/ptys/ has more of the notes, and a link to the rbenv docs where this is also neatly written about. To make a long story short, if programs were well behaved and written without a pty they would fail to retrieve any capabilities from termcap and then shouldn't expect to be able to do box drawing, or anything else relying on there being a "device" which might have "capabilities" which might be useful.

One strange caveat is that often $stdin.tty? is used as a guard around prompting for input, that's not strictly correct, as you might still get piped input via stdin, but you won't logically be able to print a prompt, and expect to just get one line, that's why things like readline are useful in practice.

To quote a grumpy old unix neckbeard colleague of mine:

The problem with software today is that it's all written by kids, in scripting languages.

He's not wrong, there's a lot of lost knowledge here, I'm just glad there's still enough people around writing decent terminal emulators, and keeping this stuff alive. Also, this is worth a watch - https://www.destroyallsoftware.com/talks/a-whole-new-world

[robd]

@leehambley Thanks vm for the long explanation. This is starting to make more sense. My understanding from this is whether a program requires a pty is completely dependant on the implementation details of the program (eg whether it includes checks to $stdin.tty? to guard for input).

I guess one problem with the current implementation in this PR, is that it will not support the use case of sending input directly to a command which has no prompt. I don't know if this is a problem in the real world, and I am tempted to only add support for things when there is a real use case, so we can add tests. Also, this would require a more fundamental rework of the ssh session handling code

That a whole new world video is great - thanks!

[leehambley]

whether a program requires a pty is completely dependant on the implementation details of the program.

Right, and as my colleague likes to say, since most things these days seem to be implemented by kids using scripting languages, they often don't behave correctly. i.e just because $stdin isn't a tty, doesn't mean you can't read from it! (good example echo yes | apt-get install something vs. apt-get install -y something)

I guess one problem with the current implementation in this PR, is that it will not support the use case of sending input directly to a command which has no prompt.

Indeed, I'd have to check, but I'm pretty sure, for example that apt-get just fails if there's a question to be answered, and it can't read from stdin, and stdout isn't a tty. Usually reads from stdin (like any reads) would block until there is something to be read, programs might make use of man 2 stat and the value of S_ISCHR to find out if stdin is a character device, this also won't work in this case, as far as I know. Ruby implements that as FileTest.chardev?, but I've honestly never seen how a simple program that uses this responds when called via Capistrano/Net::SSH/SSHKit, et al.

Final word on this about "if it doesn't prompt for input", it's poorly written, harking back to the apt-get example, they've provided three ways to give it answers:

1. Pipe something in to it's stdin
2. Give it a command line flag
3. Answer the prompt

This is what I might call "well written", without finger pointing other programs that don't behave this way.

I'm glad you enjoyed "A whole new world", I didn't enjoy the ending :)

[leehambley]

Merging because I just saw that you corrected the hash syntax as I suggested, thank you kindly!

[robd]

Thanks for merging this. I will try using it a bit and there may be more pty related PRs needed. You probably saw this, but I should point out I put a placeholder in the readme for the version in which interaction_handlers are released.

As my colleague likes to say, since most things these days seem to be implemented by kids using scripting languages...

I think one of the problems is that there is a great deal of subtlety to these things, and, with pty/tty at least a lot of people only end up dealing with those issues in passing (like me!), whilst trying to get something else to work. There is a lot of incorrect or out of date information around and sometimes it is hard to build up a correct mental model of these concepts. Much of it is historical (as mentioned in that video), and can be hard to understand the evolution of the concepts.

I just saw that you corrected the hash syntax as I suggested, thank you kindly!

Yes forgot to say thanks for pointing that out. Would you believe, I've only just moved off 1.8.7 on the main app I'm working on!

[leehambley]

I think one of the problems is that there is a great deal of subtlety to these things, and, with pty/tty at least a lot of people only end up dealing with those issues in passing (like me!), whilst trying to get something else to work.

Indeed, that's one of the reasons that Capistrano and SSHKit don't have communities like other Gems/etc in my opinion, we fall right in the spot between success (“I just finished a feature!”) and future success (“I just want to deploy my new feature!”), and that's the wrong time for people to run into really tricky arcane stuff that isn't wholly intuitive.

We (that is my company, and some former/current Capistrano contributors) are working on a webbased, hosted CI, which can largely be thought of as a "webbased" version of Capistrano/SSHKit (i.e designed to define tasks, and triggers for those tasks, and run them in a safe, reliable, repeatable way with decent logging and integrations), vs the traditional "the tests passed" kind of CI… we're hurting for money to get to alpha, and whilst "Kickstart it from the community" is a valid argument for something like Travis with the weight of the TDD movement behind it, in spite of our best efforts using Capistrano/SSHKit can be kinda janky, and it's not (all) entirely our fault, but people generally don't like this layer, so asking for money to support us isn't exactly a compelling case.

[leehambley]

There is a lot of incorrect or out of date information around and sometimes it is hard to build up a correct mental model of these concepts. Much of it is historical (as mentioned in that video), and can be hard to understand the evolution of the concepts.

Gary Burnhardt's "Destroy All Software" series, which I used to be subscribed to, and have all archived has a series of excellent things, from "Refactoring some complex Ruby" down to "How does /usr/bin/time differ from the shell builtin, and how does it do magic with streams" (ever noticed how time something > /dev/null doesn't /dev/null the output from time? 😉 )

[leehambley]

… by kids in scripting languages…

Is as much poking fun at the sad state of affairs we have in education, as well as language and library and package design, more than it's poking fun at any individuals, some of the common things that annoy me:

• Colours when no tty
• No options to control colours
• No respect of common env vars (… I'm still complaining about colours!)
• Not supporting \0 delimited output for piping

These are all really, really prevalent, and they cause issues for Capistrano, moreover things like Thor terrify me, Thor will _never_ exit with non-zero exit status, because their Aruba CLI test framework dies if the process it's testing exits non-zero… the mind simply boggles, I reported it and campaigned for it to be fixed, and it was closed as being not-worth-it!

Bigger issues that tie me up, are how logging is broken, how many, many things (and SSHKit, by extension of Ruby's logger class being broken is also guilty) don't allow loggers to take two streams, $stdout for debug and info, and $stderr for warn and fatal, the typical cron use-case, which relies on this behaviour to know when to send failure emails with the output when something was written to stderr. If you write a low level language coughCcough then you don't have logger objects to worry about, you have macros which expand to something like #define debug(M, ...) fprintf(stderr, "DEBUG %s:%d: " M "\n", __FILE__, __LINE__, ##__VA_ARGS__)!

[robd]

@leehambley Sorry was out of contact - really interesting to read your thoughts here - a lot to think about. I think your point about where capistrano / sshkit sit in the workflow is completely right. I think the users are often inexperienced and short of time. Inevitably this means they skip the docs or are never going to invest enough time to understand the complexities of the issues at hand.

IMHO, for this type of user (such as myself), I think the most helpful docs are the example based ones, so I will try to add at least one to the capistrano docs about interaction handlers. I think the SSHKit docs are pretty good now on this feature, so hopefully we will see fewer issues with people confused about stdin. I was thinking of adding details on how to program interaction with stdin to capistranorb.com. Do you have a feeling for where this should live? - I was thinking a new page under Getting Started or Advanced Features with a couple of examples and a link to the sshkit docs.

One of the examples I would like to add is how to implement interactive sudo (which is how I got in here in the first place:). It's now very easy to implement this by redefining sudo on the capistrano dsl and providing the visiblepw and requiretty sudoers options are set correctly, this works without pty AFAICT:

module DSL
  def sudo(*args)
    interactive_sudo_options = { interaction_handler: {
      '[sudo] password for username: ' => "#{fetch(:sudo_password)}\n",
      "\r\n" => nil,
    } }
    options = interactive_sudo_options.merge(args.extract_options!)
    execute :sudo, *args, options
  end
end

...

ask(:sudo_password, '', echo: false)

I may have got the wrong end of the stick, but I think you may not be a fan of interactive sudo. I'm not sure if this is just the pty aspect or if you think it's wrong even without needing pty. Are you happy to include the above example about how to do interactive sudo on capistranorb.com? I think it would be useful to have some canonical instructions on this so that we can keep it up to date with any changes and also provide links from stack overflow / capistrano github issues.

[leehambley]

I was thinking a new page under Getting Started or Advanced Features with a couple of examples and a link to the sshkit docs.

Sounds about right, maybe in the FAQ, something like "how can I use this without passwordless sudo". I'd also like to ask if you could draw the comparison to GNU expect when you alter the docs, as I think for many 'old beards" (myself included), it's a tool we know, with a familiar API, and it then allows use-cases for expect to help cue people to get the best out of Cap.

I may have got the wrong end of the stick, but I think you may not be a fan of interactive sudo. I'm not sure if this is just the pty aspect or if you think it's wrong even without needing pty.

I'm a fan of solving problems at the source, the source of the "I have an automated script that needs sudo" is to grant, ergo the best place to fix it is to make sudo do the right thing in it's own configuration.

I understand however that enumerating all the allowed commands in the sudoers file, making any command runnable, or the workarounds with groups, permissions, setuid, etc, etc, etc are untenable for many people, not to mention the complexity when you get into AppArmor and SELinux, and the now emerging problems with cgroups and containerisation. Essentially the problem again is knowledge, do we encourage, or discourage "hacking" this in Ruby, rather than encourating, or discouraging learning how the underlying tool even works. How many people will use this feature and never know it's possible to solve this by reconfiguring sudo?

I think our industry generally suffers from sticking bandaids over problems that originate in lower parts of the stack of which we are ignorant, but the Ruby community in particular is keen on reinventing all the things in Ruby space, personally I prefer a polyglot world where you use the right tool for the job, even if that means you have to learn 5 new tools to get the job done :) otherwise, it's all just a race to the top, and a growing base of ignorance of how things really work, that's part of the general reason that I generally prefer not to expose channels, etc through SSHKit, they belong to a lower logical level, one with an easy to exercise API.

Like anything, if the implementation is good, and it solves a real problem, and it's tested and documented, the contributor has demostrated that for some subset of some people, this represents the correct way to solve the issue, and whilst I might not always be in 100% agreement, in this case I'm in favour of it, simply because the barrier from "read the Cap README" to "learn how to Ops" is quite a high one, and the risk of failure, syntax error in the sudoers file… well, it's usually fatal. At leat we all have solid configuration management though, right?

[leehambley]

@robd I wasn't exactly familiar with requiretty in sudo meant, so I checked:

requiretty
If set, sudo will only run when the user is logged in to a real tty. When this flag is set, sudo can only be run from a login session and not via other means such as cron(8) or cgi-bin scripts. This flag is off by default.

I also checked here, and it seems there's limited, legacy, not-really-valid-anymore reasoning behind it: http://unix.stackexchange.com/a/65789/10773

[robd]

@leehambley I will get this underway over in the https://github.com/capistrano/documentation repo.

Sounds about right, maybe in the FAQ, something like "how can I use this without passwordless sudo". I'd also like to ask if you could draw the comparison to GNU expect when you alter the docs, as I think for many 'old beards" (myself included), it's a tool we know, with a familiar API, and it then allows use-cases for expect to help cue people to get the best out of Cap.

Yes I will look more at expect and make the comparison. I hadn't looked at it when I proposed the interaction handler approach but it is a happy coincidence that I have stumbled onto a similar approach.

I understand however that enumerating all the allowed commands in the sudoers file, making any command runnable, or the workarounds with groups, permissions, setuid, etc, etc, etc are untenable for many people, not to mention the complexity when you get into AppArmor and SELinux, and the now emerging problems with cgroups and containerisation. Essentially the problem again is knowledge, do we encourage, or discourage "hacking" this in Ruby, rather than encouraging, or discouraging learning how the underlying tool even works. How many people will use this feature and never know it's possible to solve this by reconfiguring sudo?

Yes, I will also include a note to tell users think about the pros and cons of solving this in the sudoers config vs using a password. I think all you can do is try to get peoples' interest and point out other resources for them to explore. If someone is time strapped and/or inexperienced it is likely they will end up with something that is hacked together anyway, but if we can include links to resources (eg sudoers config, expect etc), then people may make a more informed decision about what is a good fit for them. Something like capistranorb.com is a very good place to get rubyists interested in other tools because it is widely read and referenced by the ruby community.

I generally prefer not to expose channels, etc through SSHKit, they belong to a lower logical level, one with an easy to exercise API.

Yes, I agree. I hope the sort of expect-like mapping we've discussed on #242 provides the right abstraction level for SSHKit. People only have to get involved with channels if they need to do something very unusual, in which case they (hopefully) know what they're getting into.

in this case I'm in favour of it, simply because the barrier from "read the Cap README" to "learn how to Ops" is quite a high one, and the risk of failure, syntax error in the sudoers file… well, it's usually fatal

OK that's great - thanks. I did lock myself out of my box at one stage recently :)

One other question I guess is whether you'd like to bake interactive sudo into Capistrano as a specific 'officially supported' option

ask(:interactive_sudo_password, '', echo: false) # or
set(:interactive_sudo_password, load_password_from_secrets()) 

It sort of feels like if we are going to give this our blessing, then we could bake a high quality well tested interactive sudo interaction handler into Capistrano itself, which might avoid support overhead of people incorrectly patching the DSL.

Another thing I have been thinking about, (and I think it might be a better way to handle this), is supporting interactive sudo at the SSHKit level by extending the command mapping capability. At the moment, if I understand it correctly, we support 2 mapping aspects for commands:

1. Adding a command prefix
2. Mapping a command to another command

One approach would be to allow mapping a command to some default options. This way, adding interactive sudo support could be achieved like this:

SSHKit.config.command_map.default_options[:sudo] = {
  interaction_handler: InteractiveSudo.new(fetch(:sudo_password))
}

This has the advantage that this would then hook other peoples' gems which use execute(:sudo, 'whatever') or capture(:sudo, whatever). In this case, InteractiveSudo which is an :interaction_handler implementation probably should live in a separate gem (unless you are prepared to have this SSHKit). Having it in SSHKit would have the major advantage that I could write functional tests for it using the existing vagrant setup (which is great), but you may feel that this is too much scope creep for SSHKit.

Obviously this is a bigger change than patching the capistrano DSL.

[leehambley]

omg, longest comment thread in the world lemme just address the important part:

One approach would be to allow mapping a command to some default options. This way, adding interactive sudo support could be achieved like this:

I like it, good idea!

[robd]

@leehambley :) Yeah, a bit of a stream of consciousness there - sorry. Could I ask you to have a look at my updates on #242. If you are happy, I would like to get this merged before I start work on supporting default_options.