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

allowing synchronize to elevate permissions when sudo requires password entry - implements #334 #335

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

markasbach
Copy link

SUMMARY

This PR implements feature request #334, so that ansible.posix.synchronize will work with become: yes even if the ssh account is not set up for passwordless sudo.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

ansible.posix.synchronize

ADDITIONAL INFORMATION

Steps to reproduce:

  • On the target machine, set up an account with publickey authorization that is part of the sudo group, but make sure it has not been set up for NOPASSWD in the sudoers configuration.
  • Use ansible.posix.synchronize with become: yes and use the account described above.
  • The module will fail to complete because the invocation of sudo rsync on the remote machine brings up a password prompt and lets rsync fail.

With this PR, it will no longer fail.

I've been trying to follow the community guidelines for setting up this PR, but I will need some help regarding the following topics:

  • I'm not sure if I can use a hardcoded /bin/sh for invoking bash within the wrapping code that I introduce in plugins.modules.synchronize.py. Hope this is all correct.
  • Could someone cross-check I've not broken quoting / escaping on the strings that I manipulate?
  • I don't know how I can add testing for this feature or actually test if my changes break anything. The repository seems to fail tests on github already.

Thanks in advance ;-)

@softwarefactory-project-zuul

This comment was marked as outdated.

@maxamillion
Copy link
Collaborator

Closing and reopening to kick CI.

@maxamillion maxamillion closed this Jan 9, 2024
@maxamillion maxamillion reopened this Jan 9, 2024
Copy link
Contributor

@maxamillion
Copy link
Collaborator

Hello @markasbach! My most sincere apologies for the lag time on this. Thank you so much for the contribution. If you're still interested in getting this merged I wanted to follow-up with you on it.

When I tested this patch, it resulted in a rsync error:

100.80.246.157 | FAILED! => {
    "changed": false,
    "cmd": "/usr/bin/rsync --delay-updates -F --compress --archive --rsh='/bin/sh -c \"{ echo $BECOME_PASS; cat - ; } | /usr/bin/ssh -S none -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null $0 $* &\"' --rsync-path='sudo -S -u root rsync' --out-format='<<CHANGED>>%i %n%L' /tmp/foo 100.80.246.157:/tmp/bar",
    "invocation": {
        "module_args": {
            "_local_rsync_password": null,
            "_local_rsync_path": "rsync",
            "_ssh_wrapper": true,
            "_substitute_controller": false,
            "archive": true,
            "checksum": false,
            "compress": true,
            "copy_links": false,
            "delay_updates": true,
            "delete": false,
            "dest": "100.80.246.157:/tmp/bar",
            "dest_port": null,
            "dirs": false,
            "existing_only": false,
            "group": null,
            "link_dest": null,
            "links": null,
            "mode": "push",
            "owner": null,
            "partial": false,
            "perms": null,
            "private_key": null,
            "recursive": null,
            "rsync_opts": [],
            "rsync_path": "sudo -S -u root rsync",
            "rsync_timeout": 0,
            "set_remote_user": true,
            "src": "/tmp/foo",
            "ssh_args": null,
            "ssh_connection_multiplexing": false,
            "times": null,
            "verify_host": false
        }
    },
    "msg": "100.80.246.157: 1: Syntax error: end of file unexpected (expecting \"}\")\nrsync: connection unexpectedly closed (0 bytes received so far) [sender]\nrsync error: error in rsync protocol data stream (code 12) at io.c(231) [sender=3.2.7]\n",
    "rc": 12
}

I'm also mildly concerned about the possibility of the password leaking but I've yet to find a way that might happen. Honestly, it might be perfectly fine and there's no real world potential of the password leaking, but I'm always paranoid I'm missing something 😉

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

Successfully merging this pull request may close these issues.

2 participants