diff --git a/ChangeLog.md b/ChangeLog.md new file mode 100644 --- /dev/null +++ b/ChangeLog.md @@ -0,0 +1,28 @@ +# Revision history for h3spec + +## 0.1.14 + +* Using `http3` v0.1.7 and `quic` v0.3.13. +* 27 new test cases, 77 in all; none of the old ones are removed. +* QUIC: CRYPTO_BUFFER_EXCEEDED for CRYPTO data buffered beyond the limit; + FRAME_ENCODING_ERROR for an ACK range below zero; PROTOCOL_VIOLATION + for an ACK of a packet never sent, for STREAM_DATA_BLOCKED and + DATA_BLOCKED in Handshake packets, and for a RETIRE_CONNECTION_ID never + issued; TRANSPORT_PARAMETER_ERROR for a malformed parameter value and + for initial_max_streams_bidi or initial_max_streams_uni over 2^60. +* HTTP/3 and QPACK: H3_STREAM_CREATION_ERROR for a second control, + encoder or decoder stream and for a client's push stream; + H3_CLOSED_CRITICAL_STREAM for a closed encoder or decoder stream and + for an encoder stream ending inside an instruction; + QPACK_DECOMPRESSION_FAILED for a reference to a dynamic table entry + that is not there; a request with a connection-specific field treated + as malformed, one case for each of connection, keep-alive, + proxy-connection, transfer-encoding, upgrade and TE other than + trailers, and TE with trailers accepted; a frame longer than + SETTINGS_MAX_FIELD_SECTION_SIZE not buffered; H3_FRAME_ERROR for a + SETTINGS frame ending mid-parameter; settings read past a reserved + identifier, and a repeated identifier treated as an error. +* The cases that open a second control, encoder or decoder stream, or a + push stream, are pending against a server that lets a client have only + the three unidirectional streams it needs, since such a stream cannot + be opened there at all. diff --git a/HTTP3Error.hs b/HTTP3Error.hs --- a/HTTP3Error.hs +++ b/HTTP3Error.hs @@ -3,16 +3,21 @@ module HTTP3Error ( h3ErrorSpec, + h3DrainSpec, ) where import Control.Concurrent import qualified Control.Exception as E +import Control.Monad (forM_) import Data.ByteString () import qualified Data.ByteString as BS import qualified Data.ByteString.Char8 as C8 +import qualified Data.CaseInsensitive as CI +import Data.IORef import Network.HTTP.Types import qualified Network.HTTP3.Client as H3 import Network.HTTP3.Internal +import Network.QPACK (QDecoderConfig (..), QEncoderConfig (..)) import Network.QPACK.Internal import Network.QUIC import Network.QUIC.Client @@ -46,8 +51,42 @@ threadDelay 100000 return ret -h3ErrorSpec :: ClientConfig -> H3.ClientConfig -> Millisecond -> SpecWith a -h3ErrorSpec qcc cconf ms = do +-- | Like 'runC', but sending a request of our own choosing. +runCReq + :: H3.Request + -> ClientConfig + -> H3.ClientConfig + -> H3.Config + -> Millisecond + -> IO (Maybe ()) +runCReq req qcc cconf conf ms = timeout us $ run qcc $ \conn -> do + info <- getConnectionInfo conn + case alpn info of + Just proto | "hq" `BS.isPrefixOf` proto -> do + waitEstablished conn + E.throwIO $ ApplicationProtocolErrorIsReceived H3InternalError "" + _ -> H3.run conn cconf conf client + where + us = ms * 1000 + client sendRequest _aux = do + ret <- sendRequest req $ \_rsp -> return () + threadDelay 100000 + return ret + +-- | The error cases for an HTTP/3 server. +-- +-- These also make h3spec, which runs them against a server somewhere else. +-- The last argument says how, from what the spec is run with, to read how +-- many requests the server has handed its application. Only a server in the +-- same process can say; given 'Nothing', a test does not check that a +-- request the server refused never got that far. +h3ErrorSpec + :: ClientConfig + -> H3.ClientConfig + -> Millisecond + -> (a -> Maybe (IO Int)) + -> SpecWith a +h3ErrorSpec qcc cconf ms served = do conf0 <- runIO H3.allocSimpleConfig describe "HTTP/3 servers" $ do it @@ -63,11 +102,12 @@ `shouldThrow` applicationProtocolErrorsIn [H3MessageError] it "MUST send H3_MESSAGE_ERROR if mandatory pseudo-header fields are absent [HTTP/3 4.1.3]" - $ \_ -> do + $ \x -> do let conf = addHook conf0 $ setOnHeadersFrameCreated illegalHeader0 qcc' = addQUICHook qcc $ setOnResetStreamReceived $ \_strm aerr -> E.throwIO (ApplicationProtocolErrorIsReceived aerr "") - runC qcc' cconf conf ms - `shouldThrow` applicationProtocolErrorsIn [H3MessageError] + refusedNotServed (served x) $ + runC qcc' cconf conf ms + `shouldThrow` applicationProtocolErrorsIn [H3MessageError] it "MUST send H3_MESSAGE_ERROR if prohibited pseudo-header fields are present[HTTP/3 4.1.3]" $ \_ -> do @@ -82,6 +122,21 @@ qcc' = addQUICHook qcc $ setOnResetStreamReceived $ \_strm aerr -> E.throwIO (ApplicationProtocolErrorIsReceived aerr "") runC qcc' cconf conf ms `shouldThrow` applicationProtocolErrorsIn [H3MessageError] + forM_ connectionSpecific $ \field@(name, _) -> + it + ( "MUST treat a request with " + ++ C8.unpack (CI.original name) + ++ " as malformed [HTTP/3 4.2]" + ) + $ \x -> do + let req = H3.requestNoBody methodGet "/" [field] + qcc' = addQUICHook qcc $ setOnResetStreamReceived $ \_strm aerr -> E.throwIO (ApplicationProtocolErrorIsReceived aerr "") + refusedNotServed (served x) $ + runCReq req qcc' cconf conf0 ms + `shouldThrow` applicationProtocolErrorsIn [H3MessageError] + it "MUST accept TE with trailers [HTTP/3 4.2]" $ \_ -> do + let req = H3.requestNoBody methodGet "/" [("te", "trailers")] + runCReq req qcc cconf conf0 ms `shouldReturn` Just () it "MUST send H3_MISSING_SETTINGS if the first control frame is not SETTINGS [HTTP/3 6.2.1]" $ \_ -> do @@ -118,6 +173,24 @@ runC qcc cconf conf ms `shouldThrow` applicationProtocolErrorsIn [H3SettingsError] it + "MUST send H3_FRAME_ERROR if a SETTINGS frame stops mid-parameter [HTTP/3 7.1]" + $ \_ -> do + let conf = addHook conf0 $ setOnControlFrameCreated truncatedSettings + runC qcc cconf conf ms + `shouldThrow` applicationProtocolErrorsIn [H3FrameError] + it + "MUST NOT stop reading settings at a reserved identifier [HTTP/3 7.2.4.1]" + $ \_ -> do + let conf = addHook conf0 $ setOnControlFrameCreated greaseThenHttp2Setting + runC qcc cconf conf ms + `shouldThrow` applicationProtocolErrorsIn [H3SettingsError] + it + "treats a repeated setting identifier as an error whatever its value [HTTP/3 7.2.4.1]" + $ \_ -> do + let conf = addHook conf0 $ setOnControlFrameCreated duplicateLargeSetting + runC qcc cconf conf ms + `shouldThrow` applicationProtocolErrorsIn [H3SettingsError] + it "MUST send H3_FRAME_UNEXPECTED if CANCEL_PUSH is received in a request stream [HTTP/3 7.2.5]" $ \_ -> do let conf = addHook conf0 $ setOnHeadersFrameCreated requestCancelPush @@ -130,6 +203,18 @@ runC qcc cconf conf ms `shouldThrow` applicationProtocolErrorsIn [QpackDecompressionFailed] it + "MUST send QPACK_DECOMPRESSION_FAILED if a field line references a dynamic table entry that is not there [QPACK 2.1.2]" + $ \_ -> do + let conf = addHook conf0 $ setOnHeadersFrameCreated illegalHeader5 + runC qcc cconf conf ms + `shouldThrow` applicationProtocolErrorsIn [QpackDecompressionFailed] + it + "MUST NOT buffer a frame longer than SETTINGS_MAX_FIELD_SECTION_SIZE [HTTP/3 7.1]" + $ \_ -> do + let conf = addHook conf0 $ setOnControlStreamCreated overLongFrame + runC qcc cconf conf ms + `shouldThrow` applicationProtocolErrorsIn [H3ExcessiveLoad] + it "MUST send QPACK_ENCODER_STREAM_ERROR if a new dynamic table capacity value exceeds the limit [QPACK 4.1.3]" $ \_ -> do let conf = addHook conf0 $ setOnEncoderStreamCreated largeTableCapacity @@ -142,6 +227,55 @@ runC qcc cconf conf ms `shouldThrow` applicationProtocolErrorsIn [H3ClosedCriticalStream] it + "MUST send H3_STREAM_CREATION_ERROR if a second control stream is opened [HTTP/3 6.2.1]" + $ \_ -> do + -- A stream type of 0x00, then an empty SETTINGS frame. + opened <- newIORef False + let conf = addHook conf0 $ setOnControlStreamCreated $ openAnother opened "\x00\x04\x00" + expectIfOpened opened (runC qcc cconf conf ms) [H3StreamCreationError] + it + "MUST send H3_STREAM_CREATION_ERROR if a client opens a push stream [HTTP/3 6.2.2]" + $ \_ -> do + -- A stream type of 0x01, then push ID 0. + opened <- newIORef False + let conf = addHook conf0 $ setOnControlStreamCreated $ openAnother opened "\x01\x00" + expectIfOpened opened (runC qcc cconf conf ms) [H3StreamCreationError] + it + "MUST send H3_STREAM_CREATION_ERROR if a second encoder stream is opened [QPACK 4.2]" + $ \_ -> do + opened <- newIORef False + let conf = addHook conf0 $ setOnEncoderStreamCreated $ openAnother opened "\x02" + expectIfOpened opened (runC qcc cconf conf ms) [H3StreamCreationError] + it + "MUST send H3_STREAM_CREATION_ERROR if a second decoder stream is opened [QPACK 4.2]" + $ \_ -> do + opened <- newIORef False + let conf = addHook conf0 $ setOnDecoderStreamCreated $ openAnother opened "\x03" + expectIfOpened opened (runC qcc cconf conf ms) [H3StreamCreationError] + it + "MUST send H3_CLOSED_CRITICAL_STREAM if an encoder stream is closed [QPACK 4.2]" + $ \_ -> do + let conf = + noEncoderTable $ addHook conf0 $ setOnEncoderStreamCreated closeStream + runC qcc cconf conf ms + `shouldThrow` applicationProtocolErrorsIn [H3ClosedCriticalStream] + it + "MUST send H3_CLOSED_CRITICAL_STREAM if a decoder stream is closed [QPACK 4.2]" + $ \_ -> do + let conf = + noDecoderTable $ addHook conf0 $ setOnDecoderStreamCreated closeStream + runC qcc cconf conf ms + `shouldThrow` applicationProtocolErrorsIn [H3ClosedCriticalStream] + it + "MUST send H3_CLOSED_CRITICAL_STREAM if an encoder stream ends inside an instruction [QPACK 4.2]" + $ \_ -> do + -- The first octet of a Set Dynamic Table Capacity that goes on. + let conf = noEncoderTable $ addHook conf0 $ setOnEncoderStreamCreated $ \strm -> do + sendStream strm "\x3f" + closeStream strm + runC qcc cconf conf ms + `shouldThrow` applicationProtocolErrorsIn [H3ClosedCriticalStream] + it "MUST send QPACK_DECODER_STREAM_ERROR if Insert Count Increment is 0 [QPACK 4.4.3]" $ \_ -> do let conf = addHook conf0 $ setOnDecoderStreamCreated zeroInsertCountIncrement @@ -150,6 +284,75 @@ ---------------------------------------------------------------- +-- | Making a request the server must refuse and, where we can see what the +-- server hands its application, checking that it handed over nothing: a +-- stream used to be reset and the request handed over anyway. +refusedNotServed :: Maybe (IO Int) -> IO () -> IO () +refusedNotServed Nothing refused = refused +refusedNotServed (Just count) refused = do + before' <- count + refused + threadDelay 200000 + count `shouldReturn` before' + +-- | A client whose encoder will not use the dynamic table, and so has +-- nothing to write on its encoder stream. For the tests that close that +-- stream: a client that went on writing on it would see the closure itself +-- and close the connection, and the test is about what the server does. +noEncoderTable :: H3.Config -> H3.Config +noEncoderTable conf = + conf + { H3.confQEncoderConfig = + (H3.confQEncoderConfig conf){ecMaxTableCapacity = 0} + } + +-- | A client whose decoder offers no dynamic table, and so has nothing to +-- write on its decoder stream: no acknowledgements, since the server cannot +-- refer to a table, and no Stream Cancellations, which are not sent then. +-- For the test that closes that stream, as 'noEncoderTable'. +noDecoderTable :: H3.Config -> H3.Config +noDecoderTable conf = + conf + { H3.confQDecoderConfig = + (H3.confQDecoderConfig conf){dcMaxTableCapacity = 0} + } + +-- | The error cases that need the server to read a request's body. +-- +-- A server checks content-length, and the fields of trailers, only as its +-- application reads the body; an application that answers without reading +-- it never gets to either, and nothing tells from outside which paths read +-- one. These go to /drain, where our test server reads the body, and so +-- are not part of 'h3ErrorSpec': run against a server elsewhere, as h3spec +-- does, they would fail whatever that server does. +h3DrainSpec + :: ClientConfig + -> H3.ClientConfig + -> Millisecond + -> SpecWith a +h3DrainSpec qcc cconf ms = do + conf0 <- runIO H3.allocSimpleConfig + describe "HTTP/3 servers reading a request's body" $ do + it + "MUST treat content that does not match content-length as malformed [HTTP/3 4.1.2]" + $ \_ -> do + -- content-length says five octets and the request carries + -- none. /drain reads the body, which is where the count is + -- checked; a body nobody reads is never counted. + let req = H3.requestNoBody methodPost "/drain" [("content-length", "5")] + qcc' = addQUICHook qcc $ setOnResetStreamReceived $ \_strm aerr -> E.throwIO (ApplicationProtocolErrorIsReceived aerr "") + runCReq req qcc' cconf conf0 ms + `shouldThrow` applicationProtocolErrorsIn [H3MessageError] + it + "MUST treat a request whose trailers carry connection as malformed [HTTP/3 4.2]" + $ \_ -> do + -- /drain reads the body, and so the trailers after it. + let req0 = H3.requestBuilder methodPost "/drain" [] "hello" + req = H3.setRequestTrailersMaker req0 connectionTrailer + qcc' = addQUICHook qcc $ setOnResetStreamReceived $ \_strm aerr -> E.throwIO (ApplicationProtocolErrorIsReceived aerr "") + runCReq req qcc' cconf conf0 ms + `shouldThrow` applicationProtocolErrorsIn [H3MessageError] + addHook :: H3.Config -> (H3.Hooks -> H3.Hooks) -> H3.Config addHook conf modify = conf' where @@ -201,7 +404,7 @@ -- [(":method","GET") -- ,(":scheme","https") --- ,(":autority","127.0.0.1") +-- ,(":authority","127.0.0.1") -- ,(":path","/") -- ,(":foo","bar") -- the presence of prohibited fields or pseudo-header fields, -- ] @@ -209,12 +412,12 @@ illegalHeader1 _ = [ H3Frame H3FrameHeaders - "\x00\x00\xd1\xd7\x27\x02\x3a\x61\x75\x74\x6f\x72\x69\x74\x79\x09\x31\x32\x37\x2e\x30\x2e\x30\x2e\x31\xc1\x24\x3a\x66\x6f\x6f\x03\x62\x61\x72" + "\x00\x00\xd1\xd7\x50\x09\x31\x32\x37\x2e\x30\x2e\x30\x2e\x31\xc1\x24\x3a\x66\x6f\x6f\x03\x62\x61\x72" ] -- [(":method","GET") -- ,(":scheme","https") --- ,(":autority","127.0.0.1") +-- ,(":authority","127.0.0.1") -- ,("foo","bar") -- ,(":path","/") -- pseudo-header fields after fields -- ] @@ -222,7 +425,7 @@ illegalHeader2 _ = [ H3Frame H3FrameHeaders - "\x00\x00\xd1\xd7\x27\x02\x3a\x61\x75\x74\x6f\x72\x69\x74\x79\x09\x31\x32\x37\x2e\x30\x2e\x30\x2e\x31\x23\x66\x6f\x6f\x03\x62\x61\x72\xc1" + "\x00\x00\xd1\xd7\x50\x09\x31\x32\x37\x2e\x30\x2e\x30\x2e\x31\x23\x66\x6f\x6f\x03\x62\x61\x72\xc1" ] -- [(":method","GET") @@ -248,6 +451,21 @@ "\x00\x00\xd1\xd7\x50\x09\x31\x32\x37\x2e\x30\x2e\x30\x2e\x31\xc1\xff\x24" ] +-- [(":method","GET") +-- ,(":scheme","https") +-- ,(":authority","127.0.0.1") +-- ,(":path","/")] ++ dynamic index 0 +-- +-- The Required Insert Count in the prefix is 0 and nothing has been inserted, +-- so there is no entry 0 to name. The decoder used to fold the index back +-- into the table and hand over whatever slot it landed on. +illegalHeader5 :: [H3Frame] -> [H3Frame] +illegalHeader5 _ = + [ H3Frame + H3FrameHeaders + "\x00\x00\xd1\xd7\x50\x09\x31\x32\x37\x2e\x30\x2e\x30\x2e\x31\xc1\x80" + ] + {- -- [(SettingsQpackBlockedStreams,100) -- ,(SettingsQpackMaxTableCapacity,4096) @@ -261,6 +479,32 @@ -- ,(H3SettingsKey 0x2,200) -- HTTP/2 Settings -- ,(SettingsQpackMaxTableCapacity,4096) -- ,(SettingsMaxFieldSectionSize,32768)] +-- An identifier with no value behind it. +-- +-- Each parameter is a pair of variable-length integers; this frame stops +-- between them, which section 7.1 makes H3_FRAME_ERROR. Reading off the end +-- used to raise BufferOverrun, which nothing here catches, so the peer was +-- told nothing at all. +truncatedSettings :: [H3Frame] -> [H3Frame] +truncatedSettings _ = [H3Frame H3FrameSettings "\x01"] + +-- [(H3SettingsKey 0x21,0) -- reserved, to be ignored +-- ,(H3SettingsKey 0x2,0)] -- HTTP/2 Settings, which must be refused +-- +-- 0x21 is the first of the identifiers section 7.2.4.1 reserves and tells +-- endpoints they SHOULD send. Reading used to stop at it, so the HTTP/2 +-- setting behind it went unseen -- along with anything else a peer put there. +greaseThenHttp2Setting :: [H3Frame] -> [H3Frame] +greaseThenHttp2Setting _ = [H3Frame H3FrameSettings "\x21\x00\x02\x00"] + +-- [(H3SettingsKey 0x40,0) +-- ,(H3SettingsKey 0x40,0)] -- the same identifier twice +-- +-- 64 in the two-octet form. The duplicate check was bits in an Int, so +-- identifiers this large repeated unnoticed. +duplicateLargeSetting :: [H3Frame] -> [H3Frame] +duplicateLargeSetting _ = [H3Frame H3FrameSettings "\x40\x40\x00\x40\x40\x00"] + illegalSettings1 :: [H3Frame] -> [H3Frame] illegalSettings1 _ = [ H3Frame @@ -269,6 +513,64 @@ ] ---------------------------------------------------------------- + +-- | The fields RFC 9114, section 4.2, names as connection-specific, and TE +-- with a value other than "trailers". +connectionSpecific :: [(HeaderName, BS.ByteString)] +connectionSpecific = + [ ("connection", "close") + , ("keep-alive", "timeout=5") + , ("proxy-connection", "keep-alive") + , ("transfer-encoding", "chunked") + , ("upgrade", "websocket") + , ("te", "gzip") + ] + +-- | Trailers of a single Connection field. +connectionTrailer :: H3.TrailersMaker +connectionTrailer Nothing = return $ H3.Trailers [("connection", "close")] +connectionTrailer (Just _) = return $ H3.NextTrailersMaker connectionTrailer + +-- | Opening a unidirectional stream of our own next to the one given, and +-- sending it these octets: a stream type and whatever follows it. +-- +-- A client needs three unidirectional streams, and a server may let it have +-- no more than that: quic's default does. Then the stream cannot be opened +-- at all, and this gives up after a tenth of a second rather than waiting +-- for room that will not come, saying so in the 'IORef'. +openAnother :: IORef Bool -> BS.ByteString -> Stream -> IO () +openAnother opened bs strm = do + mstrm <- timeout 100000 $ unidirectionalStream $ streamConnection strm + forM_ mstrm $ \strm' -> do + sendStream strm' bs + writeIORef opened True + +-- | Expecting one of the errors when 'openAnother' could open its stream, +-- and leaving the test pending when it could not: against a server that +-- lets a client have only the streams it needs, a second control or QPACK +-- stream cannot be opened, and there is nothing to check. +expectIfOpened + :: IORef Bool -> IO (Maybe ()) -> [ApplicationProtocolError] -> IO () +expectIfOpened opened action errs = do + r <- E.try action + ok <- readIORef opened + if not ok + then + pendingWith + "the server allows no unidirectional stream beyond the three a client needs" + else case r of + Left e + | applicationProtocolErrorsIn errs e -> return () + | otherwise -> expectationFailure $ "unexpected exception: " ++ show e + Right _ -> expectationFailure "did not get expected exception" + +-- A GOAWAY frame announcing 2^30 octets and then sending none of them. +-- +-- A frame length is a variable-length integer, so a peer can claim up to +-- 2^62-1 and have the other end hold whatever it sends towards that. Nothing +-- has to arrive for the claim to be refused. +overLongFrame :: Stream -> IO () +overLongFrame strm = sendStream strm "\x07\xc0\x00\x00\x00\x40\x00\x00\x00" -- SetDynamicTableCapacity 10000000000 largeTableCapacity :: Stream -> IO () diff --git a/TransportError.hs b/TransportError.hs --- a/TransportError.hs +++ b/TransportError.hs @@ -75,6 +75,11 @@ let cc = addHook cc0 $ setOnTransportParametersCreated setStatelessResetToken runCnoOp cc ms `shouldThrow` transportErrorsIn [TransportParameterError] it + "MUST send TRANSPORT_PARAMETER_ERROR if a parameter value is malformed [Transport 18]" + $ \_ -> do + let cc = addHook cc0 $ setOnTLSExtensionCreated danglingParameter + runCnoOp cc ms `shouldThrow` transportErrorsIn [TransportParameterError] + it "MUST send TRANSPORT_PARAMETER_ERROR if max_udp_payload_size < 1200 [Transport 7.4 and 18.2]" $ \_ -> do let cc = addHook cc0 $ setOnTransportParametersCreated setMaxUdpPayloadSize @@ -90,6 +95,26 @@ let cc = addHook cc0 $ setOnTransportParametersCreated setMaxAckDelay runCnoOp cc ms `shouldThrow` transportErrorsIn [TransportParameterError] it + "MUST send TRANSPORT_PARAMETER_ERROR if initial_max_streams_bidi > 2^60 [Transport 18.2]" + $ \_ -> do + let cc = addHook cc0 $ setOnTransportParametersCreated setMaxStreamsBidi + runCnoOp cc ms `shouldThrow` transportErrorsIn [TransportParameterError] + it + "MUST send TRANSPORT_PARAMETER_ERROR if initial_max_streams_uni > 2^60 [Transport 18.2]" + $ \_ -> do + let cc = addHook cc0 $ setOnTransportParametersCreated setMaxStreamsUni + runCnoOp cc ms `shouldThrow` transportErrorsIn [TransportParameterError] + it + "MUST send FRAME_ENCODING_ERROR if an ACK range reaches below zero [Transport 19.3.1]" + $ \_ -> do + let cc = addHook cc0 $ setOnPlainCreated impossibleAckRange + runCnoOp cc ms `shouldThrow` transportErrorsIn [FrameEncodingError] + it + "SHOULD send PROTOCOL_VIOLATION on an ACK for a packet never sent [Transport 13.1]" + $ \_ -> do + let cc = addHook cc0 $ setOnPlainCreated ackForUnsentPacket + runCnoOp cc ms `shouldThrow` transportErrorsIn [ProtocolViolation] + it "MUST send FRAME_ENCODING_ERROR if a frame of unknown type is received [Transport 12.4]" $ \_ -> do let cc = addHook cc0 $ setOnPlainCreated unknownFrame @@ -103,11 +128,31 @@ let cc = addHook cc0 $ setOnPlainCreated $ rrBits HandshakeLevel runCnoOp cc ms `shouldThrow` transportError it + "MUST send CRYPTO_BUFFER_EXCEEDED if CRYPTO data is buffered beyond the limit [Transport 7.5]" + $ \_ -> do + let cc = addHook cc0 $ setOnPlainCreated cryptoBeyondBuffer + runCnoOp cc ms `shouldThrow` transportErrorsIn [CryptoBufferExceeded] + it "MUST send PROTOCOL_VIOLATION if PATH_CHALLENGE in Handshake is received [Transport 17.2.4]" $ \_ -> do 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 + runCnoOp cc ms `shouldThrow` transportError + it + "MUST send PROTOCOL_VIOLATION if DATA_BLOCKED in Handshake is received [Transport 12.4]" + $ \_ -> do + let cc = addHook cc0 $ setOnPlainCreated handshakeDataBlocked + runCnoOp cc ms `shouldThrow` transportError + it "MUST send PROTOCOL_VIOLATION if reserved bits in Short are non-zero [Transport 17.2]" $ \_ -> do let cc = addHook cc0 $ setOnPlainCreated $ rrBits RTT1Level @@ -233,6 +278,14 @@ setOnTransportParametersCreated :: (Parameters -> Parameters) -> Hooks -> Hooks setOnTransportParametersCreated f hooks = hooks{onTransportParametersCreated = f} +-- initial_max_data announcing a zero-length value. Everything the peer +-- really sent is left in front of it, so this is the value alone being wrong +-- rather than the list being cut short. The value of an integer parameter is +-- one variable-length integer, and there is no such thing in no octets. +danglingParameter :: [ExtensionRaw] -> [ExtensionRaw] +danglingParameter [ExtensionRaw eid v] = [ExtensionRaw eid (v <> "\x04\x00")] +danglingParameter xs = xs + setOnTLSExtensionCreated :: ([ExtensionRaw] -> [ExtensionRaw]) -> Hooks -> Hooks setOnTLSExtensionCreated f params = params{onTLSExtensionCreated = f} @@ -292,8 +345,21 @@ setMaxAckDelay :: Parameters -> Parameters setMaxAckDelay params = params{maxAckDelay = 2 ^ (15 :: Int)} +-- A stream id has 62 bits, two of them saying who opened it and whether it is +-- bidirectional, so a count past 2^60 names no stream. +setMaxStreamsBidi :: Parameters -> Parameters +setMaxStreamsBidi params = params{initialMaxStreamsBidi = 2 ^ (60 :: Int) + 1} + +setMaxStreamsUni :: Parameters -> Parameters +setMaxStreamsUni params = params{initialMaxStreamsUni = 2 ^ (60 :: Int) + 1} + ---------------------------------------------------------------- +-- 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} @@ -308,16 +374,66 @@ where fake = StreamF 1000000000 0 ["GET /\r\n"] True +-- Largest acknowledged 5, then a gap of 10: the next range would start at +-- 5 - 10 - 2, which is not a packet number. +impossibleAckRange :: EncryptionLevel -> Plain -> Plain +impossibleAckRange lvl plain + | lvl == RTT1Level = + plain{plainFrames = Ack (AckInfo 5 0 [(10, 0)]) 0 : plainFrames plain} + | otherwise = plain + +-- Nobody has sent a million packets down this connection. +ackForUnsentPacket :: EncryptionLevel -> Plain -> Plain +ackForUnsentPacket lvl plain + | lvl == RTT1Level = + plain{plainFrames = Ack (AckInfo 1000000 0 []) 0 : plainFrames plain} + | otherwise = plain + unknownFrame :: EncryptionLevel -> Plain -> Plain unknownFrame lvl plain | lvl == RTT1Level = plain{plainFrames = UnknownFrame 0x20 : plainFrames plain} | otherwise = plain +-- CRYPTO frames are outside flow control, so nothing but the buffer limit +-- stops a peer from parking a fragment far past where the stream has got to +-- and having it held. One octet at this offset is enough to ask for more +-- than any bound the receiver could sensibly hold. +cryptoBeyondBuffer :: EncryptionLevel -> Plain -> Plain +cryptoBeyondBuffer lvl plain + | lvl == HandshakeLevel = + plain{plainFrames = CryptoF 100000000 "x" : plainFrames plain} + | otherwise = plain + handshakePathChallenge :: EncryptionLevel -> Plain -> Plain 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 +-- 1-RTT packets only, and Sec 12.4 makes a frame in a packet that may not +-- carry it a connection error of type PROTOCOL_VIOLATION. +handshakeStreamDataBlocked :: EncryptionLevel -> Plain -> Plain +handshakeStreamDataBlocked lvl plain + | lvl == HandshakeLevel = + plain{plainFrames = StreamDataBlocked 0 0 : plainFrames plain} + | otherwise = plain + +handshakeDataBlocked :: EncryptionLevel -> Plain -> Plain +handshakeDataBlocked lvl plain + | lvl == HandshakeLevel = + plain{plainFrames = DataBlocked 0 : plainFrames plain} | otherwise = plain noFrames :: EncryptionLevel -> Plain -> Plain diff --git a/h3spec.cabal b/h3spec.cabal --- a/h3spec.cabal +++ b/h3spec.cabal @@ -1,5 +1,5 @@ name: h3spec -version: 0.1.12 +version: 0.1.14 synopsis: QUIC description: Test tool for error cases of QUIC and HTTP/3 license: BSD3 @@ -9,7 +9,7 @@ -- copyright: category: Web build-type: Simple --- extra-source-files: ChangeLog.md +extra-source-files: ChangeLog.md cabal-version: >= 1.10 executable h3spec @@ -22,11 +22,12 @@ ghc-options: -Wall -threaded -rtsopts build-depends: base >= 4.9 && < 5 , bytestring + , case-insensitive , hspec , hspec-core , http-types - , http3 >= 0.0.10 + , http3 >= 0.1.7 && < 0.2 , network - , quic >= 0.2 && < 0.3 + , quic >= 0.3.13 && < 0.4 , tls default-extensions: Strict StrictData diff --git a/h3spec.hs b/h3spec.hs --- a/h3spec.hs +++ b/h3spec.hs @@ -124,10 +124,10 @@ | null (optSkip opts) = qcArgs0 | otherwise = "--skip" : (intersperse "--skip" $ reverse $ optSkip opts) - h3cc = H3.ClientConfig "https" host + h3cc = H3.defaultClientConfig{H3.authority = host} ms = optTimeout opts H.readConfig H.defaultConfig qcArgs - >>= withArgs [] . H.runSpec (transportErrorSpec cc ms >> h3ErrorSpec cc h3cc ms) + >>= withArgs [] . H.runSpec (transportErrorSpec cc ms >> h3ErrorSpec cc h3cc ms (const Nothing)) >>= H.evaluateSummary getLogger :: Maybe FilePath -> (String -> IO ())