From 59068ca95527c39ff5fcf5aabc159124ab76b72b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 27 Jan 2019 20:16:35 +0000 Subject: [PATCH 01/19] tests: give ansible/gcloud/ its own requirements file. --- tests/ansible/gcloud/requirements.txt | 1 + tests/ansible/requirements.txt | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 tests/ansible/gcloud/requirements.txt diff --git a/tests/ansible/gcloud/requirements.txt b/tests/ansible/gcloud/requirements.txt new file mode 100644 index 00000000..0df6728c --- /dev/null +++ b/tests/ansible/gcloud/requirements.txt @@ -0,0 +1 @@ +google-api-python-client==1.6.5 diff --git a/tests/ansible/requirements.txt b/tests/ansible/requirements.txt index 7cab6681..47ed9abb 100644 --- a/tests/ansible/requirements.txt +++ b/tests/ansible/requirements.txt @@ -1,7 +1,6 @@ ansible; python_version >= '2.7' ansible<2.7; python_version < '2.7' paramiko==2.3.2 # Last 2.6-compat version. -google-api-python-client==1.6.5 hdrhistogram==0.6.1 PyYAML==3.11; python_version < '2.7' PyYAML==3.13; python_version >= '2.7' From 73979043addeffa43cac37261477dd7064bfcdb4 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 27 Jan 2019 21:48:14 +0000 Subject: [PATCH 02/19] gcloud: small updates --- tests/ansible/gcloud/controller.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/ansible/gcloud/controller.yml b/tests/ansible/gcloud/controller.yml index 494c2164..3c7f9ea0 100644 --- a/tests/ansible/gcloud/controller.yml +++ b/tests/ansible/gcloud/controller.yml @@ -24,10 +24,13 @@ src: ssh_config.j2 - lineinfile: - line: "net.ipv4.ip_forward=1" + line: "{{item}}" path: /etc/sysctl.conf - register: sysctl_conf become: true + with_items: + - net.ipv4.ip_forward=1 + - kernel.perf_event_paranoid=-1 + register: sysctl_conf - shell: /sbin/sysctl -p when: sysctl_conf.changed @@ -46,6 +49,7 @@ - python-virtualenv - strace - libldap2-dev + - linux-perf - libsasl2-dev - build-essential - git @@ -66,8 +70,8 @@ - git: dest: ~/ansible - repo: https://github.com/dw/ansible.git - version: dmw + repo: https://github.com/ansible/ansible.git + #version: dmw - pip: virtualenv: ~/venv From eb93f82d057b2a58c2ecafac93734b2bbd1c3a77 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 27 Jan 2019 21:48:28 +0000 Subject: [PATCH 03/19] tests: ensure file is closed in connection_test. --- tests/ansible/tests/connection_test.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/ansible/tests/connection_test.py b/tests/ansible/tests/connection_test.py index aaf4bf42..401cbe9e 100644 --- a/tests/ansible/tests/connection_test.py +++ b/tests/ansible/tests/connection_test.py @@ -96,7 +96,11 @@ class PutFileTest(ConnectionMixin, unittest2.TestCase): def setUpClass(cls): super(PutFileTest, cls).setUpClass() cls.big_path = tempfile.mktemp(prefix='mitotestbig') - open(cls.big_path, 'w').write('x'*1048576) + fp = open(cls.big_path, 'w') + try: + fp.write('x'*1048576) + finally: + fp.close() @classmethod def tearDownClass(cls): From ec789513dce9b072a512deb6628375e88b84026b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 27 Jan 2019 22:46:06 +0000 Subject: [PATCH 04/19] ssh: tidy up logs and stream names. --- mitogen/core.py | 2 +- mitogen/doas.py | 5 ++--- mitogen/docker.py | 5 ++--- mitogen/jail.py | 5 ++--- mitogen/kubectl.py | 5 ++--- mitogen/lxc.py | 5 ++--- mitogen/lxd.py | 5 ++--- mitogen/parent.py | 20 ++++++++++++++------ mitogen/setns.py | 5 ++++- mitogen/ssh.py | 16 ++++++++-------- mitogen/su.py | 5 ++--- mitogen/sudo.py | 5 ++--- 12 files changed, 43 insertions(+), 40 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index 6979c878..fc473ae7 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -1703,7 +1703,7 @@ class Stream(BasicStream): def __repr__(self): cls = type(self) - return '%s.%s(%r)' % (cls.__module__, cls.__name__, self.name) + return "%s.%s('%s')" % (cls.__module__, cls.__name__, self.name) class Context(object): diff --git a/mitogen/doas.py b/mitogen/doas.py index 0f6a106c..fe814d7b 100644 --- a/mitogen/doas.py +++ b/mitogen/doas.py @@ -66,9 +66,8 @@ class Stream(mitogen.parent.Stream): if incorrect_prompts is not None: self.incorrect_prompts = map(str.lower, incorrect_prompts) - def connect(self): - super(Stream, self).connect() - self.name = u'doas.' + mitogen.core.to_text(self.username) + def _get_name(self): + return u'doas.' + mitogen.core.to_text(self.username) def get_boot_command(self): bits = [self.doas_path, '-u', self.username, '--'] diff --git a/mitogen/docker.py b/mitogen/docker.py index 36b0635b..3962f13c 100644 --- a/mitogen/docker.py +++ b/mitogen/docker.py @@ -62,9 +62,8 @@ class Stream(mitogen.parent.Stream): if username: self.username = username - def connect(self): - super(Stream, self).connect() - self.name = u'docker.' + (self.container or self.image) + def _get_name(self): + return u'docker.' + (self.container or self.image) def get_boot_command(self): args = ['--interactive'] diff --git a/mitogen/jail.py b/mitogen/jail.py index 726b60e0..85a4a6ca 100644 --- a/mitogen/jail.py +++ b/mitogen/jail.py @@ -52,9 +52,8 @@ class Stream(mitogen.parent.Stream): if jexec_path: self.jexec_path = jexec_path - def connect(self): - super(Stream, self).connect() - self.name = u'jail.' + self.container + def _get_name(self): + return u'jail.' + self.container def get_boot_command(self): bits = [self.jexec_path] diff --git a/mitogen/kubectl.py b/mitogen/kubectl.py index e6758ec4..7079bcab 100644 --- a/mitogen/kubectl.py +++ b/mitogen/kubectl.py @@ -55,9 +55,8 @@ class Stream(mitogen.parent.Stream): self.kubectl_path = kubectl_path self.kubectl_args = kubectl_args or [] - def connect(self): - super(Stream, self).connect() - self.name = u'kubectl.%s%s' % (self.pod, self.kubectl_args) + def _get_name(self): + return u'kubectl.%s%s' % (self.pod, self.kubectl_args) def get_boot_command(self): bits = [self.kubectl_path] + self.kubectl_args + ['exec', '-it', self.pod] diff --git a/mitogen/lxc.py b/mitogen/lxc.py index cd85be4f..78c32528 100644 --- a/mitogen/lxc.py +++ b/mitogen/lxc.py @@ -60,9 +60,8 @@ class Stream(mitogen.parent.Stream): if lxc_attach_path: self.lxc_attach_path = lxc_attach_path - def connect(self): - super(Stream, self).connect() - self.name = u'lxc.' + self.container + def _get_name(self): + return u'lxc.' + self.container def get_boot_command(self): bits = [ diff --git a/mitogen/lxd.py b/mitogen/lxd.py index a28e1aaa..eef53c73 100644 --- a/mitogen/lxd.py +++ b/mitogen/lxd.py @@ -61,9 +61,8 @@ class Stream(mitogen.parent.Stream): if lxc_path: self.lxc_path = lxc_path - def connect(self): - super(Stream, self).connect() - self.name = u'lxd.' + self.container + def _get_name(self): + return u'lxd.' + self.container def get_boot_command(self): bits = [ diff --git a/mitogen/parent.py b/mitogen/parent.py index 50862418..76437ebb 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -1001,7 +1001,7 @@ class DiagLogStream(mitogen.core.BasicStream): self.buf = '' def __repr__(self): - return 'mitogen.parent.DiagLogStream(fd=%r, %r)' % ( + return "mitogen.parent.DiagLogStream(fd=%r, '%s')" % ( self.receive_side.fd, self.stream.name, ) @@ -1017,11 +1017,11 @@ class DiagLogStream(mitogen.core.BasicStream): return self.on_disconnect(broker) self.buf += buf.decode('utf-8', 'replace') - while '\n' in self.buf: + while u'\n' in self.buf: lines = self.buf.split('\n') self.buf = lines[-1] for line in lines[:-1]: - LOG.debug('%r: %r', self, line.rstrip()) + LOG.debug('%s: %s', self.stream.name, line.rstrip()) class Stream(mitogen.core.Stream): @@ -1288,10 +1288,18 @@ class Stream(mitogen.core.Stream): if self.eof_error_hint: e.args = ('%s\n\n%s' % (e.args[0], self.eof_error_hint),) + def _get_name(self): + """ + Called by :meth:`connect` after :attr:`pid` is known. Subclasses can + override it to specify a default stream name, or set + :attr:`name_prefix` to generate a default format. + """ + return u'%s.%s' % (self.name_prefix, self.pid) + def connect(self): LOG.debug('%r.connect()', self) self.pid, fd, diag_fd = self.start_child() - self.name = u'%s.%s' % (self.name_prefix, self.pid) + self.name = self._get_name() self.receive_side = mitogen.core.Side(self, fd) self.transmit_side = mitogen.core.Side(self, os.dup(fd)) if diag_fd is not None: @@ -1299,8 +1307,8 @@ class Stream(mitogen.core.Stream): else: self.diag_stream = None - LOG.debug('%r.connect(): stdin=%r, stdout=%r, diag=%r', - self, self.receive_side.fd, self.transmit_side.fd, + LOG.debug('%r.connect(): pid:%r stdin:%r, stdout:%r, diag:%r', + self, self.pid, self.receive_side.fd, self.transmit_side.fd, self.diag_stream and self.diag_stream.receive_side.fd) try: diff --git a/mitogen/setns.py b/mitogen/setns.py index 0c94cd0b..ced44add 100644 --- a/mitogen/setns.py +++ b/mitogen/setns.py @@ -223,11 +223,14 @@ class Stream(mitogen.parent.Stream): def create_child(self, args): return mitogen.parent.create_child(args, preexec_fn=self.preexec_fn) + def _get_name(self): + return u'setns.' + self.container + def connect(self): + self.name = self._get_name() attr, func = self.GET_LEADER_BY_KIND[self.kind] tool_path = getattr(self, attr) self.leader_pid = func(tool_path, self.container) LOG.debug('Leader PID for %s container %r: %d', self.kind, self.container, self.leader_pid) super(Stream, self).connect() - self.name = u'setns.' + self.container diff --git a/mitogen/ssh.py b/mitogen/ssh.py index f8fe20aa..607d7f44 100644 --- a/mitogen/ssh.py +++ b/mitogen/ssh.py @@ -105,7 +105,7 @@ def filter_debug(stream, it): if b('\n') not in buf: break line, _, buf = bytes_partition(buf, b('\n')) - LOG.debug('%r: %s', stream, + LOG.debug('%s: %s', stream.name, mitogen.core.to_text(line.rstrip())) state = 'start_of_line' elif state == 'in_plain': @@ -239,11 +239,11 @@ class Stream(mitogen.parent.Stream): base = super(Stream, self).get_boot_command() return bits + [shlex_quote(s).strip() for s in base] - def connect(self): - super(Stream, self).connect() - self.name = u'ssh.' + mitogen.core.to_text(self.hostname) + def _get_name(self): + s = u'ssh.' + mitogen.core.to_text(self.hostname) if self.port: - self.name += u':%s' % (self.port,) + s += u':%s' % (self.port,) + return s auth_incorrect_msg = 'SSH authentication is incorrect' password_incorrect_msg = 'SSH password is incorrect' @@ -261,7 +261,7 @@ class Stream(mitogen.parent.Stream): def _host_key_prompt(self): if self.check_host_keys == 'accept': - LOG.debug('%r: accepting host key', self) + LOG.debug('%s: accepting host key', self.name) self.diag_stream.transmit_side.write(b('yes\n')) return @@ -273,7 +273,7 @@ class Stream(mitogen.parent.Stream): def _connect_input_loop(self, it): password_sent = False for buf, partial in filter_debug(self, it): - LOG.debug('%r: received %r', self, buf) + LOG.debug('%s: stdout: %s', self.name, buf.rstrip()) if buf.endswith(self.EC0_MARKER): self._ec0_received() return @@ -295,7 +295,7 @@ class Stream(mitogen.parent.Stream): elif partial and PASSWORD_PROMPT in buf.lower(): if self.password is None: raise PasswordError(self.password_required_msg) - LOG.debug('%r: sending password', self) + LOG.debug('%s: sending password', self.name) self.diag_stream.transmit_side.write( (self.password + '\n').encode() ) diff --git a/mitogen/su.py b/mitogen/su.py index b0bac28b..9401aee9 100644 --- a/mitogen/su.py +++ b/mitogen/su.py @@ -80,9 +80,8 @@ class Stream(mitogen.parent.Stream): if incorrect_prompts is not None: self.incorrect_prompts = map(str.lower, incorrect_prompts) - def connect(self): - super(Stream, self).connect() - self.name = u'su.' + mitogen.core.to_text(self.username) + def _get_name(self): + return u'su.' + mitogen.core.to_text(self.username) def get_boot_command(self): argv = mitogen.parent.Argv(super(Stream, self).get_boot_command()) diff --git a/mitogen/sudo.py b/mitogen/sudo.py index b97118c9..221b8391 100644 --- a/mitogen/sudo.py +++ b/mitogen/sudo.py @@ -140,9 +140,8 @@ class Stream(mitogen.parent.Stream): self.selinux_role = option(self.selinux_role, selinux_role, opts.role) self.selinux_type = option(self.selinux_type, selinux_type, opts.type) - def connect(self): - super(Stream, self).connect() - self.name = u'sudo.' + mitogen.core.to_text(self.username) + def _get_name(self): + return u'sudo.' + mitogen.core.to_text(self.username) def get_boot_command(self): # Note: sudo did not introduce long-format option processing until July From 960e505f07ed61041be6930c2a5583e2f0c84b6b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 Jan 2019 00:47:39 +0000 Subject: [PATCH 05/19] issue #429: install i18n-related bits in test images. --- tests/image_prep/_container_setup.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/image_prep/_container_setup.yml b/tests/image_prep/_container_setup.yml index 6fe06079..3d942a48 100644 --- a/tests/image_prep/_container_setup.yml +++ b/tests/image_prep/_container_setup.yml @@ -30,6 +30,7 @@ "9": - libjson-perl - python-virtualenv + - locales CentOS: "5": - perl @@ -67,12 +68,24 @@ with_items: - /var/cache/apt - /var/lib/apt/lists + + - copy: + dest: /etc/locale.gen + content: | + fr_FR.UTF-8 UTF-8 + when: distro == "Debian" + + - shell: locale-gen when: distro == "Debian" # Vanilla Ansible needs simplejson on CentOS 5. + - shell: mkdir -p /usr/lib/python2.4/site-packages/simplejson/ + when: distro == "CentOS" and ver == "5" + - synchronize: dest: /usr/lib/python2.4/site-packages/simplejson/ src: ../../ansible_mitogen/compat/simplejson/ + when: distro == "CentOS" and ver == "5" - user: name: root From 6e9f8e829efd4927433e290cb3ed1628f62d133b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 Jan 2019 00:48:14 +0000 Subject: [PATCH 06/19] issue #429: teach sudo about every know i18n password string. --- misc/pogrep.py | 40 ++++++++++++++++++++++++ mitogen/sudo.py | 77 ++++++++++++++++++++++++++++++++++++++++++++-- tests/sudo_test.py | 41 ++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 misc/pogrep.py diff --git a/misc/pogrep.py b/misc/pogrep.py new file mode 100644 index 00000000..b837bcfd --- /dev/null +++ b/misc/pogrep.py @@ -0,0 +1,40 @@ + +# issue #429: tool for extracting keys out of message catalogs and turning them +# into the big gob of base64 as used in mitogen/sudo.py +# +# Usage: +# - apt-get source libpam0g +# - cd */po/ +# - python ~/pogrep.py "Password: " + +import sys +import shlex +import glob + + +last_word = None + +for path in glob.glob('*.po'): + for line in open(path): + bits = shlex.split(line, comments=True) + if not bits: + continue + + word = bits[0] + if len(bits) < 2 or not word: + continue + + rest = bits[1] + if not rest: + continue + + if last_word == 'msgid' and word == 'msgstr': + if last_rest == sys.argv[1]: + thing = rest.rstrip(': ').decode('utf-8').lower().encode('utf-8').encode('base64').rstrip() + print ' %-60s # %s' % (repr(thing)+',', path) + + last_word = word + last_rest = rest + +#ag -A 1 'msgid "Password: "'|less | grep msgstr | grep -v '""'|cut -d'"' -f2|cut -d'"' -f1| tr -d : + diff --git a/mitogen/sudo.py b/mitogen/sudo.py index 221b8391..05483f7e 100644 --- a/mitogen/sudo.py +++ b/mitogen/sudo.py @@ -26,8 +26,10 @@ # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. +import base64 import logging import optparse +import re import mitogen.core import mitogen.parent @@ -35,6 +37,73 @@ from mitogen.core import b LOG = logging.getLogger(__name__) + +# These are base64-encoded UTF-8 as our existing minifier/module server +# struggles with Unicode Python source in some (forgotten) circumstances. +PASSWORD_PROMPTS = [ + 'cGFzc3dvcmQ=', # english + 'bG96aW5rYQ==', # sr@latin.po + '44OR44K544Ov44O844OJ', # ja.po + '4Kaq4Ka+4Ka44KaT4Kef4Ka+4Kaw4KeN4Kah', # bn.po + '2YPZhNmF2Kkg2KfZhNiz2LE=', # ar.po + 'cGFzYWhpdHph', # eu.po + '0L/QsNGA0L7Qu9GM', # uk.po + 'cGFyb29s', # et.po + 'c2FsYXNhbmE=', # fi.po + '4Kiq4Ki+4Ki44Ki14Kiw4Kih', # pa.po + 'Y29udHJhc2lnbm8=', # ia.po + 'Zm9jYWwgZmFpcmU=', # ga.po + '16HXodee15Q=', # he.po + '4Kqq4Kq+4Kq44Kq14Kqw4KuN4Kqh', # gu.po + '0L/QsNGA0L7Qu9Cw', # bg.po + '4Kyq4K2N4Kyw4Kys4K2H4Ky2IOCsuOCsmeCtjeCsleCth+CspA==', # or.po + '4K6V4K6f4K614K+B4K6a4K+N4K6a4K+K4K6y4K+N', # ta.po + 'cGFzc3dvcnQ=', # de.po + '7JWU7Zi4', # ko.po + '0LvQvtC30LjQvdC60LA=', # sr.po + 'beG6rXQga2jhuql1', # vi.po + 'c2VuaGE=', # pt_BR.po + 'cGFzc3dvcmQ=', # it.po + 'aGVzbG8=', # cs.po + '5a+G56K877ya', # zh_TW.po + 'aGVzbG8=', # sk.po + '4LC44LCC4LCV4LGH4LCk4LCq4LCm4LCu4LGB', # te.po + '0L/QsNGA0L7Qu9GM', # kk.po + 'aGFzxYJv', # pl.po + 'Y29udHJhc2VueWE=', # ca.po + 'Y29udHJhc2XDsWE=', # es.po + '4LSF4LSf4LSv4LS+4LSz4LS14LS+4LSV4LWN4LSV4LWN', # ml.po + 'c2VuaGE=', # pt.po + '5a+G56CB77ya', # zh_CN.po + '4KSX4KWB4KSq4KWN4KSk4KS24KSs4KWN4KSm', # mr.po + 'bMO2c2Vub3Jk', # sv.po + '4YOe4YOQ4YOg4YOd4YOa4YOY', # ka.po + '4KS24KSs4KWN4KSm4KSV4KWC4KSf', # hi.po + 'YWRnYW5nc2tvZGU=', # da.po + '4La74LeE4LeD4LeK4La04Lav4La6', # si.po + 'cGFzc29yZA==', # nb.po + 'd2FjaHR3b29yZA==', # nl.po + '4Kaq4Ka+4Ka44KaT4Kef4Ka+4Kaw4KeN4Kah', # bn_IN.po + 'cGFyb2xh', # tr.po + '4LKX4LOB4LKq4LON4LKk4LKq4LKm', # kn.po + 'c2FuZGk=', # id.po + '0L/QsNGA0L7Qu9GM', # ru.po + 'amVsc3rDsw==', # hu.po + 'bW90IGRlIHBhc3Nl', # fr.po + 'aXBoYXNpd2VkaQ==', # zu.po + '4Z6W4Z624Z6A4Z+S4Z6Z4Z6f4Z6Y4Z+S4Z6E4Z624Z6P4Z+LwqDhn5Y=', # km.po + '4KaX4KeB4Kaq4KeN4Kak4Ka24Kas4KeN4Kam', # as.po +] + + +PASSWORD_PROMPT_RE = re.compile( + u'|'.join( + base64.b64decode(s).decode('utf-8') + for s in PASSWORD_PROMPTS + ) +) + + PASSWORD_PROMPT = b('password') SUDO_OPTIONS = [ #(False, 'bool', '--askpass', '-A') @@ -170,11 +239,15 @@ class Stream(mitogen.parent.Stream): password_sent = False for buf in it: - LOG.debug('%r: received %r', self, buf) + LOG.debug('%s: received %r', self.name, buf) if buf.endswith(self.EC0_MARKER): self._ec0_received() return - elif PASSWORD_PROMPT in buf.lower(): + + match = PASSWORD_PROMPT_RE.search(buf.decode('utf-8').lower()) + if match is not None: + LOG.debug('%s: matched password prompt %r', + self.name, match.group(0)) if self.password is None: raise PasswordError(self.password_required_msg) if password_sent: diff --git a/tests/sudo_test.py b/tests/sudo_test.py index 87a13cf9..5bf9f4de 100644 --- a/tests/sudo_test.py +++ b/tests/sudo_test.py @@ -56,5 +56,46 @@ class ConstructorTest(testlib.RouterMixin, testlib.TestCase): ]) +class NonEnglishPromptTest(testlib.DockerMixin, testlib.TestCase): + # Only mitogen/debian-test has a properly configured sudo. + mitogen_test_distro = 'debian' + + def test_password_required(self): + ssh = self.docker_ssh( + username='mitogen__has_sudo', + password='has_sudo_password', + ) + ssh.call(os.putenv, 'LANGUAGE', 'fr') + ssh.call(os.putenv, 'LC_ALL', 'fr_FR.UTF-8') + e = self.assertRaises(mitogen.core.StreamError, + lambda: self.router.sudo(via=ssh) + ) + self.assertTrue(mitogen.sudo.Stream.password_required_msg in str(e)) + + def test_password_incorrect(self): + ssh = self.docker_ssh( + username='mitogen__has_sudo', + password='has_sudo_password', + ) + ssh.call(os.putenv, 'LANGUAGE', 'fr') + ssh.call(os.putenv, 'LC_ALL', 'fr_FR.UTF-8') + e = self.assertRaises(mitogen.core.StreamError, + lambda: self.router.sudo(via=ssh, password='x') + ) + self.assertTrue(mitogen.sudo.Stream.password_incorrect_msg in str(e)) + + def test_password_okay(self): + ssh = self.docker_ssh( + username='mitogen__has_sudo', + password='has_sudo_password', + ) + ssh.call(os.putenv, 'LANGUAGE', 'fr') + ssh.call(os.putenv, 'LC_ALL', 'fr_FR.UTF-8') + e = self.assertRaises(mitogen.core.StreamError, + lambda: self.router.sudo(via=ssh, password='rootpassword') + ) + self.assertTrue(mitogen.sudo.Stream.password_incorrect_msg in str(e)) + + if __name__ == '__main__': unittest2.main() From cd1e5e01385ef1259b69f38a564f5e170294c597 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 Jan 2019 00:48:14 +0000 Subject: [PATCH 07/19] issue #429: update Changelog. --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 9be70eb7..2bd30e6a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -248,6 +248,9 @@ Fixes number of 2->3 bugs were fixed, mostly in the form of Unicode/bytes mismatches. +* `#429 `_: the ``sudo`` method can + now recognize internationalized password prompts. + * `#362 `_, `#435 `_: the previous fix for slow Python 2.x subprocess creation on Red Hat caused newly spawned children to @@ -464,6 +467,7 @@ bug reports, testing, features and fixes in this release contributed by `Strahinja Kustudic `_, `Tom Parker-Shemilt `_, `Younès HAFRI `_, +`@killua-eu `_, `@myssa91 `_, `@s3c70r `_, `@syntonym `_, From 77e7cadd228cacbe5bad72d53980787b2ed15cf3 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 Jan 2019 00:48:14 +0000 Subject: [PATCH 08/19] issue #429: update Changelog. --- docs/changelog.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 2bd30e6a..7e221b0a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -469,6 +469,7 @@ bug reports, testing, features and fixes in this release contributed by `Younès HAFRI `_, `@killua-eu `_, `@myssa91 `_, +`@ohmer1 `_, `@s3c70r `_, `@syntonym `_, `@trim777 `_, From 2fdbd0cfcd5357af0745aae8459d0c05f78a6ae4 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 Jan 2019 01:00:34 +0000 Subject: [PATCH 09/19] ssh: fix test to match updated log format. --- tests/ssh_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/ssh_test.py b/tests/ssh_test.py index abb25a58..496710b8 100644 --- a/tests/ssh_test.py +++ b/tests/ssh_test.py @@ -57,7 +57,8 @@ class SshTest(testlib.DockerMixin, testlib.TestCase): finally: s = capture.stop() - self.assertTrue("'): debug1: Reading configuration data" in s) + expect = "%s: debug1: Reading configuration data" % (context.name,) + self.assertTrue(expect in s) def test_stream_name(self): context = self.docker_ssh( From a40946297fd6e06c6b76153f8d467eeb885b9f9a Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 28 Jan 2019 19:35:34 +0000 Subject: [PATCH 10/19] issue #497: do our best to cope with crap upstream code --- tests/ansible/integration/async/runner_one_job.yml | 4 ++++ .../integration/async/runner_two_simultaneous_jobs.yml | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/tests/ansible/integration/async/runner_one_job.yml b/tests/ansible/integration/async/runner_one_job.yml index 19fba7de..ca798a7f 100644 --- a/tests/ansible/integration/async/runner_one_job.yml +++ b/tests/ansible/integration/async/runner_one_job.yml @@ -43,10 +43,14 @@ - result1.finished == 1 - result1.rc == 0 - result1.start|length == 26 + + - assert: + that: - result1.stderr == "" - result1.stderr_lines == [] - result1.stdout == "alldone" - result1.stdout_lines == ["alldone"] + when: ansible_version.full > '2.8' # ansible#51393 - assert: that: diff --git a/tests/ansible/integration/async/runner_two_simultaneous_jobs.yml b/tests/ansible/integration/async/runner_two_simultaneous_jobs.yml index 9474263b..fdde0463 100644 --- a/tests/ansible/integration/async/runner_two_simultaneous_jobs.yml +++ b/tests/ansible/integration/async/runner_two_simultaneous_jobs.yml @@ -56,4 +56,8 @@ that: - result1.rc == 0 - result2.rc == 0 + + - assert: + that: - result2.stdout == 'im_alive' + when: ansible_version.full > '2.8' # ansible#51393 From 7531af3ee0048c3d88de23edc97ffeebe771bcb3 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 29 Jan 2019 04:20:14 +0000 Subject: [PATCH 11/19] issue #499: fix another mind-numbingly stupid vanilla inconsistency --- tests/image_prep/_container_setup.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/image_prep/_container_setup.yml b/tests/image_prep/_container_setup.yml index 3d942a48..e7a47915 100644 --- a/tests/image_prep/_container_setup.yml +++ b/tests/image_prep/_container_setup.yml @@ -130,6 +130,11 @@ Defaults>mitogen__require_tty requiretty Defaults>mitogen__require_tty_pw_required requiretty,targetpw + # Prevent permission denied errors. + - file: + path: /etc/sudoers.d/README + state: absent + - lineinfile: path: /etc/sudoers line: "%wheel ALL=(ALL) ALL" From 60fe3fd6f5ca7c50725952c96f87aa8a11194da4 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 29 Jan 2019 03:03:16 +0000 Subject: [PATCH 12/19] issue #429: enable en_US locale to unbreak debops test. --- tests/image_prep/_container_setup.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/image_prep/_container_setup.yml b/tests/image_prep/_container_setup.yml index e7a47915..dc0bbf53 100644 --- a/tests/image_prep/_container_setup.yml +++ b/tests/image_prep/_container_setup.yml @@ -72,6 +72,7 @@ - copy: dest: /etc/locale.gen content: | + en_US.UTF-8 UTF-8 fr_FR.UTF-8 UTF-8 when: distro == "Debian" From e2478dcb9f6dbb1a223609c9cd42be86b3f2544b Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 29 Jan 2019 04:41:02 +0000 Subject: [PATCH 13/19] issue #498: wrap Router dict mutations in a lock --- mitogen/core.py | 79 +++++++++++++++++++++++++++++++++-------------- mitogen/parent.py | 41 ++++++++++++++++++------ 2 files changed, 87 insertions(+), 33 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index fc473ae7..9a5e85db 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -2423,9 +2423,10 @@ class Router(object): listen(broker, 'exit', self._on_broker_exit) self._setup_logging() - #: context ID -> Stream + self._write_lock = threading.Lock() + #: context ID -> Stream; must hold _write_lock to edit or iterate self._stream_by_id = {} - #: List of contexts to notify of shutdown. + #: List of contexts to notify of shutdown; must hold _write_lock self._context_by_id = {} self._last_handle = itertools.count(1000) #: handle -> (persistent?, func(msg)) @@ -2456,21 +2457,31 @@ class Router(object): :class:`mitogen.parent.RouteMonitor` in an upgraded context. """ LOG.error('%r._on_del_route() %r', self, msg) - if not msg.is_dead: - target_id_s, _, name = bytes_partition(msg.data, b(':')) - target_id = int(target_id_s, 10) - if target_id not in self._context_by_id: - LOG.debug('DEL_ROUTE for unknown ID %r: %r', target_id, msg) - return + if msg.is_dead: + return - fire(self._context_by_id[target_id], 'disconnect') + target_id_s, _, name = bytes_partition(msg.data, b(':')) + context = self._context_by_id.get(int(target_id_s, 10)) + if context: + fire(context, 'disconnect') + else: + LOG.debug('DEL_ROUTE for unknown ID %r: %r', target_id, msg) def _on_stream_disconnect(self, stream): - for context in self._context_by_id.values(): - stream_ = self._stream_by_id.get(context.context_id) - if stream_ is stream: - del self._stream_by_id[context.context_id] - context.on_disconnect() + notify = [] + self._write_lock.acquire() + try: + for context in list(self._context_by_id.values()): + stream_ = self._stream_by_id.get(context.context_id) + if stream_ is stream: + del self._stream_by_id[context.context_id] + notify.append(context) + finally: + self._write_lock.release() + + # Happens outside lock as e.g. RouteMonitor wants the same lock. + for context in notify: + context.on_disconnect() broker_exit_msg = 'Broker has exitted' @@ -2492,14 +2503,27 @@ class Router(object): def context_by_id(self, context_id, via_id=None, create=True, name=None): """ Messy factory/lookup function to find a context by its ID, or construct - it. In future this will be replaced by a much more sensible interface. + it. This will eventually be replaced by a more sensible interface. """ context = self._context_by_id.get(context_id) - if create and not context: - context = self.context_class(self, context_id, name=name) - if via_id is not None: - context.via = self.context_by_id(via_id) - self._context_by_id[context_id] = context + if context: + return context + + if create and via_id is not None: + via = self.context_by_id(via_id) + else: + via = None + + self._write_lock.acquire() + try: + context = self._context_by_id.get(context_id) + if create and not context: + context = self.context_class(self, context_id, name=name) + context.via = via + self._context_by_id[context_id] = context + finally: + self._write_lock.release() + return context def register(self, context, stream): @@ -2509,8 +2533,13 @@ class Router(object): public while the design has not yet settled. """ _v and LOG.debug('register(%r, %r)', context, stream) - self._stream_by_id[context.context_id] = stream - self._context_by_id[context.context_id] = context + self._write_lock.acquire() + try: + self._stream_by_id[context.context_id] = stream + self._context_by_id[context.context_id] = context + finally: + self._write_lock.release() + self.broker.start_receive(stream) listen(stream, 'disconnect', lambda: self._on_stream_disconnect(stream)) @@ -2520,8 +2549,10 @@ class Router(object): `dst_id`. If a specific route for `dst_id` is not known, a reference to the parent context's stream is returned. """ - parent = self._stream_by_id.get(mitogen.parent_id) - return self._stream_by_id.get(dst_id, parent) + return ( + self._stream_by_id.get(dst_id) or + self._stream_by_id.get(mitogen.parent_id) + ) def del_handler(self, handle): """ diff --git a/mitogen/parent.py b/mitogen/parent.py index 76437ebb..fcefd460 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -1688,6 +1688,10 @@ class RouteMonitor(object): child is beging upgraded in preparation to become a parent of children of its own. + By virtue of only being active while responding to messages from a handler, + RouteMonitor lives entirely on the broker thread, so its data requires no + locking. + :param Router router: Router to install handlers on. :param Context parent: @@ -1697,6 +1701,9 @@ class RouteMonitor(object): def __init__(self, router, parent=None): self.router = router self.parent = parent + #: Mapping of Stream instance to integer context IDs reachable via the + #: stream; used to cleanup routes during disconnection. + self._routes_by_stream = {} self.router.add_handler( fn=self._on_add_route, handle=mitogen.core.ADD_ROUTE, @@ -1711,9 +1718,6 @@ class RouteMonitor(object): policy=is_immediate_child, overwrite=True, ) - #: Mapping of Stream instance to integer context IDs reachable via the - #: stream; used to cleanup routes during disconnection. - self._routes_by_stream = {} def __repr__(self): return 'RouteMonitor()' @@ -1775,7 +1779,7 @@ class RouteMonitor(object): :param int target_id: ID of the connecting or disconnecting context. """ - for stream in itervalues(self.router._stream_by_id): + for stream in self.router.get_streams(): if target_id in stream.egress_ids: self._send_one(stream, mitogen.core.DEL_ROUTE, target_id, None) @@ -1918,6 +1922,16 @@ class Router(mitogen.core.Router): stream.detached = True msg.reply(None) + def get_streams(self): + """ + Return a snapshot of all streams in existence at time of call. + """ + self._write_lock.acquire() + try: + return itervalues(self._stream_by_id) + finally: + self._write_lock.release() + def add_route(self, target_id, stream): """ Arrange for messages whose `dst_id` is `target_id` to be forwarded on @@ -1929,11 +1943,12 @@ class Router(mitogen.core.Router): LOG.debug('%r.add_route(%r, %r)', self, target_id, stream) assert isinstance(target_id, int) assert isinstance(stream, Stream) + + self._write_lock.acquire() try: self._stream_by_id[target_id] = stream - except KeyError: - LOG.error('%r: cant add route to %r via %r: no such stream', - self, target_id, stream) + finally: + self._write_lock.release() def del_route(self, target_id): LOG.debug('%r.del_route(%r)', self, target_id) @@ -1942,7 +1957,11 @@ class Router(mitogen.core.Router): # 'disconnect' event on the appropriate Context instance. In that case, # we won't a matching _stream_by_id entry for the disappearing route, # so don't raise an error for a missing key here. - self._stream_by_id.pop(target_id, None) + self._write_lock.acquire() + try: + self._stream_by_id.pop(target_id, None) + finally: + self._write_lock.release() def get_module_blacklist(self): if mitogen.context_id == 0: @@ -2001,7 +2020,11 @@ class Router(mitogen.core.Router): name = u'%s.%s' % (via_context.name, resp['name']) context = self.context_class(self, resp['id'], name=name) context.via = via_context - self._context_by_id[context.context_id] = context + self._write_lock.acquire() + try: + self._context_by_id[context.context_id] = context + finally: + self._write_lock.release() return context def doas(self, **kwargs): From 9aa845669ce2bfe700e22ce6babf7d64e03c82a5 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 29 Jan 2019 04:41:02 +0000 Subject: [PATCH 14/19] issue #413: don't double-propagate DEL_ROUTE to parent. propagate_up() sends ADD_ROUTE and DEL_ROUTE propagate_down() sends only DEL_ROUTE, but didn't bother checking if up() had sent it already. Fixes: ERROR! [pid 41060] 17:55:30.739159 E mitogen.ctx.ssh.localhost: mitogen: RouteMonitor(): received DEL_ROUTE for 6081 from mitogen.fork.Stream(u'fork.41142'), expected mitogen.core.Stream('parent') --- mitogen/parent.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index fcefd460..e23ffa3c 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -1780,7 +1780,10 @@ class RouteMonitor(object): ID of the connecting or disconnecting context. """ for stream in self.router.get_streams(): - if target_id in stream.egress_ids: + if target_id in stream.egress_ids and ( + (self.parent is None) or + (self.parent.context_id != stream.remote_id) + ): self._send_one(stream, mitogen.core.DEL_ROUTE, target_id, None) def notice_stream(self, stream): From 407294cd79591efb8f58dbeb660ffff46245f218 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 29 Jan 2019 04:41:02 +0000 Subject: [PATCH 15/19] issue #498: prevent crash on double 'disconnect' signal. Fixes: ERROR! [pid 1096] 23:31:48.363215 E mitogen: _broker_main() crashed Traceback (most recent call last): File "/home/dmw/src/mitogen/mitogen/core.py", line 2917, in _broker_main self._loop_once() File "/home/dmw/src/mitogen/mitogen/core.py", line 2875, in _loop_once self._call(side.stream, func) File "/home/dmw/src/mitogen/mitogen/core.py", line 2860, in _call stream.on_disconnect(self) File "/home/dmw/src/mitogen/mitogen/parent.py", line 1161, in on_disconnect super(Stream, self).on_disconnect(broker) File "/home/dmw/src/mitogen/mitogen/core.py", line 1534, in on_disconnect fire(self, 'disconnect') File "/home/dmw/src/mitogen/mitogen/core.py", line 390, in fire func(*args, **kwargs) File "/home/dmw/src/mitogen/mitogen/parent.py", line 1794, in func=lambda: self._on_stream_disconnect(stream), File "/home/dmw/src/mitogen/mitogen/parent.py", line 1810, in _on_stream_disconnect routes = self._routes_by_stream.pop(stream) KeyError: mitogen.ssh.Stream('ssh.localhost:2236') --- mitogen/parent.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/mitogen/parent.py b/mitogen/parent.py index e23ffa3c..241f6243 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -1812,9 +1812,15 @@ class RouteMonitor(object): def _on_stream_disconnect(self, stream): """ - Respond to disconnection of a local stream by + Respond to disconnection of a local stream by propagating DEL_ROUTE for + any contexts we know were attached to it. """ - routes = self._routes_by_stream.pop(stream) + # During a stream crash it is possible for disconnect signal to fire + # twice, in which case ignore the second instance. + routes = self._routes_by_stream.pop(stream, None) + if routes is None: + return + LOG.debug('%r: %r is gone; propagating DEL_ROUTE for %r', self, stream, routes) for target_id in routes: From 0c0d34241b64799b30bebf4b28c8d4547a08bbf0 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 29 Jan 2019 05:02:34 +0000 Subject: [PATCH 16/19] core: Latch._wake improvements os.write() can fail with EINTR due to signals, so wrap it in io_op(). Closes #483. Masking EBADF looks like it is/was almost certainly papering over a bug, remove it and suffer the bug reports. Closes #495. --- mitogen/core.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/mitogen/core.py b/mitogen/core.py index 9a5e85db..4bc754ba 100644 --- a/mitogen/core.py +++ b/mitogen/core.py @@ -2220,12 +2220,8 @@ class Latch(object): self._lock.release() def _wake(self, wsock, cookie): - try: - os.write(wsock.fileno(), cookie) - except OSError: - e = sys.exc_info()[1] - if e.args[0] != errno.EBADF: - raise + written, disconnected = io_op(os.write, wsock.fileno(), cookie) + assert written == len(cookie) and not disconnected def __repr__(self): return 'Latch(%#x, size=%d, t=%r)' % ( From ae8173a29f94d0478cc7557c84639c1a986b8bf0 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 29 Jan 2019 05:33:42 +0000 Subject: [PATCH 17/19] misc: rename to scripts. tab completion!! --- {misc => scripts}/pogrep.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {misc => scripts}/pogrep.py (100%) diff --git a/misc/pogrep.py b/scripts/pogrep.py similarity index 100% rename from misc/pogrep.py rename to scripts/pogrep.py From fca5513610060a56af18ad51ab2bd40f58d47fc8 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 29 Jan 2019 05:34:44 +0000 Subject: [PATCH 18/19] issue #429: fix sudo regression. --- mitogen/sudo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mitogen/sudo.py b/mitogen/sudo.py index 05483f7e..708953cd 100644 --- a/mitogen/sudo.py +++ b/mitogen/sudo.py @@ -253,7 +253,7 @@ class Stream(mitogen.parent.Stream): if password_sent: raise PasswordError(self.password_incorrect_msg) self.diag_stream.transmit_side.write( - mitogen.core.to_text(self.password + '\n').encode('utf-8') + (mitogen.core.to_text(self.password) + '\n').encode('utf-8') ) password_sent = True From fe745779bac786d97a6a71005a97a633eb76d72a Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 29 Jan 2019 05:42:28 +0000 Subject: [PATCH 19/19] Use develop mode in tox --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 2f7bb38f..80f835ee 100644 --- a/tox.ini +++ b/tox.ini @@ -7,6 +7,7 @@ envlist = py37, [testenv] +usedevelop = True deps = -r{toxinidir}/dev_requirements.txt -r{toxinidir}/tests/ansible/requirements.txt