From 334e993a71d3abb7d30cb5ee05d578cecf0c3f67 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Sat, 11 Jun 2016 20:42:28 +0200 Subject: [PATCH] gpg: Remove C-99ism, re-indent, and simplify one function. * g10/call-agent.c (struct keyinfo_data): Rename to keyinfo_data_parm_s. (agent_get_keyinfo): Replace C-99 style init. (keyinfo_status_cb): Use new fucntion split_fields. * g10/export.c (match_curve_skey_pk): Add missings returns error cases. (cleartext_secret_key_to_openpgp): Better clear PK->PKEY first. -- Signed-off-by: Werner Koch --- g10/call-agent.c | 47 ++++++++---------- g10/export.c | 122 ++++++++++++++++++++++++++++++----------------- g10/keygen.c | 12 +++-- g10/keylist.c | 3 +- 4 files changed, 107 insertions(+), 77 deletions(-) diff --git a/g10/call-agent.c b/g10/call-agent.c index 06a2d8678..a023654bc 100644 --- a/g10/call-agent.c +++ b/g10/call-agent.c @@ -1671,44 +1671,35 @@ agent_probe_any_secret_key (ctrl_t ctrl, kbnode_t keyblock) -struct keyinfo_data { +struct keyinfo_data_parm_s +{ char *serialno; int cleartext; }; + static gpg_error_t keyinfo_status_cb (void *opaque, const char *line) { - struct keyinfo_data *data = opaque; + struct keyinfo_data_parm_s *data = opaque; int is_smartcard; - const char *s, *s2; + char *s; if ((s = has_leading_keyword (line, "KEYINFO")) && data) { - s = strchr (s, ' '); - if (s) + /* Parse the arguments: + * 0 1 2 3 4 5 + * + */ + char *fields[6]; + + if (split_fields (s, fields, DIM (fields)) == 6) { - is_smartcard = (s[1] == 'T'); - if ( s[2] == ' ' && s[3] ) - { - s += 3; - s2 = strchr (s, ' '); - if ( s2 > s ) - { - if (is_smartcard && !data->serialno) - { - data->serialno = xtrymalloc ((s2 - s)+1); - if (data->serialno) - { - memcpy (data->serialno, s, s2 - s); - (data->serialno)[s2 - s] = 0; - } - } - if (s2 = strchr (s2 + 1, ' '), s2) /* skip IDSTR (can IDSTR contain a space?) */ - if (s2 = strchr (s2 + 1, ' '), s2) /* skip CACHED */ - data->cleartext = (s2[1] == 'C'); /* 'P' for protected, 'C' for clear */ - } - } + is_smartcard = (fields[1][0] == 'T'); + if (is_smartcard && !data->serialno && strcmp (fields[2], "-")) + data->serialno = xtrystrdup (fields[2]); + /* 'P' for protected, 'C' for clear */ + data->cleartext = (fields[5][0] == 'C'); } } return 0; @@ -1728,7 +1719,9 @@ agent_get_keyinfo (ctrl_t ctrl, const char *hexkeygrip, { gpg_error_t err; char line[ASSUAN_LINELENGTH]; - struct keyinfo_data keyinfo = { .serialno = NULL, .cleartext = 0 }; + struct keyinfo_data_parm_s keyinfo; + + memset (&keyinfo, 0,sizeof keyinfo); *r_serialno = NULL; diff --git a/g10/export.c b/g10/export.c index 870cb458e..b067376e1 100644 --- a/g10/export.c +++ b/g10/export.c @@ -390,28 +390,32 @@ exact_subkey_match_p (KEYDB_SEARCH_DESC *desc, KBNODE node) return result; } -/* return an error if the key represented by the S-expression s_key - and the OpenPGP key represented by pk do not use the same curve. */ + +/* Return an error if the key represented by the S-expression S_KEY + * and the OpenPGP key represented by PK do not use the same curve. */ static gpg_error_t match_curve_skey_pk (gcry_sexp_t s_key, PKT_public_key *pk) { - gcry_sexp_t curve = NULL, flags = NULL; - char *curve_str = NULL, *flag; + gcry_sexp_t curve = NULL; + gcry_sexp_t flags = NULL; + char *curve_str = NULL; + char *flag; const char *oidstr = NULL; gcry_mpi_t curve_as_mpi = NULL; gpg_error_t err; - int is_eddsa = 0, idx = 0; + int is_eddsa = 0; + int idx = 0; - if (!(pk->pubkey_algo==PUBKEY_ALGO_ECDH || - pk->pubkey_algo==PUBKEY_ALGO_ECDSA || - pk->pubkey_algo==PUBKEY_ALGO_EDDSA)) + if (!(pk->pubkey_algo==PUBKEY_ALGO_ECDH + || pk->pubkey_algo==PUBKEY_ALGO_ECDSA + || pk->pubkey_algo==PUBKEY_ALGO_EDDSA)) return gpg_error (GPG_ERR_PUBKEY_ALGO); curve = gcry_sexp_find_token (s_key, "curve", 0); if (!curve) { log_error ("no reported curve\n"); - err = gpg_error (GPG_ERR_UNKNOWN_CURVE); + return gpg_error (GPG_ERR_UNKNOWN_CURVE); } curve_str = gcry_sexp_nth_string (curve, 1); gcry_sexp_release (curve); curve = NULL; @@ -424,30 +428,32 @@ match_curve_skey_pk (gcry_sexp_t s_key, PKT_public_key *pk) if (!oidstr) { log_error ("no OID known for curve '%s'\n", curve_str); - gcry_free (curve_str); + xfree (curve_str); return gpg_error (GPG_ERR_UNKNOWN_CURVE); } - gcry_free (curve_str); + xfree (curve_str); err = openpgp_oid_from_str (oidstr, &curve_as_mpi); if (err) return err; - if (gcry_mpi_cmp(pk->pkey[0], curve_as_mpi)) + if (gcry_mpi_cmp (pk->pkey[0], curve_as_mpi)) { log_error ("curves do not match\n"); - err = gpg_error (GPG_ERR_INV_CURVE); + gcry_mpi_release (curve_as_mpi); + return gpg_error (GPG_ERR_INV_CURVE); } gcry_mpi_release (curve_as_mpi); flags = gcry_sexp_find_token (s_key, "flags", 0); if (flags) - for (idx = 1; idx < gcry_sexp_length (flags); idx++) - { - flag = gcry_sexp_nth_string (flags, idx); - if (flag && (strcmp ("eddsa", flag) == 0)) - is_eddsa = 1; - gcry_free (flag); - } - if (is_eddsa != - (pk->pubkey_algo==PUBKEY_ALGO_EDDSA)) + { + for (idx = 1; idx < gcry_sexp_length (flags); idx++) + { + flag = gcry_sexp_nth_string (flags, idx); + if (flag && (strcmp ("eddsa", flag) == 0)) + is_eddsa = 1; + gcry_free (flag); + } + } + if (is_eddsa != (pk->pubkey_algo == PUBKEY_ALGO_EDDSA)) { log_error ("disagreement about EdDSA\n"); err = gpg_error (GPG_ERR_INV_CURVE); @@ -456,6 +462,7 @@ match_curve_skey_pk (gcry_sexp_t s_key, PKT_public_key *pk) return err; } + /* Return a canonicalized public key algoithms. This is used to compare different flavors of algorithms (e.g. ELG and ELG_E are considered the same). */ @@ -476,8 +483,9 @@ canon_pk_algo (enum gcry_pk_algos algo) } } -/* take a cleartext dump of a secret key in PK and change the - parameter array in PK to include the secret parameters. */ + +/* Take a cleartext dump of a secret key in PK and change the + * parameter array in PK to include the secret parameters. */ static gpg_error_t cleartext_secret_key_to_openpgp (gcry_sexp_t s_key, PKT_public_key *pk) { @@ -503,7 +511,7 @@ cleartext_secret_key_to_openpgp (gcry_sexp_t s_key, PKT_public_key *pk) key_type = gcry_sexp_nth_string(key, 0); pk_algo = gcry_pk_map_name (key_type); - log_assert(pk->seckey_info == NULL); + log_assert (!pk->seckey_info); pk->seckey_info = ski = xtrycalloc (1, sizeof *ski); if (!ski) @@ -525,15 +533,24 @@ cleartext_secret_key_to_openpgp (gcry_sexp_t s_key, PKT_public_key *pk) if (gcry_mpi_cmp(pk->pkey[idx], pub_params[idx])) err = gpg_error (GPG_ERR_BAD_PUBKEY); if (!err) - err = gcry_sexp_extract_param (key, NULL, "dpqu", - &pk->pkey[2], - &pk->pkey[3], - &pk->pkey[4], - &pk->pkey[5], - NULL); + { + for (idx = 2; idx < 6 && !err; idx++) + { + gcry_mpi_release (pk->pkey[idx]); + pk->pkey[idx] = NULL; + } + err = gcry_sexp_extract_param (key, NULL, "dpqu", + &pk->pkey[2], + &pk->pkey[3], + &pk->pkey[4], + &pk->pkey[5], + NULL); + } if (!err) - for (idx = 2; idx < 6; idx++) - ski->csum += checksum_mpi (pk->pkey[idx]); + { + for (idx = 2; idx < 6; idx++) + ski->csum += checksum_mpi (pk->pkey[idx]); + } break; case GCRY_PK_DSA: @@ -549,9 +566,13 @@ cleartext_secret_key_to_openpgp (gcry_sexp_t s_key, PKT_public_key *pk) if (gcry_mpi_cmp(pk->pkey[idx], pub_params[idx])) err = gpg_error (GPG_ERR_BAD_PUBKEY); if (!err) - err = gcry_sexp_extract_param (key, NULL, "x", - &pk->pkey[4], - NULL); + { + gcry_mpi_release (pk->pkey[4]); + pk->pkey[4] = NULL; + err = gcry_sexp_extract_param (key, NULL, "x", + &pk->pkey[4], + NULL); + } if (!err) ski->csum += checksum_mpi (pk->pkey[4]); break; @@ -568,9 +589,13 @@ cleartext_secret_key_to_openpgp (gcry_sexp_t s_key, PKT_public_key *pk) if (gcry_mpi_cmp(pk->pkey[idx], pub_params[idx])) err = gpg_error (GPG_ERR_BAD_PUBKEY); if (!err) - err = gcry_sexp_extract_param (key, NULL, "x", - &pk->pkey[3], - NULL); + { + gcry_mpi_release (pk->pkey[3]); + pk->pkey[3] = NULL; + err = gcry_sexp_extract_param (key, NULL, "x", + &pk->pkey[3], + NULL); + } if (!err) ski->csum += checksum_mpi (pk->pkey[3]); break; @@ -590,9 +615,13 @@ cleartext_secret_key_to_openpgp (gcry_sexp_t s_key, PKT_public_key *pk) if (pk->pubkey_algo == PUBKEY_ALGO_ECDH) sec_start += 1; if (!err) - err = gcry_sexp_extract_param (key, NULL, "d", - &pk->pkey[sec_start], - NULL); + { + gcry_mpi_release (pk->pkey[sec_start]); + pk->pkey[sec_start] = NULL; + err = gcry_sexp_extract_param (key, NULL, "d", + &pk->pkey[sec_start], + NULL); + } if (!err) ski->csum += checksum_mpi (pk->pkey[sec_start]); @@ -600,9 +629,11 @@ cleartext_secret_key_to_openpgp (gcry_sexp_t s_key, PKT_public_key *pk) default: pk->seckey_info = NULL; - free (ski); + xfree (ski); err = gpg_error (GPG_ERR_NOT_IMPLEMENTED); + break; } + leave: gcry_sexp_release (top_list); gcry_sexp_release (key); @@ -621,6 +652,7 @@ cleartext_secret_key_to_openpgp (gcry_sexp_t s_key, PKT_public_key *pk) goto leave; } + /* Use the key transfer format given in S_PGP to create the secinfo structure in PK and change the parameter array in PK to include the secret parameters. */ @@ -1042,8 +1074,8 @@ print_status_exported (PKT_public_key *pk) * Then, parse the decrypted key data in transfer format, and put * secret parameters into PK. * - * if CLEARTEXT is 0, store the secret key material - * passphrase-protected. otherwise, store secret key material in the + * If CLEARTEXT is 0, store the secret key material + * passphrase-protected. Otherwise, store secret key material in the * clear. * * CACHE_NONCE_ADDR is used to share nonce for multple key retrievals. diff --git a/g10/keygen.c b/g10/keygen.c index afb13e057..74fd37052 100644 --- a/g10/keygen.c +++ b/g10/keygen.c @@ -4066,19 +4066,23 @@ card_store_key_with_backup (ctrl_t ctrl, PKT_public_key *sub_psk, goto leave; } - err = gcry_cipher_open (&cipherhd, GCRY_CIPHER_AES128, GCRY_CIPHER_MODE_AESWRAP, 0); + err = gcry_cipher_open (&cipherhd, GCRY_CIPHER_AES128, + GCRY_CIPHER_MODE_AESWRAP, 0); if (!err) err = gcry_cipher_setkey (cipherhd, kek, keklen); if (err) { - log_error ("error setting up an encryption context: %s\n", gpg_strerror (err)); + log_error ("error setting up an encryption context: %s\n", + gpg_strerror (err)); goto leave; } - err = receive_seckey_from_agent (ctrl, cipherhd, 0, &cache_nonce, hexgrip, sk); + err = receive_seckey_from_agent (ctrl, cipherhd, 0, + &cache_nonce, hexgrip, sk); if (err) { - log_error ("error getting secret key from agent: %s\n", gpg_strerror (err)); + log_error ("error getting secret key from agent: %s\n", + gpg_strerror (err)); goto leave; } diff --git a/g10/keylist.c b/g10/keylist.c index 973a9fb9d..d77c86b8b 100644 --- a/g10/keylist.c +++ b/g10/keylist.c @@ -1354,7 +1354,8 @@ list_keyblock_colon (ctrl_t ctrl, kbnode_t keyblock, log_error ("error computing a keygrip: %s\n", gpg_strerror (rc)); } stubkey = 0; - if ((secret||has_secret) && agent_get_keyinfo (NULL, hexgrip, &serialno, NULL)) + if ((secret || has_secret) + && agent_get_keyinfo (NULL, hexgrip, &serialno, NULL)) stubkey = 1; /* Key not found. */ keyid_from_pk (pk, keyid);