arlolra / ctypes-otr

js-ctypes wrapper for libotr
Mozilla Public License 2.0
29 stars 7 forks source link

Provide functionality for adding verified fingerprints in management pane #69

Closed vqhuy closed 8 years ago

vqhuy commented 8 years ago

This feature was added as suggestion #66. Added new button "add" in fingerprint management pane and it will open file picker dialog for users to choose the import file. I think allowing user import fingerprints from file is better than forcing them input every record by hand.

arlolra commented 8 years ago

@c633 Thanks a lot for the patch. It looks good, but there're a few things we should think about first.

I think allowing user import fingerprints from file is better than forcing them input every record by hand.

Right, so then what you've implemented here feels more like an "Import" button, closer to a start on #60.

The idea for #66 was explicitly for the case where you've just made a single new contact and now you want to input their fingerprint w/o the need for initiating a session. I imagine the UI would be similar to the File > Add Contact menu, where you choose an account and contact, then paste in their fingerprint, which normalizes it and calls something like what's in otrl_privkey_read_fingerprints_FILEp https://github.com/off-the-record/libotr/blob/master/src/privkey.c#L755-L760

Anyways, there're a few problems with the import button you've implemented. For one, it blindly overwrites trust state. If the current fingerprints file is empty, that's alright. But maybe you've verified a few contacts that coincide with what is being imported. Which state do we prefer? Also, it may change the trust state of an active conversation but doesn't do anything to update it (see setTrust), which is my main objection.

Also, where did you imagine this import file coming from? Any client using libotr generates them (like Pidgin and Adium), but the protocol field might have slight variations requiring some preprocessing (ex. prpl-irc to irc, jabber to xmpp, etc.)

vqhuy commented 8 years ago

The idea for #66 was explicitly for the case where you've just made a single new contact and now you want to input their fingerprint w/o the need for initiating a session. I imagine the UI would be similar to the File > Add Contact menu, where you choose an account and contact, then paste in their fingerprint.

Yup, I think it's a better UX. But the user still need to input new contact's fingerprint manually, so where does he/she get that fingerprint?

Also, it may change the trust state of an active conversation but doesn't do anything to update it (see setTrust), which is my main objection.

Thanks for pointing out the problem.

Anyways, there're a few problems with the import button you've implemented. For one, it blindly overwrites trust state. If the current fingerprints file is empty, that's alright. But maybe you've verified a few contacts that coincide with what is being imported. Which state do we prefer? Also, where did you imagine this import file coming from? Any client using libotr generates them (like Pidgin and Adium), but the protocol field might have slight variations requiring some preprocessing (ex. prpl-irc to irc, jabber to xmpp, etc.)

I have thought about the case that there are coincidences. I thought the import file coming from other devices of the user which is also using Instantbird. Anw, I haven't thought about the case you mentioned yet. So letting the user paste the fingerprint manually after adding new contact from File > Add Contact menu is a good way.

arlolra commented 8 years ago

But the user still need to input new contact's fingerprint manually, so where does he/she get that fingerprint?

Same as for PGP: keysigning parties, trusted channels, etc. Mine's here, https://people.torproject.org/~arlo/otr.txt

Remember, that's why we want CONIKS, to avoid this cumbersome system.

vqhuy commented 8 years ago

Yes, I get it. Should I close this PR?

arlolra commented 8 years ago

Not yet, I'd like to think about it a bit. Thanks.