1646367 - Crash after: delete OpenPGP key, then import again

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect, P1)

Description

[Kai Engert [:KaiE:]]

I have the key for kaie at thunderbird already imported at the time I start TB.
Open key management, delete that key.
Go to the email that has that key attached.
Right click import.
Crash

stack:

#5  0x0000780d678d6730 in <signal handler called> () at /lib/x86_64-linux-gnu/libpthread.so.0
#6  0x0000780d4eea9432 in std::char_traits<char>::assign(char&, char const&) (__c1=<optimized out>, __c2=<optimized out>) at /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/char_traits.h:287
#7  0x0000780d4eea9432 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_set_length(unsigned long) (this=0x780d50509198, __n=0)
    at /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/basic_string.h:206
#8  0x0000780d4eea9432 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator=(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&)
    (this=0x780d50509198, __str=...) at /usr/lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/basic_string.h:752
#9  0x0000780d4eea649e in pgp_revoke_t::operator=(pgp_revoke_t&&) (this=<optimized out>) at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/lib/types.h:364
#10 0x0000780d4eea649e in pgp_key_clear_revokes(pgp_key_t*) (key=<optimized out>) at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:234
#11 0x0000780d4eea6028 in pgp_subkey_refresh_data(pgp_key_t*, pgp_key_t*) (sub=0x780d50506010, key=<optimized out>) at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:1158
#12 0x0000780d4eea89ce in pgp_key_revalidate_updated(pgp_key_t*, rnp_key_store_t*) (key=0x780d446e6010, keyring=<optimized out>) at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:2090
#13 0x0000780d4eebf17d in rnp_key_store_import_key(rnp_key_store_t*, pgp_key_t*, bool, pgp_key_import_status_t*) (keyring=0x780d4fe79a00, srckey=<optimized out>, pubkey=<optimized out>, status=0x7ffdd109b580)
    at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/librekey/rnp_key_store.cpp:557
#14 0x0000780d4eeac6c9 in rnp_import_keys(rnp_ffi_t, rnp_input_t, uint32_t, char**) (ffi=0x780d4f68af90, input=<optimized out>, flags=<optimized out>, results=0x0)
    at /home/user/moz/commcent/mozilla/comm/third_party/rnp/src/lib/rnp.cpp:1311

found by Magnus while testing bug 1646331 (which is difficult to reproduce)

Comment 1

[Kai Engert [:KaiE:]]

Can reproduce on 78 beta branch, too.

Nickolay, could you please have a look at this crash stack? Is it sufficient to allow you to identify the possible cause for the crash?

Comment 2

[Nickolay Olshevsky]

Kai, does this happen with every key within this scenario, or only with some particular one?
In the last case could I have that particular key right as it was sent in email?

Comment 3

[Kai Engert [:KaiE:]]

Attachment: OpenPGP_0x40DC6189F9269749.asc

This is the key that triggers the crash with the stack above (delete, then import).

Comment 4

[Kai Engert [:KaiE:]]

Attachment: OpenPGP_0x1C27423725007724.asc

When testing a different key (this one), I ran into a crash with a different stack:

#5  0x00007b8dfa880730 in <signal handler called> () at /lib/x86_64-linux-gnu/libpthread.so.0
#6  0x00007b8de19a6271 in pgp_subkey_validate_self_signatures(pgp_key_t*, pgp_key_t*) (sub=0x7b8dd8243010, key=0x7b8ddadb5010) at /home/user/moz/comm-beta/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:1103
#7  0x00007b8de19a8557 in pgp_key_validate_subkey(pgp_key_t*, pgp_key_t*) (subkey=0x7b8dd8243010, key=0x7b8ddadb5010) at /home/user/moz/comm-beta/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:2033
#8  0x00007b8de19a89c3 in pgp_key_revalidate_updated(pgp_key_t*, rnp_key_store_t*) (key=0x7b8ddadb5010, keyring=<optimized out>) at /home/user/moz/comm-beta/mozilla/comm/third_party/rnp/src/lib/pgp-key.cpp:2089
#9  0x00007b8de19bf17d in rnp_key_store_import_key(rnp_key_store_t*, pgp_key_t*, bool, pgp_key_import_status_t*) (keyring=0x7b8de284ae00, srckey=<optimized out>, pubkey=<optimized out>, status=0x7ffc27bbe760)
    at /home/user/moz/comm-beta/mozilla/comm/third_party/rnp/src/librekey/rnp_key_store.cpp:557
#10 0x00007b8de19ac6c9 in rnp_import_keys(rnp_ffi_t, rnp_input_t, uint32_t, char**) (ffi=0x7b8de2774200, input=<optimized out>, flags=<optimized out>, results=0x0)
    at /home/user/moz/comm-beta/mozilla/comm/third_party/rnp/src/lib/rnp.cpp:1311

I hope you are able to reproduce easily.

If not, we'll have to investigate the TB code.
I cannot completely rule out the possibility that there's a bug in the C API definitions that we use in the JavaScript to C bridge.

Comment 5

[Nickolay Olshevsky]

Thanks, Kai and Magnus. I was able to reproduce this issue. Recently reported crashes bp-4355857e-94f1-4453-9e74-a72e00200615 and bp-79add039-c2c4-4b5f-99af-03a4c0200615 are caused by this issue.
Will let you know once fix is available.

Comment 6

[Nickolay Olshevsky]

Btw, just a side note - did you consider to build Thunderbird betas with sanitizers? That could give a lot more information, allowing to find the crash source much quicker.

Comment 7

[Kai Engert [:KaiE:]]

(In reply to Nickolay Olshevsky from comment #5)

Thanks, Kai and Magnus. I was able to reproduce this issue. Recently reported crashes bp-4355857e-94f1-4453-9e74-a72e00200615 and bp-79add039-c2c4-4b5f-99af-03a4c0200615 are caused by this issue.

https://github.com/rnpgp/rnp/issues/1171

Comment 8

[Magnus Melin [:mkmelin]]

Note: if you add "bp-" (for breakpad, old name) in front of the crash id bugzilla will autolink it. As bp-4355857e-94f1-4453-9e74-a72e00200615 and bp-79add039-c2c4-4b5f-99af-03a4c0200615

Comment 9

[Nickolay Olshevsky]

Update: crash source was detected and fixed, however need some more time to update tests and add some new functionality.

Comment 10

[Nickolay Olshevsky]

Issue was fixed in PR #1176 (https://github.com/rnpgp/rnp/pull/1176).

Kai: please also note the new flag for rnp_key_remove(). Previously it was deleting primary key without the subkeys.

Comment 11

[Kai Engert [:KaiE:]]

(In reply to Nickolay Olshevsky from comment #10)

Kai: please also note the new flag for rnp_key_remove(). Previously it was deleting primary key without the subkeys.

Thanks a lot for making me aware of this detail!

Comment 12

[Kai Engert [:KaiE:]]

Attachment: Bug 1646367 - Fix crash after incomplete delete, use new flag RNP_KEY_REMOVE_SUBKEYS. r=mkmelin

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/552a164af0b8
Fix crash after incomplete delete, use new flag RNP_KEY_REMOVE_SUBKEYS. r=mkmelin

Comment 14

[Kai Engert [:KaiE:]]

Comment on attachment 9158478 [details]
Bug 1646367 - Fix crash after incomplete delete, use new flag RNP_KEY_REMOVE_SUBKEYS. r=mkmelin

Required for correct delete/re-import functionality of OpenPGP keys.

Comment 15

[Wayne Mery (:wsmwk)]

Comment on attachment 9158478 [details]
Bug 1646367 - Fix crash after incomplete delete, use new flag RNP_KEY_REMOVE_SUBKEYS. r=mkmelin

Approved for beta

Comment 16

[Kai Engert [:KaiE:]]

https://hg.mozilla.org/releases/comm-beta/rev/e9d431c06249a0c60e48ed647c6aeedd24ada2fc