prevent http connections to localhost and private ips by default

Security fix!

* git-annex will refuse to download content from http servers on
  localhost, or any private IP addresses, to prevent accidental
  exposure of internal data. This can be overridden with the
  annex.security.allowed-http-addresses setting.
* Since curl's interface does not have a way to prevent it from accessing
  localhost or private IP addresses, curl defaults to not being used
  for url downloads, even if annex.web-options enabled it before.
  Only when annex.security.allowed-http-addresses=all will curl be used.

Since S3 and WebDav use the Manager, the same policies apply to them too.

youtube-dl is not handled yet, and a http proxy configuration can bypass
these checks too. Those cases are still TBD.

This commit was sponsored by Jeff Goeke-Smith on Patreon.

Annex/Url.hs

@@ -1,5 +1,5 @@
 {- Url downloading, with git-annex user agent and configured http
- - headers and curl options.
+ - headers, security restrictions, etc.
  -
  - Copyright 2013-2018 Joey Hess <id@joeyh.name>
  -
@@ -16,8 +16,12 @@ module Annex.Url (
 import Annex.Common
 import qualified Annex
 import Utility.Url as U
+import Utility.IPAddress
+import Utility.HttpManagerRestricted
 import qualified BuildInfo
 
+import Network.Socket
+
 defaultUserAgent :: U.UserAgent
 defaultUserAgent = "git-annex/" ++ BuildInfo.packageversion
 
@@ -34,16 +38,51 @@ getUrlOptions = Annex.getState Annex.urloptions >>= \case
 			{ Annex.urloptions = Just uo }
 		return uo
   where
-	mk = mkUrlOptions
-		<$> getUserAgent
-		<*> headers
-		<*> options
-		<*> liftIO (U.newManager U.managerSettings)
-		<*> (annexAllowedUrlSchemes <$> Annex.getGitConfig)
+	mk = do
+		(urldownloader, manager) <- checkallowedaddr
+		mkUrlOptions
+			<$> getUserAgent
+			<*> headers
+			<*> pure urldownloader
+			<*> pure manager
+			<*> (annexAllowedUrlSchemes <$> Annex.getGitConfig)
+	
 	headers = annexHttpHeadersCommand <$> Annex.getGitConfig >>= \case
 		Just cmd -> lines <$> liftIO (readProcess "sh" ["-c", cmd])
 		Nothing -> annexHttpHeaders <$> Annex.getGitConfig
-	options = map Param . annexWebOptions <$> Annex.getGitConfig
+	
+	checkallowedaddr = words . annexAllowedHttpAddresses <$> Annex.getGitConfig >>= \case
+		["all"] -> do
+			-- Only allow curl when all are allowed,
+			-- as its interface does not allow preventing
+			-- it from accessing specific IP addresses.
+			curlopts <- map Param . annexWebOptions <$> Annex.getGitConfig
+			let urldownloader = if null curlopts
+				then U.DownloadWithCurl curlopts
+				else U.DownloadWithConduit
+			manager <- liftIO $ U.newManager U.managerSettings
+			return (urldownloader, manager)
+		allowedaddrs -> do
+			addrmatcher <- liftIO $ 
+				(\l v -> any (\f -> f v) l) . catMaybes
+					<$> mapM makeAddressMatcher allowedaddrs
+			-- Default to not allowing access to loopback
+			-- and private IP addresses to avoid data
+			-- leakage.
+			let isallowed addr
+				| addrmatcher addr = True
+				| isLoopbackAddress addr = False
+				| isPrivateAddress addr = False
+				| otherwise = True
+			let r = Restriction
+				{ addressRestriction = \addr ->
+					if isallowed (addrAddress addr)
+						then Nothing
+						else Just (addrConnectionRestricted addr)
+				}
+			manager <- liftIO $ U.newManager $
+				restrictManagerSettings r U.managerSettings
+			return (U.DownloadWithConduit, manager)
 
 withUrlOptions :: (U.UrlOptions -> Annex a) -> Annex a
 withUrlOptions a = a =<< getUrlOptions

CHANGELOG

@@ -7,6 +7,14 @@ git-annex (6.20180622) UNRELEASED; urgency=high
     supporting annex.security.allowed-url-schemes across redirects.
     If you used this setting, you may want to instead use annex.web-options
     to pass options to curl.
+  * git-annex will refuse to download content from http servers on
+    localhost, or any private IP addresses, to prevent accidental
+    exposure of internal data. This can be overridden with the
+    annex.security.allowed-http-addresses setting.
+  * Since curl's interface does not have a way to prevent it from accessing
+    localhost or private IP addresses, curl defaults to not being used
+    for url downloads, even if annex.web-options enabled it before.
+    Only when annex.security.allowed-http-addresses=all will curl be used.
  
  -- Joey Hess <id@joeyh.name>  Wed, 30 May 2018 11:49:08 -0400
 

Types/GitConfig.hs

@@ -95,6 +95,7 @@ data GitConfig = GitConfig
 	, annexRetry :: Maybe Integer
 	, annexRetryDelay :: Maybe Seconds
 	, annexAllowedUrlSchemes :: S.Set Scheme
+	, annexAllowedHttpAddresses :: String
 	, coreSymlinks :: Bool
 	, coreSharedRepository :: SharedRepository
 	, receiveDenyCurrentBranch :: DenyCurrentBranch
@@ -163,6 +164,8 @@ extractGitConfig r = GitConfig
 	, annexAllowedUrlSchemes = S.fromList $ map mkScheme $
 		maybe ["http", "https"] words $
 			getmaybe (annex "security.allowed-url-schemes")
+	, annexAllowedHttpAddresses = fromMaybe "" $
+		getmaybe (annex "security.allowed-http-addresses")
 	, coreSymlinks = getbool "core.symlinks" True
 	, coreSharedRepository = getSharedRepository r
 	, receiveDenyCurrentBranch = getDenyCurrentBranch r

Utility/Url.hs

@@ -18,6 +18,7 @@ module Utility.Url (
 	Scheme,
 	mkScheme,
 	allowedScheme,
+	UrlDownloader(..),
 	UrlOptions(..),
 	defUrlOptions,
 	mkUrlOptions,
@@ -37,6 +38,7 @@ module Utility.Url (
 
 import Common
 import Utility.Metered
+import Utility.HttpManagerRestricted
 
 import Network.URI
 import Network.HTTP.Types
@@ -100,19 +102,10 @@ defUrlOptions = UrlOptions
 	<*> newManager managerSettings
 	<*> pure (S.fromList $ map mkScheme ["http", "https"])
 
-mkUrlOptions :: Maybe UserAgent -> Headers -> [CommandParam] -> Manager -> S.Set Scheme -> UrlOptions
-mkUrlOptions defuseragent reqheaders reqparams manager =
+mkUrlOptions :: Maybe UserAgent -> Headers -> UrlDownloader -> Manager -> S.Set Scheme -> UrlOptions
+mkUrlOptions defuseragent reqheaders urldownloader manager =
 	UrlOptions useragent reqheaders urldownloader applyrequest manager
   where
-	urldownloader = if null reqparams
-#if MIN_VERSION_cryptonite(0,6,0)
-		then DownloadWithConduit
-#else
-		-- Work around for old cryptonite bug that broke tls.
-		-- https://github.com/vincenthz/hs-tls/issues/109
-		then DownloadWithCurl reqparams
-#endif
-		else DownloadWithCurl reqparams
 	applyrequest = \r -> r { requestHeaders = requestHeaders r ++ addedheaders }
 	addedheaders = uaheader ++ otherheaders
 	useragent = maybe defuseragent (Just . B8.toString . snd)
@@ -154,6 +147,12 @@ checkPolicy uo u onerr a
 		hFlush stderr
 		return onerr
 
+unsupportedUrlScheme :: URI -> IO ()
+unsupportedUrlScheme u = do
+	hPutStrLn stderr $ 
+		"Unsupported url scheme" ++ show u
+	hFlush stderr
+
 allowedScheme :: UrlOptions -> URI -> Bool
 allowedScheme uo u = uscheme `S.member` allowedSchemes uo
   where
@@ -194,31 +193,24 @@ getUrlInfo :: URLString -> UrlOptions -> IO UrlInfo
 getUrlInfo url uo = case parseURIRelaxed url of
 	Just u -> checkPolicy uo u dne $
 		case (urlDownloader uo, parseUrlConduit (show u)) of
-		(DownloadWithConduit, Just req) -> catchJust
-			-- When http redirects to a protocol which 
-			-- conduit does not support, it will throw
-			-- a StatusCodeException with found302.
-			(matchStatusCodeException (== found302))
-			(existsconduit req)
-			(const (existscurl u))
-			`catchNonAsync` (const $ return dne)
-		-- http-conduit does not support file:, ftp:, etc urls,
-		-- so fall back to reading files and using curl.
-		_
-			| uriScheme u == "file:" -> do
-				let f = unEscapeString (uriPath u)
-				s <- catchMaybeIO $ getFileStatus f
-				case s of
-					Just stat -> do
-						sz <- getFileSize' f stat
-						found (Just sz) Nothing
-					Nothing -> return dne
-			| otherwise -> existscurl u
+			(DownloadWithConduit, Just req) ->
+				existsconduit req
+					`catchNonAsync` (const $ return dne)
+			(DownloadWithConduit, Nothing)
+				| isfileurl u -> existsfile u
+				| otherwise -> do
+					unsupportedUrlScheme u
+					return dne
+			(DownloadWithCurl _, _) 
+				| isfileurl u -> existsfile u
+				| otherwise -> existscurl u
 	Nothing -> return dne
   where
 	dne = UrlInfo False Nothing Nothing
 	found sz f = return $ UrlInfo True sz f
 
+	isfileurl u = uriScheme u == "file:"
+
 	curlparams = curlParams uo $
 		[ Param "-s"
 		, Param "--head"
@@ -266,6 +258,15 @@ getUrlInfo url uo = case parseURIRelaxed url of
 			-- got a length, it's good
 			_ | isftp && isJust len -> good
 			_ -> return dne
+	
+	existsfile u = do
+		let f = unEscapeString (uriPath u)
+		s <- catchMaybeIO $ getFileStatus f
+		case s of
+			Just stat -> do
+				sz <- getFileSize' f stat
+				found (Just sz) Nothing
+			Nothing -> return dne
 
 -- Parse eg: attachment; filename="fname.ext"
 -- per RFC 2616
@@ -287,10 +288,6 @@ headRequest r = r
 	}
 
 {- Download a perhaps large file, with auto-resume of incomplete downloads.
- -
- - By default, conduit is used for the download, except for file: urls,
- - which are copied. If the url scheme is not supported by conduit, falls
- - back to using curl.
  -
  - Displays error message on stderr when download failed.
  -}
@@ -302,21 +299,19 @@ download meterupdate url file uo =
 	go = case parseURIRelaxed url of
 		Just u -> checkPolicy uo u False $
 			case (urlDownloader uo, parseUrlConduit (show u)) of
-			(DownloadWithConduit, Just req) -> catchJust
-				-- When http redirects to a protocol which 
-				-- conduit does not support, it will throw
-				-- a StatusCodeException with found302.
-				(matchStatusCodeException (== found302))
-				(downloadconduit req)
-				(const downloadcurl)
-			_
-				| uriScheme u == "file:" -> do
-					let src = unEscapeString (uriPath u)
-					withMeteredFile src meterupdate $
-						L.writeFile file
-					return True
-				| otherwise -> downloadcurl
+				(DownloadWithConduit, Just req) ->
+					downloadconduit req
+				(DownloadWithConduit, Nothing)
+					| isfileurl u -> downloadfile u
+					| otherwise -> do
+						unsupportedUrlScheme u
+						return False
+				(DownloadWithCurl _, _)
+					| isfileurl u -> downloadfile u
+					| otherwise -> downloadcurl
 		Nothing -> return False
+	
+	isfileurl u = uriScheme u == "file:"
 
 	downloadconduit req = catchMaybeIO (getFileSize file) >>= \case
 		Nothing -> runResourceT $ do
@@ -369,6 +364,10 @@ download meterupdate url file uo =
 		let msg = case he of
 			HttpExceptionRequest _ (StatusCodeException _ msgb) ->
 				B8.toString msgb
+			HttpExceptionRequest _ (InternalException ie) -> 
+				case fromException ie of
+					Nothing -> show ie
+					Just (ConnectionRestricted why) -> why
 			HttpExceptionRequest _ other -> show other
 			_ -> show he
 #else
@@ -401,6 +400,12 @@ download meterupdate url file uo =
 			, Param "-C", Param "-"
 			]
 		boolSystem "curl" (ps ++ [Param "-o", File file, File url])
+					
+	downloadfile u = do
+		let src = unEscapeString (uriPath u)
+		withMeteredFile src meterupdate $
+			L.writeFile file
+		return True
 
 {- Sinks a Response's body to a file. The file can either be opened in
  - WriteMode or AppendMode. Updates the meter as data is received.

doc/git-annex.mdwn

@@ -1356,12 +1356,16 @@ Here are all the supported configuration settings.
 
 * `annex.web-options`
 
-  Setting this makes git-annex use curl to download urls
+  Options to pass to curl when git-annex uses it to download urls
   (rather than the default built-in url downloader).
 
   For example, to force IPv4 only, set it to "-4".
   Or to make curl use your ~/.netrc file, set it to "--netrc".
 
+  Setting this option makes git-annex use curl, but only
+  when annex.security.allowed-http-addresses is configured in a
+  specific way. See its documentation.
+
 * `annex.youtube-dl-options`
 
   Options to pass to youtube-dl when using it to find the url to download
@@ -1393,15 +1397,34 @@ Here are all the supported configuration settings.
   The default is "http https".
 
   Think very carefully before changing this; there are security
-  implications. For example, if it's changed to allow "file" URLs,
-  then anyone who can get a commit into your git-annex repository
-  could add a pointer to a private file located outside that repository, 
-  risking it being copied into the repository and transferred on to other
-  remotes, exposing its content.
+  implications. For example, if it's changed to allow "file" URLs, then
+  anyone who can get a commit into your git-annex repository could
+  `git-annex addurl` a pointer to a private file located outside that
+  repository, possibly causing it to be copied into your repository
+  and transferred on to other remotes, exposing its content.
 
   Some special remotes support their own domain-specific URL
   schemes; those are not affected by this configuration setting.
 
+* `annex.security.allowed-http-addresses`
+
+  By default, git-annex only makes HTTP connections to public IP addresses;
+  it will refuse to use HTTP servers on localhost or on a private network.
+
+  This setting can override that behavior, allowing access to particular
+  IP addresses. For example "127.0.0.1 ::1" allows access to localhost
+  (both IPV4 and IPV6). To allow access to all IP addresses, use "all"
+  
+  Think very carefully before changing this; there are security
+  implications. Anyone who can get a commit into your git-annex repository
+  could `git annex addurl` an url on a private http server, possibly
+  causing it to be downloaded into your repository transferred to
+  other remotes, exposing its content.
+
+  Note that, since curl's interface does not allow these IP address
+  restrictions to be enforced, any configuration that enables use of curl
+  will be ignored unless annex.security.allowed-http-addresses=all.
+
 * `annex.secure-erase-command`
 
   This can be set to a command that should be run whenever git-annex