platform/upstream/rpm.git
12 years agoMake base64 encoding/decoding part of rpmio public API
Panu Matilainen [Wed, 9 Nov 2011 13:04:13 +0000 (15:04 +0200)]
Make base64 encoding/decoding part of rpmio public API

- Base64 is present in headers and all, it's only reasonable that
  our API users have access to this functionality without having
  to link to other libraries. Even if we didn't want to carry the
  implementation forever in our codebase, we should provide a wrapping
  for this (much like the other crypto stuff) for the reason stated above.
- A bigger issue is that our dirty little (badly hidden) secret was using
  non-namespaced function names, clashing with at least beecrypt. And we
  couldn't have made these internal-only symbols even on platforms that
  support it, because they are used all over the place outside rpmio.
  So... rename the b64 functions to rpmLikeNamingStyle and make 'em public.
  No functional changes, just trivial renaming despite touching numerous
  places.

12 years agoEliminate uses of pgpDig in package signing routines
Panu Matilainen [Wed, 9 Nov 2011 11:55:08 +0000 (13:55 +0200)]
Eliminate uses of pgpDig in package signing routines

- No functional changes, just eliminates pile of unnecessary allocations
  and other calls, simplifying the code a bit.

12 years agoEliminate uses of pgpDig in package reading & signature checking
Panu Matilainen [Wed, 9 Nov 2011 11:43:09 +0000 (13:43 +0200)]
Eliminate uses of pgpDig in package reading & signature checking

- No functional changes, just eliminates pile of unnecessary allocations
  and other calls, simplifying the code a bit.

12 years agoTake advantage of pgpPrtParams() directly in pgpsigFormat() extension
Panu Matilainen [Wed, 9 Nov 2011 11:28:23 +0000 (13:28 +0200)]
Take advantage of pgpPrtParams() directly in pgpsigFormat() extension

- No functional changes, just bypassing an unnecessary round-trip to
  a function really intended for other purposes, now that we can.

12 years agoSwitch to using rpmKeyringVerifySig() internally
Panu Matilainen [Wed, 9 Nov 2011 11:05:08 +0000 (13:05 +0200)]
Switch to using rpmKeyringVerifySig() internally

- Change rpmVerifySignature() to take just the signature parameters
  instead of the whole dig (this is an internal API so we're free
  to mess with it) from which it only needed the signature params.
- The internal low-level verifySignature() is thus reduced to
  to a call to rpmKeyringVerifySig() and spitting some silly
  strings to msg.
- With this, keyring can now use and reuse the its internally stored
  pgp key parameters instead of having to parse the same PGP packets
  over and over. As a result, signature checking is faster now. Not
  dramatically so but measurably nevertheless.

12 years agoAdd a signature verification method to keyring
Panu Matilainen [Wed, 9 Nov 2011 10:47:02 +0000 (12:47 +0200)]
Add a signature verification method to keyring

- At least within rpm itself, callers aren't particularly interested
  in the actual key that matches a given signature, they just want
  simple good/bad/nokey answers. This makes life simple for them
  and avoids exposing further rpmPubkey internals through APIs.

12 years agoSplit keyring find-by-signature to helper function, document...
Panu Matilainen [Wed, 9 Nov 2011 10:31:23 +0000 (12:31 +0200)]
Split keyring find-by-signature to helper function, document...

- Document the broken rpmKeyringLookup() behavior / side-effect,
  the new helper uses the values from our stored pgp parameters though.
- Shouldn't make any difference functionality-wise, but we'll need
  the helper function shortly.

12 years agoParse pubkey parameters on rpmPubkeyNew() already and store results
Panu Matilainen [Wed, 9 Nov 2011 09:59:31 +0000 (11:59 +0200)]
Parse pubkey parameters on rpmPubkeyNew() already and store results

- Yet more pre-requisites for separating key and signature management.
  In addition this gains us more thorough initial sanity checking and
  will allow reusing the parameters instead of having to parse
  the same packets over and over again on every single verification
  against this key. Unfortunately rpmKeyringLookup() is so braindead
  it prevents us from doing this right now, we'll need a better
  interface to take advantage of the stored pgp key parameters.

12 years agoAdd an alternative API for parsing PGP packets
Panu Matilainen [Wed, 9 Nov 2011 09:04:51 +0000 (11:04 +0200)]
Add an alternative API for parsing PGP packets

- pgpPrtParams() returns a pointer to an allocated pgpDigParams
  on success, eliminating the need for callers to worry about
  freeing "target buffer" on failure and bypassing the now rather
  useless pgpDig middleman. Also allows specifying the expected
  packet type so if we expect a key we'll error out if we get a signature
  instead.
- pgpPrtPkts() is basically just a wrapper to pgpPrtParams()
- Further pre-requisites for separating key and signature management.
- Yes, pgpPrtParams() is a stupid name for this. However all the saner
  ones are already taken for other purposes (for which the names are
  just as bad/misleading, sigh)

12 years agoAllocate signature and pubkey dynamically within pgpDig on PGP parse
Panu Matilainen [Wed, 9 Nov 2011 07:49:26 +0000 (09:49 +0200)]
Allocate signature and pubkey dynamically within pgpDig on PGP parse

- This way we can parse the whole thing into a private storage first
  and only if its actually successful we return anything through the
  pgpDig. Previously we would return partial garbage on failure
  and/or consecutive calls unless manually "cleaned" as we were
  parsing directly into the pgpDig.
- Dynamic allocation is a pre-requirement separating management of
  keys and signatures: while they walk hand in hand much of the time,
  they come from different sources and have different lifetimes and
  should be managed separately.
- Dynamic allocation of these is also a pre-requirement for handling
  more than one public key, ie mainly subkeys.

12 years agoUse pgpDigGetParams() in pgpVerifySig() compat wrapper too
Panu Matilainen [Wed, 9 Nov 2011 07:37:15 +0000 (09:37 +0200)]
Use pgpDigGetParams() in pgpVerifySig() compat wrapper too

- The fewer places that "know" about pgpDig allocation internals the better...

12 years agoDon't make assumptions about how pgpDig allocates things
Panu Matilainen [Wed, 9 Nov 2011 07:19:48 +0000 (09:19 +0200)]
Don't make assumptions about how pgpDig allocates things

- Only call pgpDigGetParams() on the public key once we've at least
  tried to fetch it via rpmKeyringLookup(). This way we dont assume
  things about how pgpDig internal allocation is done - currently
  it does return what's essentially a static pointer into pgpDig,
  but this is not a reasonable assumption for an opaque type.
  No functional changes.

12 years agoRevert "Take advantage of pgpDigParamsCmp() in rpmKeyringLookup()"
Panu Matilainen [Tue, 8 Nov 2011 13:08:01 +0000 (15:08 +0200)]
Revert "Take advantage of pgpDigParamsCmp() in rpmKeyringLookup()"

- This only "works" because of other brokenness in the sig/key
  parsing, revert while we can
- This reverts commit 4c51eff3f0fa5e67494b6b192aa1c087f57abed6.

12 years agoTolerate NULL key in pgpVerifySignature()
Panu Matilainen [Tue, 8 Nov 2011 12:08:40 +0000 (14:08 +0200)]
Tolerate NULL key in pgpVerifySignature()

12 years agoDo not let 'rpm -q foo-' find package 'foo'. (RhBug:488567)
Ales Kozumplik [Tue, 8 Nov 2011 08:01:59 +0000 (09:01 +0100)]
Do not let 'rpm -q foo-' find package 'foo'. (RhBug:488567)

- Includes a test suite for the case.

Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
12 years agoEliminate unused params member from pgpDigParams
Panu Matilainen [Mon, 7 Nov 2011 13:39:39 +0000 (15:39 +0200)]
Eliminate unused params member from pgpDigParams

- Rpm has never used this for anything, amounting to helluva lot
  unnecessary free()'s over the years.

12 years agoTake advantage of pgpDigParamsCmp() in rpmKeyringLookup()
Panu Matilainen [Mon, 7 Nov 2011 12:49:47 +0000 (14:49 +0200)]
Take advantage of pgpDigParamsCmp() in rpmKeyringLookup()

- Besides eliminating a couple of direct struct accesses,
  pgpDigParamsCmp() does a much more thorough job of comparing
  the parameters than we ever did here (ie less chance for returning
  ok for for a wrong key, although because the interface is as
  braindead as it is, it doesn't make a whole lot of difference)

12 years agoUse pgpDigParamsAlgo() throughout the codebase
Panu Matilainen [Mon, 7 Nov 2011 12:47:03 +0000 (14:47 +0200)]
Use pgpDigParamsAlgo() throughout the codebase

- Tedious but straightforward conversion to use the API instead
  of going to the struct directly.
- Remove digest.h includes where no longer necessary

12 years agoAdd ad API for retrieving algorithm values from digest parameter containers
Panu Matilainen [Mon, 7 Nov 2011 12:42:13 +0000 (14:42 +0200)]
Add ad API for retrieving algorithm values from digest parameter containers

- Mildly annoying but necessary in order to make pgpDigParams properly
  opaque some day (and also allow sane access to this data)

12 years agoAdd an API for comparing two digest parameter containers
Panu Matilainen [Mon, 7 Nov 2011 11:18:13 +0000 (13:18 +0200)]
Add an API for comparing two digest parameter containers

- Lift the digest parameter comparison from librpmsign to rpmpgp.c
  where it really belongs.

12 years agoAnd finally, make pgpDig struct fully opaque
Panu Matilainen [Mon, 7 Nov 2011 10:56:55 +0000 (12:56 +0200)]
And finally, make pgpDig struct fully opaque

- As long as this was exposed and relied on, we couldn't really
  make any changes to how this stuff is stored. Now we have a chance...

12 years agoEliminate direct pgpDig accesses from signing code
Panu Matilainen [Mon, 7 Nov 2011 10:56:03 +0000 (12:56 +0200)]
Eliminate direct pgpDig accesses from signing code

12 years agoEliminate direct pgpDig accesses from pubkey importing
Panu Matilainen [Mon, 7 Nov 2011 10:55:27 +0000 (12:55 +0200)]
Eliminate direct pgpDig accesses from pubkey importing

12 years agoEliminate direct pgpDig access from package reading code
Panu Matilainen [Mon, 7 Nov 2011 10:55:02 +0000 (12:55 +0200)]
Eliminate direct pgpDig access from package reading code

12 years agoEliminate direct pgpDig accesses from lowlevel signature code
Panu Matilainen [Mon, 7 Nov 2011 10:54:30 +0000 (12:54 +0200)]
Eliminate direct pgpDig accesses from lowlevel signature code

12 years agoEliminate direct pgpDig accesses from keyring
Panu Matilainen [Mon, 7 Nov 2011 10:53:47 +0000 (12:53 +0200)]
Eliminate direct pgpDig accesses from keyring

12 years agoAdd a dumb API to retrieve pubkey / signature params from pgpDig
Panu Matilainen [Mon, 7 Nov 2011 10:52:42 +0000 (12:52 +0200)]
Add a dumb API to retrieve pubkey / signature params from pgpDig

12 years agoTake advantage of parsePGPSig() in pgpsigFormat() too
Panu Matilainen [Mon, 7 Nov 2011 09:19:25 +0000 (11:19 +0200)]
Take advantage of parsePGPSig() in pgpsigFormat() too

- Doesn't make for less lines in this case but unifying the accesses
  is good anyway.

12 years agoUnify the parsePGP() variants from package.c and rpmchecksig.c
Panu Matilainen [Mon, 7 Nov 2011 09:09:08 +0000 (11:09 +0200)]
Unify the parsePGP() variants from package.c and rpmchecksig.c

- Hide allocation inside the helper, automatically free on failure
- Return pointer to the signature parameters on success to simplify
  life for callers
- Don't bother checking or reporting the signature version: the
  pgp parser errors out if it encounters unsupported version and
  does not scrible anything to the version field in that case,
  mumbling about "V0 signatures" is not particularly helpful.
- Log the bad package names from rpmpkgReadHeader() too

12 years agoHide pgpDig alloc etc details in the parsePGP helper
Panu Matilainen [Mon, 7 Nov 2011 08:45:56 +0000 (10:45 +0200)]
Hide pgpDig alloc etc details in the parsePGP helper

- Return a pointer to the signature part on success, hide allocation
  (and free on failure) in the helper. Makes life a little bit
  saner for the callers and limits the places where we access
  the full pgpDig further.

12 years agoProcess all keys and signatures we find
Panu Matilainen [Mon, 7 Nov 2011 06:33:02 +0000 (08:33 +0200)]
Process all keys and signatures we find

- We still can't store more than one signature / key at a time, but
  since we can easily *process* them without trashing already stored
  values, lets do so to avoid returning errors from legal packets.
  Also pay attention to only store data matching our expected type,
  ie dont store signature data into pubkey parameters and vice versa.
- This mostly affects pubkey packets which can have more than one
  key present and which (can) also carry certification signature
  which we currently do not handle properly at all.

12 years agoMake pgpPrtPubkeyParams() return an int like all the others do too
Panu Matilainen [Mon, 7 Nov 2011 06:20:14 +0000 (08:20 +0200)]
Make pgpPrtPubkeyParams() return an int like all the others do too

- No functional changes, just making the interfaces consistent

12 years agoAdd another pgpVerify variant which takes key and sig as separate args
Panu Matilainen [Sun, 6 Nov 2011 18:58:56 +0000 (20:58 +0200)]
Add another pgpVerify variant which takes key and sig as separate args

- pgpVerifySig() is now just a dumb wrapper around pgpVerifySignature()
  which does the real work.
- Update the sole caller to use the new interface instead, deprecate
  the old dig interface.
- First steps towards getting rig of pgpDig which always was a
  strange creature and now is nothing but a nuisance and obfuscation.
  Yes keys and signatures walk hand in hand much of the time, but
  they come from different sources and want to be handled as
  separate data really.

12 years agoEliminate couple of unnecessary pgpDig usages
Panu Matilainen [Sun, 6 Nov 2011 18:43:57 +0000 (20:43 +0200)]
Eliminate couple of unnecessary pgpDig usages

- stashKeyid() only wants the signature, not the whole dig
- dig argument to readFile() was simply unused

12 years agoClean up pgpPrtPkts() and friends a bit
Panu Matilainen [Sun, 6 Nov 2011 15:54:38 +0000 (17:54 +0200)]
Clean up pgpPrtPkts() and friends a bit

- Use decodePkt() for added initial sanity check + grabbing the "main" tag
  instead of duplicating the tag decoding here.
- Call decodePkt() from the main parse loop instead of pgpPrtPkt(),
  and return simple ok/error codes from pgpPrtPkt() and do the
  length calculation in the main loop.
- Besides making the code simpler and more obvious this fixes some
  fishy cases where we previously would've returned 0 for success
  despite there being an error.

12 years agoBury all NSS specifics into a separate source
Panu Matilainen [Fri, 4 Nov 2011 14:28:50 +0000 (16:28 +0200)]
Bury all NSS specifics into a separate source

- Not everybody needs/wants the certified monster that NSS is
  (along with all its quirks), this leaves room for alternative
  compile-time selectable crypto backends. Besides that, we get
  a clean functionality separation for the PGP parser and the
  cryptography parts.
- The whole crypto abstraction works inspired + somewhat based on
  Michael Schroeder's similar patch in Suse, kudos.
- TODO: port beecrypt support from Suse to the new interface.

12 years agoAdd a couple of missing includes, masked by NSS headers
Panu Matilainen [Fri, 4 Nov 2011 14:28:13 +0000 (16:28 +0200)]
Add a couple of missing includes, masked by NSS headers

12 years agoImplement PGP key & sig algorithm specific part OO-style
Panu Matilainen [Fri, 4 Nov 2011 14:05:59 +0000 (16:05 +0200)]
Implement PGP key & sig algorithm specific part OO-style

- Collect the crypto algorithm specific bits into new struct
  with function pointers for the necessary set/verify/free methods,
  adjust callers to operate on these. This will allow nice and
  clean switching for different underlying crypto implementations
  with differing supported algorithms etc with minimal internals exposure.
- pgpSignatureNew() and pgpPubkeyNew() never fail to avoid having
  to check for NULL's over and over, they just return a "null object"
  object for which all operations return failure instead.
- This shreds out some of the output --prtpkts used to give. Wouldn't
  be hard to preserve but the stderr fprintf() spew is not very
  useful nor library-like behavior.

12 years agoLift RSA/DSA specific signature verification to helper functions
Panu Matilainen [Fri, 4 Nov 2011 12:18:29 +0000 (14:18 +0200)]
Lift RSA/DSA specific signature verification to helper functions

- Use function pointers to call appropriate helper, cleaning up
  pgpVerifySig() a bit. Supposedly no functional changes, just
  further isolation of NSS specifics.
- Pass down pubkey and signature as separate pointers, we'll
  want to get rid of pgpDig eventually as it only obfuscates things.

12 years agoLift RSA/DSA key MPI calculations to helper functions
Panu Matilainen [Fri, 4 Nov 2011 12:17:48 +0000 (14:17 +0200)]
Lift RSA/DSA key MPI calculations to helper functions

- Same as commit 8473a5b6ce2050a8e899b0be4a012f5724eb0b6d, only for keys
- Use function pointers to call appropriate helper etc, cleaning up
  pgpPrtPubkeyParams() considerably. Supposedly no functional changes,
  just further isolating NSS specifics behind generic interfaces.

12 years agoLift RSA/DSA signature MPI calculations to helper functions
Panu Matilainen [Fri, 4 Nov 2011 12:15:26 +0000 (14:15 +0200)]
Lift RSA/DSA signature MPI calculations to helper functions

- Use function pointers to call appropriate helper, cleaning up
  pgpPrtSigParams() considerably. Supposedly no functional changes.
- Also serves as first step towards isolating NSS-specific bits
  and pieces behind more generic interfaces to enable using
  alternative crypto "engines" later on.

12 years agoRemove now redundant NULL digparam checks within the PGP parser
Panu Matilainen [Fri, 4 Nov 2011 10:44:58 +0000 (12:44 +0200)]
Remove now redundant NULL digparam checks within the PGP parser

- Since the only entry to these is pgpPrtPkts() and that ensures
  the internals are never called with non-NULL digp... Cleans up
  and simplifies the internals.

12 years agoArrange temporary storage for parsing if called with NULL dig
Panu Matilainen [Fri, 4 Nov 2011 10:32:17 +0000 (12:32 +0200)]
Arrange temporary storage for parsing if called with NULL dig

- The only known caller with NULL dig is in the rather useless
  wrapping of prtPrtPkts() in the python bindings, but since in
  theory some other callers could use this just for validating
  a PGP packet .. preserve the behavior since it's easy. The
  actual benefit here is that this frees the parser internals
  of having to check for NULL pointers everywhere.

12 years agoAdded sanity checks on pgpPrtPkts() entry
Panu Matilainen [Fri, 4 Nov 2011 10:24:32 +0000 (12:24 +0200)]
Added sanity checks on pgpPrtPkts() entry

- Error out cleanly on NULL pkts pointer (caller error but not worth
  dying for)
- Error out early if packet is clearly not valid

12 years agoEliminate bunch of unused/useless debug cruft from pgp parser
Panu Matilainen [Wed, 2 Nov 2011 09:02:28 +0000 (11:02 +0200)]
Eliminate bunch of unused/useless debug cruft from pgp parser

12 years agoSplit digest parameter freeing into a separate helper function
Panu Matilainen [Wed, 2 Nov 2011 08:26:34 +0000 (10:26 +0200)]
Split digest parameter freeing into a separate helper function

- The data is all the same except for rsa/dsa specific bits,
  to me this calls for a function. We might want to export
  pgpCleanDigParams() or such later on but for now keep it static.
  No functional changes.

12 years agoStore the rsa/dsa parameters in pgpDigParamers struct directly
Panu Matilainen [Wed, 2 Nov 2011 08:00:32 +0000 (10:00 +0200)]
Store the rsa/dsa parameters in pgpDigParamers struct directly

- Avoids having to pass around pgpDig pointers in addition to
  pgpDigParamrs pointers. The type (key vs sig) is determined
  early on in pgpPrtPkts() and doesn't change, and the rsa/dsa
  data is associated with that always. No functional changes,
  just makes the whole thing just a little bit cleaner.

12 years agoVerify PGP signature packet sizes and number of MPIs match expectations
Panu Matilainen [Tue, 1 Nov 2011 09:23:34 +0000 (11:23 +0200)]
Verify PGP signature packet sizes and number of MPIs match expectations

- Similar to commit 807b402d95702f3f91e9e2bfbd2b5ca8c9964ed9 but
  for signature packets: packet must be larger than the "intro"
  structure, and verify the calculated sizes match our expectations.

12 years agoEliminate buggy pgpPrtComment()
Panu Matilainen [Tue, 1 Nov 2011 08:52:13 +0000 (10:52 +0200)]
Eliminate buggy pgpPrtComment()

- Removes another source of stupid bugs: for rpm's purposes we're not
  interested in PGP comment tag contents, and the implementation
  here was unsafe as it assumes there always is a terminating \0
  somewhere in the packet which might not be true for a malformed packet.

12 years agoVerify PGP key packet sizes and number of MPIs match expectations, part II
Panu Matilainen [Tue, 1 Nov 2011 08:40:02 +0000 (10:40 +0200)]
Verify PGP key packet sizes and number of MPIs match expectations, part II

- Same as commit 807b402d95702f3f91e9e2bfbd2b5ca8c9964ed9 but for
  retrieving the actual key data instead of its fingerprint.
- Only look inside keys whose pubkey algo we actually support:
  DSA and RSA. Anything else is better left untouched and treated
  as an error to avoid nasty surprises.

12 years agoVerify PGP key packet sizes and number of MPIs match expectations
Panu Matilainen [Tue, 1 Nov 2011 07:44:31 +0000 (09:44 +0200)]
Verify PGP key packet sizes and number of MPIs match expectations

- A key packet must be larger than the "intro" structure to have
  room for the trailing MPIs, ie in order to be valid. This also
  ensures we can safely access the pubkey algorithm data.
- Verify the number of trailing MPI's and their total size matches
  the expectations and packet size exactly before bothering with
  digest calculations.
- Also use sizeof(keyid) instead of "magic eight" and memcpy()
  instead of memmove(), the argument keyid and memory returned
  from rpmDigestFinal() cannot overlap.

12 years agoVerify MPI size is within packet boundary in pgpMpiItem()
Panu Matilainen [Wed, 26 Oct 2011 11:01:41 +0000 (14:01 +0300)]
Verify MPI size is within packet boundary in pgpMpiItem()

- Malformed data can claim the MPI size to be "arbitrarily" large,
  pass packet end pointer to pgpMpiItem() and validate we have enough
  bytes in the packet to contain the MPI before copying.

12 years agoRemove support for V3 public keys
Panu Matilainen [Wed, 26 Oct 2011 09:42:26 +0000 (12:42 +0300)]
Remove support for V3 public keys

- V3 keys have been long since deprecated and extinct for all practical
  purposes for more than a decade by now (for example, Red Hat Linux 6.0
  from 1999 was the last RHL to use a V3 key for signing). RFC-4880
  says V3 keys MUST NOT be generated (they have a number of weaknesses),
  but implementations MAY accept them. We choose not to accept them
  anymore, eliminating a code path that would essentially only get
  triggered by malformed packages. The said code path also contained
  a few buffer overflows and other bugs, so its more than just
  "good riddance."
- Worth nothing is that only support for V3 *keys* is removed, V3
  signatures are still supported along with V4 ones.

12 years agoWe dont deal with secret keys, leave them alone
Panu Matilainen [Wed, 26 Oct 2011 06:14:40 +0000 (09:14 +0300)]
We dont deal with secret keys, leave them alone

- As we only do OpenPGP signature verification and never signing /
  encrypting content ourselves, we have no need to know anything
  about secret keys. One less place to worry about, tripping up
  on bad data that we dont even try to use would be pretty dumb.

12 years agoCentralize PGP packet decoding and sanity checking into helper function
Panu Matilainen [Tue, 25 Oct 2011 12:47:15 +0000 (15:47 +0300)]
Centralize PGP packet decoding and sanity checking into helper function

- Stricter sanity checking on both old and new packet types - whereas
  new format packets were mostly covered by pgpLen() changes already,
  old format has similar case where malformed packet could cause us
  to read beyond packet (buffer) end.
- Collect the necessary packet data into a struct that's nicer to
  pass around (taking advantage of this mostly left for next steps)

12 years agoVerify there are sufficient number of bytes to calculate packet length
Panu Matilainen [Tue, 25 Oct 2011 11:47:44 +0000 (14:47 +0300)]
Verify there are sufficient number of bytes to calculate packet length

- The number of bytes used to store a PGP packet body length is not
  known until we decode the first byte, pass remaining packet length
  to pgpLen() and verify there are sufficient bytes for the
  used encoding before reading them.
- Clarify the function description while at it.

12 years agoAvoid redundant calculations on pubkey fingerprint retrieval
Panu Matilainen [Tue, 25 Oct 2011 11:22:07 +0000 (14:22 +0300)]
Avoid redundant calculations on pubkey fingerprint retrieval

- In pgpPrtPkt() we just calculated the packet body and length,
  avoid redoing it for the fingerprint by splitting the actual
  fingerprint calculation out of pgpPubkeyFingerprint() into a helper
  function and calling that instead.

12 years agopgpPubkeyFingerprint() can fail, propagate errors part II
Panu Matilainen [Tue, 25 Oct 2011 11:03:43 +0000 (14:03 +0300)]
pgpPubkeyFingerprint() can fail, propagate errors part II

- rpmPubkeyNew() needs to return NULL if we fail to grab the
  keyid, make it so...

12 years agopgpPubkeyFingerprint() can fail, propagate errors
Panu Matilainen [Tue, 25 Oct 2011 10:53:42 +0000 (13:53 +0300)]
pgpPubkeyFingerprint() can fail, propagate errors

- Rpm itself doesn't even use pgpExtractPubkeyFingerprint() anymore
  but there appear to be other users so leaving it alone for now,
  just behave sanely on errors.

12 years agoEliminate useless pgpIsPkt() helper function
Panu Matilainen [Mon, 24 Oct 2011 10:45:00 +0000 (13:45 +0300)]
Eliminate useless pgpIsPkt() helper function

- While I can imagine uses for such a function, our only caller is
  using it in a bogus way: decodePkts() is trying to avoid looking into
  binary-only data by calling it, but then pgpIsPkt() returns
  "not pgp tag" for various things that *are* pgp tags, making the
  whole thing just moot. If such checks are actually needed, we'd be better
  of checking for printable characters or such.

12 years agoEliminate broken pgpLen() from the API
Panu Matilainen [Mon, 24 Oct 2011 09:39:51 +0000 (12:39 +0300)]
Eliminate broken pgpLen() from the API

- pgpLen() only works for new format packets, and even for those
  its unsafe and cannot be fixed without breaking the API. Start
  by taking it behind the barn for further, err, operations. Rpm has
  no users outside rpmpgp.c now and anybody else using it will be
  better off not doing so.

12 years agoSanitize pgpsigFormat()
Panu Matilainen [Mon, 24 Oct 2011 09:21:01 +0000 (12:21 +0300)]
Sanitize pgpsigFormat()

- Eliminate bogus size calculations: we have a buffer of td->count size
  that may or may not contain legal OpenPGP signature. Leave it up to
  pgpPrtPkts() to validate & figure it out and check its return code instead,
  eliminating need to repeat a bunch of tedious calculations here.
- Use non-zero signature version is used as a hint for valid signature,
  should be "close enough" for the rest of the code.

12 years agoValid PGP packets are always at least two bytes long
Panu Matilainen [Mon, 24 Oct 2011 08:04:51 +0000 (11:04 +0300)]
Valid PGP packets are always at least two bytes long

- Old format tags encode the number of body length bytes in the packet
  header, new format encodes it in the first body length byte. In
  both cases there must be at least two bytes worth of data for it
  to be a valid header. Sanity check before accessing.

12 years agoFix unterminated buffer after readlink() call
Thomas Jarosch [Fri, 21 Oct 2011 21:05:54 +0000 (23:05 +0200)]
Fix unterminated buffer after readlink() call

readlink() never terminates the buffer.

Detected by "cppcheck" (git HEAD)

Signed-off-by: Thomas Jarosch <thomas.jarosch@intra2net.com>
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
12 years agoLog an error on signing if we can't even parse the gpg-generated signature
Panu Matilainen [Sun, 23 Oct 2011 11:23:12 +0000 (14:23 +0300)]
Log an error on signing if we can't even parse the gpg-generated signature

- The error message is not very helpful but if pgpPrtPkts() fails
  we dont have a whole lot clue in the caller why it failed, spitting
  out at least *some* error is better than silently failing
  (RhBug:748116, RhBug:719154)

12 years agoWarn but don't fail the build on missing excluded files (RhBug:745629)
Panu Matilainen [Sun, 23 Oct 2011 10:59:46 +0000 (13:59 +0300)]
Warn but don't fail the build on missing excluded files (RhBug:745629)

- If a file/directory is not to be packaged, there's not a whole lot
  point making the build fail if its missing. In case exclude is
  used to leave certain files to sub-packages, the sub-package file
  lists will catch out missing files that are really missing as a
  result of actual build failure or such (except perhaps for some
  glob cases but missing files can go unnoticed in those cases anyway)

12 years agoEliminate bunch of exit points in addFile()
Panu Matilainen [Sun, 23 Oct 2011 10:53:21 +0000 (13:53 +0300)]
Eliminate bunch of exit points in addFile()

- Assume failure, handle setting fl->processingFailed centrally at exit
  (file too large wasn't resulting in processingFailed getting set)

12 years agoFix ancient off-by-one at end boundary in string array size calculation
Panu Matilainen [Fri, 21 Oct 2011 08:49:53 +0000 (11:49 +0300)]
Fix ancient off-by-one at end boundary in string array size calculation

- String array size calculation could read one byte past data end
  pointer when expected count and number of \0's disagree (ie invalid data)
  due to while condition side-effects + bounds checking being in
  the inner loop.
- Lift the string length calculation to inline helper function, used for
  both string and string array types.
- Streamline the calculations:
  - Eliminate unnecessary length increments, calculate the length
    from pointer distance
  - Eliminate end pointer NULL checking within the loop: when caller
    doesn't supply end pointer, cap to HEADER_MAX_DATA (ie 16MB),
    anything larger would trip up in later hdrchkData() checks anyway.
  - Avoid the off-by-one by eliminating the problematic inner loop.

12 years agoVerify the entire region trailer, not just its offset, is within data area
Panu Matilainen [Thu, 20 Oct 2011 07:37:31 +0000 (10:37 +0300)]
Verify the entire region trailer, not just its offset, is within data area

- Offset being within the data area doesn't help if the actual data doesn't
  fit. Since the trailer size is well known, we can just as easily
  make the check accurate to prevent reading beyond end of data in case
  the offset is subtly wrong.
- In headerLoad(), region offset of zero doesn't need sanity checking,
  only validate if its something else and do so accurately there too.

12 years agoUpdate man page(s) verify output description to match current behavior
Panu Matilainen [Mon, 17 Oct 2011 06:27:34 +0000 (09:27 +0300)]
Update man page(s) verify output description to match current behavior

- Since addition of file capability verification in rpm 4.7.x,
  the verify output has nine characters, not eight (RhBug:746525)

12 years agoFix pretrans dependency calculation when provider is upgraded
Panu Matilainen [Wed, 12 Oct 2011 06:33:36 +0000 (09:33 +0300)]
Fix pretrans dependency calculation when provider is upgraded

- Pretrans-dependencies are twisty little beasts unlike anything else...
  When a pretrans-dependency provider is updated, the currently installed
  version is the provider for that transaction, unlike others where
  the packages from installing set act as providers for updates. So
  when looking up pretrans deps, we must not prune the to-be-erased
  packages from the db match iterators. As an added twist, we also
  must not cache these non-pruned cases as it would mess up the
  cache for "regular" dependencies.
- Fixes this case reported on fedora-devel:
  http://lists.fedoraproject.org/pipermail/devel/2011-October/158058.html

12 years agorpmio: Set a umask before using mkstemp()
Mukund Sivaraman [Fri, 30 Sep 2011 10:04:44 +0000 (15:34 +0530)]
rpmio: Set a umask before using mkstemp()

This commit sets a restrictive umask before calling mkstemp().
This is because the permissions of files created by mkstemp() are
not defined in POSIX. Old versions of glibc created files with
mode 0666 which can be a security hole. Because the behavior is
implementation-dependent, we set a umask.

12 years agorpmio: Don't de-ref lzfile which was freed in lzclose()
Mukund Sivaraman [Thu, 29 Sep 2011 07:09:13 +0000 (12:39 +0530)]
rpmio: Don't de-ref lzfile which was freed in lzclose()

12 years agobuild: Update .gitignore rules
Mukund Sivaraman [Thu, 29 Sep 2011 07:09:12 +0000 (12:39 +0530)]
build: Update .gitignore rules

12 years agoLet headerLoad() failure message come through
Panu Matilainen [Tue, 11 Oct 2011 07:31:40 +0000 (10:31 +0300)]
Let headerLoad() failure message come through

- headerVerify() always returns with a message even for OK results,
  which was masking the error message from headerLoad(), sometimes
  giving not very helpful "headerRead failed: Header sanity check OK"
  style messages.

12 years agoEliminate headerCheckPayloadFormat() from the API
Panu Matilainen [Thu, 6 Oct 2011 12:16:47 +0000 (15:16 +0300)]
Eliminate headerCheckPayloadFormat() from the API

- While we're on API killing spree... Exporting this was needless and
  dumb to begin with (greetings to self in 2007...), bury it inside
  depends.c as static and let rot there.
- Might be a better idea to kill it completely with some other
  mechanism such as turning payload format into rpmlib() dependency
  internally but just get it out of public sight for now.

12 years agoEliminate headerMergeLegacySigs() from the API
Panu Matilainen [Thu, 6 Oct 2011 12:13:22 +0000 (15:13 +0300)]
Eliminate headerMergeLegacySigs() from the API

- No need to export this in the API - if you want merged signature
  tags you use rpm's package reading functions.

12 years agoEliminate leftover headerRegenSigHeader() function
Panu Matilainen [Thu, 6 Oct 2011 12:05:11 +0000 (15:05 +0300)]
Eliminate leftover headerRegenSigHeader() function

- This was only ever used by repackage support inside rpm and has been
  orphan since 2008, likely more than just a little broken too as it
  doesn't know about 64bit types and all. RIP.

12 years agoOnly bother allocating a pgpDig when needed
Panu Matilainen [Thu, 6 Oct 2011 10:06:28 +0000 (13:06 +0300)]
Only bother allocating a pgpDig when needed

- Now that rpmVerifySignature() doesn't require a non-null dig
  for digests, don't bother allocating one unless necessary.
- pgpNewDig() cannot fail so dont bother checking.

12 years agoEliminate redundant NULL-checks in lower level sigchecking functions
Panu Matilainen [Thu, 6 Oct 2011 09:56:13 +0000 (12:56 +0300)]
Eliminate redundant NULL-checks in lower level sigchecking functions

- sigtd->data and dig checking (where needed) is done at
  rpmVerifySignature() level, dont bother double-checking
- Hash context is dup'ed, which CAN fail, so while we dont need
  to check the argument for non-null, the dup result needs to
  be checked for digests. For actual signatures the dup happens
  elsewhere, we dont need to check the argument for non-null here.

12 years agoSanitize rpmVerifySignature() a bit
Panu Matilainen [Thu, 6 Oct 2011 09:49:18 +0000 (12:49 +0300)]
Sanitize rpmVerifySignature() a bit

- Hash context is required for everything, require non-NULL ctx
  in rpmVerifySignature() already
- pgpDig is only relevant for true signature, digest checking doesn't
  need it - dont require dummy dig to be passed for digests.
- Treat unknown signatures as a case of bad parameters: we're the
  only caller of rpmVerifySignature() so it'd be us screwing up if
  we ask for unknown signature to be verified.
- Treat bad parameters as a hard failure instead of "not found",
  bad parameters mean we cannot verify the signature which really
  equals FAIL.

12 years agoAlso add RPMTAG_OPTFLAGS during spec parse since we easily can...
Panu Matilainen [Thu, 6 Oct 2011 08:14:05 +0000 (11:14 +0300)]
Also add RPMTAG_OPTFLAGS during spec parse since we easily can...

12 years agoAdd implicit self-provides during spec parse already
Panu Matilainen [Thu, 6 Oct 2011 08:02:31 +0000 (11:02 +0300)]
Add implicit self-provides during spec parse already

- Makes the self-provides accessible on spec parse queries, shouldn't
  affect anything else.

12 years agoSplit signature/digest verification out of headerVerify()
Panu Matilainen [Wed, 5 Oct 2011 13:54:15 +0000 (16:54 +0300)]
Split signature/digest verification out of headerVerify()

- headerVerify() is big enough without having all the signature
  goo inline, just lift the whole signature/digest business into
  separate function. Supposedly no functional changes...

12 years agoUnobfuscate header digest calculation in headerVerify()
Panu Matilainen [Wed, 5 Oct 2011 12:08:07 +0000 (15:08 +0300)]
Unobfuscate header digest calculation in headerVerify()

- Assigning goo to temporary variables for calling rpmDigestUpdate()
  doesn't make it any more readable, more the contrary. Also
  don't bother with htonl() (calls that should've been ntohl()
  for "correctness") when we have the data elsewhere in host order already.

12 years agoUnobfuscate headerVerify() exit logic
Panu Matilainen [Wed, 5 Oct 2011 11:46:12 +0000 (14:46 +0300)]
Unobfuscate headerVerify() exit logic

- Jumping forwards is one thing, jumping backwards and forwards to an
  exit label residing in the middle of a function is something else...
  Refactor to single point of exit, at the end of the function.
- Handle the no header-only signature/digest case (whether disabled
  or v3 package) and cleanup centrally at the exit label, everything
  falls through there now.

12 years agoEliminate pointless exit label from headerVerify()
Panu Matilainen [Wed, 5 Oct 2011 10:58:33 +0000 (13:58 +0300)]
Eliminate pointless exit label from headerVerify()

- pgpNewDig() like most rpm "constructor" functions cannot fail,
  no point checking the result. Allows an icky backwards goto + label
  to be eliminated.

12 years agoPush couple of variables to more local scope
Panu Matilainen [Tue, 4 Oct 2011 12:35:42 +0000 (15:35 +0300)]
Push couple of variables to more local scope

- No functional changes, just preparing to tidy up the headerVerify()
  monster a bit.

12 years agoEliminate redundant local variable in headerLoad()
Panu Matilainen [Tue, 4 Oct 2011 11:59:49 +0000 (14:59 +0300)]
Eliminate redundant local variable in headerLoad()

12 years agoSanity check region length on header load
Panu Matilainen [Mon, 3 Oct 2011 14:49:29 +0000 (17:49 +0300)]
Sanity check region length on header load

- Region size can't obviously be larger than the containing header,
  sanity check to avoid crashes from malformed packages.
- We should really test for length equality here, but with dribbles
  the size is sometimes off by three, whatever the reason (bug likely),
  leaving that investigation for some sunnier day...

12 years agoSanity check OpenPGP packet lengths in pgpPrtSubType()
Panu Matilainen [Fri, 30 Sep 2011 12:28:00 +0000 (15:28 +0300)]
Sanity check OpenPGP packet lengths in pgpPrtSubType()

- Sub-packet prefix length + packet length can't very well be larger
  than the remaining packet length. In addition to sanity checking,
  return an error code and have callers actually check for it.
- Fixes (yet another) segfault on malformed package (RhBug:742499)

12 years agoSanity check region offset range on headerLoad()
Panu Matilainen [Thu, 29 Sep 2011 10:22:32 +0000 (13:22 +0300)]
Sanity check region offset range on headerLoad()

- Fixes the  first case crash of RhBug:741606 / CVE-2011-3378 where
  immutable region offset is way out of bounds.

12 years agoSanity check region offset in regionSwab()
Panu Matilainen [Thu, 29 Sep 2011 09:50:57 +0000 (12:50 +0300)]
Sanity check region offset in regionSwab()

- Region offsets are supposed to be negative when when an entry
  is involved, otherwise zero. Fixes some cases of crash'n'burn on
  malformed headers having bogus offsets (CVE-2011-3378)

12 years agoWhoops, ftell() and rpmio equivalents should return long not off_t
Panu Matilainen [Thu, 15 Sep 2011 12:01:27 +0000 (15:01 +0300)]
Whoops, ftell() and rpmio equivalents should return long not off_t

12 years agoFix up a few strict-prototype warnings on x86
Panu Matilainen [Thu, 15 Sep 2011 11:58:19 +0000 (14:58 +0300)]
Fix up a few strict-prototype warnings on x86

12 years agoKick out ppc arch detection leftovers
Panu Matilainen [Thu, 15 Sep 2011 09:21:21 +0000 (12:21 +0300)]
Kick out ppc arch detection leftovers

- This should've been in commit 6e2f56fe25a9ee62af51e0408861a8a43c97a709
  all the way back then, unused ever since...

12 years agoEliminate hysterical copy-paste comments from rpmrc
Panu Matilainen [Thu, 15 Sep 2011 09:06:50 +0000 (12:06 +0300)]
Eliminate hysterical copy-paste comments from rpmrc

12 years agoBit of rpmrc spring-cleaning: nuke detection for some extinct creatures
Panu Matilainen [Thu, 15 Sep 2011 09:05:52 +0000 (12:05 +0300)]
Bit of rpmrc spring-cleaning: nuke detection for some extinct creatures

12 years agoAdd Transifex config + adjust .gitignore to track it
Panu Matilainen [Tue, 13 Sep 2011 11:11:14 +0000 (14:11 +0300)]
Add Transifex config + adjust .gitignore to track it

12 years agoNuke ancient ChangeLog in po/
Panu Matilainen [Tue, 13 Sep 2011 11:07:29 +0000 (14:07 +0300)]
Nuke ancient ChangeLog in po/