Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1
Hello, On Tue 03 May 2022 at 12:13PM -04, Joey Hess wrote: > Fixed with attached patch. Cool, thanks. Seems like there might be a release soon so I'll hold off patching Debian's version? -- Sean Whitton signature.asc Description: PGP signature
Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1
Fixed with attached patch. -- see shy jo From 43701759a32e38613c61de6dc923c24069f435d6 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 3 May 2022 12:12:25 -0400 Subject: [PATCH] disable shellescape for rsync 3.2.4 rsync 3.2.4 broke backwards-compatability by preventing exposing filenames to the shell. Made the rsync and gcrypt special remotes detect this and disable shellescape. An alternative fix would have been to always set RSYNC_OLD_ARGS=1. Which would avoid the overhead of probing rsync --help for each affected remote. But that is really very fast to run, and it seemed better to switch to the modern code path rather than keeping on using the bad old code path. Sponsored-by: Tobias Ammann on Patreon --- CHANGELOG | 3 +++ Remote/GCrypt.hs | 3 ++- Remote/Rsync.hs| 27 ++- doc/special_remotes/rsync.mdwn | 15 --- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 263da8e6e..6f53e3e1a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -11,6 +11,9 @@ git-annex (10.20220323) UNRELEASED; urgency=medium * Fix test failure on NFS when cleaning up gpg temp directory. * Fix a build failure with ghc 9.2.2. Thanks, gnezdo for the patch. + * rsync 3.2.4 broke backwards-compatability by preventing exposing +filenames to the shell. Made the rsync and gcrypt special remotes +detect this and disable shellescape. Closes: #1010397 -- Joey Hess Mon, 28 Mar 2022 14:46:10 -0400 diff --git a/Remote/GCrypt.hs b/Remote/GCrypt.hs index 9662e75d4..0b120806a 100644 --- a/Remote/GCrypt.hs +++ b/Remote/GCrypt.hs @@ -130,7 +130,8 @@ gen' r u c gc rs = do cst <- remoteCost gc $ if repoCheap r then nearlyCheapRemoteCost else expensiveRemoteCost let (rsynctransport, rsyncurl, accessmethod) = rsyncTransportToObjects r gc - let rsyncopts = Remote.Rsync.genRsyncOpts c gc rsynctransport rsyncurl + protectsargs <- liftIO Remote.Rsync.probeRsyncProtectsArgs + let rsyncopts = Remote.Rsync.genRsyncOpts protectsargs c gc rsynctransport rsyncurl let this = Remote { uuid = u , cost = cst diff --git a/Remote/Rsync.hs b/Remote/Rsync.hs index 81d60311e..e7e9ff740 100644 --- a/Remote/Rsync.hs +++ b/Remote/Rsync.hs @@ -16,7 +16,8 @@ module Remote.Rsync ( withRsyncScratchDir, rsyncRemoteConfigs, genRsyncOpts, - RsyncOpts + RsyncOpts, + probeRsyncProtectsArgs, ) where import Annex.Common @@ -36,6 +37,7 @@ import Remote.Rsync.RsyncUrl import Crypto import Utility.Rsync import Utility.CopyFile +import Utility.Process.Transcript import Messages.Progress import Utility.Metered import Types.Transfer @@ -74,7 +76,8 @@ gen r u rc gc rs = do cst <- remoteCost gc expensiveRemoteCost (transport, url) <- rsyncTransport gc $ fromMaybe (giveup "missing rsyncurl") $ remoteAnnexRsyncUrl gc - let o = genRsyncOpts c gc transport url + protectsargs <- liftIO probeRsyncProtectsArgs + let o = genRsyncOpts protectsargs c gc transport url let islocal = rsyncUrlIsPath $ rsyncUrl o return $ Just $ specialRemote c (fileStorer $ store o) @@ -124,6 +127,18 @@ gen r u rc gc rs = do , remoteStateHandle = rs } +-- | Since 3.2.4, rsync protects filenames from being exposed to the shell. +newtype RsyncProtectsArgs = RsyncProtectsArgs Bool + +probeRsyncProtectsArgs :: IO RsyncProtectsArgs +probeRsyncProtectsArgs = do + (helpoutput, _) <- processTranscript "rsync" ["--help"] Nothing + -- The --old-args option was added to disable the new arg + -- protection, so use it to detect when that feature is supported + -- by rsync, rather than parsing versions. + return (RsyncProtectsArgs $ "--old-args" `isInfixOf` helpoutput) + + -- Things used by genRsyncOpts rsyncRemoteConfigs :: [RemoteConfigFieldParser] rsyncRemoteConfigs = @@ -131,15 +146,17 @@ rsyncRemoteConfigs = (FieldDesc "set to no to avoid usual shell escaping (not recommended)") ] -genRsyncOpts :: ParsedRemoteConfig -> RemoteGitConfig -> Annex [CommandParam] -> RsyncUrl -> RsyncOpts -genRsyncOpts c gc transport url = RsyncOpts +genRsyncOpts :: RsyncProtectsArgs -> ParsedRemoteConfig -> RemoteGitConfig -> Annex [CommandParam] -> RsyncUrl -> RsyncOpts +genRsyncOpts (RsyncProtectsArgs protectsargs) c gc transport url = RsyncOpts { rsyncUrl = url , rsyncOptions = appendtransport $ opts [] , rsyncUploadOptions = appendtransport $ opts (remoteAnnexRsyncUploadOptions gc) , rsyncDownloadOptions = appendtransport $ opts (remoteAnnexRsyncDownloadOptions gc) - , rsyncShellEscape = fromMaybe True (getRemoteConfigValue shellEscapeField c) + , rsyncShellEscape = if protectsargs + then False + else fromMaybe True (getRemoteConfigValue shellEscapeField c) } where appendtransport l = (++ l) <$> transport diff --git a/doc/special_remotes/rsync.mdwn b/doc/special_remotes/rsync.mdwn index 96e452b10..e9f54dc68 100644 --- a/doc/special_remotes/rsync.mdwn +++ b/doc/special_remotes/rsync.mdwn @@ -26,13
Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1
> > git-annex enableremote foo shellescape=no I've confirmed that this workaround works. Also, the client's version of rsync is what matters. 3.2.3 client and 3.2.4 server does not have the problem. -- see shy jo signature.asc Description: PGP signature
Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1
Sean Whitton wrote: > > git-annex can be configured to not do its own shell escaping when accessing > > a rsync special remote. If your remote is named "foo", you can configure it > > that way as follows: > > > > git-annex enableremote foo shellescape=no > > Thanks for looking into it. Do you see this as a workaround or what > people using newer rsync should use going forward? That's a workaround. If it works, perhaps git-annex could probe the rsync version and do the same thing automatically. Depends on it only the local rsync version matters, not the remote one. -- see shy jo signature.asc Description: PGP signature
Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1
Hello, On Mon 02 May 2022 at 12:51PM -04, Joey Hess wrote: > Joey Hess wrote: >> The rsync command that git-annex runs has a trailing close quote, as >> seen in the first excerpt above. But rsync then complains about the path >> with that close quote removed. >> >> I'm having a hard time not seeing this as a bug in rsync. > > # NEWS for rsync 3.2.4 (15 Apr 2022) > > ## Changes in this version: > > ### BEHAVIOR CHANGES: > > - A new form of arg protection was added that works similarly to the older >[`--protect-args`](rsync.1#opt) (`-s`) option but in a way that avoids >breaking things like rrsync (the restricted rsync script): rsync now uses >backslash escaping for sending "shell-active" characters to the remote >shell. This includes spaces, so fetching a remote file via a simple quoted >filename value now works by default without any extra quoting > > This change must be the cause of the problem. > > git-annex can be configured to not do its own shell escaping when accessing > a rsync special remote. If your remote is named "foo", you can configure it > that way as follows: > > git-annex enableremote foo shellescape=no Thanks for looking into it. Do you see this as a workaround or what people using newer rsync should use going forward? -- Sean Whitton
Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1
Joey Hess wrote: > The rsync command that git-annex runs has a trailing close quote, as > seen in the first excerpt above. But rsync then complains about the path > with that close quote removed. > > I'm having a hard time not seeing this as a bug in rsync. # NEWS for rsync 3.2.4 (15 Apr 2022) ## Changes in this version: ### BEHAVIOR CHANGES: - A new form of arg protection was added that works similarly to the older [`--protect-args`](rsync.1#opt) (`-s`) option but in a way that avoids breaking things like rrsync (the restricted rsync script): rsync now uses backslash escaping for sending "shell-active" characters to the remote shell. This includes spaces, so fetching a remote file via a simple quoted filename value now works by default without any extra quoting This change must be the cause of the problem. git-annex can be configured to not do its own shell escaping when accessing a rsync special remote. If your remote is named "foo", you can configure it that way as follows: git-annex enableremote foo shellescape=no -- see shy jo signature.asc Description: PGP signature
Bug#1010397: Fwd: Bug#1010397: git-annex sync fails in rsync remotes with rsync 3.2.4-1
> [2022-04-30 13:00:20.39881781] (Utility.Process) process [1635096] read: > rsync ["-e","'ssh' '-S' '../.git/annex/ssh/user@host-key' '-o' > 'ControlMaster=auto' '-o' 'ControlPersist=yes' '-l' 'user' > '-T'","--progress","--inplace", > "user@host-key:/backup/dspace/repositories/repository.git/ff7/84a/'GPGHMACSHA1--f5a2f4c1b3562feff26d0d1d6db70298087fd851/GPGHMACSHA1--f5a2f4c1b3562feff26d0d1d6db70298087fd851'" > ,"../.git/annex/tmp/GPGHMACSHA1--f5a2f4c1b3562feff26d0d1d6db70298087fd851"] > rsync: [sender] change_dir > "/backup/dspace/repositories/repository.git/ff7/84a/'GPGHMACSHA1--f5a2f4c1b3562feff26d0d1d6db70298087fd851" > failed: No such file or directory (2) The rsync command that git-annex runs has a trailing close quote, as seen in the first excerpt above. But rsync then complains about the path with that close quote removed. I'm having a hard time not seeing this as a bug in rsync. -- see shy jo signature.asc Description: PGP signature