summaryrefslogtreecommitdiff
path: root/rpmio/rpmpgp.c
Commit message (Collapse)AuthorAgeFilesLines
* Move digest functionality to the internal OpenPGP implementationNeal H. Walfield2022-04-131-1/+0
| | | | | | | | 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.
* Rename the internal rpmpgp.h header to rpmpgpval.h for clarityPanu Matilainen2022-04-131-1/+1
| | | | | | | | | | 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.
* Move the internal OpenPGP implementation to its own file.Neal H. Walfield2022-04-121-1501/+3
| | | | | | | | | | | | | | | | | | | | | 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.
* Axe pgpDig and related APIsPanu Matilainen2022-04-081-77/+0
| | | | | These haven't been used by rpm in years but have been left to linger, perhaps for too long. Bye now.
* Make pgpDigParams opaqueNeal H. Walfield2022-03-311-8/+57
| | | | | | | | | | | | - 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.
* Avoid unneded MPI reparsingDemi Marie Obenour2022-03-311-2/+2
| | | | | | | | 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().
* Ignore subkeys that cannot be used for signingDemi Marie Obenour2022-03-311-1/+47
| | | | | | | | | | | | | | | 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.
* Parse key usage flagsDemi Marie Obenour2022-03-311-1/+12
| | | | | | | | | | 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.
* Add a hashed flag to pgpPrtSubtype()Demi Marie Obenour2022-03-311-3/+5
| | | | | | 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.
* Reject OpenPGP data at or over 64KiBDemi Marie Obenour2022-03-211-1/+6
| | | | | | | | 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.
* Require creation time to be unique and hashedDemi Marie Obenour2022-03-181-10/+19
| | | | | | | | 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.
* Fix memory leak in pgpPrtParams()Demi Marie Obenour2022-03-181-0/+1
| | | | Found by leak sanitizer on a fuzzed test case.
* Drop misleading hardcoded NSS-3 string from import public keysPanu Matilainen2022-03-141-1/+1
| | | | | | 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...
* Rename pgpHexStr() to rpmhex(), but preserve ABI for nowPanu Matilainen2022-02-251-2/+2
| | | | | | Fixup internal callers to use rpmhex(), deprecate pgpHexStr(). pgpHexStr() should be dropped at next soname bump, whenever that happens.
* Move pgpHexStr() out of rpmpgp.h, it has nothing to do with PGPPanu Matilainen2022-02-251-15/+0
|
* Detach rpm's hash algorithm values from PGP hash algorith valuesPanu Matilainen2022-02-251-1/+1
| | | | | | | | | 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
* Bail out if a key ID cannot be obtainedDemi Marie Obenour2022-02-091-4/+6
| | | | | | 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.
* Check that the CRC length is correctDemi Marie Obenour2022-02-071-1/+2
| | | | Also fix a memory leak in an error path.
* Fix bounds checks in public key parsingDemi Marie Obenour2022-01-251-21/+20
| | | | | | | | | | | | | | | | 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.
* Fix memory leak in pgpPrtParams()Michal Domonkos2022-01-101-5/+4
| | | | | | | | 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.
* Fix hash context leakJustus Winter2021-11-101-0/+2
| | | | | The hash context is duplicated unconditionally, but there is an execution path exiting the function without it being finalized.
* Fix signature subpacket type handlingJustus Winter2021-11-101-1/+1
| | | | Mask out the critical bit before switching over the subpacket type.
* Simplify bounds check in old-format packet parsingDemi Marie Obenour2021-11-011-5/+5
| | | | | | 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.
* Clean up a bounds check in the PGP codeDemi Marie Obenour2021-11-011-2/+2
| | | | The new code is easier to read and avoids pointer arithmetics.
* Move MPI processing into common codeDemi Marie Obenour2021-11-011-35/+26
| | | | This is much cleaner than repeating the code three different places.
* verifySignature(): package signatures must be PGPSIGTYPE_BINARYDemi Marie Obenour2021-11-011-0/+10
| | | | | | | | | | | 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.
* Validate and require subkey binding signatures on PGP public keysPanu Matilainen2021-10-191-7/+91
| | | | | | | | | | | | | | | | | | 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.
* Refactor pgpDigParams construction to helper functionPanu Matilainen2021-10-191-4/+9
| | | | | No functional changes, just to reduce code duplication and needed by the following commits.
* Process MPI's from all kinds of signaturesPanu Matilainen2021-10-191-7/+5
| | | | No immediate effect but needed by the following commits.
* Fix memory leak in decodePkts()Michal Domonkos2021-06-281-1/+5
| | | | Found by Coverity.
* Use a variable for h + hlenDemi Marie Obenour2021-06-211-5/+6
| | | | instead of recomputing it four places.
* Reduce undefined pointer arithmeticDemi Marie Obenour2021-06-211-2/+2
| | | | | | 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.
* pgpGet(): check that the returned length is in boundsDemi Marie Obenour2021-06-211-14/+28
| | | | | | | 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.
* Do not allow extra packets to follow a signatureDemi Marie Obenour2021-06-211-0/+2
| | | | | | According to RFC 4880 § 11.4, a detached signature is “simply a Signature packet”. Therefore, extra packets following a detached signature are not allowed.
* Reject unimplemented critical PGP packets as per RFC-4880Panu Matilainen2021-06-151-0/+7
| | | | | | | | | | | | | 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.
* Refactor error tracking to helper variable in PGP subtype parsingPanu Matilainen2021-06-151-2/+7
| | | | | No functional changes here, but makes easier to flag different errors in the next commits.
* Validate the buffer size when calculating PGP packet sizePanu Matilainen2021-06-151-0/+3
| | | | | | Check that the buffer can actually hold the computed number of bytes. Initial patch by Demi Marie Obenour.
* Fix bugs in new format PGP packet length decoding detectionPanu Matilainen2021-06-151-2/+3
| | | | | | | | | 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.
* Minor const correctness fixDemi Marie Obenour2021-06-151-1/+1
| | | | No change in behavior
* A signature is not a keyDemi Marie Obenour2021-06-141-1/+1
| | | | so the correct wording should be used in the warning.
* Remove dead codeDemi Marie Obenour2021-06-141-15/+0
| | | | pgpMpiStr() is not used.
* Shut up bogus Doxygen warnings about undocumented parametersPanu Matilainen2020-10-281-1/+1
| | | | | | | | | | 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.
* Support the EdDSA public key algorithmMichael Schroeder2020-05-261-3/+42
| | | | | | | | | 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).
* Replace use of obsolete ctime() with strftime()Panu Matilainen2019-10-311-1/+4
| | | | | | | | | 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.
* rpmpgp: Handle EOF without EOL better at END PGPStepan Broz2019-08-131-2/+3
|
* Compare tag and userid too in pgpDigParamsCmp()Panu Matilainen2017-05-261-0/+4
| | | | | | | 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.
* Consolidate OpenPGP time debug printing into a helper functionPanu Matilainen2017-04-131-14/+16
|
* Store signature/key creation time in a saner format internallyPanu Matilainen2017-04-131-3/+3
| | | | | | | | 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.
* Fixup ages old confusion wrt OpenPGP fingerprint vs Key IDPanu Matilainen2017-04-121-11/+26
| | | | | | | | | | | | | 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...
* Remove pgpExtractPubkeyFingerprint()Panu Matilainen2017-04-121-16/+0
| | | | | | | 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...