Browse Source

report apt-key errors via status-fd messages

We report warnings from apt-key this way already since
29c590951f, so reporting errors seems like
a good addition. Most of those errors aren't really from apt-key
through, but from the code setting up and actually calling it which used
to just print to stderr which might or might not intermix them with
(other) progress lines in update calls. Having them as proper error
messages in the system means that the errors are actually collected
later on for the list instead of ending up with our relatively generic
but in those cases bogus hint regarding "is gpgv installed?".

The effective difference is minimal as the errors apply mostly to
systems which have far worse problems than a not as nice looking error
message, which makes this pretty hard to test – but at least now the
hint that your system is broken can be read in proper order (= there
aren't many valid cases in which the permissions of /tmp are messed up…).

LP: #1522988
debian/1.8.y
David Kalnischkies 6 years ago
parent
commit
8e438ede2f
  1. 63
      apt-pkg/contrib/gpgv.cc
  2. 37
      cmdline/apt-key.in
  3. 7
      methods/gpgv.cc

63
apt-pkg/contrib/gpgv.cc

@ -20,6 +20,7 @@
#include <algorithm>
#include <fstream>
#include <iostream>
#include <sstream>
#include <string>
#include <vector>
@ -44,6 +45,47 @@ static char * GenerateTemporaryFileTemplate(const char *basename) /*{{{*/
And as a cherry on the cake, we use our apt-key wrapper to do part
of the lifting in regards to merging keyrings. Fun for the whole family.
*/
static bool iovprintf(std::ostream &out, const char *format,
va_list &args, ssize_t &size) {
char *S = (char*)malloc(size);
ssize_t const n = vsnprintf(S, size, format, args);
if (n > -1 && n < size) {
out << S;
free(S);
return true;
} else {
if (n > -1)
size = n + 1;
else
size *= 2;
}
free(S);
return false;
}
static void APT_PRINTF(4) apt_error(std::ostream &outterm, int const statusfd, int fd[2], const char *format, ...)
{
std::ostringstream outstr;
std::ostream &out = (statusfd == -1) ? outterm : outstr;
va_list args;
ssize_t size = 400;
while (true) {
bool ret;
va_start(args,format);
ret = iovprintf(out, format, args, size);
va_end(args);
if (ret == true)
break;
}
if (statusfd != -1)
{
auto const errtag = "[APTKEY:] ERROR ";
outstr << '\n';
auto const errtext = outstr.str();
if (FileFd::Write(fd[1], errtag, strlen(errtag)) == false ||
FileFd::Write(fd[1], errtext.data(), errtext.size()) == false)
outterm << errtext << std::flush;
}
}
void ExecGPGV(std::string const &File, std::string const &FileGPG,
int const &statusfd, int fd[2], std::string const &key)
{
@ -112,12 +154,12 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
{
conf = GenerateTemporaryFileTemplate("apt.conf");
if (conf == nullptr) {
ioprintf(std::cerr, "Couldn't create tempfile names for passing config to apt-key");
apt_error(std::cerr, statusfd, fd, "Couldn't create tempfile names for passing config to apt-key");
local_exit(EINTERNAL);
}
int confFd = mkstemp(conf);
if (confFd == -1) {
ioprintf(std::cerr, "Couldn't create temporary file %s for passing config to apt-key", conf);
apt_error(std::cerr, statusfd, fd, "Couldn't create temporary file %s for passing config to apt-key", conf);
local_exit(EINTERNAL);
}
local_exit.files.push_back(conf);
@ -140,7 +182,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
data = GenerateTemporaryFileTemplate("apt.data");
if (sig == NULL || data == NULL)
{
ioprintf(std::cerr, "Couldn't create tempfile names for splitting up %s", File.c_str());
apt_error(std::cerr, statusfd, fd, "Couldn't create tempfile names for splitting up %s", File.c_str());
local_exit(EINTERNAL);
}
@ -152,7 +194,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
local_exit.files.push_back(sig);
if (sigFd == -1 || dataFd == -1)
{
ioprintf(std::cerr, "Couldn't create tempfiles for splitting up %s", File.c_str());
apt_error(std::cerr, statusfd, fd, "Couldn't create tempfiles for splitting up %s", File.c_str());
local_exit(EINTERNAL);
}
@ -164,7 +206,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
if (signature.Failed() == true || message.Failed() == true ||
SplitClearSignedFile(File, &message, &dataHeader, &signature) == false)
{
ioprintf(std::cerr, "Splitting up %s into data and signature failed", File.c_str());
apt_error(std::cerr, statusfd, fd, "Splitting up %s into data and signature failed", File.c_str());
local_exit(112);
}
Args.push_back(sig);
@ -203,7 +245,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
// and we do an additional check, so fork yet another time …
pid_t pid = ExecFork();
if(pid < 0) {
ioprintf(std::cerr, "Fork failed for %s to check %s", Args[0], File.c_str());
apt_error(std::cerr, statusfd, fd, "Fork failed for %s to check %s", Args[0], File.c_str());
local_exit(EINTERNAL);
}
if(pid == 0)
@ -211,7 +253,7 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
if (statusfd != -1)
dup2(fd[1], statusfd);
execvp(Args[0], (char **) &Args[0]);
ioprintf(std::cerr, "Couldn't execute %s to check %s", Args[0], File.c_str());
apt_error(std::cerr, statusfd, fd, "Couldn't execute %s to check %s", Args[0], File.c_str());
local_exit(EINTERNAL);
}
@ -221,21 +263,22 @@ void ExecGPGV(std::string const &File, std::string const &FileGPG,
{
if (errno == EINTR)
continue;
ioprintf(std::cerr, _("Waited for %s but it wasn't there"), "apt-key");
apt_error(std::cerr, statusfd, fd, _("Waited for %s but it wasn't there"), "apt-key");
local_exit(EINTERNAL);
}
// check if it exit'ed normally …
if (WIFEXITED(Status) == false)
{
ioprintf(std::cerr, _("Sub-process %s exited unexpectedly"), "apt-key");
apt_error(std::cerr, statusfd, fd, _("Sub-process %s exited unexpectedly"), "apt-key");
local_exit(EINTERNAL);
}
// … and with a good exit code
if (WEXITSTATUS(Status) != 0)
{
ioprintf(std::cerr, _("Sub-process %s returned an error code (%u)"), "apt-key", WEXITSTATUS(Status));
// we forward the statuscode, so don't generate a message on the fd in this case
apt_error(std::cerr, -1, fd, _("Sub-process %s returned an error code (%u)"), "apt-key", WEXITSTATUS(Status));
local_exit(WEXITSTATUS(Status));
}

37
cmdline/apt-key.in

@ -17,7 +17,7 @@ aptkey_echo() { echo "$@"; }
requires_root() {
if [ "$(id -u)" -ne 0 ]; then
echo >&2 "ERROR: This command can only be used by root."
apt_error "This command can only be used by root."
exit 1
fi
}
@ -61,11 +61,11 @@ add_keys_with_verify_against_master_keyring() {
MASTER="$2"
if [ ! -f "$ADD_KEYRING" ]; then
echo >&2 "ERROR: '$ADD_KEYRING' not found"
apt_error "Keyring '$ADD_KEYRING' to be added not found"
return
fi
if [ ! -f "$MASTER" ]; then
echo >&2 "ERROR: '$MASTER' not found"
apt_error "Master-Keyring '$MASTER' not found"
return
fi
@ -127,13 +127,13 @@ net_update() {
fi
if [ -z "$ARCHIVE_KEYRING_URI" ]; then
echo >&2 "ERROR: Your distribution is not supported in net-update as no uri for the archive-keyring is set"
apt_error 'Your distribution is not supported in net-update as no uri for the archive-keyring is set'
exit 1
fi
# in theory we would need to depend on wget for this, but this feature
# isn't useable in debian anyway as we have no keyring uri nor a master key
if ! command_available 'wget'; then
echo >&2 "ERROR: an installed wget is required for a network-based update"
apt_error 'wget is required for a network-based update, but it is not installed'
exit 1
fi
if [ ! -d "${APT_DIR}/var/lib/apt/keyrings" ]; then
@ -164,8 +164,7 @@ update() {
fi
fi
if [ ! -f "$ARCHIVE_KEYRING" ]; then
echo >&2 "ERROR: Can't find the archive-keyring"
echo >&2 "Is the &keyring-package; package installed?"
apt_error "Can't find the archive-keyring (Is the &keyring-package; package installed?)"
exit 1
fi
@ -184,7 +183,7 @@ update() {
foreach_keyring_do 'remove_key_from_keyring' "$key"
done
else
echo >&2 "Warning: removed keys keyring $REMOVED_KEYS missing or not readable"
apt_warn "Removed keys keyring '$REMOVED_KEYS' missing or not readable"
fi
}
@ -239,7 +238,7 @@ accessible_file_exists() {
if test -r "$1"; then
return 0
fi
warn "The key(s) in the keyring $1 are ignored as the file is not readable by user '$USER' executing apt-key."
apt_warn "The key(s) in the keyring $1 are ignored as the file is not readable by user '$USER' executing apt-key."
return 1
}
@ -486,7 +485,7 @@ find_gpgv_status_fd() {
}
GPGSTATUSFD="$(find_gpgv_status_fd "$@")"
warn() {
apt_warn() {
if [ -z "$GPGHOMEDIR" ]; then
echo >&2 'W:' "$@"
else
@ -496,6 +495,16 @@ warn() {
echo >&${GPGSTATUSFD} '[APTKEY:] WARNING' "$@"
fi
}
apt_error() {
if [ -z "$GPGHOMEDIR" ]; then
echo >&2 'E:' "$@"
else
echo 'E:' "$@" > "${GPGHOMEDIR}/aptwarnings.log"
fi
if [ -n "$GPGSTATUSFD" ]; then
echo >&${GPGSTATUSFD} '[APTKEY:] ERROR' "$@"
fi
}
cleanup_gpg_home() {
if [ -z "$GPGHOMEDIR" ]; then return; fi
@ -522,7 +531,7 @@ create_gpg_home() {
CURRENTTRAP="${CURRENTTRAP} cleanup_gpg_home;"
trap "${CURRENTTRAP}" 0 HUP INT QUIT ILL ABRT FPE SEGV PIPE TERM
if [ -z "$GPGHOMEDIR" ]; then
echo "ERROR: Could not create temporary gpg home directory in apt-key ($TMPDIR)"
apt_error "Could not create temporary gpg home directory in $TMPDIR (wrong permissions?)"
exit 28
fi
chmod 700 "$GPGHOMEDIR"
@ -553,9 +562,7 @@ EOF
elif command_available 'gpg1'; then
GPG_EXE="gpg1"
else
echo >&2 "Error: gnupg, gnupg2 and gnupg1 do not seem to be installed,"
echo >&2 "Error: but apt-key requires gnupg, gnupg2 or gnupg1 for this operation."
echo >&2
apt_error 'gnupg, gnupg2 and gnupg1 do not seem to be installed, but one of them is required for this operation'
exit 255
fi
@ -663,7 +670,7 @@ case "$command" in
elif command_available 'gpgv2'; then GPGV='gpgv2';
elif command_available 'gpgv1'; then GPGV='gpgv1';
else
echo >&2 'ERROR: gpgv, gpgv2 or gpgv1 required for verification'
apt_error 'gpgv, gpgv2 or gpgv1 required for verification, but neither seems installed'
exit 29
fi
# for a forced keyid we need gpg --export, so full wrapping required

7
methods/gpgv.cc

@ -40,6 +40,7 @@ using std::vector;
#define GNUPGREVKEYSIG "[GNUPG:] REVKEYSIG"
#define GNUPGNODATA "[GNUPG:] NODATA"
#define APTKEYWARNING "[APTKEY:] WARNING"
#define APTKEYERROR "[APTKEY:] ERROR"
struct Digest {
enum class State {
@ -241,6 +242,8 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
}
else if (strncmp(buffer, APTKEYWARNING, sizeof(APTKEYWARNING)-1) == 0)
Warning("%s", buffer + sizeof(APTKEYWARNING));
else if (strncmp(buffer, APTKEYERROR, sizeof(APTKEYERROR)-1) == 0)
_error->Error("%s", buffer + sizeof(APTKEYERROR));
}
fclose(pipein);
free(buffer);
@ -372,9 +375,11 @@ bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
URIStart(Res);
// Run apt-key on file, extract contents and get the key ID of the signer
string msg = VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(), key,
string const msg = VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(), key,
GoodSigners, BadSigners, WorthlessSigners,
SoonWorthlessSigners, NoPubKeySigners);
if (_error->PendingError())
return false;
// Check if all good signers are soon worthless and warn in that case
if (std::all_of(GoodSigners.begin(), GoodSigners.end(), [&](std::string const &good) {

Loading…
Cancel
Save