1
\$\begingroup\$

I wrote some code to execute remote commands over ssh, using expect, from c++. The code works but is a bit of a mess of c / c++ since there doesn't seem to be an idiomatic way to achieve much of what I wanted in pure c++. I appreciate that there are nominally some security concerns with the way I'm using passwords, but these aren't a concern for my use cases.

remote_commmand.h

#pragma once

#include <stdexcept>

struct RemoteException : public std::runtime_error {
    using std::runtime_error::runtime_error;
};

void remote_command(const std::string &host, const std::string &user, const std::string &password, const std::string &cmd);

remote_command.cpp

#include "remote_command.h"
#include <cstdio>
#include <cstring>
#include <unistd.h>
#include <fstream>
#include <vector>

using std::string;

namespace {

const char* expect_template =
"set timeout 60\n"
"spawn ssh %s@%s %s\n"
"expect {\n"
"   \"*yes/no*\"\n"
"   {\n"
"       send \"yes\\r\"\n"
"       exp_continue\n"
"   }\n"
"   \"*?assword:*\"\n"
"   {\n"
"       send -- \"%s\\r\"\n"
"       send -- \"\\r\"\n"
"       expect eof\n"
"   }\n"
"}\n";

class TempFile {

public:
    explicit TempFile(const char* contents){
        file_name = std::tmpnam(nullptr);
        std::ofstream f(file_name.c_str(), std::ios::out);
        if(!f.write(contents, std::strlen(contents))){
            throw std::runtime_error("ofstream");
        }
    }

    ~TempFile(){
        std::remove(file_name.c_str());
    }

    TempFile(TempFile &other) = delete;
    TempFile& operator=(TempFile &other) = delete;

    std::string file_name;
};

}


void remote_command(const string &host, const string &user, const string &password, const string &cmd){
    std::vector<char> formatted_cmd;
    const std::size_t formatted_size = std::strlen(expect_template) + host.size() + user.size() + password.size() + cmd.size();
    formatted_cmd.resize(formatted_size);

    const std::size_t res = std::snprintf(&formatted_cmd[0], formatted_size, expect_template, user.c_str(), host.c_str(), cmd.c_str(), password.c_str());

    if (res <= 0 || res >= formatted_size ){
        throw RemoteException("infsufficient size for snprintf");
    }

    TempFile tempfile(&formatted_cmd[0]);

    int r = execl("/usr/bin/expect", "-f", tempfile.file_name.c_str(), (char *)0);
    if (r == -1){
        throw RemoteException(string("execl") + std::strerror(errno) );
    }
}

example usage

remote_command("my-host", "user", "pw", "/path/to/my/script.sh");
\$\endgroup\$
2
\$\begingroup\$

Consider using raw literals

Your expect_template is a prime candidate to be written as a raw string literal:

const char* expect_template =
R"(set timeout 60
spawn ssh %s@%s %s
expect {
   "*yes/no*"
   {
       send "yes\r"
       exp_continue
   }
   "*?assword:*"
   {
       send -- "%s\r"
       send -- "\r"
       expect eof
   }
}
)";

I may have missed something in the editing, but the basic idea is pretty simple: you don't use escapes at all, just insert exactly what you want the string to contain (including new-lines).

Given an R immediately before the quote, the compiler treats the content as a raw literal. The opening delimiter is the quote, other optional "stuff", and an open paren. The closing delimiter is a close paren, the same optional stuff, and the close quote. In your case, the optional "stuff" isn't needed--you'd use it if you might need to include )" as part of your string. In that case, you might have something like R"<^>( at the beginning so the end of the string would only be signaled by )<^>" (the exact content doesn't matter a whole lot--you just have to be sure it's something that can't occur inside the string).

Use of .c_str()

C++98/03 required that you use s.c_str() when passing s to a stream's constructor or open member function. C++11 eliminated that requirement, so you can just pass the string directly.

Passing strings as parameters

Absent a reason to do otherwise, I'd at least consider passing std::string const & as the parameter to (for one example) TempFile::TempFile. This can simplify some of the content a little, and still lets you pass a C-style string when/if you want:

explicit TempFile(std::string const &contents) {
    file_name = std::tmpnam(nullptr);
    std::ofstream f(file_name);
    if(!(f << contents)) {
        throw std::runtime_error("writing ofstream");
    }
}

Use of snprintf

You might want to consider using a stringstream instead of snprintf. This does ease at least a few parts of the job, although it's not exactly perfect either.

std::ostringstream s;

s << 
R("set timeout 60
spawn ssh )" << user << "@" host << " " command << R"(
expect {
   "*yes/no*"
   {
       send "yes\r"
       exp_continue
   }
   "*?assword:*"
   {    
       send -- ")"
<< password << R"(\r"
        send -- "\r"
        expect eof
    }
}
)";

TempFile f(s.str());
\$\endgroup\$
  • \$\begingroup\$ Great, thanks for the raw literals. I looked into using them, but the fact they could be used multiline totally passed me by. \$\endgroup\$ – cloakedlearning Aug 16 '16 at 19:48
  • \$\begingroup\$ Unless I'm missing something basic_ofstream::write must take a const char* not a std::string so I still need the c_str() at that call site. \$\endgroup\$ – cloakedlearning Aug 16 '16 at 20:10
  • \$\begingroup\$ @cloakedlearning: Oops--yes, you do. Sorry 'bout that. \$\endgroup\$ – Jerry Coffin Aug 16 '16 at 20:47
  • \$\begingroup\$ No problem. Seems like an odd omission to me. Hopefully string_view will help clean things up a bit in c++17 \$\endgroup\$ – cloakedlearning Aug 16 '16 at 21:02
  • \$\begingroup\$ @cloakedlearning: Modified code to do it the obvious way. \$\endgroup\$ – Jerry Coffin Aug 16 '16 at 21:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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