From 714c50bdb711082bb6430758e5ed9103fee2531c Mon Sep 17 00:00:00 2001 From: Hideki Saito Date: Mon, 18 May 2026 10:18:52 +0900 Subject: [PATCH] Merge pull request #639 from Klaas-/Klaas-fix_authorized_key (#731) Fixes #462 notice permission denied on authorized_key module SUMMARY As of right now the authorized_key module does not notice on an "absent" if a authorized_keys file is simply not readable to the executing user. I am trying to fix that ISSUE TYPE Bugfix Pull Request COMPONENT NAME authorized_key ADDITIONAL INFORMATION Execute as a user that does not have access to the root users authorized keys file - name: Delete key from root user ansible.posix.authorized_key: state: absent user: root key: ssh-rsa xxxxxxxx - name: Delete key from root user become: true ansible.posix.authorized_key: state: absent user: root key: ssh-rsa xxxxxxxx The one without become will succeed before my change and will fail with a permission denied error after my change. The 2nd task will actually remove a key from root user if become privileges are available for the executing user Reviewed-by: Brian Coca Reviewed-by: Klaas Demter Reviewed-by: Felix Fontein Reviewed-by: Hideki Saito (cherry picked from commit 72a6eb972944dcb6938673e1b0b6031d6b037b83) Co-authored-by: softwarefactory-project-zuul[bot] <33884098+softwarefactory-project-zuul[bot]@users.noreply.github.com> --- .../fragments/639_fix_authorized_key.yml | 3 ++ plugins/modules/authorized_key.py | 24 ++++++----- .../tasks/check_permissions.yml | 41 +++++++++++++++++++ .../targets/authorized_key/tasks/main.yml | 3 ++ 4 files changed, 61 insertions(+), 10 deletions(-) create mode 100644 changelogs/fragments/639_fix_authorized_key.yml create mode 100644 tests/integration/targets/authorized_key/tasks/check_permissions.yml diff --git a/changelogs/fragments/639_fix_authorized_key.yml b/changelogs/fragments/639_fix_authorized_key.yml new file mode 100644 index 0000000..4477ce4 --- /dev/null +++ b/changelogs/fragments/639_fix_authorized_key.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - ansible.posix.authorized_key - fixes error on permission denied in authorized_key module (https://github.com/ansible-collections/ansible.posix/issues/462). diff --git a/plugins/modules/authorized_key.py b/plugins/modules/authorized_key.py index 1d68b4e..d712a10 100644 --- a/plugins/modules/authorized_key.py +++ b/plugins/modules/authorized_key.py @@ -225,6 +225,8 @@ import os.path import tempfile import re import shlex +import errno +import traceback from operator import itemgetter from ansible.module_utils._text import to_native @@ -475,16 +477,18 @@ def parsekey(module, raw_key, rank=None): return (key, key_type, options, comment, rank) -def readfile(filename): - - if not os.path.isfile(filename): - return '' - - f = open(filename) +def readfile(module, filename): try: - return f.read() - finally: - f.close() + with open(filename, 'r') as f: + return f.read() + except IOError as e: + if e.errno == errno.EACCES: + module.fail_json(msg="Permission denied on file or path for authorized keys file: %s" % filename, + exception=traceback.format_exc()) + elif e.errno == errno.ENOENT: + return '' + else: + raise def parsekeys(module, lines): @@ -597,7 +601,7 @@ def enforce_state(module, params): # check current state -- just get the filename, don't create file do_write = False params["keyfile"] = keyfile(module, user, do_write, path, manage_dir) - existing_content = readfile(params["keyfile"]) + existing_content = readfile(module, params["keyfile"]) existing_keys = parsekeys(module, existing_content) # Add a place holder for keys that should exist in the state=present and diff --git a/tests/integration/targets/authorized_key/tasks/check_permissions.yml b/tests/integration/targets/authorized_key/tasks/check_permissions.yml new file mode 100644 index 0000000..2d77059 --- /dev/null +++ b/tests/integration/targets/authorized_key/tasks/check_permissions.yml @@ -0,0 +1,41 @@ +--- +# ------------------------------------------------------------- +# check permissions + +- name: Create a file that is not accessible + ansible.builtin.file: + state: touch + path: "{{ output_dir | expanduser }}/file_permissions" + owner: root + mode: '0000' + +- name: Create unprivileged user + ansible.builtin.user: + name: nopriv + create_home: true + +- name: Try to delete a key from an unreadable file + become: true + become_user: nopriv + ansible.posix.authorized_key: + user: root + key: "{{ dss_key_basic }}" + state: absent + path: "{{ output_dir | expanduser }}/file_permissions" + register: result + ignore_errors: true + +- name: Assert that the key deletion has failed + ansible.builtin.assert: + that: + - result is failed + +- name: Remove the file + ansible.builtin.file: + state: absent + path: "{{ output_dir | expanduser }}/file_permissions" + +- name: Remove the user + ansible.builtin.user: + name: nopriv + state: absent diff --git a/tests/integration/targets/authorized_key/tasks/main.yml b/tests/integration/targets/authorized_key/tasks/main.yml index d687f17..761b6d5 100644 --- a/tests/integration/targets/authorized_key/tasks/main.yml +++ b/tests/integration/targets/authorized_key/tasks/main.yml @@ -34,3 +34,6 @@ - name: Test for specifying key as a path ansible.builtin.import_tasks: check_path.yml + +- name: Test for permission denied files + ansible.builtin.import_tasks: check_permissions.yml