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

Ansible 7 (core 2.14) support, AnsibleUnsafeText fixes #1017

Merged
merged 4 commits into from
Apr 4, 2024

Conversation

moreati
Copy link
Member

@moreati moreati commented Aug 2, 2023

Rather than allowing AnsibleUnsafeText to be pickled/unpickled this PR adds ansible_mitogen.utils.unsafe.cast() to cast them back to plain strings before sending to the target. This follows the implicit behaviour of plain Ansible, in which serialising to JSON then deserialising has the same effect.

Thanks to @opoplawski for PR #977 on which this is based, and @AKrumov for investigation in #1034.

@moreati moreati changed the title pr977 rebuild pr977 rebuild (ansible-core 2.14) Aug 3, 2023
@doobry-systemli
Copy link

Thanks a lot for looking into this @moreati!

@guenhter
Copy link

@moreati Any update on this? Is there something holding this back?

@dj-wasabi
Copy link

And what about using 2.15? Or do I have to create a seperate issue for?

@hishamanver
Copy link

LGTM! <3

@ialmahi22
Copy link

@moreati Thanks for looking into this, we're using Ansible 2.14 and after this PR, is there any pending updates on it?

@opoplawski
Copy link
Contributor

Fixes #974

@moreati moreati force-pushed the 2.14 branch 15 times, most recently from a7c81e1 to 745cd4b Compare March 28, 2024 00:40
opoplawski and others added 3 commits March 28, 2024 14:16
Co-authored-by: Orion Poplawski <orion@nwra.com>
If casting a string fails then raise a TypeError. This is potentially an API
breaking change; chosen as the lesser evil vs. allowing silent errors.

`cast()` relies on `bytes(obj)` & `str(obj)` returning the respective
supertype. That's no longer the case for `AnsibleUnsafeBytes` &
`AnsibleUnsafeText`; since fixes/mitigations for  CVE-2023-5764.

fixes mitogen-hq#1046, refs mitogen-hq#977

See also
- GHSA-7j69-qfc3-2fq9
- ansible/ansible#82293
Prep work for ansible_mitogen.utils.unsafe
@moreati moreati force-pushed the 2.14 branch 2 times, most recently from be1708e to 7694334 Compare March 28, 2024 15:02
Follwing fixes in Ansible 7-9 for CVE-2023-5764 cating `AnsibleUnsafeBytes` &
`AnsibleUnsafeText` to `bytes()` or `str()` requires special handling. The
handling is Ansible specific, so it shouldn't go in the mitogen package but
rather the ansible_mitogen package.

`ansible_mitogen.utils.unsafe.cast()` is most like `mitogen.utils.cast()`.
During development it began as `ansible_mitogen.utils.unsafe.unwrap_var()`,
closer to an inverse of `ansible.utils.unsafe_procy.wrap_var()`. Future
enhancements may move in this direction.

refs mitogen-hq#977, refs mitogen-hq#1046

See also
- GHSA-7j69-qfc3-2fq9
- ansible/ansible#82293
- https://github.com/mitogen-hq/mitogen/wiki/AnsibleUnsafe-notes
@moreati moreati marked this pull request as ready for review March 29, 2024 09:54
@moreati moreati changed the title pr977 rebuild (ansible-core 2.14) Ansible 7 (core 2.14) support, AnsibleUnsafeText fixes Mar 29, 2024
@huyz
Copy link

huyz commented Mar 29, 2024

I'm trying this out (first time trying out Mitogen actually) and I get an error fairly early in my playbook.

My task is:

    - name: Add to authorized_keys the content of {{ pub_files_for_ssh }}
      ansible.posix.authorized_key:
        user: "{{ ansible_user }}"
        key: "{{ lookup('file', item) }}"
        comment: >-
          {{ item
            | replace('~/.ssh/+', '+')
            | replace('~/.ssh/', '+' + controller_hostname + '/')
            | replace('.pub', '')
            | trim }} (by ansible@{{ controller_hostname }})
      loop: "{{ pub_files_for_ssh }}"

Looks like there's a problem while invoking the lookup command:

File lookup using /Users/huyz/.ssh/id_ed25519.pub as file
[task 72194] 12:26:27.559050 D ansible_mitogen.process: will use multiplexer 0 (/var/folders/cd/80666smx571dkz_nttfhf7dw0000gn/T/mitogen_unix_oqpc2g0s.sock) to connect to "tmac"

[…]

[mux  72009] 12:26:27.632176 D mitogen.io: SetupProtocol(ssh.tmac).on_receive()
[mux  72009] 12:26:27.632602 D mitogen.parent: ssh.tmac: (unrecognized): hostname contains invalid characters

[…]

[mux  72009] 12:26:27.636263 D mitogen: <Side of ssh.tmac fd 117>: empty read, disconnecting
[mux  72009] 12:26:27.636533 D mitogen.parent: failing connection ssh.tmac due to EofError('EOF on stream; last 100 lines received:\nhostname contains invalid characters\r')
[mux  72009] 12:26:27.636799 D mitogen.parent: PopenProcess ssh.tmac pid 72244: exited with return code 255

[…]

TASK [Add to authorized_keys the content of ['~/.ssh/id_ed25519.pub'] ********************************************************************************************************************************************************************
failed: [tmac] (item=~/.ssh/id_ed25519.pub) => {
    "ansible_loop_var": "item",
    "item": "~/.ssh/id_ed25519.pub",
    "msg": "EOF on stream; last 100 lines received:\nhostname contains invalid characters\r",
    "unreachable": true
}

@moreati
Copy link
Member Author

moreati commented Apr 2, 2024

I'm trying this out (first time trying out Mitogen actually) and I get an error fairly early in my playbook.

Could you reduce this to a Minimal Reprodicing Example? It's difficult to say what is causing the failure in your pasted code - without knowing values of the variables involved.

Could you also comfirm whether the latest release of Mitogen (0.3.5) exhibits the same error? You may need to try it with an earlier release of Ansible.

@moreati moreati merged commit b822f20 into mitogen-hq:master Apr 4, 2024
44 checks passed
@moreati moreati deleted the 2.14 branch April 4, 2024 08:11
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.

8 participants