Age | Commit message (Collapse) | Author | Files | Lines |
|
- Fix regression from commit 807b402d95702f3f91e9e2bfbd2b5ca8c9964ed9,
the array gets passed as a pointer (how else would it work at all),
so despite having seemingly correct type, sizeof(keyid) depends
on the pointer size. This happens to be 8 on x86_64 and friends
but breaks pgp fingerprint calculation on eg i386.
- Also return the explicit size from pgpExtractPubkeyFingerprint(),
this has been "broken" for much longer but then all callers should
really care about is -1 for error.
|
|
- Should've been in commit 70f063cb773bedb7d336429d9bc8ed1d4e5d18f4
|
|
- 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.
|
|
- 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)
|
|
- 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.
|
|
- The fewer places that "know" about pgpDig allocation internals the better...
|
|
|
|
- Rpm has never used this for anything, amounting to helluva lot
unnecessary free()'s over the years.
|
|
- Mildly annoying but necessary in order to make pgpDigParams properly
opaque some day (and also allow sane access to this data)
|
|
- Lift the digest parameter comparison from librpmsign to rpmpgp.c
where it really belongs.
|
|
- 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...
|
|
|
|
- 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.
|
|
- No functional changes, just making the interfaces consistent
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- Error out cleanly on NULL pkts pointer (caller error but not worth
dying for)
- Error out early if packet is clearly not valid
|
|
|
|
- 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.
|
|
- 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.
|
|
- Similar to commit 807b402d95702f3f91e9e2bfbd2b5ca8c9964ed9 but
for signature packets: packet must be larger than the "intro"
structure, and verify the calculated sizes match our expectations.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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)
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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.
|
|
- 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)
|
|
|
|
- NULL hash is pretty much a can't happen-case here but lets be
sane if it happens afterall - NULL hash would be an error and
we dont want to process the rest if that happened.
|
|
- Fixes a regression originating all the way back from commit
c7fc09d585ff3831924f72f61d990aa791f2c3f2 (ie rpm >= 4.8.0)
where a package with a bogus signature can slip through undetected
if we dont have a key for it.
- This additional sanity check on the signature prevents is enough
to prevent the fuzzed package in RhBug:721225 from crashing us
by stopping the bad package at the front door. That we don't have
proper tag data validation is another, much wider issue...
|
|
- This is not "correct", we should permit more than one user id.
Leaking memory is still worse than not leaking, corrent behavior
or not.
|
|
- While OpenPGP permits arbitrary number of keys per packet/armor,
we can't handle more than one, error out early. The poor user
wont get much of a clue as to what went wrong, but thats still
better than crashing and burning.
- Return NULL from pgpPrtPubkeyParams() on errors and pass it onwards
from pgpPrtKey() which propagates it up to callers. Besides
the crash, this also fixes the error path from pgpNewPublicKey()
failures.
|
|
|
|
|
|
- Fixes "Unknown hash algorithm" message but this is cosmetic only as
NSS doesn't currently support SHA-224.
|