From bbc9eccf4d73b451278a73f06869c2d43c8709ef Mon Sep 17 00:00:00 2001 From: Alexander Bondarenko <486682+dpwiz@users.noreply.github.com> Date: Thu, 28 Mar 2024 20:12:48 +0200 Subject: [PATCH] xftp: prevent overwriting completed upload (#1063) * xftp: prevent overwriting completed upload * add size check for skipCommitted * fix import * fail on incorrect size --------- Co-authored-by: Evgeny Poberezkin --- src/Simplex/FileTransfer/Server.hs | 32 ++++++++++++++++++++---------- tests/XFTPServerTests.hs | 20 +++++++++++++++++++ 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/Simplex/FileTransfer/Server.hs b/src/Simplex/FileTransfer/Server.hs index 100301cac..4874aea1a 100644 --- a/src/Simplex/FileTransfer/Server.hs +++ b/src/Simplex/FileTransfer/Server.hs @@ -4,6 +4,7 @@ {-# LANGUAGE FlexibleContexts #-} {-# LANGUAGE GADTs #-} {-# LANGUAGE LambdaCase #-} +{-# LANGUAGE MultiWayIf #-} {-# LANGUAGE NamedFieldPuns #-} {-# LANGUAGE NumericUnderscores #-} {-# LANGUAGE OverloadedLists #-} @@ -25,7 +26,7 @@ import Data.List (intercalate) import Data.List.NonEmpty (NonEmpty) import qualified Data.List.NonEmpty as L import qualified Data.Map.Strict as M -import Data.Maybe (fromMaybe) +import Data.Maybe (fromMaybe, isJust) import qualified Data.Text as T import Data.Time.Clock (UTCTime (..), diffTimeToPicoseconds, getCurrentTime) import Data.Time.Clock.System (SystemTime (..), getSystemTime) @@ -47,13 +48,14 @@ import qualified Simplex.Messaging.Crypto as C import qualified Simplex.Messaging.Crypto.Lazy as LC import qualified Simplex.Messaging.Encoding.Base64.URL as U import Simplex.Messaging.Encoding.String -import Simplex.Messaging.Protocol (CorrId, RcvPublicDhKey, RcvPublicAuthKey, RecipientId, TransmissionAuth) +import Simplex.Messaging.Protocol (CorrId, RcvPublicAuthKey, RcvPublicDhKey, RecipientId, TransmissionAuth) import Simplex.Messaging.Server (dummyVerifyCmd, verifyCmdAuthorization) import Simplex.Messaging.Server.Expiration import Simplex.Messaging.Server.Stats import Simplex.Messaging.Transport (THandleParams (..)) import Simplex.Messaging.Transport.Buffer (trimCR) import Simplex.Messaging.Transport.HTTP2 +import Simplex.Messaging.Transport.HTTP2.File (fileBlockSize) import Simplex.Messaging.Transport.HTTP2.Server import Simplex.Messaging.Transport.Server (runTCPServer) import Simplex.Messaging.Util @@ -67,13 +69,12 @@ import qualified UnliftIO.Exception as E type M a = ReaderT XFTPEnv IO a -data XFTPTransportRequest = - XFTPTransportRequest - { thParams :: THandleParams XFTPVersion, - reqBody :: HTTP2Body, - request :: H.Request, - sendResponse :: H.Response -> IO () - } +data XFTPTransportRequest = XFTPTransportRequest + { thParams :: THandleParams XFTPVersion, + reqBody :: HTTP2Body, + request :: H.Request, + sendResponse :: H.Response -> IO () + } runXFTPServer :: XFTPServerConfig -> IO () runXFTPServer cfg = do @@ -373,8 +374,19 @@ processXFTPRequest HTTP2Body {bodyPart} = \case receiveServerFile FileRec {senderId, fileInfo = FileInfo {size, digest}, filePath} = case bodyPart of Nothing -> pure $ FRErr SIZE -- TODO validate body size from request before downloading, once it's populated - Just getBody -> ifM reserve receive (pure $ FRErr QUOTA) -- TODO: handle duplicate uploads + Just getBody -> skipCommitted $ ifM reserve receive (pure $ FRErr QUOTA) where + -- having a filePath means the file is already uploaded and committed, must not change anything + skipCommitted = ifM (isJust <$> readTVarIO filePath) (liftIO $ drain $ fromIntegral size) + where + -- can't send FROk without reading the request body or a client will block on sending it + -- can't send any old error as the client would fail or restart indefinitely + drain s = do + bs <- B.length <$> getBody fileBlockSize + if + | bs == s -> pure FROk + | bs == 0 || bs > s -> pure $ FRErr SIZE + | otherwise -> drain (s - bs) reserve = do us <- asks $ usedStorage . store quota <- asks $ fromMaybe maxBound . fileSizeQuota . config diff --git a/tests/XFTPServerTests.hs b/tests/XFTPServerTests.hs index 4650edf57..71700280a 100644 --- a/tests/XFTPServerTests.hs +++ b/tests/XFTPServerTests.hs @@ -60,6 +60,7 @@ xftpServerTests = it "prohibited when FNEW disabled" $ testFileBasicAuth False (Just "pwd") (Just "pwd") False it "allowed with correct basic auth" $ testFileBasicAuth True (Just "pwd") (Just "pwd") True it "allowed with auth on server without auth" $ testFileBasicAuth True Nothing (Just "any") True + it "should not change content for uploaded and committed files" testFileSkipCommitted chSize :: Integral a => a chSize = kb 128 @@ -372,3 +373,22 @@ testFileBasicAuth allowNewFiles newFileBasicAuth clntAuth success = else do void (createXFTPChunk c spKey file [rcvKey] clntAuth) `catchError` (liftIO . (`shouldBe` PCEProtocolError AUTH)) + +testFileSkipCommitted :: IO () +testFileSkipCommitted = + withXFTPServerCfg testXFTPServerConfig $ + \_ -> testXFTPClient $ \c -> do + g <- C.newRandom + (sndKey, spKey) <- atomically $ C.generateAuthKeyPair C.SEd25519 g + (rcvKey, rpKey) <- atomically $ C.generateAuthKeyPair C.SEd25519 g + bytes <- createTestChunk testChunkPath + digest <- LC.sha256Hash <$> LB.readFile testChunkPath + let file = FileInfo {sndKey, size = chSize, digest} + chunkSpec = XFTPChunkSpec {filePath = testChunkPath, chunkOffset = 0, chunkSize = chSize} + runRight_ $ do + (sId, [rId]) <- createXFTPChunk c spKey file [rcvKey] Nothing + uploadXFTPChunk c spKey sId chunkSpec + void . liftIO $ createTestChunk testChunkPath -- trash chunk contents + uploadXFTPChunk c spKey sId chunkSpec -- upload again to get FROk without getting stuck + downloadXFTPChunk g c rpKey rId $ XFTPRcvChunkSpec "tests/tmp/received_chunk" chSize digest + liftIO $ B.readFile "tests/tmp/received_chunk" `shouldReturn` bytes -- new chunk content got ignored