ansible-core 2.16.1/2.16.2 break action plugin parameters due to changed casting

[SirUli]

Summary

When upgrading to ansible-core 2.16.1 or 2.16.2, the behaviour of module parameters has changed thanks to various changes resulting from CVE-2023-5764. For us the biggest change in our custom modules was that a AnsibleUnsafeText cast to "str" leads to an AnsibleUnsafeText object which to me is a breaking change and could have been annnounced more clearly.

One of the visible changes is that when you are using Pathlib.Path in a custom module, the library dumps with an TypeError: can't intern AnsibleUnsafeText as the Pathlib cannot cast the string properly to str.

Today our workaround is:

from ansible.utils.unsafe_proxy import _is_unsafe
pathvar = module.params.get("path", None)
if is_unsafe(pathvar):
  pathvar = pathvar._strip_unsafe()
Pathlib.Path(pathvar)

which i believe shouldn't be the workaround. The alternative is serializing to json and deserializing again but agin - i believe this shouldn't work this way.

The issue came up after the changes from the CVE remediation, e.g. this one where str was modified to return the AnsibleUnsafeText instead of str.

I'm not too deep in ansible-core development but i think there are different ways of solving that:

• Announce it as a breaking change and offer public methods to get rid of the "unsafe" state, e.g. by removing the private-state of the _strip_unsafe() method
• Change the behaviour of str (and potentially others) back to the original one (casting into a string) (i think you explicitly wanted to prevent that so potentially not an option)

Building a sample repository would take quite an effort - hope you can see the challenge already otherwise let me know what to provide.

Thanks in advance!

Best Regards,
Uli

Issue Type

Bug Report

Component Name

core

Ansible Version

$ ansible --version
ansible [core 2.16.1]
  config file = ~/.ansible.cfg
  configured module search path = ['~/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = ~/.pyenv/versions/3.11.4/lib/python3.11/site-packages/ansible
  ansible collection location = ~/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/wolfu3/.pyenv/versions/3.11.4/bin/ansible
  python version = 3.11.4 (main, Jul  6 2023, 10:09:16) [GCC 11.3.0] (~/.pyenv/versions/3.11.4/bin/python3.11)
  jinja version = 3.1.2
  libyaml = True

Configuration

Doesn't matter - irrespective of the environment.

OS / Environment

RHEL8 - but doesn't matter.

Steps to Reproduce

In a custom module call to retrieve the path variable

pathvar = module.params.get("path", None)
Pathlib.Path(pathvar)

Expected Results

Pathlib's Path is created properly

Actual Results

TypeError: can't intern AnsibleUnsafeText

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibot]

Files identified in the description:

None

If these files are incorrect, please update the component name section of the description or use the component bot command.

[SirUli]

@ansibot component =lib/ansible/utils/unsafe_proxy.py

[ansibot]

Files identified in the description:

• lib/ansible/utils/unsafe_proxy.py

If these files are incorrect, please update the component name section of the description or use the component bot command.

[sivel]

I do understand what's happening, but I don't understand how you have gotten to a point where you are actually experiencing this issue. Are you directly consuming other Python APIs within ansible-core?

Generally speaking, by the time that a module receives the arguments, you wouldn't be dealing with unsafe, as there is no concept of unsafe within a module.

Could you expand the example a little more to give more context?

You are correct that the decision to make str() effectively a no-op is purposeful. And by that same design, there are no public methods to strip unsafe on purpose.

[SirUli]

Hi @sivel

Sure. In this case there is a collection which consists of multiple hundred custom ansible modules which are specific to custom services we have built. We execute these modules (as usual) through ansible-playbook and are NOT using, e.g. direct python APIs within core. We first detected this yesterday in an action module and were hoping for 2.16.2 to fix that but that didn't help.

I totally agree with you on the "there is no concept of unsafe within a module" as we never dealt with that so far.

Nevertheless, even in lower Ansible Versions (e.g. 2.16.0) the type of a passed variable to an Action module was of type AnsibleUnsafeText but since the AnsibleUnsafeText was inherited from text_type (which effectively was str) we didn't experience trouble. Indeed my first thought was "we should just be getting the type we describe in the arguments, e.g. str" but indeed we get that AnsibleUnsafeText passed over. Since the security fix implementation the only occurrence of this issue is in Pathlib based modules where Pathlib tries a call to sys.intern and that fails since it cannot cast the variable properly.

Here is the demo repo (collection): https://github.com/SirUli/ansibleunsafetext

To show the trouble: ansible-playbook siruli.ansibleunsafetext.producer -vvv

While building my demo repository i was able to further isolate the problem: We use heavily imports of variable files and sometimes lookups in these files to serve variables from the environment. When the variable in the imported file is non-jinja, it works. See here when you uncomment line 6 instead of line 4.

Does that help?

[sivel]

Does that help?

Yes, it does. The clarity on using an action plugins, and the example make it more clear.

I don't have any immediate info to give you about when we may have an answer for this use case. Our next release is scheduled for 29 January 2024, so should a release be a required, you may not see movement until closer to that date.

As an short term solution, using os instead of pathlib may alleviate the issues, although I cannot say from the example what your overall goal is.

Just as a heads up, _strip_unsafe will not exist in 2.17, which is another reason it's not public. There are other mechanisms that result in unsafe being stripped, that rely on underlying C functions that directly touch the string buffer. The overridden methods don't work for everything, so you may find other solutions that currently strip unsafe. Modules like re and io are specific modules that bypass the typical dunder methods.

As for another note, anything that was originally unsafe, if it is somehow converted to safe, should probably be converted back to unsafe before being returned to the user.

[sivel]

This looks a little funny, but seems to work, and although seeming redundant, keeps within the same realm of dealing with paths:

pathlib.Path(os.path.abspath(path))

I'll want to talk with some other devs about this, and what recommendations we might be able to officially make going forward. That will unfortunately have to wait until January. But I've given some work arounds that should suffice for now.

[bcoca]

in most cases, for arguments they should be using teh unfrackpath function

[gzm0]

FWIW, this is a minimum reproducible example (without pathlib):

import sys
from ansible.utils.unsafe_proxy import AnsibleUnsafeText 
x = AnsibleUnsafeText("asdf")
sys.intern(str(x)) # fails on 2.16.2, passes on 2.16.0

Python 3.12.1

[agowa]

Python isn't really designed to prevent you from calling such methods. There are countless ways to workaround this patch and keep your code working. Tbh, I also don't understand why it's such a big deal that you could intentionally loose the Unsafe. It would be different if it was unintentional, but having a public _strip_unsafe and people using it? If people want to shoot themselves in the foot, let them...

Also all of these exist, too:

• import types; type(types.MethodType(str.__str__, x)())
• x.__class__.__str__ = str.__str__; type(str(x))
• type(str.__dict__['__str__'].__get__(x, AnsibleUnsafeText)())
• x.__class__.to_fake_safe_str = str.__str__
• type(str.__str__(x))
• x.__class__.__base__.__str__(x)
• x.__str__ = x.__class__.__base__.__str__

If it's a hotfix on a friday evening I'd have probably settled with a search and replace of str(x) with str.__str__(x), but after a bit more thinking about this 'challenge' it's kinda easy to undo the entire patch on a per-instance basis (and that's probably also the safest way to shoot yourself in the foot, besides _strip_unsafe):

_str = AnsibleUnsafeText.__base__
class FakeSafeAnsibleUnsafeText(_str):
  pass

x = AnsibleUnsafeText("asdf")
x.__class__ = FakeSafeAnsibleUnsafeText
type(str(x))

Don't you all like reflection, introspection, and polymorphic code? 🙃

But before you use any of these, as always 'just because we could, doesn't mean we should' 😉

[SirUli]

Generally agree - to fix the problem I can see countless ways. I just had the issue in various places across multiple action modules and I initially had a hard time figuring out where this exactly comes from as I didn't expect a patch release to change such major areas.

The fix is quite easy for the Friday afternoon... Roll back to ansible-core 2.16.0 and care about it on Monday ;)

Cheers!

[briantist]

Just as a heads up, _strip_unsafe will not exist in 2.17, which is another reason it's not public.

Thanks for mentioning this here, I've been in the middle of working on trying to fix problems in my collection caused by the CVE patch, and was looking at that method as an option.

I was actually about to report a bug with it (that it seems to be completely broken in AnsibleUnsafeBytes) but I guess that isn't going to be fixed if it's not going to exist.

---

As for why I was stripping unsafe in the first place, it's due to the change in requests where values must be str or bytes (and not a subtype for some reason) otherwise it raises an exception:

• Breaking change in 2.28.0 when using string enums as Headers (working in v2.27.1) psf/requests#6159
• token must be of type str or bytes, not <class 'ansible.utils.unsafe_proxy.AnsibleUnsafeText'> ansible-collections/community.hashi_vault#289
• Fix "Unsafe" token values that originate from lookups ansible-collections/community.hashi_vault#291
• Fix namespace value with newer requests ansible-collections/community.hashi_vault#310

Looking now, it seems that they did in fact relax the requirements:

• Allow str/bytes subclasses to be used as header parts psf/requests#6356

So I'll look into what versions of requests I have to require now, but I know that the ansible-test containers control some of the versions so I'll have to see if this is feasible from the testing POV, hopefully I can get rid of all the workarounds.

[ChristianCiach]

Generally speaking, by the time that a module receives the arguments, you wouldn't be dealing with unsafe, as there is no concept of unsafe within a module.

I am sorry, but I can't follow you. We have a simple ActionPlugin that just consumes the task args, which happens to be a list of AnsibleUnsafeText:

from ansible.plugins.action import ActionBase

from ansible.utils.unsafe_proxy import AnsibleUnsafeText


class ActionModule(ActionBase):
    def run(self, tmp=None, task_vars=None):
        super().run(tmp, task_vars)
        
        for c in self._task.args["certs"]:
            assert type(c) == AnsibleUnsafeText

        return dict()

The action plugin is part of a role:

- name: 'Generate cacert.pem'
  cpng_cacerts:
    certs: '{{ query("ansible.builtin.fileglob", role_path ~ "/files/certs/*.pem") }}'

Are we using the Ansible API incorrectly? Or is it actually a bug in ansible that the certs parameter given to the action-plugin is actually list[AnsibleUnsafeText] instead of the expected list[str]?

[xeor]

For me now, the solution ended up being

In []: x, type(x)
Out[]: ('some text', ansible.utils.unsafe_proxy.AnsibleUnsafeText)

In []: x = "".join(x)

In []: x, type(x)
Out[]: ('some text', str)

hope there will be a better way to convert the string. Is this only a problem for pathlib?

[sivel]

This has been resolved in #82510 and will be included in 2.16.3, 2.15.9, and 2.14.14 scheduled for rc1 release on Jan 22, and GA on Jan 29.

If you have further questions please stop by IRC or the mailing list:

• IRC: #ansible on irc.libera.chat
• mailing list: https://groups.google.com/forum/#!forum/ansible-project