Fixes #280: Migrated HTTP communication to use requests

[andy-maier]

Ready for review and manual tests (see below for hints).

In addition to issue #280, it also addresses these issues:

• Return to a single SSL environment for both py2 and py3 (issue Get back to a single SSL environment across Python 2 and 3 #496)
• Use of ssl.create_default_context() to auto-adjust to better security (issue Use of ssl.create_default_context() to auto-adjust to better security #893)
• Hard coded default directories for CA certificates (issue Hard coded default directories for CA certificates #889)
• cim_operations appears to have no disconnect (issue ResourceWarnings in py3 during test #86)

Note: If you want to test this PR with pywbemtools, it requires using PR pywbem/pywbemtools#467.

• Particular review points:

  • Changes to ca_cert parameter of WBEMConnection
  • Removal of communication via UNIX sockets
  • Removal of local authentication schemes
  • The new pywbem.PYWBEM_USES_REQUESTS attribute for indicating requests support to users

• Manual tests against real WBEM servers, including error behavior:

  • Different client envs:
    • Python 2 on Linux
    • Python 3 on Linux
    • Python 2 on native Windows
    • Python 3 on native Windows
  • tests for ca_certs:
    • No server certificate verification
    • Verification of self-signed server certificate with specified path to PEM file
    • Verification of self-signed server certificate with specified path to directory
  • tests for 2-way auth (x509 parm):
    • client certificate in PEM file, client private key in PEM file
    • client certificate and client private key in single PEM file (specified as 'cert_file')

• Tests that have been moved to new issue Tests with CA-chain server certificates #2038:

  • Verification of CA-chain server certificate with specified path to PEM file
  • Verification of CA-chain server certificate with specified path to directory
  • Verification of CA-chain server certificate with certifi-provided root certificates

[coveralls]

Coverage increased (+1.0%) to 86.866% when pulling 67a76e8 on andy/use-requests into 6800b15 on master.

[KSchopmeyer]

We were not passing the scheme or port on to requests Since we are no longer calling parse_url for each operation which properly sets up the port with defaults if not supplied. I did a temp fix on about line 246 of cim_http.py

if cimxml_headers is None:
    cimxml_headers = []

target = '/cimom'
host, port, ssl = parse_url(conn.url)               <----------------brokout host, port ssl
scheme = 'https' if ssl else 'http'
target_url = '{}://{}:{}{}'.format(scheme, host, port, target)   <---------------reassembled with host and p

# Make sure the data parameter is converted to a UTF-8 encoded byte string.
# This is important because according to RFC2616, the Content-Length HTTP
# header must be measured in Bytes (and the Content-Type header will
# indicate UTF-8).
req_body = _ensure_bytes(req_data)

But that is not the correct place to be doing this since it should be done once per connection. I noted that"

a. in old code the http lib required host and port separate. So parse_url separted them for every wbem_connection call.

b. We were also separating them for some of the pull operations using parse_url in cim_operations.py which means that this code was called for every pull.

c. We were also separating scheme, etc. in individual properties within cim_operations.py (see the scheme property.

Why not do this once per connection during construction and maintain the scheme, host parameters to be used where we are using parse_url in cim_operations.

Also, since it is no longer required to separate host and port for the requests call should we redefine the cim_operations url to include the port and use parse_url once on each connection in WBEMConnection init to sort out the port including the default port.

In fact, should we redo parse_url and get it out of cim_http.py????

NOTE: This is not a complete review since I am working primarily just to get through tests now.

Update by Andy: I have changed the approach for normalizing and defaulting the URL components in PR #2048. This should address these comments.

[KSchopmeyer]

Reviewed the whole set. See comments below. Since I have not gotten through all tests, This is still incomplete but removed the review needed to flag you

[KSchopmeyer]

NOTE: I also found the following code to log actual requests at the HTTPConnection level and inserted it for these tests with conn.debug around it. (Not the whole context but the debug_requests_on. Just a question but would it be logical to get it into our code with the debug, or some other flag.

def debug_requests_on():
'''Switches on logging of the requests module.'''
HTTPConnection.debuglevel = 1

logging.basicConfig()
logging.getLogger().setLevel(logging.DEBUG)
requests_log = logging.getLogger("requests.packages.urllib3")
requests_log.setLevel(logging.DEBUG)
requests_log.propagate = True

def debug_requests_off():
'''Switches off logging of the requests module, might be some side-effects'''
HTTPConnection.debuglevel = 0

root_logger = logging.getLogger()
root_logger.setLevel(logging.WARNING)
root_logger.handlers = []
requests_log = logging.getLogger("requests.packages.urllib3")
requests_log.setLevel(logging.WARNING)
requests_log.propagate = False

@contextlib.contextmanager
def debug_requests():
'''Use with 'with'!'''
debug_requests_on()
yield
debug_requests_off()

[KSchopmeyer]

Finally for Christmas day. I also ran with Python 2 locally and passed the tests with these fixes. The remaining test is with windows.

[KSchopmeyer]

We are getting OpenSSL information on errors with requests that are http based and should not involve SSL.

(pywbem3) C:\Users\kscho\dev\pywbem\examples>enuminstances.py
Usage: C:\Users\kscho\dev\pywbem\examples\enuminstances.py server_url username password namespace' ' classname
Using internal defaults
Requesting url=http://localhost, ns=root/cimv2, class=CIM_ComputerSystem
TARGET_URL http://localhost:5988/cimom
Operation failed: Failed to establish a new connection: [WinError 10061] No connection could be made because the target machine actively refused it; OpenSSL version used: OpenSSL 1.0.2o 27 Mar 2018

Update by Andy: The "OpenSSL version used ..." text is inserted by pywbem. This could be improved by only inserting it for scheme='https'. Will do that in a separate PR #2049.

[KSchopmeyer]

Testing status: Wed 26 Dec. With temp patches per earlier comments

platform	http	https/no-verify	https/ca-certfile	https/ca-certdir	mutual
python 3	OK	OK	OK	OK	OK
python 2	OK	OK	OK	OK	OK
windows	OK	OK	OK

TODO:

1. ca cert by directory in windows
2. mutual auth, windows. Need to install tools for c_rehash for windows
   3 try to test certifi
3. Test errors like bad certs

[andy-maier]

The latest update to this PR along with PR #2048 and the to be created one for the ssl messages for http connections should address all comments by Karl so far. Setting to review needed again, to get the changes verified.

[KSchopmeyer]

Testing done using url-normalize branch