gpg: Provide better diagnostic for replaced card keys.

* agent/divert-scd.c (divert_pksign): Add arg 'grip'.  Replace OPENPGP
key reference to keygrips.
(divert_pkdecrypt): Ditto.
* agent/protect.c (parse_shadow_info): Trim spaces.
* agent/pkdecrypt.c (agent_pkdecrypt): Pass the keygrip.
* agent/pksign.c (agent_pksign_do): Ditto.

* g10/mainproc.c (print_pkenc_list): Print extra info for an invalid
id error.
* g10/sign.c (do_sign): Ditto.
--

Using the keygrip instead of the identifier works on OpenPGP cards and
thus we use that to make sure that we are working on the right card.
For other cards we better don't do that to avoid regressions.  Those
other cards are also usually provided and do not allow to
self-generate the keys.

Note that old versions of the code (gpg 1.4) used the fingerprint as
additional check but that was eventually removed and now that we use
the keygrip all over the place, it is best to use this to identify a
key.

Signed-off-by: Werner Koch <wk@gnupg.org>
This commit is contained in:
Werner Koch 2020-11-13 16:05:28 +01:00
parent aeed0b93ff
commit 5d98f95aa9
No known key found for this signature in database
GPG Key ID: E3FDFF218E45B72B
7 changed files with 51 additions and 5 deletions

View File

@ -545,10 +545,12 @@ void agent_reload_trustlist (void);
/*-- divert-scd.c --*/
int divert_pksign (ctrl_t ctrl, const char *desc_text,
const unsigned char *digest, size_t digestlen, int algo,
const unsigned char *grip,
const unsigned char *shadow_info, unsigned char **r_sig,
size_t *r_siglen);
int divert_pkdecrypt (ctrl_t ctrl, const char *desc_text,
const unsigned char *cipher,
const unsigned char *grip,
const unsigned char *shadow_info,
char **r_buf, size_t *r_len, int *r_padding);
int divert_generic_cmd (ctrl_t ctrl,

View File

@ -79,7 +79,7 @@ ask_for_card (ctrl_t ctrl, const unsigned char *shadow_info, char **r_kid)
rc = agent_card_serialno (ctrl, &serialno, want_sn);
if (!rc)
{
log_debug ("detected card with S/N %s\n", serialno);
log_info ("detected card with S/N %s\n", serialno);
i = strcmp (serialno, want_sn);
xfree (serialno);
serialno = NULL;
@ -93,13 +93,13 @@ ask_for_card (ctrl_t ctrl, const unsigned char *shadow_info, char **r_kid)
}
else if (gpg_err_code (rc) == GPG_ERR_ENODEV)
{
log_debug ("no device present\n");
log_info ("no device present\n");
rc = 0;
no_card = 1;
}
else if (gpg_err_code (rc) == GPG_ERR_CARD_NOT_PRESENT)
{
log_debug ("no card present\n");
log_info ("no card present\n");
rc = 0;
no_card = 2;
}
@ -436,6 +436,7 @@ getpin_cb (void *opaque, const char *desc_text, const char *info,
int
divert_pksign (ctrl_t ctrl, const char *desc_text,
const unsigned char *digest, size_t digestlen, int algo,
const unsigned char *grip,
const unsigned char *shadow_info, unsigned char **r_sig,
size_t *r_siglen)
{
@ -450,6 +451,22 @@ divert_pksign (ctrl_t ctrl, const char *desc_text,
if (rc)
return rc;
/* For OpenPGP cards we better use the keygrip as key reference.
* This has the advantage that app-openpgp can check that the stored
* key matches our expectation. This is important in case new keys
* have been created on the same card but the sub file has not been
* updated. In that case we would get a error from our final
* signature checking code or, if the pubkey algo is different,
* weird errors from the card (Conditions of use not satisfied). */
if (kid && grip && !strncmp (kid, "OPENPGP.", 8))
{
xfree (kid);
kid = bin2hex (grip, KEYGRIP_LEN, NULL);
if (!kid)
return gpg_error_from_syserror ();
}
if (algo == MD_USER_TLS_MD5SHA1)
{
int save = ctrl->use_auth_call;
@ -491,6 +508,7 @@ divert_pksign (ctrl_t ctrl, const char *desc_text,
int
divert_pkdecrypt (ctrl_t ctrl, const char *desc_text,
const unsigned char *cipher,
const unsigned char *grip,
const unsigned char *shadow_info,
char **r_buf, size_t *r_len, int *r_padding)
{
@ -585,6 +603,21 @@ divert_pkdecrypt (ctrl_t ctrl, const char *desc_text,
if (rc)
return rc;
/* For OpenPGP cards we better use the keygrip as key reference.
* This has the advantage that app-openpgp can check that the stored
* key matches our expectation. This is important in case new keys
* have been created on the same card but the sub file has not been
* updated. In that case we would get a error from our final
* signature checking code or, if the pubkey algo is different,
* weird errors from the card (Conditions of use not satisfied). */
if (kid && grip && !strncmp (kid, "OPENPGP.", 8))
{
xfree (kid);
kid = bin2hex (grip, KEYGRIP_LEN, NULL);
if (!kid)
return gpg_error_from_syserror ();
}
rc = agent_card_pkdecrypt (ctrl, kid, getpin_cb, ctrl, NULL,
ciphertext, ciphertextlen,
&plaintext, &plaintextlen, r_padding);

View File

@ -86,7 +86,8 @@ agent_pkdecrypt (ctrl_t ctrl, const char *desc_text,
goto leave;
}
rc = divert_pkdecrypt (ctrl, desc_text, ciphertext, shadow_info,
rc = divert_pkdecrypt (ctrl, desc_text, ciphertext,
ctrl->keygrip, shadow_info,
&buf, &len, r_padding);
if (rc)
{

View File

@ -356,6 +356,7 @@ agent_pksign_do (ctrl_t ctrl, const char *cache_nonce,
err = divert_pksign (ctrl, desc2? desc2 : desc_text,
data, datalen,
ctrl->digest.algo,
ctrl->keygrip,
shadow_info, &buf, &len);
xfree (desc2);
}

View File

@ -1725,6 +1725,7 @@ parse_shadow_info (const unsigned char *shadow_info,
}
memcpy (*r_idstr, s, n);
(*r_idstr)[n] = 0;
trim_spaces (*r_idstr);
}
/* Parse the optional PINLEN. */

View File

@ -618,6 +618,9 @@ print_pkenc_list (ctrl_t ctrl, struct kidlist_item *list, int failed)
{
log_info (_("public key decryption failed: %s\n"),
gpg_strerror (list->reason));
if (gpg_err_source (list->reason) == GPG_ERR_SOURCE_SCD
&& gpg_err_code (list->reason) == GPG_ERR_INV_ID)
print_further_info ("a reason might be a card with replaced keys");
write_status_error ("pkdecrypt_failed", list->reason);
}
}

View File

@ -462,7 +462,12 @@ do_sign (ctrl_t ctrl, PKT_public_key *pksk, PKT_signature *sig,
leave:
if (err)
log_error (_("signing failed: %s\n"), gpg_strerror (err));
{
log_error (_("signing failed: %s\n"), gpg_strerror (err));
if (gpg_err_source (err) == GPG_ERR_SOURCE_SCD
&& gpg_err_code (err) == GPG_ERR_INV_ID)
print_further_info ("a reason might be a card with replaced keys");
}
else
{
if (opt.verbose)