Skip to content

Fixes #25402 - bad handshake failure#26235

Closed
aurelienmaury wants to merge 3 commits intoansible:develfrom
aurelienmaury:devel
Closed

Fixes #25402 - bad handshake failure#26235
aurelienmaury wants to merge 3 commits intoansible:develfrom
aurelienmaury:devel

Conversation

@aurelienmaury
Copy link
Copy Markdown

@aurelienmaury aurelienmaury commented Jun 29, 2017 •
edited
Loading

SUMMARY

This is a patch candidate for the #25402 issue.

This modifies the PROTOCOL floor value in urls.py, so that modules like get_url could validate certificates for website accepting only TLSv1_2 and no less.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

module_utils/urls.py

ANSIBLE VERSION
ansible 2.3.1.0
  config file =
  configured module search path = Default w/o overrides
  python version = 2.7.13 (default, Jan 19 2017, 14:48:08) [GCC 6.3.0 20170118]
ADDITIONAL INFORMATION
  • BEFORE
ansible -m get_url -a "url=https://releases.hashicorp.com dest=/tmp/hashicorp_index.html" localhost
 [WARNING]: Host file not found: /etc/ansible/hosts

 [WARNING]: provided hosts list is empty, only localhost is available

localhost | FAILED! => {
    "changed": false,
    "failed": true,
    "msg": "Failed to validate the SSL certificate for releases.hashicorp.com:443. Make sure your managed systems have a valid CA certificate installed. You can use validate_certs=False if you do not need to confirm the servers identity but this is unsafe and not recommended. Paths checked for this platform: /etc/ssl/certs, /etc/pki/ca-trust/extracted/pem, /etc/pki/tls/certs, /usr/share/ca-certificates/cacert.org, /etc/ansible. The exception msg was: (\"bad handshake: Error([('SSL routines', 'ssl3_read_bytes', 'tlsv1 alert protocol version')],)\",)."
}
  • AFTER
ansible -m get_url -a "url=https://releases.hashicorp.com dest=/tmp/hashicorp_index.html" localhost
 [WARNING]: Host file not found: /etc/ansible/hosts

 [WARNING]: provided hosts list is empty, only localhost is available

localhost | SUCCESS => {
    "changed": true,
    "checksum_dest": null,
    "checksum_src": "afdac380532e75798fa0afc46d06d3cd0a9e6b84",
    "dest": "/tmp/hashicorp_index.html",
    "gid": 0,
    "group": "root",
    "md5sum": "e978f533ad1eefc5110b6354760e12f5",
    "mode": "0644",
    "msg": "OK (7998 bytes)",
    "owner": "root",
    "size": 7998,
    "src": "/tmp/tmpRL_auy",
    "state": "file",
    "status_code": 200,
    "uid": 0,
    "url": "https://releases.hashicorp.com"
}

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bugfix_pull_request c:module_utils/ c:module_utils/urls needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jun 29, 2017
@nitzmahone nitzmahone removed the needs_triage Needs a first human triage before being processed. label Jun 29, 2017
@aurelienmaury
Copy link
Copy Markdown
Author

aurelienmaury commented Jul 1, 2017

Hi @nitzmahone sorry to bother, but I think there is a lack with this PR's labels: like stated in the description, it clearly affects 2.3, I have not even tested with 2.4.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 18, 2017
@andrewkroh andrewkroh mentioned this pull request Aug 28, 2017
Closed
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Oct 18, 2017
@alikins
Copy link
Copy Markdown
Contributor

alikins commented Oct 23, 2017

possibly related to #31998 ?

@jctanner
Copy link
Copy Markdown
Contributor

jctanner commented Oct 23, 2017

@aurelienmaury the bot uses the version of the "devel" branch at time of submission to flag the "affects_x.x" labels for PRs. Your PR would not have been merged into the 2.3 branch unless a backport had been requested or you submitted the PR against the 2.3 branch.

@sivel
Copy link
Copy Markdown
Member

sivel commented Oct 23, 2017

This will need more branches. Mac does not include ssl.PROTOCOL_TLSv1_2.

python -c 'import ssl; ssl.PROTOCOL_TLSv1_2'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
AttributeError: 'module' object has no attribute 'PROTOCOL_TLSv1_2'

So although it has HAS_SSLCONTEXT v1_2 doesn't exist.

@abadger
Copy link
Copy Markdown
Contributor

abadger commented Oct 23, 2017

@aurelienmaury We can't use this verbatim as ssl.PROTOCOL_TLSv1_2 wasn't available until python-2.7.9 (we need to support python-2.6.x in his code). If you have ideas on rectifying that, it would be great to hear. I'm spending a little time looking as well but I can't guarantee I'll come up with a solution.

abadger added a commit to abadger/ansible that referenced this pull request Oct 23, 2017
@abadger
Prefer the stdlib SSLContext over urllib3 context
34ac059 
We do not go through the effort of finding the right PROTOCOL setting if
we have SSLContext in the stdlib.  So we do not want to hit the code
that uses PROTOCOL to set the urllib3-provided ssl context when
SSLContext is available.  Also, the urllib3 implementation appears to
have a bug in some recent versions.  Preferring the stdlib version will
work around that for those with Python-2.7.9+ as well.

Fixes ansible#26235
Fixes ansible#25402
Fixes ansible#31998
@abadger abadger mentioned this pull request Oct 23, 2017
Merged
@abadger
Copy link
Copy Markdown
Contributor

abadger commented Oct 23, 2017

@aurelienmaury Okay, I think I've tracked down a better way to fix this. See the code in PR #32053 if you'd care to test out my fix.

ganeshrn pushed a commit to ganeshrn/ansible that referenced this pull request Oct 24, 2017
@abadger
Prefer the stdlib SSLContext over urllib3 context
725ae96 
We do not go through the effort of finding the right PROTOCOL setting if
we have SSLContext in the stdlib.  So we do not want to hit the code
that uses PROTOCOL to set the urllib3-provided ssl context when
SSLContext is available.  Also, the urllib3 implementation appears to
have a bug in some recent versions.  Preferring the stdlib version will
work around that for those with Python-2.7.9+ as well.

Fixes ansible#26235
Fixes ansible#25402
Fixes ansible#31998
abadger added a commit that referenced this pull request Oct 24, 2017
@abadger
Prefer the stdlib SSLContext over urllib3 context
f48af16 
We do not go through the effort of finding the right PROTOCOL setting if
we have SSLContext in the stdlib.  So we do not want to hit the code
that uses PROTOCOL to set the urllib3-provided ssl context when
SSLContext is available.  Also, the urllib3 implementation appears to
have a bug in some recent versions.  Preferring the stdlib version will
work around that for those with Python-2.7.9+ as well.

Fixes #26235
Fixes #25402
Fixes #31998

(cherry picked from commit 725ae96)
abadger added a commit that referenced this pull request Oct 24, 2017
@abadger
Prefer the stdlib SSLContext over urllib3 context
dc41e1c 
We do not go through the effort of finding the right PROTOCOL setting if
we have SSLContext in the stdlib.  So we do not want to hit the code
that uses PROTOCOL to set the urllib3-provided ssl context when
SSLContext is available.  Also, the urllib3 implementation appears to
have a bug in some recent versions.  Preferring the stdlib version will
work around that for those with Python-2.7.9+ as well.

Fixes #26235
Fixes #25402
Fixes #31998

(cherry picked from commit 725ae96)
abadger added a commit that referenced this pull request Nov 4, 2017
@abadger
Prefer the stdlib SSLContext over urllib3 context
0796190 
We do not go through the effort of finding the right PROTOCOL setting if
we have SSLContext in the stdlib.  So we do not want to hit the code
that uses PROTOCOL to set the urllib3-provided ssl context when
SSLContext is available.  Also, the urllib3 implementation appears to
have a bug in some recent versions.  Preferring the stdlib version will
work around that for those with Python-2.7.9+ as well.

Fixes #26235
Fixes #25402
Fixes #31998

(cherry picked from commit 725ae96)
abadger added a commit that referenced this pull request Feb 23, 2018
@abadger
Prefer the stdlib SSLContext over urllib3 context
6020c0d 
We do not go through the effort of finding the right PROTOCOL setting if
we have SSLContext in the stdlib.  So we do not want to hit the code
that uses PROTOCOL to set the urllib3-provided ssl context when
SSLContext is available.  Also, the urllib3 implementation appears to
have a bug in some recent versions.  Preferring the stdlib version will
work around that for those with Python-2.7.9+ as well.

Fixes #26235
Fixes #25402
Fixes #31998

(cherry picked from commit 725ae96)
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. c:module_utils/urls c:module_utils/ module_utils/urls new_contributor This PR is the first contribution by a new community member. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@aurelienmaury @alikins @jctanner @sivel @abadger @ansibot @nitzmahone