Merge branch 'trust-deprecation' into 'main'

Warn about missing Signed-By in .list format

See merge request apt-team/apt!431

8 files changed, 28 insertions, 139 deletions

diff --git a/apt-pkg/contrib/gpgv.cc b/apt-pkg/contrib/gpgv.cc
index 237ddc856..173bfdf22 100644
--- a/apt-pkg/contrib/gpgv.cc
+++ b/apt-pkg/contrib/gpgv.cc
@@ -399,8 +399,8 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
    if (not FoundKeyring)
    {
       Parts = GetListOfFilesInDir(_config->FindDir("Dir::Etc::TrustedParts"), std::vector<std::string>{"gpg", "asc"}, true);
-      if (char *env = getenv("APT_KEY_NO_LEGACY_KEYRING"); env == nullptr || not StringToBool(env, false))
-	 Parts.insert(Parts.begin(), _config->FindFile("Dir::Etc::Trusted"));
+      if (auto trusted = _config->FindFile("Dir::Etc::Trusted"); not trusted.empty())
+	 Parts.push_back(trusted);
       for (auto &Part : Parts)
       {
 	 if (Debug)
diff --git a/apt-pkg/init.cc b/apt-pkg/init.cc
index d71d9541f..d56973eeb 100644
--- a/apt-pkg/init.cc
+++ b/apt-pkg/init.cc
@@ -159,7 +159,6 @@ bool pkgInitConfig(Configuration &Cnf)
    Cnf.CndSet("Dir::Etc::parts","apt.conf.d");
    Cnf.CndSet("Dir::Etc::preferences","preferences");
    Cnf.CndSet("Dir::Etc::preferencesparts","preferences.d");
-   Cnf.CndSet("Dir::Etc::trusted", "trusted.gpg");
    Cnf.CndSet("Dir::Etc::trustedparts","trusted.gpg.d");
    Cnf.CndSet("Dir::Bin::methods", LIBEXEC_DIR "/methods");
    Cnf.CndSet("Dir::Bin::solvers::",LIBEXEC_DIR  "/solvers");
diff --git a/apt-private/private-update.cc b/apt-private/private-update.cc
index 694480a96..d5faf5d2f 100644
--- a/apt-private/private-update.cc
+++ b/apt-private/private-update.cc
@@ -106,15 +106,23 @@ bool DoUpdate()
 
    if (_config->FindB("APT::Get::Update::SourceListWarnings::SignedBy", SLWarnings))
    {
+      bool allDeb822 = true;
       for (auto *S : *List)
       {
-	 if (not S->HasFlag(metaIndex::Flag::DEB822) || not S->GetSignedBy().empty())
+	 if (not S->GetSignedBy().empty())
 	    continue;
 
 	 URI uri(S->GetURI());
 	 // TRANSLATOR: the first is manpage reference, the last the URI from a sources.list
 	 _error->Notice(_("Missing Signed-By in the %s entry for '%s'"),
 			"sources.list(5)", URI::ArchiveOnly(uri).c_str());
+	 allDeb822 &= S->HasFlag(metaIndex::Flag::DEB822);
+      }
+      if (not allDeb822)
+      {
+	 _error->Notice(_("Consider migrating all sources.list(5) entries to the deb822 .sources format"));
+	 _error->Notice(_("The deb822 .sources format supports both embedded as well as external OpenPGP keys"));
+	 _error->Notice(_("See apt-secure(7) for best practices in configuring repository signing."));
       }
    }
 
diff --git a/methods/gpgv.cc b/methods/gpgv.cc
index 1a6356dee..ecdbe5898 100644
--- a/methods/gpgv.cc
+++ b/methods/gpgv.cc
@@ -121,10 +121,6 @@ class GPGVMethod : public aptMethod
 				vector<string> const &keyFpts,
 				vector<string> const &keyFiles,
 				SignersStorage &Signers);
-   string VerifyGetSignersWithLegacy(const char *file, const char *outfile,
-				     vector<string> const &keyFpts,
-				     vector<string> const &keyFiles,
-				     SignersStorage &Signers);
 
    protected:
    bool URIAcquire(std::string const &Message, FetchItem *Itm) override;
@@ -482,49 +478,6 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
    else
       return _("Unknown error executing gpgv");
 }
-string GPGVMethod::VerifyGetSignersWithLegacy(const char *file, const char *outfile,
-					      vector<string> const &keyFpts,
-					      vector<string> const &keyFiles,
-					      SignersStorage &Signers)
-{
-   string const msg = VerifyGetSigners(file, outfile, keyFpts, keyFiles, Signers);
-   if (_error->PendingError())
-      return msg;
-
-   // Bad signature always remains bad, no need to retry against trusted.gpg
-   if (!Signers.Bad.empty())
-      return msg;
-
-   // We do not have a key file pinned, did not find a good signature, but found
-   // missing keys - let's retry with trusted.gpg
-   if (keyFiles.empty() && Signers.Valid.empty() && !Signers.NoPubKey.empty())
-   {
-      std::vector<std::string> legacyKeyFiles{_config->FindFile("Dir::Etc::trusted")};
-      if (legacyKeyFiles[0].empty())
-	 return msg;
-      if (DebugEnabled())
-	 std::clog << "Retrying against " << legacyKeyFiles[0] << "\n";
-
-      SignersStorage legacySigners;
-
-      string const legacyMsg = VerifyGetSigners(file, outfile, keyFpts, legacyKeyFiles, legacySigners);
-      if (_error->PendingError())
-	 return legacyMsg;
-      // Hooray, we found a key apparently, something verified as good or bad
-      if (!legacySigners.Valid.empty() || !legacySigners.Bad.empty())
-      {
-	 std::string warning;
-	 strprintf(warning,
-		   _("Key is stored in legacy trusted.gpg keyring (%s). Use Signed-By instead. See the USER CONFIGURATION section in apt-secure(8) for details."),
-		   legacyKeyFiles[0].c_str());
-	 Warning(std::move(warning));
-	 Signers = std::move(legacySigners);
-	 return legacyMsg;
-      }
-
-   }
-   return msg;
-}
 static std::string GenerateKeyFile(std::string const key)
 {
    FileFd fd;
@@ -563,7 +516,7 @@ bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
    }
 
    // Run gpgv on file, extract contents and get the key ID of the signer
-   string const msg = VerifyGetSignersWithLegacy(Path.c_str(), Itm->DestFile.c_str(), keyFpts, keyFiles, Signers);
+   string const msg = VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(), keyFpts, keyFiles, Signers);
    if (_error->PendingError())
       return false;
 
diff --git a/methods/sqv.cc b/methods/sqv.cc
index ce7e36657..9dbe75994 100644
--- a/methods/sqv.cc
+++ b/methods/sqv.cc
@@ -133,6 +133,8 @@ bool SQVMethod::VerifyGetSigners(const char *file, const char *outfile,
    if (keyFiles.empty())
    {
       auto Parts = GetListOfFilesInDir(_config->FindDir("Dir::Etc::TrustedParts"), std::vector<std::string>{"gpg", "asc"}, true);
+      if (auto trusted = _config->FindFile("Dir::Etc::Trusted"); not trusted.empty())
+	 Parts.push_back(trusted);
       for (auto &Part : Parts)
       {
 	 if (Debug)
@@ -298,27 +300,8 @@ bool SQVMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
 
    // Run sqv on file, extract contents and get the key ID of the signer
    VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(), keyFiles, Signers);
-   if (_error->PendingError())
-   {
-      // Legacy fallback to trusted.gpg
-      auto trusted = _config->FindFile("Dir::Etc::Trusted");
-      _error->PushToStack();
-      VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(), {trusted}, Signers);
-      bool error = _error->PendingError();
-      _error->RevertToStack();
-      if (error)
-	 return false;
-
-      _error->Discard();
-      std::string warning;
-      strprintf(warning,
-		_("Key is stored in legacy trusted.gpg keyring (%s). Use Signed-By instead. See the USER CONFIGURATION section in apt-secure(8) for details."),
-		trusted.c_str());
-      Warning(std::move(warning));
-   }
-
    if (Signers.empty())
-      return _error->Error("No good signature");
+      return _error->PendingError() ? false : _error->Error("No good signature");
 
    if (not keyFpts.empty())
    {
diff --git a/test/integration/framework b/test/integration/framework
index 7c1b31ca2..3784ece06 100644
--- a/test/integration/framework
+++ b/test/integration/framework
@@ -580,6 +580,8 @@ EOF
 
 	echo 'APT::Machine-ID "912e43bd1c1d4ba481f9f8ccab25f9ee";' > rootdir/etc/apt/apt.conf.d/machine-id
 
+	echo "APT::Get::Update::SourceListWarnings::SignedBy \"false\";" >> rootdir/etc/apt/apt.conf.d/signed-by
+
 	configcompression '.' 'gz' #'bz2' 'lzma' 'xz'
 	confighashes 'SHA256' # these are tests, not security best-practices
 
diff --git a/test/integration/test-apt-get-update-sourceslist-warning b/test/integration/test-apt-get-update-sourceslist-warning
index 115dd3e16..652f9f49f 100755
--- a/test/integration/test-apt-get-update-sourceslist-warning
+++ b/test/integration/test-apt-get-update-sourceslist-warning
@@ -8,6 +8,7 @@ setupenvironment
 configarchitecture 'amd64'
 setupaptarchive --no-update
 
+rm rootdir/etc/apt/apt.conf.d/signed-by
 testsuccess apt update
 testsuccess apt update --no-download
 
@@ -44,7 +45,15 @@ rm rootdir/etc/apt/sources.list.d/example.sources
 msgmsg 'Detect login info embedded in sources.list'
 echo 'deb http://apt:debian@example.org/debian bookworm main' > rootdir/etc/apt/sources.list.d/example.list
 testsuccessequal "$BOILERPLATE
-N: Usage of apt_auth.conf(5) should be preferred over embedding login information directly in the sources.list(5) entry for 'http://example.org/debian'" apt update --no-download
+N: Usage of apt_auth.conf(5) should be preferred over embedding login information directly in the sources.list(5) entry for 'http://example.org/debian'
+N: Missing Signed-By in the sources.list(5) entry for 'http://example.org/debian'
+N: Consider migrating all sources.list(5) entries to the deb822 .sources format
+N: The deb822 .sources format supports both embedded as well as external OpenPGP keys
+N: See apt-secure(7) for best practices in configuring repository signing." apt update --no-download
 echo 'deb tor+https://apt:debian@example.org/debian bookworm main' > rootdir/etc/apt/sources.list.d/example.list
 testsuccessequal "$BOILERPLATE
-N: Usage of apt_auth.conf(5) should be preferred over embedding login information directly in the sources.list(5) entry for 'tor+https://example.org/debian'" apt update --no-download
+N: Usage of apt_auth.conf(5) should be preferred over embedding login information directly in the sources.list(5) entry for 'tor+https://example.org/debian'
+N: Missing Signed-By in the sources.list(5) entry for 'tor+https://example.org/debian'
+N: Consider migrating all sources.list(5) entries to the deb822 .sources format
+N: The deb822 .sources format supports both embedded as well as external OpenPGP keys
+N: See apt-secure(7) for best practices in configuring repository signing." apt update --no-download
diff --git a/test/integration/test-method-gpgv-legacy-keyring b/test/integration/test-method-gpgv-legacy-keyring
deleted file mode 100755
index cc500ad1c..000000000
--- a/test/integration/test-method-gpgv-legacy-keyring
+++ /dev/null
@@ -1,65 +0,0 @@
-#!/bin/sh
-set -e
-
-TESTDIR="$(readlink -f "$(dirname "$0")")"
-. "$TESTDIR/framework"
-
-setupenvironment
-configarchitecture "amd64"
-
-insertpackage 'testing' 'foo' 'all' '1'
-insertpackage 'unstable' 'foo' 'all' '1'
-
-buildaptarchive
-setupaptarchive --no-update
-changetowebserver
-
-alias inrelease_size="stat -c %s aptarchive/dists/testing/InRelease"
-alias uinrelease_size="stat -c %s aptarchive/dists/unstable/InRelease"
-
-testsuccessequal "Get:1 http://localhost:${APTHTTPPORT} testing InRelease [$(inrelease_size) B]
-Get:2 http://localhost:${APTHTTPPORT} unstable InRelease [$(uinrelease_size) B]
-Get:3 http://localhost:${APTHTTPPORT} testing/main all Packages [248 B]
-Get:4 http://localhost:${APTHTTPPORT} testing/main Translation-en [225 B]
-Get:5 http://localhost:${APTHTTPPORT} unstable/main all Packages [246 B]
-Get:6 http://localhost:${APTHTTPPORT} unstable/main Translation-en [224 B]
-Reading package lists..." aptget update -q
-
-mv rootdir/etc/apt/trusted.gpg.d/joesixpack.gpg rootdir/etc/apt/trusted.gpg
-
-testwarningequal "Hit:1 http://localhost:${APTHTTPPORT} testing InRelease
-Hit:2 http://localhost:${APTHTTPPORT} unstable InRelease
-Reading package lists...
-W: http://localhost:${APTHTTPPORT}/dists/testing/InRelease: Key is stored in legacy trusted.gpg keyring (${TMPWORKINGDIRECTORY}/rootdir/etc/apt/trusted.gpg). Use Signed-By instead. See the USER CONFIGURATION section in apt-secure(8) for details.
-W: http://localhost:${APTHTTPPORT}/dists/unstable/InRelease: Key is stored in legacy trusted.gpg keyring (${TMPWORKINGDIRECTORY}/rootdir/etc/apt/trusted.gpg). Use Signed-By instead. See the USER CONFIGURATION section in apt-secure(8) for details." aptget update -q
-
-# 2.4.0 regression: If the InRelease file was signed with two keys, fallback to trusted.gpg did not
-# work: It ran the fallback, but then ignored the result, as keys were still missing.
-original_inrelease_size=$(inrelease_size)
-cp -a aptarchive/dists/unstable/InRelease aptarchive/dists/unstable/InRelease.bak
-redatereleasefiles '+1 hour' 'Joe Sixpack,Marvin Paranoid'
-cp -a aptarchive/dists/unstable/InRelease.bak aptarchive/dists/unstable/InRelease
-testwarningequal "Get:1 http://localhost:${APTHTTPPORT} testing InRelease [$(inrelease_size) B]
-Hit:2 http://localhost:${APTHTTPPORT} unstable InRelease
-Reading package lists...
-W: http://localhost:${APTHTTPPORT}/dists/testing/InRelease: Key is stored in legacy trusted.gpg keyring (${TMPWORKINGDIRECTORY}/rootdir/etc/apt/trusted.gpg). Use Signed-By instead. See the USER CONFIGURATION section in apt-secure(8) for details.
-W: http://localhost:${APTHTTPPORT}/dists/unstable/InRelease: Key is stored in legacy trusted.gpg keyring (${TMPWORKINGDIRECTORY}/rootdir/etc/apt/trusted.gpg). Use Signed-By instead. See the USER CONFIGURATION section in apt-secure(8) for details." aptget update -q -omsg=with-two-signatures
-
-# Now the first one is good, hooray
-cp keys/marvinparanoid.pub rootdir/etc/apt/trusted.gpg.d/marvinparanoid.gpg
-testwarningequal "Hit:1 http://localhost:${APTHTTPPORT} testing InRelease
-Hit:2 http://localhost:${APTHTTPPORT} unstable InRelease
-Reading package lists...
-W: http://localhost:${APTHTTPPORT}/dists/unstable/InRelease: Key is stored in legacy trusted.gpg keyring (${TMPWORKINGDIRECTORY}/rootdir/etc/apt/trusted.gpg). Use Signed-By instead. See the USER CONFIGURATION section in apt-secure(8) for details." aptget update -q -omsg=with-two-signatures
-
-# Now the 2nd one is good
-cp keys/marvinparanoid.pub rootdir/etc/apt/trusted.gpg.d/marvinparanoid.gpg
-redatereleasefiles '+2 hour' 'Joe Sixpack'
-cp -a aptarchive/dists/testing/InRelease aptarchive/dists/testing/InRelease.bak
-redatereleasefiles '+2 hour' 'Joe Sixpack,Marvin Paranoid'
-cp -a aptarchive/dists/testing/InRelease.bak aptarchive/dists/testing/InRelease
-testwarningequal "Get:1 http://localhost:${APTHTTPPORT} testing InRelease [$(inrelease_size) B]
-Get:2 http://localhost:${APTHTTPPORT} unstable InRelease [$(uinrelease_size) B]
-Reading package lists...
-W: http://localhost:${APTHTTPPORT}/dists/testing/InRelease: Key is stored in legacy trusted.gpg keyring (${TMPWORKINGDIRECTORY}/rootdir/etc/apt/trusted.gpg). Use Signed-By instead. See the USER CONFIGURATION section in apt-secure(8) for details." aptget update -q -omsg=with-two-signatures
-