Use the ed22519 public key as the group session id.

Some clients expect the session id to be globally unique,
so allowing the end devices to pick the session id will cause
problems.

Include the current ratchet index with the initial keys, this decreases
the risk that the client will supply the wrong index causing problems.

Sign the initial keys with the ratchet ed25519 key, this reduces the
risk of a client claiming a session that they didn't create.

2 files changed, 40 insertions, 28 deletions

diff --git a/src/inbound_group_session.c b/src/inbound_group_session.c
index 11e3dbe..f2a310a 100644
--- a/src/inbound_group_session.c
+++ b/src/inbound_group_session.c
@@ -30,7 +30,7 @@
 
 #define OLM_PROTOCOL_VERSION     3
 #define PICKLE_VERSION           1
-#define SESSION_KEY_VERSION      1
+#define SESSION_KEY_VERSION      2
 
 struct OlmInboundGroupSession {
     /** our earliest known ratchet value */
@@ -71,12 +71,12 @@ size_t olm_clear_inbound_group_session(
 }
 
 #define SESSION_KEY_RAW_LENGTH \
-    (1 + MEGOLM_RATCHET_LENGTH + ED25519_PUBLIC_KEY_LENGTH)
+    (1 + 4 + MEGOLM_RATCHET_LENGTH + ED25519_PUBLIC_KEY_LENGTH\
+        + ED25519_SIGNATURE_LENGTH)
 
 /** init the session keys from the un-base64-ed session keys */
 static size_t _init_group_session_keys(
     OlmInboundGroupSession *session,
-    uint32_t message_index,
     const uint8_t *key_buf
 ) {
     const uint8_t *ptr = key_buf;
@@ -87,13 +87,27 @@ static size_t _init_group_session_keys(
         return (size_t)-1;
     }
 
-    megolm_init(&session->initial_ratchet, ptr, message_index);
-    megolm_init(&session->latest_ratchet, ptr, message_index);
+    uint32_t counter = 0;
+    counter <<= 8; counter |= *ptr++;
+    counter <<= 8; counter |= *ptr++;
+    counter <<= 8; counter |= *ptr++;
+    counter <<= 8; counter |= *ptr++;
+
+    megolm_init(&session->initial_ratchet, ptr, counter);
+    megolm_init(&session->latest_ratchet, ptr, counter);
+
     ptr += MEGOLM_RATCHET_LENGTH;
     memcpy(
         session->signing_key.public_key, ptr, ED25519_PUBLIC_KEY_LENGTH
     );
     ptr += ED25519_PUBLIC_KEY_LENGTH;
+
+    if (!_olm_crypto_ed25519_verify(
+        &session->signing_key, key_buf, ptr - key_buf, ptr
+    )) {
+        session->last_error = OLM_BAD_SIGNATURE;
+        return (size_t)-1;
+    }
     return 0;
 }
 
@@ -117,7 +131,7 @@ size_t olm_init_inbound_group_session(
     }
 
     _olm_decode_base64(session_key, session_key_length, key_buf);
-    result = _init_group_session_keys(session, message_index, key_buf);
+    result = _init_group_session_keys(session, key_buf);
     _olm_unset(key_buf, SESSION_KEY_RAW_LENGTH);
     return result;
 }
@@ -288,7 +302,6 @@ static size_t _decrypt(
         return (size_t)-1;
     }
 
-
     max_length = megolm_cipher->ops->decrypt_max_plaintext_length(
         megolm_cipher,
         decoded_results.ciphertext_length
diff --git a/src/outbound_group_session.c b/src/outbound_group_session.c
index a605fa6..7116547 100644
--- a/src/outbound_group_session.c
+++ b/src/outbound_group_session.c
@@ -29,10 +29,9 @@
 #include "olm/pickle_encoding.h"
 
 #define OLM_PROTOCOL_VERSION     3
-#define SESSION_ID_RANDOM_BYTES  4
-#define GROUP_SESSION_ID_LENGTH (sizeof(struct timeval) + SESSION_ID_RANDOM_BYTES)
+#define GROUP_SESSION_ID_LENGTH  ED25519_PUBLIC_KEY_LENGTH
 #define PICKLE_VERSION           1
-#define SESSION_KEY_VERSION      1
+#define SESSION_KEY_VERSION      2
 
 struct OlmOutboundGroupSession {
     /** the Megolm ratchet providing the encryption keys */
@@ -41,9 +40,6 @@ struct OlmOutboundGroupSession {
     /** The ed25519 keypair used for signing the messages */
     struct _olm_ed25519_key_pair signing_key;
 
-    /** unique identifier for this session */
-    uint8_t session_id[GROUP_SESSION_ID_LENGTH];
-
     enum OlmErrorCode last_error;
 };
 
@@ -80,8 +76,6 @@ static size_t raw_pickle_length(
     length += _olm_pickle_uint32_length(PICKLE_VERSION);
     length += megolm_pickle_length(&(session->ratchet));
     length += _olm_pickle_ed25519_key_pair_length(&(session->signing_key));
-    length += _olm_pickle_bytes_length(session->session_id,
-                                       GROUP_SESSION_ID_LENGTH);
     return length;
 }
 
@@ -108,7 +102,6 @@ size_t olm_pickle_outbound_group_session(
     pos = _olm_pickle_uint32(pos, PICKLE_VERSION);
     pos = megolm_pickle(&(session->ratchet), pos);
     pos = _olm_pickle_ed25519_key_pair(pos, &(session->signing_key));
-    pos = _olm_pickle_bytes(pos, session->session_id, GROUP_SESSION_ID_LENGTH);
 
     return _olm_enc_output(key, key_length, pickled, raw_length);
 }
@@ -138,7 +131,6 @@ size_t olm_unpickle_outbound_group_session(
     }
     pos = megolm_unpickle(&(session->ratchet), pos, end);
     pos = _olm_unpickle_ed25519_key_pair(pos, end, &(session->signing_key));
-    pos = _olm_unpickle_bytes(pos, end, session->session_id, GROUP_SESSION_ID_LENGTH);
 
     if (end != pos) {
         /* We had the wrong number of bytes in the input. */
@@ -157,8 +149,7 @@ size_t olm_init_outbound_group_session_random_length(
      * session id.
      */
     return MEGOLM_RATCHET_LENGTH +
-        ED25519_RANDOM_LENGTH +
-        SESSION_ID_RANDOM_BYTES;
+        ED25519_RANDOM_LENGTH;
 }
 
 size_t olm_init_outbound_group_session(
@@ -177,13 +168,6 @@ size_t olm_init_outbound_group_session(
     _olm_crypto_ed25519_generate_key(random, &(session->signing_key));
     random += ED25519_RANDOM_LENGTH;
 
-    /* initialise the session id. This just has to be unique. We use the
-     * current time plus some random data.
-     */
-    gettimeofday((struct timeval *)(session->session_id), NULL);
-    memcpy((session->session_id) + sizeof(struct timeval),
-           random, SESSION_ID_RANDOM_BYTES);
-
     return 0;
 }
 
@@ -314,7 +298,9 @@ size_t olm_outbound_group_session_id(
         return (size_t)-1;
     }
 
-    return _olm_encode_base64(session->session_id, GROUP_SESSION_ID_LENGTH, id);
+    return _olm_encode_base64(
+        session->signing_key.public_key.public_key, GROUP_SESSION_ID_LENGTH, id
+    );
 }
 
 uint32_t olm_outbound_group_session_message_index(
@@ -324,7 +310,8 @@ uint32_t olm_outbound_group_session_message_index(
 }
 
 #define SESSION_KEY_RAW_LENGTH \
-    (1 + MEGOLM_RATCHET_LENGTH + ED25519_PUBLIC_KEY_LENGTH)
+    (1 + 4 + MEGOLM_RATCHET_LENGTH + ED25519_PUBLIC_KEY_LENGTH\
+        + ED25519_SIGNATURE_LENGTH)
 
 size_t olm_outbound_group_session_key_length(
     const OlmOutboundGroupSession *session
@@ -349,6 +336,12 @@ size_t olm_outbound_group_session_key(
     raw = ptr = key + encoded_length - SESSION_KEY_RAW_LENGTH;
     *ptr++ = SESSION_KEY_VERSION;
 
+    uint32_t counter = session->ratchet.counter;
+    *ptr++ = 0xFF & (counter >> 24); counter <<= 8;
+    *ptr++ = 0xFF & (counter >> 24); counter <<= 8;
+    *ptr++ = 0xFF & (counter >> 24); counter <<= 8;
+    *ptr++ = 0xFF & (counter >> 24); counter <<= 8;
+
     memcpy(ptr, megolm_get_data(&session->ratchet), MEGOLM_RATCHET_LENGTH);
     ptr += MEGOLM_RATCHET_LENGTH;
 
@@ -358,5 +351,11 @@ size_t olm_outbound_group_session_key(
     );
     ptr += ED25519_PUBLIC_KEY_LENGTH;
 
+    /* sign the whole thing with the ed25519 key. */
+    _olm_crypto_ed25519_sign(
+        &(session->signing_key),
+        raw, ptr - raw, ptr
+    );
+
     return _olm_encode_base64(raw, SESSION_KEY_RAW_LENGTH, key);
 }