RomanNum3ral
/
scrapy
mirror of https://github.com/scrapy/scrapy.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

feat: check for invalid hostname 

- Initiating requests having hostname or (ip_address, port) different
from the peer to which HTTP/2 connection is made can lead to closing the
whole connection and close out all the pending streams.
- This change aims to fix that problem
- Add required tests
- Save hostname & port in H2ConnectionMetadataDict
This commit is contained in:
Aditya 2020-07-01 18:14:44 +05:30
parent 7b1ad995a4
commit c361fe0d3b
4 changed files with 118 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
scrapy/core/http2/protocol.py
View File

@ -2,7 +2,7 @@ import ipaddress
import itertools
import logging
from collections import deque
from typing import Union, Dict
from typing import Dict, Optional
from h2.config import H2Configuration
from h2.connection import H2Connection
@ -26,10 +26,6 @@ class H2ClientProtocol(Protocol):
config = H2Configuration(client_side=True, header_encoding='utf-8')
self.conn = H2Connection(config=config)
# Address of the server we are connected to
# these are updated when connection is successfully made
self.destination = None
# ID of the next request stream
# Following the convention made by hyper-h2 each client ID
# will be odd.
@ -50,11 +46,13 @@ class H2ClientProtocol(Protocol):
# Save an instance of ProtocolError raised by hyper-h2
# We pass this instance to the streams ResponseFailed() failure
self._protocol_error: Union[None, ProtocolError] = None
self._protocol_error: Optional[ProtocolError] = None
self._metadata: H2ConnectionMetadataDict = {
'certificate': None,
'ip_address': None
'ip_address': None,
'hostname': None,
'port': None
}
@property
@ -123,7 +121,7 @@ class H2ClientProtocol(Protocol):
def request(self, request: Request):
if not isinstance(request, Request):
raise TypeError(f'Expected type scrapy.http.Request but received {request.__class__.__name__}')
raise TypeError(f'Expected scrapy.http.Request, received {request.__class__.__name__}')
stream = self._new_stream(request)
d = stream.get_response()
@ -136,9 +134,11 @@ class H2ClientProtocol(Protocol):
"""Called by Twisted when the connection is established. We can start
sending some data now: we should open with the connection preamble.
"""
self.destination = self.transport.getPeer()
logger.info(f'Connection made to {self.destination}')
self._metadata['ip_address'] = ipaddress.ip_address(self.destination.host)
destination = self.transport.getPeer()
logger.debug('Connection made to {}'.format(destination))
self._metadata['ip_address'] = ipaddress.ip_address(destination.host)
self._metadata['port'] = destination.port
self._metadata['hostname'] = self.transport.transport.addr[0]
self.conn.initiate_connection()
self._write_to_transport()
@ -148,8 +148,6 @@ class H2ClientProtocol(Protocol):
events = self.conn.receive_data(data)
self._handle_events(events)
except ProtocolError as e:
# TODO: In case of InvalidBodyLengthError -- terminate only one stream
# Save this error as ultimately the connection will be dropped
# internally by hyper-h2. Saved error will be passed to all the streams
# closed with the connection.
@ -201,7 +199,7 @@ class H2ClientProtocol(Protocol):
elif isinstance(event, SettingsAcknowledged):
self.settings_acknowledged(event)
else:
logger.debug(f'Received unhandled event {event}')
logger.debug('Received unhandled event {}'.format(event))
# Event handler functions starts here
def data_received(self, event: DataReceived):

50
scrapy/core/http2/stream.py
View File

@ -29,6 +29,17 @@ class InactiveStreamClosed(ConnectionClosed):
self.request = request
class InvalidHostname(Exception):
def __init__(self, request: Request, expected_hostname, expected_netloc):
self.request = request
self.expected_hostname = expected_hostname
self.expected_netloc = expected_netloc
def __str__(self):
return f'InvalidHostname: Expected {self.expected_hostname} or {self.expected_netloc} in {self.request}'
class StreamCloseReason(Enum):
# Received a StreamEnded event
ENDED = 1
@ -49,6 +60,10 @@ class StreamCloseReason(Enum):
# Connection lost and the stream was not initiated
INACTIVE = 6
# The hostname of the request is not same as of connected peer hostname
# As a result sending this request will the end the connection
INVALID_HOSTNAME = 7
class Stream:
"""Represents a single HTTP/2 Stream.
@ -163,6 +178,15 @@ class Stream:
"""
return self._deferred_response
def check_request_url(self) -> bool:
# Make sure that we are sending the request to the correct URL
url = urlparse(self._request.url)
return (
url.netloc == self._conn_metadata['hostname']
or url.netloc == f'{self._conn_metadata["hostname"]}:{self._conn_metadata["port"]}'
or url.netloc == f'{self._conn_metadata["ip_address"]}:{self._conn_metadata["port"]}'
)
def _get_request_headers(self):
url = urlparse(self._request.url)
@ -184,10 +208,15 @@ class Stream:
return headers
def initiate_request(self):
headers = self._get_request_headers()
self._conn.send_headers(self.stream_id, headers, end_stream=False)
self.request_sent = True
self.send_data()
if self.check_request_url():
headers = self._get_request_headers()
self._conn.send_headers(self.stream_id, headers, end_stream=False)
self.request_sent = True
self.send_data()
else:
# Close this stream calling the response errback
# Note that we have not sent any headers
self.close(StreamCloseReason.INVALID_HOSTNAME)
def send_data(self):
"""Called immediately after the headers are sent. Here we send all the
@ -310,6 +339,9 @@ class Stream:
if self.stream_closed_server:
raise StreamClosedError(self.stream_id)
if not isinstance(reason, StreamCloseReason):
raise TypeError(f'Expected StreamCloseReason, received {reason.__class__.__name__}')
self._cb_close(self.stream_id)
self.stream_closed_server = True
@ -353,6 +385,13 @@ class Stream:
elif reason is StreamCloseReason.INACTIVE:
self._deferred_response.errback(InactiveStreamClosed(self._request))
elif reason is StreamCloseReason.INVALID_HOSTNAME:
self._deferred_response.errback(InvalidHostname(
self._request,
self._conn_metadata['hostname'],
f'{self._conn_metadata["ip_address"]}:{self._conn_metadata["port"]}'
))
def _fire_response_deferred(self, flags: List[str] = None):
"""Builds response from the self._response dict
and fires the response deferred callback with the
@ -365,10 +404,9 @@ class Stream:
body=body
)
status = self._response['headers'][':status']
response = response_cls(
url=self._request.url,
status=status,
status=self._response['headers'][':status'],
headers=self._response['headers'],
body=body,
request=self._request,

11
scrapy/core/http2/types.py
View File

@ -14,8 +14,19 @@ class H2ConnectionMetadataDict(TypedDict):
initialized when connection is successfully made
"""
certificate: Optional[Certificate]
# Address of the server we are connected to which
# is updated when HTTP/2 connection is made successfully
ip_address: Optional[Union[IPv4Address, IPv6Address]]
# Name of the peer HTTP/2 connection is established
hostname: Optional[str]
port: Optional[int]
# Both ip_address and hostname are used by the Stream before
# initiating the request to verify that the base address
class H2ResponseDict(TypedDict):
# Data received frame by frame from the server is appended

64
tests/test_http2_client_protocol.py
View File

@ -10,7 +10,7 @@ from urllib.parse import urlencode
from h2.exceptions import InvalidBodyLengthError
from twisted.internet import reactor
from twisted.internet.defer import inlineCallbacks, DeferredList, CancelledError
from twisted.internet.endpoints import SSL4ClientEndpoint, SSL4ServerEndpoint, TCP4ServerEndpoint
from twisted.internet.endpoints import SSL4ClientEndpoint, SSL4ServerEndpoint
from twisted.internet.protocol import Factory
from twisted.internet.ssl import optionsForClientTLS, PrivateCertificate, Certificate
from twisted.python.failure import Failure
@ -20,7 +20,7 @@ from twisted.web.server import Site, NOT_DONE_YET
from twisted.web.static import File
from scrapy.core.http2.protocol import H2ClientProtocol
from scrapy.core.http2.stream import InactiveStreamClosed
from scrapy.core.http2.stream import InactiveStreamClosed, InvalidHostname
from scrapy.http import Request, Response, JsonRequest
from scrapy.utils.python import to_bytes, to_unicode
from tests.mockserver import ssl_context_factory, LeafResource, Status
@ -135,7 +135,7 @@ class QueryParams(LeafResource):
return to_bytes(json.dumps(query_params))
def get_client_certificate(key_file, certificate_file):
def get_client_certificate(key_file, certificate_file) -> PrivateCertificate:
with open(key_file, 'r') as key, open(certificate_file, 'r') as certificate:
pem = ''.join(key.readlines()) + ''.join(certificate.readlines())
@ -171,19 +171,16 @@ class Https2ClientProtocolTestCase(TestCase):
# Start server for testing
self.hostname = u'localhost'
if self.scheme == 'https':
context_factory = ssl_context_factory(self.key_file, self.certificate_file)
server_endpoint = SSL4ServerEndpoint(reactor, 0, context_factory, interface=self.hostname)
else:
server_endpoint = TCP4ServerEndpoint(reactor, 0, interface=self.hostname)
context_factory = ssl_context_factory(self.key_file, self.certificate_file)
server_endpoint = SSL4ServerEndpoint(reactor, 0, context_factory, interface=self.hostname)
self.server = yield server_endpoint.listen(self.site)
self.port_number = self.server.getHost().port
# Connect H2 client with server
client_certificate = get_client_certificate(self.key_file, self.certificate_file)
self.client_certificate = get_client_certificate(self.key_file, self.certificate_file)
client_options = optionsForClientTLS(
hostname=self.hostname,
trustRoot=client_certificate,
trustRoot=self.client_certificate,
acceptableProtocols=[b'h2']
)
h2_client_factory = Factory.forProtocol(H2ClientProtocol)
@ -440,8 +437,8 @@ class Https2ClientProtocolTestCase(TestCase):
yield self._check_log_warnsize(request, warn_pattern, Data.NO_CONTENT_LENGTH)
def test_max_concurrent_streams(self):
"""Send 1000 requests to check if we can handle
very large number of request
"""Send 500 requests at one to check if we can handle
very large number of request.
"""
def get_deferred():
@ -451,7 +448,7 @@ class Https2ClientProtocolTestCase(TestCase):
200
)
return self._check_repeat(get_deferred, 1000)
return self._check_repeat(get_deferred, 500)
def test_inactive_stream(self):
"""Here we send 110 requests considering the MAX_CONCURRENT_STREAMS
@ -524,6 +521,10 @@ class Https2ClientProtocolTestCase(TestCase):
def assert_metadata(response: Response):
self.assertEqual(response.request, request)
self.assertIsInstance(response.certificate, Certificate)
self.assertIsNotNone(response.certificate.original)
self.assertEqual(response.certificate.getIssuer(), self.client_certificate.getIssuer())
self.assertTrue(response.certificate.getPublicKey().matches(self.client_certificate.getPublicKey()))
self.assertIsInstance(response.ip_address, IPv4Address)
self.assertEqual(str(response.ip_address), '127.0.0.1')
@ -532,3 +533,40 @@ class Https2ClientProtocolTestCase(TestCase):
d.addErrback(self.fail)
return d
def _check_invalid_netloc(self, url):
request = Request(url)
def assert_invalid_hostname(failure: Failure):
self.assertIsNotNone(failure.check(InvalidHostname))
error_msg = str(failure.value)
self.assertIn('localhost', error_msg)
self.assertIn('127.0.0.1', error_msg)
self.assertIn(str(request), error_msg)
d = self.client.request(request)
d.addCallback(self.fail)
d.addErrback(assert_invalid_hostname)
return d
def test_invalid_hostname(self):
return self._check_invalid_netloc('https://notlocalhost.notlocalhostdomain')
def test_invalid_host_port(self):
port = self.port_number + 1
return self._check_invalid_netloc(f'https://127.0.0.1:{port}')
def test_connection_stays_with_invalid_requests(self):
d_list = [
self.test_invalid_hostname(),
self.test_invalid_host_port(),
self._check_GET(Request(self.get_url('/get-data-html-small')), Data.HTML_SMALL, 200),
self._check_POST_json(
JsonRequest(url=self.get_url('/post-data-json-small'), method='POST', data=Data.JSON_SMALL),
Data.JSON_SMALL,
Data.EXTRA_SMALL,
200
)
]
return DeferredList(d_list, fireOnOneErrback=True)