From 448c38bdcd72b52f11ec5f326f822cf57653f81c Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 6 Jun 2015 12:28:00 +0200 Subject: rework hashsum verification in the acquire system MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Having every item having its own code to verify the file(s) it handles is an errorprune process and easy to break, especially if items move through various stages (download, uncompress, patching, …). With a giant rework we centralize (most of) the verification to have a better enforcement rate and (hopefully) less chance for bugs, but it breaks the ABI bigtime in exchange – and as we break it anyway, it is broken even harder. It shouldn't effect most frontends as they don't deal with the acquire system at all or implement their own items, but some do and will need to be patched (might be an opportunity to use apt on-board material). The theory is simple: Items implement methods to decide if hashes need to be checked (in this stage) and to return the expected hashes for this item (in this stage). The verification itself is done in worker message passing which has the benefit that a hashsum error is now a proper error for the acquire system rather than a Done() which is later revised to a Failed(). --- methods/file.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'methods') diff --git a/methods/file.cc b/methods/file.cc index 043ab04b8..353e54bd5 100644 --- a/methods/file.cc +++ b/methods/file.cc @@ -57,7 +57,11 @@ bool FileMethod::Fetch(FetchItem *Itm) Res.LastModified = Buf.st_mtime; Res.IMSHit = false; if (Itm->LastModified == Buf.st_mtime && Itm->LastModified != 0) - Res.IMSHit = true; + { + unsigned long long const filesize = Itm->ExpectedHashes.FileSize(); + if (filesize != 0 && filesize == Res.Size) + Res.IMSHit = true; + } } // See if the uncompressed file exists and reuse it -- cgit v1.2.3 From 3679515479136179e0d95325a6559fcc6d0af7f8 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 6 Jun 2015 19:16:45 +0200 Subject: check patch hashes in rred worker instead of in the handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rred is responsible for unpacking and reading the patch files in one go, but we currently only have hashes for the uncompressed patch files, so the handler read the entire patch file before dispatching it to the worker which would read it again – both with an implicit uncompress. Worse, while the workers operate in parallel the handler is the central orchestration unit, so having it busy with work means the workers do (potentially) nothing. This means rred is working with 'untrusted' data, which is bad. Yet, having the unpack in the handler meant that the untrusted uncompress was done as root which isn't better either. Now, we have it at least contained in a binary which we can harden a bit better. In the long run, we want hashes for the compressed patch files through to be safe. --- methods/rred.cc | 62 +++++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 10 deletions(-) (limited to 'methods') diff --git a/methods/rred.cc b/methods/rred.cc index 554ac99b4..3da33c126 100644 --- a/methods/rred.cc +++ b/methods/rred.cc @@ -388,7 +388,7 @@ class Patch { public: - void read_diff(FileFd &f) + void read_diff(FileFd &f, Hashes * const h) { char buffer[BLOCK_SIZE]; bool cmdwanted = true; @@ -396,6 +396,8 @@ class Patch { Change ch(0); while(f.ReadLine(buffer, sizeof(buffer))) { + if (h != NULL) + h->Add(buffer); if (cmdwanted) { char *m, *c; size_t s, e; @@ -519,8 +521,29 @@ class RredMethod : public pkgAcqMethod { private: bool Debug; + struct PDiffFile { + std::string FileName; + HashStringList ExpectedHashes; + PDiffFile(std::string const &FileName, HashStringList const &ExpectedHashes) : + FileName(FileName), ExpectedHashes(ExpectedHashes) {} + }; + + HashStringList ReadExpectedHashesForPatch(unsigned int const patch, std::string const &Message) + { + HashStringList ExpectedHashes; + for (char const * const * type = HashString::SupportedHashes(); *type != NULL; ++type) + { + std::string tagname; + strprintf(tagname, "Patch-%d-%s-Hash", patch, *type); + std::string const hashsum = LookupTag(Message, tagname.c_str()); + if (hashsum.empty() == false) + ExpectedHashes.push_back(HashString(*type, hashsum)); + } + return ExpectedHashes; + } + protected: - virtual bool Fetch(FetchItem *Itm) { + virtual bool URIAcquire(std::string const &Message, FetchItem *Itm) { Debug = _config->FindB("Debug::pkgAcquire::RRed", false); URI Get = Itm->Uri; std::string Path = Get.Host + Get.Path; // rred:/path - no host @@ -534,11 +557,17 @@ class RredMethod : public pkgAcqMethod { } else URIStart(Res); - std::vector patchpaths; + std::vector patchfiles; Patch patch; if (FileExists(Path + ".ed") == true) - patchpaths.push_back(Path + ".ed"); + { + HashStringList const ExpectedHashes = ReadExpectedHashesForPatch(0, Message); + std::string const FileName = Path + ".ed"; + if (ExpectedHashes.usable() == false) + return _error->Error("No hashes found for uncompressed patch: %s", FileName.c_str()); + patchfiles.push_back(PDiffFile(FileName, ExpectedHashes)); + } else { _error->PushToStack(); @@ -546,18 +575,27 @@ class RredMethod : public pkgAcqMethod { _error->RevertToStack(); std::string const baseName = Path + ".ed."; + unsigned int seen_patches = 0; for (std::vector::const_iterator p = patches.begin(); p != patches.end(); ++p) + { if (p->compare(0, baseName.length(), baseName) == 0) - patchpaths.push_back(*p); + { + HashStringList const ExpectedHashes = ReadExpectedHashesForPatch(seen_patches, Message); + if (ExpectedHashes.usable() == false) + return _error->Error("No hashes found for uncompressed patch %d: %s", seen_patches, p->c_str()); + patchfiles.push_back(PDiffFile(*p, ExpectedHashes)); + ++seen_patches; + } + } } std::string patch_name; - for (std::vector::iterator I = patchpaths.begin(); - I != patchpaths.end(); + for (std::vector::iterator I = patchfiles.begin(); + I != patchfiles.end(); ++I) { - patch_name = *I; + patch_name = I->FileName; if (Debug == true) std::clog << "Patching " << Path << " with " << patch_name << std::endl; @@ -569,8 +607,12 @@ class RredMethod : public pkgAcqMethod { _error->DumpErrors(std::cerr); abort(); } - patch.read_diff(p); + Hashes patch_hash(I->ExpectedHashes); + patch.read_diff(p, &patch_hash); p.Close(); + HashStringList const hsl = patch_hash.GetHashStringList(); + if (hsl != I->ExpectedHashes) + return _error->Error("Patch %s doesn't have the expected hashsum", patch_name.c_str()); } if (Debug == true) @@ -643,7 +685,7 @@ int main(int argc, char **argv) _error->DumpErrors(std::cerr); exit(1); } - patch.read_diff(p); + patch.read_diff(p, NULL); } if (just_diff) { -- cgit v1.2.3 From 6d3e5bd8e08564c5eb12ecd869de5bd71e25f59d Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 7 Jun 2015 02:17:15 +0200 Subject: add more parsing error checking for rred The rred parser is very accepting regarding 'invalid' files. Given that we can't trust the input it might be a bit too relaxed. In any case, checking for more errors can't hurt given that we support only a very specific subset of ed commands. --- methods/rred.cc | 70 ++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 21 deletions(-) (limited to 'methods') diff --git a/methods/rred.cc b/methods/rred.cc index 3da33c126..81ecf8553 100644 --- a/methods/rred.cc +++ b/methods/rred.cc @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -35,7 +36,7 @@ class MemBlock { char *start; size_t size; char *free; - struct MemBlock *next; + MemBlock *next; MemBlock(size_t size) : size(size), next(NULL) { @@ -116,7 +117,7 @@ struct Change { size_t add_len; /* bytes */ char *add; - Change(int off) + Change(size_t off) { offset = off; del_cnt = add_cnt = add_len = 0; @@ -388,30 +389,37 @@ class Patch { public: - void read_diff(FileFd &f, Hashes * const h) + bool read_diff(FileFd &f, Hashes * const h) { char buffer[BLOCK_SIZE]; bool cmdwanted = true; - Change ch(0); - while(f.ReadLine(buffer, sizeof(buffer))) - { + Change ch(std::numeric_limits::max()); + if (f.ReadLine(buffer, sizeof(buffer)) == NULL) + return _error->Error("Reading first line of patchfile %s failed", f.Name().c_str()); + do { if (h != NULL) h->Add(buffer); if (cmdwanted) { char *m, *c; size_t s, e; - s = strtol(buffer, &m, 10); - if (m == buffer) { - s = e = ch.offset + ch.add_cnt; - c = buffer; - } else if (*m == ',') { - m++; + errno = 0; + s = strtoul(buffer, &m, 10); + if (unlikely(m == buffer || s == ULONG_MAX || errno != 0)) + return _error->Error("Parsing patchfile %s failed: Expected an effected line start", f.Name().c_str()); + else if (*m == ',') { + ++m; e = strtol(m, &c, 10); + if (unlikely(m == c || e == ULONG_MAX || errno != 0)) + return _error->Error("Parsing patchfile %s failed: Expected an effected line end", f.Name().c_str()); + if (unlikely(e < s)) + return _error->Error("Parsing patchfile %s failed: Effected lines end %lu is before start %lu", f.Name().c_str(), e, s); } else { e = s; c = m; } + if (s > ch.offset) + return _error->Error("Parsing patchfile %s failed: Effected line is after previous effected line", f.Name().c_str()); switch(*c) { case 'a': cmdwanted = false; @@ -422,6 +430,8 @@ class Patch { ch.del_cnt = 0; break; case 'c': + if (unlikely(s == 0)) + return _error->Error("Parsing patchfile %s failed: Change command can't effect line zero", f.Name().c_str()); cmdwanted = false; ch.add = NULL; ch.add_cnt = 0; @@ -430,6 +440,8 @@ class Patch { ch.del_cnt = e - s + 1; break; case 'd': + if (unlikely(s == 0)) + return _error->Error("Parsing patchfile %s failed: Delete command can't effect line zero", f.Name().c_str()); ch.offset = s - 1; ch.del_cnt = e - s + 1; ch.add = NULL; @@ -437,9 +449,11 @@ class Patch { ch.add_len = 0; filechanges.add_change(ch); break; + default: + return _error->Error("Parsing patchfile %s failed: Unknown command", f.Name().c_str()); } } else { /* !cmdwanted */ - if (buffer[0] == '.' && buffer[1] == '\n') { + if (strcmp(buffer, ".\n") == 0) { cmdwanted = true; filechanges.add_change(ch); } else { @@ -465,7 +479,8 @@ class Patch { } } } - } + } while(f.ReadLine(buffer, sizeof(buffer))); + return true; } void write_diff(FILE *f) @@ -601,14 +616,14 @@ class RredMethod : public pkgAcqMethod { << std::endl; FileFd p; + Hashes patch_hash(I->ExpectedHashes); // all patches are compressed, even if the name doesn't reflect it - if (p.Open(patch_name, FileFd::ReadOnly, FileFd::Gzip) == false) { - std::cerr << "Could not open patch file " << patch_name << std::endl; + if (p.Open(patch_name, FileFd::ReadOnly, FileFd::Gzip) == false || + patch.read_diff(p, &patch_hash) == false) + { _error->DumpErrors(std::cerr); - abort(); + return false; } - Hashes patch_hash(I->ExpectedHashes); - patch.read_diff(p, &patch_hash); p.Close(); HashStringList const hsl = patch_hash.GetHashStringList(); if (hsl != I->ExpectedHashes) @@ -624,7 +639,6 @@ class RredMethod : public pkgAcqMethod { FILE *out = fopen(Itm->DestFile.c_str(), "w"); Hashes hash(Itm->ExpectedHashes); - patch.apply_against_file(out, inp, &hash); fclose(out); @@ -657,6 +671,16 @@ class RredMethod : public pkgAcqMethod { return true; } + bool Configuration(std::string Message) + { + if (pkgAcqMethod::Configuration(Message) == false) + return false; + + DropPrivsOrDie(); + + return true; + } + public: RredMethod() : pkgAcqMethod("2.0",SingleInstance | SendConfig), Debug(false) {} }; @@ -685,7 +709,11 @@ int main(int argc, char **argv) _error->DumpErrors(std::cerr); exit(1); } - patch.read_diff(p, NULL); + if (patch.read_diff(p, NULL) == false) + { + _error->DumpErrors(std::cerr); + exit(2); + } } if (just_diff) { -- cgit v1.2.3 From 4f51fd8636592a96aecf17c8bf4cfdb3ea2207cc Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 8 Jun 2015 00:06:41 +0200 Subject: support hashes for compressed pdiff files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the moment we only have hashes for the uncompressed pdiff files, but via the new '$HASH-Download' field in the .diff/Index hashes can be provided for the .gz compressed pdiff file, which apt will pick up now and use to verify the download. Now, we "just" need a buy in from the creators of repositories… --- methods/rred.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'methods') diff --git a/methods/rred.cc b/methods/rred.cc index 81ecf8553..12cf2b4a5 100644 --- a/methods/rred.cc +++ b/methods/rred.cc @@ -627,7 +627,7 @@ class RredMethod : public pkgAcqMethod { p.Close(); HashStringList const hsl = patch_hash.GetHashStringList(); if (hsl != I->ExpectedHashes) - return _error->Error("Patch %s doesn't have the expected hashsum", patch_name.c_str()); + return _error->Error("Hash Sum mismatch for uncompressed patch %s", patch_name.c_str()); } if (Debug == true) -- cgit v1.2.3 From c69e8370947d765dd94f142d18dc11d5a76af443 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 9 Jun 2015 15:15:33 +0200 Subject: replace ULONG_MAX with c++ style std::numeric_limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For some reason travis seems to be unhappy about it claiming it is not defined. Well, lets not think to deeply about it… Git-Dch: Ignore --- methods/rred.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'methods') diff --git a/methods/rred.cc b/methods/rred.cc index 12cf2b4a5..54123ab9c 100644 --- a/methods/rred.cc +++ b/methods/rred.cc @@ -405,12 +405,12 @@ class Patch { size_t s, e; errno = 0; s = strtoul(buffer, &m, 10); - if (unlikely(m == buffer || s == ULONG_MAX || errno != 0)) + if (unlikely(m == buffer || s == std::numeric_limits::max() || errno != 0)) return _error->Error("Parsing patchfile %s failed: Expected an effected line start", f.Name().c_str()); else if (*m == ',') { ++m; e = strtol(m, &c, 10); - if (unlikely(m == c || e == ULONG_MAX || errno != 0)) + if (unlikely(m == c || e == std::numeric_limits::max() || errno != 0)) return _error->Error("Parsing patchfile %s failed: Expected an effected line end", f.Name().c_str()); if (unlikely(e < s)) return _error->Error("Parsing patchfile %s failed: Effected lines end %lu is before start %lu", f.Name().c_str(), e, s); -- cgit v1.2.3 From 9f697f69cf1adaced476598cfe08ab03c76c5d18 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 15 Jun 2015 12:51:22 +0200 Subject: ensure valid or remove destination file in file method 'file' isn't using the destination file per-se, but returns another name via "Filename" header. It still should deal with destination files as they could exist (pkgAcqFile e.g. creates links in that location) and are potentially bogus. --- methods/file.cc | 44 +++++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 11 deletions(-) (limited to 'methods') diff --git a/methods/file.cc b/methods/file.cc index 353e54bd5..5d5fffa67 100644 --- a/methods/file.cc +++ b/methods/file.cc @@ -48,8 +48,24 @@ bool FileMethod::Fetch(FetchItem *Itm) if (Get.Host.empty() == false) return _error->Error(_("Invalid URI, local URIS must not start with //")); - // See if the file exists struct stat Buf; + // deal with destination files which might linger around + if (lstat(Itm->DestFile.c_str(), &Buf) == 0) + { + if ((Buf.st_mode & S_IFREG) != 0) + { + if (Itm->LastModified == Buf.st_mtime && Itm->LastModified != 0) + { + HashStringList const hsl = Itm->ExpectedHashes; + if (Itm->ExpectedHashes.VerifyFile(File)) + Res.IMSHit = true; + } + } + } + if (Res.IMSHit != true) + unlink(Itm->DestFile.c_str()); + + // See if the file exists if (stat(File.c_str(),&Buf) == 0) { Res.Size = Buf.st_size; @@ -65,6 +81,8 @@ bool FileMethod::Fetch(FetchItem *Itm) } // See if the uncompressed file exists and reuse it + FetchResult AltRes; + AltRes.Filename.clear(); std::vector extensions = APT::Configuration::getCompressorExtensions(); for (std::vector::const_iterator ext = extensions.begin(); ext != extensions.end(); ++ext) { @@ -73,29 +91,33 @@ bool FileMethod::Fetch(FetchItem *Itm) std::string const unfile = File.substr(0, File.length() - ext->length() - 1); if (stat(unfile.c_str(),&Buf) == 0) { - FetchResult AltRes; AltRes.Size = Buf.st_size; AltRes.Filename = unfile; AltRes.LastModified = Buf.st_mtime; AltRes.IMSHit = false; if (Itm->LastModified == Buf.st_mtime && Itm->LastModified != 0) AltRes.IMSHit = true; - - URIDone(Res,&AltRes); - return true; + break; } // no break here as we could have situations similar to '.gz' vs '.tar.gz' here } } - if (Res.Filename.empty() == true) + if (Res.Filename.empty() == false) + { + Hashes Hash(Itm->ExpectedHashes); + FileFd Fd(Res.Filename, FileFd::ReadOnly); + Hash.AddFD(Fd); + Res.TakeHashes(Hash); + } + + if (AltRes.Filename.empty() == false) + URIDone(Res,&AltRes); + else if (Res.Filename.empty() == false) + URIDone(Res); + else return _error->Error(_("File not found")); - Hashes Hash(Itm->ExpectedHashes); - FileFd Fd(Res.Filename, FileFd::ReadOnly); - Hash.AddFD(Fd); - Res.TakeHashes(Hash); - URIDone(Res); return true; } /*}}}*/ -- cgit v1.2.3 From ff86d7df6a53ff6283de4b9a858c1dad98ed887f Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 15 Jun 2015 13:36:11 +0200 Subject: call URIStart in cdrom and file method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All other methods call it, so they should follow along even if the work they do afterwards is hardly breathtaking and usually results in a URIDone pretty soon, but the acquire system tells the individual item about this via a virtual method call, so even through none of our existing items contains any critical code in these, maybe one day they might. Consistency at least once… Which is also why this has a good sideeffect: file: and cdrom: requests appear now in the 'apt-get update' output. Finally - it never made sense to hide them for me. Okay, I guess it made before the new hit behavior, but now that you can actually see the difference in an update it makes sense to see if a file: repository changed or not as well. --- methods/cdrom.cc | 3 ++- methods/file.cc | 18 ++++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) (limited to 'methods') diff --git a/methods/cdrom.cc b/methods/cdrom.cc index 10cb29f66..67265cfa3 100644 --- a/methods/cdrom.cc +++ b/methods/cdrom.cc @@ -260,7 +260,8 @@ bool CDROMMethod::Fetch(FetchItem *Itm) struct stat Buf; if (stat(Res.Filename.c_str(),&Buf) != 0) return _error->Error(_("File not found")); - + + URIStart(Res); if (NewID.empty() == false) CurrentID = NewID; Res.LastModified = Buf.st_mtime; diff --git a/methods/file.cc b/methods/file.cc index 5d5fffa67..5c76ec122 100644 --- a/methods/file.cc +++ b/methods/file.cc @@ -58,7 +58,10 @@ bool FileMethod::Fetch(FetchItem *Itm) { HashStringList const hsl = Itm->ExpectedHashes; if (Itm->ExpectedHashes.VerifyFile(File)) + { + Res.Filename = Itm->DestFile; Res.IMSHit = true; + } } } } @@ -78,7 +81,14 @@ bool FileMethod::Fetch(FetchItem *Itm) if (filesize != 0 && filesize == Res.Size) Res.IMSHit = true; } + + Hashes Hash(Itm->ExpectedHashes); + FileFd Fd(File, FileFd::ReadOnly); + Hash.AddFD(Fd); + Res.TakeHashes(Hash); } + if (Res.IMSHit == false) + URIStart(Res); // See if the uncompressed file exists and reuse it FetchResult AltRes; @@ -103,14 +113,6 @@ bool FileMethod::Fetch(FetchItem *Itm) } } - if (Res.Filename.empty() == false) - { - Hashes Hash(Itm->ExpectedHashes); - FileFd Fd(Res.Filename, FileFd::ReadOnly); - Hash.AddFD(Fd); - Res.TakeHashes(Hash); - } - if (AltRes.Filename.empty() == false) URIDone(Res,&AltRes); else if (Res.Filename.empty() == false) -- cgit v1.2.3 From b0d408547734100bf86781615f546487ecf390d9 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 24 Jun 2015 19:31:22 +0200 Subject: implement Signed-By option for sources.list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Limits which key(s) can be used to sign a repository. Not immensely useful from a security perspective all by itself, but if the user has additional measures in place to confine a repository (like pinning) an attacker who gets the key for such a repository is limited to its potential and can't use the key to sign its attacks for an other (maybe less limited) repository… (yes, this is as weak as it sounds, but having the capability might come in handy for implementing other stuff later). --- methods/gpgv.cc | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'methods') diff --git a/methods/gpgv.cc b/methods/gpgv.cc index 41f138be6..014430041 100644 --- a/methods/gpgv.cc +++ b/methods/gpgv.cc @@ -37,13 +37,14 @@ class GPGVMethod : public pkgAcqMethod { private: string VerifyGetSigners(const char *file, const char *outfile, - vector &GoodSigners, + std::string const &key, + vector &GoodSigners, vector &BadSigners, vector &WorthlessSigners, vector &NoPubKeySigners); protected: - virtual bool Fetch(FetchItem *Itm); + virtual bool URIAcquire(std::string const &Message, FetchItem *Itm); virtual bool Configuration(string Message); public: @@ -61,6 +62,7 @@ bool GPGVMethod::Configuration(string Message) } string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, + std::string const &key, vector &GoodSigners, vector &BadSigners, vector &WorthlessSigners, @@ -80,7 +82,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, if (pid < 0) return string("Couldn't spawn new process") + strerror(errno); else if (pid == 0) - ExecGPGV(outfile, file, 3, fd); + ExecGPGV(outfile, file, 3, fd, key); close(fd[1]); FILE *pipein = fdopen(fd[0], "r"); @@ -174,11 +176,11 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, return _("Unknown error executing apt-key"); } -bool GPGVMethod::Fetch(FetchItem *Itm) +bool GPGVMethod::URIAcquire(std::string const &Message, FetchItem *Itm) { - URI Get = Itm->Uri; - string Path = Get.Host + Get.Path; // To account for relative paths - string keyID; + URI const Get = Itm->Uri; + string const Path = Get.Host + Get.Path; // To account for relative paths + std::string const key = LookupTag(Message, "Signed-By"); vector GoodSigners; vector BadSigners; // a worthless signature is a expired or revoked one @@ -190,7 +192,7 @@ bool GPGVMethod::Fetch(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(), + string msg = VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(), key, GoodSigners, BadSigners, WorthlessSigners, NoPubKeySigners); if (GoodSigners.empty() || !BadSigners.empty() || !NoPubKeySigners.empty()) -- cgit v1.2.3 From 653ef26c70dc9c0e2cbfdd4e79117876bb63e87d Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 30 Jun 2015 10:53:51 +0200 Subject: allow individual targets to be kept compressed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is an option to keep all targets (Packages, Sources, …) compressed for a while now, but the all-or-nothing approach is a bit limited for our purposes with additional targets as some of them are very big (Contents) and rarely used in comparison, so keeping them compressed by default can make sense, while others are still unpacked. Most interesting is the copy-change maybe: Copy is used by the acquire system as an uncompressor and it is hence expected that it returns the hashes for the "output", not the input. Now, in the case of keeping a file compressed, the output is never written to disk, but generated in memory and we should still validated it, so for compressed files copy is expected to return the hashes of the uncompressed file. We used to use the config option to enable on-the-fly decompress in the method, but in reality copy is never used in a way where it shouldn't decompress a compressed file to get its hashes, so we can save us the trouble of sending this information to the method and just do it always. --- methods/copy.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'methods') diff --git a/methods/copy.cc b/methods/copy.cc index a8e289df5..4bdc15f75 100644 --- a/methods/copy.cc +++ b/methods/copy.cc @@ -38,11 +38,7 @@ class CopyMethod : public pkgAcqMethod void CopyMethod::CalculateHashes(FetchItem const * const Itm, FetchResult &Res) { Hashes Hash(Itm->ExpectedHashes); - FileFd::CompressMode CompressMode = FileFd::None; - if (_config->FindB("Acquire::GzipIndexes", false) == true) - CompressMode = FileFd::Extension; - - FileFd Fd(Res.Filename, FileFd::ReadOnly, CompressMode); + FileFd Fd(Res.Filename, FileFd::ReadOnly, FileFd::Extension); Hash.AddFD(Fd); Res.TakeHashes(Hash); } @@ -53,7 +49,7 @@ void CopyMethod::CalculateHashes(FetchItem const * const Itm, FetchResult &Res) bool CopyMethod::Fetch(FetchItem *Itm) { // this ensures that relative paths work in copy - std::string File = Itm->Uri.substr(Itm->Uri.find(':')+1); + std::string const File = Itm->Uri.substr(Itm->Uri.find(':')+1); // Stat the file and send a start message struct stat Buf; -- cgit v1.2.3 From 4e03c47de15164f2656d9655edab6fb3570cb2f2 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 7 Jul 2015 22:11:20 +0200 Subject: implement Signed-By without using gpg for verification The previous commit returns to the possibility of using just gpgv for verification proposes. There is one problem through: We can't enforce a specific keyid without using gpg, but our acquire method can as it parses gpgv output anyway, so it can deal with good signatures from not expected signatures and treats them as unknown keys instead. Git-Dch: Ignore --- methods/gpgv.cc | 69 +++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 9 deletions(-) (limited to 'methods') diff --git a/methods/gpgv.cc b/methods/gpgv.cc index 014430041..d88a06789 100644 --- a/methods/gpgv.cc +++ b/methods/gpgv.cc @@ -15,6 +15,8 @@ #include #include #include + +#include #include #include #include @@ -74,6 +76,7 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, std::clog << "inside VerifyGetSigners" << std::endl; int fd[2]; + bool const keyIsID = (key.empty() == false && key[0] != '/'); if (pipe(fd) < 0) return "Couldn't create pipe"; @@ -82,12 +85,13 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, if (pid < 0) return string("Couldn't spawn new process") + strerror(errno); else if (pid == 0) - ExecGPGV(outfile, file, 3, fd, key); + ExecGPGV(outfile, file, 3, fd, (keyIsID ? "" : key)); close(fd[1]); FILE *pipein = fdopen(fd[0], "r"); // Loop over the output of apt-key (which really is gnupg), and check the signatures. + std::vector ValidSigners; size_t buffersize = 0; char *buffer = NULL; while (1) @@ -107,32 +111,31 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, std::clog << "Got BADSIG! " << std::endl; BadSigners.push_back(string(buffer+sizeof(GNUPGPREFIX))); } - - if (strncmp(buffer, GNUPGNOPUBKEY, sizeof(GNUPGNOPUBKEY)-1) == 0) + else if (strncmp(buffer, GNUPGNOPUBKEY, sizeof(GNUPGNOPUBKEY)-1) == 0) { if (Debug == true) std::clog << "Got NO_PUBKEY " << std::endl; NoPubKeySigners.push_back(string(buffer+sizeof(GNUPGPREFIX))); } - if (strncmp(buffer, GNUPGNODATA, sizeof(GNUPGBADSIG)-1) == 0) + else if (strncmp(buffer, GNUPGNODATA, sizeof(GNUPGBADSIG)-1) == 0) { if (Debug == true) std::clog << "Got NODATA! " << std::endl; BadSigners.push_back(string(buffer+sizeof(GNUPGPREFIX))); } - if (strncmp(buffer, GNUPGKEYEXPIRED, sizeof(GNUPGKEYEXPIRED)-1) == 0) + else if (strncmp(buffer, GNUPGKEYEXPIRED, sizeof(GNUPGKEYEXPIRED)-1) == 0) { if (Debug == true) std::clog << "Got KEYEXPIRED! " << std::endl; WorthlessSigners.push_back(string(buffer+sizeof(GNUPGPREFIX))); } - if (strncmp(buffer, GNUPGREVKEYSIG, sizeof(GNUPGREVKEYSIG)-1) == 0) + else if (strncmp(buffer, GNUPGREVKEYSIG, sizeof(GNUPGREVKEYSIG)-1) == 0) { if (Debug == true) std::clog << "Got REVKEYSIG! " << std::endl; WorthlessSigners.push_back(string(buffer+sizeof(GNUPGPREFIX))); } - if (strncmp(buffer, GNUPGGOODSIG, sizeof(GNUPGGOODSIG)-1) == 0) + else if (strncmp(buffer, GNUPGGOODSIG, sizeof(GNUPGGOODSIG)-1) == 0) { char *sig = buffer + sizeof(GNUPGPREFIX); char *p = sig + sizeof("GOODSIG"); @@ -143,10 +146,48 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, std::clog << "Got GOODSIG, key ID:" << sig << std::endl; GoodSigners.push_back(string(sig)); } + else if (strncmp(buffer, GNUPGVALIDSIG, sizeof(GNUPGVALIDSIG)-1) == 0) + { + char *sig = buffer + sizeof(GNUPGVALIDSIG); + char *p = sig; + while (*p && isxdigit(*p)) + p++; + *p = 0; + if (Debug == true) + std::clog << "Got VALIDSIG, key ID: " << sig << std::endl; + ValidSigners.push_back(string(sig)); + } } fclose(pipein); free(buffer); + // 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 + if (keyIsID == true) + { + if (Debug == true) + std::clog << "GoodSigs needs to be limited to keyid " << key << std::endl; + std::vector::iterator const foundItr = std::find(ValidSigners.begin(), ValidSigners.end(), key); + bool const found = (foundItr != ValidSigners.end()); + std::copy(GoodSigners.begin(), GoodSigners.end(), std::back_insert_iterator >(NoPubKeySigners)); + if (found) + { + // we look for GOODSIG here as well as an expired sig is a valid sig as well (but not a good one) + std::string const goodlongkeyid = "GOODSIG " + key.substr(24, 16); + bool const foundGood = std::find(GoodSigners.begin(), GoodSigners.end(), goodlongkeyid) != GoodSigners.end(); + if (Debug == true) + std::clog << "Key " << key << " is valid sig, is " << goodlongkeyid << " also a good one? " << (foundGood ? "yes" : "no") << std::endl; + GoodSigners.clear(); + if (foundGood) + { + GoodSigners.push_back(goodlongkeyid); + NoPubKeySigners.erase(std::remove(NoPubKeySigners.begin(), NoPubKeySigners.end(), goodlongkeyid), NoPubKeySigners.end()); + } + } + else + GoodSigners.clear(); + } + int status; waitpid(pid, &status, 0); if (Debug == true) @@ -156,8 +197,18 @@ string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile, if (WEXITSTATUS(status) == 0) { - if (GoodSigners.empty()) - return _("Internal error: Good signature, but could not determine key fingerprint?!"); + if (keyIsID) + { + // 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()) + return _("At least one invalid signature was encountered."); + } + else + { + if (GoodSigners.empty()) + return _("Internal error: Good signature, but could not determine key fingerprint?!"); + } return ""; } else if (WEXITSTATUS(status) == 1) -- cgit v1.2.3 From 3b3028467ceccca0b73a8f53051c0fa4de313111 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Thu, 9 Jul 2015 00:35:40 +0200 Subject: add c++11 override marker to overridden methods C++11 adds the 'override' specifier to mark that a method is overriding a base class method and error out if not. We hide it in the APT_OVERRIDE macro to ensure that we keep compiling in pre-c++11 standards. Reported-By: clang-modernize -add-override -override-macros Git-Dch: Ignore --- methods/cdrom.cc | 4 ++-- methods/copy.cc | 2 +- methods/file.cc | 2 +- methods/ftp.cc | 1 - methods/ftp.h | 6 ++++-- methods/gpgv.cc | 4 ++-- methods/gzip.cc | 4 ++-- methods/http.h | 34 +++++++++++++++++----------------- methods/https.h | 34 +++++++++++++++++----------------- methods/mirror.h | 10 +++++----- methods/rred.cc | 4 ++-- methods/rsh.h | 4 ++-- methods/server.h | 4 ++-- 13 files changed, 57 insertions(+), 56 deletions(-) (limited to 'methods') diff --git a/methods/cdrom.cc b/methods/cdrom.cc index 67265cfa3..d9ddecb6a 100644 --- a/methods/cdrom.cc +++ b/methods/cdrom.cc @@ -42,9 +42,9 @@ class CDROMMethod : public pkgAcqMethod bool IsCorrectCD(URI want, string MountPath, string& NewID); bool AutoDetectAndMount(const URI, string &NewID); - virtual bool Fetch(FetchItem *Itm); + virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE; string GetID(string Name); - virtual void Exit(); + virtual void Exit() APT_OVERRIDE; public: diff --git a/methods/copy.cc b/methods/copy.cc index 4bdc15f75..0c9f322e6 100644 --- a/methods/copy.cc +++ b/methods/copy.cc @@ -27,7 +27,7 @@ class CopyMethod : public pkgAcqMethod { - virtual bool Fetch(FetchItem *Itm); + virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE; void CalculateHashes(FetchItem const * const Itm, FetchResult &Res); public: diff --git a/methods/file.cc b/methods/file.cc index 5c76ec122..40e85bce5 100644 --- a/methods/file.cc +++ b/methods/file.cc @@ -30,7 +30,7 @@ class FileMethod : public pkgAcqMethod { - virtual bool Fetch(FetchItem *Itm); + virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE; public: diff --git a/methods/ftp.cc b/methods/ftp.cc index 92d8573f1..5fee20f23 100644 --- a/methods/ftp.cc +++ b/methods/ftp.cc @@ -39,7 +39,6 @@ // Internet stuff #include -#include #include #include diff --git a/methods/ftp.h b/methods/ftp.h index 2efd28ec6..2c4e9f57a 100644 --- a/methods/ftp.h +++ b/methods/ftp.h @@ -10,8 +10,10 @@ #ifndef APT_FTP_H #define APT_FTP_H +#include #include +#include #include #include #include @@ -71,8 +73,8 @@ class FTPConn class FtpMethod : public pkgAcqMethod { - virtual bool Fetch(FetchItem *Itm); - virtual bool Configuration(std::string Message); + virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE; + virtual bool Configuration(std::string Message) APT_OVERRIDE; FTPConn *Server; diff --git a/methods/gpgv.cc b/methods/gpgv.cc index d88a06789..490833d8c 100644 --- a/methods/gpgv.cc +++ b/methods/gpgv.cc @@ -46,8 +46,8 @@ class GPGVMethod : public pkgAcqMethod vector &NoPubKeySigners); protected: - virtual bool URIAcquire(std::string const &Message, FetchItem *Itm); - virtual bool Configuration(string Message); + virtual bool URIAcquire(std::string const &Message, FetchItem *Itm) APT_OVERRIDE; + virtual bool Configuration(string Message) APT_OVERRIDE; public: GPGVMethod() : pkgAcqMethod("1.0",SingleInstance | SendConfig) {}; diff --git a/methods/gzip.cc b/methods/gzip.cc index 65519633c..637aae124 100644 --- a/methods/gzip.cc +++ b/methods/gzip.cc @@ -32,8 +32,8 @@ const char *Prog; class GzipMethod : public pkgAcqMethod { - virtual bool Fetch(FetchItem *Itm); - virtual bool Configuration(std::string Message); + virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE; + virtual bool Configuration(std::string Message) APT_OVERRIDE; public: diff --git a/methods/http.h b/methods/http.h index e73871931..da6139b02 100644 --- a/methods/http.h +++ b/methods/http.h @@ -99,23 +99,23 @@ struct HttpServerState: public ServerState int ServerFd; protected: - virtual bool ReadHeaderLines(std::string &Data); - virtual bool LoadNextResponse(bool const ToFile, FileFd * const File); - virtual bool WriteResponse(std::string const &Data); + virtual bool ReadHeaderLines(std::string &Data) APT_OVERRIDE; + virtual bool LoadNextResponse(bool const ToFile, FileFd * const File) APT_OVERRIDE; + virtual bool WriteResponse(std::string const &Data) APT_OVERRIDE; public: - virtual void Reset() { ServerState::Reset(); ServerFd = -1; }; + virtual void Reset() APT_OVERRIDE { ServerState::Reset(); ServerFd = -1; }; - virtual bool RunData(FileFd * const File); + virtual bool RunData(FileFd * const File) APT_OVERRIDE; - virtual bool Open(); - virtual bool IsOpen(); - virtual bool Close(); - virtual bool InitHashes(HashStringList const &ExpectedHashes); - virtual Hashes * GetHashes(); - virtual bool Die(FileFd &File); - virtual bool Flush(FileFd * const File); - virtual bool Go(bool ToFile, FileFd * const File); + virtual bool Open() APT_OVERRIDE; + virtual bool IsOpen() APT_OVERRIDE; + virtual bool Close() APT_OVERRIDE; + virtual bool InitHashes(HashStringList const &ExpectedHashes) APT_OVERRIDE; + virtual Hashes * GetHashes() APT_OVERRIDE; + virtual bool Die(FileFd &File) APT_OVERRIDE; + virtual bool Flush(FileFd * const File) APT_OVERRIDE; + virtual bool Go(bool ToFile, FileFd * const File) APT_OVERRIDE; HttpServerState(URI Srv, HttpMethod *Owner); virtual ~HttpServerState() {Close();}; @@ -124,12 +124,12 @@ struct HttpServerState: public ServerState class HttpMethod : public ServerMethod { public: - virtual void SendReq(FetchItem *Itm); + virtual void SendReq(FetchItem *Itm) APT_OVERRIDE; - virtual bool Configuration(std::string Message); + virtual bool Configuration(std::string Message) APT_OVERRIDE; - virtual ServerState * CreateServerState(URI uri); - virtual void RotateDNS(); + virtual ServerState * CreateServerState(URI uri) APT_OVERRIDE; + virtual void RotateDNS() APT_OVERRIDE; protected: std::string AutoDetectProxyCmd; diff --git a/methods/https.h b/methods/https.h index 57fc292ee..29b20b921 100644 --- a/methods/https.h +++ b/methods/https.h @@ -32,23 +32,23 @@ class HttpsServerState : public ServerState Hashes * Hash; protected: - virtual bool ReadHeaderLines(std::string &/*Data*/) { return false; } - virtual bool LoadNextResponse(bool const /*ToFile*/, FileFd * const /*File*/) { return false; } + virtual bool ReadHeaderLines(std::string &/*Data*/) APT_OVERRIDE { return false; } + virtual bool LoadNextResponse(bool const /*ToFile*/, FileFd * const /*File*/) APT_OVERRIDE { return false; } public: - virtual bool WriteResponse(std::string const &/*Data*/) { return false; } + virtual bool WriteResponse(std::string const &/*Data*/) APT_OVERRIDE { return false; } /** \brief Transfer the data from the socket */ - virtual bool RunData(FileFd * const /*File*/) { return false; } + virtual bool RunData(FileFd * const /*File*/) APT_OVERRIDE { return false; } - virtual bool Open() { return false; } - virtual bool IsOpen() { return false; } - virtual bool Close() { return false; } - virtual bool InitHashes(HashStringList const &ExpectedHashes); - virtual Hashes * GetHashes(); - virtual bool Die(FileFd &/*File*/) { return false; } - virtual bool Flush(FileFd * const /*File*/) { return false; } - virtual bool Go(bool /*ToFile*/, FileFd * const /*File*/) { return false; } + virtual bool Open() APT_OVERRIDE { return false; } + virtual bool IsOpen() APT_OVERRIDE { return false; } + virtual bool Close() APT_OVERRIDE { return false; } + virtual bool InitHashes(HashStringList const &ExpectedHashes) APT_OVERRIDE; + virtual Hashes * GetHashes() APT_OVERRIDE; + virtual bool Die(FileFd &/*File*/) APT_OVERRIDE { return false; } + virtual bool Flush(FileFd * const /*File*/) APT_OVERRIDE { return false; } + virtual bool Go(bool /*ToFile*/, FileFd * const /*File*/) APT_OVERRIDE { return false; } HttpsServerState(URI Srv, HttpsMethod *Owner); virtual ~HttpsServerState() {Close();}; @@ -59,7 +59,7 @@ class HttpsMethod : public ServerMethod // minimum speed in bytes/se that triggers download timeout handling static const int DL_MIN_SPEED = 10; - virtual bool Fetch(FetchItem *); + virtual bool Fetch(FetchItem *) APT_OVERRIDE; static size_t parse_header(void *buffer, size_t size, size_t nmemb, void *userp); static size_t write_data(void *buffer, size_t size, size_t nmemb, void *userp); @@ -70,14 +70,14 @@ class HttpsMethod : public ServerMethod ServerState *Server; // Used by ServerMethods unused by https - virtual void SendReq(FetchItem *) { exit(42); } - virtual void RotateDNS() { exit(42); } + virtual void SendReq(FetchItem *) APT_OVERRIDE { exit(42); } + virtual void RotateDNS() APT_OVERRIDE { exit(42); } public: FileFd *File; - virtual bool Configuration(std::string Message); - virtual ServerState * CreateServerState(URI uri); + virtual bool Configuration(std::string Message) APT_OVERRIDE; + virtual ServerState * CreateServerState(URI uri) APT_OVERRIDE; using pkgAcqMethod::FetchResult; using pkgAcqMethod::FetchItem; diff --git a/methods/mirror.h b/methods/mirror.h index 6c0ce370e..425bea673 100644 --- a/methods/mirror.h +++ b/methods/mirror.h @@ -46,14 +46,14 @@ class MirrorMethod : public HttpMethod bool Clean(std::string dir); // we need to overwrite those to transform the url back - virtual void Fail(std::string Why, bool Transient = false); - virtual void URIStart(FetchResult &Res); - virtual void URIDone(FetchResult &Res,FetchResult *Alt = 0); - virtual bool Configuration(std::string Message); + virtual void Fail(std::string Why, bool Transient = false) APT_OVERRIDE; + virtual void URIStart(FetchResult &Res) APT_OVERRIDE; + virtual void URIDone(FetchResult &Res,FetchResult *Alt = 0) APT_OVERRIDE; + virtual bool Configuration(std::string Message) APT_OVERRIDE; public: MirrorMethod(); - virtual bool Fetch(FetchItem *Itm); + virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE; }; diff --git a/methods/rred.cc b/methods/rred.cc index 54123ab9c..91b6dda22 100644 --- a/methods/rred.cc +++ b/methods/rred.cc @@ -558,7 +558,7 @@ class RredMethod : public pkgAcqMethod { } protected: - virtual bool URIAcquire(std::string const &Message, FetchItem *Itm) { + virtual bool URIAcquire(std::string const &Message, FetchItem *Itm) APT_OVERRIDE { Debug = _config->FindB("Debug::pkgAcquire::RRed", false); URI Get = Itm->Uri; std::string Path = Get.Host + Get.Path; // rred:/path - no host @@ -671,7 +671,7 @@ class RredMethod : public pkgAcqMethod { return true; } - bool Configuration(std::string Message) + bool Configuration(std::string Message) APT_OVERRIDE { if (pkgAcqMethod::Configuration(Message) == false) return false; diff --git a/methods/rsh.h b/methods/rsh.h index dd259e744..34492971c 100644 --- a/methods/rsh.h +++ b/methods/rsh.h @@ -56,8 +56,8 @@ class RSHConn class RSHMethod : public pkgAcqMethod { - virtual bool Fetch(FetchItem *Itm); - virtual bool Configuration(std::string Message); + virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE; + virtual bool Configuration(std::string Message) APT_OVERRIDE; RSHConn *Server; diff --git a/methods/server.h b/methods/server.h index 8d7d33ee6..f9f6e9071 100644 --- a/methods/server.h +++ b/methods/server.h @@ -106,7 +106,7 @@ struct ServerState class ServerMethod : public pkgAcqMethod { protected: - virtual bool Fetch(FetchItem *); + virtual bool Fetch(FetchItem *) APT_OVERRIDE; ServerState *Server; std::string NextURI; @@ -146,7 +146,7 @@ class ServerMethod : public pkgAcqMethod static time_t FailTime; static APT_NORETURN void SigTerm(int); - virtual bool Configuration(std::string Message); + virtual bool Configuration(std::string Message) APT_OVERRIDE; virtual bool Flush() { return Server->Flush(File); }; int Loop(); -- cgit v1.2.3 From c7872a2c857bc54febb09721f5256786a15c002d Mon Sep 17 00:00:00 2001 From: Daniel Hartwig Date: Tue, 11 Aug 2015 17:59:13 +0200 Subject: support setting a port for rsh:// in sources.list [Commiter comment: Untested, but looks and compiles fine, so what could possibly go wrong] Closes: 624727 --- methods/rsh.cc | 22 ++++++++++++++++++++-- methods/rsh.h | 1 + 2 files changed, 21 insertions(+), 2 deletions(-) (limited to 'methods') diff --git a/methods/rsh.cc b/methods/rsh.cc index 52349c61c..7ef2f7c7a 100644 --- a/methods/rsh.cc +++ b/methods/rsh.cc @@ -84,7 +84,7 @@ bool RSHConn::Open() if (Process != -1) return true; - if (Connect(ServerName.Host,ServerName.User) == false) + if (Connect(ServerName.Host,ServerName.Port,ServerName.User) == false) return false; return true; @@ -93,8 +93,15 @@ bool RSHConn::Open() // RSHConn::Connect - Fire up rsh and connect /*{{{*/ // --------------------------------------------------------------------- /* */ -bool RSHConn::Connect(std::string Host, std::string User) +bool RSHConn::Connect(std::string Host, unsigned int Port, std::string User) { + char *PortStr = NULL; + if (Port != 0) + { + if (asprintf (&PortStr, "%d", Port) == -1 || PortStr == NULL) + return _error->Errno("asprintf", _("Failed")); + } + // Create the pipes int Pipes[4] = {-1,-1,-1,-1}; if (pipe(Pipes) != 0 || pipe(Pipes+2) != 0) @@ -140,6 +147,10 @@ bool RSHConn::Connect(std::string Host, std::string User) Args[i++] = "-l"; Args[i++] = User.c_str(); } + if (PortStr != NULL) { + Args[i++] = "-p"; + Args[i++] = PortStr; + } if (Host.empty() == false) { Args[i++] = Host.c_str(); } @@ -149,6 +160,9 @@ bool RSHConn::Connect(std::string Host, std::string User) exit(100); } + if (PortStr != NULL) + free(PortStr); + ReadFd = Pipes[0]; WriteFd = Pipes[3]; SetNonBlock(Pipes[0],true); @@ -157,6 +171,10 @@ bool RSHConn::Connect(std::string Host, std::string User) close(Pipes[2]); return true; +} +bool RSHConn::Connect(std::string Host, std::string User) +{ + return Connect(Host, 0, User); } /*}}}*/ // RSHConn::ReadLine - Very simple buffered read with timeout /*{{{*/ diff --git a/methods/rsh.h b/methods/rsh.h index 34492971c..e6839711b 100644 --- a/methods/rsh.h +++ b/methods/rsh.h @@ -36,6 +36,7 @@ class RSHConn // Raw connection IO bool WriteMsg(std::string &Text,bool Sync,const char *Fmt,...); bool Connect(std::string Host, std::string User); + bool Connect(std::string Host, unsigned int Port, std::string User); bool Comp(URI Other) const {return Other.Host == ServerName.Host && Other.Port == ServerName.Port;}; // Connection control -- cgit v1.2.3 From f8043f219f077b1cfc6c5ad2263c4caa4709a00d Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 14 Aug 2015 19:44:25 +0200 Subject: Make apt compile with clang++ again This allows us to run the clang static analyzer and to run the testsuite with the clang MemorySanitizer. --- methods/ftp.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'methods') diff --git a/methods/ftp.cc b/methods/ftp.cc index 5fee20f23..1a9a1c4eb 100644 --- a/methods/ftp.cc +++ b/methods/ftp.cc @@ -745,7 +745,7 @@ bool FTPConn::CreateDataFd() } // Bind and listen - if (bind(DataListenFd,BindAddr->ai_addr,BindAddr->ai_addrlen) < 0) + if (::bind(DataListenFd,BindAddr->ai_addr,BindAddr->ai_addrlen) < 0) { freeaddrinfo(BindAddr); return _error->Errno("bind",_("Could not bind a socket")); -- cgit v1.2.3