Remove the unix_ts_ms arg from decoding, use the sent timestamp internally

Author: Doy-lee

include/session/session_protocol.h

@@ -458,7 +458,8 @@ session_protocol_pro_features_for_msg session_protocol_pro_features_for_utf16(
 /// - `ed25519_privkey` -- The sender's libsodium-style secret key (64 bytes). Can also be passed as
 ///   a 32-byte seed. Used to encrypt the plaintext.
 /// - `ed25519_privkey_len` -- The length of the ed25519_privkey buffer in bytes (32 or 64).
-/// - `sent_timestamp_ms` -- The timestamp to assign to the message envelope, in milliseconds.
+/// - `sent_timestamp_ms` -- The timestamp to assign to the message envelope, in milliseconds. This
+///   should match the protobuf encoded Content's `sigtimestamp` in the given `plaintext`.
 /// - `pro_rotating_ed25519_privkey` -- Optional rotating Session Pro Ed25519 key (64-bytes or
 ///   32-byte seed) to sign the encoded content if you wish to entitle the message to Session Pro.
 ///   If provided, the corresponding proof must be set in the `Content`. The signature must not be
@@ -747,9 +748,6 @@ LIBSESSION_EXPORT void session_protocol_encode_for_destination_free(
 /// caller is done with the result.
 ///
 /// Inputs:
-/// - `unix_ts_ms` -- pass in the current system time which is used to determine, whether or
-///   not the Session Pro proof has expired or not if it is in the payload. Ignored if there's no
-///   proof in the message.
 /// - `pro_backend_pubkey` -- the Session Pro backend public key to verify the signature embedded in
 ///   the proof, validating whether or not the attached proof was indeed issued by an authorised
 ///   issuer. Ignored if there's no proof in the message.
@@ -800,7 +798,6 @@ session_protocol_decoded_envelope session_protocol_decode_envelope(
         const session_protocol_decode_envelope_keys* keys,
         const void* envelope_plaintext,
         size_t envelope_plaintext_len,
-        uint64_t unix_ts_ms,
         OPTIONAL const void* pro_backend_pubkey,
         size_t pro_backend_pubkey_len,
         OPTIONAL char* error,

include/session/session_protocol.hpp

@@ -397,7 +397,8 @@ std::vector<uint8_t> pad_message(std::span<const uint8_t> payload);
 ///   not be already encrypted and must not be padded.
 /// - ed25519_privkey -- The sender's libsodium-style secret key (64 bytes). Can also be passed as
 ///   a 32-byte seed. Used to encrypt the plaintext.
-/// - sent_timestamp -- The timestamp to assign to the message envelope, in milliseconds.
+/// - sent_timestamp -- The timestamp to assign to the message envelope, in milliseconds. This
+///   should match the protobuf encoded Content's `sigtimestamp` in the given `plaintext`.
 /// - recipient_pubkey -- The recipient's Session public key (33 bytes).
 /// - pro_rotating_ed25519_privkey -- Optional libsodium-style secret key (64 bytes) that is the
 ///   secret component of the user's Session Pro Proof `rotating_pubkey`. This key is authorised to
@@ -558,6 +559,12 @@ std::vector<uint8_t> encode_for_destination(
 /// the pro fields will be populated with data about the Session Pro proof embedded in the envelope
 /// including the features used and if the proof was valid/expired e.t.c.
 ///
+/// The sent timestamp of the protobuf encoded content is the timestamp that the Session Pro proof
+/// is checked against to ensure that it was not expired at the time the message was constructed.
+/// Once a proof is decoded successfully and was not deemed expired (e.g. pro status returned
+/// success) any additional timestamp can be checked against the proof by comparing the timestamp on
+/// the proof itself directly (since the proof has been cryptographically verified at that point).
+///
 /// This function will throw if parsing failed such as a required field is missing, the field is
 /// smaller or larger than expected, decryption failed, or an invariant failed. Notably this
 /// function does not throw if the Session Pro proof failed to verify. Always check the pro status
@@ -574,9 +581,6 @@ std::vector<uint8_t> encode_for_destination(
 ///
 /// - `envelope_payload` -- the envelope payload either encrypted (groups v2 style) or unencrypted
 ///   (1o1 or legacy groups).
-/// - `unix_ts` -- pass in the current system time which is used to determine, whether or
-///   not the Session Pro proof has expired or not if it is in the payload. Ignored if there's no
-///   proof in the message.
 /// - `pro_backend_pubkey` -- the Session Pro backend public key to verify the signature embedded in
 ///   the proof, validating whether or not the attached proof was indeed issued by an authorised
 ///   issuer
@@ -600,7 +604,6 @@ std::vector<uint8_t> encode_for_destination(
 DecodedEnvelope decode_envelope(
         const DecodeEnvelopeKey& keys,
         std::span<const uint8_t> envelope_payload,
-        std::chrono::sys_time<std::chrono::milliseconds> unix_ts,
         const array_uc32& pro_backend_pubkey);
 
 /// API: session_protocol/decode_for_community

src/session_protocol.cpp

@@ -649,7 +649,6 @@ std::vector<uint8_t> encode_for_destination(
 DecodedEnvelope decode_envelope(
         const DecodeEnvelopeKey& keys,
         std::span<const uint8_t> envelope_payload,
-        std::chrono::sys_time<std::chrono::milliseconds> unix_ts,
         const array_uc32& pro_backend_pubkey) {
     DecodedEnvelope result = {};
     SessionProtos::Envelope envelope = {};
@@ -819,7 +818,7 @@ DecodedEnvelope decode_envelope(
     SessionProtos::Content content = {};
     if (!content.ParseFromArray(result.content_plaintext.data(), result.content_plaintext.size()))
         throw std::runtime_error{fmt::format(
-                "Parse content from envelope failed: {}", result.content_plaintext.size())};
+                "Parse content from envelope failed: {}b", result.content_plaintext.size())};
 
     // A signature must always be present on the envelope. This is to make a pro and non-pro
     // envelope indistinguishable. If the message does not have pro then this signature must still
@@ -842,6 +841,12 @@ DecodedEnvelope decode_envelope(
         std::memcpy(result.envelope.pro_sig.data(), pro_sig.data(), pro_sig.size());
 
         if (content.has_promessage()) {
+            if (!content.sigtimestamp())
+                throw std::runtime_error{fmt::format(
+                        "Content does not have signature timestamp set, pro proof expiry is "
+                        "unverifiable (content was {}b)",
+                        result.content_plaintext.size())};
+
             // Mark the envelope as having a pro signature that the caller can use.
             result.envelope.flags |= SESSION_PROTOCOL_ENVELOPE_FLAGS_PRO_SIG;
             DecodedPro& pro = result.pro.emplace();
@@ -891,6 +896,8 @@ DecodedEnvelope decode_envelope(
 
             // Note that we sign the envelope content wholesale. For 1o1 which are padded to 160
             // bytes, this means that we expected the user to have signed the padding as well.
+            auto unix_ts = std::chrono::sys_time<std::chrono::milliseconds>(
+                    std::chrono::milliseconds(content.sigtimestamp()));
             signed_msg.msg = to_span(envelope.content());
             pro.status = proof.status(pro_backend_pubkey, unix_ts, signed_msg);
         }
@@ -1428,7 +1435,6 @@ session_protocol_decoded_envelope session_protocol_decode_envelope(
         const session_protocol_decode_envelope_keys* keys,
         const void* envelope_plaintext,
         size_t envelope_plaintext_len,
-        uint64_t unix_ts_ms,
         const void* pro_backend_pubkey,
         size_t pro_backend_pubkey_len,
         char* error,
@@ -1465,8 +1471,6 @@ session_protocol_decoded_envelope session_protocol_decode_envelope(
             result_cpp = decode_envelope(
                     keys_cpp,
                     {static_cast<const uint8_t*>(envelope_plaintext), envelope_plaintext_len},
-                    std::chrono::sys_time<std::chrono::milliseconds>(
-                            std::chrono::milliseconds(unix_ts_ms)),
                     pro_backend_pubkey_cpp.data);
             result.success = true;
             break;

tests/test_session_protocol.cpp

@@ -28,13 +28,18 @@ static SerialisedProtobufContentWithProForTesting build_protobuf_content_with_se
         std::string_view data_body,
         const array_uc64& user_rotating_privkey,
         const array_uc64& pro_backend_privkey,
+        std::chrono::sys_seconds content_unix_ts,
         std::chrono::sys_seconds pro_expiry_unix_ts,
         session_protocol_pro_message_bitset msg_bitset,
         session_protocol_pro_profile_bitset profile_bitset) {
     SerialisedProtobufContentWithProForTesting result = {};
 
     // Create protobuf `Content.dataMessage`
     SessionProtos::Content content = {};
+    content.set_sigtimestamp(std::chrono::duration_cast<std::chrono::milliseconds>(
+                                     content_unix_ts.time_since_epoch())
+                                     .count());
+
     SessionProtos::DataMessage* data = content.mutable_datamessage();
     data->set_body(std::string(data_body));
 
@@ -234,6 +239,8 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
         std::string plaintext;
         {
             SessionProtos::Content content = {};
+            content.set_sigtimestamp(timestamp_ms.time_since_epoch().count());
+
             SessionProtos::DataMessage* data = content.mutable_datamessage();
             data->set_body(std::string(data_body));
             plaintext = content.SerializeAsString();
@@ -268,7 +275,6 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
                 &decrypt_keys,
                 encrypt_result.ciphertext.data,
                 encrypt_result.ciphertext.size,
-                timestamp_ms.time_since_epoch().count(),
                 pro_backend_ed_pk.data(),
                 pro_backend_ed_pk.size(),
                 error,
@@ -309,6 +315,7 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
                     /*data_body*/ data_body,
                     /*user_rotating_privkey*/ user_pro_ed_sk,
                     /*pro_backend_privkey*/ pro_backend_ed_sk,
+                    /*content_unix_ts=*/timestamp_s,
                     /*pro_expiry_unix_ts*/ timestamp_s,
                     /*msg_bitset*/ {},
                     /*profile_bitset*/ {});
@@ -388,7 +395,6 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
                 &decrypt_keys,
                 encrypt_result.ciphertext.data,
                 encrypt_result.ciphertext.size,
-                timestamp_ms.time_since_epoch().count(),
                 pro_backend_ed_pk.data(),
                 pro_backend_ed_pk.size(),
                 error,
@@ -434,6 +440,7 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
                         /*data_body*/ large_message,
                         /*user_rotating_privkey*/ user_pro_ed_sk,
                         /*pro_backend_privkey*/ pro_backend_ed_sk,
+                        /*content_unix_ts*/ timestamp_s,
                         /*pro_expiry_unix_ts*/ timestamp_s,
                         /*msg_bitset*/ pro_msg.bitset,
                         /*proilfe_bitset*/ profile_bitset);
@@ -462,7 +469,6 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
                 &decrypt_keys,
                 encrypt_result.ciphertext.data,
                 encrypt_result.ciphertext.size,
-                timestamp_ms.time_since_epoch().count(),
                 pro_backend_ed_pk.data(),
                 pro_backend_ed_pk.size(),
                 error,
@@ -565,7 +571,6 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
                 &decrypt_keys,
                 encrypt_result.ciphertext.data,
                 encrypt_result.ciphertext.size,
-                timestamp_ms.time_since_epoch().count(),
                 pro_backend_ed_pk.data(),
                 pro_backend_ed_pk.size(),
                 error,
@@ -609,7 +614,6 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
                     &decrypt_keys,
                     encrypt_result.ciphertext.data,
                     encrypt_result.ciphertext.size,
-                    timestamp_ms.time_since_epoch().count(),
                     pro_backend_ed_pk.data(),
                     pro_backend_ed_pk.size(),
                     error,
@@ -639,11 +643,44 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
 
         // Try decrypt with a timestamp past the pro proof expiry date
         {
+            // Build protobuf `Content` message, serialise to `plaintext` and get it signed by the
+            // user's "Session Pro" key into `sig_over_plaintext_with_user_pro_key`
+            std::chrono::milliseconds bad_timestamp_ms =
+                    std::chrono::duration_cast<std::chrono::milliseconds>(
+                            protobuf_content.proof.expiry_unix_ts.time_since_epoch()) +
+                    std::chrono::seconds(1);
+
+            SerialisedProtobufContentWithProForTesting bad_protobuf_content =
+                    build_protobuf_content_with_session_pro(
+                            /*data_body*/ data_body,
+                            /*user_rotating_privkey*/ user_pro_ed_sk,
+                            /*pro_backend_privkey*/ pro_backend_ed_sk,
+                            /*content_unix_ts=*/
+                            std::chrono::sys_seconds(
+                                    std::chrono::duration_cast<std::chrono::seconds>(
+                                            bad_timestamp_ms)),
+                            /*pro_expiry_unix_ts*/ timestamp_s,
+                            /*msg_bitset*/ {},
+                            /*profile_bitset*/ {});
+
+            session_protocol_encoded_for_destination encrypt_bad_result =
+                    session_protocol_encode_for_1o1(
+                            bad_protobuf_content.plaintext.data(),
+                            bad_protobuf_content.plaintext.size(),
+                            keys.ed_sk0.data(),
+                            keys.ed_sk0.size(),
+                            bad_timestamp_ms.count(),
+                            &base_dest.recipient_pubkey,
+                            user_pro_ed_sk.data(),
+                            user_pro_ed_sk.size(),
+                            error,
+                            sizeof(error));
+            REQUIRE(encrypt_bad_result.error_len_incl_null_terminator == 0);
+
             session_protocol_decoded_envelope decrypt_result = session_protocol_decode_envelope(
                     &decrypt_keys,
-                    encrypt_result.ciphertext.data,
-                    encrypt_result.ciphertext.size,
-                    protobuf_content.proof.expiry_unix_ts.time_since_epoch().count() + 1,
+                    encrypt_bad_result.ciphertext.data,
+                    encrypt_bad_result.ciphertext.size,
                     pro_backend_ed_pk.data(),
                     pro_backend_ed_pk.size(),
                     error,
@@ -662,7 +699,6 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
                     &decrypt_keys,
                     encrypt_result.ciphertext.data,
                     encrypt_result.ciphertext.size,
-                    protobuf_content.proof.expiry_unix_ts.time_since_epoch().count(),
                     bad_pro_backend_ed_pk.data(),
                     bad_pro_backend_ed_pk.size(),
                     error,
@@ -684,7 +720,6 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
                     &bad_decrypt_keys,
                     encrypt_result.ciphertext.data,
                     encrypt_result.ciphertext.size,
-                    protobuf_content.proof.expiry_unix_ts.time_since_epoch().count(),
                     pro_backend_ed_pk.data(),
                     pro_backend_ed_pk.size(),
                     error,
@@ -707,9 +742,6 @@ TEST_CASE("Session protocol helpers C API", "[session-protocol][helpers]") {
                     &multi_decrypt_keys,
                     encrypt_result.ciphertext.data,
                     encrypt_result.ciphertext.size,
-                    std::chrono::duration_cast<std::chrono::seconds>(
-                            protobuf_content.proof.expiry_unix_ts.time_since_epoch())
-                            .count(),
                     pro_backend_ed_pk.data(),
                     pro_backend_ed_pk.size(),
                     error,