Re: [PATCH] Add key expiration date to key menu for classic PGP and GPGME.

--qgmvxbrkopjchm3k
Content-Type: text/plain; protected-headers=v1; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
From: Alejandro Colomar <[email protected]>
To: [email protected]
Subject: Re: [PATCH] Add key expiration date to key menu for classic PGP and
 GPGME.
Message-ID: <aku5OQsmkciVoO_M@devuan>
References: <[email protected]>
MIME-Version: 1.0
In-Reply-To: <[email protected]>

Hi Kevin,

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.
>=20
> Add "expires" to $pgp_sort_keys methods.
>=20
> 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(-)
>=20
> 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 '(');
> =20
>          p =3D dest;
> =20
> @@ -3306,7 +3308,7 @@ static const char *crypt_entry_fmt(char *dest,
>            do_locales =3D 1;
> =20
>          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;
> =20
> -          if (key->kobj->subkeys && (key->kobj->subkeys->timestamp > 0))
> +          if (expires && key->kobj->subkeys && (key->kobj->subkeys->expi=
res > 0))
> +            tt =3D key->kobj->subkeys->expires;
> +          else if (!expires && key->kobj->subkeys && (key->kobj->subkeys=
->timestamp > 0))
>              tt =3D key->kobj->subkeys->timestamp;
> =20
>            tm =3D localtime(&tt);
> @@ -3539,6 +3543,32 @@ static int crypt_compare_date(const void *a, const=
 void *b)
>                                           :  _crypt_compare_date(a, b));
>  }
> =20
> +/* 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?

> +
> +  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 *k=
eys,
>      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)
>    }
>  }
> =20
> +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.

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?

> +      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?

> +}
> +
>  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)
> =20
>        }
>        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 {  /* D=
T_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 ex=
pression
> +  ** .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 '(');
> =20
>        p =3D dest;
> =20
> @@ -172,7 +173,7 @@ static const char *pgp_entry_fmt(char *dest,
>          do_locales =3D 1;
> =20
>        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;
> =20
> =20
> -      tm =3D localtime(&key->gen_time);
> +      tm =3D localtime(expires ? &key->exp_time : &key->gen_time);
> =20
>        if (!do_locales)
>          setlocale(LC_TIME, "C");
> @@ -355,6 +356,23 @@ static int pgp_compare_date(const void *a, const voi=
d *b)
>            : _pgp_compare_date(a, b));
>  }
> =20
> +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_a=
ux */
>  #define SORT_UID        21  /* used internally by the IMAP code */
> +#define SORT_EXPIRES    22
> =20
>  /* Sort and sort_aux are shorts, and are a composite of a
>   * constant sort operation number and a set of compounded
> --=20
> 2.55.0
>=20

--=20
<https://www.alejandro-colomar.es>

--qgmvxbrkopjchm3k
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEES7Jt9u9GbmlWADAi64mZXMKQwqkFAmpLvzgACgkQ64mZXMKQ
wqkdpRAAr3iRvD2YkI5nMdomMl7BGeB+lxrGKIPXDfa8fD8gTFeo+8UNn1/xuooh
DvRQiUOqUpGw0n5my98GgmkOro9VINC65VnFi/VTuXHb6Vx3FMQJahwe+Hcg4mlr
O1nnZagSDG1cgHIc7yEJQL5Fq+jVSkRzZyXOWzBLeAgjrXyZ8FqQOOn3+QXenRFZ
P10InshbhT/xJqgxwIcE7xKtoSVbfuwl0LpexvVNVFE0c30bNHIOQ/ARm2EXhKz6
yxjebU8ETzmMYsT4Yp3xW4tNSUywpmfEW+mB1esj3BbEzRek9gACT+ESoNuk+MMo
gCA8J5BvsaeG+b78gcYnEXYIn2VY0w0rkjQJMk1RgsY1tbx9+S5up3oqco06n60P
orLPK5edePNNQk5fsfxQ6lJ1+QzdmqfLaJvX36c2v4GZ6k5OIahPd3XQmgK9bWe8
aqCgCayesWDTFsdraIHe5a8cvz2Jdoo9LZ+hxU8JHRKbaZD6tANehMaURdGfyhWB
o/8T+GdScIrz6uNO9O3C2WzvAhLnfGPmxtRPXf6O0XxaJ34TBCP/BcwUgJ8TbFl8
Mt704UiSe9kC7qEpb9L0ogSP20WI7p1dMd/ITcNceNi6c3cWyR6gbg/mSj8BfUzy
/CwuixEf72/78qLEUvPdaG+/FVH5UYTwXd9LuWLpT2YFGq7Fvds=
=TEsH
-----END PGP SIGNATURE-----

--qgmvxbrkopjchm3k--