From 1e581619bf5315957f2be06b3b1a7f513304c126 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 6 Mar 2025 17:17:17 +0100 Subject: [PATCH] gpg: Fix regression for the recent malicious subkey DoS fix. * g10/packet.h (PUBKEY_USAGE_VERIFY): New. * g10/getkey.c (get_pubkey_for_sig): Pass new flag also to requested usage. (finish_lookup): Introduce a verify_mode. -- Fixes-commit: da0164efc7f32013bc24d97b9afa9f8d67c318bb GnuPG-bug-id: 7547 CVE: CVE-2025-30258 Upstream-Status: Backport [https://dev.gnupg.org/rG1e581619bf5315957f2be06b3b1a7f513304c126] Reference: https://git.launchpad.net/ubuntu/+source/gnupg2/commit/?id=d086c55a85faafdf8448c12ed726d587e729d2d0 Signed-off-by: Yogita Urade --- g10/getkey.c | 42 ++++++++++++++++++++++++++---------------- g10/packet.h | 5 +++-- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/g10/getkey.c b/g10/getkey.c index 0fa763a..2a1b330 100644 --- a/g10/getkey.c +++ b/g10/getkey.c @@ -309,11 +309,12 @@ pk_from_block (PKT_public_key *pk, kbnode_t keyblock, kbnode_t found_key) /* Specialized version of get_pubkey which retrieves the key based on - * information in SIG. In contrast to get_pubkey PK is required. IF + * information in SIG. In contrast to get_pubkey PK is required. If * FORCED_PK is not NULL, this public key is used and copied to PK. * If R_KEYBLOCK is not NULL the entire keyblock is stored there if * found and FORCED_PK is not used; if not used or on error NULL is - * stored there. */ + * stored there. Use this function only to find the key for + * verification; it can't be used to select a key for signing. */ gpg_error_t get_pubkey_for_sig (ctrl_t ctrl, PKT_public_key *pk, PKT_signature *sig, PKT_public_key *forced_pk, kbnode_t *r_keyblock) @@ -333,8 +334,9 @@ get_pubkey_for_sig (ctrl_t ctrl, PKT_public_key *pk, PKT_signature *sig, /* Make sure to request only keys cabable of signing. This makes * sure that a subkey w/o a valid backsig or with bad usage flags - * will be skipped. */ - pk->req_usage = PUBKEY_USAGE_SIG; + * will be skipped. We also request the verification mode so that + * expired and reoked keys are returned. */ + pk->req_usage = (PUBKEY_USAGE_SIG | PUBKEY_USAGE_VERIFY); /* First try the ISSUER_FPR info. */ fpr = issuer_fpr_raw (sig, &fprlen); @@ -398,10 +400,10 @@ get_pubkey_bykid (ctrl_t ctrl, PKT_public_key *pk, kbnode_t *r_keyblock, /* Try to get it from the cache. We don't do this when pk is * NULL as it does not guarantee that the user IDs are cached. * The old get_pubkey_function did not check PK->REQ_USAGE when - * reading form the caceh. This is probably a bug. Note that + * reading from the cache. This is probably a bug. Note that * the cache is not used when the caller asked to return the * entire keyblock. This is because the cache does not - * associate the public key wit its primary key. */ + * associate the public key with its primary key. */ pk_cache_entry_t ce; for (ce = pk_cache; ce; ce = ce->next) { @@ -3634,11 +3636,17 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact, PKT_public_key *pk; int req_prim; int diag_exactfound = 0; + int verify_mode = 0; u32 curtime = make_timestamp (); if (r_flags) *r_flags = 0; + /* The verify mode is used to change the behaviour so that we can + * return an expired or revoked key for signature verification. */ + verify_mode = ((req_usage & PUBKEY_USAGE_VERIFY) + && (req_usage & PUBKEY_USAGE_SIG)); + #define USAGE_MASK (PUBKEY_USAGE_SIG|PUBKEY_USAGE_ENC|PUBKEY_USAGE_CERT) req_usage &= USAGE_MASK; @@ -3682,9 +3690,9 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact, } if (DBG_LOOKUP) - log_debug ("finish_lookup: checking key %08lX (%s)(req_usage=%x)\n", + log_debug ("finish_lookup: checking key %08lX (%s)(req_usage=%x%s)\n", (ulong) keyid_from_pk (keyblock->pkt->pkt.public_key, NULL), - foundk ? "one" : "all", req_usage); + foundk ? "one" : "all", req_usage, verify_mode? ",verify":""); if (diag_exactfound && DBG_LOOKUP) log_debug ("\texact search requested and found\n"); @@ -3747,28 +3755,28 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact, } n_subkeys++; - if (pk->flags.revoked) + if (!verify_mode && pk->flags.revoked) { if (DBG_LOOKUP) log_debug ("\tsubkey has been revoked\n"); n_revoked_or_expired++; continue; } - if (pk->has_expired) + if (!verify_mode && pk->has_expired) { if (DBG_LOOKUP) log_debug ("\tsubkey has expired\n"); n_revoked_or_expired++; continue; } - if (pk->timestamp > curtime && !opt.ignore_valid_from) + if (!verify_mode && pk->timestamp > curtime && !opt.ignore_valid_from) { if (DBG_LOOKUP) log_debug ("\tsubkey not yet valid\n"); continue; } - if (want_secret) + if (!verify_mode && want_secret) { int secret_key_avail = agent_probe_secret_key (NULL, pk); @@ -3788,7 +3796,8 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact, } if (DBG_LOOKUP) - log_debug ("\tsubkey might be fine\n"); + log_debug ("\tsubkey might be fine%s\n", + verify_mode? " for verification":""); /* In case a key has a timestamp of 0 set, we make sure that it is used. A better change would be to compare ">=" but that might also change the selected keys and @@ -3829,12 +3838,12 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact, log_debug ("\tprimary key usage does not match: " "want=%x have=%x\n", req_usage, pk->pubkey_usage); } - else if (pk->flags.revoked) + else if (!verify_mode && pk->flags.revoked) { if (DBG_LOOKUP) log_debug ("\tprimary key has been revoked\n"); } - else if (pk->has_expired) + else if (!verify_mode && pk->has_expired) { if (DBG_LOOKUP) log_debug ("\tprimary key has expired\n"); @@ -3842,7 +3851,8 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact, else /* Okay. */ { if (DBG_LOOKUP) - log_debug ("\tprimary key may be used\n"); + log_debug ("\tprimary key may be used%s\n", + verify_mode? " for verification":""); latest_key = keyblock; } } diff --git a/g10/packet.h b/g10/packet.h index 669739a..061a9b1 100644 --- a/g10/packet.h +++ b/g10/packet.h @@ -135,6 +135,7 @@ typedef struct { gcry_mpi_t data[PUBKEY_MAX_NENC]; } PKT_pubkey_enc; +#define PUBKEY_USAGE_VERIFY 16384 /* Verify only modifier. */ /* An object to build a list of public-key encrypted session key. */ struct pubkey_enc_list @@ -385,8 +386,8 @@ typedef struct byte selfsigversion; /* highest version of all of the self-sigs */ /* The public key algorithm. (Serialized.) */ byte pubkey_algo; - byte pubkey_usage; /* for now only used to pass it to getkey() */ - byte req_usage; /* hack to pass a request to getkey() */ + u16 pubkey_usage; /* for now only used to pass it to getkey() */ + u16 req_usage; /* hack to pass a request to getkey() */ byte fprlen; /* 0 or length of FPR. */ u32 has_expired; /* set to the expiration date if expired */ /* keyid of the primary key. Never access this value directly. -- 2.40.0