]> git.saurik.com Git - apt.git/commitdiff
handle weak-security repositories as unauthenticated
authorDavid Kalnischkies <david@kalnischkies.de>
Thu, 17 Mar 2016 15:36:14 +0000 (16:36 +0100)
committerDavid Kalnischkies <david@kalnischkies.de>
Wed, 22 Jun 2016 12:05:01 +0000 (14:05 +0200)
APT can be forced to deal with repositories which have no security
features whatsoever, so just giving up on repositories which "just" fail
our current criteria of good security features is the wrong incentive.

Of course, repositories are better of fixing their setup to provide the
minimum of security features, but sometimes this isn't possible:
Historic repositories for example which do not change (anymore).

That also fixes problem with repositories which are marked as trusted,
but are providing only weak security features which would fail the
parsing of the Release file.

Closes: 827364
apt-pkg/acquire-item.cc
apt-pkg/deb/debmetaindex.cc
test/integration/test-apt-update-weak-hashes

index 699c306035dc937af7ce04c3384c233569df3eba..6f479cceff255e3c88ea4dbc6f405f89c154e42c 100644 (file)
@@ -1149,6 +1149,8 @@ bool pkgAcqMetaBase::CheckAuthDone(string const &Message)         /*{{{*/
    // valid signature from a key in the trusted keyring.  We
    // perform additional verification of its contents, and use them
    // to verify the indexes we are about to download
+   if (_config->FindB("Debug::pkgAcquire::Auth", false))
+      std::cerr << "Signature verification succeeded: " << DestFile << std::endl;
 
    if (TransactionManager->IMSHit == false)
    {
@@ -1169,7 +1171,9 @@ bool pkgAcqMetaBase::CheckAuthDone(string const &Message)         /*{{{*/
       LoadLastMetaIndexParser(TransactionManager, FinalRelease, FinalInRelease);
    }
 
-   if (TransactionManager->MetaIndexParser->Load(DestFile, &ErrorText) == false)
+   bool const GoodAuth = TransactionManager->MetaIndexParser->Load(DestFile, &ErrorText);
+   if (GoodAuth == false && AllowInsecureRepositories(_("The repository '%s' provides only weak security information."),
+                                                        Target.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == false)
    {
       Status = StatAuthError;
       return false;
@@ -1181,14 +1185,10 @@ bool pkgAcqMetaBase::CheckAuthDone(string const &Message)               /*{{{*/
       return false;
    }
 
-   if (_config->FindB("Debug::pkgAcquire::Auth", false))
-      std::cerr << "Signature verification succeeded: "
-                << DestFile << std::endl;
-
    // Download further indexes with verification
-   TransactionManager->QueueIndexes(true);
+   TransactionManager->QueueIndexes(GoodAuth);
 
-   return true;
+   return GoodAuth;
 }
                                                                        /*}}}*/
 void pkgAcqMetaClearSig::QueueIndexes(bool const verify)                       /*{{{*/
@@ -1197,8 +1197,14 @@ void pkgAcqMetaClearSig::QueueIndexes(bool const verify)                 /*{{{*/
    ExpectedAdditionalItems = 0;
 
    std::set<std::string> targetsSeen;
-   bool const metaBaseSupportsByHash = TransactionManager->MetaIndexParser->GetSupportsAcquireByHash();
-   for (auto &Target: TransactionManager->MetaIndexParser->GetIndexTargets())
+   bool const hasReleaseFile = TransactionManager->MetaIndexParser != NULL;
+   bool const metaBaseSupportsByHash = hasReleaseFile && TransactionManager->MetaIndexParser->GetSupportsAcquireByHash();
+   bool hasHashes = true;
+   auto IndexTargets = TransactionManager->MetaIndexParser->GetIndexTargets();
+   if (hasReleaseFile && verify == false)
+      hasHashes = std::any_of(IndexTargets.begin(), IndexTargets.end(),
+           [&](IndexTarget const &Target) { return TransactionManager->MetaIndexParser->Exists(Target.MetaKey); });
+   for (auto&& Target: IndexTargets)
    {
       // if we have seen a target which is created-by a target this one here is declared a
       // fallback to, we skip acquiring the fallback (but we make sure we clean up)
@@ -1214,7 +1220,7 @@ void pkgAcqMetaClearSig::QueueIndexes(bool const verify)                  /*{{{*/
       // download time, bandwidth and diskspace for nothing, BUT Debian doesn't feature all
       // in the set of supported architectures, so we can filter based on this property rather
       // than invent an entirely new flag we would need to carry for all of eternity.
-      if (Target.Option(IndexTarget::ARCHITECTURE) == "all")
+      if (hasReleaseFile && Target.Option(IndexTarget::ARCHITECTURE) == "all")
       {
         if (TransactionManager->MetaIndexParser->IsArchitectureSupported("all") == false ||
               TransactionManager->MetaIndexParser->IsArchitectureAllSupportedFor(Target) == false)
@@ -1225,12 +1231,12 @@ void pkgAcqMetaClearSig::QueueIndexes(bool const verify)                        /*{{{*/
       }
 
       bool trypdiff = Target.OptionBool(IndexTarget::PDIFFS);
-      if (verify == true)
+      if (hasReleaseFile == true)
       {
         if (TransactionManager->MetaIndexParser->Exists(Target.MetaKey) == false)
         {
            // optional targets that we do not have in the Release file are skipped
-           if (Target.IsOptional)
+           if (hasHashes == true && Target.IsOptional)
            {
               new CleanupItem(Owner, TransactionManager, Target);
               continue;
@@ -1249,18 +1255,26 @@ void pkgAcqMetaClearSig::QueueIndexes(bool const verify)                        /*{{{*/
               // if the architecture is officially supported but currently no packages for it available,
               // ignore silently as this is pretty much the same as just shipping an empty file.
               // if we don't know which architectures are supported, we do NOT ignore it to notify user about this
-              if (TransactionManager->MetaIndexParser->IsArchitectureSupported("*undefined*") == false)
+              if (hasHashes == true && TransactionManager->MetaIndexParser->IsArchitectureSupported("*undefined*") == false)
               {
                  new CleanupItem(Owner, TransactionManager, Target);
                  continue;
               }
            }
 
-           Status = StatAuthError;
-           strprintf(ErrorText, _("Unable to find expected entry '%s' in Release file (Wrong sources.list entry or malformed file)"), Target.MetaKey.c_str());
-           return;
+           if (hasHashes == true)
+           {
+              Status = StatAuthError;
+              strprintf(ErrorText, _("Unable to find expected entry '%s' in Release file (Wrong sources.list entry or malformed file)"), Target.MetaKey.c_str());
+              return;
+           }
+           else
+           {
+              new pkgAcqIndex(Owner, TransactionManager, Target);
+              continue;
+           }
         }
-        else
+        else if (verify)
         {
            auto const hashes = GetExpectedHashesFor(Target.MetaKey);
            if (hashes.empty() == false)
@@ -1530,6 +1544,17 @@ void pkgAcqMetaClearSig::Done(std::string const &Message,
         new NoActionItem(Owner, DetachedSigTarget);
       }
    }
+   else if (Status != StatAuthError)
+   {
+      string const FinalFile = GetFinalFileNameFromURI(DetachedDataTarget.URI);
+      string const OldFile = GetFinalFilename();
+      if (TransactionManager->IMSHit == false)
+        TransactionManager->TransactionStageCopy(this, DestFile, FinalFile);
+      else if (RealFileExists(OldFile) == false)
+        new NoActionItem(Owner, DetachedDataTarget);
+      else
+        TransactionManager->TransactionStageCopy(this, OldFile, FinalFile);
+   }
 }
                                                                        /*}}}*/
 void pkgAcqMetaClearSig::Failed(string const &Message,pkgAcquire::MethodConfig const * const Cnf) /*{{{*/
@@ -1735,6 +1760,14 @@ void pkgAcqMetaSig::Done(string const &Message, HashStringList const &Hashes,
         TransactionManager->TransactionStageCopy(MetaIndex, MetaIndex->DestFile, MetaIndex->GetFinalFilename());
       }
    }
+   else if (MetaIndex->Status != StatAuthError)
+   {
+      std::string const FinalFile = MetaIndex->GetFinalFilename();
+      if (TransactionManager->IMSHit == false)
+        TransactionManager->TransactionStageCopy(MetaIndex, MetaIndex->DestFile, FinalFile);
+      else
+        TransactionManager->TransactionStageCopy(MetaIndex, FinalFile, FinalFile);
+   }
 }
                                                                        /*}}}*/
 void pkgAcqMetaSig::Failed(string const &Message,pkgAcquire::MethodConfig const * const Cnf)/*{{{*/
index c70c39a45e49daef26e0adfea61c3835d3233822..0c9cde620e048f4299ced4b48e890c4d31212678 100644 (file)
@@ -440,18 +440,13 @@ bool debReleaseIndex::Load(std::string const &Filename, std::string * const Erro
       }
    }
 
+   bool AuthPossible = false;
    if(FoundHashSum == false)
-   {
-      if (ErrorText != NULL)
-        strprintf(*ErrorText, _("No Hash entry in Release file %s"), Filename.c_str());
-      return false;
-   }
-   if(FoundStrongHashSum == false)
-   {
-      if (ErrorText != NULL)
-        strprintf(*ErrorText, _("No Hash entry in Release file %s which is considered strong enough for security purposes"), Filename.c_str());
-      return false;
-   }
+      _error->Warning(_("No Hash entry in Release file %s"), Filename.c_str());
+   else if(FoundStrongHashSum == false)
+      _error->Warning(_("No Hash entry in Release file %s which is considered strong enough for security purposes"), Filename.c_str());
+   else
+      AuthPossible = true;
 
    std::string const StrDate = Section.FindS("Date");
    if (RFC1123StrToTime(StrDate.c_str(), Date) == false)
@@ -539,8 +534,9 @@ bool debReleaseIndex::Load(std::string const &Filename, std::string * const Erro
       }
    }
 
-   LoadedSuccessfully = TRI_YES;
-   return true;
+   if (AuthPossible)
+      LoadedSuccessfully = TRI_YES;
+   return AuthPossible;
 }
                                                                        /*}}}*/
 metaIndex * debReleaseIndex::UnloadedClone() const                     /*{{{*/
index 18674ecd23ac97b1fb7d2f3c19a96583b4c73792..9395b10b0cfc4ccf43bbb9e592a4aecb78d408c8 100755 (executable)
@@ -7,6 +7,7 @@ TESTDIR="$(readlink -f "$(dirname "$0")")"
 setupenvironment
 configarchitecture 'i386'
 confighashes 'MD5'
+export APT_DONT_SIGN=''
 
 insertpackage 'unstable' 'foo' 'i386' '1.0'
 insertsource 'unstable' 'foo' 'any' '1.0'
@@ -14,27 +15,144 @@ insertsource 'unstable' 'foo' 'any' '1.0'
 setupaptarchive --no-update
 APTARCHIVE="$(readlink -f ./aptarchive)"
 
-msgmsg 'Release contains only weak hashes'
-FILENAME="${APTARCHIVE}/dists/unstable/InRelease"
-MANGLED="$(readlink -f ./rootdir)/var/lib/apt/lists/partial/$(echo "$FILENAME" | sed 's#/#_#g')"
-testfailuremsg "E: Failed to fetch file:${FILENAME}  No Hash entry in Release file ${MANGLED} which is considered strong enough for security purposes
-E: Some index files failed to download. They have been ignored, or old ones used instead." apt update
-testnopackage foo
-testnosrcpackage foo
+testnopkg() {
+       testnopackage "$@"
+       testnosrcpackage "$@"
+}
+testbadpkg() {
+       testempty find rootdir/var/lib/apt/lists -maxdepth 1 -name '*InRelease' -o -name '*Release.gpg'
+       testnotempty find rootdir/var/lib/apt/lists -maxdepth 1 -name '*Release'
+       testnotempty apt show "$@"
+       testnotempty apt showsrc "$@"
+       testfailureequal "WARNING: The following packages cannot be authenticated!
+  $*
+E: There were unauthenticated packages and -y was used without --allow-unauthenticated" aptget install -qq -y "$@"
+       testfailureequal "WARNING: The following packages cannot be authenticated!
+  $*
+E: Some packages could not be authenticated" aptget source -qq "$@"
+}
 
-msgmsg 'Release contains no hashes'
-sed -i -e '/^ / d' -e '/^MD5Sum:/ d' "$APTARCHIVE/dists/unstable/Release"
+testrun() {
+       local TYPE="$1"
+       local FILENAME="$2"
+       shift 2
+       local MANGLED="$(readlink -f ./rootdir)/var/lib/apt/lists/partial/$(echo "$FILENAME" | sed 's#/#_#g')"
+       msgmsg "$TYPE contains only weak hashes"
+       confighashes 'MD5'
+       generatereleasefiles
+       signreleasefiles
+       preparetest
+       if [ -z "$1" ]; then
+               listcurrentlistsdirectory > lists.before
+               testfailuremsg "W: No Hash entry in Release file ${MANGLED} which is considered strong enough for security purposes
+E: The repository 'file:${APTARCHIVE} unstable $(basename "$FILENAME")' provides only weak security information.
+N: Updating from such a repository can't be done securely, and is therefore disabled by default.
+N: See apt-secure(8) manpage for repository creation and user configuration details." apt update
+               testfileequal lists.before "$(listcurrentlistsdirectory)"
+               testnopkg 'foo'
+       else
+               testwarningmsg "W: No Hash entry in Release file ${MANGLED} which is considered strong enough for security purposes
+W: The repository 'file:${APTARCHIVE} unstable $(basename "$FILENAME")' provides only weak security information.
+N: Data from such a repository can't be authenticated and is therefore potentially dangerous to use.
+N: See apt-secure(8) manpage for repository creation and user configuration details." apt update "$@"
+               testbadpkg 'foo'
+       fi
+
+       msgmsg "$TYPE contains no hashes"
+       generatereleasefiles
+       sed -i -e '/^ / d' -e '/^MD5Sum:/ d' "$APTARCHIVE/dists/unstable/Release"
+       signreleasefiles
+       preparetest
+       if [ -z "$1" ]; then
+               listcurrentlistsdirectory > lists.before
+               testfailuremsg "W: No Hash entry in Release file ${MANGLED}
+E: The repository 'file:${APTARCHIVE} unstable $(basename "$FILENAME")' provides only weak security information.
+N: Updating from such a repository can't be done securely, and is therefore disabled by default.
+N: See apt-secure(8) manpage for repository creation and user configuration details." apt update
+               testfileequal lists.before "$(listcurrentlistsdirectory)"
+               testnopkg 'foo'
+       else
+               testwarningmsg "W: No Hash entry in Release file ${MANGLED}
+W: The repository 'file:${APTARCHIVE} unstable $(basename "$FILENAME")' provides only weak security information.
+N: Data from such a repository can't be authenticated and is therefore potentially dangerous to use.
+N: See apt-secure(8) manpage for repository creation and user configuration details." apt update "$@"
+               testbadpkg 'foo'
+       fi
+
+       msgmsg "$TYPE contains only weak hashes for some files"
+       confighashes 'MD5' 'SHA256'
+       generatereleasefiles
+       sed -i '/^ [0-9a-fA-Z]\{64\} .*Sources$/d' "$APTARCHIVE/dists/unstable/Release"
+       signreleasefiles
+       preparetest
+       # trust is a repository property, so individual files can't be insecure
+       testwarningmsg "W: Skipping acquire of configured file 'main/source/Sources' as repository 'file:${APTARCHIVE} unstable InRelease' provides only weak security information for it" apt update "$@"
+       testsuccess apt show foo
+       testnosrcpackage foo
+}
+
+genericprepare() {
+       rm -rf rootdir/var/lib/apt/lists
+       mkdir -p rootdir/var/lib/apt/lists/partial
+       touch rootdir/var/lib/apt/lists/lock
+       local RELEASEGPG="$(readlink -f ./rootdir)/var/lib/apt/lists/partial/$(echo "${APTARCHIVE}/dists/unstable/Release.gpg" | sed 's#/#_#g')"
+       touch "$RELEASEGPG"
+       chmod 644 "$RELEASEGPG"
+       local INRELEASE="$(readlink -f ./rootdir)/var/lib/apt/lists/partial/$(echo "${APTARCHIVE}/dists/unstable/InRelease" | sed 's#/#_#g')"
+       touch "$INRELEASE"
+       chmod 644 "$INRELEASE"
+}
+preparetest() {
+       rm -f "${APTARCHIVE}/dists/unstable/Release" "${APTARCHIVE}/dists/unstable/Release.gpg"
+       genericprepare
+}
+testrun 'InRelease' "${APTARCHIVE}/dists/unstable/InRelease"
+testrun 'InRelease' "${APTARCHIVE}/dists/unstable/InRelease" --allow-insecure-repositories -o APT::Get::List-Cleanup=0
+
+preparetest() {
+       rm -f "${APTARCHIVE}/dists/unstable/InRelease"
+       genericprepare
+}
+testrun 'Release+Release.gpg' "${APTARCHIVE}/dists/unstable/Release"
+testrun 'Release+Release.gpg' "${APTARCHIVE}/dists/unstable/Release" --allow-insecure-repositories -o APT::Get::List-Cleanup=0
+
+preparetest() {
+       rm -f "${APTARCHIVE}/dists/unstable/InRelease" "${APTARCHIVE}/dists/unstable/Release.gpg"
+       genericprepare
+}
+
+msgmsg 'Moving between Release files with good and bad hashes'
+rm -rf rootdir/var/lib/apt/lists
+confighashes 'MD5'
+generatereleasefiles 'now - 1 day'
 signreleasefiles
-testfailuremsg "E: Failed to fetch file:${FILENAME}  No Hash entry in Release file ${MANGLED}
-E: Some index files failed to download. They have been ignored, or old ones used instead." apt update
-testnopackage foo
-testnosrcpackage foo
+testfailure apt update
+testnopkg 'foo'
+testwarning apt update --allow-insecure-repositories
+testbadpkg 'foo'
 
-msgmsg 'Release contains only weak hashes for some files'
 confighashes 'MD5' 'SHA256'
-generatereleasefiles
-sed -i '/^ [0-9a-fA-Z]\{64\} .*Sources$/d' "$APTARCHIVE/dists/unstable/Release"
-signreleasefiles
-testwarningmsg "W: Skipping acquire of configured file 'main/source/Sources' as repository 'file:${APTARCHIVE} unstable InRelease' provides only weak security information for it" apt update
-testsuccess apt show foo
-testnosrcpackage foo
+rm -rf aptarchive/dists
+insertpackage 'unstable' 'foo2' 'i386' '1.0'
+insertsource 'unstable' 'foo2' 'any' '1.0'
+setupaptarchive --no-update 'now - 12 hours'
+testsuccess apt update
+testnopkg foo
+testnotempty find rootdir/var/lib/apt/lists -maxdepth 1 -name '*InRelease' -o -name '*Release.gpg'
+testnotempty apt show foo2
+testnotempty apt showsrc foo2
+
+confighashes 'MD5'
+rm -rf aptarchive/dists
+insertpackage 'unstable' 'foo3' 'i386' '1.0'
+insertsource 'unstable' 'foo3' 'any' '1.0'
+setupaptarchive --no-update
+testfailure apt update
+testnopkg foo
+testnopkg foo3
+testnotempty find rootdir/var/lib/apt/lists -maxdepth 1 -name '*InRelease' -o -name '*Release.gpg'
+testnotempty apt show foo2
+testnotempty apt showsrc foo2
+testwarning apt update --allow-insecure-repositories
+testnopkg foo2
+testbadpkg foo3