Merge pull request #21 from matrix-org/markjh/fix_session_ids

Use the ed25519 public key as the group session id.

3 files changed, 48 insertions, 35 deletions

diff --git a/src/inbound_group_session.c b/src/inbound_group_session.c
index 11e3dbe..82ff66f 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;
+    // Decode counter as a big endian 32-bit number.
+    for (unsigned i = 0; i < 4; i++) {
+        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..4e4561a 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;
+    // Encode counter as a big endian 32-bit number.
+    for (unsigned i = 0; i < 4; i++) {
+        *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);
 }
diff --git a/tests/test_group_session.cpp b/tests/test_group_session.cpp
index 7ac91c3..ce889ad 100644
--- a/tests/test_group_session.cpp
+++ b/tests/test_group_session.cpp
@@ -97,7 +97,7 @@ int main() {
     uint8_t memory[size];
     OlmOutboundGroupSession *session = olm_outbound_group_session(memory);
 
-    assert_equals((size_t)164,
+    assert_equals((size_t)160,
                   olm_init_outbound_group_session_random_length(session));
 
     size_t res = olm_init_outbound_group_session(
@@ -154,14 +154,15 @@ int main() {
     size_t plaintext_length = sizeof(plaintext) - 1;
 
     uint8_t session_key[] =
-        "ATAxMjM0NTY3ODlBQkRFRjAxMjM0NTY3ODlBQkNERUYwMTIzNDU2Nzg5QUJERUYw"
-        "MTIzNDU2Nzg5QUJDREVGMDEyMzQ1Njc4OUFCREVGMDEyMzQ1Njc4OUFCQ0RFRjAx"
-        "MjM0NTY3ODlBQkRFRjAxMjM0NTY3ODlBQkNERUYwMTIzDRt2DUEOrg/H+yUGjDTq"
-        "ryf8H1YF/BZjI04HwOVSZcY";
+        "AgAAAAAwMTIzNDU2Nzg5QUJERUYwMTIzNDU2Nzg5QUJDREVGMDEyMzQ1Njc4OUFCREVGM"
+        "DEyMzQ1Njc4OUFCQ0RFRjAxMjM0NTY3ODlBQkRFRjAxMjM0NTY3ODlBQkNERUYwMTIzND"
+        "U2Nzg5QUJERUYwMTIzNDU2Nzg5QUJDREVGMDEyMztqJ7zOtqQtYqOo0CpvDXNlMhV3HeJ"
+        "DpjrASKGLWdop4lx1cSN3Xv1TgfLPW8rhGiW+hHiMxd36nRuxscNv9k4oJA/KP+o0mi1w"
+        "v44StrEJ1wwx9WZHBUIWkQbaBSuBDw";
 
     uint8_t message[] =
-        "AwgAEhAcbh6UpbByoyZxufQ+h2B+8XHMjhR69G8F4+qjMaFlnIXusJZX3r8LnROR"
-        "G9T3DXFdbVuvIWrLyRfm4i8QRbe8VPwGRFG57B1CtmxanuP8bHtnnYqlwPsD";
+        "AwgAEhAcbh6UpbByoyZxufQ+h2B+8XHMjhR69G8nP4pNZGl/3QMgrzCZPmP+F2aPLyKPz"
+        "xRPBMUkeXRJ6Iqm5NeOdx2eERgTW7P20CM+lL3Xpk+ZUOOPvsSQNaAL";
     size_t msglen = sizeof(message)-1;
 
     /* build the inbound session */