Extensions for TLS 1.3 #278
Conversation
renaming getVersion to getVersion' to avoid conflict.
<message> <old> <new> ServerHello True MsgTServerHello ClientHello False MsgTClientHello False in Client.hs was a mistake, I believe.
There was a problem hiding this comment.
I really look forward to this despite limited time I can allocate.
It's probably possible to improve style slightly in this PR or next ones, for example I don't think you used the same indentation than existing code for data-type declarations.
But this is minor compared to huge achievement of bringing TLS13.
core/Network/TLS/Extension.hs
Outdated
| @@ -296,3 +356,162 @@ instance Extension SignatureAlgorithms where | |||
| runGetMaybe $ do | |||
| len <- getWord16 | |||
| SignatureAlgorithms <$> getList (fromIntegral len) (getSignatureHashAlgorithm >>= \sh -> return (2, sh)) | |||
|
|
|||
| type SignatureAlgorithmsCert = SignatureAlgorithms | |||
There was a problem hiding this comment.
I don't understand how that one works. extensionID will be same, no?
There was a problem hiding this comment.
My fault. They have different IDs. I will fix it.
core/Network/TLS/Packet.hs
Outdated
| return $ verOfNum (major, minor) | ||
|
|
||
| putVersion' :: Version -> Put | ||
| putVersion' ver = putWord8 major >> putWord8 minor |
There was a problem hiding this comment.
Other ideas for naming: putBinaryVersion, putWireVersion, putSerializedVersion, putVersionBytes.
There was a problem hiding this comment.
I took putBinaryVersion. Thank you for your nice suggestion!
| -- | Extension class to transform bytes to and from a high level Extension type. | ||
| class Extension a where | ||
| extensionID :: a -> ExtensionID | ||
| extensionDecode :: Bool -> ByteString -> Maybe a | ||
| extensionDecode :: MessageType -> ByteString -> Maybe a |
There was a problem hiding this comment.
👍 for moving away from Bool
core/Network/TLS/Extension.hs
Outdated
| instance Extension KeyShare where | ||
| extensionID _ = extensionID_KeyShare | ||
| extensionEncode (KeyShareClientHello kses) = runPut $ do | ||
| let !len = sum $ map (\(KeyShareEntry _ key) -> B.length key + 4) kses |
There was a problem hiding this comment.
Would probably look nicer with list comprehension.
|
Thank you for your review.
Honestly I think we should use a formatter rather than hand-styling. |
|
Merged. Thank you for reviewing! |
RFC 8446 for TLS 1.3 is coming. My implementation of TLS 1.3 shows good inter-operability with other implementations (#167). So, I would like to start merge it into
masterstep by step.