summaryrefslogtreecommitdiff
path: root/methods/gpgv.cc
diff options
context:
space:
mode:
authorDavid Kalnischkies <david@kalnischkies.de>2018-09-11 16:45:06 +0200
committerDavid Kalnischkies <david@kalnischkies.de>2019-01-22 12:24:22 +0100
commit6b01cd087e6f92c5511fe6eea73699e075aa699a (patch)
tree49cf7a0fa86d0cd7aafd6d316adde14f1675b5f5 /methods/gpgv.cc
parentb1314a7f81cde0b4320ad9296d8fcc9bd6fbc55f (diff)
Refactor internal Signers information storage in gpgv
Having a method take a bunch of string vectors is bad style, so we change this to a wrapping struct and adapt the rest of the code brushing it up slightly in the process, which results even in a slightly "better" debug output, no practical change otherwise. Gbp-Dch: Ignore
Diffstat (limited to 'methods/gpgv.cc')
-rw-r--r--methods/gpgv.cc140
1 files changed, 67 insertions, 73 deletions
diff --git a/methods/gpgv.cc b/methods/gpgv.cc
index 9135b49c5..1b9f6dd64 100644
--- a/methods/gpgv.cc
+++ b/methods/gpgv.cc
@@ -102,17 +102,21 @@ static bool IsTheSameKey(std::string const &validsig, std::string const &goodsig
return validsig.compare(24, 16, goodsig, strlen("GOODSIG "), 16) == 0;
}
+struct APT_HIDDEN SignersStorage {
+ std::vector<std::string> Good;
+ std::vector<std::string> Bad;
+ std::vector<std::string> Worthless;
+ // a worthless signature is a expired or revoked one
+ std::vector<Signer> SoonWorthless;
+ std::vector<std::string> NoPubKey;
+};
class GPGVMethod : public aptMethod
{
private:
string VerifyGetSigners(const char *file, const char *outfile,
vector<string> const &keyFpts,
vector<string> const &keyFiles,
- vector<string> &GoodSigners,
- vector<string> &BadSigners,
- vector<string> &WorthlessSigners,
- vector<Signer> &SoonWorthlessSigners,
- vector<string> &NoPubKeySigners);
+ SignersStorage &Signers);
protected:
virtual bool URIAcquire(std::string const &Message, FetchItem *Itm) APT_OVERRIDE;
public:
@@ -146,14 +150,18 @@ static void PushEntryWithUID(std::vector<std::string> &Signers, char * const buf
std::clog << "Got " << msg << " !" << std::endl;
Signers.push_back(msg);
}
+static void implodeVector(std::vector<std::string> const &vec, std::ostream &out, char const * const sep)
+{
+ if (vec.empty())
+ return;
+ std::copy(vec.begin(), std::prev(vec.end()), std::ostream_iterator<std::string>(out, sep));
+ out << *vec.rbegin();
+ return;
+}
string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
vector<string> const &keyFpts,
vector<string> const &keyFiles,
- vector<string> &GoodSigners,
- vector<string> &BadSigners,
- vector<string> &WorthlessSigners,
- vector<Signer> &SoonWorthlessSigners,
- vector<string> &NoPubKeySigners)
+ SignersStorage &Signers)
{
bool const Debug = DebugEnabled();
@@ -171,11 +179,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
else if (pid == 0)
{
std::ostringstream keys;
- if (keyFiles.empty() == false)
- {
- std::copy(keyFiles.begin(), keyFiles.end()-1, std::ostream_iterator<std::string>(keys, ","));
- keys << *keyFiles.rbegin();
- }
+ implodeVector(keyFiles, keys, ",");
ExecGPGV(outfile, file, 3, fd, keys.str());
}
close(fd[1]);
@@ -201,25 +205,25 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
// if we improve the apt method communication stuff later
// it will be better.
if (strncmp(buffer, GNUPGBADSIG, sizeof(GNUPGBADSIG)-1) == 0)
- PushEntryWithUID(BadSigners, buffer, Debug);
+ PushEntryWithUID(Signers.Bad, buffer, Debug);
else if (strncmp(buffer, GNUPGERRSIG, sizeof(GNUPGERRSIG)-1) == 0)
PushEntryWithKeyID(ErrSigners, buffer, Debug);
else if (strncmp(buffer, GNUPGNOPUBKEY, sizeof(GNUPGNOPUBKEY)-1) == 0)
{
- PushEntryWithKeyID(NoPubKeySigners, buffer, Debug);
+ PushEntryWithKeyID(Signers.NoPubKey, buffer, Debug);
ErrSigners.erase(std::remove_if(ErrSigners.begin(), ErrSigners.end(), [&](std::string const &errsig) {
return errsig.compare(strlen("ERRSIG "), 16, buffer, sizeof(GNUPGNOPUBKEY), 16) == 0; }), ErrSigners.end());
}
else if (strncmp(buffer, GNUPGNODATA, sizeof(GNUPGNODATA)-1) == 0)
gotNODATA = true;
else if (strncmp(buffer, GNUPGEXPKEYSIG, sizeof(GNUPGEXPKEYSIG)-1) == 0)
- PushEntryWithUID(WorthlessSigners, buffer, Debug);
+ PushEntryWithUID(Signers.Worthless, buffer, Debug);
else if (strncmp(buffer, GNUPGEXPSIG, sizeof(GNUPGEXPSIG)-1) == 0)
- PushEntryWithUID(WorthlessSigners, buffer, Debug);
+ PushEntryWithUID(Signers.Worthless, buffer, Debug);
else if (strncmp(buffer, GNUPGREVKEYSIG, sizeof(GNUPGREVKEYSIG)-1) == 0)
- PushEntryWithUID(WorthlessSigners, buffer, Debug);
+ PushEntryWithUID(Signers.Worthless, buffer, Debug);
else if (strncmp(buffer, GNUPGGOODSIG, sizeof(GNUPGGOODSIG)-1) == 0)
- PushEntryWithKeyID(GoodSigners, buffer, Debug);
+ PushEntryWithKeyID(Signers.Good, buffer, Debug);
else if (strncmp(buffer, GNUPGVALIDSIG, sizeof(GNUPGVALIDSIG)-1) == 0)
{
std::istringstream iss(buffer + sizeof(GNUPGVALIDSIG));
@@ -232,16 +236,16 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
case Digest::State::Weak:
// Treat them like an expired key: For that a message about expiry
// is emitted, a VALIDSIG, but no GOODSIG.
- SoonWorthlessSigners.push_back({sig, digest.name});
+ Signers.SoonWorthless.push_back({sig, digest.name});
if (Debug == true)
std::clog << "Got weak VALIDSIG, key ID: " << sig << std::endl;
break;
case Digest::State::Untrusted:
// Treat them like an expired key: For that a message about expiry
// is emitted, a VALIDSIG, but no GOODSIG.
- WorthlessSigners.push_back(sig);
- GoodSigners.erase(std::remove_if(GoodSigners.begin(), GoodSigners.end(), [&](std::string const &goodsig) {
- return IsTheSameKey(sig, goodsig); }), GoodSigners.end());
+ Signers.Worthless.push_back(sig);
+ Signers.Good.erase(std::remove_if(Signers.Good.begin(), Signers.Good.end(), [&](std::string const &goodsig) {
+ return IsTheSameKey(sig, goodsig); }), Signers.Good.end());
if (Debug == true)
std::clog << "Got untrusted VALIDSIG, key ID: " << sig << std::endl;
break;
@@ -264,7 +268,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
}
fclose(pipein);
free(buffer);
- std::move(ErrSigners.begin(), ErrSigners.end(), std::back_inserter(WorthlessSigners));
+ std::move(ErrSigners.begin(), ErrSigners.end(), std::back_inserter(Signers.Worthless));
// apt-key has a --keyid parameter, but this requires gpg, so we call it without it
// and instead check after the fact which keyids where used for verification
@@ -273,11 +277,11 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
if (Debug == true)
{
std::clog << "GoodSigs needs to be limited to keyid(s): ";
- std::copy(keyFpts.begin(), keyFpts.end(), std::ostream_iterator<std::string>(std::clog, ", "));
+ implodeVector(keyFpts, std::clog, ", ");
std::clog << "\n";
}
std::vector<std::string> filteredGood;
- for (auto &&good: GoodSigners)
+ for (auto &&good: Signers.Good)
{
if (Debug == true)
std::clog << "Key " << good << " is good sig, is it also a valid and allowed one? ";
@@ -320,9 +324,9 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
if (found)
filteredGood.emplace_back(std::move(good));
else
- NoPubKeySigners.emplace_back(std::move(good));
+ Signers.NoPubKey.emplace_back(std::move(good));
}
- GoodSigners = std::move(filteredGood);
+ Signers.Good= std::move(filteredGood);
}
int status;
@@ -334,16 +338,16 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
if (Debug)
{
- std::cerr << "Summary:" << std::endl << " Good: ";
- std::copy(GoodSigners.begin(), GoodSigners.end(), std::ostream_iterator<std::string>(std::cerr, ", "));
- std::cerr << std::endl << " Bad: ";
- std::copy(BadSigners.begin(), BadSigners.end(), std::ostream_iterator<std::string>(std::cerr, ", "));
- std::cerr << std::endl << " Worthless: ";
- std::copy(WorthlessSigners.begin(), WorthlessSigners.end(), std::ostream_iterator<std::string>(std::cerr, ", "));
- std::cerr << std::endl << " SoonWorthless: ";
- std::for_each(SoonWorthlessSigners.begin(), SoonWorthlessSigners.end(), [](Signer const &sig) { std::cerr << sig.key << ", "; });
- std::cerr << std::endl << " NoPubKey: ";
- std::copy(NoPubKeySigners.begin(), NoPubKeySigners.end(), std::ostream_iterator<std::string>(std::cerr, ", "));
+ std::cerr << "Summary:\n Good: ";
+ implodeVector(Signers.Good, std::cerr, ", ");
+ std::cerr << "\n Bad: ";
+ implodeVector(Signers.Bad, std::cerr, ", ");
+ std::cerr << "\n Worthless: ";
+ implodeVector(Signers.Worthless, std::cerr, ", ");
+ std::cerr << "\n SoonWorthless: ";
+ std::for_each(Signers.SoonWorthless.begin(), Signers.SoonWorthless.end(), [](Signer const &sig) { std::cerr << sig.key << ", "; });
+ std::cerr << "\n NoPubKey: ";
+ implodeVector(Signers.NoPubKey, std::cerr, ", ");
std::cerr << std::endl << " NODATA: " << (gotNODATA ? "yes" : "no") << std::endl;
}
@@ -369,12 +373,12 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
{
// gpgv will report success, but we want to enforce a certain keyring
// so if we haven't found the key the valid we found is in fact invalid
- if (GoodSigners.empty())
+ if (Signers.Good.empty())
return _("At least one invalid signature was encountered.");
}
else
{
- if (GoodSigners.empty())
+ if (Signers.Good.empty())
return _("Internal error: Good signature, but could not determine key fingerprint?!");
}
return "";
@@ -391,13 +395,8 @@ bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
{
URI const Get = Itm->Uri;
string const Path = Get.Host + Get.Path; // To account for relative paths
- vector<string> GoodSigners;
- vector<string> BadSigners;
- // a worthless signature is a expired or revoked one
- vector<string> WorthlessSigners;
- vector<Signer> SoonWorthlessSigners;
- vector<string> NoPubKeySigners;
-
+ SignersStorage Signers;
+
FetchResult Res;
Res.Filename = Itm->DestFile;
URIStart(Res);
@@ -410,68 +409,63 @@ bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
keyFpts.emplace_back(std::move(key));
// 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,
- GoodSigners, BadSigners, WorthlessSigners,
- SoonWorthlessSigners, NoPubKeySigners);
+ string const msg = VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(), keyFpts, keyFiles, Signers);
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) {
- return std::any_of(SoonWorthlessSigners.begin(), SoonWorthlessSigners.end(), [&](Signer const &weak) {
+ if (std::all_of(Signers.Good.begin(), Signers.Good.end(), [&](std::string const &good) {
+ return std::any_of(Signers.SoonWorthless.begin(), Signers.SoonWorthless.end(), [&](Signer const &weak) {
return IsTheSameKey(weak.key, good);
});
}))
{
- for (auto const & Signer : SoonWorthlessSigners)
+ for (auto const & Signer : Signers.SoonWorthless)
// TRANSLATORS: The second %s is the reason and is untranslated for repository owners.
Warning(_("Signature by key %s uses weak digest algorithm (%s)"), Signer.key.c_str(), Signer.note.c_str());
}
- if (GoodSigners.empty() || !BadSigners.empty() || !NoPubKeySigners.empty())
+ if (Signers.Good.empty() || !Signers.Bad.empty() || !Signers.NoPubKey.empty())
{
string errmsg;
// In this case, something bad probably happened, so we just go
// with what the other method gave us for an error message.
- if (BadSigners.empty() && WorthlessSigners.empty() && NoPubKeySigners.empty())
+ if (Signers.Bad.empty() && Signers.Worthless.empty() && Signers.NoPubKey.empty())
errmsg = msg;
else
{
- if (!BadSigners.empty())
+ if (!Signers.Bad.empty())
{
errmsg += _("The following signatures were invalid:\n");
- for (vector<string>::iterator I = BadSigners.begin();
- I != BadSigners.end(); ++I)
- errmsg += (*I + "\n");
+ for (auto const &I : Signers.Bad)
+ errmsg.append(I).append("\n");
}
- if (!WorthlessSigners.empty())
+ if (!Signers.Worthless.empty())
{
errmsg += _("The following signatures were invalid:\n");
- for (vector<string>::iterator I = WorthlessSigners.begin();
- I != WorthlessSigners.end(); ++I)
- errmsg += (*I + "\n");
+ for (auto const &I : Signers.Worthless)
+ errmsg.append(I).append("\n");
}
- if (!NoPubKeySigners.empty())
+ if (!Signers.NoPubKey.empty())
{
errmsg += _("The following signatures couldn't be verified because the public key is not available:\n");
- for (vector<string>::iterator I = NoPubKeySigners.begin();
- I != NoPubKeySigners.end(); ++I)
- errmsg += (*I + "\n");
+ for (auto const &I : Signers.NoPubKey)
+ errmsg.append(I).append("\n");
}
}
// this is only fatal if we have no good sigs or if we have at
// least one bad signature. good signatures and NoPubKey signatures
// happen easily when a file is signed with multiple signatures
- if(GoodSigners.empty() or !BadSigners.empty())
+ if(Signers.Good.empty() or !Signers.Bad.empty())
return _error->Error("%s", errmsg.c_str());
}
// Just pass the raw output up, because passing it as a real data
// structure is too difficult with the method stuff. We keep it
// as three separate vectors for future extensibility.
- Res.GPGVOutput = GoodSigners;
- std::move(BadSigners.begin(), BadSigners.end(), std::back_inserter(Res.GPGVOutput));
- std::move(NoPubKeySigners.begin(), NoPubKeySigners.end(), std::back_inserter(Res.GPGVOutput));
+ Res.GPGVOutput = Signers.Good;
+ std::move(Signers.Bad.begin(), Signers.Bad.end(), std::back_inserter(Res.GPGVOutput));
+ std::move(Signers.NoPubKey.begin(), Signers.NoPubKey.end(), std::back_inserter(Res.GPGVOutput));
URIDone(Res);
if (DebugEnabled())