rework errors and warnings around insecure repositories

Insecure (aka unsigned) repositories are bad, period. We want to get
right of them finally and as a first step we are printing scary
warnings. This is already done, this commit just changes the messages to
be more consistent and prevents them from being displayed if
authenticity is guaranteed some other way (as indicated with
trusted=yes).

The idea is to first print the pure fact like "repository isn't signed"
as a warning (and later as an error), while giving an explaination in a
immediately following notice (which is displayed only in quiet level 0:
so in interactive use, not in scripts and alike).

Closes: 796549

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 98739f7a6a4b73e8561463e98d2ec71eaef8a43b..0c7c7c75c3066944615325f12b3e4a2d0ba0cd92 100644 (file)
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -148,12 +148,41 @@ static bool BootstrapPDiffWith(std::string const &PartialFile, std::string const
 }
                                                                        /*}}}*/
 
-static bool AllowInsecureRepositories(metaIndex const * const MetaIndexParser, pkgAcqMetaClearSig * const TransactionManager, pkgAcquire::Item * const I) /*{{{*/
+static bool MessageInsecureRepository(bool const isError, std::string const &msg)/*{{{*/
 {
-   if(MetaIndexParser->GetTrusted() == metaIndex::TRI_YES || _config->FindB("Acquire::AllowInsecureRepositories") == true)
+   if (isError)
+   {
+      _error->Error("%s", msg.c_str());
+      _error->Notice("%s", _("Updating such a repository securily is impossible and therefore disabled by default."));
+   }
+   else
+   {
+      _error->Warning("%s", msg.c_str());
+      _error->Notice("%s", _("Data from such a repository can not be authenticated and is therefore potentially dangerous to use."));
+   }
+   return false;
+}
+static bool 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 AllowInsecureRepositories(char const * const msg, std::string const &repo,/*{{{*/
+      metaIndex const * const MetaIndexParser, pkgAcqMetaClearSig * const TransactionManager, pkgAcquire::Item * const I)
+{
+   if(MetaIndexParser->GetTrusted() == metaIndex::TRI_YES)
       return true;
 
-   _error->Error(_("Use --allow-insecure-repositories to force the update"));
+   if (_config->FindB("Acquire::AllowInsecureRepositories") == true)
+   {
+      MessageInsecureRepository(false, msg, repo);
+      return true;
+   }
+
+   MessageInsecureRepository(true, msg, repo);
+   _error->Notice(_("Use --allow-insecure-repositories to force an insecure update"));
    TransactionManager->AbortTransaction();
    I->Status = pkgAcquire::Item::StatError;
    return false;
@@ -1308,10 +1337,10 @@ void pkgAcqMetaClearSig::Failed(string const &Message,pkgAcquire::MethodConfig c
 
    if (AuthPass == false)
    {
-      if (Status == StatAuthError)
+      if (Status == StatAuthError || Status == StatTransientNetworkError)
       {
-        // if we expected a ClearTextSignature (InRelease) and got a file,
-        // but it wasn't valid we end up here (see VerifyDone).
+        // if we expected a ClearTextSignature (InRelease) but got a network
+        // error or got a file, but it wasn't valid, we end up here (see VerifyDone).
         // As these is usually called by web-portals we do not try Release/Release.gpg
         // as this is gonna fail anyway and instead abort our try (LP#346386)
         TransactionManager->AbortTransaction();
@@ -1331,14 +1360,10 @@ void pkgAcqMetaClearSig::Failed(string const &Message,pkgAcquire::MethodConfig c
       if(CheckStopAuthentication(this, Message))
          return;
 
-      _error->Warning(_("The data from '%s' is not signed. Packages "
-                        "from that repository can not be authenticated."),
-                      ClearsignedTarget.Description.c_str());
-
       // No Release file was present, or verification failed, so fall
       // back to queueing Packages files without verification
       // only allow going further if the users explicitely wants it
-      if(AllowInsecureRepositories(TransactionManager->MetaIndexParser, TransactionManager, this) == true)
+      if(AllowInsecureRepositories(_("The repository '%s' is not signed."), ClearsignedTarget.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == true)
       {
         Status = StatDone;
 
@@ -1433,14 +1458,10 @@ void pkgAcqMetaIndex::Failed(string const &Message,
    pkgAcquire::Item::Failed(Message, Cnf);
    Status = StatDone;
 
-   _error->Warning(_("The repository '%s' does not have a Release file. "
-                     "This is deprecated, please contact the owner of the "
-                     "repository."), Target.Description.c_str());
-
    // No Release file was present so fall
    // back to queueing Packages files without verification
    // only allow going further if the users explicitely wants it
-   if(AllowInsecureRepositories(TransactionManager->MetaIndexParser, TransactionManager, this) == true)
+   if(AllowInsecureRepositories(_("The repository '%s' does not have a Release file."), Target.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == true)
    {
       // ensure old Release files are removed
       TransactionManager->TransactionStageRemoval(this, GetFinalFilename());
@@ -1578,7 +1599,7 @@ void pkgAcqMetaSig::Failed(string const &Message,pkgAcquire::MethodConfig const
                            "given to override it."));
          Status = StatDone;
       } else {
-         _error->Error("%s", downgrade_msg.c_str());
+        MessageInsecureRepository(true, downgrade_msg);
         if (TransactionManager->IMSHit == false)
            Rename(MetaIndex->DestFile, MetaIndex->DestFile + ".FAILED");
         Item::Failed("Message: " + downgrade_msg, Cnf);
@@ -1586,16 +1607,12 @@ void pkgAcqMetaSig::Failed(string const &Message,pkgAcquire::MethodConfig const
          return;
       }
    }
-   else
-      _error->Warning(_("The data from '%s' is not signed. Packages "
-              "from that repository can not be authenticated."),
-           MetaIndex->Target.Description.c_str());
 
    // ensures that a Release.gpg file in the lists/ is removed by the transaction
    TransactionManager->TransactionStageRemoval(this, DestFile);
 
    // only allow going further if the users explicitely wants it
-   if(AllowInsecureRepositories(TransactionManager->MetaIndexParser, TransactionManager, this) == true)
+   if (AllowInsecureRepositories(_("The repository '%s' is not signed."), MetaIndex->Target.Description, TransactionManager->MetaIndexParser, TransactionManager, this) == true)
    {
       if (RealFileExists(FinalReleasegpg) || RealFileExists(FinalInRelease))
       {

diff --git a/test/integration/framework b/test/integration/framework
index a9acd83a977899313105bc3e71e9b64c3c3e7342..b4220c8b56bb6bc22550a0ee4876596e643a2fc1 100644 (file)
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -1665,6 +1665,17 @@ testfailuremsg() {
        testoutputequal "$COMPAREFILE" echo "$CMP"
        msggroup
 }
+testwarningmsg() {
+       msggroup 'testwarningmsg'
+       local CMP="$1"
+       shift
+       testwarning "$@"
+       msgtest 'Check that the output of the previous warned command has expected' 'warnings'
+       local COMPAREFILE="${TMPWORKINGDIRECTORY}/rootdir/tmp/testwarningmsg.comparefile"
+       grep '^\(W\|E\):' "${TMPWORKINGDIRECTORY}/rootdir/tmp/testwarning.output" > "$COMPAREFILE" 2>&1 || true
+       testoutputequal "$COMPAREFILE" echo "$CMP"
+       msggroup
+}
 
 testfilestats() {
        msggroup 'testfilestats'

diff --git a/test/integration/test-apt-get-update-unauth-warning b/test/integration/test-apt-get-update-unauth-warning
index b2d79eb2b9abeef6deca3e6a53a653681c5e36c6..fad1cf62701cc7437d2723c7bf1eff8da4d12cf6 100755 (executable)
--- a/test/integration/test-apt-get-update-unauth-warning
+++ b/test/integration/test-apt-get-update-unauth-warning
@@ -27,8 +27,9 @@ Get:2 file:$APTARCHIVE unstable Release
 Err:2 file:$APTARCHIVE unstable Release
   File not found
 Reading package lists...
-W: The repository 'file:$APTARCHIVE unstable Release' does not have a Release file. This is deprecated, please contact the owner of the repository.
-E: Use --allow-insecure-repositories to force the update" aptget update --no-allow-insecure-repositories
+E: The repository 'file:$APTARCHIVE unstable Release' does not have a Release file.
+N: Updating such a repository securily is impossible and therefore disabled by default.
+N: Use --allow-insecure-repositories to force an insecure update" aptget update --no-allow-insecure-repositories -q=0
 
 # no package foo
 testsuccessequal 'Listing...' apt list foo
@@ -78,7 +79,8 @@ Get:3 file:$APTARCHIVE unstable/main Sources [$(filesize 'Sources') B]
 Get:4 file:$APTARCHIVE unstable/main i386 Packages [$(filesize 'Packages') B]
 Get:5 file:$APTARCHIVE unstable/main Translation-en [$(filesize 'Translations') B]
 Reading package lists...
-W: The repository 'file:$APTARCHIVE unstable Release' does not have a Release file. This is deprecated, please contact the owner of the repository." aptget update --allow-insecure-repositories
+W: The repository 'file:$APTARCHIVE unstable Release' does not have a Release file.
+N: Data from such a repository can not be authenticated and is therefore potentially dangerous to use." aptget update --allow-insecure-repositories -q=0
 # ensure we can not install the package
 testfailureequal "WARNING: The following packages cannot be authenticated!
   foo

diff --git a/test/integration/test-apt-update-ims b/test/integration/test-apt-update-ims
index bc7a2b1bfa55582631147f260f074e9dc151b467..3a66a546f4b630a2731cdd49859f17bce1aad0c0 100755 (executable)
--- a/test/integration/test-apt-update-ims
+++ b/test/integration/test-apt-update-ims
@@ -37,7 +37,7 @@ runtest() {
 
     # check that I-M-S header is kept in redirections
     echo "$EXPECT" | sed -e 's#(invalid since [^)]\+)#(invalid since)#' > expected.output
-    $TEST aptget update  -o Debug::pkgAcquire::Worker=0 -o Debug::Acquire::http=0
+    $TEST aptget update  -o Debug::pkgAcquire::Worker=0 -o Debug::Acquire::http=0 -q=0
     sed -i -e 's#(invalid since [^)]\+)#(invalid since)#' rootdir/tmp/${TEST}.output
     testequal "$(cat expected.output)" cat rootdir/tmp/${TEST}.output
     testfileequal 'listsdir.lst' "$(listcurrentlistsdirectory)"
@@ -80,7 +80,8 @@ Hit:2 http://localhost:${APTHTTPPORT} unstable Release
 Ign:3 http://localhost:${APTHTTPPORT} unstable Release.gpg
   404  Not Found
 Reading package lists...
-W: The data from 'http://localhost:${APTHTTPPORT} unstable Release' is not signed. Packages from that repository can not be authenticated."
+W: The repository 'http://localhost:${APTHTTPPORT} unstable Release' is not signed.
+N: Data from such a repository can not be authenticated and is therefore potentially dangerous to use."
 find aptarchive -name 'Release.gpg' -delete
 echo 'Acquire::GzipIndexes "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
 runtest 'warning'
@@ -123,7 +124,8 @@ Hit:2 http://localhost:${APTHTTPPORT} unstable Release
 Ign:3 http://localhost:${APTHTTPPORT} unstable Release.gpg
   404  Not Found
 Reading package lists...
-W: The data from 'http://localhost:${APTHTTPPORT} unstable Release' is not signed. Packages from that repository can not be authenticated.
+W: The repository 'http://localhost:${APTHTTPPORT} unstable Release' is not signed.
+N: Data from such a repository can not be authenticated and is therefore potentially dangerous to use.
 E: Release file for http://localhost:${APTHTTPPORT}/dists/unstable/Release is expired (invalid since). Updates for this repository will not be applied."
 find aptarchive -name 'Release.gpg' -delete
 echo 'Acquire::GzipIndexes "0";' > rootdir/etc/apt/apt.conf.d/02compressindex
@@ -159,7 +161,8 @@ Hit:3 http://localhost:${APTHTTPPORT} unstable/main Sources
 Hit:4 http://localhost:${APTHTTPPORT} unstable/main amd64 Packages
 Hit:5 http://localhost:${APTHTTPPORT} unstable/main Translation-en
 Reading package lists...
-W: The repository 'http://localhost:${APTHTTPPORT} unstable Release' does not have a Release file. This is deprecated, please contact the owner of the repository."
+W: The repository 'http://localhost:${APTHTTPPORT} unstable Release' does not have a Release file.
+N: Data from such a repository can not be authenticated and is therefore potentially dangerous to use."
 find aptarchive -name '*Release*' -delete
 echo 'Acquire::GzipIndexes "0";
 Acquire::PDiffs "0";' > rootdir/etc/apt/apt.conf.d/02compressindex

diff --git a/test/integration/test-bug-596498-trusted-unsigned-repo b/test/integration/test-bug-596498-trusted-unsigned-repo
index 94f280b8139b5f97859ec747ef11d4f415dd6fa5..a9e894bc9a93471d5e6c3a37c8c8294c3eade508 100755 (executable)
--- a/test/integration/test-bug-596498-trusted-unsigned-repo
+++ b/test/integration/test-bug-596498-trusted-unsigned-repo
@@ -8,6 +8,7 @@ configarchitecture 'i386'
 
 buildsimplenativepackage 'cool' 'i386' '1.0' 'unstable'
 
+msgmsg 'default setup'
 setupaptarchive
 
 aptgetupdate() {
@@ -28,6 +29,7 @@ testsuccessequal "$PKGTEXT
 $DOWNLOG
 Download complete and in download only mode" aptget install cool --assume-no -d --allow-unauthenticated
 
+msgmsg 'sources marked trusted=no'
 sed -i -e 's#\(deb\(-src\)\?\) #\1 [trusted=no] #' $DEBFILE
 aptgetupdate 'testsuccess'
 
@@ -47,6 +49,7 @@ Download complete and in download only mode" aptget install cool:i386 --assume-n
 configarchitecture 'i386'
 
 find aptarchive/ \( -name 'Release.gpg' -o -name 'InRelease' \) -delete
+msgmsg 'unsigned repo'
 sed -i -e 's#\(deb\(-src\)\?\) \[trusted=no\] #\1 #' $DEBFILE
 aptgetupdate
 
@@ -63,8 +66,9 @@ Authentication warning overridden.
 $DOWNLOG
 Download complete and in download only mode" aptget install cool --assume-no -d --allow-unauthenticated
 
+msgmsg 'sources marked trusted=yes'
 sed -i -e 's#\(deb\(-src\)\?\) #\1 [trusted=yes] #' $DEBFILE
-aptgetupdate
+aptgetupdate 'testsuccess'
 
 testsuccessequal "$PKGTEXT
 $DOWNLOG

diff --git a/test/integration/test-sourceslist-trusted-options b/test/integration/test-sourceslist-trusted-options
index 86036e2427351fac78d40bb40be9ea121945b804..78c705b0f21d480bf3704c593e75281ba4fa794c 100755 (executable)
--- a/test/integration/test-sourceslist-trusted-options
+++ b/test/integration/test-sourceslist-trusted-options
@@ -109,7 +109,7 @@ everythingsucceeds -t testing
 msgmsg 'Test with trusted=yes option and good and unsigned sources'
 cp -a rootdir/etc/apt/sources.list.d.bak/* rootdir/etc/apt/sources.list.d/
 sed -i 's#^deb\(-src\)\? #deb\1 [trusted=yes] #' rootdir/etc/apt/sources.list.d/*
-aptgetupdate 'testwarning'
+aptgetupdate
 everythingsucceeds
 everythingsucceeds -t stable
 everythingsucceeds -t testing
@@ -187,7 +187,7 @@ everythingfails -t testing
 msgmsg 'Test with trusted=yes option and unsigned and good sources'
 cp -a rootdir/etc/apt/sources.list.d.bak/* rootdir/etc/apt/sources.list.d/
 sed -i 's#^deb\(-src\)\? #deb\1 [trusted=yes] #' rootdir/etc/apt/sources.list.d/*
-aptgetupdate 'testwarning'
+aptgetupdate
 everythingsucceeds
 everythingsucceeds -t stable
 everythingsucceeds -t testing