Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Allow providing credentials to HTTPS_PROXY #9657

Merged
merged 7 commits into from
Mar 22, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 34 additions & 7 deletions synapse/http/connectproxyclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from twisted.internet.interfaces import IStreamClientEndpoint
from twisted.internet.protocol import connectionDone
from twisted.web import http
from twisted.web.http_headers import Headers

logger = logging.getLogger(__name__)

Expand All @@ -47,19 +48,23 @@ class HTTPConnectProxyEndpoint:
proxy
host (bytes): hostname that we want to CONNECT to
port (int): port that we want to connect to
headers: Extra HTTP headers to include in the CONNECT request
"""

def __init__(self, reactor, proxy_endpoint, host, port):
def __init__(self, reactor, proxy_endpoint, host, port, headers):
self._reactor = reactor
self._proxy_endpoint = proxy_endpoint
self._host = host
self._port = port
self._headers = headers

def __repr__(self):
return "<HTTPConnectProxyEndpoint %s>" % (self._proxy_endpoint,)

def connect(self, protocolFactory):
f = HTTPProxiedClientFactory(self._host, self._port, protocolFactory)
f = HTTPProxiedClientFactory(
self._host, self._port, protocolFactory, self._headers
)
d = self._proxy_endpoint.connect(f)
# once the tcp socket connects successfully, we need to wait for the
# CONNECT to complete.
Expand All @@ -77,12 +82,14 @@ class HTTPProxiedClientFactory(protocol.ClientFactory):
dst_host (bytes): hostname that we want to CONNECT to
dst_port (int): port that we want to connect to
wrapped_factory (protocol.ClientFactory): The original Factory
headers: Extra HTTP headers to include in the CONNECT request
"""

def __init__(self, dst_host, dst_port, wrapped_factory):
def __init__(self, dst_host, dst_port, wrapped_factory, headers):
self.dst_host = dst_host
self.dst_port = dst_port
self.wrapped_factory = wrapped_factory
self.headers = headers
self.on_connection = defer.Deferred()

def startedConnecting(self, connector):
Expand All @@ -92,7 +99,11 @@ def buildProtocol(self, addr):
wrapped_protocol = self.wrapped_factory.buildProtocol(addr)

return HTTPConnectProtocol(
self.dst_host, self.dst_port, wrapped_protocol, self.on_connection
self.dst_host,
self.dst_port,
wrapped_protocol,
self.on_connection,
self.headers,
)

def clientConnectionFailed(self, connector, reason):
Expand Down Expand Up @@ -122,14 +133,22 @@ class HTTPConnectProtocol(protocol.Protocol):

connected_deferred (Deferred): a Deferred which will be callbacked with
wrapped_protocol when the CONNECT completes

headers: Extra HTTP headers to include in the CONNECT request
"""

def __init__(self, host, port, wrapped_protocol, connected_deferred):
def __init__(
self, host, port, wrapped_protocol, connected_deferred, headers: Headers
):
self.host = host
self.port = port
self.wrapped_protocol = wrapped_protocol
self.connected_deferred = connected_deferred
self.http_setup_client = HTTPConnectSetupClient(self.host, self.port)
self.headers = headers

self.http_setup_client = HTTPConnectSetupClient(
self.host, self.port, self.headers
)
self.http_setup_client.on_connected.addCallback(self.proxyConnected)

def connectionMade(self):
Expand Down Expand Up @@ -170,16 +189,24 @@ class HTTPConnectSetupClient(http.HTTPClient):
Args:
host (bytes): The hostname to send in the CONNECT message
port (int): The port to send in the CONNECT message
headers: Extra headers to send with the CONNECT message
"""

def __init__(self, host, port):
def __init__(self, host, port, headers: Headers):
self.host = host
self.port = port
self.headers = headers
self.on_connected = defer.Deferred()

def connectionMade(self):
logger.debug("Connected to proxy, sending CONNECT")
self.sendCommand(b"CONNECT", b"%s:%d" % (self.host, self.port))

# Send any additional specified headers
for name, values in self.headers.getAllRawHeaders():
for value in values:
self.sendHeader(name, value)

self.endHeaders()

def handleStatus(self, version, status, message):
Expand Down
73 changes: 67 additions & 6 deletions synapse/http/proxyagent.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,21 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import base64
import logging
import re
from typing import Optional, Tuple
from urllib.request import getproxies_environment, proxy_bypass_environment

import attr
from zope.interface import implementer

from twisted.internet import defer
from twisted.internet.endpoints import HostnameEndpoint, wrapClientTLS
from twisted.python.failure import Failure
from twisted.web.client import URI, BrowserLikePolicyForHTTPS, _AgentBase
from twisted.web.error import SchemeNotSupported
from twisted.web.http_headers import Headers
from twisted.web.iweb import IAgent

from synapse.http.connectproxyclient import HTTPConnectProxyEndpoint
Expand All @@ -32,6 +36,22 @@
_VALID_URI = re.compile(br"\A[\x21-\x7e]+\Z")


@attr.s
class ProxyCredentials:
username_password = attr.ib(type=bytes)

def as_proxy_authorization_value(self) -> bytes:
"""
Return the value for a Proxy-Authorization header (i.e. 'Basic abdef==').

Returns:
A transformation of the authentication string the encoded value for
a Proxy-Authorization header.
"""
# Encode as base64 and prepend the authorization type
return b"Basic " + base64.encodebytes(self.username_password)


@implementer(IAgent)
class ProxyAgent(_AgentBase):
"""An Agent implementation which will use an HTTP proxy if one was requested
Expand Down Expand Up @@ -96,6 +116,9 @@ def __init__(
https_proxy = proxies["https"].encode() if "https" in proxies else None
no_proxy = proxies["no"] if "no" in proxies else None

# Parse credentials from https proxy connection string if present
self.https_proxy_creds, https_proxy = parse_username_password(https_proxy)

self.http_proxy_endpoint = _http_proxy_endpoint(
http_proxy, self.proxy_reactor, **self._endpoint_kwargs
)
Expand Down Expand Up @@ -175,11 +198,22 @@ def request(self, method, uri, headers=None, bodyProducer=None):
and self.https_proxy_endpoint
and not should_skip_proxy
):
connect_headers = Headers()

# Determine whether we need to set Proxy-Authorization headers
if self.https_proxy_creds:
# Set a Proxy-Authorization header
connect_headers.addRawHeader(
b"Proxy-Authorization",
self.https_proxy_creds.as_proxy_authorization_value(),
)

endpoint = HTTPConnectProxyEndpoint(
self.proxy_reactor,
self.https_proxy_endpoint,
parsed_uri.host,
parsed_uri.port,
headers=connect_headers,
)
else:
# not using a proxy
Expand Down Expand Up @@ -223,16 +257,43 @@ def _http_proxy_endpoint(proxy, reactor, **kwargs):
if proxy is None:
return None

# currently we only support hostname:port. Some apps also support
# protocol://<host>[:port], which allows a way of requiring a TLS connection to the
# proxy.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment still seems valid?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort of. We support HTTP_PROXY and HTTPS_PROXY, which does allow us to require a TLS connection to the proxy. But that's done via two separate environment variables, instead of specifying the protocol to a single env var.

Because of that I'm not sure if we still needed to include the bit about TLS.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the pertinent point here is that there are different formats for this env var and we only support a subset of formats.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. I'll try to work that back in!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0c5a5b9 is hopefully a bit clearer?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It just feels a bit sad to lose that information when its already there

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'd missed the underlying point of the comment, assuming that it was old and just referring to the lack of being able to use TLS with a proxy 🙂


# Parse the connection string, supporting the form: [<username>:<password>@]<host>[:port]
host, port = parse_host_port(proxy, default_port=1080)
return HostnameEndpoint(reactor, host, port, **kwargs)


def parse_host_port(hostport, default_port=None):
# could have sworn we had one of these somewhere else...
def parse_username_password(proxy: bytes) -> Tuple[Optional[ProxyCredentials], bytes]:
"""
Parses the username and password from a proxy declaration e.g
username:password@hostname:port.

Args:
proxy: The proxy connection string.

Returns
An instance of ProxyCredentials and the proxy connection string with any credentials
stripped, i.e u:p@host:port -> host:port. If no credentials were found, the
ProxyCredentials instance is replaced with None.
"""
if proxy and b"@" in proxy:
# We use rsplit here as the password could contain an @ character
credentials, proxy_without_credentials = proxy.rsplit(b"@", 1)
return ProxyCredentials(credentials), proxy_without_credentials

return None, proxy


def parse_host_port(hostport: bytes, default_port: int = None) -> Tuple[bytes, int]:
"""
Parse the hostname and port from a proxy connection byte string.

Args:
hostport: The proxy connection string. Must be in the form 'host[:port]'.
default_port: The default port to return if one is not found in `hostport`.

Returns:
A tuple containing the hostname and port. Uses `default_port` if one was not found.
"""
if b":" in hostport:
host, port = hostport.rsplit(b":", 1)
try:
Expand Down