Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new endpoint for cipher testing #19

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sivel
Copy link
Member

@sivel sivel commented May 20, 2024

Adds new insecure.ansible.http.tests with only 1 cipher configured. The inversion for the cipher we want disabled is mostly just documentation.

nginx.sites.conf Outdated
ssl_certificate_key /root/ca/private/insecure.ansible.http.tests-key.pem;

ssl_protocols TLSv1 TLSv1.1 TLSv1.2;
ssl_ciphers ECDHE-RSA-AES128-SHA256:!ECDHE-ECDSA-AES128-SHA;
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 we're going to need something a bit more nuanced than this single endpoint with these two cipher suites.

The default cipher suites for our tested clients vary, so some of them will be able to connect to this endpoint without additional configuration.

Copy link
Member Author

Choose a reason for hiding this comment

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

The tests themselves handle it, because they both explicitly declare the cipher to use. Or are you saying that it doesn't really test the explicit use of a cipher as intended, because the default cipher suite in python will include this cipher?

Copy link
Member

Choose a reason for hiding this comment

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

Or are you saying that it doesn't really test the explicit use of a cipher as intended, because the default cipher suite in python will include this cipher?

Correct. This endpoint should use a cipher suite that the client supports, but not by default. Otherwise a successful connection doesn't tell us whether or not the cipher suite selection had any effect.

Additionally, the test should try to connect with the default settings and verify the connection fails. That way we'll know if the client defaults are suitable for testing against this endpoint.

The reverse is actually needed for the "test bad cipher" case. We need an endpoint that works with the default cipher suite selection on the client (verified by the test), but that fails when a cipher is chosen that the server does not support.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, think I've got this taken care of.

Copy link
Member Author

Choose a reason for hiding this comment

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

scratch that, I've expanded my testing, and need to work out details of the weak cipher a bit more.

@sivel sivel marked this pull request as draft May 21, 2024 16:30
nginx.sites.conf Outdated

server {
listen 80;
listen 445 ssl;
Copy link
Member

Choose a reason for hiding this comment

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

Should this second endpoint use a different port from the one above?

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 doesn't appear to be necessary, as long as both support the same TLS protocol, they don't need to share the ciphers, but to avoid issues, and confusion, it's probably best to separate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants