Add '-h 127.0.0.1' to sudo parameters to avoid running commands twice #76

zvin · 2018-09-06T12:11:53Z

When sudo can't resolve the hostname, it stderrs
sudo: unable to resolve host <hostname> but still runs the command.
This results in the command being launched twice:

once by sudo
once by kdesudo because the stderr of sudo is interpreted as an error

This is to avoid running the child-writer twice when the hostname isn't set in /etc/hosts. See jorangreef/sudo-prompt#76 Change-type: patch Signed-off-by: Alexis Svinartchouk <alexis@resin.io>

jhermsmeier · 2018-09-06T12:44:43Z

index.js

+  // Use localhost as sudo may stderr 'sudo: unable to resolve host <hostname>'
+  // if the hostname is not in /etc/hosts while still executing the command.
+  command.push('-h');
+  command.push('localhost');


Should this maybe use 127.0.0.1 instead, as localhost doesn't necessarily resolve to 127.0.0.1?

Good point.

@jorangreef the PR is already updated to use 127.0.0.1

Thanks, I saw. Just wanted to say that 127.0.0.1 was a good suggestion from @jhermsmeier.

When sudo can't resolve the hostname, it stderrs 'sudo: unable to resolve host <hostname>' but still runs the command. This results in the command being launched twice: * once by sudo * once by kdesudo because the stderr of sudo is interpreted as an error

This is to avoid running the child-writer twice when the hostname isn't set in /etc/hosts. See jorangreef/sudo-prompt#76 Change-type: patch Signed-off-by: Alexis Svinartchouk <alexis@resin.io>

lurch · 2018-09-06T13:48:33Z

I just tried doing some tests on my local Ubuntu 14.04 laptop, and as you say if I take my hostname out of /etc/hosts and run e.g. sudo ls then I get sudo: unable to resolve host shyknee on stderr but it still asks me for my password and runs the command.
However if (in a new window, to git rid of sudo's "password cache") I run either sudo -h localhost ls or sudo -h 127.0.0.1 ls I still get sudo: unable to resolve host shyknee printed to stderr 😕

Perhaps a better fix is to look for (and explicitly ignore) any stderr response starting with sudo: unable to resolve host ?

EDIT: In case it helps:

andrew@shyknee:~$ uname -a
Linux shyknee 3.19.0-82-generic #90~14.04.1-Ubuntu SMP Thu Feb 23 01:12:44 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
andrew@shyknee:~$ dpkg -l sudo
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name                Version        Architecture   Description
+++-===================-==============-==============-===========================================
ii  sudo                1.8.9p5-1ubunt amd64          Provide limited super user privileges to sp
andrew@shyknee:~$ sudo -h 127.0.0.1 ls
sudo: unable to resolve host shyknee
[sudo] password for andrew: 
<snipped ls output>

jorangreef · 2018-09-07T09:16:41Z

sudo: unable to resolve host but still runs the command.

Thanks, great find @zvin !

I was able to reproduce this on Ubuntu 13.10 with the following script after changing my hostname to something unresolvable:

var command = [];
command.push('/usr/bin/sudo');
command.push('-n');
command.push('-E');
command.push('--');
command.push('ls');
command = command.join(' ');
var cp = require('child_process').exec(command, {},
  function(error, stdout, stderr) {
    console.log('error: ' + JSON.stringify(error));
    console.log('stdout: ' + JSON.stringify(stdout));
    console.log('stderr: ' + JSON.stringify(stderr));
    if (error) console.log('error.code: ' + error.code);
    console.log('cp.exitCode: ' + cp.exitCode);
    console.log('cp.killed: ' + cp.killed);
  }
);

Here are the results:

NO SUDO SESSION AVAILABLE:
error: {"killed":false,"code":1,"signal":null,"cmd":"/usr/bin/sudo -n -E -- ls"}
stdout: ""
stderr: "sudo: a password is required\n"
error.code: 1
cp.exitCode: 1
cp.killed: false

SUDO SESSION AVAILABLE (BUT UNABLE TO RESOLVE HOSTNAME):
error: null
stdout: "idempotent.js\n"
stderr: "sudo: unable to resolve host test123\n"
cp.exitCode: 0
cp.killed: false

SUDO SESSION AVAILABLE:
error: null
stdout: "idempotent.js\n"
stderr: ""
cp.exitCode: 0
cp.killed: false

The issue, as you say, is that sudo-prompt is not idempotent, and that's because we check and use stderr to determine whether or not the command was run, when in fact stderr can be present even in the event of success.

In the past, we used to check stderr against a specific error: sudo: a password is required so this was not an issue, but this was changed to just checking against sudo: in d08ec68 to accommodate various system languages.

And now we're getting a casual sudo: error in stderr, but the child process is not actually exiting with an error code.

Perhaps a better fix is to look for (and explicitly ignore) any stderr response starting with sudo: unable to resolve host ?

Thanks @lurch, I agree we shouldn't use -h to work around this because the root cause is that we're checking stderr when in fact we should be checking error or exitCode first.

Although, we don't actually need to check for sudo: unable to resolve host for our purposes (this won't work on non-English systems anyway), since a failed sudo: a password is required can be distinguished from a successful sudo: unable to resolve host just using error or exitCode.

In other words, we should only call Linux() or Mac() if error is present and stderr contains the sudo: prefix. This should do the trick:

if (error && /sudo: /i.test(stderr)) {

zvin · 2018-09-07T09:23:01Z

@jorangreef there's an issue with this approach: the program being elevated may exit with an expected error status. You'll still launch it 2 times in that case.

lurch · 2018-09-07T09:31:33Z

this won't work on non-English systems anyway

Can't you manually override the locale in the temporary execution environment you're creating to force English?

jorangreef · 2018-09-07T09:36:38Z

Ah yes. We could then either:

Check against error but add an exception for unable to resolve... (it looks like sudo doesn't translate that string: https://github.com/millert/sudo/search?q=unable+to+resolve+host&unscoped_q=unable+to+resolve+host it uses an empty msgstr in the language files). We might need to add a few more exceptions for other potential non-fatal stderr messages from sudo.
Alternatively, wrap the user's command in our own script, so we can know for sure that the user's command ran.
According to the manpage, does sudo provide any other indication of success when using -n?

jorangreef · 2018-09-07T09:41:46Z

Can't you manually override the locale in the temporary execution environment you're creating to force English?

I think this would also affect the user's command output, so we can't do that.

zvin · 2018-09-07T09:42:49Z

From the sudo manpage

Upon successful execution of a command, the exit status from sudo will be the exit status of the program that was executed.
...
Otherwise, sudo exits with a value of 1 if there is a configuration/permission problem or if sudo cannot execute the given command.

You can't differentiate between an auth failure or a program exiting with 1.

Passing -h 127.0.0.1 seems simple and future proof.

zvin · 2018-09-07T09:44:05Z

Or you could wrap the user's program in a script that would add 1 to its exit code, but that seems like a hack.

jorangreef · 2018-09-07T09:44:27Z

Passing -h 127.0.0.1 seems simple and future proof.

@lurch showed a case where -h doesn't work?

jorangreef · 2018-09-07T09:46:05Z

Another idea: we could use the -n option just to launch a placebo, something we know will work if a session exists. So we stop using -n to do two things (test the sudo session and run the user's command). We just use it to test the sudo session.

zvin · 2018-09-07T09:47:55Z

I don't think @lurch's case can happen when using the -n option.

lurch · 2018-09-07T09:49:16Z

andrew@shyknee:~$ sudo -h 127.0.0.1 -n ls /root
sudo: unable to resolve host shyknee
sudo: a password is required

😕

EDIT...

andrew@shyknee:~$ sudo -n ls /root
sudo: unable to resolve host shyknee
sudo: a password is required

Adding the -h 127.0.0.1 seems to make no difference at all?

zvin · 2018-09-07T09:49:58Z

@lurch that's ok, it's going to get launched by kdesudo in that case.

zvin · 2018-09-07T10:03:14Z

Adding the -h 127.0.0.1 seems to make no difference at all?

@lurch strange, I don't get this behaviour.

lurch · 2018-09-07T10:47:21Z

@zvin Hmmm, are you remembering to use sudo -k between tests (when necessary) to invalidate the cached credentials? Perhaps different versions of sudo behave slightly differently? (I included the version of sudo my distro is using in an earlier comment)

zvin · 2018-09-07T10:57:04Z

@lurch I was running sudo while already being root. So it didn't need any credential.

zvin · 2018-09-07T10:58:17Z

I was testing with sudo v1.8.21p2

jorangreef · 2018-09-07T11:13:48Z

Perhaps the simplest, safest option is to remove the sudo -n call from Attempt() completely and branch out from within Attempt() directly to Linux() and Mac().

The only downside is that a prompt will be shown even if a sudo session exists. Then again, most Linux platforms have TTY tickets enabled, and macOS started enabling TTY tickets a year or two back so this is not likely a problem.

lurch · 2018-09-07T11:20:25Z

Hmmm, I've just tried switching to root and running sudo from there, and no matter which combinations of -n and -h 127.0.0.1 I try, it always displays sudo: unable to resolve host shyknee :-/

zvin · 2018-09-07T11:47:46Z

Perhaps the simplest, safest option is to remove the sudo -n call from Attempt() completely and branch out from within Attempt() directly to Linux() and Mac()

I agree

This is to avoid running the child-writer twice when the hostname isn't set in /etc/hosts. See jorangreef/sudo-prompt#76 Change-type: patch Signed-off-by: Alexis Svinartchouk <alexis@resin.io>

jorangreef · 2018-09-11T13:23:23Z

Fixed in 8c1a302

This is to avoid running the child-writer twice when the hostname isn't set in /etc/hosts. See jorangreef/sudo-prompt#76 Change-type: patch Signed-off-by: Alexis Svinartchouk <alexis@resin.io>

This was referenced Sep 6, 2018

fix(sudo-prompt): Use a fork of sudo-prompt balena-io/etcher#2469

Merged

Fix flash-after-verify in Etcher integration balena-io-modules/etcher-sdk#12

Closed

jhermsmeier reviewed Sep 6, 2018

View reviewed changes

zvin changed the title ~~Add '-h localhost' to sudo parameters to avoid running commands twice~~ Add '-h 127.0.0.1' to sudo parameters to avoid running commands twice Sep 6, 2018

jorangreef closed this Sep 11, 2018

jorangreef referenced this pull request Dec 10, 2018

Run command idempotently when hostname cannot be resolved

8c1a302

build mentioned this pull request Jun 24, 2019

feat(updates): add update functionality tidev/pulsar-titanium#162

Merged

build mentioned this pull request Jul 15, 2019

chore(release): release v1.8.0 tidev/pulsar-titanium#169

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add '-h 127.0.0.1' to sudo parameters to avoid running commands twice #76

Add '-h 127.0.0.1' to sudo parameters to avoid running commands twice #76

zvin commented Sep 6, 2018 •

edited

Loading

jhermsmeier Sep 6, 2018

jorangreef Sep 7, 2018

zvin Sep 7, 2018

jorangreef Sep 7, 2018

lurch commented Sep 6, 2018 •

edited

Loading

jorangreef commented Sep 7, 2018 •

edited

Loading

zvin commented Sep 7, 2018

lurch commented Sep 7, 2018

jorangreef commented Sep 7, 2018 •

edited

Loading

jorangreef commented Sep 7, 2018 •

edited

Loading

zvin commented Sep 7, 2018 •

edited

Loading

zvin commented Sep 7, 2018 •

edited

Loading

jorangreef commented Sep 7, 2018

jorangreef commented Sep 7, 2018

zvin commented Sep 7, 2018

lurch commented Sep 7, 2018 •

edited

Loading

zvin commented Sep 7, 2018

zvin commented Sep 7, 2018 •

edited

Loading

lurch commented Sep 7, 2018

zvin commented Sep 7, 2018

zvin commented Sep 7, 2018

jorangreef commented Sep 7, 2018

lurch commented Sep 7, 2018

zvin commented Sep 7, 2018

jorangreef commented Sep 11, 2018

Add '-h 127.0.0.1' to sudo parameters to avoid running commands twice #76

Add '-h 127.0.0.1' to sudo parameters to avoid running commands twice #76

Conversation

zvin commented Sep 6, 2018 • edited Loading

jhermsmeier Sep 6, 2018

Choose a reason for hiding this comment

jorangreef Sep 7, 2018

Choose a reason for hiding this comment

zvin Sep 7, 2018

Choose a reason for hiding this comment

jorangreef Sep 7, 2018

Choose a reason for hiding this comment

lurch commented Sep 6, 2018 • edited Loading

jorangreef commented Sep 7, 2018 • edited Loading

zvin commented Sep 7, 2018

lurch commented Sep 7, 2018

jorangreef commented Sep 7, 2018 • edited Loading

jorangreef commented Sep 7, 2018 • edited Loading

zvin commented Sep 7, 2018 • edited Loading

zvin commented Sep 7, 2018 • edited Loading

jorangreef commented Sep 7, 2018

jorangreef commented Sep 7, 2018

zvin commented Sep 7, 2018

lurch commented Sep 7, 2018 • edited Loading

zvin commented Sep 7, 2018

zvin commented Sep 7, 2018 • edited Loading

lurch commented Sep 7, 2018

zvin commented Sep 7, 2018

zvin commented Sep 7, 2018

jorangreef commented Sep 7, 2018

lurch commented Sep 7, 2018

zvin commented Sep 7, 2018

jorangreef commented Sep 11, 2018

zvin commented Sep 6, 2018 •

edited

Loading

lurch commented Sep 6, 2018 •

edited

Loading

jorangreef commented Sep 7, 2018 •

edited

Loading

jorangreef commented Sep 7, 2018 •

edited

Loading

jorangreef commented Sep 7, 2018 •

edited

Loading

zvin commented Sep 7, 2018 •

edited

Loading

zvin commented Sep 7, 2018 •

edited

Loading

lurch commented Sep 7, 2018 •

edited

Loading

zvin commented Sep 7, 2018 •

edited

Loading