Skip to content

Commit

Permalink
fix pvmomi the proxy support (#799)
Browse files Browse the repository at this point in the history
- ensure the proxy configuration is passed to `__GetElementTree()`
- reuse @blacksponge fix for the  `SSLTunnelConnection` class
- add `test_ssl_tunnelwith_cert_file` test to validate that `cert_file`
  and `key_file` are passed as expected.

The patch adds a new `HTTPProxyConnection` class to handle the `CONNECT`
based proxies.
The patch does not touch the `SSLTunnelConnection` class. Both are
really difference, and I think it's a better approach.

To summarize, if the user set `sslProxyPath`, we use the original method,
if instead `httpProxyHost` is set, we use the new class. Both should not
be used at the same time.

closes: #620
closes: #567
closes: #198
  • Loading branch information
goneri authored and pgbidkar committed Jul 23, 2019
1 parent 724a14c commit 8c062b3
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 39 deletions.
27 changes: 19 additions & 8 deletions pyVim/connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ def __exit__(self, *exc_info):
Disconnect(self.si)
self.si = None

def __GetElementTree(protocol, server, port, path, sslContext):
def __GetElementTree(protocol, server, port, path, sslContext, httpProxyHost=None, httpProxyPort=None):
"""
Private method that returns a root from ElementTree for a remote XML document.
Expand All @@ -594,7 +594,11 @@ def __GetElementTree(protocol, server, port, path, sslContext):
@type sslContext: SSL.Context
"""

if protocol == "https":
if httpProxyHost:
kwargs = {"context": sslContext} if sslContext else {}
conn = http_client.HTTPSConnection(httpProxyHost, port=httpProxyPort, **kwargs)
conn.set_tunnel(server, port)
elif protocol == "https":
kwargs = {"context": sslContext} if sslContext else {}
conn = http_client.HTTPSConnection(server, port=port, **kwargs)
elif protocol == "http":
Expand All @@ -615,7 +619,7 @@ def __GetElementTree(protocol, server, port, path, sslContext):
## supported by the specified server. The result will be vimServiceVersions.xml
## if it exists, otherwise vimService.wsdl if it exists, otherwise None.

def __GetServiceVersionDescription(protocol, server, port, path, sslContext):
def __GetServiceVersionDescription(protocol, server, port, path, sslContext, httpProxyHost=None, httpProxyPort=None):
"""
Private method that returns a root from an ElementTree describing the API versions
supported by the specified server. The result will be vimServiceVersions.xml
Expand All @@ -635,12 +639,14 @@ def __GetServiceVersionDescription(protocol, server, port, path, sslContext):
"""

tree = __GetElementTree(protocol, server, port,
path + "/vimServiceVersions.xml", sslContext)
path + "/vimServiceVersions.xml", sslContext,
httpProxyHost, httpProxyPort)
if tree is not None:
return tree

tree = __GetElementTree(protocol, server, port,
path + "/vimService.wsdl", sslContext)
path + "/vimService.wsdl", sslContext,
httpProxyHost, httpProxyPort)
return tree


Expand Down Expand Up @@ -689,7 +695,7 @@ def __VersionIsSupported(desiredVersion, serviceVersionDescription):
## Private method that returns the most preferred API version supported by the
## specified server,

def __FindSupportedVersion(protocol, server, port, path, preferredApiVersions, sslContext):
def __FindSupportedVersion(protocol, server, port, path, preferredApiVersions, sslContext, httpProxyHost=None, httpProxyPort=None):
"""
Private method that returns the most preferred API version supported by the
specified server,
Expand All @@ -715,7 +721,9 @@ def __FindSupportedVersion(protocol, server, port, path, preferredApiVersions, s
server,
port,
path,
sslContext)
sslContext,
httpProxyHost,
httpProxyPort)
if serviceVersionDescription is None:
return None

Expand Down Expand Up @@ -759,7 +767,10 @@ def SmartStubAdapter(host='localhost', port=443, path='/sdk',
port,
path,
preferredApiVersions,
sslContext)
sslContext,
httpProxyHost,
httpProxyPort
)
if supportedVersion is None:
raise Exception("%s:%s is not a VIM server" % (host, port))

Expand Down
51 changes: 34 additions & 17 deletions pyVmomi/SoapAdapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@
OS_VERSION = platform.uname()[2]
OS_ARCH = platform.uname()[4]

SOAP_ADAPTER_ARGS = [
"server_side", "cert_reqs", "ssl_version", "ca_certs", "do_handshake_on_connect",
"suppress_ragged_eofs", "ciphers"]


## Thumbprint mismatch exception
#
class ThumbprintMismatchException(Exception):
Expand Down Expand Up @@ -1016,9 +1021,7 @@ def __init__(self, *args, **kwargs):
# and push back the params in connect()
self._sslArgs = {}
tmpKwargs = kwargs.copy()
for key in ["server_side", "cert_reqs", "ssl_version", "ca_certs",
"do_handshake_on_connect", "suppress_ragged_eofs",
"ciphers"]:
for key in SOAP_ADAPTER_ARGS:
if key in tmpKwargs:
self._sslArgs[key] = tmpKwargs.pop(key)
http_client.HTTPSConnection.__init__(self, *args, **tmpKwargs)
Expand Down Expand Up @@ -1061,8 +1064,10 @@ def connect(self):
# dercert = self.sock.getpeercert(False)
# # pemcert = ssl.DER_cert_to_PEM_cert(dercert)

## Stand-in for the HTTPSConnection class that will connect to a proxy and
## issue a CONNECT command to start an SSL tunnel.

## Stand-in for the HTTPSConnection class that will connect to a SSL proxy,
## VCenter's /sdkTunnel endpoint. It will issue a CONNECT command to start
## an SSL tunnel.
class SSLTunnelConnection(object):
# @param proxyPath The path to pass to the CONNECT command.
def __init__(self, proxyPath):
Expand Down Expand Up @@ -1094,6 +1099,25 @@ def __call__(self, path, key_file=None, cert_file=None, **kwargs):
return retval


## Stand-in for the HTTPSConnection class that will connect to a regular HTTP
## proxy.
class HTTPProxyConnection(object):
# @param proxyPath The path to pass to the CONNECT command.
def __init__(self, proxyPath):
self.proxyPath = proxyPath

# Connects to a HTTP proxy server and initiates a tunnel to the destination
# specified by proxyPath. If successful, a new HTTPSConnection is returned.
#
# @param path The destination URL path.
# @param args Arguments are ignored
# @param kwargs Arguments for HTTPSConnection
def __call__(self, path, *args, **kwargs):
httpsConnArgs = {k: kwargs[k] for k in kwargs if k not in SOAP_ADAPTER_ARGS}
conn = http_client.HTTPSConnection(path, **httpsConnArgs)
conn.set_tunnel(self.proxyPath)
return conn

class GzipReader:
GZIP = 1
DEFLATE = 2
Expand Down Expand Up @@ -1237,20 +1261,13 @@ def __init__(self, host='localhost', port=443, ns=None, path='/sdk',
else:
self.thumbprint = None

self.is_ssl_tunnel = False
self.is_tunnel = False
if sslProxyPath:
self.scheme = SSLTunnelConnection(sslProxyPath)
self.is_ssl_tunnel = True
self.is_tunnel = True
elif httpProxyHost:
if self.scheme == _HTTPSConnection:
self.scheme = SSLTunnelConnection(self.host)
self.is_ssl_tunnel = True
else:
if url:
self.path = url
else:
self.path = "https://{0}/{1}".format(self.host, path)
# Swap the actual host with the proxy.
self.scheme = HTTPProxyConnection(self.host)
self.is_tunnel = True
self.host = "{0}:{1}".format(httpProxyHost, httpProxyPort)
self.poolSize = poolSize
self.pool = []
Expand Down Expand Up @@ -1439,7 +1456,7 @@ def ReturnConnection(self, conn):
self.lock.acquire()
self._CloseIdleConnections()
# In case of ssl tunneling, only add the conn if the conn has not been closed
if len(self.pool) < self.poolSize and (not self.is_ssl_tunnel or conn.sock):
if len(self.pool) < self.poolSize and (not self.is_tunnel or conn.sock):
self.pool.insert(0, (conn, time.time()))
self.lock.release()
else:
Expand Down
1 change: 1 addition & 0 deletions test-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
tox
testtools>=0.9.34
vcrpy<2
mock
32 changes: 32 additions & 0 deletions tests/fixtures/http_proxy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
interactions:
- request:
body: null
headers: {}
method: GET
uri: https://my-http-proxy:8080/
response:
body:
string: "bla"
headers:
Connection:
- Keep-Alive
Content-Length:
- '3'
Content-Security-Policy:
- block-all-mixed-content
Content-Type:
- text/html
Date:
- Wed, 26 Jun 2019 20:49:55 GMT
Strict-Transport-Security:
- max-age=31536000
X-Content-Type-Options:
- nosniff
X-Frame-Options:
- DENY
X-XSS-Protection:
- '1'
status:
code: 200
message: OK
version: 1
33 changes: 25 additions & 8 deletions tests/fixtures/ssl_tunnel_http_failure.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,31 @@ interactions:
- request:
body: null
headers: {}
method: CONNECT
uri: http:https://vcsavcsa:80
method: GET
uri: https:https://vcsa:80/
response:
body: {string: !!python/unicode '<HTML><BODY><H1>404 Not Found</H1></BODY></HTML>'}
body:
string: ""
headers:
connection: [close]
content-length: ['48']
content-type: [text/html]
date: ['Fri, 9 Oct 2015 17:38:33 GMT']
status: {code: 404, message: Not Found}
Connection:
- Keep-Alive
Content-Length:
- '0'
Content-Security-Policy:
- block-all-mixed-content
Content-Type:
- text/html
Date:
- Wed, 26 Jun 2019 20:19:39 GMT
Strict-Transport-Security:
- max-age=31536000
X-Content-Type-Options:
- nosniff
X-Frame-Options:
- DENY
X-XSS-Protection:
- '1'
status:
code: 404
message: KO
version: 1
45 changes: 39 additions & 6 deletions tests/test_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,16 @@

import tests
import unittest
import sys

from pyVim import connect
from pyVmomi import vim

if sys.version_info >= (3, 3):
from unittest.mock import patch, MagicMock
else:
from mock import patch, MagicMock


class ConnectionTests(tests.VCRTestBase):

Expand Down Expand Up @@ -89,14 +95,41 @@ def test_disconnect_on_no_connection(self):
def test_ssl_tunnel(self):
connect.SoapStubAdapter('sdkTunnel', 8089, httpProxyHost='vcsa').GetConnection()

@tests.VCRTestBase.my_vcr.use_cassette('ssl_tunnel_http_failure.yaml',
cassette_library_dir=tests.fixtures_path,
record_mode='none')
def test_ssl_tunnel_http_failure(self):
from six.moves import http_client
import socket
def should_fail():
connect.SoapStubAdapter('vcsa', 80, httpProxyHost='vcsa').GetConnection()
self.assertRaises(http_client.HTTPException, should_fail)
conn = connect.SoapStubAdapter('vcsa', 80, httpProxyHost='unreachable').GetConnection()
conn.request('GET', '/')
conn.getresponse()
self.assertRaises((OSError, socket.gaierror), should_fail)

@tests.VCRTestBase.my_vcr.use_cassette('ssl_tunnel.yaml',
cassette_library_dir=tests.fixtures_path,
record_mode='none')
def test_http_proxy(self):
connect.SoapStubAdapter('sdkTunnel', 8089, httpProxyHost='vcsa').GetConnection()

@patch('six.moves.http_client.HTTPSConnection')
def test_http_proxy_with_cert_file(self, hs):
conn = connect.SoapStubAdapter(
'sdkTunnel', 8089, httpProxyHost='vcsa',
certKeyFile='my_key_file', certFile='my_cert_file').GetConnection()
conn.request('GET', '/')
hs.assert_called_once_with('vcsa:80', cert_file='my_cert_file', key_file='my_key_file')
conn.set_tunnel.assert_called_once_with('sdkTunnel:8089')

@tests.VCRTestBase.my_vcr.use_cassette('http_proxy.yaml',
cassette_library_dir=tests.fixtures_path,
record_mode='once')
def test_http_proxy(self):
conn = connect.SoapStubAdapter(
'vcenter.test', httpProxyHost='my-http-proxy',
httpProxyPort=8080).GetConnection()
self.assertEqual(conn._tunnel_host, 'vcenter.test')
self.assertEqual(conn._tunnel_port, 443)
conn.request('GET', '/')
conn.getresponse()


if __name__ == '__main__':
unittest.main()

0 comments on commit 8c062b3

Please sign in to comment.