SSHKit - Non-passwordless sudo command support

[thbar]

Hello! I would like to discuss an enhancement ; this is a bit of a follow-up of:

• https://github.com/bitcrowd/sshkit.ex/issues/169

Situation

If one connects to a remote server, then issues a command that requires sudo, on a system where the user is not a password less sudo user (for improved security), there is no logic in SSHKit (if I understand) to transmit the user password to the command generation.

This can be reproduced when one comments this line before running mix test:

https://github.com/bitcrowd/sshkit.ex/blob/408141e2a73bffcaf0ab9098b82eb912202ef05a/test/support/docker/Dockerfile#L25

mix test

❯ mix test
Running ExUnit with seed: 657181, max_cases: 32
#SNIP

  1) test run/2 with group (SSHKitFunctionalTest)
     test/sshkit_functional_test.exs:103
     Assertion with == failed
     code:  assert status == 0
     left:  1
     right: 0
     stacktrace:
       test/sshkit_functional_test.exs:118: (test)

  2) test run/2 with user (SSHKitFunctionalTest)
     test/sshkit_functional_test.exs:86
     ** (ArgumentError) argument error
     code: IO.puts output
     stacktrace:
       (stdlib 6.2.2.1) io.erl:203: :io.put_chars(:standard_io, [[stderr: "sudo: a password is required\n"], 10])
       test/sshkit_functional_test.exs:97: (test)

  3) test run/2 with path, umask, user, group and env (SSHKitFunctionalTest)
     test/sshkit_functional_test.exs:123
     Assertion with == failed
     code:  assert status == 0
     left:  1
     right: 0
     stacktrace:
       test/sshkit_functional_test.exs:141: (test)

Finished in 22.3 seconds (22.3s async, 0.00s sync)
2 doctests, 159 tests, 3 failures

What is happening (IMO)

My understanding is that there is an implicit requirement that the user (as soon as a user is passed to the context) is passwordless-sudoer, and we go "non interactive" here with -n in that case:

https://github.com/bitcrowd/sshkit.ex/blob/408141e2a73bffcaf0ab9098b82eb912202ef05a/lib/sshkit/context.ex#L30

Proposal

Passing the password via a specific variable, making sure we obfuscate things properly in case of exception, and inject it interactively when requested, would let users like me cover more scenarios.

Prior art: ServerSpec core SpecInfra does this:

https://github.com/mizzy/specinfra/blob/master/lib/specinfra/backend/ssh.rb#L131-L132

(a lot of other automation tools also do something similar).

A solution similar to this could be to:

• pass a :sudo_password extra config flag
• run the command, but watch for the prompt for some time on STDIN
• reply to the prompt (or timeout)

I do not think it is possible to achieve this right now, without forking (but only had a quick first look).

What do you think?

[andreasknoepfle]

Hey @thbar,

I think this would be a great addition. I think it would make sense to add this to the context in some way, maybe when setting the user. Then the -S could be set as well on the command, to instruct sudo to read the password from stdin. We could also try to send the password and a newline over before we read the command output, so we do not need to wait on the password prompt to appear.

Theoretically you can already try to implement this with the internal SSH.Channel module and see if it works in general. Probably one also needs to call Channel.ptty/1.

Something like that (Untested):

{:ok, conn} = SSH.connect(host.name, host.options)
cmd = Context.build(context, command)
{:ok, chan} = Channel.open(conn)         
:success = Channel.ptty(chan)      
:success = Channel.exec(chan, cmd)  
:ok = Channel.send(chan, password <> "\n") 
Channel.loop(chan, {:cont, {[], nil}}, SSH.capture/2)

[andreasknoepfle]

Just did a basic test with the approach outlined above and it seems to work. The tricky bit is, that the password and the prompt come back as command output, so we need to somehow cut them off again to get the command output. We also need to add the -S and remove the -n property on the sudo when building the command. Also -p with a custom prompt could be helpful, since maybe we can cut off everything up to that point. We also might want to build in some kind of filter mechanism that filters passwords from the output as a last resort, not sure.