rnpgp / rnp

RNP: high performance C++ OpenPGP library used by Mozilla Thunderbird
https://www.rnpgp.org
Other
196 stars 55 forks source link

Memory overhead for in-memory keyring is > factor 130 #1181

Closed kaie closed 4 years ago

kaie commented 4 years ago

I have a migrated keyring from GnuPG, which has 980 public keys in it. Size of the file on disk is: 53 MB

In Thunderbird I'd like to load the keyring once using rnp_load_keys and keep it in memory, and work with is as necessary.

However, RNP requires a lot of additional memory to hold those keys.

I've compared loading the keyring, with the application's memory footprint when not loading it. With loading enabled, after loading, the virtual memory size is 7000 MB more, and the resident memory size is 5500 MB more.

I reproduced this using the rnpkeys application. Using rnpkeys --list-keys, I see similar numbers of memory consumption by the rnpkeys process.

Do you understand why RNP requires that much memory to hold the keys? Are there leaks? Or do you build extensive data structures? Even if you did, it's not clear why you would need that much memory.

@pbrunschwig @mkmelin

kaie commented 4 years ago

May keys contain compressed data? Could I have poisoned keys, downloaded from keyservers, which are bloated?

kaie commented 4 years ago

Using https://tech.michaelaltfield.net/2019/07/14/mitigating-poisoned-pgp-certificates/ for analysis, I don't seem to have poisoned keys. The largest key is 2047355 bytes, and it doesn't have a poison-like peak of signatures on a given date. Most is 280. Probably someone having attended key signing parties.

ni4 commented 4 years ago

@kaie Thanks for reporting, I see the comparable behavior when loading the huge keyring. We should not have such significant memory leaks since all tests are also run with allocation/memory sanitizers.

Most likely it is related to large memory overhead on C unions, used to store signature/signature subpacket/key info. Since it was originally C code, instead of dynamic allocation we used statically sized mpi for those types, so current sizes of those are: sig size: 4176, key size: 12720, subpkt size: 4200. I.e. for 1000 keys, with 20 signatures each, and at least 3 subpackets each, that would be around 500Mb of memory. Multiplied by 2 since of public + secret keyring, and again by 2 because imported keys are stored in temporary keyring, that would give the same magnitude of memory as you've got.

Definitely requires fixing.

ni4 commented 4 years ago

@kaie PR #1207 with memory-related improvements is merged, so could you please check, once you have time, whether loading of huge keyring doesn't consume a lot of memory now? In my tests on macOS, keyring which used to occupy 2-3Gb, now takes 150-170Mb of memory.

kaie commented 4 years ago

Thanks, that's a very good improvement.

ronaldtse commented 4 years ago

@kaie now that #1207 is merged the problem seems to have gone away. Could we close this? Thanks!

kaie commented 4 years ago

ok thanks again