Custom credential file injector writes literal "\n" instead of newline

[ghjm]

ISSUE TYPE

• Bug Report

SUMMARY

After creating a custom credential type with multiline: true and using a file injector to write it to a file, the resulting file is a single line of text with the literal characters "\n" where the line breaks should be.

ENVIRONMENT

• Tower 3.8.1
• Ansible 2.9.15
• Operating System: RHEL 8.3

STEPS TO REPRODUCE

Create a custom credential type similar to the following:

{
    "name": "SSH Keypair",
    "kind": "cloud",
    "inputs": {
        "fields": [
            {
                "id": "public_key",
                "type": "string",
                "label": "Public Key",
                "multiline": false
            },
            {
                "id": "private_key",
                "type": "string",
                "label": "Private Key",
                "format": "ssh_private_key",
                "secret": true,
                "multiline": true
            }
        ],
        "required": [
            "public_key",
            "private_key"
        ]
    },
    "injectors": {
        "file": {
            "template.public_file": "{{ public_key }}",
            "template.private_file": "{{ private_key }}"
        },
        "extra_vars": {
            "ssh_public_key": "{{ tower.filename.public_file }}",
            "ssh_private_key": "{{ tower.filename.private_file }}"
        }
    }
}

From the UI, create a credential of this type. Run a playbook that uses it. Try to use it as an ansible_ssh_private_key_file in an add_host. Observe that connecting to the host fails with an OpenSSH error of invalid format.

EXPECTED RESULTS

That the ssh_private_key file would be a valid SSH private key, with newlines.

ACTUAL RESULTS

The ssh_private_key file is a single line of text, with literal "\n" characters where the newlines should be.

[ghjm]

If you go to the credential in the API and PATCH the private key (with \n characters in the JSON), it gets injected correctly, with a file containing newlines. The problem only seems to happen if you use the UI to set the private key.

[chrismeyersfsu]

Thanks @ghjm this might explain a 1-off that I witnessed when configuring the SAML public/private key via the UI.

[jakemcdermott]

There's a good chance this is addressed on devel - moving to test for verification.

[unlikelyzero]

@akus062381 can you perform manually verification? If you have questions, you can always ask @ghjm :)

[akus062381]

@ghjm I'd like to chat with you about this if you're available next week

[ghjm]

Sure @akus062381, I'm happy to chat about it

[ryanpetrello]

Hey @ghjm,

@akus062381 and I spent some time trying to reproduce this, and I think we're seeing the same thing:

- name: Set up a new host
  hosts: all
  tasks:
    - add_host:
        name: 'ryanpetrello.com'
        ansible_ssh_private_key_file: "{{ ssh_private_key }}"

- name: Talk to ryanpetrello.com
  hosts: ryanpetrello.com
  tasks:
    - name: Hello Message
      debug:
        msg: "Hello World!"

[ryanpetrello]

cc @jakemcdermott I think this might actually be a bug in the way the UI saves multi-line inputs like private keys. What do you think?

[ryanpetrello]

@ghjm I don't think the issue is that the pasted value the UI sends has incorrect newlines, I think it's that it doesn't end with a newline. When I reproduced this above, I noticed that the file laid down by AWX looked correct, but that SSH didn't like it unless I added newline to the end of the private key. This diff fixes the problem for me in devel, does it fix it for you?

diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py
index 1d29fbb5e7..f37bfee884 100644
--- a/awx/main/models/credential/__init__.py
+++ b/awx/main/models/credential/__init__.py
@@ -466,10 +466,14 @@ class CredentialType(CommonModelNameNotUnique):
             if len(value):
                 namespace[field_name] = value

-        # default missing boolean fields to False
         for field in self.inputs.get('fields', []):
+            # default missing boolean fields to False
             if field['type'] == 'boolean' and field['id'] not in credential.inputs.keys():
                 namespace[field['id']] = safe_namespace[field['id']] = False
+            # make sure private keys end with a \n
+            if field.get('format') == 'ssh_private_key':
+                if field['id'] in namespace and not namespace[field['id']].endswith('\n'):
+                    namespace[field['id']] += '\n'

[ryanpetrello]

For what it's worth, the code for injecting SSH (Machine) credentials does something similar ☝️

awx/awx/main/tasks.py

Lines 973 to 976 in 2799d09

	# OpenSSH formatted keys must have a trailing newline to be
	# accepted by ssh-add.
	if 'OPENSSH PRIVATE KEY' in data and not data.endswith('\n'):
	data += '\n'

cc @jakemcdermott @chrismeyersfsu @AlanCoding

[wenottingham]

ssh-agent requires a trailing newline on openssh format keys that it doesn't on non-openssh format keys. Why? Who knows.

ansible/ansible-runner#544 (comment)

[ghjm]

Yes, that patch fixes it for me @ryanpetrello @akus062381. Or just putting a blank line after the key.

[ryanpetrello]

Thanks @ghjm, I'll open up a PR.

[jakemcdermott]

This all seems vaguely familiar. I think we've encountered a bug like this before but in a different context.

[jakemcdermott]

Oh, yep 😅 see bill's comment

Looks like we'll add in the newline. Another option could be validating the input data before saving but modifying it automatically before running seems nicer and more convenient.

@ryanpetrello @akus062381 Just to confirm my understanding so far:

I don't think the issue is that the pasted value the UI sends has incorrect newlines I think it's that it doesn't end with a newline.

Is the ui unexpectedly deleting the newline from the user's data before presenting it to the api? If so I'd like to fix this client-side as well.

[ryanpetrello]

I don't think we need UI work here. The data could come in from other clients too. We've got precedent on auto-fixing this folks on the backend in other places:

#9361 (comment)

If you want to push a commit to my PR, though, go for it 😄

[chrismeyersfsu]

I think this is a problem for the saml certs also.

[AlanCoding]

If you want to push a commit to my PR, though, go for it 😄

Double-check me on this, but it looks like @ryanpetrello's merged PR does this for the ssh_private_key format. So this shouldn't need anything else done, it just needs to be tested, is that right?

[akus062381]

I was able to verify that this has been fixed.