From 2dd6b4b998dd6e156e2e75ede0f40fb768c69f40 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Mon, 13 Jan 2020 18:24:01 +0100 Subject: [PATCH] scd: Make the PIN cache robust against wrongdoing of gpg-agent. * scd/app-openpgp.c (struct app_local_s): New field pincache. (cache_pin): Set it. (pin_from_cache): Consult it. * scd/app-piv.c (struct app_local_s): New field pincache. (cache_pin): Set it. (pin_from_cache): Consult it. -- Signed-off-by: Werner Koch --- scd/app-openpgp.c | 37 +++++++++++++++++++++++++++++++++--- scd/app-piv.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/scd/app-openpgp.c b/scd/app-openpgp.c index ad88eb619..293c53cb5 100644 --- a/scd/app-openpgp.c +++ b/scd/app-openpgp.c @@ -215,6 +215,18 @@ struct app_local_s { unsigned int def_chv2:1; /* Use 123456 for CHV2. */ } flags; + + /* Keep track on whether we cache a certain PIN so that we get it + * from the cache only if we know we cached it. This inhibits the + * use of the same cache entry for a card plugged in and out without + * gpg-agent having noticed that due to a bug. */ + struct + { + unsigned int maybe_chv1:1; + unsigned int maybe_chv2:1; + unsigned int maybe_chv3:1; + } pincache; + /* Pinpad request specified on card. */ struct { @@ -223,8 +235,8 @@ struct app_local_s { int fixedlen_admin; } pinpad; - struct - { + struct + { key_type_t key_type; union { struct { @@ -240,6 +252,7 @@ struct app_local_s { } ecc; }; } keyattr[3]; + }; #define ECC_FLAG_DJB_TWEAK (1 << 0) @@ -2227,6 +2240,13 @@ cache_pin (app_t app, ctrl_t ctrl, int chvno, const char *pin) pincache_put (ctrl, app_get_slot (app), "openpgp", keyref, pin, pin? strlen (pin):0); + + switch (chvno) + { + case 1: app->app_local->pincache.maybe_chv1 = !!pin; break; + case 2: app->app_local->pincache.maybe_chv2 = !!pin; break; + case 3: app->app_local->pincache.maybe_chv3 = !!pin; break; + } } @@ -2237,6 +2257,7 @@ static int pin_from_cache (app_t app, ctrl_t ctrl, int chvno, char **r_pin) { const char *keyref = chvno_to_keyref (chvno); + int maybe_cached; *r_pin = NULL; @@ -2248,6 +2269,17 @@ pin_from_cache (app_t app, ctrl_t ctrl, int chvno, char **r_pin) default: return 0; } + switch (chvno) + { + case 1: maybe_cached = app->app_local->pincache.maybe_chv1; break; + case 2: maybe_cached = app->app_local->pincache.maybe_chv2; break; + case 3: maybe_cached = app->app_local->pincache.maybe_chv3; break; + default: maybe_cached = 0; break; + } + + if (!maybe_cached) + return 0; + if (pincache_get (ctrl, app_get_slot (app), "openpgp", keyref, r_pin)) return 0; @@ -2255,7 +2287,6 @@ pin_from_cache (app_t app, ctrl_t ctrl, int chvno, char **r_pin) } - /* Verify a CHV either using the pinentry or if possible by using a pinpad. PINCB and PINCB_ARG describe the usual callback for the pinentry. CHVNO must be either 1 or 2. SIGCOUNT is only diff --git a/scd/app-piv.c b/scd/app-piv.c index d9e495ae2..31bde4666 100644 --- a/scd/app-piv.c +++ b/scd/app-piv.c @@ -213,6 +213,21 @@ struct app_local_s { unsigned int yubikey:1; /* This is on a Yubikey. */ } flags; + /* Keep track on whether we cache a certain PIN so that we get it + * from the cache only if we know we cached it. This inhibits the + * use of the same cache entry for a card plugged in and out without + * gpg-agent having noticed that due to a bug. */ + struct + { + unsigned int maybe_00:1; + unsigned int maybe_80:1; + unsigned int maybe_81:1; + unsigned int maybe_96:1; + unsigned int maybe_97:1; + unsigned int maybe_98:1; + unsigned int maybe_9B:1; + } pincache; + }; @@ -1726,8 +1741,22 @@ cache_pin (app_t app, ctrl_t ctrl, int pinno, default: return; } + + snprintf (pinref, sizeof pinref, "%02x", pinno); pincache_put (ctrl, app_get_slot (app), "piv", pinref, pin, pinlen); + + switch (pinno) + { + case 0x00: app->app_local->pincache.maybe_00 = !!pin; break; + case 0x80: app->app_local->pincache.maybe_80 = !!pin; break; + case 0x81: app->app_local->pincache.maybe_81 = !!pin; break; + case 0x96: app->app_local->pincache.maybe_96 = !!pin; break; + case 0x97: app->app_local->pincache.maybe_97 = !!pin; break; + case 0x98: app->app_local->pincache.maybe_98 = !!pin; break; + case 0x9B: app->app_local->pincache.maybe_9B = !!pin; break; + } + } @@ -1738,6 +1767,7 @@ static int pin_from_cache (app_t app, ctrl_t ctrl, int pinno, char **r_pin) { char pinref[20]; + int maybe_cached; *r_pin = NULL; @@ -1749,6 +1779,21 @@ pin_from_cache (app_t app, ctrl_t ctrl, int pinno, char **r_pin) default: return 0; } + switch (pinno) + { + case 0x00: maybe_cached = app->app_local->pincache.maybe_00; break; + case 0x80: maybe_cached = app->app_local->pincache.maybe_80; break; + case 0x81: maybe_cached = app->app_local->pincache.maybe_81; break; + case 0x96: maybe_cached = app->app_local->pincache.maybe_96; break; + case 0x97: maybe_cached = app->app_local->pincache.maybe_97; break; + case 0x98: maybe_cached = app->app_local->pincache.maybe_98; break; + case 0x9B: maybe_cached = app->app_local->pincache.maybe_9B; break; + default: maybe_cached = 0; + } + + if (!maybe_cached) + return 0; + snprintf (pinref, sizeof pinref, "%02x", pinno); if (pincache_get (ctrl, app_get_slot (app), "piv", pinref, r_pin)) return 0; @@ -1875,7 +1920,8 @@ ask_and_prepare_chv (app_t app, ctrl_t ctrl, default: return gpg_error (GPG_ERR_INV_ID); } - +#warning debug... + no_cache =0; /* Ask for the PIN. */ if (!no_cache && remaining >= 3 && pin_from_cache (app, ctrl, keyref, &pinvalue))