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

Session compression and SNI #223

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion core/Network/TLS/Handshake/Client.hs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ handshakeClient cparams ctx = do
usingState_ ctx $ setClientALPNSuggest protos
return $ Just $ toExtensionRaw $ ApplicationLayerProtocolNegotiation protos
sniExtension = if clientUseServerNameIndication cparams
then return $ Just $ toExtensionRaw $ ServerName [ServerNameHostName $ fst $ clientServerIdentification cparams]
then do let sni = fst $ clientServerIdentification cparams
usingState_ ctx $ setClientSNI sni
return $ Just $ toExtensionRaw $ ServerName [ServerNameHostName sni]
else return Nothing

curveExtension = return $ Just $ toExtensionRaw $ NegotiatedGroups ((supportedGroups $ ctxSupported ctx) `intersect` availableGroups)
Expand Down
10 changes: 7 additions & 3 deletions core/Network/TLS/Handshake/Common.hs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ module Network.TLS.Handshake.Common
import Control.Concurrent.MVar

import Network.TLS.Parameters
import Network.TLS.Compression
import Network.TLS.Context.Internal
import Network.TLS.Session
import Network.TLS.Struct
Expand Down Expand Up @@ -123,14 +124,17 @@ runRecvState ctx iniState = recvPacketHandshake ctx >>= onRecvStateHand
getSessionData :: Context -> IO (Maybe SessionData)
getSessionData ctx = do
ver <- usingState_ ctx getVersion
sni <- usingState_ ctx getClientSNI
mms <- usingHState ctx (gets hstMasterSecret)
tx <- liftIO $ readMVar (ctxTxState ctx)
case mms of
Nothing -> return Nothing
Just ms -> return $ Just $ SessionData
{ sessionVersion = ver
, sessionCipher = cipherID $ fromJust "cipher" $ stCipher tx
, sessionSecret = ms
{ sessionVersion = ver
, sessionCipher = cipherID $ fromJust "cipher" $ stCipher tx
, sessionCompression = compressionID $ stCompression tx
, sessionClientSNI = sni
, sessionSecret = ms
}

extensionLookup :: ExtensionID -> [ExtensionRaw] -> Maybe Bytes
Expand Down
7 changes: 3 additions & 4 deletions core/Network/TLS/Handshake/Server.hs
Original file line number Diff line number Diff line change
Expand Up @@ -212,17 +212,16 @@ handshakeServerWith sparams ctx clientHello@(ClientHello clientVersion _ clientS
commonCompressions = compressionIntersectID (supportedCompressions $ ctxSupported ctx) compressions
usedCompression = head commonCompressions

-- FIXME should also validate compression_methods and server_name
-- (see RFC 5246 at 7.4.1.2 and RFC 6066), but necessary parameters
-- are not stored in SessionData currently
validateSession _ Nothing = Nothing
validateSession _ m@(Just sd)
validateSession sni m@(Just sd)
-- SessionData parameters are assumed to match the local server configuration
-- so we need to compare only to ClientHello inputs. Abbreviated handshake
-- uses the same server_name than full handshake so the same
-- credentials (and thus ciphers) are available.
| clientVersion < sessionVersion sd = Nothing
| sessionCipher sd `notElem` ciphers = Nothing
| sessionCompression sd `notElem` compressions = Nothing
| isJust sni && sessionClientSNI sd /= sni = Nothing
| otherwise = m


Expand Down
10 changes: 7 additions & 3 deletions core/Network/TLS/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ module Network.TLS.Types
import Data.ByteString (ByteString)
import Data.Word

type HostName = String
Copy link
Collaborator

Choose a reason for hiding this comment

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

I found the same code in multiple modules. I would like to remove the other definitions and let the modules import Types.

I guess that this redundancy is the reason why HostName is not highlighted in the HTML doc produced by haddock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I've seen it too and just replicated what already exists. But I can try to do better.

IIRC it's potentially more complex that it looks because x509 packages have this too.
If a type alias is to be exported, it could be from there and not tls.

Copy link
Collaborator

Choose a reason for hiding this comment

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

the goal of this alias was to remember to do something sensible about this, not really expose it to the user. A real Hostname type is coming soon (couple of months away), and when that happens, I'm planning to move those types in tls and x509 to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then in the PR I'd rather keep this 5th local definition of a type alias for now and remove them all when a remplacement is ready.

Does the Hostname plan include something for IDN support?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, we're still not quite there but the plan is to have punycode and full IDN


-- | Versions known to TLS
--
-- SSL2 is just defined, but this version is and will not be supported.
Expand All @@ -29,9 +31,11 @@ type SessionID = ByteString

-- | Session data to resume
data SessionData = SessionData
{ sessionVersion :: Version
, sessionCipher :: CipherID
, sessionSecret :: ByteString
{ sessionVersion :: Version
, sessionCipher :: CipherID
, sessionCompression :: CompressionID
, sessionClientSNI :: Maybe HostName
, sessionSecret :: ByteString
} deriving (Show,Eq)

-- | Cipher identification
Expand Down