From 6d80d934cd3727f2fec320e722124562ffb7dd21 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 28 Sep 2016 18:49:56 +0100 Subject: [PATCH 01/28] typo --- docs/megolm.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/megolm.rst b/docs/megolm.rst index 7853963..2ea0b94 100644 --- a/docs/megolm.rst +++ b/docs/megolm.rst @@ -27,7 +27,7 @@ The Megolm ratchet is intended for encrypted messaging applications where there may be a large number of recipients of each message, thus precluding the use of peer-to-peer encryption systems such as `Olm`_. -It also allows a receipient to decrypt received messages multiple times. For +It also allows a recipient to decrypt received messages multiple times. For instance, in client/server applications, a copy of the ciphertext can be stored on the (untrusted) server, while the client need only store the session keys. From 63800ad8e61ebdfa756d8bde8466b70337b85d67 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 2 Oct 2016 00:47:29 +0100 Subject: [PATCH 02/28] s/PCKS/PKCS/ --- docs/megolm.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/megolm.rst b/docs/megolm.rst index 2ea0b94..1f04840 100644 --- a/docs/megolm.rst +++ b/docs/megolm.rst @@ -143,7 +143,7 @@ copy of the counter, ratchet, and public key. Message encryption ~~~~~~~~~~~~~~~~~~ -This version of Megolm uses AES-256_ in CBC_ mode with `PCKS#7`_ padding and +This version of Megolm uses AES-256_ in CBC_ mode with `PKCS#7`_ padding and HMAC-SHA-256_ (truncated to 64 bits). The 256 bit AES key, 256 bit HMAC key, and 128 bit AES IV are derived from the megolm ratchet :math:`R_i`: @@ -285,6 +285,6 @@ Version 2.0 `_. .. _`SHA-256`: https://tools.ietf.org/html/rfc6234 .. _`AES-256`: http://csrc.nist.gov/publications/fips/fips197/fips-197.pdf .. _`CBC`: http://csrc.nist.gov/publications/nistpubs/800-38a/sp800-38a.pdf -.. _`PCKS#7`: https://tools.ietf.org/html/rfc2315 +.. _`PKCS#7`: https://tools.ietf.org/html/rfc2315 .. _`Olm`: ./olm.html .. _`Protocol Buffers encoding`: https://developers.google.com/protocol-buffers/docs/encoding From 68ec41f8ca731b8e9335dbfd691b8339f030ee4d Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 2 Oct 2016 00:48:06 +0100 Subject: [PATCH 03/28] s/PCKS/PKCS/ --- docs/olm.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/olm.rst b/docs/olm.rst index 99417e0..af42215 100644 --- a/docs/olm.rst +++ b/docs/olm.rst @@ -279,7 +279,7 @@ Olm Authenticated Encryption Version 1 ~~~~~~~~~ -Version 1 of Olm uses AES-256_ in CBC_ mode with `PCKS#7`_ padding for +Version 1 of Olm uses AES-256_ in CBC_ mode with `PKCS#7`_ padding for encryption and HMAC-SHA-256_ (truncated to 64 bits) for authentication. The 256 bit AES key, 256 bit HMAC key, and 128 bit AES IV are derived from the message key using HKDF-SHA-256_ using the default salt and an info of @@ -323,4 +323,4 @@ an entirely new implementation written by the Matrix.org team. .. _`SHA-256`: https://tools.ietf.org/html/rfc6234 .. _`AES-256`: http://csrc.nist.gov/publications/fips/fips197/fips-197.pdf .. _`CBC`: http://csrc.nist.gov/publications/nistpubs/800-38a/sp800-38a.pdf -.. _`PCKS#7`: https://tools.ietf.org/html/rfc2315 +.. _`PKCS#7`: https://tools.ietf.org/html/rfc2315 From 38acc352a3f3aac40c132e5321da540da38c832e Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 2 Oct 2016 02:50:52 +0100 Subject: [PATCH 04/28] fix missing ctypes function signatures These missing signatures were causing OSX to truncate 64-bit pointers to 32-bit pointers when calling the missing methods, causing segfaults --- python/olm/account.py | 1 + python/olm/session.py | 1 + 2 files changed, 2 insertions(+) diff --git a/python/olm/account.py b/python/olm/account.py index 7673329..3fa1049 100644 --- a/python/olm/account.py +++ b/python/olm/account.py @@ -41,6 +41,7 @@ account_function(lib.olm_account_one_time_keys_length) account_function(lib.olm_account_one_time_keys, c_void_p, c_size_t) account_function(lib.olm_account_mark_keys_as_published) account_function(lib.olm_account_max_number_of_one_time_keys) +account_function(lib.olm_pickle_account_length) account_function( lib.olm_account_generate_one_time_keys_random_length, c_size_t diff --git a/python/olm/session.py b/python/olm/session.py index 308f220..19d43d3 100644 --- a/python/olm/session.py +++ b/python/olm/session.py @@ -58,6 +58,7 @@ session_function( c_void_p, c_size_t, # Identity Key c_void_p, c_size_t, # Pre Key Message ) +session_function(lib.olm_pickle_session_length) session_function(lib.olm_encrypt_message_type) session_function(lib.olm_encrypt_random_length) session_function(lib.olm_encrypt_message_length, c_size_t) From cada801de524fcbb085bced6fb49a079fad2c1e0 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 19 Oct 2016 14:59:50 +0100 Subject: [PATCH 05/28] Add a README for the fuzzers --- fuzzers/README.rst | 51 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 fuzzers/README.rst diff --git a/fuzzers/README.rst b/fuzzers/README.rst new file mode 100644 index 0000000..b3142ca --- /dev/null +++ b/fuzzers/README.rst @@ -0,0 +1,51 @@ +Fuzzers +======= + +This directory contains a collection of fuzzing tools. Each tests a different +entry point to the code. + +Usage notes: + +1. Install AFL: + + .. code:: + + apt-get install afl + +2. Build the fuzzers: + + .. code:: + + make fuzzers + +3. Some of the tests (eg ``fuzz_decrypt`` and ``fuzz_group_decrypt``) require a + session file. You can use the ones generated by the python test script + (``python/test.sh``). + +4. Make some work directories: + + .. code:: + + mkdir -p fuzzing/in fuzzing/out + +5. Generate starting input: + + .. code:: + + echo "Test" > fuzzing/in/test + +6. Run the test under ``afl-fuzz``: + + .. code:: + + afl-fuzz -i fuzzing/in -o fuzzing/out -- \ + ./build/fuzzers/fuzz_ [] + +7. To resume with the data produced by an earlier run: + + .. code:: + + ./afl-fuzz -i- -o existing_output_dir [...etc...] + +8. If it shows failures, pipe the failure case into + ``./build/fuzzers/debug_``, fix, and repeat. From 1ff64391edf9f2e3180238271858698a5a6f30c6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 19 Oct 2016 15:03:40 +0100 Subject: [PATCH 06/28] Fix a buffer bounds check when decoding group messages Fixes a segfault when a group message had exactly the length of the mac + signature. Also tweak skipping of unknown tags to avoid an extra trip around the loop. --- src/message.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/message.cpp b/src/message.cpp index 05fe2c7..1c11a4a 100644 --- a/src/message.cpp +++ b/src/message.cpp @@ -214,11 +214,13 @@ void olm::decode_message( reader.ciphertext = nullptr; reader.ciphertext_length = 0; - if (pos == end) return; if (input_length < mac_length) return; + + if (pos == end) return; reader.version = *(pos++); while (pos != end) { + unknown = pos; pos = decode( pos, end, RATCHET_KEY_TAG, reader.ratchet_key, reader.ratchet_key_length @@ -234,7 +236,6 @@ void olm::decode_message( if (unknown == pos) { pos = skip_unknown(pos, end); } - unknown = pos; } } @@ -303,6 +304,7 @@ void olm::decode_one_time_key_message( reader.version = *(pos++); while (pos != end) { + unknown = pos; pos = decode( pos, end, ONE_TIME_KEY_ID_TAG, reader.one_time_key, reader.one_time_key_length @@ -322,7 +324,6 @@ void olm::decode_one_time_key_message( if (unknown == pos) { pos = skip_unknown(pos, end); } - unknown = pos; } } @@ -377,9 +378,12 @@ void _olm_decode_group_message( results->ciphertext_length = 0; if (input_length < trailer_length) return; + + if (pos == end) return; results->version = *(pos++); while (pos != end) { + unknown = pos; pos = decode( pos, end, GROUP_MESSAGE_INDEX_TAG, results->message_index, has_message_index @@ -391,7 +395,6 @@ void _olm_decode_group_message( if (unknown == pos) { pos = skip_unknown(pos, end); } - unknown = pos; } results->has_message_index = (int)has_message_index; From d48dc8197680dce2bb810c5714f17d1a35dcb3d0 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 19 Oct 2016 17:27:24 +0100 Subject: [PATCH 07/28] Document the unknown key-share attacks and mitigation (#29) --- docs/olm.rst | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/docs/olm.rst b/docs/olm.rst index af42215..34bf9ba 100644 --- a/docs/olm.rst +++ b/docs/olm.rst @@ -298,6 +298,37 @@ and the IV :math:`AES\_IV_{i,j}` to give the cipher-text, :math:`X_{i,j}`. Then the entire message (including the Version Byte and all Payload Bytes) are passed through HMAC-SHA-256. The first 8 bytes of the MAC are appended to the message. +Message authentication concerns +------------------------------- + +To avoid unknown key-share attacks, the application must include identifying +data for the sending and receiving user in the plain-text of (at least) the +pre-key messages. Such data could be a user ID, a telephone number; +alternatively it could be the public part of a keypair which the relevant user +has proven ownership of. + +.. admonition:: Example attacks + + 1. Alice publishes her public Curve25519 identity key, :math:`I_A`. Eve + publishes the same identity key, claiming it as her own. Bob downloads + Eve's keys, and associates :math:`I_A` with Eve. Alice sends a message to + Bob; Eve intercepts it before forwarding it to Bob. Bob believes the + message came from Eve rather than Alice. + + This is prevented if Alice includes her user ID in the plain-text of the + pre-key message, so that Bob can see that the message was sent by Alice + originally. + + 2. Bob publishes his public Curve25519 identity key, :math:`I_B`. Eve + publishes the same identity key, claiming it as her own. Alice downloads + Eve's keys, and associates :math:`I_B` with Eve. Alice sends a message to + Eve; Eve cannot decrypt it, but forwards it to Bob. Bob believes the + Alice sent the message to him, wheras Alice intended it to go to Eve. + + This is prevented by Alice including the user ID of the intended recpient + (Eve) in the plain-text of the pre-key message. Bob can now tell that the + message was meant for Eve rather than him. + IPR --- From 23fdc0b0f96eb871d5ce72ed668784018bc026a0 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 19 Oct 2016 19:14:18 +0100 Subject: [PATCH 08/28] Link to the megolm spec --- README.rst | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 21ae529..be1fb1a 100644 --- a/README.rst +++ b/README.rst @@ -5,8 +5,12 @@ An implementation of the Double Ratchet cryptographic ratchet described by https://github.com/trevp/double_ratchet/wiki, written in C and C++11 and exposed as a C API. -The specification of the Olm ratchet can be found in docs/olm.rst or -https://matrix.org/docs/spec/olm.html +The specification of the Olm ratchet can be found in ``docs/olm.rst`` or +https://matrix.org/docs/spec/olm.html. + +This library also includes an implementation of the Megolm cryptographic +ratchet, as specified in ``docs/megolm.rst`` or +https://matrix.org/docs/spec/megolm.html. Building -------- From df04cd509a13fb1a3c8b953de4f987b15fcde9b1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 19 Oct 2016 19:16:23 +0100 Subject: [PATCH 09/28] Add notes on limitations to megolm spec --- docs/megolm.rst | 69 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/docs/megolm.rst b/docs/megolm.rst index 1f04840..4929349 100644 --- a/docs/megolm.rst +++ b/docs/megolm.rst @@ -199,9 +199,9 @@ session. In order to maintain the ability to decrypt conversation history, inbound sessions should store a copy of their earliest known ratchet value (unless they -explicitly want to drop the ability to decrypt that history). They may also -choose to cache calculated ratchet values, but the decision of which ratchet -states to cache is left to the application. +explicitly want to drop the ability to decrypt that history - see `Partial +Forward Secrecy`_\ ). They may also choose to cache calculated ratchet values, +but the decision of which ratchet states to cache is left to the application. Data exchange formats --------------------- @@ -269,7 +269,68 @@ protocol). The MAC protects all of the bytes preceding the MAC. The length of the signature is determined by the signing algorithm being used (64 bytes in this version of the protocol). The signature covers all of the -bytes preceding the signaure. +bytes preceding the signature. + +Limitations +----------- + +Lack of Transcript Consistency +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +In a group conversation, there is no guarantee that all recipients have +received the same messages. For example, if Alice is in a conversation with Bob +and Charlie, she could send different messages to Bob and Charlie, or could +send some messages to Bob but not Charlie, or vice versa. + +Solving this is, in general, a hard problem, particularly in a protocol which +does not guarantee in-order message delivery. For now it remains the subject of +future research. + +Lack of Backward Secrecy +~~~~~~~~~~~~~~~~~~~~~~~~ + +Once the key to a Megolm session is compromised, the attacker can decrypt any +future messages sent via that session. + +In order to mitigate this, the application should ensure that Megolm sessions +are not used indefinitely. Instead it should periodically start a new session, +with new keys shared over a secure channel. + +.. TODO: Can we recommend sensible lifetimes for Megolm sessions? Probably + depends how paranoid we're feeling, but some guidelines might be useful. + +Partial Forward Secrecy +~~~~~~~~~~~~~~~~~~~~~~~ + +Each recipient maintains a record of the ratchet value which allows them to +decrypt any messages sent in the session after the corresponding point in the +conversation. If this value is compromised, an attacker can similarly decrypt +those past messages. + +To mitigate this issue, the application should offer the user the option to +discard historical conversations, by winding forward any stored ratchet values, +or discarding sessions altogether. + +Dependency on secure channel for key exchange +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The design of the Megolm ratchet relies on the availability of a secure +peer-to-peer channel for the exchange of session keys. Any vulnerabilities in +the underlying channel are likely to be amplified when applied to Megolm +session setup. + +For example, if the peer-to-peer channel is vulnerable to an unknown key-share +attack, the entire Megolm session become similarly vulnerable. For example: +Alice starts a group chat with Eve, and shares the session keys with Eve. Eve +uses the unknown key-share attack to forward the session keys to Bob, who +believes Alice is starting the session with him. Eve then forwards messages +from the Megolm session to Bob, who again believes they are coming from +Alice. Provided the peer-to-peer channel is not vulnerable to this attack, Bob +will realise that the key-sharing message was forwarded by Eve, and can treat +the Megolm session as a forgery. + +A second example: if the peer-to-peer channel is vulnerable to a replay +attack, this can be extended to entire Megolm sessions. License ------- From 5f1b93bd0fa06a603d5d4a88af71bef7fd5143fb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 19 Oct 2016 19:18:58 +0100 Subject: [PATCH 10/28] s/ephemeral/one-time/ in olm spec We're standardising on 'one-time keys' as a term for the thing that Bob uploads for prekey messages. --- docs/olm.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/olm.rst b/docs/olm.rst index 34bf9ba..093cb47 100644 --- a/docs/olm.rst +++ b/docs/olm.rst @@ -30,7 +30,7 @@ Initial setup ~~~~~~~~~~~~~ The setup takes four Curve25519_ inputs: Identity keys for Alice and Bob, -:math:`I_A` and :math:`I_B`, and ephemeral keys for Alice and Bob, +:math:`I_A` and :math:`I_B`, and one-time keys for Alice and Bob, :math:`E_A` and :math:`E_B`. A shared secret, :math:`S`, is generated using `Triple Diffie-Hellman`_. The initial 256 bit root key, :math:`R_0`, and 256 bit chain key, :math:`C_{0,0}`, are derived from the shared secret using an From 653790eacbf7dcf94cbf181657cdb0c30c890c3f Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 20 Oct 2016 09:58:55 +0100 Subject: [PATCH 11/28] Return the message index when decrypting group messages. Applications can use the index to detect replays of the same message. --- include/olm/inbound_group_session.h | 3 ++- javascript/demo/group_demo.js | 4 ++-- javascript/olm_inbound_group_session.js | 9 +++++++-- python/olm/__main__.py | 2 +- python/olm/inbound_group_session.py | 8 ++++++-- src/inbound_group_session.c | 11 ++++++++--- tests/test_group_session.cpp | 8 +++++--- 7 files changed, 31 insertions(+), 14 deletions(-) diff --git a/include/olm/inbound_group_session.h b/include/olm/inbound_group_session.h index 59146c2..f8a0bc3 100644 --- a/include/olm/inbound_group_session.h +++ b/include/olm/inbound_group_session.h @@ -140,7 +140,8 @@ size_t olm_group_decrypt( uint8_t * message, size_t message_length, /* output */ - uint8_t * plaintext, size_t max_plaintext_length + uint8_t * plaintext, size_t max_plaintext_length, + uint32_t * message_index ); diff --git a/javascript/demo/group_demo.js b/javascript/demo/group_demo.js index 1b8f7ab..42a3d84 100644 --- a/javascript/demo/group_demo.js +++ b/javascript/demo/group_demo.js @@ -403,8 +403,8 @@ DemoUser.prototype.decryptGroup = function(jsonpacket, callback) { throw new Error("Unknown session id " + session_id); } - var plaintext = session.decrypt(packet.body); - done(plaintext); + var result = session.decrypt(packet.body); + done(result.plaintext); }, callback); }; diff --git a/javascript/olm_inbound_group_session.js b/javascript/olm_inbound_group_session.js index 6058233..1b7fcfe 100644 --- a/javascript/olm_inbound_group_session.js +++ b/javascript/olm_inbound_group_session.js @@ -73,10 +73,12 @@ InboundGroupSession.prototype['decrypt'] = restore_stack(function( // So we copy the array to a new buffer var message_buffer = stack(message_array); var plaintext_buffer = stack(max_plaintext_length + NULL_BYTE_PADDING_LENGTH); + var message_index = stack(4); var plaintext_length = inbound_group_session_method(Module["_olm_group_decrypt"])( this.ptr, message_buffer, message_array.length, - plaintext_buffer, max_plaintext_length + plaintext_buffer, max_plaintext_length, + message_index ); // Pointer_stringify requires a null-terminated argument (the optional @@ -86,7 +88,10 @@ InboundGroupSession.prototype['decrypt'] = restore_stack(function( 0, "i8" ); - return Pointer_stringify(plaintext_buffer); + return { + "plaintext": Pointer_stringify(plaintext_buffer), + "message_index": Module['getValue'](message_index, "i32") + } }); InboundGroupSession.prototype['session_id'] = restore_stack(function() { diff --git a/python/olm/__main__.py b/python/olm/__main__.py index cf9158d..eb76301 100755 --- a/python/olm/__main__.py +++ b/python/olm/__main__.py @@ -328,7 +328,7 @@ def do_group_decrypt(args): session = InboundGroupSession() session.unpickle(args.key, read_base64_file(args.session_file)) message = args.message_file.read() - plaintext = session.decrypt(message) + plaintext, message_index = session.decrypt(message) with open(args.session_file, "wb") as f: f.write(session.pickle(args.key)) args.plaintext_file.write(plaintext) diff --git a/python/olm/inbound_group_session.py b/python/olm/inbound_group_session.py index d5547fd..27a569c 100644 --- a/python/olm/inbound_group_session.py +++ b/python/olm/inbound_group_session.py @@ -43,6 +43,7 @@ inbound_group_session_function( lib.olm_group_decrypt, c_void_p, c_size_t, # message c_void_p, c_size_t, # plaintext + POINTER(c_uint32), # message_index ) inbound_group_session_function(lib.olm_inbound_group_session_id_length) @@ -82,11 +83,14 @@ class InboundGroupSession(object): ) plaintext_buffer = create_string_buffer(max_plaintext_length) message_buffer = create_string_buffer(message) + + message_index = c_uint32() plaintext_length = lib.olm_group_decrypt( self.ptr, message_buffer, len(message), - plaintext_buffer, max_plaintext_length + plaintext_buffer, max_plaintext_length, + byref(message_index) ) - return plaintext_buffer.raw[:plaintext_length] + return plaintext_buffer.raw[:plaintext_length], message_index def session_id(self): id_length = lib.olm_inbound_group_session_id_length(self.ptr) diff --git a/src/inbound_group_session.c b/src/inbound_group_session.c index bf00008..ed313a0 100644 --- a/src/inbound_group_session.c +++ b/src/inbound_group_session.c @@ -263,7 +263,8 @@ size_t olm_group_decrypt_max_plaintext_length( static size_t _decrypt( OlmInboundGroupSession *session, uint8_t * message, size_t message_length, - uint8_t * plaintext, size_t max_plaintext_length + uint8_t * plaintext, size_t max_plaintext_length, + uint32_t * message_index ) { struct _OlmDecodeGroupMessageResults decoded_results; size_t max_length, r; @@ -286,6 +287,8 @@ static size_t _decrypt( return (size_t)-1; } + *message_index = decoded_results.message_index; + /* verify the signature. We could do this before decoding the message, but * we allow for the possibility of future protocol versions which use a * different signing mechanism; we would rather throw "BAD_MESSAGE_VERSION" @@ -349,7 +352,8 @@ static size_t _decrypt( size_t olm_group_decrypt( OlmInboundGroupSession *session, uint8_t * message, size_t message_length, - uint8_t * plaintext, size_t max_plaintext_length + uint8_t * plaintext, size_t max_plaintext_length, + uint32_t * message_index ) { size_t raw_message_length; @@ -361,7 +365,8 @@ size_t olm_group_decrypt( return _decrypt( session, message, raw_message_length, - plaintext, max_plaintext_length + plaintext, max_plaintext_length, + message_index ); } diff --git a/tests/test_group_session.cpp b/tests/test_group_session.cpp index 9930927..b15875c 100644 --- a/tests/test_group_session.cpp +++ b/tests/test_group_session.cpp @@ -161,8 +161,9 @@ int main() { memcpy(msgcopy, msg, msglen); size = olm_group_decrypt_max_plaintext_length(inbound_session, msgcopy, msglen); uint8_t plaintext_buf[size]; + uint32_t message_index; res = olm_group_decrypt(inbound_session, msg, msglen, - plaintext_buf, size); + plaintext_buf, size, &message_index); assert_equals(plaintext_length, res); assert_equals(plaintext, plaintext_buf, res); } @@ -208,8 +209,9 @@ int main() { memcpy(msgcopy, message, msglen); uint8_t plaintext_buf[size]; + uint32_t message_index; res = olm_group_decrypt( - inbound_session, msgcopy, msglen, plaintext_buf, size + inbound_session, msgcopy, msglen, plaintext_buf, size, &message_index ); assert_equals(plaintext_length, res); assert_equals(plaintext, plaintext_buf, res); @@ -227,7 +229,7 @@ int main() { memcpy(msgcopy, message, msglen); res = olm_group_decrypt( inbound_session, msgcopy, msglen, - plaintext_buf, size + plaintext_buf, size, &message_index ); assert_equals((size_t)-1, res); assert_equals( From 3091dc2b1d5eab575bfe886bf1d6f2414797373f Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 20 Oct 2016 11:35:45 +0100 Subject: [PATCH 12/28] Add NULL check for message_index pointer --- src/inbound_group_session.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/inbound_group_session.c b/src/inbound_group_session.c index ed313a0..a54e55f 100644 --- a/src/inbound_group_session.c +++ b/src/inbound_group_session.c @@ -287,7 +287,9 @@ static size_t _decrypt( return (size_t)-1; } - *message_index = decoded_results.message_index; + if (message_index != NULL) { + *message_index = decoded_results.message_index; + } /* verify the signature. We could do this before decoding the message, but * we allow for the possibility of future protocol versions which use a From 9a8d2d15d97dc17d8f33b7d45b0fefc1267b57c4 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Thu, 20 Oct 2016 11:51:56 +0100 Subject: [PATCH 13/28] Check the message index in the tests --- tests/test_group_session.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_group_session.cpp b/tests/test_group_session.cpp index b15875c..df46f0e 100644 --- a/tests/test_group_session.cpp +++ b/tests/test_group_session.cpp @@ -166,6 +166,7 @@ int main() { plaintext_buf, size, &message_index); assert_equals(plaintext_length, res); assert_equals(plaintext, plaintext_buf, res); + assert_equals(message_index, uint32_t(0)); } { @@ -213,6 +214,7 @@ int main() { res = olm_group_decrypt( inbound_session, msgcopy, msglen, plaintext_buf, size, &message_index ); + assert_equals(message_index, uint32_t(0)); assert_equals(plaintext_length, res); assert_equals(plaintext, plaintext_buf, res); From 8c4a11a92d2eac501e06659dff062d84d5c855ec Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 21 Oct 2016 15:13:20 +0100 Subject: [PATCH 14/28] Document the potential for message replays and possible mitigations --- docs/megolm.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/megolm.rst b/docs/megolm.rst index 4929349..56e5f1d 100644 --- a/docs/megolm.rst +++ b/docs/megolm.rst @@ -274,6 +274,16 @@ bytes preceding the signature. Limitations ----------- +Message Replays +--------------- + +A message can be decrypted successfully multiple times. This means that a MITM +server can send multiple copies of a message and they will successfully decrypt. + +To mitigate this it is recomendend that applications track the message indicies +they have recieved and that they reject messages with indicies that they've +already decrypted. + Lack of Transcript Consistency ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 0a7d4e35ccee89c34adeb03b112d243a27326fda Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 21 Oct 2016 15:44:53 +0100 Subject: [PATCH 15/28] Reword and s/message index/ratchet index/ --- docs/megolm.rst | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/megolm.rst b/docs/megolm.rst index 56e5f1d..0994100 100644 --- a/docs/megolm.rst +++ b/docs/megolm.rst @@ -277,12 +277,13 @@ Limitations Message Replays --------------- -A message can be decrypted successfully multiple times. This means that a MITM -server can send multiple copies of a message and they will successfully decrypt. +A message can be decrypted successfully multiple times. This means that an +attacker can re-send a copy of an old message, and the recipient will treat it +as a new message. -To mitigate this it is recomendend that applications track the message indicies -they have recieved and that they reject messages with indicies that they've -already decrypted. +To mitigate this it is recomendend that applications track the ratchet indicies +they have recieved and that they reject messages with a ratchet index that +they've already decrypted. Lack of Transcript Consistency ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 092bf880f5adaf9897b1c869b67f6d9c5284eda5 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 21 Oct 2016 15:45:33 +0100 Subject: [PATCH 16/28] s/they've/they have/ --- docs/megolm.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/megolm.rst b/docs/megolm.rst index 0994100..dce64d7 100644 --- a/docs/megolm.rst +++ b/docs/megolm.rst @@ -283,7 +283,7 @@ as a new message. To mitigate this it is recomendend that applications track the ratchet indicies they have recieved and that they reject messages with a ratchet index that -they've already decrypted. +they have already decrypted. Lack of Transcript Consistency ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 884ad02413e334473a338986c2291a717defb662 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Fri, 21 Oct 2016 17:07:26 +0100 Subject: [PATCH 17/28] Spelling --- docs/megolm.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/megolm.rst b/docs/megolm.rst index dce64d7..03ee426 100644 --- a/docs/megolm.rst +++ b/docs/megolm.rst @@ -281,8 +281,8 @@ A message can be decrypted successfully multiple times. This means that an attacker can re-send a copy of an old message, and the recipient will treat it as a new message. -To mitigate this it is recomendend that applications track the ratchet indicies -they have recieved and that they reject messages with a ratchet index that +To mitigate this it is recommended that applications track the ratchet indices +they have received and that they reject messages with a ratchet index that they have already decrypted. Lack of Transcript Consistency From 21ce3491dd39485eac35ad850257a20fc99f330d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 21 Oct 2016 17:19:59 +0100 Subject: [PATCH 18/28] Clear random buf in olm_init_outbound_group_session All the other methods clear their random inputs. This one needs to do the same, to reduce the risk of the randomness being used elsewhere and leaking key info. --- include/olm/outbound_group_session.h | 2 +- src/outbound_group_session.c | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/include/olm/outbound_group_session.h b/include/olm/outbound_group_session.h index 90ccca3..663f1d2 100644 --- a/include/olm/outbound_group_session.h +++ b/include/olm/outbound_group_session.h @@ -96,7 +96,7 @@ size_t olm_init_outbound_group_session_random_length( */ size_t olm_init_outbound_group_session( OlmOutboundGroupSession *session, - uint8_t const * random, size_t random_length + uint8_t *random, size_t random_length ); /** diff --git a/src/outbound_group_session.c b/src/outbound_group_session.c index 4e4561a..ae45694 100644 --- a/src/outbound_group_session.c +++ b/src/outbound_group_session.c @@ -154,20 +154,23 @@ size_t olm_init_outbound_group_session_random_length( size_t olm_init_outbound_group_session( OlmOutboundGroupSession *session, - uint8_t const * random, size_t random_length + uint8_t *random, size_t random_length ) { + const uint8_t *random_ptr = random; + if (random_length < olm_init_outbound_group_session_random_length(session)) { /* Insufficient random data for new session */ session->last_error = OLM_NOT_ENOUGH_RANDOM; return (size_t)-1; } - megolm_init(&(session->ratchet), random, 0); - random += MEGOLM_RATCHET_LENGTH; + megolm_init(&(session->ratchet), random_ptr, 0); + random_ptr += MEGOLM_RATCHET_LENGTH; - _olm_crypto_ed25519_generate_key(random, &(session->signing_key)); - random += ED25519_RANDOM_LENGTH; + _olm_crypto_ed25519_generate_key(random_ptr, &(session->signing_key)); + random_ptr += ED25519_RANDOM_LENGTH; + _olm_unset(random, random_length); return 0; } From a7310c5821513d5c5b0609ec506dad1ae51603d3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 24 Oct 2016 10:06:06 +0100 Subject: [PATCH 19/28] Return the base64-encoded length of pickles make olm_pickle_* return the lengths of the base64-encoded pickles, rather than the raw pickle. (From the application's POV, the format of the pickle is opaque: it doesn't even know that it is base64-encoded. So returning the length of the raw pickle is particularly unhelpful.) --- src/pickle_encoding.c | 2 +- tests/test_group_session.cpp | 41 ++++++++++++++++++++---------------- tests/test_olm.cpp | 16 ++++++++------ 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/src/pickle_encoding.c b/src/pickle_encoding.c index 5d5f8d7..a56e9e3 100644 --- a/src/pickle_encoding.c +++ b/src/pickle_encoding.c @@ -60,7 +60,7 @@ size_t _olm_enc_output( raw_output, length ); _olm_encode_base64(raw_output, length, output); - return raw_length; + return base64_length; } diff --git a/tests/test_group_session.cpp b/tests/test_group_session.cpp index df46f0e..ad67adb 100644 --- a/tests/test_group_session.cpp +++ b/tests/test_group_session.cpp @@ -28,23 +28,26 @@ int main() { size_t pickle_length = olm_pickle_outbound_group_session_length(session); uint8_t pickle1[pickle_length]; - olm_pickle_outbound_group_session(session, - "secret_key", 10, - pickle1, pickle_length); + size_t res = olm_pickle_outbound_group_session( + session, "secret_key", 10, pickle1, pickle_length + ); + assert_equals(pickle_length, res); + uint8_t pickle2[pickle_length]; memcpy(pickle2, pickle1, pickle_length); uint8_t buffer2[size]; OlmOutboundGroupSession *session2 = olm_outbound_group_session(buffer2); - size_t res = olm_unpickle_outbound_group_session(session2, - "secret_key", 10, - pickle2, pickle_length); + res = olm_unpickle_outbound_group_session( + session2, "secret_key", 10, pickle2, pickle_length + ); assert_not_equals((size_t)-1, res); assert_equals(pickle_length, olm_pickle_outbound_group_session_length(session2)); - olm_pickle_outbound_group_session(session2, - "secret_key", 10, - pickle2, pickle_length); + res = olm_pickle_outbound_group_session( + session2, "secret_key", 10, pickle2, pickle_length + ); + assert_equals(pickle_length, res); assert_equals(pickle1, pickle2, pickle_length); } @@ -59,23 +62,25 @@ int main() { size_t pickle_length = olm_pickle_inbound_group_session_length(session); uint8_t pickle1[pickle_length]; - olm_pickle_inbound_group_session(session, - "secret_key", 10, - pickle1, pickle_length); + size_t res = olm_pickle_inbound_group_session( + session, "secret_key", 10, pickle1, pickle_length + ); + assert_equals(pickle_length, res); + uint8_t pickle2[pickle_length]; memcpy(pickle2, pickle1, pickle_length); uint8_t buffer2[size]; OlmInboundGroupSession *session2 = olm_inbound_group_session(buffer2); - size_t res = olm_unpickle_inbound_group_session(session2, - "secret_key", 10, - pickle2, pickle_length); + res = olm_unpickle_inbound_group_session( + session2, "secret_key", 10, pickle2, pickle_length + ); assert_not_equals((size_t)-1, res); assert_equals(pickle_length, olm_pickle_inbound_group_session_length(session2)); - olm_pickle_inbound_group_session(session2, - "secret_key", 10, - pickle2, pickle_length); + res = olm_pickle_inbound_group_session( + session2, "secret_key", 10, pickle2, pickle_length + ); assert_equals(pickle1, pickle2, pickle_length); } diff --git a/tests/test_olm.cpp b/tests/test_olm.cpp index af2c9f7..4619558 100644 --- a/tests/test_olm.cpp +++ b/tests/test_olm.cpp @@ -49,7 +49,9 @@ mock_random(ot_random, sizeof(ot_random)); std::size_t pickle_length = ::olm_pickle_account_length(account); std::uint8_t pickle1[pickle_length]; -::olm_pickle_account(account, "secret_key", 10, pickle1, pickle_length); +std::size_t res = ::olm_pickle_account(account, "secret_key", 10, pickle1, pickle_length); +assert_equals(pickle_length, res); + std::uint8_t pickle2[pickle_length]; std::memcpy(pickle2, pickle1, pickle_length); @@ -59,10 +61,10 @@ assert_not_equals(std::size_t(-1), ::olm_unpickle_account( account2, "secret_key", 10, pickle2, pickle_length )); assert_equals(pickle_length, ::olm_pickle_account_length(account2)); -::olm_pickle_account(account2, "secret_key", 10, pickle2, pickle_length); +res = ::olm_pickle_account(account2, "secret_key", 10, pickle2, pickle_length); +assert_equals(pickle_length, res); assert_equals(pickle1, pickle2, pickle_length); - } @@ -122,7 +124,9 @@ mock_random(random2, sizeof(random2)); std::size_t pickle_length = ::olm_pickle_session_length(session); std::uint8_t pickle1[pickle_length]; -::olm_pickle_session(session, "secret_key", 10, pickle1, pickle_length); +std::size_t res = ::olm_pickle_session(session, "secret_key", 10, pickle1, pickle_length); +assert_equals(pickle_length, res); + std::uint8_t pickle2[pickle_length]; std::memcpy(pickle2, pickle1, pickle_length); @@ -132,10 +136,10 @@ assert_not_equals(std::size_t(-1), ::olm_unpickle_session( session2, "secret_key", 10, pickle2, pickle_length )); assert_equals(pickle_length, ::olm_pickle_session_length(session2)); -::olm_pickle_session(session2, "secret_key", 10, pickle2, pickle_length); +res = ::olm_pickle_session(session2, "secret_key", 10, pickle2, pickle_length); +assert_equals(pickle_length, res); assert_equals(pickle1, pickle2, pickle_length); - } { /** Loopback test */ From 807fec2ebfa8b33fa75d51a795b9663f0d26fa5c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 24 Oct 2016 15:17:27 +0100 Subject: [PATCH 20/28] double_ratchet.svg --- docs/double_ratchet.svg | 1007 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 1007 insertions(+) create mode 100644 docs/double_ratchet.svg diff --git a/docs/double_ratchet.svg b/docs/double_ratchet.svg new file mode 100644 index 0000000..26ee8c3 --- /dev/null +++ b/docs/double_ratchet.svg @@ -0,0 +1,1007 @@ + + + + + + + + + + + + + + image/svg+xml + + + + + + + + + Initial Shared Secret + + + + + + + Secret + + HKDF + + + Root Key + + + + Chain Key + + + + Info + + "OLM_ROOT" + + + + Salt + + "" + + + + + + + Key + + + + Data + + HMAC + + + Chain Key + + "\x02" + + + + + + "\x01" + + + + + Key + + + + Data + + HMAC + + + Message Key + + + + + + + Secret + + + + + Salt + + "" + + + + Info + + "OLM_KEYS" + + HKDF + + + Cipher Key + + + + Mac Key + + + + IV + + + + + + + + + + Secret + + HKDF + + + Root Key + + + + Chain Key + + + + Info + + "OLM_RATCHET" + + + + Salt + + + + + + Alice's Chain + Bob's Chain + + + + + + Bob's Key + + + + Alice's Key + + ECDH + + + Shared Secret + + + From 05b48086a4918b2cb4b23ffbabef564555131186 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 24 Oct 2016 15:52:05 +0100 Subject: [PATCH 21/28] remove redundant svg --- docs/Axolotl.svg | 1007 ---------------------------------------------- 1 file changed, 1007 deletions(-) delete mode 100644 docs/Axolotl.svg diff --git a/docs/Axolotl.svg b/docs/Axolotl.svg deleted file mode 100644 index 26ee8c3..0000000 --- a/docs/Axolotl.svg +++ /dev/null @@ -1,1007 +0,0 @@ - - - - - - - - - - - - - - image/svg+xml - - - - - - - - - Initial Shared Secret - - - - - - - Secret - - HKDF - - - Root Key - - - - Chain Key - - - - Info - - "OLM_ROOT" - - - - Salt - - "" - - - - - - - Key - - - - Data - - HMAC - - - Chain Key - - "\x02" - - - - - - "\x01" - - - - - Key - - - - Data - - HMAC - - - Message Key - - - - - - - Secret - - - - - Salt - - "" - - - - Info - - "OLM_KEYS" - - HKDF - - - Cipher Key - - - - Mac Key - - - - IV - - - - - - - - - - Secret - - HKDF - - - Root Key - - - - Chain Key - - - - Info - - "OLM_RATCHET" - - - - Salt - - - - - - Alice's Chain - Bob's Chain - - - - - - Bob's Key - - - - Alice's Key - - ECDH - - - Shared Secret - - - From 64130c1f8b0a2dd09379ac02544897ad5a2bfb75 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 24 Oct 2016 16:31:42 +0100 Subject: [PATCH 22/28] Fix broken fuzzer compilation fuzz_group_decrypt.cpp got broken by 653790e; fix it up --- fuzzers/README.rst | 2 +- fuzzers/fuzz_group_decrypt.cpp | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fuzzers/README.rst b/fuzzers/README.rst index b3142ca..d052303 100644 --- a/fuzzers/README.rst +++ b/fuzzers/README.rst @@ -45,7 +45,7 @@ Usage notes: .. code:: - ./afl-fuzz -i- -o existing_output_dir [...etc...] + afl-fuzz -i- -o existing_output_dir [...etc...] 8. If it shows failures, pipe the failure case into ``./build/fuzzers/debug_``, fix, and repeat. diff --git a/fuzzers/fuzz_group_decrypt.cpp b/fuzzers/fuzz_group_decrypt.cpp index 1fc99d7..bb12d0e 100644 --- a/fuzzers/fuzz_group_decrypt.cpp +++ b/fuzzers/fuzz_group_decrypt.cpp @@ -54,6 +54,8 @@ int main(int argc, const char *argv[]) { uint8_t plaintext[max_length]; + uint32_t ratchet_index; + size_t length = check_error( olm_inbound_group_session_last_error, session, @@ -61,7 +63,7 @@ int main(int argc, const char *argv[]) { olm_group_decrypt( session, message_buffer, message_length, - plaintext, max_length + plaintext, max_length, &ratchet_index ) ); From 4367afc65e31b919c04b6e950765e03020359613 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 24 Oct 2016 16:51:20 +0100 Subject: [PATCH 23/28] Prepare changelog for v2.0.0 --- CHANGELOG.rst | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0ab2eeb..044dcac 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,8 +1,24 @@ +Changes in `2.0.0 `_ +=============================================================== + +This release includes the following changes since 1.3.0: + +* Fix a buffer bounds check when decoding group messages. +* Update ``olm_group_decrypt`` to return the ratchet index for decrypted + messages. +* Fix ``olm_pickle_account``, ``olm_pickle_session``, + ``olm_pickle_inbound_group_session`` and + ``olm_pickle_outbound_group_session`` to correctly return the length of the + pickled object. +* Add a `specification <./docs/megolm.rst>`_ of the Megolm ratchet, and add + some information on mitigating unknown key-share attacks to the `Olm + specification <./docs/olm.rst>`_. + Changes in `1.3.0 `_ =============================================================== -The release updates the group session identifier to avoid collisions. -The group sessions are now identified by their ed25519 public key. +This release updates the group session identifier to avoid collisions. +Group sessions are now identified by their ed25519 public key. These changes alter the pickle format of outbound group sessions, attempting to unpickle an outbound group session created with a previous version of olm From d02c457da578f20f132e6e9b72baaaea399c2e88 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 24 Oct 2016 17:22:43 +0100 Subject: [PATCH 24/28] Changelog: Mention install-headers --- CHANGELOG.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 044dcac..a35eedf 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,8 @@ This release includes the following changes since 1.3.0: * Add a `specification <./docs/megolm.rst>`_ of the Megolm ratchet, and add some information on mitigating unknown key-share attacks to the `Olm specification <./docs/olm.rst>`_. +* Add an ``install-headers`` target to the Makefile (and run it when installing + the library). (Credit to Emmanuel Gil Peyrot). Changes in `1.3.0 `_ =============================================================== From 27c7b4a767b371a74adab04401593f5f8860c7cd Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 25 Oct 2016 11:35:20 +0100 Subject: [PATCH 25/28] Version bump for 2.0.0 --- Makefile | 4 ++-- javascript/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 4877dfb..f903c24 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ #!/usr/bin/make -f -MAJOR := 1 -MINOR := 3 +MAJOR := 2 +MINOR := 0 PATCH := 0 VERSION := $(MAJOR).$(MINOR).$(PATCH) PREFIX ?= /usr/local diff --git a/javascript/package.json b/javascript/package.json index df43ce1..b65fb2e 100644 --- a/javascript/package.json +++ b/javascript/package.json @@ -1,6 +1,6 @@ { "name": "olm", - "version": "1.3.0", + "version": "2.0.0", "description": "An implementation of the Double Ratchet cryptographic ratchet", "main": "olm.js", "files": [ From 7e9f3bebb8390f975a76c0188ce4cb460fe6692e Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Tue, 25 Oct 2016 14:42:10 +0100 Subject: [PATCH 26/28] Document the return values for olm_matches_inbound_session --- include/olm/olm.h | 6 ++++-- tests/test_olm.cpp | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/include/olm/olm.h b/include/olm/olm.h index 3257e53..5764eb2 100644 --- a/include/olm/olm.h +++ b/include/olm/olm.h @@ -320,7 +320,8 @@ int olm_session_has_received_message( /** Checks if the PRE_KEY message is for this in-bound session. This can happen * if multiple messages are sent to this account before this account sends a - * message in reply. Returns olm_error() on failure. If the base64 + * message in reply. Returns 1 if the session matches. Returns 0 if the session + * does not match. Returns olm_error() on failure. If the base64 * couldn't be decoded then olm_session_last_error will be "INVALID_BASE64". * If the message was for an unsupported protocol version then * olm_session_last_error() will be "BAD_MESSAGE_VERSION". If the message @@ -333,7 +334,8 @@ size_t olm_matches_inbound_session( /** Checks if the PRE_KEY message is for this in-bound session. This can happen * if multiple messages are sent to this account before this account sends a - * message in reply. Returns olm_error() on failure. If the base64 + * message in reply. Returns 1 if the session matches. Returns 0 if the session + * does not match. Returns olm_error() on failure. If the base64 * couldn't be decoded then olm_session_last_error will be "INVALID_BASE64". * If the message was for an unsupported protocol version then * olm_session_last_error() will be "BAD_MESSAGE_VERSION". If the message diff --git a/tests/test_olm.cpp b/tests/test_olm.cpp index 4619558..b24cd90 100644 --- a/tests/test_olm.cpp +++ b/tests/test_olm.cpp @@ -165,6 +165,9 @@ std::uint8_t o_random[::olm_account_generate_one_time_keys_random_length( mock_random_b(o_random, sizeof(o_random)); ::olm_account_generate_one_time_keys(b_account, 42, o_random, sizeof(o_random)); +std::uint8_t a_id_keys[::olm_account_identity_keys_length(a_account)]; +::olm_account_identity_keys(a_account, a_id_keys, sizeof(a_id_keys)); + std::uint8_t b_id_keys[::olm_account_identity_keys_length(b_account)]; std::uint8_t b_ot_keys[::olm_account_one_time_keys_length(b_account)]; ::olm_account_identity_keys(b_account, b_id_keys, sizeof(b_id_keys)); @@ -176,8 +179,8 @@ std::uint8_t a_rand[::olm_create_outbound_session_random_length(a_session)]; mock_random_a(a_rand, sizeof(a_rand)); assert_not_equals(std::size_t(-1), ::olm_create_outbound_session( a_session, a_account, - b_id_keys + 15, 43, - b_ot_keys + 25, 43, + b_id_keys + 15, 43, // B's curve25519 identity key + b_ot_keys + 25, 43, // B's curve25519 one time key a_rand, sizeof(a_rand) )); @@ -202,6 +205,31 @@ std::uint8_t b_session_buffer[::olm_account_size()]; b_session, b_account, tmp_message_1, sizeof(message_1) ); +// Check that the inbound session matches the message it was created from. +std::memcpy(tmp_message_1, message_1, sizeof(message_1)); +assert_equals(std::size_t(1), ::olm_matches_inbound_session( + b_session, + tmp_message_1, sizeof(message_1) +)); + +// Check that the inbound session matches the key this message is supposed +// to be from. +std::memcpy(tmp_message_1, message_1, sizeof(message_1)); +assert_equals(std::size_t(1), ::olm_matches_inbound_session_from( + b_session, + a_id_keys + 15, 43, // A's curve125519 identity key. + tmp_message_1, sizeof(message_1) +)); + +// Check that the inbound session isn't from a different user. +std::memcpy(tmp_message_1, message_1, sizeof(message_1)); +assert_equals(std::size_t(0), ::olm_matches_inbound_session_from( + b_session, + b_id_keys + 15, 43, // B's curve25519 identity key. + tmp_message_1, sizeof(message_1) +)); + +// Check that we can decrypt the message. std::memcpy(tmp_message_1, message_1, sizeof(message_1)); std::uint8_t plaintext_1[::olm_decrypt_max_plaintext_length( b_session, 0, tmp_message_1, sizeof(message_1) From 700596b46aadcc224c1768b40d11cf2150b866ac Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 25 Oct 2016 14:50:15 +0100 Subject: [PATCH 27/28] Update python wrapper to run against libolm.so.2 --- python/olm/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/olm/_base.py b/python/olm/_base.py index bc5c206..80720d9 100644 --- a/python/olm/_base.py +++ b/python/olm/_base.py @@ -7,7 +7,7 @@ def read_random(n): return f.read(n) lib = cdll.LoadLibrary(os.path.join( - os.path.dirname(__file__), "..", "..", "build", "libolm.so.1") + os.path.dirname(__file__), "..", "..", "build", "libolm.so.2") ) lib.olm_error.argtypes = [] From f6c05be8c5d35e725a8a2ed5ad661398ac9f8cd2 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 27 Oct 2016 11:53:55 +0100 Subject: [PATCH 28/28] Add a document on signing keys --- .gitignore | 1 + Makefile | 1 + docs/signing.rst | 118 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+) create mode 100644 docs/signing.rst diff --git a/.gitignore b/.gitignore index 067636a..c19e757 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ /CHANGELOG.html /docs/megolm.html /docs/olm.html +/docs/signing.html /olm-*.tgz /README.html /tracing/README.html \ No newline at end of file diff --git a/Makefile b/Makefile index f903c24..77aa485 100644 --- a/Makefile +++ b/Makefile @@ -46,6 +46,7 @@ JS_POST := javascript/olm_outbound_group_session.js \ DOCS := tracing/README.html \ docs/megolm.html \ docs/olm.html \ + docs/signing.html \ README.html \ CHANGELOG.html diff --git a/docs/signing.rst b/docs/signing.rst new file mode 100644 index 0000000..7387794 --- /dev/null +++ b/docs/signing.rst @@ -0,0 +1,118 @@ +.. Copyright 2016 OpenMarket Ltd +.. +.. Licensed under the Apache License, Version 2.0 (the "License"); +.. you may not use this file except in compliance with the License. +.. You may obtain a copy of the License at +.. +.. http://www.apache.org/licenses/LICENSE-2.0 +.. +.. Unless required by applicable law or agreed to in writing, software +.. distributed under the License is distributed on an "AS IS" BASIS, +.. WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +.. See the License for the specific language governing permissions and +.. limitations under the License. + + +Signature keys and user identity in libolm +========================================== + +The use of any public-key based cryptography system such as Olm presents the +need for our users Alice and Bob to verify that they are in fact communicating +with each other, rather than a man-in-the-middle. Typically this requires an +out-of-band process in which Alice and Bob verify that they have the correct +public keys for each other. For example, this might be done via physical +presence or via a voice call. + +In the basic `Olm `_ protocol, it is sufficient to compare the public +Curve25519 identity keys. As a naive example, Alice would meet Bob and ensure +that the identity key she downloaded from the key server matched that shown by +his device. This prevents the eavesdropper Eve from decrypting any messages +sent from Alice to Bob, or from masquerading as Bob to send messages to Alice: +she has neither Alice's nor Bob's private identity key, so cannot successfully +complete the triple-DH calculation to compute the shared secret, :math:`S`, +which in turn prevents her decrypting intercepted messages, or from creating +new messages with valid MACs. Obviously, for protection to be complete, Bob +must similarly verify Alice's key. + +However, the use of the Curve25519 key as the "fingerprint" in this way makes +it difficult to carry out signing operations. For instance, it may be useful to +cross-sign identity keys for different devices, or, as discussed below, to sign +one-time keys. Curve25519 keys are intended for use in DH calculations, and +their use to calculate signatures is non-trivial. + +The solution adopted in this library is to generate a signing key for each +user. This is an `Ed25519`_ keypair, which is used to calculate a signature on +an object including both the public Ed25519 signing key and the public +Curve25519 identity key. It is then the **public Ed25519 signing key** which is +used as the device fingerprint which Alice and Bob verify with each other. + +By verifying the signatures on the key object, Alice and Bob then get the same +level of assurance about the ownership of the Curve25519 identity keys as if +they had compared those directly. + +Signing one-time keys +--------------------- + +The Olm protocol requires users to publish a set of one-time keys to a key +server. To establish an Olm session, the originator downloads a key for the +recipient from this server. The decision of whether to sign these one-time keys +is left to the application. There are both advantages and disadvantages to +doing so. + +Consider the scenario where one-time keys are unsigned. Alice wants to initiate +an Olm session with Bob. Bob uploads his one-time keys, :math:`E_B`, but Eve +replaces them with ones she controls, :math:`E_E`. Alice downloads one of the +compromised keys, and sends a pre-key message using a shared secret :math:`S`, +where: + +.. math:: + S = ECDH\left(I_A,\,E_E\right)\;\parallel\;ECDH\left(E_A,\,I_B\right)\; + \parallel\;ECDH\left(E_A,\,E_E\right) + +Eve cannot decrypt the message because she does not have the private parts of +either :math:`E_A` nor :math:`I_B`, so cannot calculate +:math:`ECDH\left(E_A,\,I_B\right)`. However, suppose she later compromises +Bob's identity key :math:`I_B`. This would give her the ability to decrypt any +pre-key messages sent to Bob using the compromised one-time keys, and is thus a +problematic loss of forward secrecy. If Bob signs his keys with his Ed25519 +signing key (and Alice verifies the signature before using them), this problem +is avoided. + +On the other hand, signing the one-time keys leads to a reduction in +deniability. Recall that the shared secret is calculated as follows: + +.. math:: + S = ECDH\left(I_A,\,E_B\right)\;\parallel\;ECDH\left(E_A,\,I_B\right)\; + \parallel\;ECDH\left(E_A,\,E_B\right) + +If keys are unsigned, a forger can make up values of :math:`E_A` and +:math:`E_B`, and construct a transcript of a conversation which looks like it +was between Alice and Bob. Alice and Bob can therefore plausibly deny their +partition in any conversation even if they are both forced to divulge their +private identity keys, since it is impossible to prove that the transcript was +a conversation between the two of them, rather than constructed by a forger. + +If :math:`E_B` is signed, it is no longer possible to construct arbitrary +transcripts. Given a transcript and Alice and Bob's identity keys, we can now +show that at least one of Alice or Bob was involved in the conversation, +because the ability to calculate :math:`ECDH\left(I_A,\,E_B\right)` requires +knowledge of the private parts of either :math:`I_A` (proving Alice's +involvement) or :math:`E_B` (proving Bob's involvement, via the +signature). Note that it remains impossible to show that *both* Alice and Bob +were involved. + +In conclusion, applications should consider whether to sign one-time keys based +on the trade-off between forward secrecy and deniability. + +License +------- + +This document is licensed under the `Apache License, Version 2.0 +`_. + +Feedback +-------- + +Questions and feedback can be sent to richard at matrix.org. + +.. _`Ed25519`: http://ed25519.cr.yp.to/