mirrors/git-annex
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

verify that LFS server responds with requested object

Browse source 
The protocol design allows the server to respond with some other object;
if a server for some reason a server did that, it would not be right for
git-annex to download its content. I don't think it would be a security
hole, since git-annex is downloading a specific key and will verify the
key's content. Seems like a good idea to belt-and-suspenders test for
such a misuse of the protocol.
This commit is contained in:
Joey Hess 2019-08-03 16:23:47 -04:00
parent 28c0395d61
commit b82ecf7076
No known key found for this signature in database
GPG key ID: DB12DB0FF05F8F38
1 changed files with 27 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
Remote/GitLFS.hs
View file

@ -246,7 +246,7 @@ extractKeySize k
| isEncKey k = Nothing | isEncKey k = Nothing
| otherwise = keySize k | otherwise = keySize k
mkDownloadRequest :: Key -> Annex (Maybe LFS.TransferRequest) mkDownloadRequest :: Key -> Annex (Maybe (LFS.TransferRequest, LFS.SHA256, Integer))
mkDownloadRequest k = case (extractKeySha256 k, extractKeySize k) of mkDownloadRequest k = case (extractKeySha256 k, extractKeySize k) of
(Just sha256, Just sz) -> go sha256 sz (Just sha256, Just sz) -> go sha256 sz
-- TODO get from git-annex branch -- TODO get from git-annex branch
@ -257,12 +257,13 @@ mkDownloadRequest k = case (extractKeySha256 k, extractKeySize k) of
{ LFS.req_oid = sha256 { LFS.req_oid = sha256
, LFS.req_size = sz , LFS.req_size = sz
} }
return $ Just $ LFS.TransferRequest let req = LFS.TransferRequest
{ LFS.req_operation = LFS.RequestDownload { LFS.req_operation = LFS.RequestDownload
, LFS.req_transfers = [LFS.Basic] , LFS.req_transfers = [LFS.Basic]
, LFS.req_ref = Nothing , LFS.req_ref = Nothing
, LFS.req_objects = [obj] , LFS.req_objects = [obj]
} }
return $ Just (req, sha256, sz)
store :: TVar LFSHandle -> Storer store :: TVar LFSHandle -> Storer
store h = fileStorer $ \k src p -> getLFSEndpoint LFS.RequestUpload h >>= \case store h = fileStorer $ \k src p -> getLFSEndpoint LFS.RequestUpload h >>= \case
@ -303,10 +304,8 @@ store h = fileStorer $ \k src p -> getLFSEndpoint LFS.RequestUpload h >>= \case
return True return True
where where
send body sha256 size tro send body sha256 size tro
| LFS.resp_oid tro /= sha256 = | LFS.resp_oid tro /= sha256 || LFS.resp_size tro /= size =
giveup "git-lfs server requested other sha256 than the one we asked to send" giveup "git-lfs server requested other object than the one we asked to send"
| LFS.resp_size tro /= size =
giveup "git-lfs server requested other object size than we asked to send"
| otherwise = case LFS.resp_error tro of | otherwise = case LFS.resp_error tro of
Just err -> giveup $ Just err -> giveup $
T.unpack $ LFS.respobjerr_message err T.unpack $ LFS.respobjerr_message err
@ -326,12 +325,14 @@ retrieve h = fileRetriever $ \dest k p -> getLFSEndpoint LFS.RequestDownload h >
Nothing -> giveup "unable to connect to git-lfs endpoint" Nothing -> giveup "unable to connect to git-lfs endpoint"
Just endpoint -> mkDownloadRequest k >>= \case Just endpoint -> mkDownloadRequest k >>= \case
Nothing -> giveup "unable to download this object from git-lfs" Nothing -> giveup "unable to download this object from git-lfs"
Just req -> sendTransferRequest req endpoint >>= \case Just (req, sha256, size) -> sendTransferRequest req endpoint >>= \case
Left err -> giveup (show err) Left err -> giveup (show err)
Right resp -> case LFS.objects resp of Right resp -> case LFS.objects resp of
[] -> giveup "git-lfs server did not provide a way to download this object" [] -> giveup "git-lfs server did not provide a way to download this object"
(tro:_) -> receive dest p tro (tro:_)
| LFS.resp_oid tro /= sha256 || LFS.resp_size tro /= size ->
giveup "git-lfs server replied with other object than the one we requested"
| otherwise -> receive dest p tro
where where
receive dest p tro = case LFS.resp_error tro of receive dest p tro = case LFS.resp_error tro of
Just err -> giveup $ T.unpack $ LFS.respobjerr_message err Just err -> giveup $ T.unpack $ LFS.respobjerr_message err
@ -351,30 +352,34 @@ checkKey h key = getLFSEndpoint LFS.RequestDownload h >>= \case
-- Unable to find enough information to request the key -- Unable to find enough information to request the key
-- from git-lfs, so it's not present there. -- from git-lfs, so it's not present there.
Nothing -> return False Nothing -> return False
Just req -> case LFS.startTransferRequest endpoint req of Just (req, sha256, size) -> case LFS.startTransferRequest endpoint req of
Nothing -> giveup "unable to parse git-lfs endpoint url" Nothing -> giveup "unable to parse git-lfs endpoint url"
Just httpreq -> go =<< makeSmallAPIRequest httpreq Just httpreq -> go sha256 size =<< makeSmallAPIRequest httpreq
where where
go httpresp go sha256 size httpresp
| responseStatus httpresp == status200 = | responseStatus httpresp == status200 = go' sha256 size $
go' $ LFS.parseTransferResponse (responseBody httpresp) LFS.parseTransferResponse (responseBody httpresp)
| otherwise = | otherwise = giveup $
giveup $ "git-lfs server refused request: " ++ show (responseStatus httpresp) "git-lfs server refused request: " ++ show (responseStatus httpresp)
go' :: LFS.ParsedTransferResponse LFS.DownloadOperation -> Annex Bool go' :: LFS.SHA256 -> Integer -> LFS.ParsedTransferResponse LFS.DownloadOperation -> Annex Bool
go' (LFS.ParseFailed err) = go' _ _ (LFS.ParseFailed err) =
giveup $ "unable to parse response from git-lfs server: " ++ err giveup $ "unable to parse response from git-lfs server: " ++ err
-- If the server responds with a json error message, -- If the server responds with a json error message,
-- the content is presumably not present. -- the content is presumably not present.
go' (LFS.ParsedTransferResponseError _) = return False go' _ _ (LFS.ParsedTransferResponseError _) = return False
-- If the server responds with at least one download operation, -- If the server responds with at least one download operation,
-- we will assume the content is present. We could also try to HEAD -- we will assume the content is present. We could also try to HEAD
-- that download, but there's no guarantee HEAD is supported, and -- that download, but there's no guarantee HEAD is supported, and
-- at most that would detect breakage where the server is confused -- at most that would detect breakage where the server is confused
-- about what objects it has. -- about what objects it has.
go' (LFS.ParsedTransferResponse resp) = go' sha256 size (LFS.ParsedTransferResponse resp) =
return $ not $ null $ case LFS.objects resp of
mapMaybe LFS.resp_actions $ LFS.objects resp [] -> return False
(tro:_)
| LFS.resp_oid tro /= sha256 || LFS.resp_size tro /= size ->
giveup "git-lfs server replied with other object than the one we requested"
| otherwise -> return True
retrieveCheap :: Key -> AssociatedFile -> FilePath -> Annex Bool retrieveCheap :: Key -> AssociatedFile -> FilePath -> Annex Bool
retrieveCheap _ _ _ = return False retrieveCheap _ _ _ = return False