Review Of The Drop Message Documentation

[ghost]

Hi @Gottox and @thechauffeur! And also the rest!

Can you review this document. Is that a solution we can live with it https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop

CU k-c13

[thechauffeur]

Here is my review.

• https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#send-message and https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#get-message both refer to the broken link: https://github.com/Qabel/intern-doc/wiki/Qabel-Protocol-Drop#r%C3%BCckgabewerte
• https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#format-and-buildupstructure-of-a-message
  • Key 'id' should be a String. To me it is unclear how this "unique" ID can be generated and a String is more flexible.
  • Key 'sender': What is an ID of the sender? A sender can sign a message - using his private key - so we can define a field "signature". Not a sender ID - where shall this ID come from?
  • Key 'module' should be String - again it is more flexible.
  • Key 'version' should be the version of the Drop Message (protocol) used since we need such a field to distinguish between different Drop Message (protocol) versions. Module specific fields can be in the actual message from the module which generated the message - namely in the module specific JSON in the key 'data'.
  • Key 'type': Again, module specific types can be defined by the module in the message generated by the module. Also see my opinion on reserved-drop-messages below.
• https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#reserved-drop-messages IMHO the key 'type' is useless since I cannot see why we need predefined message types. But we need more opinions on this. Please discuss this one. Hint: The advantage of a version field for the Drop Message (protocol) version is that we can define fields like the type of a Drop Message in a later version of the Drop Message (protocol) if we do not need them now.
• https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#reserved-drop-messages should be split into two sections because "The data JSON object have the following keys:" should be a new section.
• "The data JSON object have the following keys:" I will give you my opinion on the fields below but we need to discuss if we want to further define this field at all. Maybe we should leave this open to the modules.
  • Key 'id' is a good idea (see my opinion on the key 'type' above)
  • Key 'value' must be removed or it has to be defined in a way so that each module can put an arbitrary JSON object into it.
  • IFF we define the data JSON then we should define a key 'version' to distinguish between different versions of the message protocol (for the particular module).
• https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#encryption seems complex - and maybe weird - to me. @Gottox I need your opinion on this.

[zuckschwerdt]

re id/sender/signature: it's common practice to attach a signature object with the fields "signer" (i.e. sender key id), "digest", "signed" (i.e. the signature itself).

"id" in the context of sender. This would be meaningful to the sender only and most likely module specific (e.g. chat threads, ping replys). Is there a reason not to put this into the scope of modules?

re module/type: as you already said, there should be one key ("module" or maybe "type") to route the message to the module and then the module can define it's own "subtype"s.

re crypto: crypto is like that, complex. What specifically is weird here?

[thechauffeur]

• id/sender/signature: I support a field for the fingerprint of the key from the sender plus a field for the signature itself.
• sender "id": I support that this is module specific and I think that the Qabel Drop Message (protocol) definition is the wrong place for this.
• @Gottox and I support that we only need to distinguish between modules for the Qabel Drop Messages and let the modules handle the type of a message.
• Crypto: It's just that... I always thought we encrypt the whole Qabel Drop Message with the recipient's public key (since drop messages shall be short etc.).

[zuckschwerdt]

a signature is always composed of a digest value and an actual signature value. With the fingerprint that's 3 (tightly) related fields.

"encrypting a message with a public key scheme" is usually done that way. The actual public key encryption can carry only a limited amount of data.

[thechauffeur]

• Signature: Aye.
• Encryption: Are you @zuckschwerdt trying to tell me the definition in https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#encryption just describes how encryption (with a public key) works? If that's the case the definition is misleading. It is written as if we would have to define (and implement) how the encryption of the whole Drop Message - using a public PGP key - is done. Like reinventing the wheel, if you know what I mean.

[thechauffeur]

Review addition:

• https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#history--persistence
  • ...refers to "Firefox Sync" which is an implementation of Qabel-Sync. It is bad practice to refer to some specific implementation. Also - in this case - it is even wrong, since we may not even use Firefox Sync anymore.
  • The client Qabel-Drop component should not be in charge for keeping some kind of - not well defined - history. At least not cross-device.

[zuckschwerdt]

ok, I see. It's a step by step guide to use with the currently implemented functions. I don't know if there is a utility function to do this. It should look like PGP if it's done right:) Someone should read RFC4880 and see if we can comply. But it's not needed.

[Gottox]

• https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#format-and-buildupstructure-of-a-message
  • Key 'id': an INT datatype is far to small to allow unique IDs. This should be a string containing a UUID or similiar
  • Key 'module': use string here
  • Key 'type': I dislike the idea of having a preset of protocol defined message types. It should be a string containing not the physical type of the data but the logical type. so instead of "SEND_STRING" it should be i.e. "MESSAGE".

https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#encryption seems complex - and maybe weird - to me. @Gottox I need your opinion on this.

This is a common pattern to encrypt data that way. The only thing I would chang is that I leave the compression out for now. This can be optionally added in a later protocol version.

• https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#history--persistence There is no FF-Sync server for now. How do we recognize messages that need to be tracked?

[zuckschwerdt]

Maximizing entropy before encryption would be recommended here. These messages are much too predictable and we need to care about kown-plaintext attacks. There is some argument that streaming protocols shouldn't use compression as to not reveal the entropy. I'd say that does not apply here.

We should also care about a more coarse padding (i.e. the message length should not indicate the message type.)

[thechauffeur]

@zuckschwerdt true and we should keep that in mind and note it somewhere (maybe a new issue?).

Nevertheless I strongly recommend to focus on getting Qabel to work - and therefore (only) document the things needed to get it to work - before enhancing specific processes to be more secure.

The thing is... we can make Qabel better and better - in theory. In praxis we do not have Qabel and we will never have (before running out of resources) if we do not focus really hard at the moment. This is me generally speaking about Qabel - not only about the referenced comment.

[Gottox]

Most crypto libraries use the combination of an asymmetric cipher with a symmetric one. This is called sealing. This does not need to be done by hand. Nevertheless it is slightly more complex than pure asymmetric encryption, but I would definitely recommend to use sealing.

With the focus of "not living in ponyland anymore" I recommend to drop compression for now, it adds a layer of indirection and obfuscation. We need to set our focus from "doing it right the first time" to "make it easier for the developer". I'm with 100% @thechauffeur in this point.

JSON a defined set of first bytes (0-9, [, {, -, t, f, n and whitespaces), so using a non-valid json-character as magic byte in later protocol specs is feasible.

[Gottox]

Key 'time_stamp'

This key describes the date of generation of a message.

The format of the timestamp is not described. I suspect, that Unix timestamp was planned. (A little sidenote: This should be long instead of int in the actual program.)

But apart from these stated issues, it looks fine to me.

[ghost]

@thechauffeur https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#reserved-drop-messages: There are two ways to it. One way is: Put everything what a module need in the module part and the module have to do everything. Every module have to write its class to parse a boolean, when you ask the other side a question. The other way: Help the module writer that he easily write code. This is my way. And i defined it on that way. When you look into that side maybe you will understand what I mean: https://github.com/Qabel/qabel-chat-module/wiki/Chat-Messages

And for the other point we will discuss that on Monday.

The other thinks

[thechauffeur]

@k-c13 if you think we have to discus some stuff on Monday it's OK for me but could you work in the things which do not need more discussion?

[ghost]

I did the last changes discuss today. Please check it

[thechauffeur]

Sorry that it took so long @k-c13.

First my review of your patch set.

• Key 'time_stamp': UNIX_TIMESTAMP is not a JSON datatype but I'm OK with it.
• Key 'version': The protocol is called "(Qabel) Drop Message (protocol)" not "drop protocol". The "(Qabel) Drop protocol" is the protocol describing the communication and is independent from the drop message.
• history: Write "TODO: Messages which are needed across all devices of an identity must be shared across all devices. They exact way has to be defined."

[thechauffeur]

@k-c13 you forgot... e.g.

• https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#send-message and https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#get-message both refer to the broken link: https://github.com/Qabel/intern-doc/wiki/Qabel-Protocol-Drop#r%C3%BCckgabewerte
• Key 'sender': What is an ID of the sender? A sender can sign a message - using his private key - so we can define a field "signature". Not a sender ID - where shall this ID come from?
  • What about reading the discussion about sender id vs. signature?

I'm sure other things are missing, too. Can you please work in all the mentioned things? This is way too time consuming to review all this again and again because more than half of the things are not updated.

[ghost]

Key 'sender': I follow the discussion. This id is the id which the sender sends to you when it try to contact you the first time. At the first time, you do not know nothing from him. It is easier to communicate if you send an identifier. And I discuss this point with you and you do not say to me that this is unimportant. So I let it inside.

By the way. You can define the thinks as you believe. I do not longer update this page.

I move that page from a different place because of renaming of thinks an work in progress. And add only my stuff which I have done and do not check the complete text which is also written by other. And when this documents so fast in change so some link can be change or one removed the complete response stuff. I only review it. But I believe that I am the only one which read all pages in the wiki.

When I believe that one key make the life for me as developer easier and you think I do not need it that is not the point to say to me: What about reading the discussion ...

[aBothe]

If the id was a string..could it be possible to store an insanely large String (i.e. base64-encoded file or so) as an ID as well? Shouldn't the identifiers' sizes be limited? If we're discussing about having UUID-formatted strings, then why not limit the string length to e.g. 36 chars or so despite it may not possible for json to limit strings?

[thechauffeur]

It's sad that we will loose time this way but I think we have to discuss this on Monday, again.

[thechauffeur]

I will update the page.

[thechauffeur]

I updated the page to reflect this review and to account for the stuff that came up the last two weeks.

[thechauffeur]

Please review this.

Oh and can someone else define how to sign drop messages? https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#signature A word on using the key ID as the sender id would also be nice. https://github.com/Qabel/intern-doc/wiki/Qabel-Client-Drop#format-and-structure-of-a-message

[L-Henke]

"The key id of the PGP key of sender." - Until now we always spoke about public/private keys without mentioning a specific asymmetric encryption standard (PGP).

[L-Henke]

I would recommend to sign using Encrypt-then-MAC as defined in ISO/IEC 19772:2009 or by utilizing AES-GCM. AES-GCM would allow authenticated encryption in one single operation. The key derived from the private key could be used to for this purpose. http://en.wikipedia.org/wiki/Galois/Counter_Mode

Every (real) cryptographic library should support the AES-GCM mode.

[thechauffeur]

I removed the term PGP.

[L-Henke]

I added a proposal for authenticated encryption using AES-GCM, but I might also change this to another EtM method.

[L-Henke]

I changed the previous signature approach. This PGP like signing better suits our use case. We might have to decide which hash function should be used (or at least be recommended). If someone could tell me where to find the source of the sequence diagrams, I would update these.

[thechauffeur]

The sequence diagrams need an update anyway. I would recommend re-doing them with http://plantuml.sourceforge.net The sources for all pictures etc. get committed into the wiki repository as well (just clone intern-doc.wiki). Which leads me to another hint: Your changes in the wiki text can have commit messages - please use them.

[L-Henke]

@thechauffeur Seems like the sources for the sequence diagrams are missing. I can recreate them with PlantUML and add the signing procedure.

[thechauffeur]

Go ahead @L-Henke. The reason I didn't redone them yet is that I like to use the corresponding participants / entities from the class diagram - and it's not finished yet.

[L-Henke]

Then I'll just copy the current diagrams (and add signing). Changing it later shouldn't be too much work.

[thechauffeur]

Aye.

[thechauffeur]

I like the new diagrams. I also like the scheme used to encrypt and sign the message.

[L-Henke]

I included the updated sequence diagrams. The sources are also in the images folder. Currently the diagrams show the sequences if messages has to be signed. Modeling the sequence of signed and unsigned messages would make the diagram confusing. If we need to model this, I would show this in a secondary diagram (at least for receiving messages)

[cburkert]

I put terminology and the message format description into beautiful tables.

[cburkert]

I think the sentence "The digest is signed with the senders private key." needs improvement. Signing with asymmetric crypto usually means hashing and encryption of the hash digest with the private key. I mean: hashing is part of "signing" and not prior to that.

[L-Henke]

@cburkert Nope. The hash is not encrypted since you want to verify the integrity of the data before decrypting it. http://en.wikipedia.org/wiki/Digital_signature#mediaviewer/File:Digital_Signature_diagram.svg http://de.wikipedia.org/wiki/Pretty_Good_Privacy#Erzeugen_einer_digitalen_Signatur

[L-Henke]

At least normally you should use Encrypt then MAC, since it is the most secure way. And this means that you are hashing the ciphertext. And in our case signing it afterwards.

Edit: PGP was a bad example, since it seems to use MAC then Encrypt, which is outdated.

[cburkert]

@L-Henke Sorry, but the example you referenced exactly confirms my explanation. A signature in asymmetric crypto essentially depends on encrypting the hash with the private key. Otherwise you can not ensure the authenticity.

[L-Henke]

@cburkert I see... This is just a terminology misunderstanding. Of course the hash value is encrypted, but it is usually not called encrypted in this context, but signed. I especially wrote that just the digest is signed, because it is not necessary to just sign a digest. If you want to, you could also sign the whole message, which would be highly unpractical.

I thought you wanted to have the hash value encrypted with the payload data, as in MtE schemes.

[cburkert]

I meant that the term "signing" in the asymmetric crypto world means hashing and encrypting. So I makes no sense to say the hash is signed, since signing implies hashing. That's all I wanted.

[L-Henke]

It is done this way by every asymmetric signing algorithm, but technically it would also work without a hash. Of course no one would ever do this.

But I wouldn't remove the term, because the section describes how we sign in detail. "A digest of the final encrypted message payload including the encrypted AES key, IV, and the ciphertext is created via a hash function like SHA1 or SHA512. The digest is signed with the senders private key."

In a more abstract description I would also write "The messaged is signed...", but not in this more detailed context.

[L-Henke]

I replaced "signed" with "encrypted" because it really is the better term in this sentence.

[cburkert]

@L-Henke Okay. But I'm afraid: I'm now confused whether the unencrypted hash is necessary at all. Wouldn't the following suffice? [256 byte encrypted AES key][16 bytes AES IV][n bytes AES ciphertext][signature (encrypted hash)].

[L-Henke]

An unencrypted should never be necessary. The data should look like you wrote (or as I wrote in https://github.com/Qabel/intern-doc/issues/29#issuecomment-51581475, but without the additional header).

[cburkert]

Good. Then we've to correct the "signature" section because, there you can read "Digest and signature are appended to the previously created encrypted message"

[L-Henke]

Actually this has been already corrected directly after I change the word encryption... "The signature is appended to the previously created encrypted message, forming the authenticated encrypted message."

I don't know where my head was when I wrote that. I meant it like in the comment posted above.

[cburkert]

Reading https://crypto.stackexchange.com/a/5466 made me think, that we should sign the plaintext rather than the ciphertext, because otherwise Mallory could replace signatures.

[L-Henke]

I think the attack stated in the paper doesn't affect us, since our drop messages always contain the sender id. If this sender id doesn't match the signing person, you can detect that the signature has been altered.