Skip to content

Commit

Permalink
Integrate trustme fake CA lib for testing TLS
Browse files Browse the repository at this point in the history
Also:
* Use real TLS context where it's disabled in tests
* Add a change note about trustme integration

Closes #3487
  • Loading branch information
webknjaz committed Jan 5, 2019
1 parent a929c06 commit c180800
Show file tree
Hide file tree
Showing 13 changed files with 138 additions and 119 deletions.
1 change: 1 addition & 0 deletions CHANGES/3487.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Integrate [`trustme`](https://trustme.readthedocs.io/en/latest/) to better test TLS support.
1 change: 1 addition & 0 deletions requirements/ci-wheel.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pytest==4.0.2
pytest-cov==2.6.0
pytest-mock==1.10.0
tox==3.6.1
trustme==0.4.0
twine==1.12.1
yarl==1.3.0

Expand Down
54 changes: 54 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import hashlib
import pathlib
import shutil
import ssl
import tempfile

import pytest
import trustme

pytest_plugins = ['aiohttp.pytest_plugin', 'pytester']

Expand All @@ -15,3 +18,54 @@ def shorttmpdir():
tmpdir = pathlib.Path(tempfile.mkdtemp())
yield tmpdir
shutil.rmtree(tmpdir, ignore_errors=True)


@pytest.fixture
def tls_certificate_authority():
return trustme.CA()


@pytest.fixture
def tls_certificate(tls_certificate_authority):
return tls_certificate_authority.issue_server_cert(
'localhost',
'127.0.0.1',
'::1',
)


@pytest.fixture
def ssl_ctx(tls_certificate):
ssl_ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
tls_certificate.configure_cert(ssl_ctx)
return ssl_ctx


@pytest.fixture
def client_ssl_ctx(tls_certificate_authority):
ssl_ctx = ssl.create_default_context(purpose=ssl.Purpose.SERVER_AUTH)
tls_certificate_authority.configure_trust(ssl_ctx)
return ssl_ctx


@pytest.fixture
def tls_ca_certificate_pem_path(tls_certificate_authority):
with tls_certificate_authority.cert_pem.tempfile() as ca_cert_pem:
yield ca_cert_pem


@pytest.fixture
def tls_certificate_pem_path(tls_certificate):
with tls_certificate.private_key_and_cert_chain_pem.tempfile() as cert_pem:
yield cert_pem


@pytest.fixture
def tls_certificate_pem_bytes(tls_certificate):
return tls_certificate.cert_chain_pems[0].bytes()


@pytest.fixture
def tls_certificate_fingerprint_sha256(tls_certificate_pem_bytes):
tls_cert_der = ssl.PEM_cert_to_DER_cert(tls_certificate_pem_bytes.decode())

This comment has been minimized.

Copy link
@webknjaz

webknjaz Jan 5, 2019

Author Member

I've just suggested to put this into trustme @ python-trio/trustme#39

return hashlib.sha256(tls_cert_der).digest()
14 changes: 0 additions & 14 deletions tests/sample.crt

This file was deleted.

Binary file removed tests/sample.crt.der
Binary file not shown.
15 changes: 0 additions & 15 deletions tests/sample.key

This file was deleted.

50 changes: 20 additions & 30 deletions tests/test_client_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import json
import pathlib
import socket
import ssl
from unittest import mock

import pytest
Expand All @@ -25,18 +24,9 @@ def here():
return pathlib.Path(__file__).parent


@pytest.fixture
def ssl_ctx(here):
ssl_ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
ssl_ctx.load_cert_chain(
str(here / 'sample.crt'),
str(here / 'sample.key'))
return ssl_ctx


@pytest.fixture
def fname(here):
return here / 'sample.key'
return here / 'conftest.py'


def ceil(val):
Expand Down Expand Up @@ -274,8 +264,11 @@ async def handler(request):
assert 200 == resp.status


async def test_ssl_client(ssl_ctx, aiohttp_server, aiohttp_client) -> None:
connector = aiohttp.TCPConnector(ssl=False)
async def test_ssl_client(
aiohttp_server, ssl_ctx,
aiohttp_client, client_ssl_ctx,
) -> None:
connector = aiohttp.TCPConnector(ssl=client_ssl_ctx)

async def handler(request):
return web.Response(text='Test message')
Expand All @@ -291,17 +284,16 @@ async def handler(request):
assert txt == 'Test message'


async def test_tcp_connector_fingerprint_ok(aiohttp_server, aiohttp_client,
ssl_ctx):

fingerprint = (b'0\x9a\xc9D\x83\xdc\x91\'\x88\x91\x11\xa1d\x97\xfd'
b'\xcb~7U\x14D@L'
b'\x11\xab\x99\xa8\xae\xb7\x14\xee\x8b')
async def test_tcp_connector_fingerprint_ok(
aiohttp_server, aiohttp_client,
ssl_ctx, tls_certificate_fingerprint_sha256,
):
tls_fingerprint = Fingerprint(tls_certificate_fingerprint_sha256)

async def handler(request):
return web.Response(text='Test message')

connector = aiohttp.TCPConnector(ssl=Fingerprint(fingerprint))
connector = aiohttp.TCPConnector(ssl=tls_fingerprint)
app = web.Application()
app.router.add_route('GET', '/', handler)
server = await aiohttp_server(app, ssl=ssl_ctx)
Expand All @@ -312,17 +304,14 @@ async def handler(request):
resp.close()


async def test_tcp_connector_fingerprint_fail(aiohttp_server, aiohttp_client,
ssl_ctx):

fingerprint = (b'0\x9a\xc9D\x83\xdc\x91\'\x88\x91\x11\xa1d\x97\xfd'
b'\xcb~7U\x14D@L'
b'\x11\xab\x99\xa8\xae\xb7\x14\xee\x8b')

async def test_tcp_connector_fingerprint_fail(
aiohttp_server, aiohttp_client,
ssl_ctx, tls_certificate_fingerprint_sha256,
):
async def handler(request):
return web.Response(text='Test message')

bad_fingerprint = b'\x00' * len(fingerprint)
bad_fingerprint = b'\x00' * len(tls_certificate_fingerprint_sha256)

connector = aiohttp.TCPConnector(ssl=Fingerprint(bad_fingerprint))

Expand All @@ -335,7 +324,7 @@ async def handler(request):
await client.get('/')
exc = cm.value
assert exc.expected == bad_fingerprint
assert exc.got == fingerprint
assert exc.got == tls_certificate_fingerprint_sha256


async def test_format_task_get(aiohttp_server) -> None:
Expand Down Expand Up @@ -1402,7 +1391,7 @@ async def handler(request):
'text/plain',
'application/octet-stream']
assert request.headers['content-disposition'] == (
"inline; filename=\"sample.key\"; filename*=utf-8''sample.key")
"inline; filename=\"conftest.py\"; filename*=utf-8''conftest.py")

return web.Response()

Expand All @@ -1428,6 +1417,7 @@ async def handler(request):
# then use 'application/octet-stream' default
assert request.content_type in ['application/pgp-keys',
'text/plain',
'text/x-python',
'application/octet-stream']
return web.Response()

Expand Down
10 changes: 5 additions & 5 deletions tests/test_client_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ async def test_chunked_transfer_encoding(loop, conn) -> None:

async def test_file_upload_not_chunked(loop) -> None:
here = os.path.dirname(__file__)
fname = os.path.join(here, 'sample.key')
fname = os.path.join(here, 'aiohttp.png')
with open(fname, 'rb') as f:
req = ClientRequest(
'post', URL('http://python.org/'),
Expand All @@ -828,7 +828,7 @@ async def test_precompressed_data_stays_intact(loop) -> None:

async def test_file_upload_not_chunked_seek(loop) -> None:
here = os.path.dirname(__file__)
fname = os.path.join(here, 'sample.key')
fname = os.path.join(here, 'aiohttp.png')
with open(fname, 'rb') as f:
f.seek(100)
req = ClientRequest(
Expand All @@ -842,7 +842,7 @@ async def test_file_upload_not_chunked_seek(loop) -> None:

async def test_file_upload_force_chunked(loop) -> None:
here = os.path.dirname(__file__)
fname = os.path.join(here, 'sample.key')
fname = os.path.join(here, 'aiohttp.png')
with open(fname, 'rb') as f:
req = ClientRequest(
'post', URL('http://python.org/'),
Expand Down Expand Up @@ -1270,11 +1270,11 @@ async def create_connection(req, traces, timeout):
conn.close()


def test_verify_ssl_false_with_ssl_context(loop) -> None:
def test_verify_ssl_false_with_ssl_context(loop, ssl_ctx) -> None:
with pytest.warns(DeprecationWarning):
with pytest.raises(ValueError):
_merge_ssl_params(None, verify_ssl=False,
ssl_context=mock.Mock(), fingerprint=None)
ssl_context=ssl_ctx, fingerprint=None)


def test_bad_fingerprint(loop) -> None:
Expand Down
28 changes: 9 additions & 19 deletions tests/test_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import asyncio
import gc
import hashlib
import os.path
import platform
import socket
import ssl
Expand Down Expand Up @@ -1999,20 +1998,16 @@ async def test_resolver_not_called_with_address_is_ip(loop) -> None:
resolver.resolve.assert_not_called()


async def test_tcp_connector_raise_connector_ssl_error(aiohttp_server) -> None:
async def test_tcp_connector_raise_connector_ssl_error(
aiohttp_server, ssl_ctx,
) -> None:
async def handler(request):
return web.Response()

app = web.Application()
app.router.add_get('/', handler)

here = os.path.join(os.path.dirname(__file__), '..', 'tests')
keyfile = os.path.join(here, 'sample.key')
certfile = os.path.join(here, 'sample.crt')
sslcontext = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
sslcontext.load_cert_chain(certfile, keyfile)

srv = await aiohttp_server(app, ssl=sslcontext)
srv = await aiohttp_server(app, ssl=ssl_ctx)

port = unused_port()
conn = aiohttp.TCPConnector(local_addr=('127.0.0.1', port))
Expand All @@ -2038,27 +2033,22 @@ async def handler(request):


async def test_tcp_connector_do_not_raise_connector_ssl_error(
aiohttp_server) -> None:
aiohttp_server, ssl_ctx, client_ssl_ctx,
) -> None:
async def handler(request):
return web.Response()

app = web.Application()
app.router.add_get('/', handler)

here = os.path.join(os.path.dirname(__file__), '..', 'tests')
keyfile = os.path.join(here, 'sample.key')
certfile = os.path.join(here, 'sample.crt')
sslcontext = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
sslcontext.load_cert_chain(certfile, keyfile)

srv = await aiohttp_server(app, ssl=sslcontext)
srv = await aiohttp_server(app, ssl=ssl_ctx)
port = unused_port()
conn = aiohttp.TCPConnector(local_addr=('127.0.0.1', port))

session = aiohttp.ClientSession(connector=conn)
url = srv.make_url('/')

r = await session.get(url, ssl=sslcontext)
r = await session.get(url, ssl=client_ssl_ctx)

r.release()
first_conn = next(iter(conn._conns.values()))[0][0]
Expand All @@ -2068,7 +2058,7 @@ async def handler(request):
except AttributeError:
_sslcontext = first_conn.transport._sslcontext

assert _sslcontext is sslcontext
assert _sslcontext is client_ssl_ctx
r.close()

await session.close()
Expand Down
4 changes: 2 additions & 2 deletions tests/test_route_def.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ def test_static(router) -> None:
info = resource.get_info()
assert info['prefix'] == '/prefix'
assert info['directory'] == folder
url = resource.url_for(filename='sample.key')
assert url == URL('/prefix/sample.key')
url = resource.url_for(filename='aiohttp.png')
assert url == URL('/prefix/aiohttp.png')


def test_head_deco(router) -> None:
Expand Down

4 comments on commit c180800

@asvetlov
Copy link
Member

Choose a reason for hiding this comment

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

Did you push the commit without review?

@webknjaz
Copy link
Member Author

Choose a reason for hiding this comment

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

I verified that tests pass and squashed the PR.

@asvetlov
Copy link
Member

Choose a reason for hiding this comment

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

I agree with the improvement but wanted to review the change carefully.
Anyway, the question is: should the commit be backported into 3.5 branch?

@webknjaz
Copy link
Member Author

Choose a reason for hiding this comment

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

It can be safely backported given that 3.5 is going to be supported for a while

Please sign in to comment.