From e62c4543c31a61186ebf2e4e0412df59fc8630c8 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 17 Jun 2018 14:46:22 -0400 Subject: [PATCH] default to not using youtube-dl, for security Pity, but same reasoning as curl applies to it. This commit was sponsored by Peter on Patreon. --- Annex/Url.hs | 5 +++ Annex/YoutubeDl.hs | 36 +++++++++++++++---- CHANGELOG | 9 ++--- NEWS | 9 ++--- doc/git-annex-addurl.mdwn | 9 +++-- doc/git-annex-importfeed.mdwn | 5 ++- doc/git-annex.mdwn | 6 ++-- doc/tips/downloading_podcasts.mdwn | 4 +++ .../using_the_web_as_a_special_remote.mdwn | 4 +++ 9 files changed, 66 insertions(+), 21 deletions(-) diff --git a/Annex/Url.hs b/Annex/Url.hs index 7b6e4ec8f0..c91c54dac3 100644 --- a/Annex/Url.hs +++ b/Annex/Url.hs @@ -11,6 +11,7 @@ module Annex.Url ( withUrlOptions, getUrlOptions, getUserAgent, + httpAddressesUnlimited, ) where import Annex.Common @@ -84,5 +85,9 @@ getUrlOptions = Annex.getState Annex.urloptions >>= \case restrictManagerSettings r U.managerSettings return (U.DownloadWithConduit, manager) +httpAddressesUnlimited :: Annex Bool +httpAddressesUnlimited = + ("all" == ) . annexAllowedHttpAddresses <$> Annex.getGitConfig + withUrlOptions :: (U.UrlOptions -> Annex a) -> Annex a withUrlOptions a = a =<< getUrlOptions diff --git a/Annex/YoutubeDl.hs b/Annex/YoutubeDl.hs index 4f8e699c23..cb4f4e23ff 100644 --- a/Annex/YoutubeDl.hs +++ b/Annex/YoutubeDl.hs @@ -1,6 +1,6 @@ {- youtube-dl integration for git-annex - - - Copyright 2017 Joey Hess + - Copyright 2017-2018 Joey Hess - - Licensed under the GNU GPL version 3 or higher. -} @@ -27,6 +27,12 @@ import Logs.Transfer import Network.URI import Control.Concurrent.Async +-- youtube-dl is can follow redirects to anywhere, including potentially +-- localhost or a private address. So, it's only allowed to be used if the +-- user has allowed access to all addresses. +youtubeDlAllowed :: Annex Bool +youtubeDlAllowed = httpAddressesUnlimited + -- Runs youtube-dl in a work directory, to download a single media file -- from the url. Reutrns the path to the media file in the work directory. -- @@ -41,7 +47,10 @@ import Control.Concurrent.Async -- (Note that we can't use --output to specifiy the file to download to, -- due to ) youtubeDl :: URLString -> FilePath -> Annex (Either String (Maybe FilePath)) -youtubeDl url workdir = withUrlOptions $ youtubeDl' url workdir +youtubeDl url workdir = ifM httpAddressesUnlimited + ( withUrlOptions $ youtubeDl' url workdir + , return (Right Nothing) + ) youtubeDl' :: URLString -> FilePath -> UrlOptions -> Annex (Either String (Maybe FilePath)) youtubeDl' url workdir uo @@ -110,7 +119,13 @@ youtubeDlMaxSize workdir = ifM (Annex.getState Annex.force) -- Download a media file to a destination, youtubeDlTo :: Key -> URLString -> FilePath -> Annex Bool -youtubeDlTo key url dest = do +youtubeDlTo key url dest = ifM youtubeDlAllowed + ( youtubeDlTo' key url dest + , return False + ) + +youtubeDlTo' :: Key -> URLString -> FilePath -> Annex Bool +youtubeDlTo' key url dest = do res <- withTmpWorkDir key $ \workdir -> youtubeDl url workdir >>= \case Right (Just mediafile) -> do @@ -137,7 +152,10 @@ youtubeDlSupported url = either (const False) id <$> youtubeDlCheck url -- Check if youtube-dl can find media in an url. youtubeDlCheck :: URLString -> Annex (Either String Bool) -youtubeDlCheck = withUrlOptions . youtubeDlCheck' +youtubeDlCheck url = ifM youtubeDlAllowed + ( withUrlOptions $ youtubeDlCheck' url + , return (Right False) + ) youtubeDlCheck' :: URLString -> UrlOptions -> Annex (Either String Bool) youtubeDlCheck' url uo @@ -150,7 +168,10 @@ youtubeDlCheck' url uo -- -- (This is not always identical to the filename it uses when downloading.) youtubeDlFileName :: URLString -> Annex (Either String FilePath) -youtubeDlFileName url = withUrlOptions go +youtubeDlFileName url = ifM youtubeDlAllowed + ( withUrlOptions go + , return nomedia + ) where go uo | supportedScheme uo url = flip catchIO (pure . Left . show) $ @@ -161,7 +182,10 @@ youtubeDlFileName url = withUrlOptions go -- Does not check if the url contains htmlOnly; use when that's already -- been verified. youtubeDlFileNameHtmlOnly :: URLString -> Annex (Either String FilePath) -youtubeDlFileNameHtmlOnly = withUrlOptions . youtubeDlFileNameHtmlOnly' +youtubeDlFileNameHtmlOnly url = ifM youtubeDlAllowed + ( withUrlOptions $ youtubeDlFileNameHtmlOnly' url + , return (Left "no media in url") + ) youtubeDlFileNameHtmlOnly' :: URLString -> UrlOptions -> Annex (Either String FilePath) youtubeDlFileNameHtmlOnly' url uo diff --git a/CHANGELOG b/CHANGELOG index a16f303289..fc08d51590 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -11,10 +11,11 @@ git-annex (6.20180622) UNRELEASED; urgency=high 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 the interfaces to curl and youtube-dl do not have a way to + prevent them from accessing localhost or private IP addresses, + they default to not being used for url downloads. + Only when annex.security.allowed-http-addresses=all will curl and + youtube-dl be used. -- Joey Hess Wed, 30 May 2018 11:49:08 -0400 diff --git a/NEWS b/NEWS index f9f42fc9e3..24672be5a1 100644 --- a/NEWS +++ b/NEWS @@ -7,14 +7,15 @@ git-annex (6.20180622) upstream; urgency=high A related security fix prevents git-annex from connecting to http servers on localhost or private networks. This can be overridden, at your own risk, using annex.security.allowed-http-addresses. + + Setting annex.web-options no longer is enough to make curl be used, + and youtube-dl is also no longer used by default. See the + documentation of annex.security.allowed-http-addresses for + details and how to enable them. The annex.web-download-command configuration has been removed, use annex.web-options instead. - Setting annex.web-options no longer is enough to make curl be used. - See the documentation of annex.security.allowed-http-addresses for - why and how to enable curl. - -- Joey Hess Fri, 15 Jun 2018 17:54:23 -0400 git-annex (6.20180309) upstream; urgency=medium diff --git a/doc/git-annex-addurl.mdwn b/doc/git-annex-addurl.mdwn index 4748073ed3..7947edc223 100644 --- a/doc/git-annex-addurl.mdwn +++ b/doc/git-annex-addurl.mdwn @@ -10,9 +10,12 @@ git annex addurl `[url ...]` Downloads each url to its own file, which is added to the annex. -When `youtube-dl` is installed, it's used to check for a video embedded in -a web page at the url, and that is added to the annex instead. - +When `youtube-dl` is installed, it can be used to check for a video +embedded in a web page at the url, and that is added to the annex instead. +(However, this is disabled by default as it can be a security risk. +See the documentation of annex.security.allowed-http-addresses +in [[git-annex]](1) for details.) + Urls to torrent files (including magnet links) will cause the content of the torrent to be downloaded, using `aria2c`. diff --git a/doc/git-annex-importfeed.mdwn b/doc/git-annex-importfeed.mdwn index 2f146fbfe0..d70b02cd90 100644 --- a/doc/git-annex-importfeed.mdwn +++ b/doc/git-annex-importfeed.mdwn @@ -13,8 +13,11 @@ content has not already been added to the repository before, so you can delete, rename, etc the resulting files and repeated runs won't duplicate them. -When `youtube-dl` is installed, it's used to download links in the feed. +When `youtube-dl` is installed, it can be used to download links in the feed. This allows importing e.g., YouTube playlists. +(However, this is disabled by default as it can be a security risk. +See the documentation of annex.security.allowed-http-addresses +in [[git-annex]](1) for details.) To make the import process add metadata to the imported files from the feed, `git config annex.genmetadata true` diff --git a/doc/git-annex.mdwn b/doc/git-annex.mdwn index 58f7e09392..c43c8012a9 100644 --- a/doc/git-annex.mdwn +++ b/doc/git-annex.mdwn @@ -1421,9 +1421,9 @@ Here are all the supported configuration settings. 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. + Note that, since the interfaces of curl and youtube-dl do not allow + these IP address restrictions to be enforced, curl and youtube-dl will + never be used unless annex.security.allowed-http-addresses=all. * `annex.secure-erase-command` diff --git a/doc/tips/downloading_podcasts.mdwn b/doc/tips/downloading_podcasts.mdwn index 7416281d25..2c523606f0 100644 --- a/doc/tips/downloading_podcasts.mdwn +++ b/doc/tips/downloading_podcasts.mdwn @@ -84,6 +84,10 @@ manually. For a channel url like "https://www.youtube.com/channel/$foo", the feed is "https://www.youtube.com/feeds/videos.xml?channel_id=$foo" +Use of youtube-dl is disabled by default as it can be a security risk. +See the documentation of annex.security.allowed-http-addresses +in [[git-annex]] for details.) + ## metadata As well as storing the urls for items imported from a feed, git-annex can diff --git a/doc/tips/using_the_web_as_a_special_remote.mdwn b/doc/tips/using_the_web_as_a_special_remote.mdwn index 5e066c9183..a206879c97 100644 --- a/doc/tips/using_the_web_as_a_special_remote.mdwn +++ b/doc/tips/using_the_web_as_a_special_remote.mdwn @@ -78,6 +78,10 @@ When you have youtube-dl installed, you can just `git annex addurl http://youtube.com/foo` and it will detect that it is a video and download the video content for offline viewing. +(However, this is disabled by default as it can be a security risk. +See the documentation of annex.security.allowed-http-addresses +in [[git-annex]] for details.) + Later, in another clone of the repository, you can run `git annex get` on the file and it will also be downloaded with youtube-dl. This works even if the video host has transcoded or otherwise changed the video