From 18fd4964f66ab297a5540f38f5dd6fb22b8e4572 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Mon, 20 Dec 2004 08:32:56 +0000 Subject: [PATCH] * call-scd.c (init_membuf, put_membuf, get_membuf): Removed. We now use the identical implementation from ../common/membuf.c. * pksign.c (agent_pksign): Changed arg OUTFP to OUTBUF and use membuf functions to return the value. * pkdecrypt.c (agent_pkdecrypt): Ditto. * genkey.c (agent_genkey): Ditto. * command.c (cmd_pksign, cmd_pkdecrypt, cmd_genkey): Replaced assuan_get_data_fp() by a the membuf scheme. (clear_outbuf, write_and_clear_outbuf): New. * membuf.c (put_membuf): Wipe out buffer after a failed realloc. --- agent/ChangeLog | 13 ++++++++ agent/agent.h | 7 +++-- agent/call-scd.c | 79 +++-------------------------------------------- agent/command.c | 64 +++++++++++++++++++++++++++++++++++--- agent/genkey.c | 18 ++++------- agent/pkdecrypt.c | 22 +++++++------ agent/pksign.c | 8 ++--- common/ChangeLog | 4 +++ common/membuf.c | 5 +++ 9 files changed, 112 insertions(+), 108 deletions(-) diff --git a/agent/ChangeLog b/agent/ChangeLog index 1c56529b8..f2b82466c 100644 --- a/agent/ChangeLog +++ b/agent/ChangeLog @@ -1,3 +1,16 @@ +2004-12-20 Werner Koch + + * call-scd.c (init_membuf, put_membuf, get_membuf): Removed. We + now use the identical implementation from ../common/membuf.c. + + * pksign.c (agent_pksign): Changed arg OUTFP to OUTBUF and use + membuf functions to return the value. + * pkdecrypt.c (agent_pkdecrypt): Ditto. + * genkey.c (agent_genkey): Ditto. + * command.c (cmd_pksign, cmd_pkdecrypt, cmd_genkey): Replaced + assuan_get_data_fp() by a the membuf scheme. + (clear_outbuf, write_and_clear_outbuf): New. + 2004-12-19 Werner Koch * query.c (initialize_module_query): New. diff --git a/agent/agent.h b/agent/agent.h index e6c8e2a35..241b37b05 100644 --- a/agent/agent.h +++ b/agent/agent.h @@ -33,6 +33,7 @@ #include #include "../common/util.h" #include "../common/errors.h" +#include "membuf.h" /* Convenience function to be used instead of returning the old GNUPG_Out_Of_Core. */ @@ -166,16 +167,16 @@ void agent_unlock_cache_entry (void **cache_id); int agent_pksign_do (CTRL ctrl, const char *desc_text, gcry_sexp_t *signature_sexp, int ignore_cache); int agent_pksign (ctrl_t ctrl, const char *desc_text, - FILE *outfp, int ignore_cache); + membuf_t *outbuf, int ignore_cache); /*-- pkdecrypt.c --*/ int agent_pkdecrypt (ctrl_t ctrl, const char *desc_text, const unsigned char *ciphertext, size_t ciphertextlen, - FILE *outfp); + membuf_t *outbuf); /*-- genkey.c --*/ int agent_genkey (ctrl_t ctrl, - const char *keyparam, size_t keyparmlen, FILE *outfp); + const char *keyparam, size_t keyparmlen, membuf_t *outbuf); int agent_protect_and_store (ctrl_t ctrl, gcry_sexp_t s_skey); /*-- protect.c --*/ diff --git a/agent/call-scd.c b/agent/call-scd.c index 90c625f8b..619a549f9 100644 --- a/agent/call-scd.c +++ b/agent/call-scd.c @@ -82,75 +82,6 @@ struct inq_needpin_s { void *getpin_cb_arg; }; -struct membuf { - size_t len; - size_t size; - char *buf; - int out_of_core; -}; - - - -/* A simple implementation of a dynamic buffer. Use init_membuf() to - create a buffer, put_membuf to append bytes and get_membuf to - release and return the buffer. Allocation errors are detected but - only returned at the final get_membuf(), this helps not to clutter - the code with out of core checks. */ - -static void -init_membuf (struct membuf *mb, int initiallen) -{ - mb->len = 0; - mb->size = initiallen; - mb->out_of_core = 0; - mb->buf = xtrymalloc (initiallen); - if (!mb->buf) - mb->out_of_core = 1; -} - -static void -put_membuf (struct membuf *mb, const void *buf, size_t len) -{ - if (mb->out_of_core) - return; - - if (mb->len + len >= mb->size) - { - char *p; - - mb->size += len + 1024; - p = xtryrealloc (mb->buf, mb->size); - if (!p) - { - mb->out_of_core = 1; - return; - } - mb->buf = p; - } - memcpy (mb->buf + mb->len, buf, len); - mb->len += len; -} - -static void * -get_membuf (struct membuf *mb, size_t *len) -{ - char *p; - - if (mb->out_of_core) - { - xfree (mb->buf); - mb->buf = NULL; - return NULL; - } - - p = mb->buf; - *len = mb->len; - mb->buf = NULL; - mb->out_of_core = 1; /* don't allow a reuse */ - return p; -} - - /* This function must be called once to initialize this module. This @@ -468,7 +399,7 @@ agent_card_serialno (ctrl_t ctrl, char **r_serialno) static AssuanError membuf_data_cb (void *opaque, const void *buffer, size_t length) { - struct membuf *data = opaque; + membuf_t *data = opaque; if (buffer) put_membuf (data, buffer, length); @@ -519,7 +450,7 @@ agent_card_pksign (ctrl_t ctrl, { int rc, i; char *p, line[ASSUAN_LINELENGTH]; - struct membuf data; + membuf_t data; struct inq_needpin_s inqparm; size_t len; unsigned char *sigbuf; @@ -591,7 +522,7 @@ agent_card_pkdecrypt (ctrl_t ctrl, { int rc, i; char *p, line[ASSUAN_LINELENGTH]; - struct membuf data; + membuf_t data; struct inq_needpin_s inqparm; size_t len; @@ -643,7 +574,7 @@ agent_card_readcert (ctrl_t ctrl, { int rc; char line[ASSUAN_LINELENGTH]; - struct membuf data; + membuf_t data; size_t len; *r_buf = NULL; @@ -679,7 +610,7 @@ agent_card_readkey (ctrl_t ctrl, const char *id, unsigned char **r_buf) { int rc; char line[ASSUAN_LINELENGTH]; - struct membuf data; + membuf_t data; size_t len, buflen; *r_buf = NULL; diff --git a/agent/command.c b/agent/command.c index 02bbf3429..9fc08e211 100644 --- a/agent/command.c +++ b/agent/command.c @@ -50,7 +50,7 @@ struct server_local_s { ASSUAN_CONTEXT assuan_ctx; int message_fd; int use_cache_for_signing; - char *keydesc; /* Allocated description fro the next key + char *keydesc; /* Allocated description for the next key operation. */ }; @@ -58,6 +58,41 @@ struct server_local_s { +/* Release the memory buffer MB but first wipe out the used memory. */ +static void +clear_outbuf (membuf_t *mb) +{ + void *p; + size_t n; + + p = get_membuf (mb, &n); + if (p) + { + memset (p, 0, n); + xfree (p); + } +} + + +/* Write the content of memory buffer MB as assuan data to CTX and + wipe the buffer out afterwards. */ +static gpg_error_t +write_and_clear_outbuf (assuan_context_t ctx, membuf_t *mb) +{ + assuan_error_t ae; + void *p; + size_t n; + + p = get_membuf (mb, &n); + if (!p) + return gpg_error (GPG_ERR_ENOMEM); + ae = assuan_send_data (ctx, p, n); + memset (p, 0, n); + xfree (p); + return map_assuan_err (ae); +} + + static void reset_notify (ASSUAN_CONTEXT ctx) { @@ -369,14 +404,21 @@ cmd_pksign (ASSUAN_CONTEXT ctx, char *line) int rc; int ignore_cache = 0; ctrl_t ctrl = assuan_get_pointer (ctx); + membuf_t outbuf; if (opt.ignore_cache_for_signing) ignore_cache = 1; else if (!ctrl->server_local->use_cache_for_signing) ignore_cache = 1; + init_membuf (&outbuf, 512); + rc = agent_pksign (ctrl, ctrl->server_local->keydesc, - assuan_get_data_fp (ctx), ignore_cache); + &outbuf, ignore_cache); + if (rc) + clear_outbuf (&outbuf); + else + rc = write_and_clear_outbuf (ctx, &outbuf); if (rc) log_error ("command pksign failed: %s\n", gpg_strerror (rc)); xfree (ctrl->server_local->keydesc); @@ -395,6 +437,7 @@ cmd_pkdecrypt (ASSUAN_CONTEXT ctx, char *line) ctrl_t ctrl = assuan_get_pointer (ctx); unsigned char *value; size_t valuelen; + membuf_t outbuf; /* First inquire the data to decrypt */ rc = assuan_inquire (ctx, "CIPHERTEXT", @@ -402,9 +445,15 @@ cmd_pkdecrypt (ASSUAN_CONTEXT ctx, char *line) if (rc) return rc; + init_membuf (&outbuf, 512); + rc = agent_pkdecrypt (ctrl, ctrl->server_local->keydesc, - value, valuelen, assuan_get_data_fp (ctx)); + value, valuelen, &outbuf); xfree (value); + if (rc) + clear_outbuf (&outbuf); + else + rc = write_and_clear_outbuf (ctx, &outbuf); if (rc) log_error ("command pkdecrypt failed: %s\n", gpg_strerror (rc)); xfree (ctrl->server_local->keydesc); @@ -434,14 +483,21 @@ cmd_genkey (ASSUAN_CONTEXT ctx, char *line) int rc; unsigned char *value; size_t valuelen; + membuf_t outbuf; /* First inquire the parameters */ rc = assuan_inquire (ctx, "KEYPARAM", &value, &valuelen, MAXLEN_KEYPARAM); if (rc) return rc; - rc = agent_genkey (ctrl, value, valuelen, assuan_get_data_fp (ctx)); + init_membuf (&outbuf, 512); + + rc = agent_genkey (ctrl, value, valuelen, &outbuf); xfree (value); + if (rc) + clear_outbuf (&outbuf); + else + rc = write_and_clear_outbuf (ctx, &outbuf); if (rc) log_error ("command genkey failed: %s\n", gpg_strerror (rc)); return map_to_assuan_status (rc); diff --git a/agent/genkey.c b/agent/genkey.c index 3c56ba33e..17d85f77c 100644 --- a/agent/genkey.c +++ b/agent/genkey.c @@ -88,7 +88,7 @@ reenter_compare_cb (struct pin_entry_info_s *pi) KEYPARAM */ int agent_genkey (CTRL ctrl, const char *keyparam, size_t keyparamlen, - FILE *outfp) + membuf_t *outbuf) { gcry_sexp_t s_keyparam, s_key, s_private, s_public; struct pin_entry_info_s *pi, *pi2; @@ -171,7 +171,8 @@ agent_genkey (CTRL ctrl, const char *keyparam, size_t keyparamlen, gcry_sexp_release (s_key); s_key = NULL; /* store the secret key */ - log_debug ("storing private key\n"); + if (DBG_CRYPTO) + log_debug ("storing private key\n"); rc = store_key (s_private, pi? pi->pin:NULL, 0); xfree (pi); pi = NULL; gcry_sexp_release (s_private); @@ -182,7 +183,8 @@ agent_genkey (CTRL ctrl, const char *keyparam, size_t keyparamlen, } /* return the public key */ - log_debug ("returning public key\n"); + if (DBG_CRYPTO) + log_debug ("returning public key\n"); len = gcry_sexp_sprint (s_public, GCRYSEXP_FMT_CANON, NULL, 0); assert (len); buf = xtrymalloc (len); @@ -195,15 +197,7 @@ agent_genkey (CTRL ctrl, const char *keyparam, size_t keyparamlen, } len = gcry_sexp_sprint (s_public, GCRYSEXP_FMT_CANON, buf, len); assert (len); - if (fwrite (buf, len, 1, outfp) != 1) - { - gpg_error_t tmperr = gpg_error (gpg_err_code_from_errno (errno)); - log_error ("error writing public key: %s\n", strerror (errno)); - gcry_sexp_release (s_private); - gcry_sexp_release (s_public); - xfree (buf); - return tmperr; - } + put_membuf (outbuf, buf, len); gcry_sexp_release (s_public); xfree (buf); diff --git a/agent/pkdecrypt.c b/agent/pkdecrypt.c index eaa2b2254..7a93e58f8 100644 --- a/agent/pkdecrypt.c +++ b/agent/pkdecrypt.c @@ -37,7 +37,7 @@ int agent_pkdecrypt (CTRL ctrl, const char *desc_text, const unsigned char *ciphertext, size_t ciphertextlen, - FILE *outfp) + membuf_t *outbuf) { gcry_sexp_t s_skey = NULL, s_cipher = NULL, s_plain = NULL; unsigned char *shadow_info = NULL; @@ -88,11 +88,16 @@ agent_pkdecrypt (CTRL ctrl, const char *desc_text, log_error ("smartcard decryption failed: %s\n", gpg_strerror (rc)); goto leave; } - /* FIXME: don't use buffering and change the protocol to return - a complete S-expression and not just a part. */ - fprintf (outfp, "%u:", (unsigned int)len); - fwrite (buf, 1, len, outfp); - putc (0, outfp); + /* FIXME: Change the protocol to return a complete S-expression + and not just a part. */ + { + char tmpbuf[50]; + + sprintf (tmpbuf, "%u:", (unsigned int)len); + put_membuf (outbuf, tmpbuf, strlen (tmpbuf)); + put_membuf (outbuf, buf, len); + put_membuf (outbuf, "", 1); + } } else { /* No smartcard, but a private key */ @@ -119,10 +124,7 @@ agent_pkdecrypt (CTRL ctrl, const char *desc_text, buf = xmalloc (len); len = gcry_sexp_sprint (s_plain, GCRYSEXP_FMT_CANON, buf, len); assert (len); - /* FIXME: we must make sure that no buffering takes place or we are - in full control of the buffer memory (easy to do) - should go - into assuan. */ - fwrite (buf, 1, len, outfp); + put_membuf (outbuf, buf, len); } diff --git a/agent/pksign.c b/agent/pksign.c index 11e964837..3337e188c 100644 --- a/agent/pksign.c +++ b/agent/pksign.c @@ -176,7 +176,8 @@ agent_pksign_do (CTRL ctrl, const char *desc_text, /* SIGN whatever information we have accumulated in CTRL and write it back to OUTFP. */ int -agent_pksign (CTRL ctrl, const char *desc_text, FILE *outfp, int ignore_cache) +agent_pksign (CTRL ctrl, const char *desc_text, + membuf_t *outbuf, int ignore_cache) { gcry_sexp_t s_sig = NULL; char *buf = NULL; @@ -193,10 +194,7 @@ agent_pksign (CTRL ctrl, const char *desc_text, FILE *outfp, int ignore_cache) len = gcry_sexp_sprint (s_sig, GCRYSEXP_FMT_CANON, buf, len); assert (len); - /* FIXME: we must make sure that no buffering takes place or we are - in full control of the buffer memory (easy to do) - should go - into assuan. */ - fwrite (buf, 1, len, outfp); + put_membuf (outbuf, buf, len); leave: gcry_sexp_release (s_sig); diff --git a/common/ChangeLog b/common/ChangeLog index 336b3928a..eeba09341 100644 --- a/common/ChangeLog +++ b/common/ChangeLog @@ -1,3 +1,7 @@ +2004-12-20 Werner Koch + + * membuf.c (put_membuf): Wipe out buffer after a failed realloc. + 2004-12-19 Werner Koch * maperror.c (map_assuan_err_with_source): Oops, args were swapped. diff --git a/common/membuf.c b/common/membuf.c index 69e4ab908..75f6bdb2a 100644 --- a/common/membuf.c +++ b/common/membuf.c @@ -60,6 +60,11 @@ put_membuf (membuf_t *mb, const void *buf, size_t len) if (!p) { mb->out_of_core = errno; + /* Wipe out what we already accumulated. This is required + in case we are storing sensitive data here. The membuf + API does not provide another way to cleanup after an + error. */ + memset (mb->buf, 0, mb->len); return; } mb->buf = p;