mitogen-hq/mitogen

Mitogen broken after ansible security patch for "unsafe variables"

upekkha opened this issue ยท 16 comments

Ansible has released a security patch for CVE-2023-5764 in all maintained versions, namely 2.16.1, 2.15.7 and 2.14.12. This patch breaks mitogen, as seen in the example of a simple package installation task:

ansible somehost -m package -a 'pkg=vim' -D -C -vvv
[...]
The full traceback is:
Traceback (most recent call last):
  File "/path/to/ansible/lib/ansible/executor/task_executor.py", line 165, in run
    res = self._execute()
          ^^^^^^^^^^^^^^^
  File "/path/to/ansible/lib/ansible/executor/task_executor.py", line 641, in _execute
    result = self._handler.run(task_vars=vars_copy)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/mitogen/ansible_mitogen/mixins.py", line 146, in run
    return super(ActionModuleMixin, self).run(tmp, task_vars)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/ansible/lib/ansible/plugins/action/package.py", line 85, in run
    result.update(self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars, wrap_async=self._task.async_val))
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/mitogen/ansible_mitogen/mixins.py", line 386, in _execute_module
    result = ansible_mitogen.planner.invoke(
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/mitogen/ansible_mitogen/planner.py", line 609, in invoke
    response = invocation.connection.get_chain().call(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/mitogen/ansible_mitogen/connection.py", line 466, in call
    return self._rethrow(recv)
           ^^^^^^^^^^^^^^^^^^^
  File "/path/to/mitogen/ansible_mitogen/connection.py", line 452, in _rethrow
    return recv.get().unpickle()
           ^^^^^^^^^^^^^^^^^^^^^
  File "/path/to/mitogen/mitogen/core.py", line 977, in unpickle
    raise obj
mitogen.core.CallError: mitogen.core.StreamError: cannot unpickle 'ansible.utils.unsafe_proxy'/'AnsibleUnsafeText'
  File "<stdin>", line 3698, in _dispatch_one
  File "<stdin>", line 3681, in _parse_request
  File "<stdin>", line 966, in unpickle
  File "<stdin>", line 757, in find_class
  File "<stdin>", line 867, in _find_global

somehost | FAILED! => {
    "msg": "Unexpected failure during module execution: mitogen.core.StreamError: cannot unpickle 'ansible.utils.unsafe_proxy'/'AnsibleUnsafeText'\n  File \"<stdin>\", line 3698, in _dispatch_one\n  File \"<stdin>\", line 3681, in _parse_request\n  File \"<stdin>\", line 966, in unpickle\n  File \"<stdin>\", line 757, in find_class\n  File \"<stdin>\", line 867, in _find_global\n",
    "stdout": ""
}

For example in 2.16.1, the culprit is ansible/ansible@270b39f. The problem persists on the current stable-2.16 branch and is not fixed by unreleased commits like "Additional unsafe fixes" ansible/ansible#82376.

Considering that Ansible 2.13 is end-of-life since November and all maintained versions (2.14-2.16) are currently broken, we are left in the undesirable situation, where no supported Ansible version remains compatible with mitogen.

Here is workaround how to make mitogen work with ansible v2.16.2:

Open file: mitogen/mitogen/core.py
Go to line: 845 and replace def _find_global with this:

    def _unpickle_ansible_unsafe_text(self, serialized_obj):
        return serialized_obj

    def _find_global(self, module, func):
        """
        Return the class implementing `module_name.class_name` or raise
        `StreamError` if the module is not whitelisted.
        """
        print(module, __name__)
        if module == __name__:
            if func == '_unpickle_call_error' or func == 'CallError':
                return _unpickle_call_error
            elif func == '_unpickle_sender':
                return self._unpickle_sender
            elif func == '_unpickle_context':
                return self._unpickle_context
            elif func == 'Blob':
                return Blob
            elif func == 'Secret':
                return Secret
            elif func == 'Kwargs':
                return Kwargs
        elif module == 'ansible.utils.unsafe_proxy' and func == 'AnsibleUnsafeText':
            return self._unpickle_ansible_unsafe_text
        elif module == '_codecs' and func == 'encode':
            return self._unpickle_bytes
        elif module == '__builtin__' and func == 'bytes':
            return BytesType
        raise StreamError('cannot unpickle %r/%r', module, func)

Open file: mitogen/ansible_mitogen/loaders.py
Go to line: 52 and replace it with:

ANSIBLE_VERSION_MAX = (2, 16)

Hey @AKrumov

Thanks for looking into this. I can confirm that your patch is working for me as well.

Many thanks @AKrumov! It's working for me too!

I'm sad to see this project is abandoned.

As part of my fixes for Mitogen 2.16 and Python 3.12, in #1033 (comment) I solved this in my patch, slightly differently:

In mitogen/core.py:

+AnsibleUnsafeText = None
+
+def lazy_AnsibleUnsafeText():
+    global AnsibleUnsafeText
+    if AnsibleUnsafeText is not None:
+        return AnsibleUnsafeText
+    mod = __import__("ansible.utils.unsafe_proxy", fromlist=("AnsibleUnsafeText",))
+    AnsibleUnsafeText = getattr(mod, "AnsibleUnsafeText")
+    assert type(AnsibleUnsafeText) is type, f"AnsibleUnsafeText {AnsibleUnsafeText} is not a type"
+    assert callable(AnsibleUnsafeText), f"AnsibleUnsafeText {AnsibleUnsafeText} is not callable"
+    return AnsibleUnsafeText
+
+
@@ -860,6 +893,8 @@ class Message(object):
                 return Secret
             elif func == 'Kwargs':
                 return Kwargs
+        elif module == 'ansible.utils.unsafe_proxy' and func == 'AnsibleUnsafeText':
+            return lazy_AnsibleUnsafeText()
         elif module == '_codecs' and func == 'encode':
             return self._unpickle_bytes
         elif module == '__builtin__' and func == 'bytes':

Thank you for the workarounds. Could we get some comments on the relative merits of the two versions? Are we simply subverting the CVE fix or actually incorporating it into mitogen?

I do not know what the Unsafe proxy actually protects against, but my random guess is that it somehow intercepts calls to logging and print, and does not allow them to be printed or logged to output of ansible (i.e. when running with -v), but conversion to str should work.

It is not a serious security risk, unless you make your log files publicly visible, or something (i.e. when running from AWX / Tower, but people that should not have access to original secrets).

@AKrumov, would mind if I use your patch for serverscom.mitogen? I already do autopatch for ANSIBLE_VERSION, seems like it's time to start adding more patches.

I'm not helping here but I'm using the patch and it fixes the issue but my plays now have lots of lines of:-

mitogen.core mitogen.core
mitogen.core mitogen.core
mitogen.core mitogen.core
mitogen.core mitogen.core
mitogen.core mitogen.core
mitogen.core mitogen.core
mitogen.core mitogen.core
mitogen.core mitogen.core
_codecs mitogen.core

Drop the print() statement in the patch. It presumably was just for debugging purposes.

@AKrumov, would mind if I use your patch for serverscom.mitogen? I already do autopatch for ANSIBLE_VERSION, seems like it's time to start adding more patches.

Absolutely! Go ahead and use the patch for serverscom.mitogen. Sorry for getting back to you late. If you need anything else or have more questions, just let me know!

I believe the changes in #1017 will fix this, based on branch https://github.com/moreati/mitogen/tree/2.14. Please try it and let me know how it goes. If all is well this will go into master, and then release 0.3.6. Sorry for the long wait, and thanks for helping with interim workarounds.

It works, even with ansible 2.16.5 and this patch:

--- a/ansible_mitogen/loaders.py
+++ b/ansible_mitogen/loaders.py
@@ -49,7 +49,7 @@ __all__ = [
 
 
 ANSIBLE_VERSION_MIN = (2, 10)
-ANSIBLE_VERSION_MAX = (2, 14)
+ANSIBLE_VERSION_MAX = (2, 16)
 
 NEW_VERSION_MSG = (
     "Your Ansible version (%s) is too recent. The most recent version\n"

Could you please add support for 2.16 too?

@nerijus just put 99 and not bother:

--- a/ansible_mitogen/loaders.py
+++ b/ansible_mitogen/loaders.py
@@ -49,7 +49,7 @@ __all__ = [
 
 
 ANSIBLE_VERSION_MIN = (2, 10)
-ANSIBLE_VERSION_MAX = (2, 13)
+ANSIBLE_VERSION_MAX = (2, 99)

Mitogen has always worked with the most recent ansible :) Not even sure why this is checked.

No, not always :)

Could you please add support for 2.16 too?

Not in the next release (0.3.6). I'm aiming for smaller, more frequent releases. 2.15 and 2.16 will be in subsequent release(s).

Mitogen 0.3.6 is now out.