From 7ffc1ac7dd95d4cc1897a4c36d5cd628741c12f2 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 6 Jun 2018 18:28:44 +0200 Subject: agent: Add DBUS_SESSION_BUS_ADDRESS et al. to the startup list. * agent/gpg-agent.c (agent_copy_startup_env): Replace explicit list with the standard list. -- Although the function agent_copy_startup_env is newer than session_env_list_stdenvnames the latter was not used. When DBUS_SESSION_BUS_ADDRESS was added to the latter it was forgotten to add it to the former as well. Having all stdnames here seems to be the Right Thing (tm) to do. GnuPG-bug-id: 3947 Signed-off-by: Werner Koch --- agent/gpg-agent.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'agent') diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c index bd9a471e8..1fdc94d0f 100644 --- a/agent/gpg-agent.c +++ b/agent/gpg-agent.c @@ -1979,15 +1979,15 @@ agent_deinit_default_ctrl (ctrl_t ctrl) gpg_error_t agent_copy_startup_env (ctrl_t ctrl) { - static const char *names[] = - {"GPG_TTY", "DISPLAY", "TERM", "XAUTHORITY", "PINENTRY_USER_DATA", NULL}; gpg_error_t err = 0; - int idx; - const char *value; + int iterator = 0; + const char *name, *value; - for (idx=0; !err && names[idx]; idx++) - if ((value = session_env_getenv (opt.startup_env, names[idx]))) - err = session_env_setenv (ctrl->session_env, names[idx], value); + while (!err && (name = session_env_list_stdenvnames (&iterator, NULL))) + { + if ((value = session_env_getenv (opt.startup_env, name))) + err = session_env_setenv (ctrl->session_env, name, value); + } if (!err && !ctrl->lc_ctype && opt.startup_lc_ctype) if (!(ctrl->lc_ctype = xtrystrdup (opt.startup_lc_ctype))) -- cgit v1.2.3 From 3978df943dc7a4781a23382be2d3b4a96a04f71f Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Mon, 2 Jul 2018 20:22:42 +0200 Subject: agent: Fix segv running in --server mode * agent/command.c (start_command_handler): Do not write to CLIENT_CREDS after an error. -- assuan_get_peercred is special insofar that it returns a pointer into CTX. Writing data via this pointer should never be done. Fixes-commit: 28aa6890588cc108639951bb4bef03ac17743046 Signed-off-by: Werner Koch --- agent/command.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'agent') diff --git a/agent/command.c b/agent/command.c index 1a08cfcc0..9bc3b027c 100644 --- a/agent/command.c +++ b/agent/command.c @@ -3351,7 +3351,8 @@ start_command_handler (ctrl_t ctrl, gnupg_fd_t listen_fd, gnupg_fd_t fd) for (;;) { - assuan_peercred_t client_creds; + assuan_peercred_t client_creds; /* Note: Points into CTX. */ + pid_t pid; rc = assuan_accept (ctx); if (gpg_err_code (rc) == GPG_ERR_EOF || rc == -1) @@ -3367,17 +3368,21 @@ start_command_handler (ctrl_t ctrl, gnupg_fd_t listen_fd, gnupg_fd_t fd) rc = assuan_get_peercred (ctx, &client_creds); if (rc) { - log_info ("Assuan get_peercred failed: %s\n", gpg_strerror (rc)); - client_creds->pid = assuan_get_pid (ctx); + + if (listen_fd == GNUPG_INVALID_FD && fd == GNUPG_INVALID_FD) + ; + else + log_info ("Assuan get_peercred failed: %s\n", gpg_strerror (rc)); + pid = assuan_get_pid (ctx); ctrl->client_uid = -1; } - ctrl->server_local->connect_from_self = - (client_creds->pid == getpid ()); - if (client_creds->pid != ASSUAN_INVALID_PID) - ctrl->client_pid = (unsigned long)client_creds->pid; else - ctrl->client_pid = 0; - ctrl->client_uid = client_creds->uid; + { + pid = client_creds->pid; + ctrl->client_uid = client_creds->uid; + } + ctrl->client_pid = (pid == ASSUAN_INVALID_PID)? 0 : (unsigned long)pid; + ctrl->server_local->connect_from_self = (pid == getpid ()); rc = assuan_process (ctx); if (rc) -- cgit v1.2.3 From 8a915cd9faf052b4faa3c415f2ac5aa8d6ea1efe Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Mon, 2 Jul 2018 21:24:15 +0200 Subject: agent: New commands PUT_SECRET and GET_SECRET. * agent/agent.h (CACHE_MODE_DATA): New const. * agent/cache.c (DEF_CACHE_TTL_DATA): new. (housekeeping): Tweak for CACHE_MODE_DATA. (cache_mode_equal): Ditto. (agent_get_cache): Ditto. (agent_put_cache): Implement CACHE_MODE_DATA. * agent/command.c (MAXLEN_PUT_SECRET): New. (parse_ttl): New. (cmd_get_secret): New. (cmd_put_secret): New. (register_commands): Register new commands. -- These commands allow to store secrets in memory for the lifetime of the gpg-agent process. Signed-off-by: Werner Koch --- agent/agent.h | 5 +- agent/cache.c | 34 ++++++--- agent/command.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 237 insertions(+), 12 deletions(-) (limited to 'agent') diff --git a/agent/agent.h b/agent/agent.h index 9fdbc76d3..9baf59601 100644 --- a/agent/agent.h +++ b/agent/agent.h @@ -304,11 +304,12 @@ enum typedef enum { CACHE_MODE_IGNORE = 0, /* Special mode to bypass the cache. */ - CACHE_MODE_ANY, /* Any mode except ignore matches. */ + CACHE_MODE_ANY, /* Any mode except ignore and data matches. */ CACHE_MODE_NORMAL, /* Normal cache (gpg-agent). */ CACHE_MODE_USER, /* GET_PASSPHRASE related cache. */ CACHE_MODE_SSH, /* SSH related cache. */ - CACHE_MODE_NONCE /* This is a non-predictable nonce. */ + CACHE_MODE_NONCE, /* This is a non-predictable nonce. */ + CACHE_MODE_DATA /* Arbitrary data. */ } cache_mode_t; diff --git a/agent/cache.c b/agent/cache.c index 238b6e214..799d595ab 100644 --- a/agent/cache.c +++ b/agent/cache.c @@ -28,6 +28,10 @@ #include "agent.h" +/* The default TTL for DATA items. This has no configure + * option because it is expected that clients provide a TTL. */ +#define DEF_CACHE_TTL_DATA (10 * 60) /* 10 minutes. */ + /* The size of the encryption key in bytes. */ #define ENCRYPTION_KEYSIZE (128/8) @@ -50,11 +54,12 @@ struct secret_data_s { char data[1]; /* A string. */ }; +/* The cache object. */ typedef struct cache_item_s *ITEM; struct cache_item_s { ITEM next; time_t created; - time_t accessed; + time_t accessed; /* Not updated for CACHE_MODE_DATA */ int ttl; /* max. lifetime given in seconds, -1 one means infinite */ struct secret_data_s *pw; cache_mode_t cache_mode; @@ -211,14 +216,18 @@ housekeeping (void) } } - /* Second, make sure that we also remove them based on the created stamp so - that the user has to enter it from time to time. */ + /* Second, make sure that we also remove them based on the created + * stamp so that the user has to enter it from time to time. We + * don't do this for data items which are used to storage secrets in + * meory and are not user entered passphrases etc. */ for (r=thecache; r; r = r->next) { unsigned long maxttl; switch (r->cache_mode) { + case CACHE_MODE_DATA: + continue; /* No MAX TTL here. */ case CACHE_MODE_SSH: maxttl = opt.max_cache_ttl_ssh; break; default: maxttl = opt.max_cache_ttl; break; } @@ -315,8 +324,11 @@ static int cache_mode_equal (cache_mode_t a, cache_mode_t b) { /* CACHE_MODE_ANY matches any mode other than CACHE_MODE_IGNORE. */ - return ((a == CACHE_MODE_ANY && b != CACHE_MODE_IGNORE) - || (b == CACHE_MODE_ANY && a != CACHE_MODE_IGNORE) || a == b); + return ((a == CACHE_MODE_ANY + && !(b == CACHE_MODE_IGNORE || b == CACHE_MODE_DATA)) + || (b == CACHE_MODE_ANY + && !(a == CACHE_MODE_IGNORE || a == CACHE_MODE_DATA)) + || a == b); } @@ -349,6 +361,7 @@ agent_put_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode, switch(cache_mode) { case CACHE_MODE_SSH: ttl = opt.def_cache_ttl_ssh; break; + case CACHE_MODE_DATA: ttl = DEF_CACHE_TTL_DATA; break; default: ttl = opt.def_cache_ttl; break; } } @@ -415,9 +428,7 @@ agent_put_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode, } -/* Try to find an item in the cache. Note that we currently don't - make use of CACHE_MODE except for CACHE_MODE_NONCE and - CACHE_MODE_USER. */ +/* Try to find an item in the cache. */ char * agent_get_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode) { @@ -458,8 +469,11 @@ agent_get_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode) && r->restricted == restricted && !strcmp (r->key, key)) { - /* Note: To avoid races KEY may not be accessed anymore below. */ - r->accessed = gnupg_get_time (); + /* Note: To avoid races KEY may not be accessed anymore + * below. Note also that we don't update the accessed time + * for data items. */ + if (r->cache_mode != CACHE_MODE_DATA) + r->accessed = gnupg_get_time (); if (DBG_CACHE) log_debug ("... hit\n"); if (r->pw->totallen < 32) diff --git a/agent/command.c b/agent/command.c index 9bc3b027c..925d1f780 100644 --- a/agent/command.c +++ b/agent/command.c @@ -50,6 +50,8 @@ #define MAXLEN_KEYPARAM 1024 /* Maximum allowed size of key data as used in inquiries (bytes). */ #define MAXLEN_KEYDATA 8192 +/* Maximum length of a secret to store under one key. */ +#define MAXLEN_PUT_SECRET 4096 /* The size of the import/export KEK key (in bytes). */ #define KEYWRAP_KEYSIZE (128/8) @@ -292,6 +294,31 @@ parse_keygrip (assuan_context_t ctx, const char *string, unsigned char *buf) } +/* Parse the TTL from STRING. Leading and trailing spaces are + * skipped. The value is constrained to -1 .. MAXINT. On error 0 is + * returned, else the number of bytes scanned. */ +static size_t +parse_ttl (const char *string, int *r_ttl) +{ + const char *string_orig = string; + long ttl; + char *pend; + + ttl = strtol (string, &pend, 10); + string = pend; + if (string == string_orig || !(spacep (string) || !*string) + || ttl < -1L || (int)ttl != (long)ttl) + { + *r_ttl = 0; + return 0; + } + while (spacep (string) || *string== '\n') + string++; + *r_ttl = (int)ttl; + return string - string_orig; +} + + /* Write an Assuan status line. KEYWORD is the first item on the * status line. The following arguments are all separated by a space * in the output. The last argument must be a NULL. Linefeeds and @@ -2567,6 +2594,187 @@ cmd_keytocard (assuan_context_t ctx, char *line) } + +static const char hlp_get_secret[] = + "GET_SECRET \n" + "\n" + "Return the secret value stored under KEY\n"; +static gpg_error_t +cmd_get_secret (assuan_context_t ctx, char *line) +{ + ctrl_t ctrl = assuan_get_pointer (ctx); + gpg_error_t err; + char *p, *key; + char *value = NULL; + size_t valuelen; + + /* For now we allow this only for local connections. */ + if (ctrl->restricted) + { + err = gpg_error (GPG_ERR_FORBIDDEN); + goto leave; + } + + line = skip_options (line); + + for (p=line; *p == ' '; p++) + ; + key = p; + p = strchr (key, ' '); + if (p) + { + *p++ = 0; + for (; *p == ' '; p++) + ; + if (*p) + { + err = set_error (GPG_ERR_ASS_PARAMETER, "too many arguments"); + goto leave; + } + } + if (!*key) + { + err = set_error (GPG_ERR_ASS_PARAMETER, "no key given"); + goto leave; + } + + + value = agent_get_cache (ctrl, key, CACHE_MODE_DATA); + if (!value) + { + err = gpg_error (GPG_ERR_NO_DATA); + goto leave; + } + + valuelen = percent_unescape_inplace (value, 0); + err = assuan_send_data (ctx, value, valuelen); + wipememory (value, valuelen); + + leave: + xfree (value); + return leave_cmd (ctx, err); +} + + +static const char hlp_put_secret[] = + "PUT_SECRET [--clear] []\n" + "\n" + "This commands stores a secret under KEY in gpg-agent's in-memory\n" + "cache. The TTL must be explicitly given by TTL and the options\n" + "from the configuration file are not used. The value is either given\n" + "percent-escaped as 3rd argument or if not given inquired by gpg-agent\n" + "using the keyword \"SECRET\".\n" + "The option --clear removes the secret from the cache." + ""; +static gpg_error_t +cmd_put_secret (assuan_context_t ctx, char *line) +{ + ctrl_t ctrl = assuan_get_pointer (ctx); + gpg_error_t err = 0; + int opt_clear; + unsigned char *value = NULL; + size_t valuelen = 0; + size_t n; + char *p, *key, *ttlstr; + unsigned char *valstr; + int ttl; + char *string = NULL; + + /* For now we allow this only for local connections. */ + if (ctrl->restricted) + { + err = gpg_error (GPG_ERR_FORBIDDEN); + goto leave; + } + + opt_clear = has_option (line, "--clear"); + line = skip_options (line); + + for (p=line; *p == ' '; p++) + ; + key = p; + ttlstr = NULL; + valstr = NULL; + p = strchr (key, ' '); + if (p) + { + *p++ = 0; + for (; *p == ' '; p++) + ; + if (*p) + { + ttlstr = p; + p = strchr (ttlstr, ' '); + if (p) + { + *p++ = 0; + for (; *p == ' '; p++) + ; + if (*p) + valstr = p; + } + } + } + if (!*key) + { + err = set_error (GPG_ERR_ASS_PARAMETER, "no key given"); + goto leave; + } + if (!ttlstr || !*ttlstr || !(n = parse_ttl (ttlstr, &ttl))) + { + err = set_error (GPG_ERR_ASS_PARAMETER, "no or invalid TTL given"); + goto leave; + } + if (valstr && opt_clear) + { + err = set_error (GPG_ERR_ASS_PARAMETER, + "value not expected with --clear"); + goto leave; + } + + if (valstr) + { + valuelen = percent_unescape_inplace (valstr, 0); + value = NULL; + } + else /* Inquire the value to store */ + { + err = print_assuan_status (ctx, "INQUIRE_MAXLEN", "%u",MAXLEN_PUT_SECRET); + if (!err) + err = assuan_inquire (ctx, "SECRET", + &value, &valuelen, MAXLEN_PUT_SECRET); + if (err) + goto leave; + } + + /* Our cache expects strings and thus we need to turn the buffer + * into a string. Instead of resorting to base64 encoding we use a + * special percent escaping which only quoted the Nul and the + * percent character. */ + string = percent_data_escape (value? value : valstr, valuelen); + if (!string) + { + err = gpg_error_from_syserror (); + goto leave; + } + err = agent_put_cache (ctrl, key, CACHE_MODE_DATA, string, ttl); + + + leave: + if (string) + { + wipememory (string, strlen (string)); + xfree (string); + } + if (value) + { + wipememory (value, valuelen); + xfree (value); + } + return leave_cmd (ctx, err); +} + + static const char hlp_getval[] = "GETVAL \n" @@ -3259,6 +3467,8 @@ register_commands (assuan_context_t ctx) { "IMPORT_KEY", cmd_import_key, hlp_import_key }, { "EXPORT_KEY", cmd_export_key, hlp_export_key }, { "DELETE_KEY", cmd_delete_key, hlp_delete_key }, + { "GET_SECRET", cmd_get_secret, hlp_get_secret }, + { "PUT_SECRET", cmd_put_secret, hlp_put_secret }, { "GETVAL", cmd_getval, hlp_getval }, { "PUTVAL", cmd_putval, hlp_putval }, { "UPDATESTARTUPTTY", cmd_updatestartuptty, hlp_updatestartuptty }, -- cgit v1.2.3