From 2e4b1f78505513c333532755a715bce0f5ad3c99 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 28 May 2024 11:52:04 +0200 Subject: [PATCH GnuPG] tpm: Do not use fprintf for logging. * tpm2d/intel-tss.h (TSS_Create): Replace fprintf logging by log_error. --- tpm2d/intel-tss.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tpm2d/intel-tss.h b/tpm2d/intel-tss.h index 53da5cee2..1649cca05 100644 --- a/tpm2d/intel-tss.h +++ b/tpm2d/intel-tss.h @@ -300,7 +300,7 @@ TSS_Create(TSS_CONTEXT **tssContext) } else { - fprintf(stderr, "Unknown TPM_INTERFACE_TYPE %s\n", intType); + log_error ("tss: Unknown TPM_INTERFACE_TYPE %s\n", intType); } } @@ -352,15 +352,15 @@ TSS_Hash_Generate(TPMT_HA *digest, ...) rc = TSS_Hash_GetMd(&algo, digest->hashAlg); if (rc) { - fprintf(stderr, "TSS_HASH_GENERATE: Unknown hash %d\n", - digest->hashAlg); + log_error ("TSS_Hash_Generate: Unknown hash %d\n", digest->hashAlg); goto out; } rc = gcry_md_open (&md, algo, 0); if (rc != 0) { - fprintf(stderr, "TSS_Hash_Generate: EVP_MD_CTX_create failed\n"); + log_error ("TSS_Hash_Generate: EVP_MD_CTX_create failed: %s\n", + gpg_strerror (rc)); rc = TPM_RC_FAILURE; goto out; } @@ -374,7 +374,7 @@ TSS_Hash_Generate(TPMT_HA *digest, ...) break; if (length < 0) { - fprintf(stderr, "TSS_Hash_Generate: Length is negative\n"); + log_error ("TSS_Hash_Generate: Length is negative\n"); goto out_free; } if (length != 0) @@ -408,9 +408,8 @@ tpm2_error(TPM_RC rc, const char *reason) { const char *msg; - fprintf(stderr, "%s failed with %d\n", reason, rc); msg = Tss2_RC_Decode(rc); - fprintf(stderr, "%s\n", msg); + log_error ("%s failed with error %d (%s)\n", reason, rc, msg); } static inline int -- 2.45.2 From e0543f97be007983a0a052df09a852a11331ee5d Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Wed, 15 May 2024 15:27:20 +0900 Subject: [PATCH GnuPG] tpm2d: Use BYTE type to acces TPM2B object. * tpm2d/tpm2.c (tpm2_SensitiveToDuplicate): Don't use the cast of (TPM2B *). -- While it works (since the actual access is done by the macros), compiler may complain the alignment property of type BYTE * and TPM2B object is different. Signed-off-by: NIIBE Yutaka --- tpm2d/tpm2.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/tpm2d/tpm2.c b/tpm2d/tpm2.c index 3e908ddb1..d0b32ed35 100644 --- a/tpm2d/tpm2.c +++ b/tpm2d/tpm2.c @@ -695,8 +695,8 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s, { TPMT_HA hash; const int hlen = TSS_GetDigestSize (nalg); - TPM2B *digest = (TPM2B *)buf; - TPM2B *s2b; + BYTE *digest; + BYTE *s2b; int32_t size; unsigned char null_iv[AES_128_BLOCK_SIZE_BYTES]; UINT16 bsize, written = 0; @@ -707,13 +707,12 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s, memset (null_iv, 0, sizeof (null_iv)); /* reserve space for hash before the encrypted sensitive */ - bsize = sizeof (digest->size) + hlen; - buf += bsize; + digest = buf; + bsize = sizeof (uint16_t /* TPM2B.size */) + hlen; p->size += bsize; - s2b = (TPM2B *)buf; + s2b = digest + bsize; /* marshal the digest size */ - buf = (BYTE *)&digest->size; bsize = hlen; size = 2; TSS_UINT16_Marshal (&bsize, &written, &buf, &size); @@ -721,13 +720,13 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s, /* marshal the unencrypted sensitive in place */ size = sizeof (*s); bsize = 0; - buf = s2b->buffer; + buf = s2b + offsetof (TPM2B, buffer); TSS_TPMT_SENSITIVE_Marshal (s, &bsize, &buf, &size); - buf = (BYTE *)&s2b->size; + buf = s2b; size = 2; TSS_UINT16_Marshal (&bsize, &written, &buf, &size); - bsize = bsize + sizeof (s2b->size); + bsize = bsize + sizeof (uint16_t /* TPM2B.size */); p->size += bsize; /* compute hash of unencrypted marshalled sensitive and @@ -736,7 +735,7 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s, TSS_Hash_Generate (&hash, bsize, s2b, name->size, name->name, 0, NULL); - memcpy (digest->buffer, &hash.digest, hlen); + memcpy (digest + offsetof (TPM2B, buffer), &hash.digest, hlen); gcry_cipher_open (&hd, GCRY_CIPHER_AES128, GCRY_CIPHER_MODE_CFB, GCRY_CIPHER_SECURE); gcry_cipher_setiv (hd, null_iv, sizeof (null_iv)); @@ -749,20 +748,20 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s, else if (symdef->algorithm == TPM_ALG_NULL) { /* Code is for debugging only, should never be used in production */ - TPM2B *s2b = (TPM2B *)buf; + BYTE *s2b = buf; int32_t size = sizeof (*s); UINT16 bsize = 0, written = 0; log_error ("Secret key sent to TPM unencrypted\n"); - buf = s2b->buffer; + buf = s2b + offsetof (TPM2B, buffer); /* marshal the unencrypted sensitive in place */ TSS_TPMT_SENSITIVE_Marshal (s, &bsize, &buf, &size); - buf = (BYTE *)&s2b->size; + buf = s2b; size = 2; TSS_UINT16_Marshal (&bsize, &written, &buf, &size); - p->size += bsize + sizeof (s2b->size); + p->size += bsize + sizeof (uint16_t /* TPM2B.size */); } else { -- 2.45.2 From d631c8198c254107c0a4e704511fa0f33d3dda5f Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 28 May 2024 12:45:21 +0200 Subject: [PATCH GnuPG] tpm: Improve error handling and check returned lengths. * tpm2d/command.c (cmd_pkdecrypt): Handle unknown algo. Also slightly rework error handling. * tpm2d/tpm2.c (sexp_to_tpm2_public_ecc): Check length before checking for 0x04. Rework error handling. (tpm2_ObjectPublic_GetName): Check the return value of TSS_GetDigestSize before use. Erro handling rework. (tpm2_SensitiveToDuplicate): Ditto. (tpm2_import_key): Ditto. * tpm2d/intel-tss.h (TSS_Hash_Generate): Check passed length for negative values. Check return value of TSS_GetDigestSize. Use dedicated 16 bit length variable. -- These are reworked and improved fixes as reported in GnuPG-bug-id: 7129 --- tpm2d/command.c | 12 ++-- tpm2d/intel-tss.h | 23 +++++--- tpm2d/tpm2.c | 139 +++++++++++++++++++++++++++++----------------- 3 files changed, 109 insertions(+), 65 deletions(-) diff --git a/tpm2d/command.c b/tpm2d/command.c index 6f8cb5506..8f69a5e11 100644 --- a/tpm2d/command.c +++ b/tpm2d/command.c @@ -291,12 +291,12 @@ cmd_pkdecrypt (assuan_context_t ctx, char *line) { ctrl_t ctrl = assuan_get_pointer (ctx); int rc; - unsigned char *shadow_info; + unsigned char *shadow_info = NULL; size_t len; TSS_CONTEXT *tssc; TPM_HANDLE key; TPMI_ALG_PUBLIC type; - unsigned char *crypto; + unsigned char *crypto = NULL; size_t cryptolen; char *buf; size_t buflen; @@ -313,7 +313,7 @@ cmd_pkdecrypt (assuan_context_t ctx, char *line) rc = assuan_inquire (ctx, "EXTRA", &crypto, &cryptolen, MAXLEN_KEYDATA); if (rc) - goto out_freeshadow; + goto out; rc = tpm2_start (&tssc); if (rc) @@ -329,6 +329,11 @@ cmd_pkdecrypt (assuan_context_t ctx, char *line) else if (type == TPM_ALG_ECC) rc = tpm2_ecc_decrypt (ctrl, tssc, key, pin_cb, crypto, cryptolen, &buf, &buflen); + else + { + rc = GPG_ERR_PUBKEY_ALGO; + goto end_out; + } tpm2_flush_handle (tssc, key); @@ -343,7 +348,6 @@ cmd_pkdecrypt (assuan_context_t ctx, char *line) out: xfree (crypto); - out_freeshadow: xfree (shadow_info); return rc; diff --git a/tpm2d/intel-tss.h b/tpm2d/intel-tss.h index 1649cca05..da085fac7 100644 --- a/tpm2d/intel-tss.h +++ b/tpm2d/intel-tss.h @@ -344,7 +344,7 @@ TSS_Hash_Generate(TPMT_HA *digest, ...) int length; uint8_t *buffer; int algo; - gcry_md_hd_t md; + gcry_md_hd_t md = NULL; va_list ap; va_start(ap, digest); @@ -353,7 +353,7 @@ TSS_Hash_Generate(TPMT_HA *digest, ...) if (rc) { log_error ("TSS_Hash_Generate: Unknown hash %d\n", digest->hashAlg); - goto out; + goto leave; } rc = gcry_md_open (&md, algo, 0); @@ -362,7 +362,7 @@ TSS_Hash_Generate(TPMT_HA *digest, ...) log_error ("TSS_Hash_Generate: EVP_MD_CTX_create failed: %s\n", gpg_strerror (rc)); rc = TPM_RC_FAILURE; - goto out; + goto leave; } rc = TPM_RC_FAILURE; @@ -374,19 +374,24 @@ TSS_Hash_Generate(TPMT_HA *digest, ...) break; if (length < 0) { - log_error ("TSS_Hash_Generate: Length is negative\n"); - goto out_free; + log_error ("%s: Length is negative\n", "TSS_Hash_Generate"); + goto leave; } if (length != 0) gcry_md_write (md, buffer, length); } - memcpy (&digest->digest, gcry_md_read (md, algo), - TSS_GetDigestSize(digest->hashAlg)); + length = TSS_GetDigestSize(digest->hashAlg); + if (length < 0) + { + log_error ("%s: Length is negative\n", "TSS_GetDigestSize"); + goto leave; + } + memcpy (&digest->digest, gcry_md_read (md, algo), length); rc = TPM_RC_SUCCESS; - out_free: + + leave: gcry_md_close (md); - out: va_end(ap); return rc; } diff --git a/tpm2d/tpm2.c b/tpm2d/tpm2.c index d0b32ed35..2a49bf99b 100644 --- a/tpm2d/tpm2.c +++ b/tpm2d/tpm2.c @@ -446,46 +446,55 @@ static int sexp_to_tpm2_public_ecc (TPMT_PUBLIC *p, gcry_sexp_t key) { const char *q; - gcry_sexp_t l; - int rc = GPG_ERR_BAD_PUBKEY; + gcry_sexp_t l = NULL; + int rc; size_t len; TPMI_ECC_CURVE curve; - char *curve_name; + char *curve_name = NULL; l = gcry_sexp_find_token (key, "curve", 0); if (!l) - return rc; + { + rc = GPG_ERR_NO_PUBKEY; + goto leave; + } curve_name = gcry_sexp_nth_string (l, 1); if (!curve_name) - goto out; + { + rc = GPG_ERR_INV_CURVE; + goto leave; + } rc = tpm2_ecc_curve (curve_name, &curve); - gcry_free (curve_name); if (rc) - goto out; - gcry_sexp_release (l); + goto leave; + gcry_sexp_release (l); l = gcry_sexp_find_token (key, "q", 0); if (!l) - return rc; + { + rc = GPG_ERR_NO_PUBKEY; + goto leave; + } q = gcry_sexp_nth_data (l, 1, &len); /* This is a point representation, the first byte tells you what * type. The only format we understand is uncompressed (0x04) * which has layout 0x04 | x | y */ - if (q[0] != 0x04) + if (!q || len < 2 || q[0] != 0x04) { - log_error ("Point format for q is not uncompressed\n"); - goto out; + log_error ("tss: point format for q is not uncompressed\n"); + rc = GPG_ERR_BAD_PUBKEY; + goto leave; } q++; len--; /* now should have to equal sized big endian point numbers */ if ((len & 0x01) == 1) { - log_error ("Point format for q has incorrect length\n"); - goto out; + log_error ("tss: point format for q has incorrect length\n"); + rc = GPG_ERR_BAD_PUBKEY; + goto leave; } - - len >>= 1; + len >>= 1; /* Compute length of one coordinate. */ p->type = TPM_ALG_ECC; p->nameAlg = TPM_ALG_SHA256; @@ -502,7 +511,9 @@ sexp_to_tpm2_public_ecc (TPMT_PUBLIC *p, gcry_sexp_t key) VAL_2B (p->unique.ecc.x, size) = len; memcpy (VAL_2B (p->unique.ecc.y, buffer), q + len, len); VAL_2B (p->unique.ecc.y, size) = len; - out: + + leave: + gcry_free (curve_name); gcry_sexp_release (l); return rc; } @@ -637,36 +648,42 @@ tpm2_ObjectPublic_GetName (NAME_2B *name, uint16_t written = 0; TPMT_HA digest; uint32_t sizeInBytes; + INT32 size = MAX_RESPONSE_SIZE; uint8_t buffer[MAX_RESPONSE_SIZE]; + uint8_t *buffer1 = buffer; + TPMI_ALG_HASH nameAlgNbo; + int length; /* marshal the TPMT_PUBLIC */ - if (rc == 0) - { - INT32 size = MAX_RESPONSE_SIZE; - uint8_t *buffer1 = buffer; - rc = TSS_TPMT_PUBLIC_Marshal (tpmtPublic, &written, &buffer1, &size); - } + rc = TSS_TPMT_PUBLIC_Marshal (tpmtPublic, &written, &buffer1, &size); + if (rc) + goto leave; + /* hash the public area */ - if (rc == 0) - { - sizeInBytes = TSS_GetDigestSize (tpmtPublic->nameAlg); - digest.hashAlg = tpmtPublic->nameAlg; /* Name digest algorithm */ - /* generate the TPMT_HA */ - rc = TSS_Hash_Generate (&digest, written, buffer, 0, NULL); - } - if (rc == 0) + length = TSS_GetDigestSize (tpmtPublic->nameAlg); + if (length < 0) { - TPMI_ALG_HASH nameAlgNbo; - - /* copy the digest */ - memcpy (name->name + sizeof (TPMI_ALG_HASH), - (uint8_t *)&digest.digest, sizeInBytes); - /* copy the hash algorithm */ - nameAlgNbo = htons (tpmtPublic->nameAlg); - memcpy (name->name, (uint8_t *)&nameAlgNbo, sizeof (TPMI_ALG_HASH)); - /* set the size */ - name->size = sizeInBytes + sizeof (TPMI_ALG_HASH); + rc = TPM_RC_VALUE; + goto leave; } + sizeInBytes = length; + digest.hashAlg = tpmtPublic->nameAlg; /* Name digest algorithm */ + + /* generate the TPMT_HA */ + rc = TSS_Hash_Generate (&digest, written, buffer, 0, NULL); + if (rc) + goto leave; + + /* copy the digest */ + memcpy (name->name + sizeof (TPMI_ALG_HASH), + (uint8_t *)&digest.digest, sizeInBytes); + /* copy the hash algorithm */ + nameAlgNbo = htons (tpmtPublic->nameAlg); + memcpy (name->name, (uint8_t *)&nameAlgNbo, sizeof (TPMI_ALG_HASH)); + /* set the size */ + name->size = sizeInBytes + sizeof (TPMI_ALG_HASH); + + leave: return rc; } @@ -694,7 +711,7 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s, && symdef->mode.aes == TPM_ALG_CFB) { TPMT_HA hash; - const int hlen = TSS_GetDigestSize (nalg); + int hlen; BYTE *digest; BYTE *s2b; int32_t size; @@ -706,6 +723,14 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s, * the AES routines alter the passed in iv */ memset (null_iv, 0, sizeof (null_iv)); + hlen = TSS_GetDigestSize (nalg); + if (hlen < 0) + { + log_error ("%s: unknown symmetric algo id %d\n", + "TSS_GetDigestSize", (int)nalg); + return TPM_RC_SYMMETRIC; + } + /* reserve space for hash before the encrypted sensitive */ digest = buf; bsize = sizeof (uint16_t /* TPM2B.size */) + hlen; @@ -765,7 +790,7 @@ TPM_RC tpm2_SensitiveToDuplicate (TPMT_SENSITIVE *s, } else { - log_error ("Unknown symmetric algorithm\n"); + log_error ("tss: Unknown symmetric algorithm\n"); return TPM_RC_SYMMETRIC; } @@ -795,7 +820,9 @@ tpm2_import_key (ctrl_t ctrl, TSS_CONTEXT *tssc, TPM_RC rc; uint32_t size; - uint16_t len; + uint16_t u16len; + size_t len; + int dlen; BYTE *buffer; int ret; char *passphrase; @@ -817,14 +844,22 @@ tpm2_import_key (ctrl_t ctrl, TSS_CONTEXT *tssc, /* add an authorization password to the key which the TPM will check */ - ret = pin_cb (ctrl, _("Please enter the TPM Authorization passphrase for the key."), &passphrase); + ret = pin_cb (ctrl, + _("Please enter the TPM Authorization passphrase for the key."), + &passphrase); if (ret) return ret; len = strlen(passphrase); - if (len > TSS_GetDigestSize(objectPublic.publicArea.nameAlg)) + dlen = TSS_GetDigestSize(objectPublic.publicArea.nameAlg); + if (dlen < 0) + { + log_error ("%s: error getting digest size\n", "TSS_GetDigestSize"); + return GPG_ERR_DIGEST_ALGO; + } + if (len > dlen) { - len = TSS_GetDigestSize(objectPublic.publicArea.nameAlg); - log_error ("Truncating Passphrase to TPM allowed %d\n", len); + len = dlen; + log_info ("tss: truncating Passphrase to TPM allowed size of %zu\n", len); } VAL_2B (s.authValue, size) = len; memcpy (VAL_2B (s.authValue, buffer), passphrase, len); @@ -885,16 +920,16 @@ tpm2_import_key (ctrl_t ctrl, TSS_CONTEXT *tssc, size = sizeof (pub); buffer = pub; - len = 0; + u16len = 0; TSS_TPM2B_PUBLIC_Marshal (&objectPublic, - &len, &buffer, &size); + &u16len, &buffer, &size); pub_len = len; size = sizeof (priv); buffer = priv; - len = 0; + u16len = 0; TSS_TPM2B_PRIVATE_Marshal ((TPM2B_PRIVATE *)&outPrivate, - &len, &buffer, &size); + &u16len, &buffer, &size); priv_len = len; *shadow_info = make_tpm2_shadow_info (parent, pub, pub_len, -- 2.45.2 From bcc002cd45d1c6bd51c2b2093f92d396970c082e Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 28 May 2024 13:46:36 +0200 Subject: [PATCH GnuPG] gpg: Avoid a double free on error in the key generation. * g10/keygen.c (card_store_key_with_backup): Avoid double free and simplify error handling. -- This is part of GnuPG-bug-id: 7129 Co-authored-by: Jakub Jelen --- g10/keygen.c | 53 +++++++++++++++++++++++----------------------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/g10/keygen.c b/g10/keygen.c index 4bdb9f53a..0c37f27f8 100644 --- a/g10/keygen.c +++ b/g10/keygen.c @@ -5846,11 +5846,10 @@ static gpg_error_t card_store_key_with_backup (ctrl_t ctrl, PKT_public_key *sub_psk, const char *backup_dir) { + gpg_error_t err; PKT_public_key *sk; gnupg_isotime_t timestamp; - gpg_error_t err; - char *hexgrip; - int rc; + char *hexgrip = NULL; struct agent_card_info_s info; gcry_cipher_hd_t cipherhd = NULL; char *cache_nonce = NULL; @@ -5858,9 +5857,14 @@ card_store_key_with_backup (ctrl_t ctrl, PKT_public_key *sub_psk, size_t keklen; char *ecdh_param_str = NULL; + memset (&info, 0, sizeof (info)); + sk = copy_public_key (NULL, sub_psk); if (!sk) - return gpg_error_from_syserror (); + { + err = gpg_error_from_syserror (); + goto leave; + } epoch2isotime (timestamp, (time_t)sk->timestamp); if (sk->pubkey_algo == PUBKEY_ALGO_ECDH) @@ -5868,37 +5872,23 @@ card_store_key_with_backup (ctrl_t ctrl, PKT_public_key *sub_psk, ecdh_param_str = ecdh_param_str_from_pk (sk); if (!ecdh_param_str) { - free_public_key (sk); - return gpg_error_from_syserror (); + err = gpg_error_from_syserror (); + goto leave; } } err = hexkeygrip_from_pk (sk, &hexgrip); if (err) - { - xfree (ecdh_param_str); - free_public_key (sk); - goto leave; - } + goto leave; - memset(&info, 0, sizeof (info)); - rc = agent_scd_getattr ("SERIALNO", &info); - if (rc) - { - xfree (ecdh_param_str); - free_public_key (sk); - err = (gpg_error_t)rc; - goto leave; - } + err = agent_scd_getattr ("SERIALNO", &info); + if (err) + goto leave; - rc = agent_keytocard (hexgrip, 2, 1, info.serialno, - timestamp, ecdh_param_str); - xfree (info.serialno); - if (rc) - { - err = (gpg_error_t)rc; - goto leave; - } + err = agent_keytocard (hexgrip, 2, 1, info.serialno, + timestamp, ecdh_param_str); + if (err) + goto leave; err = agent_keywrap_key (ctrl, 1, &kek, &keklen); if (err) @@ -5931,10 +5921,13 @@ card_store_key_with_backup (ctrl_t ctrl, PKT_public_key *sub_psk, if (err) log_error ("writing card key to backup file: %s\n", gpg_strerror (err)); else - /* Remove secret key data in agent side. */ - agent_scd_learn (NULL, 1); + { + /* Remove secret key data in agent side. */ + agent_scd_learn (NULL, 1); + } leave: + xfree (info.serialno); xfree (ecdh_param_str); xfree (cache_nonce); gcry_cipher_close (cipherhd); -- 2.45.2 From 021c27510b52f86a95ae70b5f4ed5d2c3886c3e8 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 28 May 2024 13:54:57 +0200 Subject: [PATCH GnuPG] wks: Make sure that ERR is always initialized. * tools/wks-util.c (install_key_from_spec_file): Initialize ERR in case the loop is never run. -- This is part of GnuPG-bug-id: 7129 Co-authored-by: Jakub Jelen --- tools/wks-util.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/wks-util.c b/tools/wks-util.c index 4a15d672a..fed568873 100644 --- a/tools/wks-util.c +++ b/tools/wks-util.c @@ -1185,6 +1185,7 @@ install_key_from_spec_file (const char *fname) goto leave; } + err = 0; while (es_read_line (fp, &line, &linelen, &maxlen) > 0) { if (!maxlen) -- 2.45.2 From fdc5003956407da1984f40fc27115e4704587e15 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 28 May 2024 16:37:25 +0200 Subject: [PATCH GnuPG] agent: Make sure to return success in ephemeral store mode. * agent/genkey.c (store_key): Clear ERR on success. -- This fixes a real problem which might let ephemeral store mode fail randomly. This is part of GnuPG-bug-id: 7129 Co-authored-by: Jakub Jelen --- agent/genkey.c | 1 + 1 file changed, 1 insertion(+) diff --git a/agent/genkey.c b/agent/genkey.c index 503a7eb53..bb7e74e9b 100644 --- a/agent/genkey.c +++ b/agent/genkey.c @@ -116,6 +116,7 @@ store_key (ctrl_t ctrl, gcry_sexp_t private, ek->keybuf = buf; buf = NULL; ek->keybuflen = len; + err = 0; } else err = agent_write_private_key (ctrl, grip, buf, len, force, -- 2.45.2 From bdbf5cee2ff5bc0773011abde4074003ef9dac70 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 28 May 2024 16:44:18 +0200 Subject: [PATCH GnuPG] agent: Avoid double free of empty string in the PIN caching. * agent/call-scd.c (handle_pincache_get): Set PIN to NULL. Also add DBG_CACHE conditionals and don't return the pin in the debug output. -- This is part of GnuPG-bug-id: 7129 Co-authored-by: Jakub Jelen --- agent/call-scd.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/agent/call-scd.c b/agent/call-scd.c index 3da16e619..ed6bd687e 100644 --- a/agent/call-scd.c +++ b/agent/call-scd.c @@ -196,7 +196,8 @@ handle_pincache_get (const char *args, assuan_context_t ctx) const char *key; char *pin = NULL; - log_debug ("%s: enter '%s'\n", __func__, args); + if (DBG_CACHE) + log_debug ("%s: enter '%s'\n", __func__, args); key = args; if (strlen (key) < 5) { @@ -210,11 +211,14 @@ handle_pincache_get (const char *args, assuan_context_t ctx) if (!pin || !*pin) { xfree (pin); + pin = NULL; err = 0; /* Not found is indicated by sending no data back. */ - log_debug ("%s: not cached\n", __func__); + if (DBG_CACHE) + log_debug ("%s: not cached\n", __func__); goto leave; } - log_debug ("%s: cache returned '%s'\n", __func__, pin); + if (DBG_CACHE) + log_debug ("%s: cache returned '%s'\n", __func__, "[hidden]"/*pin*/); err = assuan_send_data (ctx, pin, strlen (pin)); leave: -- 2.45.2 From 379fc5569d604c4a7b5f12b2bbfc4106893c2a9e Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 28 May 2024 16:50:59 +0200 Subject: [PATCH GnuPG] agent: Avoid uninitialized access in GENKEY command on parameter error. * agent/command.c (cmd_genkey): Moved init_membuf to the top. -- Signed-off-by: Jakub Jelen This is part of GnuPG-bug-id: 7129 --- agent/command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/command.c b/agent/command.c index b152a5ea4..417a8815f 100644 --- a/agent/command.c +++ b/agent/command.c @@ -1170,6 +1170,8 @@ cmd_genkey (assuan_context_t ctx, char *line) int c; unsigned int flags = 0; + init_membuf (&outbuf, 512); + if (ctrl->restricted) return leave_cmd (ctx, gpg_error (GPG_ERR_FORBIDDEN)); @@ -1227,8 +1229,6 @@ cmd_genkey (assuan_context_t ctx, char *line) if (rc) goto leave; - init_membuf (&outbuf, 512); - /* If requested, ask for the password to be used for the key. If this is not used the regular Pinentry mechanism is used. */ if (opt_inq_passwd && !(flags & GENKEY_FLAG_NO_PROTECTION)) -- 2.45.2 From 4c1b0070354db0b9b0516d9e5453e47fc03a0aac Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 28 May 2024 16:57:41 +0200 Subject: [PATCH GnuPG] scd: Avoid buffer overrun with more than 16 PC/SC readers. * scd/apdu.c (apdu_dev_list_start): Fix end condition. -- Signed-off-by: Jakub Jelen This is part of GnuPG-bug-id: 7129 Fixes-commit: e8534f899915a039610973a84042cbe25a5e7ce2 --- scd/apdu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scd/apdu.c b/scd/apdu.c index 98158648b..2e38a273a 100644 --- a/scd/apdu.c +++ b/scd/apdu.c @@ -2094,7 +2094,7 @@ apdu_dev_list_start (const char *portstr, struct dev_list **l_p) nreader -= n + 1; p += n + 1; dl->idx_max++; - if (dl->idx_max > MAX_READER) + if (dl->idx_max >= MAX_READER) { log_error ("too many readers from pcsc_list_readers\n"); dl->idx_max--; -- 2.45.2 From 28c705a3be5c4aec477719d652b503568abc88a0 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 28 May 2024 17:08:12 +0200 Subject: [PATCH GnuPG] gpgsm: Silence a lint warning * sm/keydb.c (keydb_search): Init skipped. -- Skipped is not actually used. This is part of GnuPG-bug-id: 7129 Reported-by: Jakub Jelen --- sm/keydb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sm/keydb.c b/sm/keydb.c index 151ae8103..9d704a110 100644 --- a/sm/keydb.c +++ b/sm/keydb.c @@ -1611,7 +1611,7 @@ keydb_search (ctrl_t ctrl, KEYDB_HANDLE hd, KEYDB_SEARCH_DESC *desc, size_t ndesc) { gpg_error_t err = gpg_error (GPG_ERR_EOF); - unsigned long skipped; + unsigned long skipped = 0; int i; if (!hd) -- 2.45.2 From dcb0b6fd4822107d68bcb046d4d0650d02c82522 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 28 May 2024 17:15:03 +0200 Subject: [PATCH GnuPG] gpgsm: Avoid double free when checking rsaPSS signatures. * sm/certcheck.c (gpgsm_check_cms_signature): Do not free s_sig on error. Its owned and freed by the caller. -- This is part of GnuPG-bug-id: 7129 Signed-off-by: Jakub Jelen Fixes-commit: 969abcf40cdfc65f3ee859c5e62889e1a8ccde91 --- sm/certcheck.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/sm/certcheck.c b/sm/certcheck.c index f4db858c3..ef1b6ec54 100644 --- a/sm/certcheck.c +++ b/sm/certcheck.c @@ -630,13 +630,11 @@ gpgsm_check_cms_signature (ksba_cert_t cert, gcry_sexp_t s_sig, rc = extract_pss_params (s_sig, &algo, &saltlen); if (rc) { - gcry_sexp_release (s_sig); return rc; } if (algo != mdalgo) { log_error ("PSS hash algo mismatch (%d/%d)\n", mdalgo, algo); - gcry_sexp_release (s_sig); return gpg_error (GPG_ERR_DIGEST_ALGO); } } -- 2.45.2 From 9adaa79ab43e2f87178b8ee5ab1a353cba384606 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 28 May 2024 17:19:37 +0200 Subject: [PATCH GnuPG] gpg-auth: Fix use after free. * tools/gpg-auth.c (ssh_authorized_keys): Move free after printing error message. -- Signed-off-by: Jakub Jelen This is part of GnuPG-bug-id: 7129 --- tools/gpg-auth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/gpg-auth.c b/tools/gpg-auth.c index a818bee5d..c47bb4e54 100644 --- a/tools/gpg-auth.c +++ b/tools/gpg-auth.c @@ -818,7 +818,6 @@ ssh_authorized_keys (const char *user, struct ssh_key_list **r_ssh_key_list) xfree (fname); return err; } - xfree (fname); maxlen = 2048; /* Set limit. */ while ((len = es_read_line (fp, &line, &length_of_line, &maxlen)) > 0) @@ -861,6 +860,7 @@ ssh_authorized_keys (const char *user, struct ssh_key_list **r_ssh_key_list) *r_ssh_key_list = ssh_key_list; leave: + xfree (fname); xfree (line); es_fclose (fp); return err; -- 2.45.2 From 62695cd1c080d6cf0557d95d4977bc69c9d1de61 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Thu, 4 Jul 2024 13:57:44 +0200 Subject: [PATCH GnuPG] tpm2: Fix key import * tpm2d/tpm2.c (tpm2_import_key): Set the lengths from right variables. -- Signed-off-by: Jakub Jelen Fixes-commit: d631c8198c254107c0a4e704511fa0f33d3dda5f --- tpm2d/tpm2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tpm2d/tpm2.c b/tpm2d/tpm2.c index 2a49bf99b..a4677fb98 100644 --- a/tpm2d/tpm2.c +++ b/tpm2d/tpm2.c @@ -923,14 +923,14 @@ tpm2_import_key (ctrl_t ctrl, TSS_CONTEXT *tssc, u16len = 0; TSS_TPM2B_PUBLIC_Marshal (&objectPublic, &u16len, &buffer, &size); - pub_len = len; + pub_len = u16len; size = sizeof (priv); buffer = priv; u16len = 0; TSS_TPM2B_PRIVATE_Marshal ((TPM2B_PRIVATE *)&outPrivate, &u16len, &buffer, &size); - priv_len = len; + priv_len = u16len; *shadow_info = make_tpm2_shadow_info (parent, pub, pub_len, priv, priv_len, shadow_len); -- 2.45.2