generalize secure->insecure downgrade protection

Handling the extra check (and force requirement) for downgrades in
security in our AllowInsecureRepositories checker helps in having this
check everywhere instead of just in the most common place and requiring
a little extra force in such cases is always good.

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 6f479cceff255e3c88ea4dbc6f405f89c154e42c..04ba2b479e708e21547f9691e7fd23e01c5a92fa 100644 (file)
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -175,41 +175,79 @@ static void ReportMirrorFailureToCentral(pkgAcquire::Item const &I, std::string
 }
                                                                        /*}}}*/
 
 }
                                                                        /*}}}*/
 

-static bool MessageInsecureRepository(bool const isError, std::string const &msg)/*{{{*/
+static bool MessageInsecureRepository(bool const isError, char const * const msg, std::string const &repo)/*{{{*/

 {
 {

+   std::string m;
+   strprintf(m, msg, repo.c_str());

    if (isError)
    {
    if (isError)
    {

-      _error->Error("%s", msg.c_str());
+      _error->Error("%s", m.c_str());

       _error->Notice("%s", _("Updating from such a repository can't be done securely, and is therefore disabled by default."));
    }
    else
    {
       _error->Notice("%s", _("Updating from such a repository can't be done securely, and is therefore disabled by default."));
    }
    else
    {

-      _error->Warning("%s", msg.c_str());
+      _error->Warning("%s", m.c_str());

       _error->Notice("%s", _("Data from such a repository can't be authenticated and is therefore potentially dangerous to use."));
    }
    _error->Notice("%s", _("See apt-secure(8) manpage for repository creation and user configuration details."));
    return false;
       _error->Notice("%s", _("Data from such a repository can't be authenticated and is therefore potentially dangerous to use."));
    }
    _error->Notice("%s", _("See apt-secure(8) manpage for repository creation and user configuration details."));
    return false;

-}
-static bool APT_NONNULL(2) MessageInsecureRepository(bool const isError, char const * const msg, std::string const &repo)
-{
-   std::string m;
-   strprintf(m, msg, repo.c_str());
-   return MessageInsecureRepository(isError, m);

 }
                                                                        /*}}}*/
 }
                                                                        /*}}}*/

-static bool APT_NONNULL(1, 3, 4, 5) AllowInsecureRepositories(char const * const msg, std::string const &repo,/*{{{*/
+// AllowInsecureRepositories                                           /*{{{*/
+enum class InsecureType { UNSIGNED, WEAK, NORELEASE };
+static bool APT_NONNULL(3, 4, 5) AllowInsecureRepositories(InsecureType msg, std::string const &repo,

       metaIndex const * const MetaIndexParser, pkgAcqMetaClearSig * const TransactionManager, pkgAcquire::Item * const I)
 {
       metaIndex const * const MetaIndexParser, pkgAcqMetaClearSig * const TransactionManager, pkgAcquire::Item * const I)
 {

+   // we skip weak downgrades as its unlikely that a repository gets really weaker –
+   // its more realistic that apt got pickier in a newer version
+   if (msg != InsecureType::WEAK)
+   {
+      std::string const FinalInRelease = TransactionManager->GetFinalFilename();
+      std::string const FinalReleasegpg = FinalInRelease.substr(0, FinalInRelease.length() - strlen("InRelease")) + "Release.gpg";
+      if (RealFileExists(FinalReleasegpg) || RealFileExists(FinalInRelease))
+      {
+        char const * msgstr = nullptr;
+        switch (msg)
+        {
+           case InsecureType::UNSIGNED: msgstr = _("The repository '%s' is no longer signed."); break;
+           case InsecureType::NORELEASE: msgstr = _("The repository '%s' does no longer have a Release file."); break;
+           case InsecureType::WEAK: /* unreachable */ break;
+        }
+        if (_config->FindB("Acquire::AllowDowngradeToInsecureRepositories"))
+        {
+           // meh, the users wants to take risks (we still mark the packages
+           // from this repository as unauthenticated)
+           _error->Warning(msgstr, repo.c_str());
+           _error->Warning(_("This is normally not allowed, but the option "
+                    "Acquire::AllowDowngradeToInsecureRepositories was "
+                    "given to override it."));
+        } else {
+           MessageInsecureRepository(true, msgstr, repo);
+           TransactionManager->AbortTransaction();
+           I->Status = pkgAcquire::Item::StatError;
+           return false;
+        }
+      }
+   }
+

    if(MetaIndexParser->GetTrusted() == metaIndex::TRI_YES)
       return true;
 
    if(MetaIndexParser->GetTrusted() == metaIndex::TRI_YES)
       return true;
 

+   char const * msgstr = nullptr;
+   switch (msg)
+   {
+      case InsecureType::UNSIGNED: msgstr = _("The repository '%s' is not signed."); break;
+      case InsecureType::NORELEASE: msgstr = _("The repository '%s' does not have a Release file."); break;
+      case InsecureType::WEAK: msgstr = _("The repository '%s' provides only weak security information."); break;
+   }
+

    if (_config->FindB("Acquire::AllowInsecureRepositories") == true)
    {
    if (_config->FindB("Acquire::AllowInsecureRepositories") == true)
    {

-      MessageInsecureRepository(false, msg, repo);
+      MessageInsecureRepository(false, msgstr, repo);

       return true;
    }
 
       return true;
    }
 

-   MessageInsecureRepository(true, msg, repo);
+   MessageInsecureRepository(true, msgstr, repo);

    TransactionManager->AbortTransaction();
    I->Status = pkgAcquire::Item::StatError;
    return false;
    TransactionManager->AbortTransaction();
    I->Status = pkgAcquire::Item::StatError;
    return false;


@@ -1172,8 +1210,7 @@ bool pkgAcqMetaBase::CheckAuthDone(string const &Message)         /*{{{*/
    }
 
    bool const GoodAuth = TransactionManager->MetaIndexParser->Load(DestFile, &ErrorText);
    }
 
    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)
+   if (GoodAuth == false && AllowInsecureRepositories(InsecureType::WEAK, Target.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == false)

    {
       Status = StatAuthError;
       return false;
    {
       Status = StatAuthError;
       return false;


@@ -1586,10 +1623,7 @@ void pkgAcqMetaClearSig::Failed(string const &Message,pkgAcquire::MethodConfig c
       if(CheckStopAuthentication(this, Message))
          return;
 
       if(CheckStopAuthentication(this, Message))
          return;
 

-      // No Release file was present, or verification failed, so fall
-      // back to queueing Packages files without verification
-      // only allow going further if the user explicitly wants it
-      if(AllowInsecureRepositories(_("The repository '%s' is not signed."), ClearsignedTarget.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == true)
+      if(AllowInsecureRepositories(InsecureType::UNSIGNED, ClearsignedTarget.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == true)

       {
         Status = StatDone;
 
       {
         Status = StatDone;
 


@@ -1660,7 +1694,7 @@ void pkgAcqMetaIndex::Failed(string const &Message,
    // No Release file was present so fall
    // back to queueing Packages files without verification
    // only allow going further if the user explicitly wants it
    // No Release file was present so fall
    // back to queueing Packages files without verification
    // only allow going further if the user explicitly wants it

-   if(AllowInsecureRepositories(_("The repository '%s' does not have a Release file."), Target.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == true)
+   if(AllowInsecureRepositories(InsecureType::NORELEASE, Target.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == true)

    {
       // ensure old Release files are removed
       TransactionManager->TransactionStageRemoval(this, GetFinalFilename());
    {
       // ensure old Release files are removed
       TransactionManager->TransactionStageRemoval(this, GetFinalFilename());


@@ -1778,40 +1812,14 @@ void pkgAcqMetaSig::Failed(string const &Message,pkgAcquire::MethodConfig const
    if (MetaIndex->AuthPass == true && MetaIndex->CheckStopAuthentication(this, Message))
          return;
 
    if (MetaIndex->AuthPass == true && MetaIndex->CheckStopAuthentication(this, Message))
          return;
 

-   string const FinalRelease = MetaIndex->GetFinalFilename();
-   string const FinalReleasegpg = GetFinalFilename();
-   string const FinalInRelease = TransactionManager->GetFinalFilename();
-
-   if (RealFileExists(FinalReleasegpg) || RealFileExists(FinalInRelease))
-   {
-      std::string downgrade_msg;
-      strprintf(downgrade_msg, _("The repository '%s' is no longer signed."),
-                MetaIndex->Target.Description.c_str());
-      if(_config->FindB("Acquire::AllowDowngradeToInsecureRepositories"))
-      {
-         // meh, the users wants to take risks (we still mark the packages
-         // from this repository as unauthenticated)
-         _error->Warning("%s", downgrade_msg.c_str());
-         _error->Warning(_("This is normally not allowed, but the option "
-                           "Acquire::AllowDowngradeToInsecureRepositories was "
-                           "given to override it."));
-         Status = StatDone;
-      } else {
-        MessageInsecureRepository(true, downgrade_msg);
-        if (TransactionManager->IMSHit == false)
-           Rename(MetaIndex->DestFile, MetaIndex->DestFile + ".FAILED");
-        Item::Failed("Message: " + downgrade_msg, Cnf);
-         TransactionManager->AbortTransaction();
-         return;
-      }
-   }
-

    // ensures that a Release.gpg file in the lists/ is removed by the transaction
    TransactionManager->TransactionStageRemoval(this, DestFile);
 
    // only allow going further if the user explicitly wants it
    // ensures that a Release.gpg file in the lists/ is removed by the transaction
    TransactionManager->TransactionStageRemoval(this, DestFile);
 
    // only allow going further if the user explicitly wants it

-   if (AllowInsecureRepositories(_("The repository '%s' is not signed."), MetaIndex->Target.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == true)
+   if (AllowInsecureRepositories(InsecureType::UNSIGNED, MetaIndex->Target.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == true)

    {
    {

+      string const FinalRelease = MetaIndex->GetFinalFilename();
+      string const FinalInRelease = TransactionManager->GetFinalFilename();

       LoadLastMetaIndexParser(TransactionManager, FinalRelease, FinalInRelease);
 
       // we parse the indexes here because at this point the user wanted
       LoadLastMetaIndexParser(TransactionManager, FinalRelease, FinalInRelease);
 
       // we parse the indexes here because at this point the user wanted


@@ -1822,8 +1830,10 @@ void pkgAcqMetaSig::Failed(string const &Message,pkgAcquire::MethodConfig const
       else
         TransactionManager->QueueIndexes(GoodLoad);
 
       else
         TransactionManager->QueueIndexes(GoodLoad);
 

-      TransactionManager->TransactionStageCopy(MetaIndex, MetaIndex->DestFile, MetaIndex->GetFinalFilename());
+      TransactionManager->TransactionStageCopy(MetaIndex, MetaIndex->DestFile, FinalRelease);

    }
    }

+   else if (TransactionManager->IMSHit == false)
+      Rename(MetaIndex->DestFile, MetaIndex->DestFile + ".FAILED");

 
    // FIXME: this is used often (e.g. in pkgAcqIndexTrans) so refactor
    if (Cnf->LocalOnly == true ||
 
    // FIXME: this is used often (e.g. in pkgAcqIndexTrans) so refactor
    if (Cnf->LocalOnly == true ||

diff --git a/test/integration/test-apt-update-nofallback b/test/integration/test-apt-update-nofallback
index c8a008214c9c41b3430c3bc6d35457f426b671a8..40fbae56097f6c768504179e176b2729185359bd 100755 (executable)
--- a/test/integration/test-apt-update-nofallback
+++ b/test/integration/test-apt-update-nofallback
@@ -93,6 +93,30 @@ test_from_inrelease_to_unsigned_with_override()
     find "$APTARCHIVE" -name '*Packages*' -exec touch -d '+2 hours' {} \;
 
     # and ensure we can update to it (with enough force) 
     find "$APTARCHIVE" -name '*Packages*' -exec touch -d '+2 hours' {} \;
 
     # and ensure we can update to it (with enough force) 

+    testfailure aptget update
+    testfailure aptget update --allow-insecure-repositories
+    testwarning aptget update --allow-insecure-repositories \
+        -o Acquire::AllowDowngradeToInsecureRepositories=1 -o Debug::pkgAcquire::Worker=1 -o Debug::pkgAcquire::Auth=1
+    # but that the individual packages are still considered untrusted
+    testfailureequal "WARNING: The following packages cannot be authenticated!
+  evil
+E: There were unauthenticated packages and -y was used without --allow-unauthenticated" aptget install -qq -y evil
+}
+
+test_from_inrelease_to_norelease_with_override()
+{
+    # setup archive with InRelease file
+    setupaptarchive_with_lists_clean
+    testsuccess aptget update
+
+    # simulate moving to a unsigned but otherwise valid repo
+    simulate_mitm_and_inject_evil_package
+    find "$APTARCHIVE" -name '*Release*' -delete
+    find "$APTARCHIVE" -name '*Packages*' -exec touch -d '+2 hours' {} \;
+
+    # and ensure we can update to it (with enough force) 
+    testfailure aptget update
+    testfailure aptget update --allow-insecure-repositories

     testwarning aptget update --allow-insecure-repositories \
         -o Acquire::AllowDowngradeToInsecureRepositories=1 -o Debug::pkgAcquire::Worker=1 -o Debug::pkgAcquire::Auth=1
     # but that the individual packages are still considered untrusted
     testwarning aptget update --allow-insecure-repositories \
         -o Acquire::AllowDowngradeToInsecureRepositories=1 -o Debug::pkgAcquire::Worker=1 -o Debug::pkgAcquire::Auth=1
     # but that the individual packages are still considered untrusted


@@ -237,3 +261,5 @@ test_release_gpg_to_invalid_release_release_gpg
 # ensure we can override the downgrade error
 msgmsg "test_from_inrelease_to_unsigned_with_override"
 test_from_inrelease_to_unsigned_with_override
 # ensure we can override the downgrade error
 msgmsg "test_from_inrelease_to_unsigned_with_override"
 test_from_inrelease_to_unsigned_with_override

+msgmsg "test_from_inrelease_to_norelease_with_override"
+test_from_inrelease_to_norelease_with_override