Merge branch 'pu/legacy-keyring-warning' into 'main'

Warn if the legacy trusted.gpg keyring is used for verification

See merge request apt-team/apt!209

6 files changed, 87 insertions, 4 deletions

diff --git a/cmdline/apt-key.in b/cmdline/apt-key.in
index 3c83a8b3d..80b0c2ade 100644
--- a/cmdline/apt-key.in
+++ b/cmdline/apt-key.in
@@ -645,6 +645,9 @@ if [ -z "$TRUSTEDFILE" ]; then
    TRUSTEDFILE="/etc/apt/trusted.gpg"
    eval $(apt-config shell TRUSTEDFILE Apt::GPGV::TrustedKeyring)
    eval $(apt-config shell TRUSTEDFILE Dir::Etc::Trusted/f)
+   if [ "$APT_KEY_NO_LEGACY_KEYRING" ]; then
+        TRUSTEDFILE="/dev/null"
+   fi
 fi
 
 command="$1"
diff --git a/debian/NEWS b/debian/NEWS
index b5547fd12..50ee37653 100644
--- a/debian/NEWS
+++ b/debian/NEWS
@@ -1,3 +1,11 @@
+apt (2.3.15) UNRELEASED; urgency=medium
+
+  GPG verification now first tries only the trusted.gpg.d keys, before
+  then falling back to the legacy trusted.gpg keyring and issuing a
+  warning to migrate keys if verification succeeded in the fallback.
+
+ -- Julian Andres Klode <jak@debian.org>  Fri, 07 Jan 2022 13:04:28 +0100
+
 apt (2.3.12) unstable; urgency=medium
 
   The solver will no longer try to remove Essential or Protected packages,
diff --git a/doc/apt-key.8.xml b/doc/apt-key.8.xml
index 6167a7826..5f2701e0c 100644
--- a/doc/apt-key.8.xml
+++ b/doc/apt-key.8.xml
@@ -202,14 +202,14 @@
 
 <para>If your existing use of <command>apt-key add</command> looks like this:</para>
 <para><literal>wget -qO- https://myrepo.example/myrepo.asc | sudo apt-key add -</literal></para>
-<para>Then you can directly replace this with:</para>
+<para>Then you can directly replace this with (though note the recommendation below):</para>
 <para><literal>wget -qO- https://myrepo.example/myrepo.asc | sudo tee /etc/apt/trusted.gpg.d/myrepo.asc</literal></para>
 <para>Make sure to use the "<literal>asc</literal>" extension for ASCII armored
 keys and the "<literal>gpg</literal>" extension for the binary OpenPGP
 format (also known as "GPG key public ring"). The binary OpenPGP format works
 for all apt versions, while the ASCII armored format works for apt version >=
 1.4.</para>
-<para>Instead of placing keys into the <filename>/etc/apt/trusted.gpg.d</filename>
+<para><emphasis>Recommended:</emphasis> Instead of placing keys into the <filename>/etc/apt/trusted.gpg.d</filename>
 directory, you can place them anywhere on your filesystem by using the
 <literal>Signed-By</literal> option in your <literal>sources.list</literal> and
 pointing to the filename of the key. See &sources-list; for details.
diff --git a/doc/sources.list.5.xml b/doc/sources.list.5.xml
index 261c90656..6c278dd61 100644
--- a/doc/sources.list.5.xml
+++ b/doc/sources.list.5.xml
@@ -299,7 +299,10 @@ deb-src [ option1=value1 option2=value2 ] uri suite [component1] [component2] [.
 		It is specified as a list of absolute paths to keyring files (have to be
 		accessible and readable for the <literal>_apt</literal> system user,
 		so ensure everyone has read-permissions on the file) and fingerprints
-		of keys to select from these keyrings. If no keyring files are specified
+		of keys to select from these keyrings. The recommended locations for keyrings
+		are <filename>/usr/share/keyrings</filename> for keyrings managed by packages,
+		and <filename>/etc/apt/keyrings</filename> for keyrings managed by the system operator.
+		If no keyring files are specified
 		the default is the <filename>trusted.gpg</filename> keyring and
 		all keyrings in the <filename>trusted.gpg.d/</filename> directory
 		(see <command>apt-key fingerprint</command>). If no fingerprint is
diff --git a/methods/gpgv.cc b/methods/gpgv.cc
index 594e8781a..fdd8586b4 100644
--- a/methods/gpgv.cc
+++ b/methods/gpgv.cc
@@ -120,6 +120,11 @@ 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:
    virtual bool URIAcquire(std::string const &Message, FetchItem *Itm) APT_OVERRIDE;
    public:
@@ -183,6 +188,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
    {
       std::ostringstream keys;
       implodeVector(keyFiles, keys, ",");
+      setenv("APT_KEY_NO_LEGACY_KEYRING", "1", true);
       ExecGPGV(outfile, file, 3, fd, keys.str());
    }
    close(fd[1]);
@@ -415,7 +421,43 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
    else
       return _("Unknown error executing apt-key");
 }
+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;
+   if (keyFiles.empty() && (Signers.Good.empty() || !Signers.Bad.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";
+
+      // Retry against trusted.gpg
+      SignersStorage legacySigners;
+
+      string const legacyMsg = VerifyGetSigners(file, outfile, keyFpts, legacyKeyFiles, legacySigners);
+      if (_error->PendingError())
+	 return legacyMsg;
+      // Hooray, we found the key now
+      if (not(legacySigners.Good.empty() || !legacySigners.Bad.empty() || !legacySigners.NoPubKey.empty()))
+      {
+	 std::string warning;
+	 strprintf(warning,
+		   _("Key is stored in legacy trusted.gpg keyring (%s), see the DEPRECATION section in apt-key(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;
@@ -454,7 +496,7 @@ bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
    }
 
    // Run apt-key on file, extract contents and get the key ID of the signer
-   string const msg = VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(), keyFpts, keyFiles, Signers);
+   string const msg = VerifyGetSignersWithLegacy(Path.c_str(), Itm->DestFile.c_str(), keyFpts, keyFiles, Signers);
    if (_error->PendingError())
       return false;
 
diff --git a/test/integration/test-method-gpgv-legacy-keyring b/test/integration/test-method-gpgv-legacy-keyring
new file mode 100755
index 000000000..37a86529a
--- /dev/null
+++ b/test/integration/test-method-gpgv-legacy-keyring
@@ -0,0 +1,27 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+
+setupenvironment
+configarchitecture "amd64"
+
+insertpackage 'unstable' 'foo' 'all' '1'
+
+buildaptarchive
+setupaptarchive --no-update
+
+testsuccessequal "Get:1 file:${TMPWORKINGDIRECTORY}/aptarchive unstable InRelease [1420 B]
+Get:1 file:${TMPWORKINGDIRECTORY}/aptarchive unstable InRelease [1420 B]
+Get:2 file:${TMPWORKINGDIRECTORY}/aptarchive unstable/main all Packages [247 B]
+Get:3 file:${TMPWORKINGDIRECTORY}/aptarchive unstable/main Translation-en [224 B]
+Reading package lists..." aptget update -q
+
+cat rootdir/etc/apt/trusted.gpg.d/*.gpg > rootdir/etc/apt/trusted.gpg
+rm rootdir/etc/apt/trusted.gpg.d/*.gpg
+
+testwarningequal "Get:1 file:${TMPWORKINGDIRECTORY}/aptarchive unstable InRelease [1420 B]
+Get:1 file:${TMPWORKINGDIRECTORY}/aptarchive unstable InRelease [1420 B]
+Reading package lists...
+W: file:${TMPWORKINGDIRECTORY}/aptarchive/dists/unstable/InRelease: Key is stored in legacy trusted.gpg keyring (${TMPWORKINGDIRECTORY}/rootdir/etc/apt/trusted.gpg), see the DEPRECATION section in apt-key(8) for details." aptget update -q