diff --git a/ChangeLog.md b/ChangeLog.md --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,5 +1,80 @@ # ChangeLog +## 0.3.12 + +A server that looked frozen, a Stateless Reset half the peers threw away, +two windows that leaked, and the AEAD limits. + +* Tell the peer before waiting for the application. The protocol threads + and the application run under one `concurrently_`; when the protocol + threads failed it cancelled the application and then waited for it, + under `uninterruptibleMask_`, for as long as it took to unwind, and the + CONNECTION_CLOSE waited with it. A peer told nothing waits out its own + idle timeout, so from the outside the server had frozen at the + handshake. Seen against mighty, where a transport error the server + raised correctly never reached the client at all; it is said between + the two now, which is the one place it can be said. + [#141](https://github.com/kazu-yamamoto/quic/pull/141) + +* Set the bit RFC 9000 fixes in a Stateless Reset. Sec 10.3 fixes the + first two bits at 01; the first byte was a random seven bits, so the + QUIC Bit was clear in half of them. A peer that has not asked for that + bit to be greased (RFC 9287) drops such a packet before anything looks + for a token in it -- and a Stateless Reset answers a packet we have no + connection for, so whether the peer asked is exactly what we cannot + know. Half went unheard, and the peer went on talking to a connection + that was gone until its idle timeout. + [#137](https://github.com/kazu-yamamoto/quic/pull/137) + +* Tell the peer when a connection ends of something in here. `closure` + turned four exceptions into a CONNECTION_CLOSE and rethrew the rest, so + a connection that ended of anything else -- the application throwing, + StreamIsClosed, an IOException -- ended in silence. Those now end with + an INTERNAL_ERROR. An idle timeout, a peer that has already closed, + and an asynchronous exception stay silent, as RFC 9000 Sec 10.1 and + 10.2 have them. + [#140](https://github.com/kazu-yamamoto/quic/pull/140) + +* Count what the application never reads against the connection's window. + It moved in `recvStream` and nowhere else, so octets an application + left behind were counted as received and never as consumed, and the + window we advertise stayed that much smaller for the rest of the + connection. A server answering a request without reading its body is + the ordinary case, and it paid for that body until the connection + ended. + [#142](https://github.com/kazu-yamamoto/quic/pull/142) + +* The AEAD limits of RFC 9001 Sec 6.6, neither of which was kept. + AEAD_LIMIT_REACHED was in the error table and nothing raised it. + Packets that fail authentication are counted now, across all keys, and + the connection closes past the integrity limit -- 2^52 for the AES-GCM + ciphers, 2^36 for ChaCha20-Poly1305. Packets each key protects are + counted too, and the connection closes past the confidentiality limit, + 2^23 under the AES-GCM ciphers. **Starting a key update instead of + closing is the better answer to the second and is not here**: the key + state holds one phase and the packet number the peer changed it at, + which is what the responding side needs and not what an initiating one + does. + [#145](https://github.com/kazu-yamamoto/quic/pull/145), + [#147](https://github.com/kazu-yamamoto/quic/pull/147) + +* Refuse a RETIRE_CONNECTION_ID we never issued. RFC 9000 Sec 19.16 + makes a sequence number greater than any we have sent a + PROTOCOL_VIOLATION; ours looked it up, found nothing and went on. One + we did send and have already retired stays ignored, since the frame may + simply have been sent twice. + [#144](https://github.com/kazu-yamamoto/quic/pull/144) + +* Say which thread ended a server connection. Six run under nested + `concurrently_`, which cancels the rest as soon as one fails; only the + sender and the receiver said why they ended, so a failure in one of the + other four left a log of two cancelled threads and no reason anywhere. + That is what made the frozen server above so hard to find. The other + half of it -- `runServer` discarding every message handed to its + `debugLog` -- went out in 0.3.11 unmentioned. + [#138](https://github.com/kazu-yamamoto/quic/pull/138), + [#139](https://github.com/kazu-yamamoto/quic/pull/139) + ## 0.3.11 A security fix, two things RFC 9000 asks of a receiver that were not diff --git a/Network/QUIC/Closer.hs b/Network/QUIC/Closer.hs --- a/Network/QUIC/Closer.hs +++ b/Network/QUIC/Closer.hs @@ -35,10 +35,45 @@ E.throwIO $ ApplicationProtocolErrorIsSent err desc | Just (VerNego vers) <- E.fromException se = do E.throwIO $ NextVersion vers - | otherwise = E.throwIO se -- including asynchronous exceptions + -- Nothing to say: the connection is over for a reason the peer knows at + -- least as well as we do. An idle timeout is closed in silence (RFC 9000 + -- Sec 10.1); the rest are the peer having closed, or told us to stop. + | isOver se = E.throwIO se + -- Asynchronous, so we are the ones being taken down, and whoever is doing + -- it says what happens next. 'run' has 'sendFinal' for its own ending. + | isAsyncException se = E.throwIO se + -- Anything else went wrong in here. RFC 9000 Sec 10.2: an endpoint that + -- ends a connection sends CONNECTION_CLOSE. Without one the peer is left + -- talking to a connection that is gone until its own idle timeout, or + -- until a Stateless Reset reaches it -- which, for a server, is a second + -- away, since the dispatcher holds a dead connection's IDs that long. + -- + -- The reason phrase says nothing of what it was. It goes to the peer, + -- and what went wrong in here is our business; the debug log has it. + | otherwise = do + closure' conn ldcc $ ConnectionClose InternalError 0 "internal error" + E.throwIO se + where + isOver e = case E.fromException e of + Just (ConnectionIsClosed _) -> True + Just (ConnectionIsTimeout _) -> True + Just ConnectionIsReset -> True + Just (TransportErrorIsReceived _ _) -> True + Just (ApplicationProtocolErrorIsReceived _ _) -> True + _ -> False +-- | Telling the peer the connection is over, once. +-- +-- Once, because it is said at the first place that knows: where the protocol +-- threads have finished and the application has not. What comes here after +-- that is the same ending on its way out, and the peer has heard it. closure' :: Connection -> LDCC -> Frame -> IO () closure' conn ldcc frame = do + already <- atomicModifyIORef' (connCloseSent conn) $ \b -> (True, b) + unless already $ closure'' conn ldcc frame + +closure'' :: Connection -> LDCC -> Frame -> IO () +closure'' conn ldcc frame = do sock <- getSocket conn peersa <- peerSockAddr <$> getPathInfo conn connected <- getSockConnected conn diff --git a/Network/QUIC/Connection/Migration.hs b/Network/QUIC/Connection/Migration.hs --- a/Network/QUIC/Connection/Migration.hs +++ b/Network/QUIC/Connection/Migration.hs @@ -15,6 +15,7 @@ setPeerCIDAndRetireCIDs, retirePeerCID, retireMyCID, + isMyCIDSeqNumIssued, addPeerCID, waitPeerCID, choosePeerCIDForPrivacy, @@ -220,6 +221,17 @@ Just ncidinfo -> (set ncidinfo False db, True) -- | Receiving RetireConnectionID +-- | Whether the sequence number is one we have given the peer. +-- +-- RFC 9000 Sec 19.16: "Receipt of a RETIRE_CONNECTION_ID frame containing a +-- sequence number greater than any previously sent to the peer MUST be +-- treated as a connection error of type PROTOCOL_VIOLATION." One we have +-- given and already retired is not that -- the frame may simply have been +-- sent twice -- so it is the ones past everything we have given that are +-- refused. +isMyCIDSeqNumIssued :: Connection -> Int -> IO Bool +isMyCIDSeqNumIssued Connection{..} n = (n <) . nextSeqNum <$> readIORef myCIDDB + retireMyCID :: Connection -> Int -> IO (Maybe CIDInfo) retireMyCID Connection{..} n = atomicModifyIORef' myCIDDB $ del' n diff --git a/Network/QUIC/Connection/Types.hs b/Network/QUIC/Connection/Types.hs --- a/Network/QUIC/Connection/Types.hs +++ b/Network/QUIC/Connection/Types.hs @@ -322,6 +322,12 @@ , encryptRes :: SizedBuffer , decryptBuf :: Buffer , connResources :: IORef (IO ()) + , connCloseSent :: IORef Bool + -- ^ Whether the peer has been told the connection is over + , connAuthFailures :: IORef Int + -- ^ Packets that have failed authentication, over the whole connection + , connKeyPackets :: IORef Int + -- ^ Packets the 1-RTT key in use has protected , -- Recovery connLDCC :: LDCC } @@ -424,6 +430,9 @@ let encryptRes = SizedBuffer encryptBuf bufsiz -- used sender decryptBuf <- mallocBytes bufsiz -- used receiver connResources <- newIORef (free encodeBuf >> free encryptBuf >> free decryptBuf) + connCloseSent <- newIORef False + connAuthFailures <- newIORef 0 + connKeyPackets <- newIORef 0 -- Recovery let put x = atomically $ writeTQueue outputQ $ OutRetrans x connLDCC <- newLDCC connState connQLog put diff --git a/Network/QUIC/Crypto/Utils.hs b/Network/QUIC/Crypto/Utils.hs --- a/Network/QUIC/Crypto/Utils.hs +++ b/Network/QUIC/Crypto/Utils.hs @@ -3,6 +3,8 @@ module Network.QUIC.Crypto.Utils ( tagLength, sampleLength, + integrityLimit, + confidentialityLimit, bsXOR, calculateIntegrityTag, ) where @@ -11,6 +13,7 @@ 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 @@ -57,3 +60,41 @@ nonce Version1 = Nonce "\x46\x15\x99\xd3\x5d\x63\x2b\xf2\x23\x98\x25\xbb" nonce Version2 = Nonce "\xd8\x69\x69\xbc\x2d\x7c\x6d\x99\x90\xef\xb0\x4a" nonce _ = Nonce "not supported" + +---------------------------------------------------------------- + +-- | How many packets may fail authentication on a connection before the AEAD +-- is no longer trusted to tell a forgery from the real thing. +-- +-- RFC 9001 Sec 6.6: "endpoints MUST count the number of received packets that +-- fail authentication during the lifetime of a connection. If the total +-- number of received packets that fail authentication within the connection, +-- across all keys, exceeds the integrity limit for the selected AEAD, the +-- endpoint MUST immediately close the connection with a connection error of +-- type AEAD_LIMIT_REACHED and not process any more packets." +-- +-- The same section gives the numbers: 2^52 for the AES-GCM ciphers and 2^36 +-- for ChaCha20-Poly1305. An AEAD we do not know gets the smaller of the two, +-- which is the safe way to be wrong. +integrityLimit :: Cipher -> Int +integrityLimit cipher + | cipher == cipher13_AES_128_GCM_SHA256 = 2 ^ (52 :: Int) + | cipher == cipher13_AES_256_GCM_SHA384 = 2 ^ (52 :: Int) + | otherwise = 2 ^ (36 :: Int) + +-- | How many packets one key may protect before the AEAD is no longer +-- trusted to keep what it has protected secret. +-- +-- RFC 9001 Sec 6.6: "Endpoints MUST count the number of encrypted packets for +-- each set of keys. If the total number of encrypted packets with the same +-- key exceeds the confidentiality limit for the selected AEAD, the endpoint +-- MUST stop using those keys." +-- +-- The same section gives 2^23 for the AES-GCM ciphers, and for +-- ChaCha20-Poly1305 a number "greater than the number of possible packets +-- (2^62) and so can be disregarded". An AEAD we do not know gets the +-- smaller, which is the safe way to be wrong. +confidentialityLimit :: Cipher -> Int +confidentialityLimit cipher + | cipher == cipher13_CHACHA20_POLY1305_SHA256 = 2 ^ (62 :: Int) + | otherwise = 2 ^ (23 :: Int) diff --git a/Network/QUIC/IO.hs b/Network/QUIC/IO.hs --- a/Network/QUIC/IO.hs +++ b/Network/QUIC/IO.hs @@ -187,6 +187,19 @@ $ do -- FLOW CONTROL: MAX_STREAMS: recv: announcing my limit properly checkMaxStreams Unidirectional + -- FLOW CONTROL: MAX_DATA: recv: the octets of this stream the + -- application will never read. Left uncounted, the window we + -- advertise stays that much smaller for the rest of the connection: + -- a server that answers a request without reading its body pays for + -- that body until the connection ends, and enough of them leave the + -- peer blocked by octets nobody is waiting for. + unread <- takeRxUnread s + when (unread > 0) $ do + mx <- updateFlowRx conn unread + forM_ mx $ \newMax -> do + sendFrames conn RTT1Level [MaxData newMax] + fire conn (Microseconds 50000) $ + sendFrames conn RTT1Level [MaxData newMax] where conn = streamConnection s sid = streamId s diff --git a/Network/QUIC/Packet.hs b/Network/QUIC/Packet.hs --- a/Network/QUIC/Packet.hs +++ b/Network/QUIC/Packet.hs @@ -3,6 +3,7 @@ encodeVersionNegotiationPacket, encodeRetryPacket, encodePlainPacket, + makeStatelessReset, -- * Decode decodePacket, diff --git a/Network/QUIC/Packet/Encode.hs b/Network/QUIC/Packet/Encode.hs --- a/Network/QUIC/Packet/Encode.hs +++ b/Network/QUIC/Packet/Encode.hs @@ -1,11 +1,20 @@ +{-# LANGUAGE CPP #-} + module Network.QUIC.Packet.Encode ( -- encodePacket encodeVersionNegotiationPacket, encodeRetryPacket, encodePlainPacket, + makeStatelessReset, ) where import qualified Data.ByteString as BS +import System.Random (getStdRandom, randomRIO) +#if MIN_VERSION_random(1,3,0) +import System.Random (uniformByteString) +#else +import System.Random (genByteString) +#endif import Foreign.ForeignPtr import Foreign.Ptr import Foreign.Storable (peek) @@ -309,3 +318,29 @@ mkBS ptr siz = do fptr <- newForeignPtr_ ptr return $ PS fptr 0 siz + +---------------------------------------------------------------- + +-- | A Stateless Reset carrying the given token: 1280 octets, of which the +-- last sixteen are the token and the rest is noise. +-- +-- RFC 9000 Sec 10.3 fixes the first two bits at 01 -- header form 0 and the +-- QUIC Bit set -- so that it cannot be told from an ordinary short header +-- packet. They used to be one random bit and one fixed at 0, so the QUIC Bit +-- was clear in half of them, and a peer that has not asked for that bit to be +-- greased (RFC 9287) discards such a packet before anything looks for a token +-- in it. We are answering a packet we have no connection for, so whether the +-- peer asked is exactly what we cannot know. +-- +-- 1280 octets is under three times the 428 the caller insists on having +-- received, which is what Sec 10.3 allows to be sent in answer. +makeStatelessReset :: StatelessResetToken -> IO ByteString +makeStatelessReset srt = do + r <- randomRIO (0, 255) + let flag = 0x40 .|. (r .&. 0x3f) +#if MIN_VERSION_random(1,3,0) + body <- getStdRandom $ uniformByteString 1263 +#else + body <- getStdRandom $ genByteString 1263 +#endif + return $ BS.concat [BS.singleton flag, body, fromStatelessResetToken srt] diff --git a/Network/QUIC/Receiver.hs b/Network/QUIC/Receiver.hs --- a/Network/QUIC/Receiver.hs +++ b/Network/QUIC/Receiver.hs @@ -203,6 +203,18 @@ qlogDebug conn $ Debug "ping for speedup" sendFrames conn lvl [Ping] Nothing -> do + -- RFC 9001 Sec 6.6: "endpoints MUST count the number of received + -- packets that fail authentication during the lifetime of a + -- connection. If the total number ... exceeds the integrity + -- limit for the selected AEAD, the endpoint MUST immediately + -- close the connection with a connection error of type + -- AEAD_LIMIT_REACHED and not process any more packets." That + -- limit is what stands between a peer and as many guesses at our + -- keys as it cares to send. + failures <- atomicModifyIORef' (connAuthFailures conn) $ \n -> (n + 1, n + 1) + cipher <- getCipher conn RTT1Level + when (failures > integrityLimit cipher) $ + closeConnection conn AeadLimitReached "too many packets failed authentication" qlogDropped conn (hdr, "decrypt_error" :: String) connDebugLog conn $ "debug: cannot decrypt: " @@ -591,6 +603,14 @@ when ng $ closeConnection conn ConnectionIdLimitError "NEW_CONNECTION_ID limit error" processFrame conn RTT1Level (RetireConnectionID sn) = do + -- RFC 9000 Sec 19.16: "Receipt of a RETIRE_CONNECTION_ID frame + -- containing a sequence number greater than any previously sent to the + -- peer MUST be treated as a connection error of type + -- PROTOCOL_VIOLATION." One we have sent and already retired is not + -- that, and is ignored below. + issued <- isMyCIDSeqNumIssued conn sn + unless issued $ + closeConnection conn ProtocolViolation "RETIRE_CONNECTION_ID never issued" -- FIXME: CID is necessary here -- The sequence number specified in a RETIRE_CONNECTION_ID frame -- MUST NOT refer to the Destination Connection ID field of the diff --git a/Network/QUIC/Sender.hs b/Network/QUIC/Sender.hs --- a/Network/QUIC/Sender.hs +++ b/Network/QUIC/Sender.hs @@ -15,6 +15,7 @@ import Network.QUIC.Config import Network.QUIC.Connection import Network.QUIC.Connector +import Network.QUIC.Crypto (confidentialityLimit) import Network.QUIC.Exception import Network.QUIC.Imports import Network.QUIC.Packet @@ -111,6 +112,29 @@ let sentPacket = sentPacket0{spTimeSent = now} qlogSent conn sentPacket now onPacketSent ldcc sentPacket + -- RFC 9001 Sec 6.6: "Endpoints MUST count the number of + -- encrypted packets for each set of keys. If the total + -- number of encrypted packets with the same key exceeds + -- the confidentiality limit for the selected AEAD, the + -- endpoint MUST stop using those keys." The way out it + -- gives is a key update, which nothing here starts, so + -- what is left is the other: "If a key update is not + -- possible ... the endpoint MUST stop using the + -- connection", and closing with AEAD_LIMIT_REACHED is how + -- that section recommends doing it. + let n1rtt = + length $ + filter ((== RTT1Level) . spEncryptionLevel) sentPackets + when (n1rtt > 0) $ do + protected <- + atomicModifyIORef' (connKeyPackets conn) $ + \n -> (n + n1rtt, n + n1rtt) + cipher <- getCipher conn RTT1Level + when (protected > confidentialityLimit cipher) $ + closeConnection + conn + AeadLimitReached + "the key has protected too many packets" buildPackets _ _ _ [] _ = error "sendPacket: buildPackets" buildPackets buf bufsiz siz [spkt] build0 = do let pkt = spPlainPacket spkt diff --git a/Network/QUIC/Server/Reader.hs b/Network/QUIC/Server/Reader.hs --- a/Network/QUIC/Server/Reader.hs +++ b/Network/QUIC/Server/Reader.hs @@ -395,7 +395,8 @@ isRetryTokenValid _ = return False sendRetry = do newdCID <- newCID - retryToken <- generateRetryToken peerVer scTicketLifetime newdCID sCID dCID peersa + retryToken <- + generateRetryToken peerVer scTicketLifetime newdCID sCID dCID peersa mnewtoken <- timeout (Microseconds 100000) "sendRetry" $ encryptToken tokenMgr retryToken case mnewtoken of @@ -441,14 +442,7 @@ srRate <- getRate statelessResetRate -- fixme: hard coding when (srRate < statelessResetLimit) $ do - flag <- randomRIO (0, 127) -#if MIN_VERSION_random(1,3,0) - body <- getStdRandom $ uniformByteString 1263 -#else - body <- getStdRandom $ genByteString 1263 -#endif - let srt = genStatelessReset dCID - statelessReset = BS.concat [BS.singleton flag, body, fromStatelessResetToken srt] + statelessReset <- makeStatelessReset $ genStatelessReset dCID send' statelessReset logAction $ "Stateless reset is sent to " <> bhow peersa Just conn -> do diff --git a/Network/QUIC/Server/Run.hs b/Network/QUIC/Server/Run.hs --- a/Network/QUIC/Server/Run.hs +++ b/Network/QUIC/Server/Run.hs @@ -119,19 +119,61 @@ server0 conn mainDone conn ldcc = connLDCC conn - let s1 = labelMe "handshaker" >> handshaker + -- Each says why it ended, as the sender and the receiver + -- already did. 'concurrently_' cancels the others as soon as one + -- of them fails, so the one that did not end in AsyncCancelled is + -- the one that ended the connection -- and it was these four that + -- said nothing, leaving a log of two cancelled threads and no + -- reason anywhere. + -- + -- Being cancelled is not worth a line: the sender and the + -- receiver already say when the connection is taken down from + -- outside, and everything else follows them. + let named nm act = + act `E.catch` \e -> do + case E.fromException e of + Just AsyncCancelled -> return () + Nothing -> connDebugLog conn $ "debug: " <> nm <> ": " <> bhow e + E.throwIO e + let s1 = labelMe "handshaker" >> named "handshaker" handshaker s2 = labelMe "sender" >> sender conn s3 = labelMe "receiver" >> receiver conn - s4 = labelMe "resender" >> resender ldcc - s5 = labelMe "ldccTimer" >> ldccTimer ldcc - s6 = labelMe "QUIC server" >> server + s4 = labelMe "resender" >> named "resender" (resender ldcc) + s5 = labelMe "ldccTimer" >> named "ldccTimer" (ldccTimer ldcc) + s6 = labelMe "QUIC server" >> named "server" server c1 = labelMe "concurrently1" >> concurrently_ s1 s2 c2 = labelMe "concurrently2" >> concurrently_ c1 s3 c3 = labelMe "concurrently3" >> concurrently_ c2 s4 c4 = labelMe "concurrently4" >> concurrently_ c3 s5 + -- Telling the peer here, and not below, because here is the + -- one place where it can be done at all: the nested + -- 'concurrently_'s above have waited for every protocol + -- thread, so the encode buffer is ours alone -- it is shared + -- with the sender -- and the application is still running, so + -- nothing has begun waiting for it yet. + -- + -- Below, after 'runThreads', is too late. The + -- 'concurrently_' on this line cancels the application when + -- the protocol threads fail and then waits for it, under + -- 'uninterruptibleMask_', for as long as it takes to unwind. + -- The CONNECTION_CLOSE waited with it, and a peer told + -- nothing waits out its own idle timeout: seen against an + -- HTTP/3 server whose application was slow to die just as the + -- handshake finished, where a transport error the server had + -- raised correctly never reached the client at all. + -- + -- 'closure' is said once; it rethrows, and the call below + -- finds the peer already told. + tellThenRaise act = + act `E.catch` \(e :: E.SomeException) -> do + sendFinal conn + setConnectionClosed conn + closure conn ldcc (Left e) c5 = labelMe "concurrently5" - >> concurrently_ (c4 `E.catch` \(_ :: InternalControl) -> return ()) s6 + >> concurrently_ + (tellThenRaise (c4 `E.catch` \(_ :: InternalControl) -> return ())) + s6 runThreads = c5 ex <- E.try runThreads sendFinal conn diff --git a/Network/QUIC/Stream.hs b/Network/QUIC/Stream.hs --- a/Network/QUIC/Stream.hs +++ b/Network/QUIC/Stream.hs @@ -28,6 +28,7 @@ noteRxFinalSize, addRxCounted, takeRxUncounted, + takeRxUnread, readStreamFlowTx, addTxStreamData, setTxMaxStreamData, diff --git a/Network/QUIC/Stream/Misc.hs b/Network/QUIC/Stream/Misc.hs --- a/Network/QUIC/Stream/Misc.hs +++ b/Network/QUIC/Stream/Misc.hs @@ -17,6 +17,7 @@ noteRxFinalSize, addRxCounted, takeRxUncounted, + takeRxUnread, -- readStreamFlowTx, addTxStreamData, @@ -194,5 +195,28 @@ where take' b@RxBounds{..} = case rxFinal of Just f - | f > rxCounted -> (b{rxCounted = f}, f - rxCounted) + | f > rxCounted -> + (b{rxCounted = f, rxCredited = rxCredited + f - rxCounted}, f - rxCounted) _ -> (b, 0) + +-- | The octets of the stream the connection's flow controller has counted +-- and the application will never read. +-- +-- What it read it has already counted itself, in 'recvStream'. What it +-- leaves -- a request whose body a server answers without reading, say -- +-- was counted as received and is never counted as consumed, so the window we +-- advertise stays that much smaller for the rest of the connection. The peer +-- is then blocked by octets nobody is waiting for. +-- +-- Answered once for each octet: what is answered here is counted as +-- credited. +takeRxUnread :: Stream -> IO Int +takeRxUnread Stream{..} = do + consumed <- rxfConsumed <$> readIORef streamFlowRx + atomicModifyIORef' streamRxBounds $ take' consumed + where + take' consumed b@RxBounds{..} + | owed > 0 = (b{rxCredited = rxCredited + owed}, owed) + | otherwise = (b, 0) + where + owed = rxCounted - consumed - rxCredited diff --git a/Network/QUIC/Stream/Types.hs b/Network/QUIC/Stream/Types.hs --- a/Network/QUIC/Stream/Types.hs +++ b/Network/QUIC/Stream/Types.hs @@ -60,11 +60,14 @@ -- ^ The largest offset plus length seen for it , rxFinal :: Maybe Int -- ^ Its final size, once that is known + , rxCredited :: Int + -- ^ Octets counted against the connection's window on the stream's + -- behalf, over and above what the application has read } deriving (Eq, Show) emptyRxBounds :: RxBounds -emptyRxBounds = RxBounds 0 0 Nothing +emptyRxBounds = RxBounds 0 0 Nothing 0 instance Show Stream where show s = show $ streamId s diff --git a/quic.cabal b/quic.cabal --- a/quic.cabal +++ b/quic.cabal @@ -1,6 +1,6 @@ cabal-version: 2.0 name: quic -version: 0.3.11 +version: 0.3.12 license: BSD3 license-file: LICENSE maintainer: kazu@iij.ad.jp @@ -237,6 +237,7 @@ ParametersSpec QLoggerSpec RecoverySpec + ResetSpec TLSSpec TokenSpec TransportError diff --git a/test/IOSpec.hs b/test/IOSpec.hs --- a/test/IOSpec.hs +++ b/test/IOSpec.hs @@ -1,4 +1,5 @@ {-# LANGUAGE OverloadedStrings #-} +{-# LANGUAGE ScopedTypeVariables #-} module IOSpec where @@ -139,8 +140,15 @@ it "refuses stream data beyond the final size" $ do withPipe (DropClientPacket []) $ testFinalSize cc sc waitS [StreamF 0 0 ["ab"] True, StreamF 0 2 ["cd"] False] + it "counts what the application never reads against the connection" $ do + withPipe (DropClientPacket []) $ testCloseWithoutReading cc sc waitS it "counts a reset stream's final size against the connection" $ do withPipe (DropClientPacket []) $ testResetCountsAgainstTheWindow cc sc waitS + describe "closing" $ do + it "tells the peer when the application gives up" $ do + withPipe (DropClientPacket []) $ testServerThrows cc sc waitS + it "tells the peer before waiting for the application" $ do + withPipe (DropClientPacket []) $ testSlowApp cc sc waitS describe "port handover" $ do it "ignores a leftover datagram from the connection that just closed" $ withPipeStray (Randomly 20) $ @@ -688,3 +696,82 @@ client = do waitS C.run cc $ \_conn -> threadDelay 1000000 + +-- | RFC 9000, section 10.2: an endpoint that ends a connection sends +-- CONNECTION_CLOSE. Anything thrown in here that is not already on its way +-- to the peer used to end the connection in silence, leaving the peer to +-- wait out its own idle timeout. +testServerThrows :: C.ClientConfig -> ServerConfig -> IO () -> IO () +testServerThrows cc sc waitS = + withAsync server $ \_ -> client `shouldThrow` internalError + where + server = run sc $ \_conn -> E.throwIO $ userError "the application gave up" + client = do + waitS + C.run cc $ \conn -> do + strm <- stream conn + sendStream strm "ping" + void $ recvStream strm 1024 + +internalError :: QUICException -> Bool +internalError (TransportErrorIsReceived InternalError _) = True +internalError _ = False + +-- | The peer hears of a transport error at once, however long the +-- application takes to unwind. +-- +-- The protocol threads and the application run under one 'concurrently_', +-- which cancels the application when they fail and then waits for it, under +-- 'uninterruptibleMask_'. Said after that, the CONNECTION_CLOSE waits with +-- it, and a peer told nothing waits out its own idle timeout. Here the +-- application sits for three seconds after it is cancelled and the client +-- allows one and a half. +testSlowApp :: C.ClientConfig -> ServerConfig -> IO () -> IO () +testSlowApp cc0 sc waitS = + withAsync server $ \_ -> client `shouldThrow` frameEncodingError + where + server = run sc $ \conn -> + forever (void $ acceptStream conn) `E.catch` \(e :: E.SomeException) -> do + threadDelay 3000000 + E.throwIO e + cc = cc0{ccHooks = (ccHooks cc0){onPlainCreated = inject}} + inject lvl plain + | lvl == RTT1Level = + plain + { plainFrames = MaxStreams Bidirectional (2 ^ (61 :: Int)) : plainFrames plain + } + | otherwise = plain + client = do + waitS + C.run cc $ \_conn -> threadDelay 1500000 + +frameEncodingError :: QUICException -> Bool +frameEncodingError (TransportErrorIsReceived FrameEncodingError _) = True +frameEncodingError _ = False + +-- | A server that answers without reading the body still gives the octets +-- back to the connection's window. +-- +-- They were counted as received and, read by nobody, were never counted as +-- consumed, so the window we advertise stayed that much smaller for the rest +-- of the connection. Here twenty streams of four kilobytes go to a server +-- that reads one octet of each, against a window of thirty-two: uncounted, +-- the client is blocked before it is halfway through. +testCloseWithoutReading :: C.ClientConfig -> ServerConfig -> IO () -> IO () +testCloseWithoutReading cc sc0 waitS = + withAsync server $ \_ -> client + where + sc = sc0{scParameters = (scParameters sc0){initialMaxData = 32768}} + server = run sc $ \conn -> forever $ do + strm <- acceptStream conn + _ <- recvStream strm 1 + closeStream strm + client = do + waitS + r <- Timeout.timeout 5000000 $ C.run cc $ \conn -> + replicateM_ 20 $ do + strm <- stream conn + sendStream strm $ BS.replicate 4096 0 + shutdownStream strm + closeStream strm + r `shouldBe` Just () diff --git a/test/ResetSpec.hs b/test/ResetSpec.hs new file mode 100644 --- /dev/null +++ b/test/ResetSpec.hs @@ -0,0 +1,42 @@ +{-# LANGUAGE OverloadedStrings #-} + +module ResetSpec where + +import Data.Bits ((.&.)) +import qualified Data.ByteString as BS +import Data.List (nub) +import Test.Hspec + +import Network.QUIC.Internal + +spec :: Spec +spec = describe "a stateless reset" $ do + it "has the two bits RFC 9000 fixes at 01" $ do + flags <- mapM (const firstByte) [1 .. 200 :: Int] + -- Header form 0 and the QUIC Bit set. A peer that has not asked for + -- that bit to be greased discards anything else before it looks for a + -- token, and we are answering a packet we have no connection for, so + -- whether it asked is what we cannot know. + all (\w -> w .&. 0xc0 == 0x40) flags `shouldBe` True + it "does not always send the same bits" $ do + flags <- mapM (const firstByte) [1 .. 200 :: Int] + length (nub flags) `shouldSatisfy` (> 1) + it "is read as a short header packet by a peer that greases nothing" $ do + -- Fifty of them: with one random bit wrong, one reset gets through + -- half the time. True here is the peer refusing a cleared QUIC Bit, + -- which is what a peer that has not asked for greasing does. + bss <- mapM (const $ makeStatelessReset token) [1 .. 50 :: Int] + pkts <- concat <$> mapM (`decodePackets` True) bss + filter broken pkts `shouldBe` [] + it "is 1280 octets, under three times the 428 it answers" $ do + bs <- makeStatelessReset token + BS.length bs `shouldBe` 1280 + BS.length bs `shouldSatisfy` (< 3 * 428) + it "ends with the token" $ do + bs <- makeStatelessReset token + BS.drop (BS.length bs - 16) bs `shouldBe` fromStatelessResetToken token + where + firstByte = BS.head <$> makeStatelessReset token + token = StatelessResetToken "0123456789abcdef" + broken (PacketIB BrokenPacket _) = True + broken _ = False diff --git a/test/TLSSpec.hs b/test/TLSSpec.hs --- a/test/TLSSpec.hs +++ b/test/TLSSpec.hs @@ -25,6 +25,30 @@ spec :: Spec spec = do + describe "the AEAD confidentiality limit" $ do + -- RFC 9001 Sec 6.6 again: past this many packets under one key the + -- AEAD is no longer trusted to keep what it has protected secret. + it "is 2^23 for the AES-GCM ciphers" $ do + confidentialityLimit cipher13_AES_128_GCM_SHA256 `shouldBe` 2 ^ (23 :: Int) + confidentialityLimit cipher13_AES_256_GCM_SHA384 `shouldBe` 2 ^ (23 :: Int) + it "is past counting for ChaCha20-Poly1305" $ + -- "greater than the number of possible packets (2^62) and so can + -- be disregarded" + confidentialityLimit cipher13_CHACHA20_POLY1305_SHA256 + `shouldBe` 2 ^ (62 :: Int) + it "is the smaller for an AEAD we do not know" $ + confidentialityLimit cipher13_AES_128_CCM_SHA256 `shouldBe` 2 ^ (23 :: Int) + describe "the AEAD integrity limit" $ do + -- RFC 9001 Sec 6.6 gives these, and the whole of the rule is in + -- them: past this many packets that fail authentication the AEAD is + -- no longer trusted to tell a forgery from the real thing. + it "is 2^52 for the AES-GCM ciphers" $ do + integrityLimit cipher13_AES_128_GCM_SHA256 `shouldBe` 2 ^ (52 :: Int) + integrityLimit cipher13_AES_256_GCM_SHA384 `shouldBe` 2 ^ (52 :: Int) + it "is 2^36 for ChaCha20-Poly1305" $ do + integrityLimit cipher13_CHACHA20_POLY1305_SHA256 `shouldBe` 2 ^ (36 :: Int) + it "is the smaller of the two for an AEAD we do not know" $ do + integrityLimit cipher13_AES_128_CCM_SHA256 `shouldBe` 2 ^ (36 :: Int) describe "server configuration" $ do it "does not request client certificates by default" $ scWantClientCert defaultServerConfig `shouldBe` False diff --git a/test/TransportError.hs b/test/TransportError.hs --- a/test/TransportError.hs +++ b/test/TransportError.hs @@ -138,6 +138,11 @@ let cc = addHook cc0 $ setOnPlainCreated handshakePathChallenge runCnoOp cc ms `shouldThrow` transportError it + "MUST send PROTOCOL_VIOLATION if RETIRE_CONNECTION_ID for a sequence number never issued [Transport 19.16]" + $ \_ -> do + let cc = addHook cc0 $ setOnPlainCreated retireUnissued + runCnoOp cc ms `shouldThrow` transportError + it "MUST send PROTOCOL_VIOLATION if STREAM_DATA_BLOCKED in Handshake is received [Transport 12.4]" $ \_ -> do let cc = addHook cc0 $ setOnPlainCreated handshakeStreamDataBlocked @@ -404,6 +409,16 @@ handshakePathChallenge lvl plain | lvl == HandshakeLevel = plain{plainFrames = PathChallenge (PathData "01234567") : plainFrames plain} + | otherwise = plain + +-- RFC 9000 Sec 19.16: "Receipt of a RETIRE_CONNECTION_ID frame containing a +-- sequence number greater than any previously sent to the peer MUST be +-- treated as a connection error of type PROTOCOL_VIOLATION." No endpoint has +-- given out a hundred thousand connection IDs. +retireUnissued :: EncryptionLevel -> Plain -> Plain +retireUnissued lvl plain + | lvl == RTT1Level = + plain{plainFrames = RetireConnectionID 100000 : plainFrames plain} | otherwise = plain -- RFC 9000 Table 3 has STREAM_DATA_BLOCKED and DATA_BLOCKED in 0-RTT and