| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
| |
rpmio/digest.h contains definitions that are only used by the interal
OpenPGP parser, and are not required by the future Sequoia backend.
Move those definitions into rpmio/rpmpgp_internal.h.
Fixes #2006.
|
|
|
|
|
|
|
|
|
|
| |
Commit d8bb57eeabe249c2c85bf46b1162d7e57a310e37 reintroduced
rpmio/rpmpgp.h which is quite confusing when we have a public header by
the same name elsewhere, and doubly more confusing to those of use who
are used to having the public header by the same name in this very
location prior to commit 650ba79f2253656f9ec8e06f399fafe40e556ed3.
No functional changes.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Split the internal OpenPGP implementation into the bits that are
needed by a new OpenPGP backend like Sequoia, and the bits that are
not needed by another OpenPGP backend.
Move most of the functionality in rpmio/rpmpgp.c into
rpmio/rpmpgp_internal.c.
Leave pgpValStr, and pgpIdentItem, which are used for printing and
needn't be reimplemented by other backends, and pgpReadPkts, which is
just a thin wrapper around pgpParsePkts, and which uses an internal
rpm function that a new backend shouldn't have to worry about
emulating or even calling.
Move the symbol tables, which are used by pgpValStr, pgpIdentItem, and
the internal OpenPGP implementation to rpmio/rpmpgp.h. These are
common to all implementations.
Fixes #2000.
|
|
|
|
|
| |
These haven't been used by rpm in years but have been left to linger,
perhaps for too long. Bye now.
|
|
|
|
|
|
|
|
|
|
|
|
| |
- Add accessor functions pgpDigParamsSignID, pgpDigParamsUserID,
pgpDigParamsVersion, and pgpDigParamsTime.
- Move the definition of `pgpDigParams_s` from `rpmio/digest.h` to
`rpmio/rpmpgp.c`.
- Change code to use the accessor functions.
- Fixes #1979.
|
|
|
|
|
|
|
|
| |
Modify pgpPrtSig() to ignore the MPIs of a signature if its `tag`
parameter is 0. The only caller that sets `tag` to 0 is
pgpPrtParamSubkeys() (via parseSubkeySig()), which does not actually
check any cryptographic signatures. The subkey binding signature has
been checked earlier in pgpPrtParams().
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This ensures that a signature is only accepted if the subkey that made
it is actually allowed to sign. Test 265 verifies that RPM ignores
subkeys that cannot sign.
A subkey is considered to be capable of signing if, and only if, its
subkey binding signature has a hashed key flags subpacket that contains
the flag 0x02. RFC4880 requires that the subkey binding signature be
v4, which this requirement enforces implicitly. RFC4880 also requires
that primary key binding signatures be present and checked. This is not
yet implemented, but may be implemented later.
Fixes #1911.
|
|
|
|
|
|
|
|
|
|
| |
RPM needs to know if a subkey can be used for signing. Signatures made
by a subkey that cannot be used for signing are invalid. Add a
key_flags member to pgpDigParams_s to store this information, and a
PGPDIG_SIG_HAS_KEY_FLAGS flag to indicate that it is valid. The key
usage flags are reset for every signature. Key usage flags in the
unhashed section are ignored. If there is more than one key usage flags
subpacket in the hashed section, the signature is rejected.
|
|
|
|
|
|
| |
This is needed for key usage flags parsing, as key usage flags outside
of the hashed region must be ignored. For now, just use it to
unconditionally ignore unhashed creation time subpackets.
|
|
|
|
|
|
|
|
| |
Such data is probably an attempt to exploit RPM, not do anything useful.
This avoids a whole class of silly integer overflow problems.
Signatures in packages are already limited to less than 64MiB by the
maximum size of the signature header, and this is already a sufficient
limitation.
|
|
|
|
|
|
|
|
| |
According to RFC 4880 §5.2.3.4 the signature creation time MUST be a
hashed subpacket. Enforce this requirement in RPM. Also set the saved
flags to PGPDIG_SAVED_TIME | PGPDIG_SAVED_ID |
PGPDIG_SAVED_CREATION_TIME for v3 signatures, and do not overwrite an
already saved key ID with one taken from a v3 signature.
|
|
|
|
| |
Found by leak sanitizer on a fuzzed test case.
|
|
|
|
|
|
| |
The crypto backend used during importing has little relevance to
anything, plus we discarded the NSS backend two years ago so it doesn't
have an even theoretical chance of being right...
|
|
|
|
|
|
| |
Fixup internal callers to use rpmhex(), deprecate pgpHexStr().
pgpHexStr() should be dropped at next soname bump, whenever that
happens.
|
| |
|
|
|
|
|
|
|
|
|
| |
At this point this is quite literally merely a symbolic change,
as values from PGP hash algo are assumed equal to RPM hash algos,
but it's a necessary first step to supporting hashes not included
in RFC-4880.
Fixes: #1899
|
|
|
|
|
|
| |
If a key ID cannot be obtained, the key is useless. This also ensures
that pgpPrtKey only needs to handle input that getKeyID has already
validated.
|
|
|
|
| |
Also fix a memory leak in an error path.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If a public key was too short for the curve ID, the code would
previously perform out-of-bounds pointer arithmetic, which is undefined
behavior in C. Check that the packet is long enough to contain the
curve ID before bumping `se` past the curve ID.
Furthermore, if a public key is too short to even contain the fixed-size
header, an out-of-bounds pointer would be created, which is also
undefined behavior. Fix this by returning early if the buffer is too
short.
Finally, return early if the public key algorithm or curve ID is
invalid, rather than relying in processMpis() to fail. While
processMpis() will error out, bailing out explicitly is much clearer.
|
|
|
|
|
|
|
|
| |
Make sure selfsig is freed in case we break out of the loop in this
block.
Note that the tests added with the binding validation commit bd36c5d do
not cover this code path so valgrind won't show this.
|
|
|
|
|
| |
The hash context is duplicated unconditionally, but there is an
execution path exiting the function without it being finalized.
|
|
|
|
| |
Mask out the critical bit before switching over the subpacket type.
|
|
|
|
|
|
| |
a44f02631adce0c17435d007df847cdcaee816a7 added additional bounds checks
to pgpGet(). Use these checks to simplify the old-format packet parsing
code by using pgpGet() instead of a manual bounds check.
|
|
|
|
| |
The new code is easier to read and avoids pointer arithmetics.
|
|
|
|
| |
This is much cleaner than repeating the code three different places.
|
|
|
|
|
|
|
|
|
|
|
| |
RPM packages are binary documents and must be signed as such. To avoid
having to access the fields of pgpDigParams() directly, this adds a new
accessor function, pgpSignatureType(). pgpSignatureType() returns the
type of a signature, or -1 if the PGP data is NULL or is not a
signature.
Prior to commit b5e8bc74b2b05aa557f663fe227b94d2bc64fbd8 this was kind
of checked inside the parser but incorrectly.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
All subkeys must be followed by a binding signature by the primary key
as per the OpenPGP RFC, enforce the presence and validity in the parser.
The implementation is as kludgey as they come to work around our
simple-minded parser structure without touching API, to maximise
backportability. Store all the raw packets internally as we decode them
to be able to access previous elements at will, needed to validate ordering
and access the actual data. Add testcases for manipulated keys whose
import previously would succeed.
Depends on the two previous commits:
7b399fcb8f52566e6f3b4327197a85facd08db91 and
236b802a4aa48711823a191d1b7f753c82a89ec5
Fixes CVE-2021-3521.
|
|
|
|
|
| |
No functional changes, just to reduce code duplication and needed by
the following commits.
|
|
|
|
| |
No immediate effect but needed by the following commits.
|
|
|
|
| |
Found by Coverity.
|
|
|
|
| |
instead of recomputing it four places.
|
|
|
|
|
|
| |
This is mostly for the benefit of fuzzers and other automated tools, and
for compilers other than GCC. On modern versions of GCC with
-fno-strict-overflow, this is harmless.
|
|
|
|
|
|
|
| |
This will be used to replace incorrect checks in the calling code.
The new pgpGet() avoids undefined pointer arithmetic, too. One
call-site of pgpGet() is broken by this change, so replace it with a
direct bounds-check.
|
|
|
|
|
|
| |
According to RFC 4880 § 11.4, a detached signature is “simply a
Signature packet”. Therefore, extra packets following a detached
signature are not allowed.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Bit 7 of the subpacket type is the "critical" bit. If set, it
denotes that the subpacket is one that is critical for the evaluator
of the signature to recognize. If a subpacket is encountered that is
marked critical but is unknown to the evaluating software, the
evaluator SHOULD consider the signature to be in error.
We only implement creation time and issuer keyid, everything else is
unimplemented and should be flagged as an error if critical as per above.
Initial patch by Demi Marie Obenour.
|
|
|
|
|
| |
No functional changes here, but makes easier to flag different errors
in the next commits.
|
|
|
|
|
|
| |
Check that the buffer can actually hold the computed number of bytes.
Initial patch by Demi Marie Obenour.
|
|
|
|
|
|
|
|
|
| |
Two-octet packets are recognized by first octet being in range 192-223,
not 192-255. Partial body lengths, which are not supported, use the
224-254 range. A valid five-octet length requires the first octet to
be 255, this was not checked.
Initial patch by Demi Marie Obenour.
|
|
|
|
| |
No change in behavior
|
|
|
|
| |
so the correct wording should be used in the warning.
|
|
|
|
| |
pgpMpiStr() is not used.
|
|
|
|
|
|
|
|
|
|
| |
As of Doxygen >= 1.8.20 it started complaining about anything marked
as @retval being undocumented. As this is widely used in rpm...
Mass-replace all @retval uses with @param[out] to silence. Some of
these are actually in/out parameters but combing through all of them
is a bit too much...
Also escape <CR><LF> in rpmpgp.h to shut up yet another new warning.
|
|
|
|
|
|
|
|
|
| |
The algorithm identifier and encoding format are taken from the
not yet accepted update to rfc4480:
https://datatracker.ietf.org/doc/draft-ietf-openpgp-rfc4880bis/
Gnupg supports EdDSA since version 2.1.0 (released in 2014).
|
|
|
|
|
|
|
|
|
| |
POSIX.1-2008 marked ctime() as obsolete and recommends strftime() instead.
We get two flies on one stroke as ctime() is also classified "dangerous"
by LGTM due to not being thread-safe.
strftime(..., "%c"...) isn't exactly the same as ctime() but is consistent
with what we use elsewhere and sufficient for debug purposes anyway.
|
| |
|
|
|
|
|
|
|
| |
Without the tag comparison, we could mistake a pubkey for a signature, doh.
The userid is relevant too, in that (freely quoting rfc-4880) a single
key might be used for different purposes and the userid allows the keyholder
to express which of their roles is making th signature.
|
| |
|
|
|
|
|
|
|
|
| |
The OpenPGP time fields are unsigned four-octet numbers, storing
it as the uint32_t it actually is makes using the value that
little bit saner.
Way too many places to update as we still have no API for this, sigh.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Originally introduced in commit f5203aea8bd83dc18e48dda4a564429c0e48bab4
in 2004, pgpPubkeyFingerprint() has been returning the 64 bits long
Key ID from the tail of 160 bits long fingerprint, not the actual
fingerprint.
Add a new public API for retrieving the Key ID specifically, adjust
the handful of internal users to use it and make pgpPubkeyFingerprint()
return the actual fingerprint. It's an API break sure but there are
unlikely to be any callers outside rpm and we're breaking the API + ABI
left and right in this release so doesn't matter...
|
|
|
|
|
|
|
| |
The sole user within rpm was removed over six years ago, never seen
a single user outside rpm, and even then it's just a wrapper around
rpmBase64Decode() and pgpPubkeyFingerprint() with a bizarre
return code. Bye bye...
|