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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Updating the Haskell stub #312

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad0f652
using LTS 12.17 and http-client (>= 0.5) for Haskell TLS.
kazu-yamamoto Nov 6, 2018
dc52217
using ciphersuite_default.
kazu-yamamoto Nov 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions stubs/haskell-http-client-tls/src/Main.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import Data.ByteString.Char8 (unpack)
import System.Environment (getArgs, getProgName)
import System.Exit (exitFailure, exitSuccess)

import Network.HTTP.Client (HttpException (..), httpLbs,
import Network.HTTP.Client (HttpException (..),
HttpExceptionContent(..),
httpLbs,
newManager, parseRequest,
responseStatus)
import Network.HTTP.Client.TLS (mkManagerSettings)
Expand All @@ -23,7 +25,7 @@ import Network.TLS (ClientParams, clientShared,
clientSupported,
defaultParamsClient, sharedCAStore,
supportedCiphers)
import Network.TLS.Extra.Cipher (ciphersuite_all)
import Network.TLS.Extra.Cipher (ciphersuite_strong)

main :: IO ()
main = do
Expand Down Expand Up @@ -54,7 +56,7 @@ main = do

_ <- catch (doGet request manager)
(\exp' -> case exp' of
TlsExceptionHostPort e _ _ -> do
HttpExceptionRequest _req (InternalException e) -> do
print e
putStrLn "REJECT"
exitSuccess
Expand Down Expand Up @@ -84,6 +86,6 @@ injectCA caBundle p =

injectCiphers :: ClientParams -> ClientParams
injectCiphers p =
p { clientSupported = supported { supportedCiphers = ciphersuite_all } }
p { clientSupported = supported { supportedCiphers = ciphersuite_strong } }
Copy link

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"?

Copy link
Author

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.

Copy link
Member

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.

Copy link

@vdukhovni vdukhovni Nov 8, 2018 •
edited
Loading

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.

Copy link
Author

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.

where
supported = clientSupported p
2 changes: 1 addition & 1 deletion stubs/haskell-http-client-tls/stack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
resolver: lts-7.15
resolver: lts-12.17
2 changes: 1 addition & 1 deletion stubs/haskell-http-client-tls/test-http-client-tls.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ executable test-http-client-tls
build-depends: base >= 4.7 && < 5
, bytestring
, connection
, http-client
, http-client >= 0.5
, http-client-tls
, http-types
, tls
Expand Down