quic 0.3.9 → 0.3.10

raw patch · 12 files changed

+151/−28 lines, 12 filesPVP: major bump suggested

API removals or changes: PVP suggests a major version bump

API changes (from Hackage documentation)

Files

• ChangeLog.md +36/−0
• Network/QUIC/Connection/Crypto.hs +22/−0
• Network/QUIC/Connection/Stream.hs +2/−2
• Network/QUIC/Connection/Types.hs +12/−6
• Network/QUIC/Crypto/Nite.hs +0/−1
• Network/QUIC/Crypto/Utils.hs +0/−1
• Network/QUIC/Handshake.hs +12/−6
• Network/QUIC/Packet/Decrypt.hs +31/−3
• Network/QUIC/Receiver.hs +20/−4
• quic.cabal +1/−1
• test/ErrorSpec.hs +10/−1
• test/TransportError.hs +5/−3

@@ -1,5 +1,41 @@ # ChangeLog +## 0.3.10++Two for the server: one that answered on a stream it should never have+been given, and one that made a lost flight cost seconds.++* Hand the application a stream only once its first frame is read.+  `openStream` created the stream and gave it to the application in one+  step, before the frame that opened it had been looked over, so a first+  STREAM frame past the flow control limit was answered by the+  application in the moment before the connection was closed over it.+  An HTTP/3 server sent a response on a stream the peer had never+  opened, and the peer called that a STREAM_STATE_ERROR, as RFC 9000 Sec+  19.8 says to, before the FLOW_CONTROL_ERROR arrived; h3spec's "MUST+  send FLOW_CONTROL_ERROR if a STREAM frame with a large offset is+  received" failed for that reason.  The stream now reaches the+  application after the frame has been checked, so a frame that closes+  the connection closes it with the application none the wiser.+  [#131](https://github.com/kazu-yamamoto/quic/pull/131)++* Keep the Initial keys of the version the client addressed us in.  A+  server that settles on a compatible version answers in it and replaces+  its Initial keys with the ones for that version (RFC 9368), but the+  client hears of the choice only when the answer arrives and until then+  retransmits in the version it started with.  Without the keys for that+  version the server could not pick the handshake up from those and+  waited out its own PTO instead -- a second, then two, then four, with+  the client's retransmissions falling into it unread.  On the test that+  loses the server's whole first flight, the handshake is done at about+  1.4 seconds where it took about 7.0.  This is the other half of #128,+  which stopped the same sequence from hanging for good; what was left+  was the waiting.+  [#132](https://github.com/kazu-yamamoto/quic/pull/132)++* The library builds without a warning: two imports left over from the+  crypton 2.1.0 work are gone.+ ## 0.3.9  A stream limit the peer could walk past, two ways for a connection to

@@ -13,6 +13,8 @@     setNegotiated,     --     dropSecrets,+    keepInitialKeys,+    getPrevInitialKeys,     --     initializeCoder,     initializeCoder1RTT,@@ -95,6 +97,26 @@ dropSecrets Connection{..} lvl = do     writeArray coders lvl initialCoder     writeArray protectors lvl initialProtector+    when (lvl == InitialLevel) $ writeIORef prevInitialKeys Nothing++-- | Keeping the Initial keys we have, before they are replaced with the ones+--   for a version we chose ourselves.+--+-- A server that settles on a compatible version (RFC 9368) answers in it, but+-- the client hears of that only when the answer arrives.  Until then it+-- retransmits its Initial packets in the version it started with, and without+-- the keys for that version they are so much noise: the server cannot pick up+-- the handshake from them, and has to wait out its own PTO instead -- a+-- second, then two, then four, with the client's retransmissions falling into+-- it unread.+keepInitialKeys :: Connection -> IO ()+keepInitialKeys conn@Connection{..} = do+    coder <- getCoder conn InitialLevel False+    protector <- getProtector conn InitialLevel+    writeIORef prevInitialKeys $ Just (coder, protector)++getPrevInitialKeys :: Connection -> IO (Maybe (Coder, Protector))+getPrevInitialKeys Connection{..} = readIORef prevInitialKeys  ---------------------------------------------------------------- 

@@ -62,8 +62,8 @@ -- -- A MAX_STREAMS that does not raise it is ignored (RFC 9000, section 4.6). -- One of them reordered on the way, or retransmitted after a newer one has--- gone out, would otherwise lower the limit.  'setTxMaxData' and--- 'setTxMaxStreamData' guard the limits on data the same way.+-- gone out, would otherwise lower the limit.  @setTxMaxData@ and+-- @setTxMaxStreamData@ guard the limits on data the same way. set :: TVar Concurrency -> Int -> IO () set tvar mx = atomically $ modifyTVar' tvar raise   where

@@ -310,6 +310,9 @@     , coders            :: IOArray EncryptionLevel Coder     , coders1RTT        :: IOArray Bool Coder1RTT     , protectors        :: IOArray EncryptionLevel Protector+    , prevInitialKeys   :: IORef (Maybe (Coder, Protector))+    -- ^ The Initial keys for the version the peer addressed us in, kept+    --   over a compatible version change (RFC 9368)     , currentKeyPhase   :: IORef (Bool, PacketNumber)     , negotiated        :: IORef Negotiated     , connMyAuthCIDs    :: IORef AuthCIDs@@ -410,6 +413,7 @@     coders            <- newArray (InitialLevel, HandshakeLevel) initialCoder     coders1RTT        <- newArray (False, True) initialCoder1RTT     protectors        <- newArray (InitialLevel, RTT1Level) initialProtector+    prevInitialKeys   <- newIORef Nothing     currentKeyPhase   <- newIORef (False, 0)     negotiated        <- newIORef initialNegotiated     connMyAuthCIDs    <- newIORef myAuthCIDs@@ -463,9 +467,10 @@ clientConnection ClientConfig{..} verInfo myAuthCIDs peerAuthCIDs =     newConnection Client ccParameters' verInfo myAuthCIDs peerAuthCIDs   where-    ccParameters' = ccParameters-      { maxDatagramFrameSize = ccMaxDatagramFrameSize-      }+    ccParameters' =+        ccParameters+            { maxDatagramFrameSize = ccMaxDatagramFrameSize+            }  serverConnection     :: ServerConfig@@ -486,9 +491,10 @@ serverConnection ServerConfig{..} verInfo myAuthCIDs peerAuthCIDs =     newConnection Server scParameters' verInfo myAuthCIDs peerAuthCIDs   where-    scParameters' = scParameters-      { maxDatagramFrameSize = scMaxDatagramFrameSize-      }+    scParameters' =+        scParameters+            { maxDatagramFrameSize = scMaxDatagramFrameSize+            }  ---------------------------------------------------------------- 

@@ -24,7 +24,6 @@ import Crypto.Cipher.ChaChaPoly1305 (aeadChacha20poly1305Init) import Crypto.Cipher.Types hiding (Cipher, IV) import Crypto.Error (maybeCryptoError)-import Data.IORef (IORef, newIORef, readIORef, writeIORef) import qualified Data.ByteArray as Byte (ByteArrayAccess (..), convert) import qualified Data.ByteString as BS import qualified Data.ByteString.Internal as BS

@@ -11,7 +11,6 @@ import qualified Data.ByteString as BS import qualified Data.ByteString.Short as Short import Network.TLS hiding (Version)-import Network.TLS.Extra.Cipher  import Network.QUIC.Crypto.Nite import Network.QUIC.Crypto.Types

@@ -4,8 +4,8 @@ module Network.QUIC.Handshake where  import qualified Control.Exception as E-import Data.List (intersect) import qualified Data.ByteString.Short as Short+import Data.List (intersect) import qualified Network.TLS as TLS import Network.TLS.QUIC @@ -107,11 +107,13 @@     qlogParamsSet conn (ccParameters conf, "local") -- fixme     handshakeClient' conf' conn myAuthCIDs <$> getVersion conn <*> newHndStateRef   where-    conf' = conf-      { ccParameters = (ccParameters conf)-        { maxDatagramFrameSize = ccMaxDatagramFrameSize conf-        }-      }+    conf' =+        conf+            { ccParameters =+                (ccParameters conf)+                    { maxDatagramFrameSize = ccMaxDatagramFrameSize conf+                    }+            }  handshakeClient'     :: ClientConfig -> Connection -> AuthCIDs -> Version -> IORef HndState -> IO ()@@ -342,6 +344,10 @@         case myVers `intersect` peerVers of             vers@(serverVer : _)                 | clientVer /= serverVer -> do+                    -- Before the keys below replace them: the client keeps+                    -- retransmitting in the version it started with until+                    -- our answer reaches it.+                    keepInitialKeys conn                     setVersionInfo conn $ VersionInfo serverVer vers                     dcid <- getClientDstCID conn                     initializeCoder conn InitialLevel $ initialSecrets serverVer dcid

@@ -21,9 +21,37 @@ ----------------------------------------------------------------  decryptCrypt :: Connection -> Crypt -> EncryptionLevel -> IO (Maybe Plain)-decryptCrypt conn Crypt{..} lvl = do-    cipher <- getCipher conn lvl+decryptCrypt conn crypt lvl = do     protector <- getProtector conn lvl+    mplain <- decryptCryptWith conn crypt lvl protector $ getCoder conn lvl+    case mplain of+        Just _ -> return mplain+        Nothing+            | lvl == InitialLevel -> do+                -- An Initial we cannot read may be one in the version the+                -- peer addressed us in, from before we settled on a+                -- compatible one of our own (RFC 9368).  The peer goes on+                -- retransmitting in that version until our answer reaches+                -- it, and those carry the handshake just as well.  Both the+                -- header protection and the payload keys differ by version,+                -- so the whole of it is tried again, not the payload alone.+                mkeys <- getPrevInitialKeys conn+                case mkeys of+                    Nothing -> return Nothing+                    Just (coder, protector') ->+                        decryptCryptWith conn crypt lvl protector' $+                            \_ -> return coder+            | otherwise -> return Nothing++decryptCryptWith+    :: Connection+    -> Crypt+    -> EncryptionLevel+    -> Protector+    -> (Bool -> IO Coder)+    -> IO (Maybe Plain)+decryptCryptWith conn Crypt{..} lvl protector getCoder' = do+    cipher <- getCipher conn lvl     let proFlags = Flags (cryptPacket `BS.index` 0)         sampleOffset = cryptPktNumOffset + 4         sampleLen = sampleLength cipher@@ -59,7 +87,7 @@             let keyPhase                     | lvl == RTT1Level = flags `testBit` 2                     | otherwise = False-            coder <- getCoder conn lvl keyPhase+            coder <- getCoder' keyPhase             siz <- decrypt coder (decryptBuf conn) ciphertext (AssDat header) pn             let rrMask                     | lvl == RTT1Level = 0x18

@@ -244,16 +244,29 @@             closeConnection conn StreamStateError emsg streamNotCreatedYet _ _ _ = return () --- | Opening a stream for a STREAM frame that found none, unless the stream---   was open once and has been closed since.  'guardStream' has already---   refused one of ours that we have not created yet.+-- | Opening a stream for a frame that found none, unless the stream was open+--   once and has been closed since.  'guardStream' has already refused one of+--   ours that we have not created yet.+--+-- The application is not given the stream here; 'deliverStream' does that,+-- once the frame that opened it has been looked over.  Handed the stream+-- first, the application could answer on it in the moment before a first+-- STREAM frame past the flow control limit closed the connection.  An HTTP/3+-- server did: it sent a response on a stream the peer had never opened, and+-- the peer called that a STREAM_STATE_ERROR, as RFC 9000 Sec 19.8 says to,+-- before our own FLOW_CONTROL_ERROR had reached it. openStream :: Connection -> StreamId -> IO (Maybe Stream) openStream conn sid     | isInitiated conn sid = return Nothing     | otherwise = do         new <- claimPeerStream conn sid-        if new then Just <$> createStream conn sid else return Nothing+        if new then Just <$> addStream conn sid else return Nothing +-- | Handing a stream the peer opened to the application, if it is new to us.+deliverStream :: Connection -> Maybe Stream -> Stream -> IO ()+deliverStream conn found strm =+    when (isNothing found) $ putInput conn $ InpStream strm+ processFrame :: Connection -> EncryptionLevel -> Frame -> IO () processFrame _ _ Padding{} = return () processFrame conn lvl Ping = do@@ -306,6 +319,7 @@     case mstrm of         Nothing -> return ()         Just strm -> do+            deliverStream conn mstrm0 strm             onResetStreamReceived (connHooks conn) strm aerr             -- Before the pseudo FIN below, so that whoever reads it can             -- tell it was not a real one.@@ -394,6 +408,7 @@                         conn                         FlowControlError                         "Flow control error for connection in 0-RTT"+        deliverStream conn mstrm strm processFrame conn RTT1Level (StreamF sid _ [""] False) = do     -- FLOW CONTROL: MAX_STREAMS: recv: rejecting if over my limit     ok <- checkRxMaxStreams conn sid@@ -438,6 +453,7 @@                         conn                         FlowControlError                         "Flow control error for connection in 1-RTT"+        deliverStream conn mstrm strm processFrame conn lvl (MaxData n) = do     when (lvl == InitialLevel || lvl == HandshakeLevel) $         closeConnection conn ProtocolViolation "MAX_DATA in Initial or Handshake"

@@ -1,6 +1,6 @@ cabal-version:      2.0 name:               quic-version:            0.3.9+version:            0.3.10 license:            BSD3 license-file:       LICENSE maintainer:         kazu@iij.ad.jp

@@ -1,3 +1,5 @@+{-# LANGUAGE OverloadedStrings #-}+ module ErrorSpec where  import Control.Concurrent@@ -23,7 +25,14 @@     threadDelay 500000 -- give enough time to the server     return tid   where-    loop conn = forever $ void $ acceptStream conn+    -- Answering on what it is given.  A server that only accepts cannot+    -- catch a stream handed to the application before the frame that opened+    -- it was looked over -- the answer is what reaches the peer, for a+    -- stream the peer never opened, and the peer calls that a+    -- STREAM_STATE_ERROR before our FLOW_CONTROL_ERROR arrives.+    loop conn = forever $ do+        strm <- acceptStream conn+        void $ forkIO $ sendStream strm "x"  teardown :: ThreadId -> IO () teardown = killThread

@@ -340,9 +340,11 @@  ---------------------------------------------------------------- --- Stream 0 is not created internally.  It is assumed that a server--- send CC without sending back Stream 0.  If the server send back any--- data for Stream 0, `streamNotCreatedYet` throws an exception, sigh.+-- Stream 0, which the client has not opened.  The server must answer the+-- offset with FLOW_CONTROL_ERROR and nothing else: anything it sends back on+-- stream 0 is, to a client that never opened it, a STREAM_STATE_ERROR+-- (RFC 9000 Sec 19.8), and the client closes the connection before the error+-- we are waiting for arrives. largeOffset :: EncryptionLevel -> Plain -> Plain largeOffset lvl plain     | lvl == RTT1Level = plain{plainFrames = fake : plainFrames plain}