(In reply to comment #58)
> Just doing the spec right now:
> 
> > The extra DBus channel interface is implemented using GDBus
> > so it needs to be exported on a different bus name.
> 
> Ugh. Can we not do strange hacks like this, please? Either use the
> extensions mechanism, or save it for 1.0.

I don't want to block on 1.0, and I don't want to write obsolete code with tp-glib's codegen. So that's the necessary evil in the meantime.

> + <interface name="im.telepathy.v1.Channel.Interface.OTR1"
> + tp:causes-havoc="experimental">
> + <tp:added version="Gabble 0.UNRELEASED">(Gabble-specific)</tp:added>
> 
> If doing this in 0.x, please use o.fd.Channel.Interface.OTR1 and add it to
> telepathy-spec (OK to go via extensions/ until we do the spec -> tp-glib
> dance, though).

I can change the iface name but it doesn't matter much. I would like to avoid extensions/ nightmare though, I don't want to write code using that in master and port it again in next.

> In 1.0, certainly add it to the spec.

Yep, planned to include that in the tp1.0 spec, and consider it private between gabble and empathy in the meantime.

> + A simple D-Bus API <a
> + href="https://otr.cypherpunks.ca/">Off The Record</a>.
> 
> "... API for <a..."
> 
> + <p>The current trust level of this channel:
> + 0=TRUST_NOT_PRIVATE, 1=TRUST_UNVERIFIED, 2=TRUST_PRIVATE,
> + 3=TRUST_FINISHED</p>
> 
> This deserves a <tp:enum> and documentation.

I didn't write it because gdbus-codegen does not use it. Opened https://bugzilla.gnome.org/show_bug.cgi?id=729762 already. But I'll add it in our spec in the meantime anyway, even if it's just for the html version of our spec.

> I assume the meanings go something like this:
> 
> TRUST_NOT_PRIVATE: not using OTR at all? (Can we also see this when using
> OTR but something has gone wrong?)
> (o.fd.Channel.I.Securable.Encrypted=FALSE,
> o.fd.Channel.I.Securable.Verified=FALSE)

yes

> TRUST_UNVERIFIED: the channel is encrypted, but you might be talking to a
> man-in-the-middle instead of the peer you expected.
> (o.fd.Channel.I.Securable.Encrypted=TRUE,
> o.fd.Channel.I.Securable.Verified=FALSE)

yes

> TRUST_PRIVATE: the channel is encrypted, and the user has indicated that the
> peer's key fingerprint is trusted to belong to that peer.
> (o.fd.Channel.I.Securable.Encrypted=TRUE,
> o.fd.Channel.I.Securable.Verified=TRUE)

yes

> TRUST_FINISHED: this channel is over, nothing more should be sent or
> received on it.
> (o.fd.Channel.I.Securable.Encrypted and o.fd.Channel.I.Securable.Verified
> keep their previous values?)

I *think* (need to double check) that in that case you'll still be sending encrypted messages but the other side won't be able to decrypt them and will display a message "The encrypted message received from %s is unreadable, as you are not currently communicating privately.".

> What are the possible state transitions? I assume "can only increase"?

I think it can only increase unless the local user did something. Like it can go from PRIVATE to UNVERIFIED if user types "/otr untrust". It currently cannot go back to NOT_PRIVATE because I don't support ending the otr session, but could add a "/otr end" for that. pidgin can do that.

> + type="(say)" access="read">
> + <p>User's current fingerprint. The first element is a human readable
> + fingerprint that can be displayed to the user so he can communicate it
> + to the other end by other means so he can trust it. The 2nd element is
> + the fingerprint raw data.</p>
> 
> Are these literally the hex and binary versions of the same digest, or do
> they have different information content? (Or is the string version some
> OTR-specific thing that is easier to transcribe than hex?)

It is the same information, the string is utf8 (ascii even) to display, the ay is hex form (20 bytes, not utf8). libotr has a function to go hex->ascii but not ascii->hex and when TrustFingerprint is called I need to tell otr which fingerprint is trusted by telling it the hex form. That's why I send both over dbus.

> + <property name="RemoteFingerprint"
> + tp:name-for-bindings="Fingerprint"
> + type="(say)" access="read">
> + <tp:docstring xmlns="http://www.w3.org/1999/xhtml">
> 
> What value does this take when the channel is not using OTR? ('', [])?

yep.

> When we're in the UNVERIFIED state, am I right in thinking that we are
> cryptographically guaranteed to have the right fingerprint for who we're
> talking to, but the thing that is unverified is that the fingerprint belongs
> to the person we wanted to talk to? (i.e. if we're talking to a
> man-in-the-middle, this would be the fingerprint of the man-in-the-middle's
> key, right?)

That's how I understand it, yes.

> Is it possible for this to change? (Presumably from ('', []) to non-empty,
> at the same time that the trust changes to UNVERIFIED or PRIVATE?)

when TrustLevel is UNVERIFIED the RemoteFingerprint cannot be empty.

> After this has become non-empty, can it change further? (I would hope not.)

I think it can if the user ends the OTR session, but I didn't implement that yet. I think if the other end stops the OTR session then trustlevel goes to FINISHED but you'll still be sending encrypted messages and the other side (pidgin-otr) will say "I received an encrypted messages, have no idea what it contains". Need to try that scenario to check.

> I think it would also be useful to spec that one of the forms of the remote
> fingerprint will appear in the message header (0'th part) of each individual
> message, perhaps { "otr-remote-fingerprint": a string }. That would make it
> easy for someone to do either of these things in a race-condition-free way:
> 
> * record in the Logger that the messages were encrypted/verified
> * give the Logger a configuration setting "[ ] do not log OTR messages"
>   (which it would recognize by seeing that they have an OTR remote
> fingerprint

Could be useful, atm the logger still record the conversation. It's a good idea to add that in the message parts. Do you consider that merge blocker? probably users expects their OTR messages to not be stored on disk. otoh if they really care about security and they don't have encrypted HDD I don't know what they are thinking about...

(In reply to comment #59)
> Implementation in Gabble:
> 
> + /* FIXME: There should be no sender for a notification, but setting handle
> to
> + * 0 makes empathy crash atm. */
> + tp_message_mixin_take_received (G_OBJECT (self),
> + tp_cm_message_new_text (base_conn,
> + tp_base_channel_get_target_handle (base_chan),
> + TP_CHANNEL_TEXT_MESSAGE_TYPE_NOTICE, text));
> 
> Is this a message from the OTR library, something like "*** Verified peer
> fingerprint: <email address hidden> ***"?

It is for all messages from otr_handle_msg_event(), so yes it's basically OTR library messages.

> I think using the target handle for this is OK semantically.

yeah, probably, but it could be local-error messages, not something sent by the remote end.

> However, I suspect remote users can spoof this by sending their own NOTICE.
> Messages coming from the OTR library should have a distinctive message
> header that an OTR-literate UI can take as evidence that they were
> locally-generated.
> 
> Ideally, that distinctive message header should be a machine-readable
> version of the message, so OTR-literate UIs (Empathy) can discard the
> untranslated version from Gabble and display something translated. We've
> always had a policy of putting UI strings and their translations in the UIs,
> not the CMs.

You're right, yes. I guess the best is to have a signal on the OTR iface to transmit the OtrlMessageEvent enum.

About translation, there is messages from otr_error_message() as well. Those are sent to the other end on error. We should probably not translate them at all, I don't want you to receive French messages from me. English is the international language, I guess. In a perfect world we could remember the language of each contact...

> + return g_variant_new ("(s@ay)", display_fp,
> + g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, fp_raw, 20,
> ...
> + guchar our_fp_raw[20];
> 
> The magic number 20 makes me nervous. Isn't there a constant for "length of
> a raw OTR fingerprint in bytes" in libotr?

It is hardcoded in libotr API.

> If there really isn't, #define'ing our own would be better than nothing.

Yep, will do that.

> +static void
> +otr_inject_message (void *opdata,
> + const gchar *accountname,
> + const gchar *protocol,
> + const gchar *recipient,
> + const gchar *message)
> +{
> + inject_message (opdata, message);
> +}
> 
> Is @message text/plain or text/html? Telepathy can only do text/plain at the
> moment, so if it's text/html, we need to strip tags, then unescape entities
> (&stuff;).

It can contain html, but I verified, wocky escape it for us.

> +static gint
> +otr_max_message_size (void *opdata,
> + ConnContext *context)
> +{
> + return 0;
> +}
> 
> We should probably give some guess at what's generally interoperable.

pidgin-otr says 0 for xmpp as well. OTR encryption can expand a bit the size of messages, but that's not new that user sending huge text could not be interoperable. I don't think gabble tries to prevent it anywhere.

> + msg = otrl_proto_default_query_msg (get_self_id (self),
> OTRL_POLICY_DEFAULT);
> 
> Do we need to update what otr_policy() would return here, too?

Oh actually that should be changed to MANUAL until we have an option to change that policy. DEFAULT means it adds some whitespace after the first message and if the other end is OTR-enabled then it will start an OTR session. See comment #50, we don't want to force encryption to all users :)

> + bus_name = g_strconcat (tp_base_connection_get_bus_name (base_conn),
> + ".OTR", NULL);
> 
> I suppose this isn't *so* bad, but the spec should tell the API user where
> to find this name.

Yep, I'll add a note in the spec about it.

> + content = wocky_node_get_content_from_child (node, "body");
> +
> + err = otrl_message_sending (userstate, ui_ops_p, self,
> + get_self_id (self), "xmpp", get_target_id (self),
> + priv->instag, content, NULL, &new_content,
> + OTRL_FRAGMENT_SEND_ALL_BUT_LAST, NULL,
> + NULL, NULL);
> 
> Does otrl_message_sending() expect @content to be text/plain or text/html?
> If it expects text/html, we need to escape special characters with
> g_markup_escape_text().

It doesn't care, it get a string, encrypt it, and set new_content to "?OTR:<base64>".

> +gchar *
> +gabble_im_channel_otr_receiving (GabbleIMChannel *self,
> + const gchar *content)
> 
> Same here.

It doesn't matter, if the message is in the form "?OTR:<base64>" then it puts new_content to whatever the original message was (html or not). OTR doesn't change anything if user wants to send html message as plaintext, empathy will escape when displaying them.