derecalliance / protobufs

The format of DeRec messages.
Apache License 2.0
2 stars 0 forks source link

secretID / nonce in plaintext in derecmessage.proto #11

Closed lbaird closed 8 months ago

lbaird commented 8 months ago

The comments in derecmessage.proto currently say:

The OpenPGP format is used for the signed-then-encrypted message, which is then sent.

That should actually say:

The OpenPGP format is used for the signed-then-encrypted message. The sender then sends the concatenation of the nonce (for pairing mesage and response) or secretID (for any other message), which is an 8-byte, big-endian integer, followed by the signed-then-encrypted message.

With this addition, the receiver of a message will always know what key to use to decrypt it (if the receiver has more than one encryption key), and what key to use to verify the signature.

jorabin commented 8 months ago

secret id is 1-16 bytes. It would be preferable to distinguish what the value of the preamble is e.g. 0 - you must guess based of trial and error decryption, 1 - it's an 8 byte nonce, 2 - it's a 1-16 byte secret id, other values for later allocation. Porbably useful for all of them to have a length byte too, so 0 0, 1 8, 2 1-16

lbaird commented 8 months ago

This extra byte isn’t necessary. But if you want to add it, I don’t mind. Just make sure it’s documented in the comments in the protobuf.

On Wed, Nov 15, 2023 at 2:06 PM Jo @.***> wrote:

secret id is 1-16 bytes. It would be preferable to distinguish what the value of the preamble is e.g. 0 - you must guess based of trial and error decryption, 1 - it's an 8 byte nonce, 2 - it's a 1-16 byte secret id, other values for later allocation. Porbably useful for all of them to have a length byte too, so 0 0, 1 8, 2 1-16

— Reply to this email directly, view it on GitHub https://github.com/derecalliance/protobufs/issues/11#issuecomment-1813187310, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDBYW64XAZHWIV73GH6PGTYEUOFNAVCNFSM6AAAAAA7KV3GBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJTGE4DOMZRGA . You are receiving this because you authored the thread.Message ID: @.***>

jorabin commented 8 months ago

There is an issue if sharers choose small secret ids. Let's say that a sharer says "I am only every going to have one secret and its id is 1". Another sharer may say the same thing. Then using the secret id as the plain text identifier for the communication leaves the helper with ambiguity as to which pairing is referred to.

Propose that instead of using the secret id as a pairing reference, the pairing process allows the helper and sharer to exchange references to be used for this purpose.

So the pair request would contain an int "My ref" which is what the helper should put into the plain text portion when sending to the sharer and the pair response would likewise contain a "My ref" field which is what the sharer should put in to the plain text portion, after the initial Pair Request, which contains the nonce from the communication info.

Noting that the secret id and the pairing reference will be in 1:1 correspondence from the sharer's point of view, the sharer might choose to use the secret if as its pairing reference, however the helper is free to choose any value it wishes, and might be expected to choose some randomly generated value.

lbaird commented 8 months ago

The secret ID is supposed to be a random number. If that wasn’t clearly documented, then it should be.

A 64-bit random number is far more than enough to ensure no random collisions. And even if a random collision occurred, that would simply cause the initial pairing to fail, and they would try again with a new random number. So collisions wouldn’t be a problem.

If an implementation has a bug where it always gives secrets the same secret ID, then that bug should be easy to detect in a test suite. The test should check that the numbers chosen during multiple runs are different, have hamming weights close to half, and have hamming distances between them that are close to half. That would easily catch bugs like these.

The current protobufs are using a 128-bit random number, which is less convenient. It could be reduced to 64 bits, or even 32 bits, with no loss of security. We should consider fixing that. But whether we stay with 128 bits or reduce it, either way, it’s far more than enough to prevent collisions.

On Fri, Nov 17, 2023 at 5:13 AM Jo @.***> wrote:

There is an issue if sharers choose small secret ids. Let's say that a sharer says "I am only every going to have one secret and its id is 1". Another sharer may say the same thing. Then using the secret id as the plain text identifier for the communication leaves the helper with ambiguity as to which pairing is referred to.

Propose that instead of using the secret id as the channel id, the pairing process allows the helper and sharer to exchange ids to be used for this purpose.

So the pair request would contain an int "My ref" which is what the helper should put into the plain text portion when sending to the sharer and the pair response would likewise contain a "My ref" field which is what the sharer should put in to the plain text portion, after the initial Pair Request, which contains the nonce from the communication info.

— Reply to this email directly, view it on GitHub https://github.com/derecalliance/protobufs/issues/11#issuecomment-1816181471, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDBYW6J5SVBQFBAJUYED4TYE5BFTAVCNFSM6AAAAAA7KV3GBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJWGE4DCNBXGE . You are receiving this because you authored the thread.Message ID: @.***>

lbaird commented 8 months ago

Actually, if you’re suggesting separate numbers for each party, then I think it could be a good idea.

It would be a 32-bit keyID. Each party would include it along with their public key when initially pairing, so it would be in the contact and in the pair request (but not the pair reply).

Each party would generate a separate keyID for each encryption key pair that they create. And each message would have no plaintext at all, other than the 32-bit keyID. That would be used by the recipient to know which key to use for decryption.

That’s cleaner. And it even increases privacy.

On Fri, Nov 17, 2023 at 7:35 AM Leemon Baird @.***> wrote:

The secret ID is supposed to be a random number. If that wasn’t clearly documented, then it should be.

A 64-bit random number is far more than enough to ensure no random collisions. And even if a random collision occurred, that would simply cause the initial pairing to fail, and they would try again with a new random number. So collisions wouldn’t be a problem.

If an implementation has a bug where it always gives secrets the same secret ID, then that bug should be easy to detect in a test suite. The test should check that the numbers chosen during multiple runs are different, have hamming weights close to half, and have hamming distances between them that are close to half. That would easily catch bugs like these.

The current protobufs are using a 128-bit random number, which is less convenient. It could be reduced to 64 bits, or even 32 bits, with no loss of security. We should consider fixing that. But whether we stay with 128 bits or reduce it, either way, it’s far more than enough to prevent collisions.

On Fri, Nov 17, 2023 at 5:13 AM Jo @.***> wrote:

There is an issue if sharers choose small secret ids. Let's say that a sharer says "I am only every going to have one secret and its id is 1". Another sharer may say the same thing. Then using the secret id as the plain text identifier for the communication leaves the helper with ambiguity as to which pairing is referred to.

Propose that instead of using the secret id as the channel id, the pairing process allows the helper and sharer to exchange ids to be used for this purpose.

So the pair request would contain an int "My ref" which is what the helper should put into the plain text portion when sending to the sharer and the pair response would likewise contain a "My ref" field which is what the sharer should put in to the plain text portion, after the initial Pair Request, which contains the nonce from the communication info.

— Reply to this email directly, view it on GitHub https://github.com/derecalliance/protobufs/issues/11#issuecomment-1816181471, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDBYW6J5SVBQFBAJUYED4TYE5BFTAVCNFSM6AAAAAA7KV3GBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJWGE4DCNBXGE . You are receiving this because you authored the thread.Message ID: @.***>

jorabin commented 8 months ago

Now moot, see following comment.

My understanding from our earlier conversations about this was that the secret id is allowed to be between 1 and 16 bytes because this accommodates both the use case of a sharer saying "I am only ever going to have one secret and its id is 1" and the use case where a sharer wishes to use a UUID as a secret id.

If the secret is randomly generated then that reduces the problem, however, in order to reduce still further the possibility of mis-operation as a result sharers mis-operating or behaving badly, there seems value in allowing helpers a choice as to the reference they would like to use for a pairing, rather than requiring them to use one that a sharer has chosen.

jorabin commented 8 months ago

Previous comment now moot and we have resolved to add a "pairing reference" or "keyId" which is generated by both parties for use by the other party to identify the pairing. We can argue about whether it is called pairing id or key id.

So for avoidance of doubt, the plain text preamble consists of a TLV triple, where type can be:

0 - No information is present, the recipient is expected to try exhaustive search over keys it knows about 1 - The value that follows is the nonce given in the communication info - this is used on the PAIR REQUEST only. 2 - The value is a pairing reference, as received in a pairing request or response.

lbaird commented 8 months ago

If a party assigns a keyID to each encryption key pair they create (rather than to each pairing), then the plaintext needs only a 32-bit keyID, and nothing else.

The contact would give the keyID, in addition to giving the public encryption key.

The pairing request message would give the initiator’s keyID, in addition to giving the initiator’s public encryption key.

Every message in the protocol would have a 32-bit plaintext preamble consisting only on the keyID. That is sufficient to know which key to use to decrypt the rest of the message.

On Fri, Nov 17, 2023 at 8:03 AM Jo @.***> wrote:

Previous comment now moot and we have resolved to add a "pairing reference" or "keyId" which is generated by both parties for use by the other party to identify the pairing. We can argue about whether it is called pairing id or key id.

So for avoidance of doubt, the plain text preamble consists of a TLV triple, where type can be:

0 - No information is present, the recipient is expected to try exhaustive search over keys it knows about 1 - The value that follows is the nonce given in the communication info - this is used on the PAIR REQUEST only. 2 - The value is a pairing reference, as received in a pairing request or response.

— Reply to this email directly, view it on GitHub https://github.com/derecalliance/protobufs/issues/11#issuecomment-1816486621, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDBYW5OGZVODFGIQ4LFUMDYE5VCNAVCNFSM6AAAAAA7KV3GBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJWGQ4DMNRSGE . You are receiving this because you authored the thread.Message ID: @.***>

jorabin commented 8 months ago

I was under the impression from earlier comments that you would like to preserve the option of having the receiver try their keys in turn, however that's not something we want/need.

I'm happy that this should be construed as a key id and not a pairing id since as noted pairings may share keys.

Happy with removal of nonce from contact, pair request and pair reply messages and substitution of key id in contact and pair request messages.

I am slightly nervous about throwing away the possibility of extensibility of what the payload for the plaintext part is. But let's go with it as discussed here.

lbaird commented 8 months ago

The nonce is still needed in the contact and the pairing request. That wouldn’t change.

It’s fine to require the keyID to be prepended to every message. The recipient is free to ignore it, and try multiple decryptions (if they assign the same keyID to all their keys). But more commonly, people will assign distinct keyIDs, and use them for knowing which key to use in decryption.

So a fixed, 32-bit keyID preamble would cover all those cases.

On Fri, Nov 17, 2023 at 8:33 AM Jo @.***> wrote:

I was under the impression from earlier comments that you would like to preserve the option of having the receiver try their keys in turn, however that's not something we want/need.

I'm happy that this should be construed as a key id and not a pairing id since as noted pairings may share keys.

Happy with removal of nonce from contact, pair request and pair reply messages and substitution of key id in contact and pair request messages.

I am slightly nervous about throwing away the possibility of extensibility of what the payload for the plaintext part is. But let's go with it as discussed here.

— Reply to this email directly, view it on GitHub https://github.com/derecalliance/protobufs/issues/11#issuecomment-1816537714, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDBYW5GPL6H4IUKNHUESLLYE5YUFAVCNFSM6AAAAAA7KV3GBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJWGUZTONZRGQ . You are receiving this because you authored the thread.Message ID: @.***>

jorabin commented 8 months ago

Nonce also appears in the pair reply message. Curious as to what purpose it serves?

lbaird commented 8 months ago

It’s needed to convince the initiator that the responder who is sending their public signing key is the same authenticated party that gave the contact. The nonce is avoiding MITM attacks in both directions, by ensuring both messages are coming from the same parties that were actually authenticated.

The alternative would have been to include the public signing key in the contact. But that would add a large number of bytes to the contact. And the contact might need to be a QR code. So we can minimize the size of the contact by including the nonce as a field in the pairing reply.

On Fri, Nov 17, 2023 at 8:45 AM Jo @.***> wrote:

Nonce also appears in the pair reply message. Curious as to what purpose it serves?

— Reply to this email directly, view it on GitHub https://github.com/derecalliance/protobufs/issues/11#issuecomment-1816555884, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDBYWYIVA2LP4ZDOERYW2TYE52ARAVCNFSM6AAAAAA7KV3GBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJWGU2TKOBYGQ . You are receiving this because you authored the thread.Message ID: @.***>

lbaird commented 8 months ago

A 32-bit keyID in plaintext should be sufficient for identifying which key to decrypt with. Then the plaintext wouldn’t need to contain anything other than that. Not even a byte to distinguish between different plaintext types.

On Fri, Nov 17, 2023 at 7:54 AM Jo @.***> wrote:

My understanding from our earlier conversations about this was that the secret id is allowed to be between 1 and 16 bytes because this accommodates both the use case of a sharer saying "I am only ever going to have one secret and its id is 1" and the use case where a sharer wishes to use a UUID as a secret id.

If the secret is randomly generated then that reduces the problem, however, in order to reduce still further the possibility of mis-operation as a result sharers mis-operating or behaving badly, there seems value in allowing helpers a choice as to the reference they would like to use for a pairing, rather than requiring them to use one that a sharer has chosen.

— Reply to this email directly, view it on GitHub https://github.com/derecalliance/protobufs/issues/11#issuecomment-1816473916, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDBYW5P5QRMWGI32W2EFULYE5UCVAVCNFSM6AAAAAA7KV3GBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJWGQ3TGOJRGY . You are receiving this because you authored the thread.Message ID: @.***>

jorabin commented 8 months ago

Yes, this is a reply to a comment marked as moot in light of subsequent discussions.

On 17 Nov 2023, at 16:10, Leemon Baird @.***> wrote:

A 32-bit keyID in plaintext should be sufficient for identifying which key to decrypt with. Then the plaintext wouldn’t need to contain anything other than that. Not even a byte to distinguish between different plaintext types.

On Fri, Nov 17, 2023 at 7:54 AM Jo @.***> wrote:

My understanding from our earlier conversations about this was that the secret id is allowed to be between 1 and 16 bytes because this accommodates both the use case of a sharer saying "I am only ever going to have one secret and its id is 1" and the use case where a sharer wishes to use a UUID as a secret id.

If the secret is randomly generated then that reduces the problem, however, in order to reduce still further the possibility of mis-operation as a result sharers mis-operating or behaving badly, there seems value in allowing helpers a choice as to the reference they would like to use for a pairing, rather than requiring them to use one that a sharer has chosen.

— Reply to this email directly, view it on GitHub https://github.com/derecalliance/protobufs/issues/11#issuecomment-1816473916, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDBYW5P5QRMWGI32W2EFULYE5UCVAVCNFSM6AAAAAA7KV3GBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJWGQ3TGOJRGY . You are receiving this because you authored the thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/derecalliance/protobufs/issues/11#issuecomment-1816701344, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWMBUHL7N44HVKEJBDSQZLYE6EAVAVCNFSM6AAAAAA7KV3GBOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJWG4YDCMZUGQ. You are receiving this because you commented.

jorabin commented 8 months ago

PR #12 addresses this