diff --git a/agent/cache.c b/agent/cache.c index 8a6c43a30..c5c67e320 100644 --- a/agent/cache.c +++ b/agent/cache.c @@ -446,7 +446,8 @@ agent_put_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode, } -/* Try to find an item in the cache. */ +/* Try to find an item in the cache. Returns NULL if not found or an + * malloced string with the value. */ char * agent_get_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode) { diff --git a/agent/call-scd.c b/agent/call-scd.c index d10bde835..cacf1f483 100644 --- a/agent/call-scd.c +++ b/agent/call-scd.c @@ -637,13 +637,9 @@ static gpg_error_t handle_pincache_put (const char *args) { gpg_error_t err; - const char *s, *key, *hexwrappedpin; + const char *s, *key, *pin; char *keybuf = NULL; - unsigned char *wrappedpin = NULL; - size_t keylen, hexwrappedpinlen, wrappedpinlen; - char *value = NULL; - size_t valuelen; - gcry_cipher_hd_t cipherhd = NULL; + size_t keylen; key = s = args; while (*s && !spacep (s)) @@ -657,7 +653,6 @@ handle_pincache_put (const char *args) goto leave; } - keybuf = xtrymalloc (keylen+1); if (!keybuf) { @@ -670,70 +665,23 @@ handle_pincache_put (const char *args) while (spacep (s)) s++; - hexwrappedpin = s; - while (*s && !spacep (s)) - s++; - hexwrappedpinlen = s - hexwrappedpin; - if (!hexwrappedpinlen) + pin = s; + if (!*pin) { - /* Flush the cache. The cache module knows aboput the structure - * of the key to flush only parts. */ + /* No value - flush the cache. The cache module knows aboput + * the structure of the key to flush only parts. */ log_debug ("%s: flushing cache '%s'\n", __func__, key); agent_put_cache (NULL, key, CACHE_MODE_PIN, NULL, -1); err = 0; goto leave; } - if (hexwrappedpinlen < 2*24) - { - log_error ("%s: ignoring request with too short cryptogram\n", __func__); - err = 0; - goto leave; - } - wrappedpinlen = hexwrappedpinlen / 2; - wrappedpin = xtrymalloc (wrappedpinlen); - if (!wrappedpin) - { - err = gpg_error_from_syserror (); - goto leave; - } - if (hex2bin (hexwrappedpin, wrappedpin, wrappedpinlen) == -1) - { - log_error ("%s: invalid hex length\n", __func__); - err = gpg_error (GPG_ERR_INV_LENGTH); - goto leave; - } - - valuelen = wrappedpinlen - 8; - value = xtrymalloc_secure (valuelen+1); - if (!value) - { - err = gpg_error_from_syserror (); - goto leave; - } - - err = gcry_cipher_open (&cipherhd, GCRY_CIPHER_AES128, - GCRY_CIPHER_MODE_AESWRAP, 0); - if (!err) - err = gcry_cipher_setkey (cipherhd, "1234567890123456", 16); - if (!err) - err = gcry_cipher_decrypt (cipherhd, value, valuelen, - wrappedpin, wrappedpinlen); - if (err) - { - log_error ("%s: error decrypting the cryptogram: %s\n", - __func__, gpg_strerror (err)); - goto leave; - } - - log_debug ("%s: caching '%s'->'%s'\n", __func__, key, value); - agent_put_cache (NULL, key, CACHE_MODE_PIN, value, -1); + log_debug ("%s: caching '%s'->'%s'\n", __func__, key, pin); + agent_put_cache (NULL, key, CACHE_MODE_PIN, pin, -1); + err = 0; leave: xfree (keybuf); - xfree (value); - xfree (wrappedpin); - gcry_cipher_close (cipherhd); return err; } @@ -754,6 +702,43 @@ pincache_put_cb (void *opaque, const char *line) } +/* Handle a PINCACHE_GET inquiry. ARGS are the arguments of the + * inquiry which should be a single string with the key for the cached + * value. CTX is the Assuan handle. */ +static gpg_error_t +handle_pincache_get (const char *args, assuan_context_t ctx) +{ + gpg_error_t err; + const char *key; + char *pin = NULL; + + log_debug ("%s: enter '%s'\n", __func__, args); + key = args; + if (strlen (key) < 5) + { + /* We need at least 2 slashes, one slot number and two 1 byte strings.*/ + err = gpg_error (GPG_ERR_INV_REQUEST); + log_debug ("%s: key too short\n", __func__); + goto leave; + } + + pin = agent_get_cache (NULL, key, CACHE_MODE_PIN); + if (!pin || !*pin) + { + xfree (pin); + err = 0; /* Not found is indicated by sending no data back. */ + log_debug ("%s: not cached\n", __func__); + goto leave; + } + log_debug ("%s: cache returned '%s'\n", __func__, pin); + err = assuan_send_data (ctx, pin, strlen (pin)); + + leave: + xfree (pin); + return err; +} + + static gpg_error_t learn_status_cb (void *opaque, const char *line) @@ -924,9 +909,7 @@ inq_needpin (void *opaque, const char *line) } else if ((s = has_leading_keyword (line, "PINCACHE_GET"))) { - /* rc = parm->getpin_cb (parm->getpin_cb_arg, parm->getpin_cb_desc, */ - /* "", NULL, 0); */ - rc = 0; + rc = handle_pincache_get (s, parm->ctx); } else if (parm->passthru) { diff --git a/common/convert.c b/common/convert.c index 54182e15b..1efaccedf 100644 --- a/common/convert.c +++ b/common/convert.c @@ -192,7 +192,7 @@ bin2hexcolon (const void *buffer, size_t length, char *stringbuf) On success the function returns a pointer to the next character after HEXSTRING (which is either end-of-string or the next white - space). If BUFLEN is not NULL the number of valid vytes in BUFFER + space). If BUFLEN is not NULL the number of valid bytes in BUFFER is stored there (an extra Nul byte is not counted); this will even be done if BUFFER has been passed as NULL. */ const char * diff --git a/scd/command.c b/scd/command.c index 36d46f084..60da1a2da 100644 --- a/scd/command.c +++ b/scd/command.c @@ -2242,12 +2242,33 @@ send_status_printf (ctrl_t ctrl, const char *keyword, const char *format, ...) } +/* Set a gcrypt key for use with the pincache. The key is a random + * key unique for this process and is useless after this process has + * terminated. This way the cached PINs stored in the gpg-agent are + * bound to this specific process. The main purpose of this + * encryption is to hide the PIN in logs of the IPC. */ +static gpg_error_t +set_key_for_pincache (gcry_cipher_hd_t hd) +{ + static int initialized; + static unsigned char keybuf[16]; + + if (!initialized) + { + gcry_randomize (keybuf, sizeof keybuf, GCRY_STRONG_RANDOM); + initialized = 1; + } + + return gcry_cipher_setkey (hd, keybuf, sizeof keybuf); +} + + /* Store the PIN in the PIN cache. The key to identify the PIN * consists of (SLOT,APPNAME,PINREF). If PIN is NULL the PIN stored * under the given key is cleared. If APPNAME and PINREF are NULL the * entire PIN cache for SLOT is cleared. If SLOT is -1 the entire PIN * cache is cleared. We do no use an scdaemon internal cache but let - * gpg-agent cache because it is better suited for this. */ + * gpg-agent cache it because it is better suited for this. */ void pincache_put (ctrl_t ctrl, int slot, const char *appname, const char *pinref, const char *pin) @@ -2286,36 +2307,35 @@ pincache_put (ctrl_t ctrl, int slot, const char *appname, const char *pinref, return; /* Ignore an empty PIN. */ snprintf (line, sizeof line, "%d/%s/%s ", - slot, appname? appname:"-", pinref? pinref:"-"); + slot, appname? appname:"", pinref? pinref:""); /* Without an APPNAME etc or without a PIN we clear the cache and * thus there is no need to send the pin - even if the caller * accidentially passed a pin. */ if (pin && slot != -1 && appname && pinref) { + /* FIXME: Replace this by OCB mode and use the cache key as + * additional data. */ pinlen = strlen (pin); - if ((pinlen % 8)) + /* Pad with zeroes (AESWRAP requires multiples of 64 bit but + * at least 128 bit data). */ + pinbuflen = pinlen + 8 - (pinlen % 8); + if (pinbuflen < 16) + pinbuflen = 16; + pinbuf = xtrycalloc_secure (1, pinbuflen); + if (!pinbuf) { - /* Pad with zeroes (AESWRAP requires multiples of 64 bit but - * at least 128 bit data). */ - pinbuflen = pinlen + 8 - (pinlen % 8); - if (pinbuflen < 16) - pinbuflen = 16; - pinbuf = xtrycalloc_secure (1, pinbuflen); - if (!pinbuf) - { - err = gpg_error_from_syserror (); - goto leave; - } - memcpy (pinbuf, pin, pinlen); - pinlen = pinbuflen; - pin = pinbuf; + err = gpg_error_from_syserror (); + goto leave; } + memcpy (pinbuf, pin, pinlen); + pinlen = pinbuflen; + pin = pinbuf; err = gcry_cipher_open (&cipherhd, GCRY_CIPHER_AES128, GCRY_CIPHER_MODE_AESWRAP, 0); if (!err) - err = gcry_cipher_setkey (cipherhd, "1234567890123456", 16); + err = set_key_for_pincache (cipherhd); if (err) goto leave; @@ -2379,45 +2399,77 @@ pincache_get (ctrl_t ctrl, int slot, const char *appname, const char *pinref, } if (!ctrl || !ctrl->server_local || !(ctx = ctrl->server_local->assuan_ctx)) { - err = set_error (GPG_ERR_USE_CONDITIONS, "called w/o assuan context"); + err = gpg_error (GPG_ERR_USE_CONDITIONS); + log_error ("%s: called w/o assuan context\n", __func__); goto leave; } snprintf (command, sizeof command, "PINCACHE_GET %d/%s/%s", - slot, appname? appname:"-", pinref? pinref:"-"); + slot, appname? appname:"", pinref? pinref:""); + /* Limit the inquire to something reasonable. The 32 extra bytes + * are a guessed size for padding etc. */ err = assuan_inquire (ctx, command, &wrappedkey, &wrappedkeylen, - MAXLEN_PIN+24); + 2*MAXLEN_PIN+32); if (gpg_err_code (err) == GPG_ERR_ASS_CANCELED) - err = set_error (GPG_ERR_NOT_SUPPORTED, - "client does not feature a PIN cache"); - else if (!err && (!wrappedkey || wrappedkeylen < 24)) - err = set_error (GPG_ERR_INV_LENGTH, "received too short cryptogram"); - else if (!err) { - valuelen = wrappedkeylen - 8; - value = xtrymalloc_secure (valuelen); - if (!value) - { - err = gpg_error_from_syserror (); - goto leave; - } - - err = gcry_cipher_open (&cipherhd, GCRY_CIPHER_AES128, - GCRY_CIPHER_MODE_AESWRAP, 0); - if (!err) - err = gcry_cipher_setkey (cipherhd, "1234567890123456", 16); - if (err) - goto leave; - - err = gcry_cipher_decrypt (cipherhd, value, valuelen, - wrappedkey, wrappedkeylen); - if (err) - goto leave; - - *r_pin = value; - value = NULL; + err = gpg_error (GPG_ERR_NOT_SUPPORTED); + log_info ("caller does not feature a PIN cache"); + goto leave; } + if (err) + { + log_error ("%s: sending PINCACHE_GET to caller failed: %s\n", + __func__, gpg_strerror (err)); + goto leave; + } + if (!wrappedkey || !wrappedkeylen) + { + err = gpg_error (GPG_ERR_NOT_FOUND); + goto leave; + } + + /* Convert to hex to binary and store it in (wrappedkey, wrappedkeylen). */ + if (!hex2str (wrappedkey, wrappedkey, wrappedkeylen, &wrappedkeylen)) + { + err = gpg_error_from_syserror (); + log_error ("%s: caller returned invalid hex string: %s\n", + __func__, gpg_strerror (err)); + goto leave; + } + + if (!wrappedkey || wrappedkeylen < 24) + { + err = gpg_error (GPG_ERR_INV_LENGTH); /* too short cryptogram */ + goto leave; + } + + valuelen = wrappedkeylen - 8; + value = xtrymalloc_secure (valuelen); + if (!value) + { + err = gpg_error_from_syserror (); + goto leave; + } + + err = gcry_cipher_open (&cipherhd, GCRY_CIPHER_AES128, + GCRY_CIPHER_MODE_AESWRAP, 0); + if (!err) + err = set_key_for_pincache (cipherhd); + if (err) + goto leave; + + err = gcry_cipher_decrypt (cipherhd, value, valuelen, + wrappedkey, wrappedkeylen); + if (err) + { + log_error ("%s: cached value could not be decrypted: %s\n", + __func__, gpg_strerror (err)); + goto leave; + } + + *r_pin = value; + value = NULL; leave: gcry_cipher_close (cipherhd);