Merge pull request #263 from badcure/ca_trust_change

Add deprecated warnings for CA Trusts defined by Env Var

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### Version 0.4.0
 - Ensure `server_cert_validation=ignore` supersedes ca_trust_path/env overrides
+- Added deprecated warnings if CA trusts defined by environment variables are used.
 - Set minimum version of requests-credssp to support Kerberos auth over CredSSP and other changes
 - Added `proxy` support where it can be defined within the application, with the ability to specify the proxy within the application
 - Fix for shell not setting all environment variables.

winrm/protocol.py

@@ -31,7 +31,7 @@ class Protocol(object):
     def __init__(
             self, endpoint, transport='plaintext', username=None,
             password=None, realm=None, service="HTTP", keytab=None,
-            ca_trust_path=None, cert_pem=None, cert_key_pem=None,
+            ca_trust_path='legacy_requests', cert_pem=None, cert_key_pem=None,
             server_cert_validation='validate',
             kerberos_delegation=False,
             read_timeout_sec=DEFAULT_READ_TIMEOUT_SEC,
@@ -50,7 +50,10 @@ def __init__(
         @param string realm: unused
         @param string service: the service name, default is HTTP
         @param string keytab: the path to a keytab file if you are using one
-        @param string ca_trust_path: Certification Authority trust path
+        @param string ca_trust_path: Certification Authority trust path. If server_cert_validation is set to 'validate':
+                                        'legacy_requests'(default) to use environment variables,
+                                        None to explicitly disallow any additional CA trust path
+                                        Any other value will be considered the CA trust path to use.
         @param string cert_pem: client authentication certificate file path in PEM format  # NOQA
         @param string cert_key_pem: client authentication certificate key file path in PEM format  # NOQA
         @param string server_cert_validation: whether server certificate should be validated on Python versions that suppport it; one of 'validate' (default), 'ignore' #NOQA

winrm/tests/test_transport.py

@@ -20,6 +20,7 @@ def setUp(self):
         os.environ.pop('HTTP_PROXY', None)
         os.environ.pop('NO_PROXY', None)
         transport.DISPLAYED_PROXY_WARNING = False
+        transport.DISPLAYED_CA_TRUST_WARNING = False
 
     def tearDown(self):
         super(TestTransport, self).tearDown()
@@ -30,6 +31,26 @@ def tearDown(self):
         os.environ.pop('HTTP_PROXY', None)
         os.environ.pop('NO_PROXY', None)
 
+    def test_build_session_cert_validate_default(self):
+        t_default = transport.Transport(endpoint="https://example.com",
+                                        username='test',
+                                        password='test',
+                                        auth_method='basic',
+                                        )
+        t_default.build_session()
+        self.assertEqual(True, t_default.session.verify)
+
+    def test_build_session_cert_validate_default_env(self):
+        os.environ['REQUESTS_CA_BUNDLE'] = 'path_to_REQUESTS_CA_CERT'
+
+        t_default = transport.Transport(endpoint="https://example.com",
+                                        username='test',
+                                        password='test',
+                                        auth_method='basic',
+                                        )
+        t_default.build_session()
+        self.assertEqual('path_to_REQUESTS_CA_CERT', t_default.session.verify)
+
     def test_build_session_cert_validate_1(self):
         os.environ['REQUESTS_CA_BUNDLE'] = 'path_to_REQUESTS_CA_CERT'
 
@@ -80,6 +101,19 @@ def test_build_session_cert_override_2(self):
         t_default.build_session()
         self.assertEqual('overridepath', t_default.session.verify)
 
+    def test_build_session_cert_override_3(self):
+        os.environ['CURL_CA_BUNDLE'] = 'path_to_CURL_CA_CERT'
+
+        t_default = transport.Transport(endpoint="https://example.com",
+                                        server_cert_validation='validate',
+                                        username='test',
+                                        password='test',
+                                        auth_method='basic',
+                                        ca_trust_path=None,
+                                        )
+        t_default.build_session()
+        self.assertEqual(True, t_default.session.verify)
+
     def test_build_session_cert_ignore_1(self):
         os.environ['REQUESTS_CA_BUNDLE'] = 'path_to_REQUESTS_CA_CERT'
         os.environ['CURL_CA_BUNDLE'] = 'path_to_CURL_CA_CERT'

winrm/transport.py

@@ -12,6 +12,7 @@
 
 is_py2 = sys.version[0] == '2'
 DISPLAYED_PROXY_WARNING = False
+DISPLAYED_CA_TRUST_WARNING = False
 
 if is_py2:
     # use six for this instead?
@@ -54,7 +55,7 @@ class UnsupportedAuthArgument(Warning):
 class Transport(object):
     def __init__(
             self, endpoint, username=None, password=None, realm=None,
-            service=None, keytab=None, ca_trust_path=None, cert_pem=None,
+            service=None, keytab=None, ca_trust_path='legacy_requests', cert_pem=None,
             cert_key_pem=None, read_timeout_sec=None, server_cert_validation='validate',
             kerberos_delegation=False,
             kerberos_hostname_override=None,
@@ -173,11 +174,11 @@ def build_session(self):
         if not DISPLAYED_PROXY_WARNING and self.proxy == 'legacy_requests' and (
                 'http' in settings['proxies'] or 'https' in settings['proxies']):
             message = "'pywinrm' will use an environment defined proxy. This feature will be disabled in " \
-                      "the future, please provide it explicitly."
+                      "the future, please specify it explicitly."
             if 'http' in settings['proxies']:
-                message += " HTTP proxy {proxy} discover.".format(proxy=settings['proxies']['http'])
+                message += " HTTP proxy {proxy} discovered.".format(proxy=settings['proxies']['http'])
             if 'https' in settings['proxies']:
-                message += " HTTPS proxy {proxy} discover.".format(proxy=settings['proxies']['https'])
+                message += " HTTPS proxy {proxy} discovered.".format(proxy=settings['proxies']['https'])
 
             DISPLAYED_PROXY_WARNING = True
             warnings.warn(message, DeprecationWarning)
@@ -188,9 +189,29 @@ def build_session(self):
         session.verify = self.server_cert_validation == 'validate'
 
         # patch in CA path override if one was specified in init or env
-        if session.verify and (self.ca_trust_path is not None or settings['verify'] is not None):
-            # session.verify can be either a bool or path to a CA store; prefer passed-in value over env if both are present
-            session.verify = self.ca_trust_path or settings['verify']
+        if session.verify:
+            if self.ca_trust_path == 'legacy_requests' and settings['verify'] is not None:
+                # We will
+                session.verify = settings['verify']
+
+                global DISPLAYED_CA_TRUST_WARNING
+
+                # We want to eventually stop reading proxy information from the environment.
+                # Also only display the warning once. This method can be called many times during an application's runtime.
+                if not DISPLAYED_CA_TRUST_WARNING and session.verify is not True:
+                    message = "'pywinrm' will use an environment variable defined CA Trust. This feature will be disabled in " \
+                              "the future, please specify it explicitly."
+                    if os.environ.get('REQUESTS_CA_BUNDLE') is not None:
+                        message += " REQUESTS_CA_BUNDLE contains {ca_path}".format(ca_path=os.environ.get('REQUESTS_CA_BUNDLE'))
+                    elif os.environ.get('CURL_CA_BUNDLE') is not None:
+                        message += " CURL_CA_BUNDLE contains {ca_path}".format(ca_path=os.environ.get('CURL_CA_BUNDLE'))
+
+                    DISPLAYED_CA_TRUST_WARNING = True
+                    warnings.warn(message, DeprecationWarning)
+
+            elif session.verify and self.ca_trust_path is not None:
+                # session.verify can be either a bool or path to a CA store; prefer passed-in value over env if both are present
+                session.verify = self.ca_trust_path
 
         encryption_available = False