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

get_url pass incorrect If-Modified-Since header #67417

Closed
wangzw opened this issue Feb 14, 2020 · 4 comments · Fixed by #67419
Closed

get_url pass incorrect If-Modified-Since header #67417

wangzw opened this issue Feb 14, 2020 · 4 comments · Fixed by #67419
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. has_pr This issue has an associated PR. module This issue/PR relates to a module. net_tools Net-tools category P3 Priority 3 - Approved, No Time Limitation support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@wangzw
Copy link
Contributor

wangzw commented Feb 14, 2020

SUMMARY

get_url module add If-Modified-Since http request header. But its value does not follow HTTP protocol.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

get_url

ANSIBLE VERSION
ansible --version
ansible 2.9.4
  config file = None
  configured module search path = [u'/Users/wangzw/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Library/Python/2.7/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 2.7.16 (default, Dec  3 2019, 02:03:47) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.31)]
CONFIGURATION
None
OS / ENVIRONMENT
STEPS TO REPRODUCE
---
- hosts: localhost
  tasks:
    - name: test
      get_url:
        url: "http://xxx/xxx.jar"
        dest: "/tmp/xxx.jar"
        mode: '0700'
EXPECTED RESULTS

Run command ansible-playbook -v -i hosts test.yml twice. Success all the time.

ACTUAL RESULTS

Run command ansible-playbook -v -i hosts test.yml twice. Success at the first time and failed at the second time. HTTP Error 400: Bad Request is reported.

ansible-playbook -v -i hosts test.yml 
No config file found; using defaults

PLAY [localhost] ********************************************************************************************************************************************

TASK [Gathering Facts] **************************************************************************************************************************************
[WARNING]: Platform darwin on host localhost is using the discovered Python interpreter at /usr/bin/python, but future installation of another Python
interpreter could change this. See https://docs.ansible.com/ansible/2.9/reference_appendices/interpreter_discovery.html for more information.

ok: [localhost]

TASK [test] *************************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"changed": false, "dest": "/tmp/xxx.jar", "elapsed": 0, "gid": 20, "group": "staff", "mode": "0700", "msg": "Request failed", "owner": "wangzw", "response": "HTTP Error 400: Bad Request", "size": 94741968, "state": "file", "status_code": 400, "uid": 501, "url": "http://xxx/xxx.jar"}

PLAY RECAP **************************************************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   

ROOT CAUSE

If-Modified-Since: Fri, 14 Feb 2020 04:50:33 -0000 is added to the request header but the HTTP server rejects the request.

Response body:

<Error>
	<Code>InvalidArgument</Code>
	<Message>The If-Modified-Since you specified is not valid</Message>
	<Resource>xxx.jar</Resource>
	<RequestId>xxx</RequestId>
	<TraceId>xxx</TraceId>
</Error>

According to Hypertext Transfer Protocol -- HTTP/1.1, the value of header If-Modified-Since should follow the grammar as follow.

       HTTP-date    = rfc1123-date | rfc850-date | asctime-date
       rfc1123-date = wkday "," SP date1 SP time SP "GMT"
       rfc850-date  = weekday "," SP date2 SP time SP "GMT"
       asctime-date = wkday SP date3 SP time SP 4DIGIT
       date1        = 2DIGIT SP month SP 4DIGIT
                      ; day month year (e.g., 02 Jun 1982)
       date2        = 2DIGIT "-" month "-" 2DIGIT
                      ; day-month-year (e.g., 02-Jun-82)
       date3        = month SP ( 2DIGIT | ( SP 1DIGIT ))
                      ; month day (e.g., Jun  2)
       time         = 2DIGIT ":" 2DIGIT ":" 2DIGIT
                      ; 00:00:00 - 23:59:59
       wkday        = "Mon" | "Tue" | "Wed"
                    | "Thu" | "Fri" | "Sat" | "Sun"
       weekday      = "Monday" | "Tuesday" | "Wednesday"
                    | "Thursday" | "Friday" | "Saturday" | "Sunday"
       month        = "Jan" | "Feb" | "Mar" | "Apr"
                    | "May" | "Jun" | "Jul" | "Aug"
                    | "Sep" | "Oct" | "Nov" | "Dec"

And some examples:

      Sun, 06 Nov 1994 08:49:37 GMT  ; RFC 822, updated by RFC 1123
      Sunday, 06-Nov-94 08:49:37 GMT ; RFC 850, obsoleted by RFC 1036
      Sun Nov  6 08:49:37 1994       ; ANSI C's asctime() format

And value Fri, 14 Feb 2020 04:50:33 -0000 passed by get_url does not follow above grammar. Correct value shoud be Fri, 14 Feb 2020 04:50:33 GMT

RELEATED ISSUE

#44857
#44868

@wangzw wangzw changed the title get_url pass incorrect If-Modified-Since header get_url pass incorrect If-Modified-Since header Feb 14, 2020
@ansibot
Copy link
Contributor

ansibot commented Feb 14, 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

@ansibot
Copy link
Contributor

ansibot commented Feb 14, 2020

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. net_tools Net-tools category support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Feb 14, 2020
@ansibot ansibot added the has_pr This issue has an associated PR. label Feb 14, 2020
@wangzw
Copy link
Contributor Author

wangzw commented Feb 19, 2020

Any comments on this issue?

@samdoran samdoran added needs_verified This issue needs to be verified/reproduced by maintainer P3 Priority 3 - Approved, No Time Limitation and removed needs_triage Needs a first human triage before being processed. labels Feb 25, 2020
@samdoran
Copy link
Contributor

cc @sivel

relrod pushed a commit that referenced this issue Apr 10, 2020
Fix #67417. HTTP header value of `If-Modified-Since` set by `get_url` does not follow HTTP protocol.
mattclay pushed a commit that referenced this issue Apr 14, 2020
Fix #67417. HTTP header value of `If-Modified-Since` set by `get_url` does not follow HTTP protocol.

(cherry picked from commit 1097694)
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Apr 29, 2020
v2.9.7
======

Minor Changes
-------------

- 'Edit on GitHub' link for plugin, cli documentation fixed to navigate to correct plugin, cli source.
- Handle get_tags_for_object API correctly in vmware_rest_client.
- Remove redundant encoding in json.load call in ipa module_utils (ansible/ansible#66592).
- ansible-test - Upgrade OpenSUSE containers to use Leap 15.1.
- ansible-test now supports testing against RHEL 7.8 when using the ``--remote`` option.
- vmware_cluster - Document alternatives for deprecated parameters

Removed Features (previously deprecated)
----------------------------------------

- ldap_attr, ldap_entry - The ``params`` option has been removed in Ansible-2.10 as it circumvents Ansible's option handling.  Setting ``bind_pw`` with the ``params`` option was disallowed in Ansible-2.7, 2.8, and 2.9 as it was insecure.  For information about this policy, see the discussion at: https://meetbot.fedoraproject.org/ansible-meeting/2017-09-28/ansible_dev_meeting.2017-09-28-15.00.log.html This fixes CVE-2020-1746

Bugfixes
--------

- **security issue** - The ``subversion`` module provided the password via the svn command line option ``--password`` and can be retrieved from the host's /proc/<pid>/cmdline file. Update the module to use the secure ``--password-from-stdin`` option instead, and add a warning in the module and in the documentation if svn version is too old to support it. (CVE-2020-1739)

- **security issue** win_unzip - normalize paths in archive to ensure extracted files do not escape from the target directory (CVE-2020-1737)

- **security_issue** - create temporary vault file with strict permissions when editing and prevent race condition (CVE-2020-1740)
- Alter task_executor's start_connection to support newer modules from collections which expect to send task UUID.
- Ansible.ModuleUtils.WebRequest - actually set no proxy when ``use_proxy: no`` is set on a Windows module - ansible/ansible#68528
- Ensure DataLoader temp files are removed at appropriate times and that we observe the LOCAL_TMP setting.
- Ensure we don't allow ansible_facts subkey of ansible_facts to override top level, also fix 'deprefixing' to prevent key transforms.
- Ensure we get an error when creating a remote tmp if it already exists. CVE-2020-1733
- Fact Delegation - Add ability to indicate which facts must always be delegated. Primarily for ``discovered_interpreter_python`` right now, but extensible later. (ansible/ansible#61002)
- Fix nxos_lacp replace operation (ansible/ansible#64074).
- Handle equal sign in password while using passwordstore lookup plugin.
- In fetch action, avoid using slurp return to set up dest, also ensure no dir traversal CVE-2019-3828.
- In vmware_guest_network module use appropriate network while creating or reconfiguring (ansible/ansible#65968).
- Log additional messages from persistent connection modules that may be missed if the module fails or returns early.
- `vmware_content_deploy_template`'s `cluster` argument no longer fails with an error message about resource pools.
- ansible command now correctly sends v2_playbook_on_start to callbacks
- ansible-galaxy - Error when install finds a tar with a file that will be extracted outside the collection install directory - CVE-2020-10691
- ansible-galaxy collection - Preserve executable bit on build and preserve mode on install from what tar member is set to - ansible/ansible#68415
- dense callback - fix plugin access to its configuration variables and remove a warning message (ansible/ansible#64628).
- display - Improve method of removing extra new line after warnings so it does not break Tower/Runner (ansible/ansible#68517)
- docker connection plugin - do not prefix remote path if running on Windows containers.
- for those running uids for invalid users (containers), fallback to uid=<uid> when logging fixes
- get_url pass incorrect If-Modified-Since header (ansible/ansible#67417)
- mysql_user - Fix idempotence when long grant lists are used (ansible/ansible#68044)
- os_user_role - Fix os_user_role issue to grant a role in a domain.
- ovirt_storage_domain: fix update_check for warning_low_space
- purefa_snmp - Fix error when deleting a manager and when creating a v2c manager (ansible/ansible#68180)
- rabbitmq_policy - Fix version parsing for RabbitMQ 3.8.
- routeros_facts - Prevent crash of module when ``ipv6`` package is not installed
- setup.ps1 - Fix ``ansible_fqdn`` using the wrong values to build the actual DNS FQDN.
@ansible ansible locked and limited conversation to collaborators May 8, 2020
@sivel sivel removed the needs_verified This issue needs to be verified/reproduced by maintainer label Feb 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. has_pr This issue has an associated PR. module This issue/PR relates to a module. net_tools Net-tools category P3 Priority 3 - Approved, No Time Limitation support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants