Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle EAGAIN automatically when writing to stdin buffer #165

Closed
zhujinhe opened this issue Mar 27, 2019 · 4 comments · Fixed by #257
Closed

Handle EAGAIN automatically when writing to stdin buffer #165

zhujinhe opened this issue Mar 27, 2019 · 4 comments · Fixed by #257

Comments

@zhujinhe
Copy link

sudo -S does not require a password when cached credential is still available.

Output might be flushed unexpectedly when there is no password required.

Code like the tutorial:

client = <..>
output = client.run_command('whoami', sudo=True)
for host in output:
    stdin = output[host].stdin
    print('start write')
    stdin.write('%s\n' % password)
    print('before flash', stdin.read())
    stdin.flush()
    print('after flush', stdin.read())

client.join(output)
print('++++++++++')
for host, host_output in output.items():
    for line in host_output.stdout:
        print(host, line)

Output:

start write
before flash (5, b'root\n')    # output flashed  unexpectedly
after flush (0, b'')
start write
before flash (-37, b'')
after flush (-37, b'')
++++++++++
host1 root
# only 1 of 2 output in host_output.stdout.

Here is my propose/question:
A: Make sure sudo password is required every time by changing sudo -S to sudo -Sk or so in pssh/clients/native/single.py#L437.

B: How can we do not flush stdin when there is no keyword show up. Any help (document/code) is appreciated.

@zhujinhe
Copy link
Author

zhujinhe commented Mar 27, 2019

I finally got the output as I expected after some dig.

from ssh2.error_codes import LIBSSH2CHANNEL_EAGAIN
# ...
output = client.run_command('whoami', sudo=True, stop_on_errors=False)

for host in output:
    stdin = output[host].stdin
    if stdin is not None:
        size_or_error_codes, data = stdin.read(0)
        if size_or_error_codes == LIBSSH2CHANNEL_EAGAIN:  # -37
            stdin.write('%s\n' % password)
            stdin.flush()
client.join(output)

for host, host_output in output.items():
    for line in host_output.stdout:
        print(host, line)

It seems flush() and read() method will consume the stdin data.

Any suggestion is appreciated if there's room for improvement.

ps: newbee to pssh ^_^.

@pkittenis
Copy link
Member

Hi there,

Thanks for the interest and report.

stdin.read() will consume the buffer. Only flush is needed after write - per example in documentation.

The other issue is that write can block and return EAGAIN - this is not handled by the client and is not documented, so there is room for improvement there. I think the library should wrap stdin with the _eagain function so that is handled automatically by the client without boiler plate code.

@pkittenis pkittenis changed the title better sudo implementation needed. Handle EAGAIN automatically when writing to stdin output buffer Mar 28, 2019
@pkittenis pkittenis changed the title Handle EAGAIN automatically when writing to stdin output buffer Handle EAGAIN automatically when writing to stdin buffer Mar 28, 2019
@zhujinhe
Copy link
Author

sudo -S does not require a password when cached credential is still available.

None-password-required output data will be lost.

So, only flush() after being asked for a password, I'm right?

output = client.run_command('whoami', sudo=True)
for host in output:
    stdin = output[host].stdin
    if stdin is not None:
        size_or_error_codes, data = stdin.read(0)
        # retry handle wrong password
        while size_or_error_codes == LIBSSH2CHANNEL_EAGAIN:  # -37
            stdin.write('%s\n' % password)
            stdin.flush()
            time.sleep(0.1)
            size_or_error_codes, data = stdin.read(0)

@pkittenis
Copy link
Member

I don't understand what "None-password-required output data will be lost." means. Stdin is an input buffer, it does not provide output nor should it be read from. Output is in stdout.

EAGAIN for stdin should be handled on call to .write, not read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants