-
Notifications
You must be signed in to change notification settings - Fork 14
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
Updating the Haskell stub #312
base: master
Are you sure you want to change the base?
Conversation
@@ -84,6 +86,6 @@ injectCA caBundle p = | |||
|
|||
injectCiphers :: ClientParams -> ClientParams | |||
injectCiphers p = | |||
p { clientSupported = supported { supportedCiphers = ciphersuite_all } } | |||
p { clientSupported = supported { supportedCiphers = ciphersuite_strong } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is ciphersuite_default
also sufficient? Or is the default "too permissive"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They result in the same. So, let's use default
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My intent when using ciphersuite_all
is to let the TLS server select the appropriate ciphersuite for a particular test. In TLS Handshake ClientHello
advertises all supported ciphersuites and ServerHello
selects the one to be used. Changing it to more restrictive list of ciphersuites might limit the testing and provide skewed results.
It's worth testing every supported ciphersuite to find the problems they might contain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well some ciphersuites are implemented, but deprecated (not recommended) and should generally not be enabled. So the question comes down to what a particular test is intended to cover. If it is a test of the correctness of the implementation of even deprecated ciphers, then it should be "all", but if the test is to make sure that weak options are not enabled, then it should use "default" or "strong". So decide what the goal is and choose wisely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ciphersuite_all
will be removed someday. It should not be used anyway.
What is the status of this PR? |
This patch updates the Haskell stub:
http-client
>= 0.5 (the current one), which does not provideTlsExceptionHostPort
ciphersuite_strong
instead of deprecatedciphersuite_all
from Haskell TLSThe author of
http-client
has already checked if this fix is correct.And note that I'm one of maintainers of Haskell TLS.
Thank you for creating a wonderful test tool!