cert validation fixes - Attempt 2 (#55953)

* Attempt 2 of cert validation fixes

* Remove unused code

* Cleanup the tmp cert using atexit

* Fix linting issues

* Only add SSLValidationHandler when not HAS_SSLCONTEXT

* Catch value errors on non PEM certs

* Only catch NotImplementedError to avoid masking issues

* set self._context even with PyOpenSSLContext for conformity

* Fix error building

* normalize how we interact with the context we create

* Remove unused code

* Address test for py3.7 message difference

* open_url should pass the ca_path through

* Account for new error in url lookup test

* Guard some code behind whether or not we are validating certs

* Make _make_context public

* Move atexit.register up to where the tmp file is created

test/units/module_utils/urls/test_RedirectHandlerFactory.py

@@ -6,7 +6,7 @@ from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
 
-from ansible.module_utils.urls import RedirectHandlerFactory, urllib_request, urllib_error
+from ansible.module_utils.urls import HAS_SSLCONTEXT, RedirectHandlerFactory, urllib_request, urllib_error
 from ansible.module_utils.six import StringIO
 
 import pytest
@@ -127,7 +127,7 @@ def test_redir_validate_certs(urllib_req, request_body, mocker):
     inst = handler()
     inst.redirect_request(urllib_req, request_body, 301, '301 Moved Permanently', {}, 'https://docs.ansible.com/')
 
-    assert opener_mock.add_handler.call_count == 1
+    assert opener_mock.add_handler.call_count == int(not HAS_SSLCONTEXT)
 
 
 def test_redir_http_error_308_urllib2(urllib_req, request_body):

test/units/module_utils/urls/test_Request.py

@@ -47,6 +47,7 @@ def test_Request_fallback(urlopen_mock, install_opener_mock, mocker):
         client_key='/tmp/client.key',
         cookies=cookies,
         unix_socket='/foo/bar/baz.sock',
+        ca_path='/foo/bar/baz.pem',
     )
     fallback_mock = mocker.spy(request, '_fallback')
 
@@ -66,10 +67,11 @@ def test_Request_fallback(urlopen_mock, install_opener_mock, mocker):
         call(None, '/tmp/client.key'),  # client_key
         call(None, cookies),  # cookies
         call(None, '/foo/bar/baz.sock'),  # unix_socket
+        call(None, '/foo/bar/baz.pem'),  # ca_path
     ]
     fallback_mock.assert_has_calls(calls)
 
-    assert fallback_mock.call_count == 13  # All but headers use fallback
+    assert fallback_mock.call_count == 14  # All but headers use fallback
 
     args = urlopen_mock.call_args[0]
     assert args[1] is None  # data, this is handled in the Request not urlopen
@@ -100,17 +102,22 @@ def test_Request_open(urlopen_mock, install_opener_mock):
     opener = install_opener_mock.call_args[0][0]
     handlers = opener.handlers
 
-    expected_handlers = (
-        SSLValidationHandler,
-        RedirectHandlerFactory(),  # factory, get handler
-    )
+    if not HAS_SSLCONTEXT:
+        expected_handlers = (
+            SSLValidationHandler,
+            RedirectHandlerFactory(),  # factory, get handler
+        )
+    else:
+        expected_handlers = (
+            RedirectHandlerFactory(),  # factory, get handler
+        )
 
     found_handlers = []
     for handler in handlers:
         if isinstance(handler, SSLValidationHandler) or handler.__class__.__name__ == 'RedirectHandler':
             found_handlers.append(handler)
 
-    assert len(found_handlers) == 2
+    assert len(found_handlers) == len(expected_handlers)
 
 
 def test_Request_open_http(urlopen_mock, install_opener_mock):
@@ -446,4 +453,4 @@ def test_open_url(urlopen_mock, install_opener_mock, mocker):
                                      url_username=None, url_password=None, http_agent=None,
                                      force_basic_auth=False, follow_redirects='urllib2',
                                      client_cert=None, client_key=None, cookies=None, use_gssapi=False,
-                                     unix_socket=None)
+                                     unix_socket=None, ca_path=None)

test/units/module_utils/urls/test_fetch_url.py

@@ -67,7 +67,7 @@ def test_fetch_url(open_url_mock, fake_ansible_module):
     open_url_mock.assert_called_once_with('http://ansible.com/', client_cert=None, client_key=None, cookies=kwargs['cookies'], data=None,
                                           follow_redirects='urllib2', force=False, force_basic_auth='', headers=None,
                                           http_agent='ansible-httpget', last_mod_time=None, method=None, timeout=10, url_password='', url_username='',
-                                          use_proxy=True, validate_certs=True, use_gssapi=False, unix_socket=None)
+                                          use_proxy=True, validate_certs=True, use_gssapi=False, unix_socket=None, ca_path=None)
 
 
 def test_fetch_url_params(open_url_mock, fake_ansible_module):
@@ -89,7 +89,7 @@ def test_fetch_url_params(open_url_mock, fake_ansible_module):
     open_url_mock.assert_called_once_with('http://ansible.com/', client_cert='client.pem', client_key='client.key', cookies=kwargs['cookies'], data=None,
                                           follow_redirects='all', force=False, force_basic_auth=True, headers=None,
                                           http_agent='ansible-test', last_mod_time=None, method=None, timeout=10, url_password='passwd', url_username='user',
-                                          use_proxy=True, validate_certs=False, use_gssapi=False, unix_socket=None)
+                                          use_proxy=True, validate_certs=False, use_gssapi=False, unix_socket=None, ca_path=None)
 
 
 def test_fetch_url_cookies(mocker, fake_ansible_module):