1
\$\begingroup\$

The idea

At my work, I'm tasked with retrieving logs for a list of online orders from a total of 12 servers. To do this, I SSH into them (with a jump host in between) one by one, run the script for every order, then parse the output. Overall, a very tedious process that I'm sure can be automated.

W -> J -> S

W = My Windows VM

J = Red Hat jump box

S = The Red Hat production servers

So the basic idea is the following:

  1. Connect to every server by SSH

  2. On every server, run the commands for all orders which are passed by command line

  3. Associate each output with the order

Here is what I've come up with so far, and it is working on my home lab. But before I bring it in to work to propose it and try it on production servers, I want to know how the code can be cleaned up or done more efficiently.

The code

require 'net/ssh'
require 'io/console'
require 'pp'

ORDERS = ARGV.dup()     
ARGV.clear()            

SERVERS = [
    "prod1",
    "prod2",
    "prod3"
]

COMMAND = "aux_search.sh"

CONFIG = "C:/Users/myuser/.ssh/config"

RESULTS = ORDERS.each_with_object({}) { |k, v| v[k] = "" }

puts("Enter your username: ")
USERNAME = gets().chomp()
puts("Enter your password for accessing the servers: ")
SERVERPASSWORD = STDIN.noecho(&:gets).chomp()
puts("Enter your sudo password: ")
SUDOPASSWORD = STDIN.noecho(&:gets).chomp()

SESSIONS = []
SERVERS.each do |server|
    session = Net::SSH.start(server, USERNAME, :password => SERVERPASSWORD, :config => CONFIG, 
        :verbose => :debug, :auth_methods => ["publickey", "password"], :verify_host_key => :accept_new)

    SESSIONS << session
end

SESSIONS.each do |ssh|
    ORDERS.each do |order|
        ssh.open_channel() do |channel|
            channel.on_data do |_, data|
                RESULTS[order].concat(data)
                if data.match(/\[sudo\]/)  
                    channel.send_data("#{SUDOPASSWORD}\n")
                end
            end

            channel.request_pty() do |_, success|
                raise "Failed to request TTY for order #{order}" unless success
            end

            channel.exec("sudo ./#{COMMAND} #{order}") do |_, success| 
                raise "Could not execute command #{COMMAND} #{order}" unless success
            end
        end
    end
    ssh.loop()
end

puts("===Results===")
pp(RESULTS)

My ssh config

Host jumpbox
    HostName 192.168.16.2
    User johrus2
    IdentityFile C:\Users\myuser\.ssh\id_rsa3
    Port 22

Host prod1
    HostName 192.168.16.3
    User johrus2
    IdentityFile C:\Users\myuser\.ssh\id_rsa3
    ProxyJump jumpbox
    Port 22

Host prod2
    HostName 192.168.16.4
    User johrus2
    IdentityFile C:\Users\myuser\.ssh\id_rsa3
    ProxyJump jumpbox
    Port 22

Host prod3
    HostName 192.168.16.5
    User johrus2
    IdentityFile C:\Users\myuser\.ssh\id_rsa3
    ProxyJump jumpbox
    Port 22

```
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

I think this is interesting code. I admit I haven't dug much into Ruby's Net::SSH library, so I bet you've learned some things about it in writing this code. It sounds to me like your biggest concern now is about professionalism, stability, and dependability which is why you want feedback before offering to use this code in a production environment. If so, I feel like you're asking all the right questions.

Here are my suggestions:

  • Search for prior art
  • Don't touch passwords if you can help it, and other security practices
  • Add some tests

Prior Art

You haven't mentioned yet if you've looked into existing tools that have already done this work. For example, you could use Capistrano, which is written in Ruby and designed for the type of scenario you're describing. Capistrano is fairly well-maintained. You could put it on your jump box, if you wanted. If Capistrano looks too cumbersome for what you're doing, you may want to use its sshkit library, which handles some of the ssh concerns you're navigating manually here.

Security Practices

I see you have public/private keypairs for every server you're connecting to. Try to use them instead of getting user input for a password. I would lean on the side of requiring them for anyone using a tool like this. In fact, I might have a distinct identity for automation so that I can revoke or recreate its keys without affecting any other way I can log in to the system. You may also want to add logging so you can audit your script to make sure it did what you intended. This is also helpful as a changelog to look back on when your script worked fine and you're troubleshooting unexpected side-effects of the commands it ran.

The nice thing about this specific identity is you can also leverage it to avoid passing sudo passwords around. This identity could have a limited list of commands it can run either in your sudoers file or without needing sudo. For example, I have used Capistrano to log in as a user that does not have root permissions, but does have elevated permissions that my basic account does not have. This elevated user had file permissions so it could update the code for the servers' websites without requiring sudo, though it couldn't do too much else.

More generally, I get nervous whenever I'm writing any code that handles user input for security credentials. I'd say that if there's already an existing project where flaws in handling security are likely to result in the announcement of a public CVE, it's better to use those projects for than writing your own security implementations to use in production.

In summary

  • use something more likely to get security audits than you are
  • have a layered plan for what you do if something goes wrong or the system is abused

Add Some Tests

Do you know what errors SSH is likely to raise if something goes wrong? Should that stop the whole process without continuing? Does your code do that in a way you're happy with?

These are the kind of answers that tests are good at answering for you. One thing I like to keep in mind when writing separate test code feels like a waste of time is the saying, "Everyone tests their code. Some people then throw the tests away, and some people keep them."

I suspect, looking at your code, it would be hard to stub out the ssh library to simulate what happens when when several commands succeed and then one fails. (A discussion of how to stub and mock that sort of thing in Ruby tests feels to big to handle in this answer. Let me know if you have trouble finding guides for it online.) The fact that writing test you'd like to have feels difficult is an excellent sign that you should split up your code differently from how you've written it.

As an example, you could have one chunk of code that accepts some generic connection management object, and then worries about the sequencing of commands and handling failed commands. Then you could have another chunk of code that is the connection object, abstracting the SSH library down to the few parts of it you care about for this script. Maybe there's an even better way to make that split that you'll find once you sit down to it.

Either way, once you reorganize your code so the tests are easier to write, you'll also find it gets easier to make changes, fix bugs, add new features even if you were to never run the tests again. (But you should run the tests again — you already wrote them, and they're just sitting there. No need to waste the effort.)

Good Luck Out There

I hope this helps. Feel free to ask me for further clarification on any of this if you want.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Hey! Just wanted to thank you for this. You and someone else on a different website suggested Capistrano/SSHKit, so I did some looking into those. You were very right when you said that SSHKit handled the stuff I was trying to do manually, and it did it much more elegantly than I did. So ultimately, I decided on it over Net:SSH and was able to write a much more clean and robust script than this one. I did take your advice and write tests for it to show that I'd thought through what could go wrong, and it was well received. Thank you for your review! \$\endgroup\$
    – JRTN
    Commented Jun 29, 2020 at 3:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.