From 9140a101998296ff7e941e812f61d1f0e70c21f5 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Tue, 12 Jun 2018 17:08:04 +0300 Subject: [PATCH] lib-dcrypt, mail-crypt: Fix leaking memory when using non-global keys The users' private keys had one reference too much. Because of key cache, most likely the keys were leaked only once at deinit. Changed the i_stream_create_decrypt_callback() API so that it allows the callback to create the key itself without having to store it anywhere. In this case the key was already added to cache, which increased its refcount. So an alternative fix would have been to simply unreferenced the key before returning it. It's a bit ugly though to rely on such caches, since without the cache the code would be buggy. --- src/lib-dcrypt/istream-decrypt.c | 2 -- src/lib-dcrypt/istream-decrypt.h | 7 ++++++- src/plugins/mail-crypt/fs-crypt-common.c | 5 ++++- src/plugins/mail-crypt/mail-crypt-plugin.c | 7 ++++++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/lib-dcrypt/istream-decrypt.c b/src/lib-dcrypt/istream-decrypt.c index ecce5a55d9..537f489fef 100644 --- a/src/lib-dcrypt/istream-decrypt.c +++ b/src/lib-dcrypt/istream-decrypt.c @@ -167,7 +167,6 @@ i_stream_decrypt_read_header_v1(struct decrypt_istream *stream, "Private key not available"); return -1; } - dcrypt_key_ref_private(stream->priv_key); } else { io_stream_set_error(&stream->istream.iostream, "Private key not available"); @@ -378,7 +377,6 @@ i_stream_decrypt_key(struct decrypt_istream *stream, const char *malg, return -1; } if (ret > 0) { - dcrypt_key_ref_private(stream->priv_key); have_key = TRUE; break; } diff --git a/src/lib-dcrypt/istream-decrypt.h b/src/lib-dcrypt/istream-decrypt.h index 021719ea1c..d531c004b8 100644 --- a/src/lib-dcrypt/istream-decrypt.h +++ b/src/lib-dcrypt/istream-decrypt.h @@ -10,7 +10,12 @@ enum decrypt_istream_format { }; /* Look for a private key for a specified public key digest and set it to - priv_key_r. Returns 1 if ok, 0 if key doesn't exist, -1 on internal error. */ + priv_key_r. Returns 1 if ok, 0 if key doesn't exist, -1 on internal error. + + Note that the private key will be unreferenced when the istream is + destroyed. If the callback is returning a persistent key, it must reference + the key first. (This is required, because otherwise a key newly created by + the callback couldn't be automatically freed.) */ typedef int i_stream_decrypt_get_key_callback_t(const char *pubkey_digest, struct dcrypt_private_key **priv_key_r, diff --git a/src/plugins/mail-crypt/fs-crypt-common.c b/src/plugins/mail-crypt/fs-crypt-common.c index eb60ff1371..d0c1b1086e 100644 --- a/src/plugins/mail-crypt/fs-crypt-common.c +++ b/src/plugins/mail-crypt/fs-crypt-common.c @@ -252,7 +252,10 @@ fs_crypt_istream_get_key(const char *pubkey_digest, return -1; *priv_key_r = mail_crypt_global_key_find(&file->fs->keys, pubkey_digest); - return *priv_key_r == NULL ? 0 : 1; + if (*priv_key_r == NULL) + return 0; + dcrypt_key_ref_private(*priv_key_r); + return 1; } static struct istream * diff --git a/src/plugins/mail-crypt/mail-crypt-plugin.c b/src/plugins/mail-crypt/mail-crypt-plugin.c index da80f34c2a..43ece3d3b5 100644 --- a/src/plugins/mail-crypt/mail-crypt-plugin.c +++ b/src/plugins/mail-crypt/mail-crypt-plugin.c @@ -127,17 +127,22 @@ static int mail_crypt_istream_get_private_key(const char *pubkey_digest, *priv_key_r = mail_crypt_global_key_find(&muser->global_keys, pubkey_digest); - if (*priv_key_r != NULL) return 1; + if (*priv_key_r != NULL) { + dcrypt_key_ref_private(*priv_key_r); + return 1; + } struct mail_namespace *ns = mailbox_get_namespace(_mail->box); if (ns->type == MAIL_NAMESPACE_TYPE_SHARED) { ret = mail_crypt_box_get_shared_key(_mail->box, pubkey_digest, priv_key_r, error_r); + /* priv_key_r is already referenced */ } else if (ns->type != MAIL_NAMESPACE_TYPE_PUBLIC) { ret = mail_crypt_get_private_key(_mail->box, pubkey_digest, FALSE, FALSE, priv_key_r, error_r); + /* priv_key_r is already referenced */ } else { *error_r = "Public emails cannot have keys"; ret = -1;