summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Joey Hess <joey@kitenet.net>2013-10-06 20:43:22 -0400
committerGravatar Joey Hess <joey@kitenet.net>2013-10-06 20:59:36 -0400
commit31807817d44e317c4cc807703ea4df6ffff6687e (patch)
tree7ad4adc3aeccfcba9222a00d444d93849f6abb6a
parente64c2e0929ee56ad44d40689d78f1f6b650d3cf9 (diff)
work around ssh brain-damange
The control socket path passed to ssh needs to be 17 characters shorter than the maximum unix domain socket length, because ssh appends stuff to it to make a temporary filename. Closes: #725512 Also, take the shorter of the relative and the absolute paths to the socket. Typically the relative path will be a lot shorter (unless deep inside a subdirectory of the repository), and so using it will avoid flirting with the maximum safe socket lenghts in more situations, and so lead to less breakage if all my attempts at fixing this are still buggy.
-rw-r--r--Annex/Ssh.hs41
-rw-r--r--debian/changelog3
2 files changed, 33 insertions, 11 deletions
diff --git a/Annex/Ssh.hs b/Annex/Ssh.hs
index 3b1e4b457..e2b6564e4 100644
--- a/Annex/Ssh.hs
+++ b/Annex/Ssh.hs
@@ -52,14 +52,30 @@ sshInfo (host, port) = go =<< sshCacheDir
where
go Nothing = return (Nothing, [])
go (Just dir) = do
- let socketfile = dir </> hostport2socket host port
- if valid_unix_socket_path socketfile
- then return (Just socketfile, sshConnectionCachingParams socketfile)
- else do
- socketfile' <- liftIO $ relPathCwdToFile socketfile
- return $ if valid_unix_socket_path socketfile'
- then (Just socketfile', sshConnectionCachingParams socketfile')
- else (Nothing, [])
+ r <- liftIO $ bestSocketPath $ dir </> hostport2socket host port
+ return $ case r of
+ Nothing -> (Nothing, [])
+ Just socketfile -> (Just socketfile, sshConnectionCachingParams socketfile)
+
+{- Given an absolute path to use for a socket file,
+ - returns whichever is shorter of that or the relative path to the same
+ - file.
+ -
+ - If no path can be constructed that is a valid socket, returns Nothing. -}
+bestSocketPath :: FilePath -> IO (Maybe FilePath)
+bestSocketPath abssocketfile = do
+ relsocketfile <- liftIO $ relPathCwdToFile abssocketfile
+ let socketfile = if length abssocketfile <= length relsocketfile
+ then abssocketfile
+ else relsocketfile
+ return $ if valid_unix_socket_path (socketfile ++ sshgarbage)
+ then Just socketfile
+ else Nothing
+ where
+ -- ssh appends a 16 char extension to the socket when setting it
+ -- up, which needs to be taken into account when checking
+ -- that a valid socket was constructed.
+ sshgarbage = take (1+16) $ repeat 'X'
sshConnectionCachingParams :: FilePath -> [CommandParam]
sshConnectionCachingParams socketfile =
@@ -96,8 +112,9 @@ sshCleanup = go =<< sshCacheDir
where
go Nothing = noop
go (Just dir) = do
- sockets <- filter (not . isLock) <$>
- liftIO (catchDefaultIO [] $ dirContents dir)
+ sockets <- liftIO $ filter (not . isLock) . catMaybes
+ <$> (mapM bestSocketPath
+ =<< catchDefaultIO [] (dirContents dir))
forM_ sockets cleanup
cleanup socketfile = do
#ifndef mingw32_HOST_OS
@@ -139,8 +156,10 @@ hostport2socket host Nothing = hostport2socket' host
hostport2socket host (Just port) = hostport2socket' $ host ++ "!" ++ show port
hostport2socket' :: String -> FilePath
hostport2socket' s
- | length s > 32 = md5s (Str s)
+ | length s > lengthofmd5s = md5s (Str s)
| otherwise = s
+ where
+ lengthofmd5s = 32
socket2lock :: FilePath -> FilePath
socket2lock socket = socket ++ lockExt
diff --git a/debian/changelog b/debian/changelog
index 6768077fd..cd0923ee9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -7,6 +7,9 @@ git-annex (4.20131003) UNRELEASED; urgency=low
* addurl: Better sanitization of generated filenames.
* Better sanitization of problem characters when generating URL and WORM
keys.
+ * The control socket path passed to ssh needs to be 17 characters
+ shorter than the maximum unix domain socket length, because ssh
+ appends stuff to it to make a temporary filename. Closes: #725512
-- Joey Hess <joeyh@debian.org> Thu, 03 Oct 2013 15:41:24 -0400