From f289f433b6d7f0b5ac76e03853ce628f23e3cdc2 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Sun, 6 Jun 2004 13:00:59 +0000 Subject: [PATCH] * configure.ac: Require libksba 0.9.7. * certreqgen.c (get_parameter_uint, create_request): Create an extension for key usage when requested. * gpgsm.c (main): Install emergency_cleanup also as an atexit handler. * verify.c (gpgsm_verify): Removed the separate error code handling for KSBA. We use shared error codes anyway. * export.c (export_p12): Removed debugging code. * encrypt.c (gpgsm_encrypt): Put the session key in to secure memory. --- ChangeLog | 4 +++ NEWS | 2 ++ TODO | 3 -- configure.ac | 2 +- sm/ChangeLog | 17 +++++++++++ sm/certreqgen.c | 40 ++++++++++++++++++++++-- sm/decrypt.c | 8 ++--- sm/encrypt.c | 4 +-- sm/export.c | 2 +- sm/gpgsm.c | 16 +++++++--- sm/keydb.c | 4 +-- sm/sign.c | 4 +-- sm/verify.c | 81 +++++++++++++++++++++---------------------------- 13 files changed, 118 insertions(+), 69 deletions(-) diff --git a/ChangeLog b/ChangeLog index ef33021cf..3a0512249 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2004-06-06 Werner Koch + + * configure.ac: Require libksba 0.9.7. + 2004-04-29 Werner Koch Released 1.9.8. diff --git a/NEWS b/NEWS index e24af0476..24523229d 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,8 @@ Noteworthy changes in version 1.9.9 allow gpg-agent to add a key to the trustlist.txt after user confirmation. + * Creating PKCS#10 requests does now honor the key usage. + Noteworthy changes in version 1.9.8 (2004-04-29) ------------------------------------------------ diff --git a/TODO b/TODO index 0ac96b85c..f22a19b41 100644 --- a/TODO +++ b/TODO @@ -24,9 +24,6 @@ might want to have an agent context for each service request * sm/decrypt.c ** replace leading zero in integer hack by a cleaner solution -* sm/sign.c -** Don't hardcode the use of RSA. - * sm/gpgsm.c ** Support --output for all commands ** mark all unimplemented commands and options. diff --git a/configure.ac b/configure.ac index 687805759..4c719fe75 100644 --- a/configure.ac +++ b/configure.ac @@ -36,7 +36,7 @@ NEED_LIBGCRYPT_VERSION=1.1.94 NEED_LIBASSUAN_VERSION=0.6.6 -NEED_KSBA_VERSION=0.9.4 +NEED_KSBA_VERSION=0.9.7 NEED_OPENSC_VERSION=0.8.0 diff --git a/sm/ChangeLog b/sm/ChangeLog index eed350b24..c1a8f6411 100644 --- a/sm/ChangeLog +++ b/sm/ChangeLog @@ -1,3 +1,20 @@ +2004-06-06 Werner Koch + + * certreqgen.c (get_parameter_uint, create_request): Create + an extension for key usage when requested. + +2004-05-12 Werner Koch + + * gpgsm.c (main): Install emergency_cleanup also as an atexit + handler. + + * verify.c (gpgsm_verify): Removed the separate error code + handling for KSBA. We use shared error codes anyway. + + * export.c (export_p12): Removed debugging code. + + * encrypt.c (gpgsm_encrypt): Put the session key in to secure memory. + 2004-05-11 Werner Koch * sign.c (gpgsm_sign): Include the error source in the final error diff --git a/sm/certreqgen.c b/sm/certreqgen.c index b1adf2974..969ed14b0 100644 --- a/sm/certreqgen.c +++ b/sm/certreqgen.c @@ -129,6 +129,9 @@ struct reqgen_ctrl_s { }; +static const char oidstr_keyUsage[] = "2.5.29.15"; + + static int proc_parameters (ctrl_t ctrl, struct para_data_s *para, struct reqgen_ctrl_s *outctrl); @@ -179,10 +182,10 @@ get_parameter_algo (struct para_data_s *para, enum para_name key) return gcry_pk_map_name (r->u.value); } -/* parse the usage parameter. Returns 0 on success. Note that we +/* Parse the usage parameter. Returns 0 on success. Note that we only care about sign and encrypt and don't (yet) allow all the other X.509 usage to be specified; instead we will use a fixed - mapping to the X.509 usage flags */ + mapping to the X.509 usage flags. */ static int parse_parameter_usage (struct para_data_s *para, enum para_name key) { @@ -222,6 +225,9 @@ get_parameter_uint (struct para_data_s *para, enum para_name key) if (!r) return 0; + if (r->key == pKEYUSAGE) + return r->u.usage; + return (unsigned int)strtoul (r->u.value, NULL, 10); } @@ -516,6 +522,7 @@ create_request (ctrl_t ctrl, ksba_stop_reason_t stopreason; int rc = 0; const char *s; + unsigned int use; err = ksba_certreq_new (&cr); if (err) @@ -576,6 +583,35 @@ create_request (ctrl_t ctrl, rc = err; goto leave; } + + + use = get_parameter_uint (para, pKEYUSAGE); + if (use == GCRY_PK_USAGE_SIGN) + { + /* For signing only we encode the bits: + KSBA_KEYUSAGE_DIGITAL_SIGNATURE + KSBA_KEYUSAGE_NON_REPUDIATION */ + err = ksba_certreq_add_extension (cr, oidstr_keyUsage, 1, + "\x03\x02\x06\xC0", 4); + } + else if (use == GCRY_PK_USAGE_ENCR) + { + /* For encrypt only we encode the bits: + KSBA_KEYUSAGE_KEY_ENCIPHERMENT + KSBA_KEYUSAGE_DATA_ENCIPHERMENT */ + err = ksba_certreq_add_extension (cr, oidstr_keyUsage, 1, + "\x03\x02\x04\x30", 4); + } + else + err = 0; /* Both or none given: don't request one. */ + if (err) + { + log_error ("error setting the key usage: %s\n", + gpg_strerror (err)); + rc = err; + goto leave; + } + do { diff --git a/sm/decrypt.c b/sm/decrypt.c index de53af9e7..8ac2e23fe 100644 --- a/sm/decrypt.c +++ b/sm/decrypt.c @@ -77,7 +77,7 @@ prepare_decryption (ctrl_t ctrl, const char *hexkeygrip, const char *desc, if (seskeylen == 24) { /* Smells like a 3-des key. This might happen because a SC has - already done the unpacking. fixme! */ + already done the unpacking. */ } else { @@ -90,18 +90,18 @@ prepare_decryption (ctrl_t ctrl, const char *hexkeygrip, const char *desc, /* FIXME: Actually the leading zero is required but due to the way we encode the output in libgcrypt as an MPI we are not able to encode that leading zero. However, when using a Smartcard we are - doing it the rightway and therefore we have to skip the zero. This + doing it the right way and therefore we have to skip the zero. This should be fixed in gpg-agent of course. */ if (!seskey[n]) n++; - if (seskey[n] != 2 ) /* wrong block type version */ + if (seskey[n] != 2 ) /* Wrong block type version. */ { rc = gpg_error (GPG_ERR_INV_SESSION_KEY); goto leave; } - for (n++; n < seskeylen && seskey[n]; n++) /* skip the random bytes */ + for (n++; n < seskeylen && seskey[n]; n++) /* Skip the random bytes. */ ; n++; /* and the zero byte */ if (n >= seskeylen ) diff --git a/sm/encrypt.c b/sm/encrypt.c index 8cc9a8828..50da92c32 100644 --- a/sm/encrypt.c +++ b/sm/encrypt.c @@ -398,8 +398,8 @@ gpgsm_encrypt (CTRL ctrl, CERTLIST recplist, int data_fd, FILE *out_fp) goto leave; } - /* create a session key */ - dek = xtrycalloc (1, sizeof *dek); /* hmmm: should we put it into secmem?*/ + /* Create a session key */ + dek = xtrycalloc_secure (1, sizeof *dek); if (!dek) rc = OUT_OF_CORE (errno); else diff --git a/sm/export.c b/sm/export.c index 2b675bdd7..3f7457502 100644 --- a/sm/export.c +++ b/sm/export.c @@ -669,7 +669,7 @@ export_p12 (const unsigned char *certimg, size_t certimglen, protect tool to figure out better error codes for CHILD_ERR. */ buffer[pos++] = c; - if (pos >= 5 /*sizeof buffer - 1*/ || c == '\n') + if (pos >= sizeof buffer - 5 || c == '\n') { buffer[pos - (c == '\n')] = 0; if (cont_line) diff --git a/sm/gpgsm.c b/sm/gpgsm.c index e1751a7aa..bf053b7a5 100644 --- a/sm/gpgsm.c +++ b/sm/gpgsm.c @@ -1197,11 +1197,17 @@ main ( int argc, char **argv) set_debug (debug_level); - /* FIXME: should set filenames of libgcrypt explicitly - * gpg_opt_homedir = opt.homedir; */ + /* Although we alwasy use gpgsm_exit, we better install a regualr + exit handler so that at least the secure memory gets wiped + out. */ + if (atexit (emergency_cleanup)) + { + log_error ("atexit failed\n"); + gpgsm_exit (2); + } - /* must do this after dropping setuid, because the mapping functions - may try to load an module and we may have disabled an algorithm */ + /* Must do this after dropping setuid, because the mapping functions + may try to load an module and we may have disabled an algorithm. */ if ( !gcry_cipher_map_name (opt.def_cipher_algoid) || !gcry_cipher_mode_from_oid (opt.def_cipher_algoid)) log_error (_("selected cipher algorithm is invalid\n")); @@ -1218,7 +1224,7 @@ main ( int argc, char **argv) if (log_get_errorcount(0)) gpgsm_exit(2); - /* set the random seed file */ + /* Set the random seed file. */ if (use_random_seed) { char *p = make_filename (opt.homedir, "random_seed", NULL); gcry_control (GCRYCTL_SET_RANDOM_SEED_FILE, p); diff --git a/sm/keydb.c b/sm/keydb.c index 8832643b9..6c5c77364 100644 --- a/sm/keydb.c +++ b/sm/keydb.c @@ -412,7 +412,7 @@ lock_all (KEYDB_HANDLE hd) int i, rc = 0; /* Fixme: This locking scheme may lead to deadlock if the resources - are not added in the same order all processes. We are + are not added in the same order by all processes. We are currently only allowing one resource so it is not a problem. */ for (i=0; i < hd->used; i++) { @@ -1051,7 +1051,7 @@ classify_user_id (const char *name, * we set it to the correct value right at the end of this function */ memset (desc, 0, sizeof *desc); *force_exact = 0; - /* skip leading spaces. Fixme: what about trailing white space? */ + /* Skip leading spaces. Fixme: what about trailing white space? */ for(s = name; *s && spacep (s); s++ ) ; diff --git a/sm/sign.c b/sm/sign.c index 3340f1066..5deef6088 100644 --- a/sm/sign.c +++ b/sm/sign.c @@ -456,8 +456,8 @@ gpgsm_sign (CTRL ctrl, CERTLIST signerlist, unsigned char *digest; size_t digest_len; /* Fixme do this for all signers and get the algo to use from - the signer's certificate - does not make mich sense, bu we - should do this consistent as we have already done it above */ + the signer's certificate - does not make mich sense, but we + should do this consistent as we have already done it above. */ algo = GCRY_MD_SHA1; hash_data (data_fd, data_md); digest = gcry_md_read (data_md, algo); diff --git a/sm/verify.c b/sm/verify.c index 4b43ed064..410e86de7 100644 --- a/sm/verify.c +++ b/sm/verify.c @@ -86,7 +86,6 @@ gpgsm_verify (CTRL ctrl, int in_fd, int data_fd, FILE *out_fp) int i, rc; Base64Context b64reader = NULL; Base64Context b64writer = NULL; - gpg_error_t err; ksba_reader_t reader; ksba_writer_t writer = NULL; ksba_cms_t cms = NULL; @@ -135,19 +134,15 @@ gpgsm_verify (CTRL ctrl, int in_fd, int data_fd, FILE *out_fp) } } - err = ksba_cms_new (&cms); - if (err) - { - rc = gpg_error (GPG_ERR_ENOMEM); - goto leave; - } + rc = ksba_cms_new (&cms); + if (rc) + goto leave; - err = ksba_cms_set_reader_writer (cms, reader, writer); - if (err) + rc = ksba_cms_set_reader_writer (cms, reader, writer); + if (rc) { log_error ("ksba_cms_set_reader_writer failed: %s\n", - gpg_strerror (err)); - rc = err; + gpg_strerror (rc)); goto leave; } @@ -163,11 +158,10 @@ gpgsm_verify (CTRL ctrl, int in_fd, int data_fd, FILE *out_fp) is_detached = 0; do { - err = ksba_cms_parse (cms, &stopreason); - if (err) + rc = ksba_cms_parse (cms, &stopreason); + if (rc) { - log_error ("ksba_cms_parse failed: %s\n", gpg_strerror (err)); - rc = err; + log_error ("ksba_cms_parse failed: %s\n", gpg_strerror (rc)); goto leave; } @@ -238,7 +232,6 @@ gpgsm_verify (CTRL ctrl, int in_fd, int data_fd, FILE *out_fp) } cert = NULL; - err = 0; for (signer=0; ; signer++) { char *issuer = NULL; @@ -249,18 +242,18 @@ gpgsm_verify (CTRL ctrl, int in_fd, int data_fd, FILE *out_fp) size_t msgdigestlen; char *ctattr; - err = ksba_cms_get_issuer_serial (cms, signer, &issuer, &serial); - if (!signer && gpg_err_code (err) == GPG_ERR_NO_DATA + rc = ksba_cms_get_issuer_serial (cms, signer, &issuer, &serial); + if (!signer && gpg_err_code (rc) == GPG_ERR_NO_DATA && data_fd == -1 && is_detached) { log_info ("certs-only message accepted\n"); - err = 0; + rc = 0; break; } - if (err) + if (rc) { - if (signer && err == -1) - err = 0; + if (signer && rc == -1) + rc = 0; break; } @@ -275,19 +268,18 @@ gpgsm_verify (CTRL ctrl, int in_fd, int data_fd, FILE *out_fp) log_printf ("\n"); } - err = ksba_cms_get_signing_time (cms, signer, sigtime); - if (gpg_err_code (err) == GPG_ERR_NO_DATA) + rc = ksba_cms_get_signing_time (cms, signer, sigtime); + if (gpg_err_code (rc) == GPG_ERR_NO_DATA) *sigtime = 0; - else if (err) + else if (rc) { - log_error ("error getting signing time: %s\n", gpg_strerror (err)); - *sigtime = 0; /* FIXME: we can't encode an error in the time - string. */ + log_error ("error getting signing time: %s\n", gpg_strerror (rc)); + *sigtime = 0; /* (we can't encode an error in the time string.) */ } - err = ksba_cms_get_message_digest (cms, signer, - &msgdigest, &msgdigestlen); - if (!err) + rc = ksba_cms_get_message_digest (cms, signer, + &msgdigest, &msgdigestlen); + if (!rc) { size_t is_enabled; @@ -304,24 +296,26 @@ gpgsm_verify (CTRL ctrl, int in_fd, int data_fd, FILE *out_fp) goto next_signer; } } - else if (gpg_err_code (err) == GPG_ERR_NO_DATA) + else if (gpg_err_code (rc) == GPG_ERR_NO_DATA) { assert (!msgdigest); - err = 0; + rc = 0; algoid = NULL; algo = 0; } else /* real error */ break; - err = ksba_cms_get_sigattr_oids (cms, signer, - "1.2.840.113549.1.9.3",&ctattr); - if (!err) + rc = ksba_cms_get_sigattr_oids (cms, signer, + "1.2.840.113549.1.9.3", &ctattr); + if (!rc) { const char *s; if (DBG_X509) - log_debug ("signer %d - content-type attribute: %s", signer, ctattr); + log_debug ("signer %d - content-type attribute: %s", + signer, ctattr); + s = ksba_cms_get_content_oid (cms, 1); if (!s || strcmp (ctattr, s)) { @@ -334,13 +328,13 @@ gpgsm_verify (CTRL ctrl, int in_fd, int data_fd, FILE *out_fp) ksba_free (ctattr); ctattr = NULL; } - else if (err != -1) + else if (rc != -1) { log_error ("error getting content-type attribute: %s\n", - gpg_strerror (err)); + gpg_strerror (rc)); goto next_signer; } - err = 0; + rc = 0; sigval = ksba_cms_get_sig_val (cms, signer); @@ -523,13 +517,6 @@ gpgsm_verify (CTRL ctrl, int in_fd, int data_fd, FILE *out_fp) cert = NULL; } rc = 0; - if (err) - { /* FIXME: still needed? */ - log_error ("ksba error: %s\n", gpg_strerror (err)); - rc = err; - } - - leave: ksba_cms_release (cms);