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

files.append with sudo failed #30

Open
yangshengBE opened this issue Nov 27, 2018 · 3 comments
Open

files.append with sudo failed #30

yangshengBE opened this issue Nov 27, 2018 · 3 comments

Comments

@yangshengBE
Copy link

Trying to use file.append with sudo option:

# ctx is fabric context
files.append(ctx, filename='/home/foo/.profile', text='export foo=bar', sudo=True)

The underneath cmd looks like:

sudo -S -p '[sudo] password: ' echo  'export foo=bar' >> /home/foo/.profile

and giving err msg:
bash: /home/foo/.profile: Permission denied
Also tried with runner_method='sudo' or runner=ctx.sudo and facing same issue

Found something might related
https://unix.stackexchange.com/questions/4335/how-to-insert-text-into-a-root-owned-file-using-sudo

@TmCTux
Copy link

TmCTux commented Apr 24, 2020

any update on this? I've faced the same error, it quite renders the whole patchwork lib unusable - we rarely use fabric for sys admin stuff without needing sudo.
thanks for any feedback.

@TmCTux
Copy link

TmCTux commented Apr 26, 2020

For people starting with fabric, I've put together the following workaround which works ok for the simple use case (default value for path is etc but can be changed or removed of course:

from patchwork.files import append,contains

@task                                                                           
def append_file(c, file, string, path='/etc'):                                  
     if contains(c,file,string,path) is not True:                                
         c.run('echo "{string}" | sudo tee -a {path}/{file} > /dev/null'.format(string=string,path=path,file=file))

@Script-Nomad
Copy link

Script-Nomad commented Feb 22, 2021

@TmCTux Thanks for the workaround. I was doing some experimentation and found another way that might be slightly easier and require less shenanigans with bash. I discovered that setting the sudopass in your fabric.Connection() context's configuration prevents calls to c.sudo() from prompting you for the password.

For those of you who are struggling with any of the helper functions in patchwork.files, the solution is to supply your sudopass to the fabric.Config overrides dict. This is an issue I've had with fabric since I started using it, that you cannot update this dictionary easily after you have established the SSH connection, but this is how you'd do it.

Code example

import fabric
import patchwork
from getpass import getpass

config = fabric.Config(
    overrides={
        'sudo': {'password': getpass('Sudo Password: ')}      # This will prevent sudo from prompting for a password
    }
)
ssh = fabric.Connection('sudouser@localhost:22', config=config)
print(patchwork.files.exists(ssh, path='/etc/hostname', sudo=True))
>>> True

Class-based approach

I have been writing a much bigger library that relies on fabric + patchwork, so it became necessary to classify all of this stuff, so I found this much easier to work with...

Here is the class, its __init__() and __exit__() to make things easier.

class SSHManager:
    def __init__(self, host, port=22, username='root', require_sudo=False, **connect_options):
        self._host = self._pinghost(host)
        self._user = username
        self._userpass = getpass(prompt='User/Sudo Password: ')
        try:
            self._agent = paramiko.agent.Agent()
            if len(self._agent.get_keys()) > 0:
                self._passphrase = None
            else:
                LOG.info("SSH Agent does not contain any private keys. Checking manually.")
                self._passphrase = getpass(prompt="SSH Private Key Passphrase")
        except paramiko.SSHException as ex:
            LOG.warning("Connection to SSH Agent could not be established. Trying to retrieve SSH Keys manually.")
            self._agent = None
            self._passphrase = getpass(prompt="SSH Private Key Passphrase: ")
        config = fabric.Config(
            overrides={'passphrase': self._passphrase,
                       'sudo': {'password': self._userpass},
                       **connect_options
                       }
        )
        LOG.info(f"Establishing SSH session to {username}@{host}:{port} ...")
        self._ssh = fabric.Connection(host=host, user=username, port=port, config=config)
        del config # Remember to destroy your configuration when done with it, since it is storing plaintext passwords
        if not self._getsudo() and require_sudo:
            LOG.error(f"Unable to proceed. Insufficient sudo permissions for user {self._user}")
            self.__exit__()
        self._home = self._ssh.run('pwd', hide=True).stdout.strip()

    def __exit__(self):
        del self._userpass
        self._ssh.close()
        del self._ssh

This class-based approach has a lot of advantages:

  1. It will automatically check for your SSH keys in your ssh agent and use them for authentication first.
  2. It will prompt you for an SSH Passphrase just in case your keys are encrypted and need to be unlocked. The class asks once, and then you don't have to worry about it anymore
  3. It will prompt you for your sudo password once, and insert it into your SSH connection's config automatically. It will never ask again.
  4. The sudo password also doubles as your SSH password, in case the connection fails to authenticate via SSH Keys. Please note I have not tested this with the above implementation, since my org exclusively uses SSH keys. You may need to insert Try/excepts for the fabric.exceptions.AuthFailed exception accordingly.
  5. The class destructor will ensure it destroys the passwords and objects containing passwords from memory when you're done with it. Fabric/Patchwork does not do this explicitly, so it's best to be safe. That being said, fabric likely leaves artifacts in memory unless you destroy the fabric.Connection() and fabric.Config() contexts ... so ... yeah, beware.

The beauty of this approach was that it makes all calls to the self._ssh methods so much simpler. I was able to make wrapper functions around patchwork to make python-equivalents to UNIX commands like grep, ls, mv, cp, rm, and so on. If ever I needed to add functionality to a new script, I could just inherit this class and add new functions easily. The only pitfall was that

Rant for how this should be fixed

The main problem is the extremely awkward and unspecific syntax required to enter the sudo user's password into the configuration, which is NOT well documented or clearly stated.

Considering how frequently users need sudo privileges for the actions they are performing with fabric, I would love to see a dedicated parameter in the fabric.Config class to prompt users for the sudo pass, ask once, then forget about it, and of course safely destroy it along with the class from the Config.__exit__() class.

Even better, it would be great to have a dedicated setter() for the sudopass, so that you aren't required to ask the user every single time if they don't add in this rather awkward hack.

I understand the desire for the overrides={} dict to make things easier for you (the fabric developers) from an input validation standpoint, but the user-friendliness leaves much to be desired with this approach, especially since the documentation does not explicitly provide these overrides in their own dedicated place within the documentation. If I could see all of the possible overrides available to the fabric.Connection() context, then the overrides would be MUCH easier to set. Having to dig through all of the documentation for these override settings is extremely tedious.

A simple helper function like the following would be awesome:

    def _setsudopass(self, password):
        self._ssh.config['sudo']['password'] = password
        del password

I also feel that fabric needs to do a better job of cleaning up password from memory. It should not be up to the user to do this. Python is well-known to leave behind artifacts in memory, and the library should be very careful to destroy its context managers every time they are closed or call exit() on their class. That's just my opinion as a paranoid security professional.

/rant

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

No branches or pull requests

3 participants