Skip to content

Use a certificate issued from the trustme CA in tests - #357

Merged
sethmlarson merged 2 commits into
encode:masterfrom
JayH5:gh-354/trustme-ca
Sep 18, 2019
Merged

sethmlarson merged 2 commits into
encode:masterfrom
JayH5:gh-354/trustme-ca

Conversation

@JayH5

@JayH5 JayH5 commented Sep 18, 2019

Copy link
Copy Markdown
Contributor

Fixes #354

Comment thread tests/conftest.py
lifespan="off",
ssl_certfile=cert_pem_file,
ssl_keyfile=cert_private_key_file,
host="localhost",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a bit of a less-than-ideal workaround for cert validation 😕. Since the server URL that the client uses is built using this host value (maybe it shouldn't be?) it needs to match the certificate name.

Comment thread tests/conftest.py
def cert_pem_file(example_cert):
with example_cert.cert_pem.tempfile() as tmp:
def cert_pem_file(localhost_cert):
with localhost_cert.cert_chain_pems[0].tempfile() as tmp:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

From the trustme docs: "The zeroth entry in this list is the actual PEM-encoded certificate..."

Just including this cert and not the whole chain is fine, since there are no intermediary CA certs.

@sethmlarson
sethmlarson self-requested a review September 18, 2019 21:42

@sethmlarson sethmlarson left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good to me, thank you for this great first contribution! :) Hope you stick around and tackle some more TLS or PKI issues!

@sethmlarson
sethmlarson merged commit 338a360 into encode:master Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Change trustme.CA() usage to mimic real-life

2 participants