From c4e57e200ae40db5e9ada6ec0933b53d249e4a97 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Mon, 20 Jan 2025 11:57:33 +0100 Subject: Warn about missing Signed-By in .list format Warn about the missing field there and suggest the transition to deb822 .sources files if we found any non-deb822 source without signed-by. --- test/integration/framework | 2 ++ test/integration/test-apt-get-update-sourceslist-warning | 13 +++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) (limited to 'test') 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 -- cgit v1.2.3-70-g09d2 From e14417db27771ad85f45880974c8f59f05d138f1 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Mon, 20 Jan 2025 12:31:55 +0100 Subject: Remove superseded warning about trusted.gpg fallback This warning has been superseded by the missing sources.list notice (which will also become a warning shortly) Adjust the sqv exit to avoid introducing a new spurious "No good signature" that we did not reach before. moo --- apt-pkg/contrib/gpgv.cc | 4 +- methods/gpgv.cc | 49 +----------------- methods/sqv.cc | 23 ++------- test/integration/test-method-gpgv-legacy-keyring | 65 ------------------------ 4 files changed, 6 insertions(+), 135 deletions(-) delete mode 100755 test/integration/test-method-gpgv-legacy-keyring (limited to 'test') 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{"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/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 const &keyFpts, vector const &keyFiles, SignersStorage &Signers); - string VerifyGetSignersWithLegacy(const char *file, const char *outfile, - vector const &keyFpts, - vector 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 const &keyFpts, - vector 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 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{"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/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 - -- cgit v1.2.3-70-g09d2