From 954a31d7a4b4b6c4d8f4eea7dccea3cece5213aa Mon Sep 17 00:00:00 2001 From: Eduardo Chappa Date: Sun, 20 May 2018 11:51:51 -0600 Subject: * Move freeing for some SSL memory until after all connections are closed. * A message could fail to verify its signature even if the certificate was saved when the message was open. Based on a report by David Woodhouse to the RedHat bugzilla system. --- alpine/alpine.c | 8 ++++---- pith/pine.hlp | 10 +++++++++- pith/send.c | 4 +++- pith/smime.c | 53 ++++++++++++++++++++++++++++++++--------------------- 4 files changed, 48 insertions(+), 27 deletions(-) diff --git a/alpine/alpine.c b/alpine/alpine.c index 0ebb9534..917e0cee 100644 --- a/alpine/alpine.c +++ b/alpine/alpine.c @@ -3234,10 +3234,6 @@ goodnight_gracey(struct pine *pine_state, int exit_val) dprint((7, "goodnight_gracey: close config files\n")); -#ifdef SMIME - smime_deinit(); -#endif - free_pinerc_strings(&pine_state); strncpy(msg, pf, sizeof(msg)); @@ -3254,6 +3250,10 @@ goodnight_gracey(struct pine *pine_state, int exit_val) ps_global->noshow_error = 1; sp_end(); +#ifdef SMIME + smime_deinit(); +#endif + /* after sp_end, which might call a filter */ completely_done_with_adrbks(); diff --git a/pith/pine.hlp b/pith/pine.hlp index 3a4f2b48..b17f57e3 100644 --- a/pith/pine.hlp +++ b/pith/pine.hlp @@ -140,7 +140,7 @@ with help text for the config screen and the composer that didn't have any reasonable place to be called from. Dummy change to get revision in pine.hlp ============= h_revision ================= -Alpine Commit 268 2018-05-02 20:16:40 +Alpine Commit 276 2018-05-20 11:51:40 ============= h_news ================= @@ -236,6 +236,14 @@ Bugs that have been addressed include:
  • If the directory where Alpine saves the certificates if empty, alpine would not create a self-signed certificate to encrypt the password file. + +
  • S/MIME: The list of public certificates is freed before it is + reused when a signature fails to verify. This causes Alpine to + crash. Patch submitted by Linus Torvalds. + +
  • S/MIME: A message could fail to verify its signature even if the + certificate was saved when the message was open. Based on a report + by David Woodhouse to the RedHat bugzilla system.

    diff --git a/pith/send.c b/pith/send.c index 460f6a24..dc5a500f 100644 --- a/pith/send.c +++ b/pith/send.c @@ -3711,8 +3711,10 @@ posting_characterset(void *data, char *preferred_charset, MsgPart mp) */ charsetmap = init_charsetchecker(preferred_charset); - if(!charsetmap) + if(!charsetmap){ + if (ucs != NULL) fs_give((void **) &ucs); return(utf8); + } validbitmap = ~0; while((validbitmap & ~0x1) && (*ucsp)){ diff --git a/pith/smime.c b/pith/smime.c index ebafe268..79a980e4 100644 --- a/pith/smime.c +++ b/pith/smime.c @@ -75,7 +75,7 @@ static void setup_storage_locations(void); static int copy_container_to_dir(WhichCerts which); static int do_fiddle_smime_message(BODY *b, long msgno, char *section); void setup_privatekey_storage(void); -int smime_extract_and_save_cert(PKCS7 *p7, int check_cert); +int smime_extract_and_save_cert(PKCS7 *p7); int same_cert(X509 *, X509 *); #ifdef PASSFILE int load_key_and_cert(char *pathkeydir, char *pathcertdir, char **keyfile, char **certfile, EVP_PKEY **pkey, X509 **pcert); @@ -2745,42 +2745,56 @@ int same_cert(X509 *x, X509 *cert) } -/* extract and save certificates from a PKCS7 package. The ctype variable - * tells us if we want to extract it to a public/ or a ca/ directory. The - * later makes sense only for recoverable errors (errors that can be fixed - * by saving to the ca/ directory before we verify the signature). +/* extract and save certificates from a PKCS7 package. * Return value: - * 0 - no errors (in public/) no need to try again, - * or validated self signed certificate (in ca/) - * < 0 - certificate error is not recoverable, don't even think about it. + * 0 - no errors. Either the certificate was in public/ + * or we could save it there. + * < 0 - the certificate was not in public/ and the user + * did not save it there. */ -int smime_extract_and_save_cert(PKCS7 *p7, int check_cert) +int +smime_extract_and_save_cert(PKCS7 *p7) { STACK_OF(X509) *signers; X509 *x, *cert; char **email; - int i, j; + int i, j, rv, already_saved; long error; + /* any signers for this message? */ if((signers = PKCS7_get0_signers(p7, NULL, 0)) == NULL) return -1; + rv = 0; for(i = 0; i < sk_X509_num(signers); i++){ if((x = sk_X509_value(signers,i)) == NULL) continue; if((email = get_x509_subject_email(x)) != NULL){ for(j = 0; email[j] != NULL; j++){ - if((cert = get_cert_for(email[j], Public, 1)) == NULL - || same_cert(x, cert) == 0){ - if(check_cert == 0 - || smime_validate_cert(x, &error) == 0 - || (*pith_smime_confirm_save)(email[j]) == 1) - save_cert_for(email[j], x, Public); + already_saved = 0; + /* check if we have the certificate for this address */ + cert = get_cert_for(email[j], Public, 1); + /* if we have one, check if it is the one packaged */ + if(cert != NULL){ + already_saved = same_cert(x, cert); + X509_free(cert); } + + /* if not saved, try to save it */ + if(already_saved == 0 + && (*pith_smime_confirm_save)(email[j]) == 1) + save_cert_for(email[j], x, Public); + + /* check if it got saved */ + cert = get_cert_for(email[j], Public, 1); + /* if saved, all is good */ if(cert != NULL) X509_free(cert); + else /* else, we do not have this certificate saved */ + rv += -1; + fs_give((void **) &email[j]); } fs_give((void **) email); @@ -2788,7 +2802,7 @@ int smime_extract_and_save_cert(PKCS7 *p7, int check_cert) } sk_X509_free(signers); - return 0; + return rv; } /* @@ -3035,7 +3049,6 @@ do_detached_signature_verify(BODY *b, long msgno, char *section) PART *p; int result, modified_the_body = 0; int flag; /* 1 silent, 0 not silent */ - int saved = 0; unsigned long mimelen, bodylen; char newSec[100], *mimetext, *bodytext; char *what_we_did; @@ -3079,9 +3092,7 @@ do_detached_signature_verify(BODY *b, long msgno, char *section) BIO_write(in, mimetext, mimelen); BIO_write(in, bodytext, bodylen); - saved = smime_extract_and_save_cert(p7, F_ON(F_USE_CERT_STORE_ONLY, ps_global)); - if(saved < 0 && F_ON(F_USE_CERT_STORE_ONLY, ps_global)) - return modified_the_body; + smime_extract_and_save_cert(p7); if((result = do_signature_verify(p7, in, NULL, 1)) == 0){ flag = (mimelen == 0 || !IS_REMOTE(ps_global->mail_stream->mailbox)) -- cgit v1.2.3-54-g00ecf