Skip to content

Commit

Permalink
lib-dcrypt, mail-crypt: Fix leaking memory when using non-global keys
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sirainen committed Aug 22, 2018
1 parent e9e2cda commit 9140a10
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 5 deletions.
2 changes: 0 additions & 2 deletions src/lib-dcrypt/istream-decrypt.c
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
}
Expand Down
7 changes: 6 additions & 1 deletion src/lib-dcrypt/istream-decrypt.h
Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion src/plugins/mail-crypt/fs-crypt-common.c
Expand Up @@ -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 *
Expand Down
7 changes: 6 additions & 1 deletion src/plugins/mail-crypt/mail-crypt-plugin.c
Expand Up @@ -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;
Expand Down

0 comments on commit 9140a10

Please sign in to comment.