[Rpm-maint] [rpm-software-management/rpm] Require creation time to be unique and hashed (PR #1912)

[Demi Marie Obenour]

@DemiMarie commented on this pull request.



>  	    impl = *p;
-	    if (!(_digp->saved & PGPDIG_SAVED_TIME) &&
-		(sigtype == PGPSIGTYPE_POSITIVE_CERT || sigtype == PGPSIGTYPE_BINARY || sigtype == PGPSIGTYPE_TEXT || sigtype == PGPSIGTYPE_STANDALONE))

> * First, there are more certifications that can be used by implementations as binding (self) signatures.  Note the "most" in Section [5.2.1 of RFC4880](https://datatracker.ietf.org/doc/html/rfc4880#section-5.2.1):
> 
> > ```
> >   Most OpenPGP implementations make their "key signatures" as 0x10
> >   certifications.  Some implementations can issue 0x11-0x13
> >   certifications, but few differentiate between the types.
> > ```

RPM does not implement self-signatures at all.  RPM basically implements a subset of `gpgv`.

> * You should really care about subkey binding signatures.  How else would you know that a subkey is eligible to make signatures in the name of the certificate, i.e. whether
>   
>   * the subkey is bound to a certificate at the time the signature in question is created, and

How should RPM determine this?  Is it by checking that the signature creation time was after the binding signature creation time?  In the presence of multiple binding signatures, is it okay to only use the first one?

>   * the subkey is marked as signing-capable?

RPM does not know either of these.  I actually consider this to be a security vulnerability, but I do not know if upstream does.  In any case there is no point in an embargo since this is already public.

> "Care about" here means that the creation time is used calculate the "release" element of the synthetic pubkey rpm package. There's nothing else rpm does with the creation time of pubkey signatures.

In this case RPM should use a completely separate field for this.  The actual time is important for the reasons @teythoon mentioned.

> @@ -473,16 +473,15 @@ static int pgpPrtSubType(const uint8_t *h, size_t hlen, pgpSigType sigtype,
 	    for (i = 1; i < plen; i++)
 		pgpPrtVal(" ", pgpKeyServerPrefsTbl, p[i]);
 	    break;
-	case PGPSUBTYPE_SIG_CREATE_TIME:
+	case PGPSUBTYPE_SIG_CREATE_TIME:  /* signature creation time */
+	    if (plen-1 != sizeof(_digp->time))
+		break; /* other lengths not understood */
+	    if (_digp->saved & PGPDIG_SAVED_TIME)
+		return 1; /* duplicate timestamps not allowed */

Okay, that makes sense.  I will clear all other flags, but leave `PGPDIG_SAVED_TIME` alone.

> the release element of the created pubkey database entries will be different.

That is surprising; I would have expected that the creation time of the _key_ would be used.  I believe the order of signatures is not something that should be relied on.  In any case, RPM really needs to switch to using full fingerprints, but that is for a separate PR.

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

Message ID: <rpm-software-management/rpm/pull/1912/review/884813346 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20220216/945d020d/attachment.html>