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 local TLS server #6662

Merged
merged 1 commit into from
Mar 14, 2024
Merged

Add local TLS server #6662

merged 1 commit into from
Mar 14, 2024

Conversation

sigmavirus24
Copy link
Contributor

This also adds certificates for testing purposes and files to make it easy to generate/regenerate them.

This also replaces an existing test of how we utilize our pool manager such that we don't connect to badssl.com

Finally, this adds additional context parameters for our pool manager to account for mTLS certificates used by clients to authenticate to a server.

Copy link
Member

@nateprewitt nateprewitt left a comment

Choose a reason for hiding this comment

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

minor nit, but otherwise looks good.

Edit: I guess we've got test failures coming from connection resets.

openssl req -key $< -new -out $@ -config cert.cnf

client.pem: client.csr
openssl x509 -req -CA ./ca/ca.crt -CAkey ./ca/ca-private.key -in client.csr -outform PEM -out client.pem -days 730 -CAcreateserial
Copy link
Member

Choose a reason for hiding this comment

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

nit; any reason we're using 730 days here v 7300 days above in tests/certs/expired/ca/Makefile?

Not an issue but we may want to keep them consistent or note the variance as significant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certificate authorities should last significantly longer (20-25 years) per CAB forum. This is a cert issued by that which could theoretically live longer than the CA if we did the same expiration. I figured ~2 years would be sufficient for this test while helping us make sure the make targets keep working

Copy link
Member

Choose a reason for hiding this comment

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

TIL, thank you!

@sigmavirus24
Copy link
Contributor Author

Ack didn't mean to add that roadmap file. Was a brain dump for years back

@sigmavirus24
Copy link
Contributor Author

Edit: I guess we've got test failures coming from connection resets.

Yeah I think having a handshake failure causes the socket to be in an unhappy place but I don't feel like making this a perfect server.

Copy link
Member

@nateprewitt nateprewitt left a comment

Choose a reason for hiding this comment

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

Looks like .coverage.enoch.2677669.XurTNrNx may have accidentally been added? Otherwise, this looks good to me. Thanks @sigmavirus24!

Copy link
Member

Choose a reason for hiding this comment

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

Was this meant to be included?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah no. I'll kill this and force push and merge when green

openssl req -key $< -new -out $@ -config cert.cnf

client.pem: client.csr
openssl x509 -req -CA ./ca/ca.crt -CAkey ./ca/ca-private.key -in client.csr -outform PEM -out client.pem -days 730 -CAcreateserial
Copy link
Member

Choose a reason for hiding this comment

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

TIL, thank you!

This also adds certificates for testing purposes and files to make it
easy to generate/regenerate them.

This also replaces an existing test of how we utilize our pool manager
such that we don't connect to badssl.com

Finally, this adds additional context parameters for our pool manager to
account for mTLS certificates used by clients to authenticate to a
server.
@sigmavirus24 sigmavirus24 merged commit eeafb6a into psf:main Mar 14, 2024
25 checks passed
@sigmavirus24 sigmavirus24 deleted the fix-tls-floppy branch March 14, 2024 11:27
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.

None yet

2 participants