(In reply to comment #53)
> (In reply to comment #46)
> > Empathy: http://cgit.collabora.com/git/user/xclaesse/empathy.git/log/?h=otr
> 
> Ok for the first commit.
> 
> Second commit:
> 
> +	tuple = empathy_gdbus_channel_interface_otr1_get_remote_fingerprint (
> +		priv->otr_proxy);
> I have no idea how these new generated API work, but GVariant API are
> usually 'return: (transfer full)'  that's not the case here?

No it returns the cached variant. There is a _dup_ method as well in the generated API.

> +	level = empathy_gdbus_channel_interface_otr1_get_trust_level (
> +		priv->otr_proxy);
> I guess this returns a cached value (not a blocking call) right? What
> happens if the proxy is not ready yet? Aren't we going to treat it as a
> wrong level and update it right after?

Properties are fetched and cached when creating the proxy, there is a _new_for_bus_sync() call. I added an extra patch to make it async now.

> +		g_variant_get (tuple, "(&s@ay)", &fp, NULL);
> What's the 'ay' arg being ignored? Please add at least one comment.

The fingerprint I send over dbus is in 2 forms: the 's' is formatted to display to the user and the 'ay' is the raw data of the fingerprint which cannot be displayed since it's not utf8.

> What happens if the user doesn't trust the fingerprint. The communication is
> still crypted?

By default it's not encrypted at all. When one side does "/otr start" it will be encrypted but a MITM could have set its own public key instead. So both sides must verify the fingerprint by other way (like calling, or asking IRL, etc) then if they checked the the fingerprint wasn't changed by a MITM they can do "/otr trust" and it will remember that <email address hidden> has that fingerprint and if future conversations uses that fingerprint then it's still trusted. That's why we have different TrustLevel:

TRUST_LEVEL_NOT_PRIVATE: it means there is no OTR at all, plain text.
TRUST_LEVEL_UNVERIFIED: it means it is encrypted but we don't know to who we are speaking. Could be encrypted with the key of an attacker...
TRUST_LEVEL_PRIVATE: it means it is encrypted and the user verified that he is talking to the real person.
TRUST_LEVEL_FINISHED: actually not sure, it's when one side stopped the otr session, probably same has NOT_PRIVATE.

> +	 N_("/otr <action>: Interact with the Off-The-Record system. Possible
> actions are:\n"
> 
> Is there a way to check (without changing) the current trust level?

Yes, "/otr status"

> +		 
> 
> +		empathy_gdbus_channel_interface_otr1_call_initialize (
> +			priv->otr_proxy, NULL, NULL, NULL);
> 
> How does the user know if the operation succeeded or not? Just wait for the
> level update message? I think we should explicitely say if it failed so user
> explicitely know the conversation is not "safe".

Yep, the user knows it succeeded when he sees the trust level change message. If it fails then trust level won't change and user doesn't know what happens. Handling the error here will only mean something failed on the DBus level, we have no way to know on the OTR level if it failed. For example if the other side does not support OTR at all, our initialization message we send won't receive any reply and that's it... You are not safe unless explicitly told you're safe. When in doubt, assume you're not safe.

> +		g_variant_get (tuple, "(s@ay)", NULL, &fp_variant);
> I think fp_variant is leaked.

Hm, no, it is unreffed after calling empathy_gdbus_channel_interface_otr1_call_trust_fingerprint().

> chat_command_otr() will crash/assert if one of the D-Bus API failed. Also,
> shouldn't we use async API here?

The proxy caches properties locally, so it cannot fail AFAIK. If it fails to fetch properties empathy_gdbus_channel_interface_otr1_proxy_new_for_bus() would have failed. There is no blocking calls there.

> trust_level_to_str(): I'd mention "encrypt using OTR" to be clearer and
> avoid confusion my server encryption.

Fixed.