From 39a1ee0b18f0fced6d7bc293cc9a46ea70ec9e96 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 1 Oct 2019 11:14:16 +0100 Subject: [PATCH 1/4] Add olm_session_describe As a way to dump the state of an olm session, ie. the chain indicies, so we can debug why olm sessions break and get out of sync. --- include/olm/olm.h | 2 ++ include/olm/session.hh | 12 ++++++++++++ javascript/olm_post.js | 7 +++++++ src/olm.cpp | 6 ++++++ src/session.cpp | 29 +++++++++++++++++++++++++++++ 5 files changed, 56 insertions(+) diff --git a/include/olm/olm.h b/include/olm/olm.h index 61ea07b..770be72 100644 --- a/include/olm/olm.h +++ b/include/olm/olm.h @@ -318,6 +318,8 @@ int olm_session_has_received_message( OlmSession *session ); +const char * olm_session_describe(OlmSession * 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. The one_time_key_message buffer is destroyed. Returns 1 if diff --git a/include/olm/session.hh b/include/olm/session.hh index 9d44816..86598dd 100644 --- a/include/olm/session.hh +++ b/include/olm/session.hh @@ -17,6 +17,8 @@ #include "olm/ratchet.hh" +#define DESCRIBE_BUFFER_LEN 256 + namespace olm { struct Account; @@ -35,6 +37,8 @@ struct Session { bool received_message; + char describe_buffer[DESCRIBE_BUFFER_LEN]; + _olm_curve25519_public_key alice_identity_key; _olm_curve25519_public_key alice_base_key; _olm_curve25519_public_key bob_one_time_key; @@ -131,6 +135,14 @@ struct Session { std::uint8_t const * message, std::size_t message_length, std::uint8_t * plaintext, std::size_t max_plaintext_length ); + + /** + * Return a string describing this session and its state (not including the + * private key) + * The pointer returned refers to an internal buffer which will be valid + * up until the next time describe() is called. + */ + const char *describe(); }; diff --git a/javascript/olm_post.js b/javascript/olm_post.js index ad058d9..c0498db 100644 --- a/javascript/olm_post.js +++ b/javascript/olm_post.js @@ -459,6 +459,13 @@ Session.prototype['decrypt'] = restore_stack(function( }); +Session.prototype['describe'] = restore_stack(function() { + var description_buf = session_method(Module['_olm_session_describe'])( + this.ptr + ); + return UTF8ToString(description_buf); +}); + function Utility() { var size = Module['_olm_utility_size'](); this.buf = malloc(size); diff --git a/src/olm.cpp b/src/olm.cpp index d626c84..65e506d 100644 --- a/src/olm.cpp +++ b/src/olm.cpp @@ -535,6 +535,12 @@ int olm_session_has_received_message( return from_c(session)->received_message; } +const char * olm_session_describe( + OlmSession * session +) { + return from_c(session)->describe(); +} + size_t olm_matches_inbound_session( OlmSession * session, void * one_time_key_message, size_t message_length diff --git a/src/session.cpp b/src/session.cpp index f1bc5a7..bad19de 100644 --- a/src/session.cpp +++ b/src/session.cpp @@ -21,6 +21,7 @@ #include "olm/pickle.hh" #include +#include namespace { @@ -397,6 +398,34 @@ std::size_t olm::Session::decrypt( return result; } +const char * olm::Session::describe() { + describe_buffer[0] = '\0'; + char *buf_pos = describe_buffer; + + buf_pos += snprintf( + buf_pos, DESCRIBE_BUFFER_LEN - (buf_pos - describe_buffer), + "sender chain index: %d ", ratchet.sender_chain[0].chain_key.index + ); + + buf_pos += snprintf(buf_pos, DESCRIBE_BUFFER_LEN - (buf_pos - describe_buffer), "receiver chain indcies:"); + for (size_t i = 0; i < ratchet.receiver_chains.size(); ++i) { + buf_pos += snprintf( + buf_pos, DESCRIBE_BUFFER_LEN - (buf_pos - describe_buffer), + " %d", ratchet.receiver_chains[i].chain_key.index + ); + } + + buf_pos += snprintf(buf_pos, DESCRIBE_BUFFER_LEN - (buf_pos - describe_buffer), " skipped message keys:"); + for (size_t i = 0; i < ratchet.skipped_message_keys.size(); ++i) { + buf_pos += snprintf( + buf_pos, DESCRIBE_BUFFER_LEN - (buf_pos - describe_buffer), + " %d", ratchet.skipped_message_keys[i].message_key.index + ); + } + + return describe_buffer; +} + namespace { // the master branch writes pickle version 1; the logging_enabled branch writes // 0x80000001. From e73a208fb2b70fb5d195a74956f22ea6557d361f Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 1 Oct 2019 11:18:05 +0100 Subject: [PATCH 2/4] doc string --- include/olm/olm.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/olm/olm.h b/include/olm/olm.h index 770be72..242277a 100644 --- a/include/olm/olm.h +++ b/include/olm/olm.h @@ -318,6 +318,12 @@ int olm_session_has_received_message( OlmSession *session ); +/** + * Return a string describing the internal state of an olm session, + * for debugging and logging purposes. The pointer returned points + * to an internal buffer and is valid until the next call to + * olm_session_describe on the same olm session object. + */ const char * olm_session_describe(OlmSession * session); /** Checks if the PRE_KEY message is for this in-bound session. This can happen From b482321213e6e896d0981c266bed12f4e1f67441 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 4 Oct 2019 11:43:40 +0100 Subject: [PATCH 3/4] Pass in a buffer to olm_session_describe instead of having a static one, as that could end up taking up a lot of memory if your app keeps olm sessions hanging about. --- include/olm/olm.h | 8 +++----- include/olm/session.hh | 14 +++++--------- javascript/olm_post.js | 14 ++++++++++---- src/olm.cpp | 6 +++--- src/session.cpp | 16 ++++++++-------- 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/include/olm/olm.h b/include/olm/olm.h index 242277a..a0ef46a 100644 --- a/include/olm/olm.h +++ b/include/olm/olm.h @@ -319,12 +319,10 @@ int olm_session_has_received_message( ); /** - * Return a string describing the internal state of an olm session, - * for debugging and logging purposes. The pointer returned points - * to an internal buffer and is valid until the next call to - * olm_session_describe on the same olm session object. + * Write a string describing the internal state of an olm session + * to the buffer provided for debugging and logging purposes. */ -const char * olm_session_describe(OlmSession * session); +void olm_session_describe(OlmSession * session, char *buf, size_t buflen); /** 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 diff --git a/include/olm/session.hh b/include/olm/session.hh index 86598dd..ce05fc8 100644 --- a/include/olm/session.hh +++ b/include/olm/session.hh @@ -17,8 +17,6 @@ #include "olm/ratchet.hh" -#define DESCRIBE_BUFFER_LEN 256 - namespace olm { struct Account; @@ -37,8 +35,6 @@ struct Session { bool received_message; - char describe_buffer[DESCRIBE_BUFFER_LEN]; - _olm_curve25519_public_key alice_identity_key; _olm_curve25519_public_key alice_base_key; _olm_curve25519_public_key bob_one_time_key; @@ -137,12 +133,12 @@ struct Session { ); /** - * Return a string describing this session and its state (not including the - * private key) - * The pointer returned refers to an internal buffer which will be valid - * up until the next time describe() is called. + * Write a string describing this session and its state (not including the + * private key) into the buffer provided. + * + * Takes a buffer to write to and the length of that buffer */ - const char *describe(); + void describe(char *buf, size_t buflen); }; diff --git a/javascript/olm_post.js b/javascript/olm_post.js index c0498db..e204f30 100644 --- a/javascript/olm_post.js +++ b/javascript/olm_post.js @@ -460,10 +460,16 @@ Session.prototype['decrypt'] = restore_stack(function( }); Session.prototype['describe'] = restore_stack(function() { - var description_buf = session_method(Module['_olm_session_describe'])( - this.ptr - ); - return UTF8ToString(description_buf); + var description_buf; + try { + description_buf = malloc(256); + session_method(Module['_olm_session_describe'])( + this.ptr, description_buf, 256 + ); + return UTF8ToString(description_buf); + } finally { + if (description_buf !== undefined) free(description_buf); + } }); function Utility() { diff --git a/src/olm.cpp b/src/olm.cpp index 65e506d..0333b10 100644 --- a/src/olm.cpp +++ b/src/olm.cpp @@ -535,10 +535,10 @@ int olm_session_has_received_message( return from_c(session)->received_message; } -const char * olm_session_describe( - OlmSession * session +void olm_session_describe( + OlmSession * session, char *buf, size_t buflen ) { - return from_c(session)->describe(); + from_c(session)->describe(buf, buflen); } size_t olm_matches_inbound_session( diff --git a/src/session.cpp b/src/session.cpp index bad19de..d6761d5 100644 --- a/src/session.cpp +++ b/src/session.cpp @@ -398,32 +398,32 @@ std::size_t olm::Session::decrypt( return result; } -const char * olm::Session::describe() { +void olm::Session::describe(char *describe_buffer, size_t buflen) { + if (buflen == 0) return; + describe_buffer[0] = '\0'; char *buf_pos = describe_buffer; buf_pos += snprintf( - buf_pos, DESCRIBE_BUFFER_LEN - (buf_pos - describe_buffer), + buf_pos, buflen - (buf_pos - describe_buffer), "sender chain index: %d ", ratchet.sender_chain[0].chain_key.index ); - buf_pos += snprintf(buf_pos, DESCRIBE_BUFFER_LEN - (buf_pos - describe_buffer), "receiver chain indcies:"); + buf_pos += snprintf(buf_pos, buflen - (buf_pos - describe_buffer), "receiver chain indcies:"); for (size_t i = 0; i < ratchet.receiver_chains.size(); ++i) { buf_pos += snprintf( - buf_pos, DESCRIBE_BUFFER_LEN - (buf_pos - describe_buffer), + buf_pos, buflen - (buf_pos - describe_buffer), " %d", ratchet.receiver_chains[i].chain_key.index ); } - buf_pos += snprintf(buf_pos, DESCRIBE_BUFFER_LEN - (buf_pos - describe_buffer), " skipped message keys:"); + buf_pos += snprintf(buf_pos, buflen - (buf_pos - describe_buffer), " skipped message keys:"); for (size_t i = 0; i < ratchet.skipped_message_keys.size(); ++i) { buf_pos += snprintf( - buf_pos, DESCRIBE_BUFFER_LEN - (buf_pos - describe_buffer), + buf_pos, buflen - (buf_pos - describe_buffer), " %d", ratchet.skipped_message_keys[i].message_key.index ); } - - return describe_buffer; } namespace { From fc423fad15ee3daaf236f211d67a25d03f63b560 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 8 Oct 2019 17:44:09 -0400 Subject: [PATCH 4/4] check return value of snprintf, fix typo, add clarification --- include/olm/olm.h | 4 ++-- src/session.cpp | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/include/olm/olm.h b/include/olm/olm.h index a0ef46a..6a2b3fb 100644 --- a/include/olm/olm.h +++ b/include/olm/olm.h @@ -319,8 +319,8 @@ int olm_session_has_received_message( ); /** - * Write a string describing the internal state of an olm session - * to the buffer provided for debugging and logging purposes. + * Write a null-terminated string describing the internal state of an olm + * session to the buffer provided for debugging and logging purposes. */ void olm_session_describe(OlmSession * session, char *buf, size_t buflen); diff --git a/src/session.cpp b/src/session.cpp index d6761d5..b619e56 100644 --- a/src/session.cpp +++ b/src/session.cpp @@ -404,25 +404,32 @@ void olm::Session::describe(char *describe_buffer, size_t buflen) { describe_buffer[0] = '\0'; char *buf_pos = describe_buffer; - buf_pos += snprintf( + int size; + + size = snprintf( buf_pos, buflen - (buf_pos - describe_buffer), "sender chain index: %d ", ratchet.sender_chain[0].chain_key.index ); + if (size > 0) buf_pos += size; - buf_pos += snprintf(buf_pos, buflen - (buf_pos - describe_buffer), "receiver chain indcies:"); + size = snprintf(buf_pos, buflen - (buf_pos - describe_buffer), "receiver chain indices:"); + if (size > 0) buf_pos += size; for (size_t i = 0; i < ratchet.receiver_chains.size(); ++i) { - buf_pos += snprintf( + size = snprintf( buf_pos, buflen - (buf_pos - describe_buffer), " %d", ratchet.receiver_chains[i].chain_key.index ); + if (size > 0) buf_pos += size; } - buf_pos += snprintf(buf_pos, buflen - (buf_pos - describe_buffer), " skipped message keys:"); + size = snprintf(buf_pos, buflen - (buf_pos - describe_buffer), " skipped message keys:"); + if (size >= 0) buf_pos += size; for (size_t i = 0; i < ratchet.skipped_message_keys.size(); ++i) { - buf_pos += snprintf( + size = snprintf( buf_pos, buflen - (buf_pos - describe_buffer), " %d", ratchet.skipped_message_keys[i].message_key.index ); + if (size > 0) buf_pos += size; } }