From 0e44c252c86c92de8188af01f2e3c5d58f972256 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 17 Mar 2021 09:41:12 -0400 Subject: [PATCH] avoid getting creds from environment during autoenable When autoenabling special remotes of type S3, weddav, or glacier, do not take login credentials from environment variables, as the user may not be expecting the autoenable to happen, and may have those set for other purposes. --- Annex/SpecialRemote.hs | 2 +- CHANGELOG | 4 ++++ Creds.hs | 15 ++++++++++----- Remote/Git.hs | 8 ++++++-- Remote/GitLFS.hs | 1 + Remote/Glacier.hs | 2 +- Remote/Helper/ExportImport.hs | 13 ++++++++----- Remote/S3.hs | 19 +++++++++++-------- Remote/WebDAV.hs | 4 ++-- Types/Remote.hs | 2 +- ..._9e694fdee4a40581a86d21b962f99f25._comment | 16 ++++++++++++++++ doc/special_remotes/S3.mdwn | 13 ++++++------- doc/special_remotes/glacier.mdwn | 4 ++-- doc/special_remotes/webdav.mdwn | 6 +++--- 14 files changed, 72 insertions(+), 37 deletions(-) create mode 100644 doc/bugs/presence_of_AWS_creds_ruins_access_to_public_urls/comment_2_9e694fdee4a40581a86d21b962f99f25._comment diff --git a/Annex/SpecialRemote.hs b/Annex/SpecialRemote.hs index 676fff7047..a7999a7756 100644 --- a/Annex/SpecialRemote.hs +++ b/Annex/SpecialRemote.hs @@ -89,7 +89,7 @@ autoEnable = do (Just name, Right t) -> whenM (canenable u) $ do showSideAction $ "Auto enabling special remote " ++ name dummycfg <- liftIO dummyRemoteGitConfig - tryNonAsync (setup t (Enable c) (Just u) Nothing c dummycfg) >>= \case + tryNonAsync (setup t (AutoEnable c) (Just u) Nothing c dummycfg) >>= \case Left e -> warning (show e) Right (_c, _u) -> when (cu /= u) $ diff --git a/CHANGELOG b/CHANGELOG index a7524723d3..fe34c9abf3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,10 @@ git-annex (8.20210311) UNRELEASED; urgency=medium * import: When the previously exported tree contained a submodule, preserve it in the imported tree so it does not get deleted. * export --json: Fill in the file field. + * When autoenabling special remotes of type S3, weddav, or glacier, + do not take login credentials from environment variables, as the user + may not be expecting the autoenable to happen, and may have those + set for other purposes. -- Joey Hess Fri, 12 Mar 2021 12:06:37 -0400 diff --git a/Creds.hs b/Creds.hs index 86acd016cb..78d38d6648 100644 --- a/Creds.hs +++ b/Creds.hs @@ -1,6 +1,6 @@ {- Credentials storage - - - Copyright 2012-2020 Joey Hess + - Copyright 2012-2021 Joey Hess - - Licensed under the GNU AGPL version 3 or higher. -} @@ -25,6 +25,7 @@ import Annex.Common import qualified Annex import Types.Creds import Types.RemoteConfig +import Types.Remote (SetupStage(..)) import Annex.SpecialRemote.Config import Annex.Perms import Utility.FileMode @@ -52,21 +53,25 @@ data CredPairStorage = CredPairStorage - that. Also caches them locally. - - The creds are found from the CredPairStorage storage if not provided, - - so may be provided by an environment variable etc. + - so may be provided by an environment variable etc. When autoenabling, + - the user would not expect to provide creds, so none are found. - - The remote's configuration should have already had a cipher stored in it - if that's going to be done, so that the creds can be encrypted using the - cipher. The EncryptionIsSetup is witness to that being the case. -} setRemoteCredPair - :: EncryptionIsSetup + :: SetupStage + -> EncryptionIsSetup -> ParsedRemoteConfig -> RemoteGitConfig -> CredPairStorage -> Maybe CredPair -> Annex RemoteConfig -setRemoteCredPair encsetup pc gc storage mcreds = unparsedRemoteConfig <$> - setRemoteCredPair' pc encsetup gc storage mcreds +setRemoteCredPair ss encsetup pc gc storage mcreds = case ss of + AutoEnable _ -> pure (unparsedRemoteConfig pc) + _ -> unparsedRemoteConfig <$> + setRemoteCredPair' pc encsetup gc storage mcreds setRemoteCredPair' :: ParsedRemoteConfig diff --git a/Remote/Git.hs b/Remote/Git.hs index 596c259c56..8cf05f53cc 100644 --- a/Remote/Git.hs +++ b/Remote/Git.hs @@ -136,7 +136,11 @@ gitSetup Init mu _ c _ = do if isNothing mu || mu == Just u then return (c, u) else error "git remote did not have specified uuid" -gitSetup (Enable _) (Just u) _ c _ = do +gitSetup (Enable _) mu _ c _ = enableRemote mu c +gitSetup (AutoEnable _) mu _ c _ = enableRemote mu c + +enableRemote :: Maybe UUID -> RemoteConfig -> Annex (RemoteConfig, UUID) +enableRemote (Just u) c = do inRepo $ Git.Command.run [ Param "remote" , Param "add" @@ -144,7 +148,7 @@ gitSetup (Enable _) (Just u) _ c _ = do , Param $ maybe (giveup "no location") fromProposedAccepted (M.lookup locationField c) ] return (c, u) -gitSetup (Enable _) Nothing _ _ _ = error "unable to enable git remote with no specified uuid" +enableRemote Nothing _ = error "unable to enable git remote with no specified uuid" {- It's assumed to be cheap to read the config of non-URL remotes, so this is - done each time git-annex is run in a way that uses remotes, unless diff --git a/Remote/GitLFS.hs b/Remote/GitLFS.hs index 31c35456f3..2cee5b1244 100644 --- a/Remote/GitLFS.hs +++ b/Remote/GitLFS.hs @@ -150,6 +150,7 @@ mySetup ss mu _ c gc = do let failinitunlessforced msg = case ss of Init -> unlessM (Annex.getState Annex.force) (giveup msg) Enable _ -> noop + AutoEnable _ -> noop case (isEncrypted pc, Git.GCrypt.urlPrefix `isPrefixOf` url) of (False, False) -> noop (True, True) -> Remote.GCrypt.setGcryptEncryption pc remotename diff --git a/Remote/Glacier.hs b/Remote/Glacier.hs index ee1377ae68..91f54212f0 100644 --- a/Remote/Glacier.hs +++ b/Remote/Glacier.hs @@ -123,7 +123,7 @@ glacierSetup' ss u mcreds c gc = do (c', encsetup) <- encryptionSetup (c `M.union` defaults) gc pc <- either giveup return . parseRemoteConfig c' =<< configParser remote c' - c'' <- setRemoteCredPair encsetup pc gc (AWS.creds u) mcreds + c'' <- setRemoteCredPair ss encsetup pc gc (AWS.creds u) mcreds pc' <- either giveup return . parseRemoteConfig c'' =<< configParser remote c'' case ss of diff --git a/Remote/Helper/ExportImport.hs b/Remote/Helper/ExportImport.hs index d9ca78b70d..dbe6ef1ffe 100644 --- a/Remote/Helper/ExportImport.hs +++ b/Remote/Helper/ExportImport.hs @@ -87,11 +87,8 @@ adjustExportImportRemoteType rt = rt { setup = setup' } | configured pc && encryptionIsEnabled pc -> giveup $ "cannot enable both encryption and " ++ fromProposedAccepted configfield | otherwise -> cont - Enable oldc -> do - oldpc <- parsedRemoteConfig rt oldc - if configured pc /= configured oldpc - then giveup $ "cannot change " ++ fromProposedAccepted configfield ++ " of existing special remote" - else cont + Enable oldc -> enable oldc pc configured configfield cont + AutoEnable oldc -> enable oldc pc configured configfield cont , if configured pc then giveup $ fromProposedAccepted configfield ++ " is not supported by this special remote" else cont @@ -99,6 +96,12 @@ adjustExportImportRemoteType rt = rt { setup = setup' } checkconfig exportSupported exportTree exportTreeField $ checkconfig importSupported importTree importTreeField $ setup rt st mu cp c gc + + enable oldc pc configured configfield cont = do + oldpc <- parsedRemoteConfig rt oldc + if configured pc /= configured oldpc + then giveup $ "cannot change " ++ fromProposedAccepted configfield ++ " of existing special remote" + else cont -- | Adjust a remote to support exporttree=yes and/or importree=yes. adjustExportImport :: Remote -> RemoteStateHandle -> Annex Remote diff --git a/Remote/S3.hs b/Remote/S3.hs index 1005b840c8..53dd9f3e6a 100644 --- a/Remote/S3.hs +++ b/Remote/S3.hs @@ -273,7 +273,7 @@ s3Setup' ss u mcreds c gc (c', encsetup) <- encryptionSetup (c `M.union` defaults) gc pc <- either giveup return . parseRemoteConfig c' =<< configParser remote c' - c'' <- setRemoteCredPair encsetup pc gc (AWS.creds u) mcreds + c'' <- setRemoteCredPair ss encsetup pc gc (AWS.creds u) mcreds pc' <- either giveup return . parseRemoteConfig c'' =<< configParser remote c'' info <- extractS3Info pc' @@ -287,14 +287,14 @@ s3Setup' ss u mcreds c gc showNote "Internet Archive mode" pc <- either giveup return . parseRemoteConfig c =<< configParser remote c - c' <- setRemoteCredPair noEncryptionUsed pc gc (AWS.creds u) mcreds + c' <- setRemoteCredPair ss noEncryptionUsed pc gc (AWS.creds u) mcreds -- Ensure user enters a valid bucket name, since -- this determines the name of the archive.org item. let validbucket = replace " " "-" $ map toLower $ maybe (giveup "specify bucket=") fromProposedAccepted (M.lookup bucketField c') let archiveconfig = - -- IA acdepts x-amz-* as an alias for x-archive-* + -- IA accepts x-amz-* as an alias for x-archive-* M.mapKeys (Proposed . replace "x-archive-" "x-amz-" . fromProposedAccepted) $ -- encryption does not make sense here M.insert encryptionField (Proposed "none") $ @@ -1273,11 +1273,8 @@ enableBucketVersioning ss info _ _ _ = do case ss of Init -> when (versioning info) $ enableversioning (bucket info) - Enable oldc -> do - oldpc <- parsedRemoteConfig remote oldc - oldinfo <- extractS3Info oldpc - when (versioning info /= versioning oldinfo) $ - giveup "Cannot change versioning= of existing S3 remote." + Enable oldc -> checkunchanged oldc + AutoEnable oldc -> checkunchanged oldc where enableversioning b = do #if MIN_VERSION_aws(0,21,1) @@ -1296,6 +1293,12 @@ enableBucketVersioning ss info _ _ _ = do ] #endif + checkunchanged oldc = do + oldpc <- parsedRemoteConfig remote oldc + oldinfo <- extractS3Info oldpc + when (versioning info /= versioning oldinfo) $ + giveup "Cannot change versioning= of existing S3 remote." + -- If the remote has versioning enabled, but the version ID is for some -- reason not being recorded, it's not safe to perform an action that -- will remove the unversioned file. The file may be the only copy of an diff --git a/Remote/WebDAV.hs b/Remote/WebDAV.hs index 31960f2b01..3ffa2dc049 100644 --- a/Remote/WebDAV.hs +++ b/Remote/WebDAV.hs @@ -128,7 +128,7 @@ gen r u rc gc rs = do chunkconfig = getChunkConfig c webdavSetup :: SetupStage -> Maybe UUID -> Maybe CredPair -> RemoteConfig -> RemoteGitConfig -> Annex (RemoteConfig, UUID) -webdavSetup _ mu mcreds c gc = do +webdavSetup ss mu mcreds c gc = do u <- maybe (liftIO genUUID) return mu url <- maybe (giveup "Specify url=") (return . fromProposedAccepted) @@ -138,7 +138,7 @@ webdavSetup _ mu mcreds c gc = do creds <- maybe (getCreds pc gc u) (return . Just) mcreds testDav url creds gitConfigSpecialRemote u c' [("webdav", "true")] - c'' <- setRemoteCredPair encsetup pc gc (davCreds u) creds + c'' <- setRemoteCredPair ss encsetup pc gc (davCreds u) creds return (c'', u) store :: DavHandleVar -> ChunkConfig -> Storer diff --git a/Types/Remote.hs b/Types/Remote.hs index 5286b26ece..ff632fad83 100644 --- a/Types/Remote.hs +++ b/Types/Remote.hs @@ -48,7 +48,7 @@ import Utility.SafeCommand import Utility.Url import Utility.DataUnits -data SetupStage = Init | Enable RemoteConfig +data SetupStage = Init | Enable RemoteConfig | AutoEnable RemoteConfig {- There are different types of remotes. -} data RemoteTypeA a = RemoteType diff --git a/doc/bugs/presence_of_AWS_creds_ruins_access_to_public_urls/comment_2_9e694fdee4a40581a86d21b962f99f25._comment b/doc/bugs/presence_of_AWS_creds_ruins_access_to_public_urls/comment_2_9e694fdee4a40581a86d21b962f99f25._comment new file mode 100644 index 0000000000..2a46ccc07e --- /dev/null +++ b/doc/bugs/presence_of_AWS_creds_ruins_access_to_public_urls/comment_2_9e694fdee4a40581a86d21b962f99f25._comment @@ -0,0 +1,16 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 2""" + date="2021-03-17T13:37:01Z" + content=""" +Ok, I've made autoenable not take creds from the environment, which will +avoid the problem. + +If there are any external special remotes that might behave similarly, +it would need an extension to the external special remote protocol to +support them. Currently `INITREMOTE` is sent during auto-enable, and so +the protocol would need to have `ENABLEREMOTE` and `AUTOENBLEREMOTE` added +to it. Since that would need an extension and I don't know if any externals +actually look at env vars etc at (auto)enable time, I've skipped doing it +for now. +"""]] diff --git a/doc/special_remotes/S3.mdwn b/doc/special_remotes/S3.mdwn index 5b8da9971c..71d74a533a 100644 --- a/doc/special_remotes/S3.mdwn +++ b/doc/special_remotes/S3.mdwn @@ -7,13 +7,12 @@ for usage examples. ## configuration -The standard environment variables `AWS_ACCESS_KEY_ID` and -`AWS_SECRET_ACCESS_KEY` are used to supply login credentials -for S3. You need to set these only when running -`git annex initremote`, as they will be cached in a file only you -can read inside the local git repository. If you’re working with -temporary security credentials, you can also set the `AWS_SESSION_TOKEN` -environment variable. +The standard environment variables `AWS_ACCESS_KEY_ID` and +`AWS_SECRET_ACCESS_KEY` are used to supply login credentials for S3. You +need to set these only when running `git annex initremote` (or +`enableremote`), as they will be cached in a file only you can read inside +the local git repository. If you’re working with temporary security +credentials, you can also set the `AWS_SESSION_TOKEN` environment variable. A number of parameters can be passed to `git annex initremote` to configure the S3 remote. diff --git a/doc/special_remotes/glacier.mdwn b/doc/special_remotes/glacier.mdwn index fe784c4531..5dde37208c 100644 --- a/doc/special_remotes/glacier.mdwn +++ b/doc/special_remotes/glacier.mdwn @@ -15,8 +15,8 @@ download the data. The standard environment variables `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` are used to supply login credentials for Amazon. You need to set these only when running -`git annex initremote`, as they will be cached in a file only you -can read inside the local git repository. +`git annex initremote` (or `enableremote`), as they will be cached in +a file only you can read inside the local git repository. A number of parameters can be passed to `git annex initremote` to configure the Glacier remote. diff --git a/doc/special_remotes/webdav.mdwn b/doc/special_remotes/webdav.mdwn index 27bd38579d..5acf99a054 100644 --- a/doc/special_remotes/webdav.mdwn +++ b/doc/special_remotes/webdav.mdwn @@ -3,9 +3,9 @@ This special remote type stores file contents in a WebDAV server. ## configuration The environment variables `WEBDAV_USERNAME` and `WEBDAV_PASSWORD` are used -to supply login credentials. You need to set these only when running -`git annex initremote`, as they will be cached in a file only you -can read inside the local git repository. +to supply login credentials. You need to set these only when running `git +annex initremote` (or `enableremote`), as they will be cached in a file +only you can read inside the local git repository. A number of parameters can be passed to `git annex initremote` to configure the webdav remote.