[Rpm-maint] [rpm-software-management/rpm] Make pgpDigParams opaque (PR #1980)

[Demi Marie Obenour]

@DemiMarie commented on this pull request.

Mostly looks good, but see individual comments for details.

> @@ -33,7 +33,13 @@ typedef struct pgpDig_s * pgpDig;
  */
 typedef struct pgpDigParams_s * pgpDigParams;
 
-typedef uint8_t pgpKeyID_t[8];
+
+/** \ingroup rpmpgp
+ * The length (in bytes) of a binary (not hex encoded) key ID.
+ */
+#define PGP_KEYID_LEN 8

Is there any chance that this could be changed to be a fingerprint in the future?  Yes, I know that is a separate issue :)

> @@ -1113,6 +1119,95 @@ int pgpDigParamsCmp(pgpDigParams p1, pgpDigParams p2);
  */
 unsigned int pgpDigParamsAlgo(pgpDigParams digp, unsigned int algotype);
 
+/** \ingroup rpmpgp
+ * Returns the issuer or the object's Key ID.
+ *
+ * If the object is a signature, then this returns the Key ID stored in the
+ * first Issuer or Issuer Fingerprint subpacket as a hex string.

Just Issuer, actually.  RPM doesn’t currently handle issuer fingerprint subpackets at all (though I have a PR for that).

> + * 			unknown, this returns an array with all zeros.
+ */
+const uint8_t *pgpDigParamsSignID(pgpDigParams digp);
+
+/** \ingroup rpmpgp
+ * Retrieve the primary User ID, if any.
+ *
+ * Returns the primary User ID, if any.
+ *
+ * If the object is a signature, then this returns NULL.
+ *
+ * If the object is a certificate or a subkey, then this returns the
+ * certificate's primary User ID, if any.
+ *
+ * This interface does not provide a way for the caller to recognize
+ * any embedded NUL characters.

I’d be fine with just rejecting certificates with user IDs that have embedded NUL characters, on the grounds that this is more likely to be due to an attack than anything else.

> + *
+ * If the object is a subkey, then this returns the version of the subkey's
+ * key packet.
+ *
+ * param digp		parameter container
+ * return		the object's version
+ */
+int pgpDigParamsVersion(pgpDigParams digp);
+
+/** \ingroup rpmpgp
+ * Retrieve the object's creation time.
+ *
+ * If the object is a signature, then this returns the signature's creation
+ * time.
+ *
+ * If the object is a certificate, then this returns the primary key's key

I don’t think this is what it actually does right now, but feel free to change it so that it does do this.

> +/** \ingroup rpmpgp
+ * Retrieve the object's creation time.
+ *
+ * If the object is a signature, then this returns the signature's creation
+ * time.
+ *
+ * If the object is a certificate, then this returns the primary key's key
+ * creation time.
+ *
+ * If the object is a subkey, then this returns the subkey's key creation
+ * time.
+ *
+ * param digp		parameter container
+ * return		seconds since the UNIX Epoch.
+ */
+uint32_t pgpDigParamsTime(pgpDigParams digp);

```suggestion
uint32_t pgpDigParamsCreationTime(pgpDigParams digp);
```

There are other timestamps that an object can have, so I suggest being specific as to which one this is.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1980#pullrequestreview-924955550
You are receiving this because you are subscribed to this thread.

Message ID: <rpm-software-management/rpm/pull/1980/review/924955550 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20220329/be047895/attachment.html>