]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
lib-dcrypt, mail-crypt: Fix leaking memory when using non-global keys
authorTimo Sirainen <timo.sirainen@dovecot.fi>
Tue, 12 Jun 2018 14:08:04 +0000 (17:08 +0300)
committerTimo Sirainen <timo.sirainen@dovecot.fi>
Wed, 22 Aug 2018 09:44:33 +0000 (12:44 +0300)
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
src/lib-dcrypt/istream-decrypt.h
src/plugins/mail-crypt/fs-crypt-common.c
src/plugins/mail-crypt/mail-crypt-plugin.c

index ecce5a55d9331b1d106c6cbc897d7341d40ecb41..537f489fef9d2eba18151662e9bd506d657b828a 100644 (file)
@@ -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;
                        }
index 021719ea1c14ca53031bbc219d3a891f092a47da..d531c004b8e1f005bcca4d394fe0fef23672e7b1 100644 (file)
@@ -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,
index eb60ff137179cd830dd590f33415aac876a18730..d0c1b1086e913089b2140816c7b4ae882e3e51f3 100644 (file)
@@ -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 *
index da80f34c2a9de0c006b155fd6ad289ca272a51b4..43ece3d3b5e3ea17e072a852ff3d7ddaee7b21b7 100644 (file)
@@ -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;