[Bug 29904] Support end-to-end encryption and authentication

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=29904

--- Comment #5 from Cosimo Cecchi <cosimoc at gnome.org> 2010-09-03 11:03:09 PDT ---
Simon, thanks for the great review! I fixed your comments in my branch.
Below is some feedback to your open questions.

The only thing among your comments that I did not touch are the added error
reasons you asked for Ch.I.XTLS and Auth.Proposal.I.SRP, because either I'm not
sure what to add (in XTLS case, more specific details for the error are
available on the protocol-specific interface of the chosen proposal object) or
can't really find other relevant error codes (in SRP case; GnuTLS also has only
Invalid_Username and Invalid_Password as SRP-specific errors. OTOH it's true
that other TLS errors could happen there, but do we really need this level of
detail there?)

(In reply to comment #3)

> I wonder whether it's actually appropriate for clients that upgrade to e2e to
> specify how in UpgradeEncryption? It seems to be a design goal that Empathy can
> flip the "encrypt me" bit and Seahorse or something can pop up to do the actual
> crypto handshaking? If that's the case, then we could detect from handlers'
> filters, or perhaps handler capability tokens, what sorts of handshake we have
> UI for.

I think it's not a bad thing to let clients decide which interface they should
upgrade with, but you're right in saying that doesn't need to be mandatory. So,
I made it optional, and it works similarly to what happens if the Encryptable
channel is requested with EncryptionRequired = True but an empty
AuthenticationInterface.

> An alternative API for upgrades would be for a second channel to pop up,
> replacing the first, using a more general form of Conference
> (Chan.I.Continuable?). Naive clients not understanding Continuable would handle
> it as if it was a new channel. That approach is probably harder to work with
> for clients, but might lead to easier conceptual design.

Doesn't this break the 'not having to handle more than one channel for
encryption' rule? Also, I'm not sure what you don't like in
UpgradeEncryption().

> This implicitly assumes that there are only two levels of
> encryption/authentication - "not enough to mention" (which includes "none") and
> "fully secure" - and it doesn't matter which specific mechanism you're using.
> This is probably true, but it's worth thinking about.

Yeah, you're right from an Encryptable interface point of view (you're either
encrypted or you aren't), though the real degree of encryption/authentication
is not something you can easily condense into a property, and in this model it
is given for a channel by this encryptable bit combined with protocol-specific
information (you could e.g. be encrypted on a leap-of-faith authentication,
...). OTOH I don't think it makes any real use allowing to call
UpgradeEncryption() on an already encrypted channel (the only good reason to do
so would be 'get even better encryption', but that's hard to define and tricky
to get for the reasons I outlined before).

> Proposal.Interface should perhaps be Proposal.Type or Proposal.ProposalType,
> consistent with Channel.ChannelType (the model is that channels have exactly
> one type, and n interfaces). Proposal should probably have an Interfaces
> property for future expansion, too.

I changed this to Proposal.ProposalType for consistency with Channel, but
didn't add the Interfaces property yet, as I'm not sure what it would be useful
for right now.
Maybe we can always add it later if it's needed?

> ClientTLSConnection doesn't seem like the right name for its functionality, if
> it's meant to pop up with EncryptionRequired=TRUE already - it's not
> establishing any sort of TLS'd connection, just exchanging certs.
> ClientCertificateExchange, perhaps?

Yes, I renamed it to ClientCertificateExchange.

> Did you get the Requested semantics of ProvideCertificate and
> ReceiveCertificate backwards? They look backwards to me but perhaps I'm not
> seeing it clearly enough.

No, it's not backwards. I hope I made it super-clear how it works now in its
description.

> Why is the received certificate a TLSCertificate and not just a
> Certificate_Data[]? I can't see what it would mean to accept or reject it at
> this stage, since it's just being provided for future reference?

Good point, I changed that.

(In reply to comment #4)
> * PeerAuthentication
> 
> I quite like PeerAuthentication as a name for this, actually.

I chose this too.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.