Re: [PATCH] Add key expiration date to key menu for classic PGP and GPGME.
"Kevin J. McCarthy" <[email protected]> Tue, 7 Jul 2026 07:01:38 +0800
| Newsgroups | gmane.mail.mutt.devel |
|---|---|
| Message-ID | <[email protected]> |
--O67xic/bRFiJ340q
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Mon, Jul 06, 2026 at 04:44:14PM +0200, Alejandro Colomar via Mutt-dev wr=
ote:
>Hi Kevin,
Thanks for the review, Alex.
I should have mentioned this was mostly a refactor, with code pulled
=66rom the existing creation date field.
>On 2026-07-06T21:41:33+0800, Kevin J. McCarthy wrote:
>> Create expando %(<datefmt>) for the classic PGP and GPGME select key
>> menu in $pgp_entry_format.
>>
>> Add "expires" to $pgp_sort_keys methods.
>>
>> Refactor the datetime parser in gnupgparse.c and apply it to both the
>> creation and expiration dates in the output.
>> ---
>> crypt-gpgme.c | 37 ++++++++++++++++++--
>> gnupgparse.c | 96 +++++++++++++++++++++++++++++----------------------
>> init.h | 3 ++
>> pgpkey.c | 27 +++++++++++++--
>> pgplib.h | 1 +
>> sort.h | 1 +
>> 6 files changed, 118 insertions(+), 47 deletions(-)
>>
>> diff --git a/crypt-gpgme.c b/crypt-gpgme.c
>> index f5ee87bd..6badab78 100644
>> --- a/crypt-gpgme.c
>> +++ b/crypt-gpgme.c
>> @@ -3287,12 +3287,14 @@ static const char *crypt_entry_fmt(char *dest,
>> switch (ascii_tolower(op))
>> {
>> case '[':
>> + case '(':
>> {
>> const char *cp;
>> char buf2[SHORT_STRING], *p;
>> int do_locales;
>> struct tm *tm;
>> size_t len;
>> + int expires =3D (op =3D=3D '(');
>>
>> p =3D dest;
>>
>> @@ -3306,7 +3308,7 @@ static const char *crypt_entry_fmt(char *dest,
>> do_locales =3D 1;
>>
>> len =3D destlen - 1;
>> - while (len > 0 && *cp !=3D ']')
>> + while (len > 0 && *cp !=3D (expires ? ')' : ']'))
>> {
>> if (*cp =3D=3D '%')
>> {
>> @@ -3332,7 +3334,9 @@ static const char *crypt_entry_fmt(char *dest,
>> {
>> time_t tt =3D 0;
>>
>> - if (key->kobj->subkeys && (key->kobj->subkeys->timestamp > 0))
>> + if (expires && key->kobj->subkeys && (key->kobj->subkeys->exp=
ires > 0))
>> + tt =3D key->kobj->subkeys->expires;
>> + else if (!expires && key->kobj->subkeys && (key->kobj->subkey=
s->timestamp > 0))
>> tt =3D key->kobj->subkeys->timestamp;
>>
>> tm =3D localtime(&tt);
>> @@ -3539,6 +3543,32 @@ static int crypt_compare_date(const void *a, cons=
t void *b)
>> : _crypt_compare_date(a, b));
>> }
>>
>> +/* Compare 2 creation dates and the addresses. For sorting. */
>> +static int _crypt_compare_expires(const void *a, const void *b)
>> +{
>> + const crypt_key_t * const *s =3D (const crypt_key_t * const *) a;
>> + const crypt_key_t * const *t =3D (const crypt_key_t * const *) b;
>> + unsigned long ts =3D 0, tt =3D 0;
>
>Why don't we use time_t? Can it overflow?
The field according to the GPGME doc is a unsigned long:
https://www.gnupg.org/documentation/manuals/gpgme/Key-objects.html
unsigned long int timestamp
This is the creation timestamp of the subkey. This is (unsigned=20
long)(-1) if the timestamp is invalid, and 0 if it is not=20
available. Note that an invalid timestamp indicates a bug in the engine.
unsigned long int expires
This is the expiration timestamp of the subkey, or 0 if the subkey does=
=20
not expire.
>> +
>> + if ((*s)->kobj->subkeys && ((*s)->kobj->subkeys->expires > 0))
>> + ts =3D (*s)->kobj->subkeys->expires;
>> + if ((*t)->kobj->subkeys && ((*t)->kobj->subkeys->expires > 0))
>> + tt =3D (*t)->kobj->subkeys->expires;
>> +
>> + if (ts > tt)
>> + return 1;
>> + if (ts < tt)
>> + return -1;
>> +
>> + return mutt_strcasecmp((*s)->uid, (*t)->uid);
>> +}
>> +
>> +static int crypt_compare_expires(const void *a, const void *b)
>> +{
>> + return ((PgpSortKeys & SORT_REVERSE) ? !_crypt_compare_expires(a, b)
>> + : _crypt_compare_expires(a, b=
));
>> +}
>> +
>> /* Compare two trust values, the key length, the creation dates. the
>> addresses and the key IDs. For sorting. */
>> static int _crypt_compare_trust(const void *a, const void *b)
>> @@ -4539,6 +4569,9 @@ static crypt_key_t *crypt_select_key(crypt_key_t *=
keys,
>> case SORT_DATE:
>> f =3D crypt_compare_date;
>> break;
>> + case SORT_EXPIRES:
>> + f =3D crypt_compare_expires;
>> + break;
>> case SORT_KEYID:
>> f =3D crypt_compare_keyid;
>> break;
>> diff --git a/gnupgparse.c b/gnupgparse.c
>> index 81b25503..f3c3d37c 100644
>> --- a/gnupgparse.c
>> +++ b/gnupgparse.c
>> @@ -116,6 +116,53 @@ static void fix_uid(char *uid)
>> }
>> }
>>
>> +static int parse_timestamp(char *p, time_t *timestamp)
>> +{
>> + if (strchr(p, '-')) /* gpg pre-2.0.10 used format (yyyy-mm-dd) */
>> + {
>> + char tstr[11];
>> + struct tm time;
>> +
>> + time.tm_sec =3D 0;
>> + time.tm_min =3D 0;
>> + time.tm_hour =3D 12;
>
>
>
>> + strncpy(tstr, p, 11);
>> + tstr[4] =3D '\0';
>> + tstr[7] =3D '\0';
>
>strncpy(3) should never be used for copying strings with truncation.
>
>Some of its issues are:
>
>- It forces you to terminate explicitly, which is error-prone.
>- It doesn't detect truncation.
>- It makes it more difficult to know if the zeroing is superfluous or
> necessary.
I agree. As you noticed in this case, I was moving very old code=20
around. We can certainly rewrite it, but I'd prefer to do that in a=20
separate commit.
>In this case, I'd use the following to replace the code above:
>
> const char *y, *m, *d;
>
> if (strlen(p) >=3D sizeof(tstr))
> goto bail;
> strcpy(tstr, p);
> p =3D tstr;
> y =3D strsep(&p, "-");
> m =3D strsep(&p, "-");
> d =3D strsep(&p, "-");
> if (p !=3D NULL)
> goto bail;
>
>> + if (mutt_atoi(tstr, &time.tm_year, 0) < 0)
>
>And then here, I'd use y (and in the next two calls, m and d).
>> + {
>> + p =3D tstr;
>
>Setting p here seems dead code; am I missing something?
No, this was a goofup on my part. I failed to notice that it was
resetting p so that it could print a readable error message:
bail:
muttdbg(5, "invalid number: '%s'", p);
return NULL;
I will rework the function to that it passes p by reference and updates
it on error.
>> + goto bail;
>> + }
>> + time.tm_year -=3D 1900;
>> + if (mutt_atoi(tstr+5, &time.tm_mon, 0) < 0)
>> + {
>> + p =3D tstr+5;
>> + goto bail;
>> + }
>> + time.tm_mon -=3D 1;
>> + if (mutt_atoi(tstr+8, &time.tm_mday, 0) < 0)
>> + {
>> + p =3D tstr+8;
>> + goto bail;
>> + }
>> + *timestamp =3D mutt_mktime(&time, 0);
>> + }
>> + else /* gpg 2.0.10+ uses seconds since 1970-01-01 */
>> + {
>> + unsigned long long secs;
>> +
>> + if (mutt_atoull(p, &secs, MUTT_ATOI_ALLOW_EMPTY) < 0)
>> + goto bail;
>> + *timestamp =3D (time_t)secs;
>> + }
>> +
>> + return 0;
>> +
>> +bail:
>> + return -1;
>
>Why not return -1 directly instead of goto?
Yeah this was also because of the refactor, trying the keep the code the=20
exact same. I'll change this too.
>> +}
>> +
>> static pgp_key_t parse_pub_line(char *buf, int *is_subkey, pgp_key_t k)
>> {
>> pgp_uid_t *uid =3D NULL;
>> @@ -245,50 +292,15 @@ static pgp_key_t parse_pub_line(char *buf, int *is=
_subkey, pgp_key_t k)
>>
>> }
>> case 6: /* timestamp (1998-02-28) */
>> - {
>> muttdbg(2, "time stamp: %s", p);
>> -
>> - if (strchr(p, '-')) /* gpg pre-2.0.10 used format (yyyy-mm-dd=
) */
>> - {
>> - char tstr[11];
>> - struct tm time;
>> -
>> - time.tm_sec =3D 0;
>> - time.tm_min =3D 0;
>> - time.tm_hour =3D 12;
>> - strncpy(tstr, p, 11);
>> - tstr[4] =3D '\0';
>> - tstr[7] =3D '\0';
>> - if (mutt_atoi(tstr, &time.tm_year, 0) < 0)
>> - {
>> - p =3D tstr;
>> - goto bail;
>
>Ohhh, now I see where it comes from. :)
>
>It would be nice to have a second commit that cleans up and removes dead
>code (if you want, I can send that).
>
>
>Have a lovely day!
>Alex
>
>> - }
>> - time.tm_year -=3D 1900;
>> - if (mutt_atoi(tstr+5, &time.tm_mon, 0) < 0)
>> - {
>> - p =3D tstr+5;
>> - goto bail;
>> - }
>> - time.tm_mon -=3D 1;
>> - if (mutt_atoi(tstr+8, &time.tm_mday, 0) < 0)
>> - {
>> - p =3D tstr+8;
>> - goto bail;
>> - }
>> - tmp.gen_time =3D mutt_mktime(&time, 0);
>> - }
>> - else /* gpg 2.0.10+ uses seconds since 1970-01=
-01 */
>> - {
>> - unsigned long long secs;
>> -
>> - if (mutt_atoull(p, &secs, MUTT_ATOI_ALLOW_EMPTY) < 0)
>> - goto bail;
>> - tmp.gen_time =3D (time_t)secs;
>> - }
>> + if (parse_timestamp(p, &tmp.gen_time))
>> + goto bail;
>> break;
>> - }
>> - case 7: /* valid for n days */
>> +
>> + case 7: /* expires timestamp */
>> + muttdbg(2, "expires time stamp: %s", p);
>> + if (parse_timestamp(p, &tmp.exp_time))
>> + goto bail;
>> break;
>> case 8: /* Local id */
>> break;
>> diff --git a/init.h b/init.h
>> index af684866..087ebfa8 100644
>> --- a/init.h
>> +++ b/init.h
>> @@ -168,6 +168,7 @@ const struct mapping_t SortAliasMethods[] =3D { /* =
DT_SORT_ALIAS */
>> const struct mapping_t SortKeyMethods[] =3D { /* DT_SORT_KEYS */
>> { "address", SORT_ADDRESS },
>> { "date", SORT_DATE },
>> + { "expires", SORT_EXPIRES },
>> { "keyid", SORT_KEYID },
>> { "trust", SORT_TRUST },
>> { NULL, 0 }
>> @@ -2641,6 +2642,7 @@ struct option_t MuttVars[] =3D {
>> ** .dt %c .dd capabilities
>> ** .dt %t .dd trust/validity of the key-uid association
>> ** .dt %[<s>] .dd date of the key where <s> is an \fCstrftime(3)\fP e=
xpression
>> + ** .dt %(<s>) .dd expires date of the key where <s> is an \fCstrftime=
(3)\fP expression
>> ** .de
>> ** .pp
>> ** (PGP only)
>> @@ -2834,6 +2836,7 @@ struct option_t MuttVars[] =3D {
>> ** .dt address .dd sort alphabetically by user id
>> ** .dt keyid .dd sort alphabetically by key id
>> ** .dt date .dd sort by key creation date
>> + ** .dt expires .dd sort by key expiration date
>> ** .dt trust .dd sort by the trust of the key
>> ** .de
>> ** .pp
>> diff --git a/pgpkey.c b/pgpkey.c
>> index a6532e4c..06789ea3 100644
>> --- a/pgpkey.c
>> +++ b/pgpkey.c
>> @@ -152,13 +152,14 @@ static const char *pgp_entry_fmt(char *dest,
>> switch (ascii_tolower(op))
>> {
>> case '[':
>> -
>> + case '(':
>> {
>> const char *cp;
>> char buf2[SHORT_STRING], *p;
>> int do_locales;
>> struct tm *tm;
>> size_t len;
>> + int expires =3D (op =3D=3D '(');
>>
>> p =3D dest;
>>
>> @@ -172,7 +173,7 @@ static const char *pgp_entry_fmt(char *dest,
>> do_locales =3D 1;
>>
>> len =3D destlen - 1;
>> - while (len > 0 && *cp !=3D ']')
>> + while (len > 0 && *cp !=3D (expires ? ')' : ']'))
>> {
>> if (*cp =3D=3D '%')
>> {
>> @@ -196,7 +197,7 @@ static const char *pgp_entry_fmt(char *dest,
>> *p =3D 0;
>>
>>
>> - tm =3D localtime(&key->gen_time);
>> + tm =3D localtime(expires ? &key->exp_time : &key->gen_time);
>>
>> if (!do_locales)
>> setlocale(LC_TIME, "C");
>> @@ -355,6 +356,23 @@ static int pgp_compare_date(const void *a, const vo=
id *b)
>> : _pgp_compare_date(a, b));
>> }
>>
>> +static int _pgp_compare_expires(const void *a, const void *b)
>> +{
>> + int r;
>> + const pgp_uid_t * const *s =3D (const pgp_uid_t * const *) a;
>> + const pgp_uid_t * const *t =3D (const pgp_uid_t * const *) b;
>> +
>> + if ((r =3D mutt_numeric_cmp((*s)->parent->exp_time, (*t)->parent->exp=
_time)))
>> + return r;
>> + return (mutt_strcasecmp((*s)->addr, (*t)->addr));
>> +}
>> +
>> +static int pgp_compare_expires(const void *a, const void *b)
>> +{
>> + return ((PgpSortKeys & SORT_REVERSE) ? !_pgp_compare_expires(a, b)
>> + : _pgp_compare_expires(a, b));
>> +}
>> +
>> static int _pgp_compare_trust(const void *a, const void *b)
>> {
>> int r;
>> @@ -502,6 +520,9 @@ static pgp_key_t pgp_select_key(pgp_key_t keys,
>> case SORT_DATE:
>> f =3D pgp_compare_date;
>> break;
>> + case SORT_EXPIRES:
>> + f =3D pgp_compare_expires;
>> + break;
>> case SORT_KEYID:
>> f =3D pgp_compare_keyid;
>> break;
>> diff --git a/pgplib.h b/pgplib.h
>> index 4a7fa94e..cccd8d9d 100644
>> --- a/pgplib.h
>> +++ b/pgplib.h
>> @@ -38,6 +38,7 @@ struct pgp_keyinfo
>> int flags;
>> short keylen;
>> time_t gen_time;
>> + time_t exp_time;
>> int numalg;
>> const char *algorithm;
>> struct pgp_keyinfo *parent;
>> diff --git a/sort.h b/sort.h
>> index 863a6655..9fe1e557 100644
>> --- a/sort.h
>> +++ b/sort.h
>> @@ -37,6 +37,7 @@
>> #define SORT_LABEL 19
>> #define SORT_AUX 20 /* $sort_thread_groups delegation to $sort_=
aux */
>> #define SORT_UID 21 /* used internally by the IMAP code */
>> +#define SORT_EXPIRES 22
>>
>> /* Sort and sort_aux are shorts, and are a composite of a
>> * constant sort operation number and a set of compounded
>> --
>> 2.55.0
>>
>
>--=20
><https://www.alejandro-colomar.es>
--=20
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA
--O67xic/bRFiJ340q
Content-Type: application/pgp-signature; name=signature.asc
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCgAdFiEEiXWpszqjeRA4XFMIre92hIAxa9oFAmpMM9IACgkQre92hIAx
a9qVWg/8Dxes3cO/32WeKWngOIrjx05hp4gd0HaE1KOsoHS+vaD4TSToRhfnuXa0
11Ur9D8gBh2KlC74xs3F6f7QneH9WDn77y3/s/S6WONrKMlAfCGvfvmOkguwiBXC
Xuznpl4H3ysb/qtDi180mA4vAWlgZa3Fj51WsKOstFrFtyQsAbzJsacZbDdCtkdg
fViHsPeVBpwgXpL0Z5OB8aKnOILMQ2La1Hj7S/U+1xDtCVVfx0wn0oT6xW72CNZc
AqhCQAlQYYsUHU5jowwBAik/JcFfNQIYAGjG7pzd09/cSTZoLiprp75ix1Bc3sSR
p/F6Cq0NfqBhn0MZFKrVX2eQkGZCL+CIM8y2ltd2j8DafrmhImQCRcOas/3XIimP
OZeijJiut69geWV3vQvwG5nOCblyNLpCHtwSikUZhxvR9B0Z1vLlg/fV63eBsSfk
KRrknt33xOG05i5IPgV+fp3vDuVD7bLP3Kq3Tpm/gTsbJWJI+0wb2G4bBYTXIAmC
8SQ0Rt6mvWZTmEwOfDFWX/g2wuty1cqu9iOaLJ2selo+0EwauNGdohz35pqhijQ4
MMGmph6a9rVvpqF2dUMn+xjS+kHF0G/7wk7q6rcxGz/9i1BIT08T5aGJzMzPDiDU
k8wCfnW4axfgkYbboqxGxQ0m3YcB+zwqiuUlyFve0Uhytsu/TME=
=F5UK
-----END PGP SIGNATURE-----
--O67xic/bRFiJ340q--