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

The default permissions used by atomic_move can create files that are world readable #67794

Closed
samdoran opened this issue Feb 26, 2020 · 20 comments · Fixed by #68970 or #70221
Closed

The default permissions used by atomic_move can create files that are world readable #67794

samdoran opened this issue Feb 26, 2020 · 20 comments · Fixed by #68970 or #70221
Labels
affects_2.10 This issue/PR affects Ansible v2.10 bug This issue/PR relates to a bug. has_pr This issue has an associated PR. security Related to a vulnerability or CVE support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@samdoran
Copy link
Contributor

samdoran commented Feb 26, 2020

SUMMARY

CVE-2020-1736

If a file doesn't exist, we create it with 0666 permissions combined with the current umask. Depending on the default umask as well as the permissions on the destination directory, this could result in world readable files.

Relevant code:
Default permissions:

_PERM_BITS = 0o7777 # file mode permission bits
_EXEC_PERM_BITS = 0o0111 # execute permission bits
_DEFAULT_PERM = 0o0666 # default file permission bits

creation in atomic_move:

if creating:
# make sure the file has the correct permissions
# based on the current value of umask
umask = os.umask(0)
os.umask(umask)
os.chmod(b_dest, DEFAULT_PERM & ~umask)
try:
os.chown(b_dest, os.geteuid(), os.getegid())

if creating:
    # make sure the file has the correct permissions
    # based on the current value of umask
    umask = os.umask(0)
    os.umask(umask)
    os.chmod(b_dest, DEFAULT_PERM & ~umask)
    try:
        os.chown(b_dest, os.geteuid(), os.getegid())
    except OSError:
        # We're okay with trying our best here.  If the user is not
        # root (or old Unices) they won't be able to chown.
        pass

Suggested correction is to use more restrictive permissions by default and/or add a mechanism to prevent creating world readable files.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

ansible/lib/ansible/module_utils/common/file.py
ansible/lib/ansible/module_utils/basic.py

ANSIBLE VERSION
2.10
CONFIGURATION
default
OS / ENVIRONMENT
STEPS TO REPRODUCE
EXPECTED RESULTS
ACTUAL RESULTS

@samdoran samdoran added the security Related to a vulnerability or CVE label Feb 26, 2020
@ansibot
Copy link
Contributor

ansibot commented Feb 26, 2020

Files identified in the description:
None

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

click here for bot help

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 bug This issue/PR relates to a bug. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Feb 26, 2020
@ansibot
Copy link
Contributor

ansibot commented Feb 26, 2020

Files identified in the description:

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

click here for bot help

@bcoca
Copy link
Member

bcoca commented Apr 2, 2020

@ansibot ansibot added the has_pr This issue has an associated PR. label Apr 2, 2020
@bcoca bcoca self-assigned this Apr 3, 2020
bcoca added a commit to bcoca/ansible that referenced this issue Apr 16, 2020
  fixes ansible#67794
  updated some tests that expected previous defaults
  CVE-2020-1736
bcoca added a commit to bcoca/ansible that referenced this issue Apr 16, 2020
  fixes ansible#67794
  updated some tests that expected previous defaults
  CVE-2020-1736
bcoca added a commit that referenced this issue Apr 16, 2020
fixes #67794
  updated some tests that expected previous defaults
  CVE-2020-1736
bcoca added a commit to bcoca/ansible that referenced this issue Apr 16, 2020
…8970)

fixes ansible#67794
  updated some tests that expected previous defaults
  CVE-2020-1736

(cherry picked from commit 566f246)
bcoca added a commit to bcoca/ansible that referenced this issue Apr 16, 2020
…8970)

fixes ansible#67794
  updated some tests that expected previous defaults
  CVE-2020-1736

(cherry picked from commit 566f246)
@bcoca bcoca reopened this Apr 16, 2020
@bcoca
Copy link
Member

bcoca commented Apr 16, 2020

reopened since PR that had been merged was reverted due to much larger impact than initially forseen.

@brianmay
Copy link

brianmay commented May 6, 2020

@bcoca Can you please point me to the commit were this was reverted? I am not seeing it. https://github.com/bcoca/ansible/commits/2127777856f81bd03666db5a68b379901a849845/lib/ansible/module_utils/common/file.py

@bcoca
Copy link
Member

bcoca commented May 7, 2020

5733660

@brianmay
Copy link

brianmay commented May 7, 2020

OK, so according to #68983 it appears this was reverted because we broke some tests in collections. What is the solution? Maybe we need a DEFAULT_UMASK setting or something. initially this could be 666, over time we could work to move this to 660.

@ansibot ansibot added the needs_triage Needs a first human triage before being processed. label May 17, 2020
@jimi-c jimi-c removed the needs_triage Needs a first human triage before being processed. label May 19, 2020
@samdoran samdoran assigned samdoran and unassigned bcoca Jun 18, 2020
nre-ableton added a commit to Ableton/ansible-role-jenkins-jcasc that referenced this issue Aug 18, 2020
As of Ansible 2.9.12, the default modes are stricter. See:
ansible/ansible#67794
@eoli3n
Copy link
Contributor

eoli3n commented Aug 19, 2020

It broke my playbook too.

Ansible should do the expected thing of deferring to the system umask or mode in the module for permissions on the final file.

Yes it should.

ghost pushed a commit to Ableton/ansible-role-jenkins-jcasc that referenced this issue Aug 19, 2020
As of Ansible 2.9.12, the default modes are stricter. See:
ansible/ansible#67794
nre-ableton added a commit to Ableton/ansible-role-jenkins-jcasc that referenced this issue Aug 19, 2020
As of Ansible 2.9.12, the default modes are stricter. See:
ansible/ansible#67794
@HontoNoRoger
Copy link

I also ran into this issue today, using Ansible 2.9.12 from the Launchpad Ansible repo:
http://ppa.launchpad.net/ansible/ansible/ubuntu bionic/main amd64 Packages

Could it be that is hasn't been reverted there? It is causing a lot of trouble for our automated installations currently.

@relrod
Copy link
Member

relrod commented Aug 20, 2020

Correct, there has not yet been another release since the change was reverted. The revert will be in the next 2.9 release.

@n-cc
Copy link

n-cc commented Aug 20, 2020

Is there a timeline for a new release? I'm specifically interested in 2.8, but it'd probably be helpful to know both.

@relrod
Copy link
Member

relrod commented Aug 20, 2020

Likely around 8/31 or 9/1. Releases typically happen every 3 weeks (though that is changing slightly, soon).

openstack-mirroring pushed a commit to openstack/openstack-ansible-os_tempest that referenced this issue Aug 22, 2020
We need to set mode for copy operation otherwise ansible-2.9.12
can lead to too restrictive default permissions[1].

Setting mode for copy operations handles the warning.

[1.] ansible/ansible#67794

Change-Id: Ieae36a1d85a7da84ee2cd982a20dcabe9e65511f
Signed-off-by: Chandan Kumar (raukadah) <chkumar@redhat.com>
openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Aug 22, 2020
* Update openstack-ansible-os_tempest from branch 'master'
  - Set mode for copy operation
    
    We need to set mode for copy operation otherwise ansible-2.9.12
    can lead to too restrictive default permissions[1].
    
    Setting mode for copy operations handles the warning.
    
    [1.] ansible/ansible#67794
    
    Change-Id: Ieae36a1d85a7da84ee2cd982a20dcabe9e65511f
    Signed-off-by: Chandan Kumar (raukadah) <chkumar@redhat.com>
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Aug 24, 2020
v2.9.12
=======

Minor Changes
-------------
- ansible-test - the ACME test container was updated, it now supports external account creation and has a basic OCSP responder (ansible/ansible#71097, https://github.com/ansible/acme-test-container/releases/tag/2.0.0).
- debconf - add a note about no_log=True since module might expose sensitive information to logs (ansible/ansible#32386).

Security Fixes
--------------
- **security issue** - copy - Redact the value of the no_log 'content' parameter in the result's invocation.module_args in check mode. Previously when used with check mode and with '-vvv', the module would not censor the content if a change would be made to the destination path. (CVE-2020-14332)

- **security issue** atomic_move - change default permissions when creating temporary files so they are not world readable (ansible/ansible#67794) (CVE-2020-1736)

- Fix warning for default permission change when no mode is specified. Follow up to ansible/ansible#67794. (CVE-2020-1736)

- Sanitize no_log values from any response keys that might be returned from the uri module (CVE-2020-14330).
- reset logging level to INFO due to CVE-2019-14846.

Bugfixes
--------
- Address compat with rpmfluff-0.6 for integration tests
- Ensure password passed in by -k is used on delegated hosts that do not have ansible_password set
- Template connection variables before using them (ansible/ansible#70598).
- Terminal plugins - add "\e[m" to the list of ANSI sequences stripped from device output
- add magic/connection vars updates from delegated host info.
- ansible-galaxy collection install - fix fallback mechanism if the AH server did not have the collection requested - ansible/ansible#70940
- ansible-test - Add ``pytest < 6.0.0`` constraint for managed installations on Python 3.x to avoid issues with relative imports.
- ansible-test - Change detection now properly resolves relative imports instead of treating them as absolute imports.
- api - time.clock is removed in Python 3.8, add backward compatible code (ansible/ansible#70649).
- avoid clobbering existing facts inside loop when task also returns ansible_facts.
- basic - use PollSelector implementation when DefaultSelector fails (ansible/ansible#70238).
- cron - encode and decode crontab files in UTF-8 explicitly to allow non-ascii chars in cron filepath and job (ansible/ansible#69492)
- ensure delegated vars can resolve hostvars object and access vars from hostvars[inventory_hostname].
- facts - account for Slackware OS with ``+`` in the name (ansible/ansible#38760)
- facts - fix incorrect UTC timestamp in ``iso8601_micro`` and ``iso8601``
- fix issue with inventory_hostname and delegated host vars mixing on connection settings.
- hashi_vault - Handle equal sign in key=value (ansible/ansible#55658).
- ipa_hostgroup - fix an issue with load-balanced ipa and cookie handling with Python 3 - (ansible/ansible#71110).
- lineinfile - fix not subscriptable error in exception handling around file creation
- linux network facts - get the correct value for broadcast address (ansible/ansible#64384)
- mysql_user - fix overriding password to the same (ansible-collections/community.general#543).
- net_put - Fixed UnboundLocalError when there is no change This is a backport from U(ansible-collections/ansible.netcommon#6)
- nxos_user - do not fail when a custom role is used (ansible-collections/cisco.nxos#130)
- ovirt_vm - fix cd_iso search
- playbooks - detect and propagate failures in ``always`` blocks after ``rescue`` (ansible/ansible#70000)
- profile_tasks - typecast result before slicing it (ansible/ansible#59059).
- reboot - Add support for the runit init system, used on Void Linux, that does not support the normal Linux syntax.
- redfish_info, redfish_config, redfish_command - Fix Redfish response payload decode on Python 3.5 (ansible/ansible#65889)
- shell - fix quoting of mkdir command in creation of remote_tmp in order to allow spaces and other special characters (ansible/ansible#69577).
- templating - fix error message for ``x in y`` when y is undefined (ansible/ansible#70984)
- unarchive - check ``fut_gid`` against ``run_gid`` in addition to supplemental groups (ansible/ansible#49284)
- user - don't create home directory and missing parents when create_home == false (ansible/ansible#70600).
- yum - fix yum list crashing if repoquery (used internally) prints errors in stdout (ansible/ansible#56800)
samy-mahmoudi added a commit to samy-mahmoudi/ansible that referenced this issue Oct 7, 2020
Add a comment to parameter 'mode' to highlight CVE-2020-1736
and the versions of Ansible that are vulnerable.

As of Ansible 2.9.10, this vulnerability has not been properly
addressed so that string '2.9.10' is subject to a future change.

Tickets:
- Refs ansible#67794
- Refs ansible#71324
samy-mahmoudi added a commit to samy-mahmoudi/ansible that referenced this issue Oct 7, 2020
Add a comment to parameter 'mode' to highlight CVE-2020-1736
and the versions of Ansible that are vulnerable.

As of Ansible 2.9.10, this vulnerability has not been properly
addressed so that string '2.9.10' is subject to a future change.

Tickets:
- Refs ansible#67794
- Refs ansible#71324
samy-mahmoudi added a commit to samy-mahmoudi/ansible that referenced this issue Oct 7, 2020
Add a comment to parameter 'mode' to highlight CVE-2020-1736
and the versions of Ansible that are vulnerable.

As of Ansible 2.9.10, this vulnerability has not been properly
addressed so that string '2.9.10' is subject to a future change.

Tickets:
- Refs ansible#67794
- Refs ansible#71324
@ansibot
Copy link
Contributor

ansibot commented Feb 16, 2021

Files identified in the description:

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

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Feb 24, 2021

Files identified in the description:

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

click here for bot help

@samdoran samdoran removed their assignment Jul 13, 2021
@mvorisek
Copy link

mvorisek commented Dec 7, 2021

I wonder is there any Ansible config where I can simply set the default file/dir mode or umask to 0644? Or is 0644 always the default?

@leegarrett
Copy link
Contributor

I wonder is there any Ansible config where I can simply set the default file/dir mode or umask to 0644? Or is 0644 always the default?

Read the bug report. The current umask is whatever the target system has set as default, which for Debian-based systems is 0022. You don't want to set it to 0644. IMHO this isn't a security bug, because you have to deliberately set the umask to make files world-readable ... to get world-readable files.

@mvorisek
Copy link

mvorisek commented Dec 7, 2021

I meant if the 0022 umask can be enforced as a default by ansible configuration.

@leegarrett
Copy link
Contributor

I meant if the 0022 umask can be enforced as a default by ansible configuration.

Please ask in a support channel as this is unrelated to the bug report.

@bcoca
Copy link
Member

bcoca commented Jul 20, 2022

closing this as it is not really an ansible bug, but how the user has configured umask on their system.

@bcoca bcoca closed this as completed Jul 20, 2022
@ansible ansible locked and limited conversation to collaborators Jul 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.10 This issue/PR affects Ansible v2.10 bug This issue/PR relates to a bug. has_pr This issue has an associated PR. security Related to a vulnerability or CVE support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet