summaryrefslogtreecommitdiff
path: root/apt-pkg/contrib/gpgv.cc
Commit message (Collapse)AuthorAgeFilesLines
* Add explicit message for unsupported binary signatureDavid Kalnischkies2019-03-031-2/+16
| | | | | | | | | | | | | | | | | | | | | | | Verifying the content of Release.gpg made us fail on binary signatures which were never officially supported (apt-secure manpage only documents only the generation of ASCII armored), but silently accepted by gpgv as we passed it on unchecked before. The binary format is complex and is itself split into old and new formats so adding support for this would not only add lots of code but also a good opportunity for bugs and dubious benefit. Reporting this issue explicitly should help repository creators figure out the problem faster than the default NODATA message hinting at captive portals. Given that the binary format has no file magic or any other clear and simple indication that this is a detached signature we guess based on the first two bits only – and by that only supporting the "old" binary format which seems to be the only one generated by gnupg in this case. References: e2965b0b6bdd68ffcad0e06d11755412a7e16e50 Closes: #921685
* gpgv: Use buffered writes for splitting clearsigned filesJulian Andres Klode2019-02-041-1/+2
| | | | | | This is safe here, as the code ensures that the file is flushed before it is being used. The next series should probably make GetTempFile() buffer writes by default.
* Avoid boolean flags by splitting writeTo functionsDavid Kalnischkies2019-02-011-14/+20
| | | | | Suggested-By: Julian Andres Klode Gbp-Dch: Ignore
* Reuse APT::StringView more in LineBufferDavid Kalnischkies2019-01-291-32/+9
| | | | | | | No effective change in behaviour, just simplifying and reusing code. Suggested-By: Julian Andres Klode Gbp-Dch: Ignore
* Rework ifs to use not instead of == false/trueDavid Kalnischkies2019-01-291-54/+66
| | | | | | | | No change in the logic itself, just dropping "== true", replacing "== false" with not and moving lines around to make branches more obvious. Suggested-By: Julian Andres Klode Gbp-Dch: Ignore
* Refuse files with lines unexpectedly starting with a dashDavid Kalnischkies2019-01-281-9/+37
| | | | | | | | | | | | | | | | We support dash-encoding even if we don't really work with files who would need it as implementations are free to encode every line, but otherwise a line starting with a dash must either be a header we parse explicitly or the file is refused. This is against the RFC which says clients should warn on such files, but given that we aren't expecting any files with dash-started lines to begin with this looks a lot like a we should not continue to touch the file as it smells like an attempt to confuse different parsers by "hiding" headers in-between others. The other slightly more reasonable explanation would be an armor header key starting with a dash, but no existing key does that and it seems unlikely that this could ever happen. Also, it is recommended that clients warn about unknown keys, so new appearance is limited.
* Use more abstraction to handle the current line bufferDavid Kalnischkies2019-01-281-83/+133
| | | | | | | This is C++, so we can use a bit more abstraction to let the code look a tiny bit nicer hopefully improving readability a bit. Gbp-Dch: Ignore
* Explicitly remove the whitespaces defined by RFCDavid Kalnischkies2019-01-281-4/+20
| | | | | | | | | | RFC 4880 section 7.1 "Dash-Escaped Text" at the end defines that only space and tab are allowed, so we should remove only these even if due to use complaining (or now failing) you can't really make use of it. Note that strrstrip was removing '\r\n\t ', not other whitespaces like \v or \f and another big reason to do it explicitly here now is to avoid that a future change adding those could have unintended consequences.
* Merge and reuse tmp file handling across the boardDavid Kalnischkies2019-01-241-53/+25
| | | | | | Having many rather similar implementations especially if one is exported while others aren't (and the rest of it not factored out at all) seems suboptimal.
* Fail on non-signature lines in Release.gpgDavid Kalnischkies2019-01-231-20/+64
| | | | | | | | | | The exploit for CVE-2019-3462 uses the fact that a Release.gpg file can contain additional content beside the expected detached signature(s). We were passing the file unchecked to gpgv which ignores these extras without complains, so we reuse the same line-reading implementation we use for InRelease splitting to detect if a Release.gpg file contains unexpected data and fail in this case given that we in the previous commit we established that we fail in the similar InRelease case now.
* Fail instead of warn for unsigned lines in InReleaseDavid Kalnischkies2019-01-231-106/+99
| | | | | | | | | | | | The warnings were introduced 2 years ago without any reports from the wild about them actually appearing for anyone, so now seems to be an as good time as any to switch them to errors. This allows rewritting the code by failing earlier instead of trying to keep going which makes the diff a bit hard to follow but should help simplifying reasoning about it. References: 6376dfb8dfb99b9d182c2fb13aa34b2ac89805e3
* Support multiple keyrings in sources.list Signed-ByDavid Kalnischkies2018-09-111-4/+7
| | | | | | | A user can specify multiple fingerprints for a while now, so its seems counter-intuitive to support only one keyring, especially if this isn't really checked or enforced and while unlikely mixtures of both should work properly, too, instead of a kinda random behaviour.
* Reformat and sort all includes with clang-formatJulian Andres Klode2017-07-121-9/+9
| | | | | | | | | | | | | This makes it easier to see which headers includes what. The changes were done by running git grep -l '#\s*include' \ | grep -E '.(cc|h)$' \ | xargs sed -i -E 's/(^\s*)#(\s*)include/\1#\2 include/' To modify all include lines by adding a space, and then running ./git-clang-format.sh.
* fix various typos reported by spellintianDavid Kalnischkies2017-01-191-1/+1
| | | | | | | | Most of them in (old) code comments. The two instances of user visible string changes the po files of the manpages are fixed up as well. Gbp-Dch: Ignore Reported-By: spellintian
* fix various typos reported by codespellDavid Kalnischkies2017-01-191-1/+1
| | | | | | | Nothing in user visible strings. Gbp-Dch: Ignore Reported-By: codespell
* warn if clearsigned file has ignored content partsDavid Kalnischkies2016-12-311-2/+17
| | | | | | | | | | | | | Clearsigned files like InRelease, .dsc, .changes and co can potentially include unsigned or additional messages blocks ignored by gpg in verification, but a potential source of trouble in our own parsing attempts – and an unneeded risk as the usecases for the clearsigned files we deal with do not reasonably include unsigned parts (like emails or some such). This commit changes the silent ignoring to warnings for now to get an impression on how widespread unintended unsigned parts are, but eventually we want to turn these into hard errors.
* gpgv: Flush the files before checking for errorsJulian Andres Klode2016-12-081-0/+6
| | | | | | | | | | | | | | | | | | This is a follow up to the previous issue where we did not check if getline() returned -1 due to an end of file or due to an error like memory allocation, treating both as end of file. Here we ensure that we also handle buffered writes correctly by flushing the files before checking for any errors in our error stack. Buffered writes themselves were introduced in 1.1.9, but the function was never called with a buffered file from inside apt until commit 46c4043d741cb2c1d54e7f5bfaa234f1b7580f6c which was first released with apt 1.2.10. The function is public, though, so fixing this is a good idea anyway. Affected: >= 1.1.9
* SECURITY UPDATE: gpgv: Check for errors when splitting files (CVE-2016-1252)Julian Andres Klode2016-12-081-1/+22
| | | | | | | | | | | | | | | | | | | | | This fixes a security issue where signatures of the InRelease files could be circumvented in a man-in-the-middle attack, giving attackers the ability to serve any packages they want to a system, in turn giving them root access. It turns out that getline() may not only return EINVAL as stated in the documentation - it might also return in case of an error when allocating memory. This fix not only adds a check that reading worked correctly, it also implicitly checks that all writes worked by reporting any other error that occurred inside the loop and was logged by apt. Affected: >= 0.9.8 Reported-By: Jann Horn <jannh@google.com> Thanks: Jann Horn, Google Project Zero for reporting the issue LP: #1647467
* report apt-key errors via status-fd messagesDavid Kalnischkies2016-11-241-10/+53
| | | | | | | | | | | | | | | | | | | | We report warnings from apt-key this way already since 29c590951f812d9e9c4f17706e34f2c3315fb1f6, 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
* Make directory paths configurableJulian Andres Klode2016-08-261-1/+1
| | | | | | | This allows other vendors to use different paths, or to build your own APT in /opt for testing. Note that this uses + 1 in some places, as the paths we receive are absolute, but we need to strip of the initial /.
* ExecGPGV: Pass current config state to apt-key via temp fileJulian Andres Klode2016-08-031-0/+23
| | | | | | | Create a temporary configuration file with a dump of our configuration and pass that to apt-key. LP: #1607283
* ExecGPGV: Fork in all casesJulian Andres Klode2016-08-031-43/+34
|
* ExecGPGV: Rework file removal on exit()Julian Andres Klode2016-08-031-28/+23
| | | | Create a local exiter object which cleans up files on exit.
* gpgv: Unlink the correct temp file in error caseJulian Andres Klode2016-08-031-4/+4
| | | | | Previously, when data could be created and sig not, we would unlink sig, not data (and vice versa).
* don't use FindFile for external Dir::Bin commandsDavid Kalnischkies2016-06-141-1/+1
| | | | | | | | | | We usually use absolute paths to specific the location of dpkg, apt-key and the like, but there is nothing wrong with using just the command name and instead let exec(3) make the lookup in PATH. We had a wild mixture before, so opting for the more accepting option out of the two seems about right especially as it makes no difference in the default case as apt uses absolute paths.
* use buffered writing for InRelease splittingDavid Kalnischkies2016-04-031-1/+1
| | | | | | Hardly noticeable, but given that we have the option to easily enable it, lets enable it as every newline in the message is written individually by the code.
* implement Signed-By option for sources.listDavid Kalnischkies2015-08-101-1/+16
| | | | | | | | | | 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).
* fix memory leaks reported by -fsanitizeDavid Kalnischkies2015-08-101-0/+2
| | | | | | | | Various small leaks here and there. Nothing particularily big, but still good to fix. Found by the sanitizers while running our testcases. Reported-By: gcc -fsanitize Git-Dch: Ignore
* add and use 'apt-key verify' which prefers gpgv over gpgDavid Kalnischkies2014-09-271-2/+1
| | | | | | | | | | | | | gnupg/gnupg2 can do verify just fine of course, so we don't need to use gpgv here, but it is what we always used in the past, so there might be scripts expecting a certain output and more importantly the output of apt-cdrom contains messages from gpg and even with all the settings we activate to prevent it, it still shows (in some versions) a quiet scary: "gpg: WARNING: Using untrusted key!" message. Keeping the use of gpgv is the simplest way to prevent it. We are increasing also the "Breaks: apt" version from libapt as it requires a newer apt-key than might be installed in partial upgrades.
* add --readonly option for apt-key advDavid Kalnischkies2014-09-271-0/+1
| | | | | | | | Some advanced commands can be executed without the keyring being modified like --verify, so this adds an option to disable the mergeback and uses it for our gpg calling code. Git-Dch: Ignore
* use apt-key adv (+ gnupg) instead of gpgv for verifyDavid Kalnischkies2014-09-271-45/+18
| | | | | | | | apt-key does the keyring merge as we need it, so we just call it instead of reimplementing it to do the merging before gpgv. This means we don't use gpgv anymore (we never depended on it explicitly - bad style), but it also means that the message in apt-cdrom add is a bit less friendly as it says loudly "untrusted key", but for a one-time command its okay.
* cleanup headers and especially #includes everywhereDavid Kalnischkies2014-03-131-8/+10
| | | | | | | | Beside being a bit cleaner it hopefully also resolves oddball problems I have with high levels of parallel jobs. Git-Dch: Ignore Reported-By: iwyu (include-what-you-use)
* correct some style/performance/warnings from cppcheckDavid Kalnischkies2014-01-161-3/+2
| | | | | | | | The most "visible" change is from utime to utimensat/futimens as the first one isn't part of POSIX anymore. Reported-By: cppcheck Git-Dch: Ignore
* rework some code to fix some scan-build warningsDavid Kalnischkies2014-01-161-3/+3
| | | | | | | | No visible functional changes, just code moved around and additional checks to eliminate impossible branches Reported-By: scan-build Git-Dch: Ignore
* factor GetTempDir outMichael Vogt2013-12-221-13/+2
|
* apt-pkg/contrib/gpgv.cc: use /tmp as fallback dirThomas Bechtold2013-12-221-1/+6
| | | | if the directory given by $TMPDIR is not available, use /tmp as fallback.
* trigger NODATA error for invalid InRelease filesDavid Kalnischkies2013-06-201-1/+1
| | | | | | | | | | | | | | | | | With the selfgrown splitting we got the problem of not recovering from networks which just reply with invalid data like those sending us login pages to authenticate with the network (e.g. hotels) back. The good thing about the InRelease file is that we know that it must be clearsigned (a Release file might or might not have a detached sig) so if we get a file but are unable to split it something is seriously wrong, so there is not much point in trying further. The Acquire system already looks out for a NODATA error from gpgv, so this adds a new error message sent to the acquire system in case the splitting we do now ourselves failed including this magic word. Closes: #712486
* apt-pkg/contrib/gpgv.cc: fix InRelease checkMichael Vogt2013-04-031-2/+2
|
* support dash-escaped text in clearsigned files as implementations areDavid Kalnischkies2013-03-191-11/+12
| | | | | | | | free to escape all lines (we have no lines in our files which need to be escaped as these would be invalid fieldnames) and while ExecGPGV would detect dash-escaped text as invalid (as its not expected in messages with detached signatures) it would be possible to "comment" lines in (signed) dsc files which are only parsed but not verified
* use FileFd instead of int fds to tidy up the interface a bitDavid Kalnischkies2013-03-191-56/+47
|
* * apt-pkg/deb/debindexfile.cc,David Kalnischkies2013-03-181-42/+2
| | | | | | | | | | | apt-pkg/deb/deblistparser.cc: - use OpenMaybeClearSignedFile to be free from detecting and skipping clearsigning metadata in dsc and Release files We can't write a "clean" file to disk as not all acquire methods copy Release files before checking them (e.g. cdrom), so this reverts recombining, but uses the method we use for dsc files also in the two places we deal with Release files
* - add method to open (maybe) clearsigned files transparentlyDavid Kalnischkies2013-03-181-1/+54
| | | | | * ftparchive/writer.cc: - use OpenMaybeClearSignedFile to be free from detecting and skipping clearsigning metadata in dsc files
* ensure that we fclose/unlink/free in the new gpg-code as soon as possibleDavid Kalnischkies2013-03-161-5/+22
|
* - if ExecGPGV deals with a clear-signed file it will split this fileDavid Kalnischkies2013-03-151-44/+258
| | | | | | | | into data and signatures, pass it to gpgv for verification and recombines it after that in a known-good way without unsigned blocks and whitespaces resulting usually in more or less the same file as before, but later code can be sure about the format * apt-pkg/deb/debmetaindex.cc: - reenable InRelease by default
* don't close stdout/stderr if it is also the statusfdDavid Kalnischkies2013-03-151-10/+11
|
* * apt-pkg/contrib/gpgv.cc:David Kalnischkies2013-03-151-6/+16
| | | | - ExecGPGV is a method which should never return, so mark it as such and fix the inconsistency of returning in error cases
* * apt-pkg/indexcopy.cc:David Kalnischkies2013-03-151-0/+138
- rename RunGPGV to ExecGPGV and move it to apt-pkg/contrib/gpgv.cc