From 2a28f5d0aece3300dea950b6f9bed9dbc1f01fa7 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 7 Feb 2002 18:43:22 +0000 Subject: [PATCH] * certlist.c (gpgsm_add_to_certlist): Check that the specified name identifies a certificate unambiguously. (gpgsm_find_cert): Ditto. * server.c (cmd_listkeys): Check that the data stream is available. (cmd_listsecretkeys): Ditto. (has_option): New. (cmd_sign): Fix ambiguousity in option recognition. * gpgsm.c (main): Enable --logger-fd. * encrypt.c (gpgsm_encrypt): Increased buffer size for better performance. * call-agent.c (gpgsm_agent_pksign): Check the S-Exp received from the agent. * keylist.c (list_cert_colon): Filter out control characters. --- common/errors.h | 1 + sm/ChangeLog | 21 +++++++++++++++++++++ sm/call-agent.c | 6 +++++- sm/certcheck.c | 2 +- sm/certlist.c | 24 ++++++++++++++++++++++-- sm/encrypt.c | 5 ++--- sm/gpgsm.c | 6 +++--- sm/keylist.c | 6 +++--- sm/server.c | 27 ++++++++++++++++++++------- sm/verify.c | 2 +- 10 files changed, 79 insertions(+), 21 deletions(-) diff --git a/common/errors.h b/common/errors.h index 52b606f38..840405d8b 100644 --- a/common/errors.h +++ b/common/errors.h @@ -84,6 +84,7 @@ enum { GNUPG_Unknown_Sexp = 55, GNUPG_Unsupported_Protection = 56, GNUPG_Corrupted_Protection = 57, + GNUPG_Ambiguous_Name = 58 }; /* Status codes - fixme: should go into another file */ diff --git a/sm/ChangeLog b/sm/ChangeLog index c2cf619a4..1cbacafe5 100644 --- a/sm/ChangeLog +++ b/sm/ChangeLog @@ -1,3 +1,24 @@ +2002-02-07 Werner Koch + + * certlist.c (gpgsm_add_to_certlist): Check that the specified + name identifies a certificate unambiguously. + (gpgsm_find_cert): Ditto. + + * server.c (cmd_listkeys): Check that the data stream is available. + (cmd_listsecretkeys): Ditto. + (has_option): New. + (cmd_sign): Fix ambiguousity in option recognition. + + * gpgsm.c (main): Enable --logger-fd. + + * encrypt.c (gpgsm_encrypt): Increased buffer size for better + performance. + + * call-agent.c (gpgsm_agent_pksign): Check the S-Exp received from + the agent. + + * keylist.c (list_cert_colon): Filter out control characters. + 2002-02-06 Werner Koch * decrypt.c (gpgsm_decrypt): Bail out after an decryption error. diff --git a/sm/call-agent.c b/sm/call-agent.c index ec294f6d6..4d7ffbcba 100644 --- a/sm/call-agent.c +++ b/sm/call-agent.c @@ -273,7 +273,11 @@ gpgsm_agent_pksign (const char *keygrip, } *r_buf = get_membuf (&data, r_buflen); - /* FIXME: check that the returned S-Exp is valid! */ + if (!gcry_sexp_canon_len (*r_buf, *r_buflen, NULL, NULL)) + { + xfree (*r_buf); *r_buf = NULL; + return GNUPG_Invalid_Value; + } return *r_buf? 0 : GNUPG_Out_Of_Core; } diff --git a/sm/certcheck.c b/sm/certcheck.c index 9108eb8e9..524ed26b8 100644 --- a/sm/certcheck.c +++ b/sm/certcheck.c @@ -255,7 +255,7 @@ gpgsm_create_cms_signature (KsbaCert cert, GCRY_MD_HD md, int mdalgo, gcry_md_get_algo_dlen (mdalgo), mdalgo, r_sigval, &siglen); xfree (grip); - /* FIXME: we should check that the returnes S-Exp is valid fits int + /* FIXME: we should check that the returned S-Exp is valid fits int siglen. It ould probaly be a good idea to scan and print it again to make this sure and be sure that we have canoncical encoding */ diff --git a/sm/certlist.c b/sm/certlist.c index c5b8c861f..440cdac74 100644 --- a/sm/certlist.c +++ b/sm/certlist.c @@ -43,7 +43,6 @@ gpgsm_add_to_certlist (const char *name, CERTLIST *listaddr) KEYDB_HANDLE kh = NULL; KsbaCert cert = NULL; - /* fixme: check that we identify excactly one cert with the name */ rc = keydb_classify_name (name, &desc); if (!rc) { @@ -55,6 +54,14 @@ gpgsm_add_to_certlist (const char *name, CERTLIST *listaddr) rc = keydb_search (kh, &desc, 1); if (!rc) rc = keydb_get_cert (kh, &cert); + if (!rc) + { + rc = keydb_search (kh, &desc, 1); + if (rc == -1) + rc = 0; + else if (!rc) + rc = GNUPG_Ambiguous_Name; + } if (!rc) rc = gpgsm_validate_path (cert); if (!rc) @@ -100,7 +107,6 @@ gpgsm_find_cert (const char *name, KsbaCert *r_cert) KEYDB_HANDLE kh = NULL; *r_cert = NULL; - /* fixme: check that we identify excactly one cert with the name */ rc = keydb_classify_name (name, &desc); if (!rc) { @@ -112,9 +118,23 @@ gpgsm_find_cert (const char *name, KsbaCert *r_cert) rc = keydb_search (kh, &desc, 1); if (!rc) rc = keydb_get_cert (kh, r_cert); + if (!rc) + { + rc = keydb_search (kh, &desc, 1); + if (rc == -1) + rc = 0; + else + { + if (!rc) + rc = GNUPG_Ambiguous_Name; + ksba_cert_release (*r_cert); + *r_cert = NULL; + } + } } } keydb_release (kh); return rc == -1? GNUPG_No_Public_Key: rc; } + diff --git a/sm/encrypt.c b/sm/encrypt.c index 1ad90f8dd..43087fc87 100644 --- a/sm/encrypt.c +++ b/sm/encrypt.c @@ -510,9 +510,8 @@ gpgsm_encrypt (CTRL ctrl, CERTLIST recplist, int data_fd, FILE *out_fp) } encparm.dek = dek; - /* fixme: we should use a larger buffer - the small one is better - for testing */ - encparm.bufsize = 10 * dek->ivlen; + /* Use a ~8k (AES) or ~4k (3DES) buffer */ + encparm.bufsize = 500 * dek->ivlen; encparm.buffer = xtrymalloc (encparm.bufsize); if (!encparm.buffer) { diff --git a/sm/gpgsm.c b/sm/gpgsm.c index 22fed31bd..cc86f053c 100644 --- a/sm/gpgsm.c +++ b/sm/gpgsm.c @@ -573,7 +573,7 @@ main ( int argc, char **argv) struct server_control_s ctrl; CERTLIST recplist = NULL; - /* FIXME: trap_unaligned ();*/ + /* fixme: trap_unaligned ();*/ set_strusage (my_strusage); gcry_control (GCRYCTL_SUSPEND_SECMEM_WARN); /* Please note that we may running SUID(ROOT), so be very CAREFUL @@ -592,7 +592,7 @@ main ( int argc, char **argv) may_coredump = disable_core_dumps (); - /* FIXME: init_signals();*/ + /* Fixme: init_signals();*/ create_dotlock (NULL); /* register locking cleanup */ i18n_init(); @@ -791,7 +791,7 @@ main ( int argc, char **argv) case oDebugWait: debug_wait = pargs.r.ret_int; break; case oStatusFD: ctrl.status_fd = pargs.r.ret_int; break; - case oLoggerFD: /* fixme: log_set_logfile (NULL, pargs.r.ret_int );*/ break; + case oLoggerFD: log_set_fd (pargs.r.ret_int ); break; case oWithFingerprint: with_fpr=1; /*fall thru*/ case oFingerprint: diff --git a/sm/keylist.c b/sm/keylist.c index 0050ac464..f681725b7 100644 --- a/sm/keylist.c +++ b/sm/keylist.c @@ -165,7 +165,7 @@ list_cert_colon (KsbaCert cert, FILE *fp, int have_secret) putc (':', fp); if ((p = ksba_cert_get_issuer (cert,0))) { - fputs (p, fp); /* FIXME: Escape colons and linefeeds */ + print_sanitized_string (fp, p, ':'); xfree (p); } putc (':', fp); @@ -188,7 +188,7 @@ list_cert_colon (KsbaCert cert, FILE *fp, int have_secret) for (idx=0; (p = ksba_cert_get_subject (cert,idx)); idx++) { fprintf (fp, "uid:%c::::::::", trustletter); - fputs (p, fp); /* FIXME: Escape colons and linefeeds */ + print_sanitized_string (fp, p, ':'); putc (':', fp); putc (':', fp); putc ('\n', fp); @@ -202,7 +202,7 @@ list_cert_colon (KsbaCert cert, FILE *fp, int have_secret) if (pp) { fprintf (fp, "uid:%c::::::::", trustletter); - fputs (pp, fp); /* FIXME: Escape colons and linefeeds */ + print_sanitized_string (fp, pp, ':'); putc (':', fp); putc (':', fp); putc ('\n', fp); diff --git a/sm/server.c b/sm/server.c index 520a3ddd1..be45060be 100644 --- a/sm/server.c +++ b/sm/server.c @@ -42,6 +42,17 @@ struct server_local_s { CERTLIST recplist; }; +/* Check whether the option NAME appears in LINE */ +static int +has_option (const char *line, const char *name) +{ + const char *s; + int n = strlen (name); + + s = strstr (line, name); + return (s && (s == line || spacep (s-1)) && (!s[n] || spacep (s+n))); +} + static void close_message_fd (CTRL ctrl) @@ -289,7 +300,7 @@ cmd_sign (ASSUAN_CONTEXT ctx, char *line) if (out_fd == -1) return set_error (No_Output, NULL); - detached = !!strstr (line, "--detached"); /* fixme: this is ambiguous */ + detached = has_option (line, "--detached"); out_fp = fdopen ( dup(out_fd), "w"); if (!out_fp) @@ -362,11 +373,12 @@ static int cmd_listkeys (ASSUAN_CONTEXT ctx, char *line) { CTRL ctrl = assuan_get_pointer (ctx); + FILE *fp = assuan_get_data_fp (ctx); + if (!fp) + return set_error (General_Error, "no data stream"); ctrl->with_colons = 1; - /* fixme: check that the returned data_fp is not NULL */ - gpgsm_list_keys (assuan_get_pointer (ctx), NULL, - assuan_get_data_fp (ctx), 3); + gpgsm_list_keys (assuan_get_pointer (ctx), NULL, fp, 3); return 0; } @@ -375,11 +387,12 @@ static int cmd_listsecretkeys (ASSUAN_CONTEXT ctx, char *line) { CTRL ctrl = assuan_get_pointer (ctx); + FILE *fp = assuan_get_data_fp (ctx); ctrl->with_colons = 1; - /* fixme: check that the returned data_fp is not NULL */ - gpgsm_list_keys (assuan_get_pointer (ctx), NULL, - assuan_get_data_fp (ctx), 2); + if (!fp) + return set_error (General_Error, "no data stream"); + gpgsm_list_keys (assuan_get_pointer (ctx), NULL, fp, 2); return 0; } diff --git a/sm/verify.c b/sm/verify.c index 3dd85c02f..d4de57866 100644 --- a/sm/verify.c +++ b/sm/verify.c @@ -34,7 +34,7 @@ #include "keydb.h" #include "i18n.h" -/* FIXME: Move this to jnlib */ +/* fixme: Move this to jnlib */ static char * strtimestamp (time_t atime) {