From be952e573afb2d3894a1752d3df148d8a3399291 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Wed, 24 May 2017 01:48:42 -0400 Subject: [PATCH] nassh/ssh_client: rework how we pass sftp subsystem down Rather than re-use the -s argument to select the sftp subsystem, add a new parameter to the plugin API so we can signal this independently of the command line. By itself, this isn't terribly useful (and is a bit redundant), but in a follow up CL, it allows us to simplify the command line parsing on the JS side significantly. BUG=chromium:725292 Change-Id: I55d04617050a3f7c76e3ecf61138ef3fad2f711c Reviewed-on: https://chromium-review.googlesource.com/586672 Reviewed-by: Brandon Gilmore Tested-by: Mike Frysinger --- nassh/doc/hack.md | 1 + nassh/js/nassh_command_instance.js | 7 ++++--- ssh_client/openssh-7.5p1.patch | 28 +++++++++++++++++++++++++++- ssh_client/src/ssh_plugin.cc | 12 ++++++++++-- 4 files changed, 42 insertions(+), 6 deletions(-) diff --git a/nassh/doc/hack.md b/nassh/doc/hack.md index 234e1417c..f96629f55 100644 --- a/nassh/doc/hack.md +++ b/nassh/doc/hack.md @@ -253,6 +253,7 @@ The session object currently has these members: * array `arguments`: Extra command line options for ssh. * int `writeWindow`: Size of the write window. * str `authAgentAppID`: Extension id to use as the ssh-agent. +* str `subsystem`: Which subsystem to launch. ## NaCl->JS API diff --git a/nassh/js/nassh_command_instance.js b/nassh/js/nassh_command_instance.js index afa545f08..abbf91c00 100644 --- a/nassh/js/nassh_command_instance.js +++ b/nassh/js/nassh_command_instance.js @@ -706,6 +706,9 @@ nassh.CommandInstance.prototype.connectTo = function(params) { argv.environment = this.environment_; argv.writeWindow = 8 * 1024; + if (this.isSftp) + argv.subsystem = 'sftp'; + argv.arguments = ['-C']; // enable compression if (params.authAgentAppID) { @@ -742,9 +745,7 @@ nassh.CommandInstance.prototype.connectTo = function(params) { // If this is a SFTP connection, the remote command args don't make sense, // and will actually cause a problem. Since it's easy to do, just ignore // them here. - if (this.isSftp) { - argv.arguments.push('-s', 'sftp'); - } else if (commandArgs) { + if (!this.isSftp && commandArgs) { argv.arguments.push(commandArgs); } diff --git a/ssh_client/openssh-7.5p1.patch b/ssh_client/openssh-7.5p1.patch index 1a360171b..403186a22 100644 --- a/ssh_client/openssh-7.5p1.patch +++ b/ssh_client/openssh-7.5p1.patch @@ -19,10 +19,32 @@ */ int -main(int ac, char **av) -+ssh_main(int ac, char **av) ++ssh_main(int ac, char **av, const char *subsystem) { struct ssh *ssh = NULL; int i, r, opt, exit_status, use_syslog, direct, config_test = 0; +@@ -971,6 +971,21 @@ + /* Initialize the command to execute on remote host. */ + buffer_init(&command); + ++ if (subsystem) { ++ /* ++ * Hijack the codeflow now that we're done parsing the command line. ++ * We want all the flags, but none of the command line. Unless they ++ * passed in -s themselves. ++ */ ++ if (!subsystem_flag) { ++ subsystem_flag = 1; ++ av = xcalloc(2, sizeof(*av)); ++ av[0] = subsystem; ++ av[1] = NULL; ++ ac = 1; ++ } ++ } ++ + /* + * Save the command to execute on the remote host in a buffer. There + * is no limit on the length of the command, except by the maximum --- a/umac.c +++ b/umac.c @@ -1179,7 +1179,7 @@ struct umac_ctx { @@ -62,6 +84,10 @@ return SSH_ERR_SYSTEM_ERROR; /* close on exec */ + +daemonized() relies on funcs we don't implement (because we don't need them), +and this func is only used in sshd. disable it to avoid link failures. + --- a/misc.c +++ b/misc.c @@ -1257,6 +1257,7 @@ bind_permitted(int port, uid_t uid) diff --git a/ssh_client/src/ssh_plugin.cc b/ssh_client/src/ssh_plugin.cc index fd75a977b..cd3ad0078 100644 --- a/ssh_client/src/ssh_plugin.cc +++ b/ssh_client/src/ssh_plugin.cc @@ -40,6 +40,7 @@ const char kEnvironmentAttr[] = "environment"; const char kArgumentsAttr[] = "arguments"; const char kWriteWindowAttr[] = "writeWindow"; const char kAuthAgentAppID[] = "authAgentAppID"; +const char kSubsystemAttr[] = "subsystem"; // These are JavaScript method names as C++ code sees them. const char kPrintLogMethodId[] = "printLog"; @@ -52,7 +53,7 @@ const char kCloseMethodId[] = "close"; const size_t kDefaultWriteWindow = 64 * 1024; -extern "C" int ssh_main(int ac, const char** av); +extern "C" int ssh_main(int ac, const char** av, const char *subsystem); //------------------------------------------------------------------------------ @@ -247,11 +248,18 @@ void SshPluginInstance::SessionThreadImpl() { argv.push_back(username_hostname.c_str()); } + std::string subsystem; + const char *csubsystem = NULL; + if (session_args_.isMember(kSubsystemAttr)) { + subsystem = session_args_[kSubsystemAttr].asString(); + csubsystem = subsystem.c_str(); + } + LOG("ssh main args:\n"); for (size_t i = 0; i < argv.size(); i++) LOG(" argv[%d] = %s\n", i, argv[i]); - SendExitCode(ssh_main(argv.size(), &argv[0])); + SendExitCode(ssh_main(argv.size(), &argv[0], csubsystem)); } void* SshPluginInstance::SessionThread(void* arg) {