Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add message events #166

Closed
claucece opened this issue Nov 6, 2018 · 7 comments
Closed

Add message events #166

claucece opened this issue Nov 6, 2018 · 7 comments
Labels
API architecture importance low An issue that is not very important. If we can have it, great - if not, it's OK too
Milestone

Comments

@claucece
Copy link
Member

claucece commented Nov 6, 2018

We can use:

/* Handle and send the appropriate message(s) to the sender/recipient
 * depending on the message events. All the events only require an opdata,
 * the event, and the context. The message and err will be NULL except for
 * some events (see below). The possible events are:
 * - OTRL_MSGEVENT_ENCRYPTION_REQUIRED
 *      Our policy requires encryption but we are trying to send
 *      an unencrypted message out.
 * - OTRL_MSGEVENT_ENCRYPTION_ERROR
 *      An error occured while encrypting a message and the message
 *      was not sent.
 * - OTRL_MSGEVENT_CONNECTION_ENDED
 *      Message has not been sent because our buddy has ended the
 *      private conversation. We should either close the connection,
 *      or refresh it.
 * - OTRL_MSGEVENT_SETUP_ERROR
 *      A private conversation could not be set up. A gcry_error_t
 *      will be passed.
 * - OTRL_MSGEVENT_MSG_REFLECTED
 *      Received our own OTR messages.
 * - OTRL_MSGEVENT_MSG_RESENT
 *      The previous message was resent.
 * - OTRL_MSGEVENT_RCVDMSG_NOT_IN_PRIVATE
 *      Received an encrypted message but cannot read
 *      it because no private connection is established yet.
 * - OTRL_MSGEVENT_RCVDMSG_UNREADABLE
 *      Cannot read the received message.
 * - OTRL_MSGEVENT_RCVDMSG_MALFORMED
 *      The message received contains malformed data.
 * - OTRL_MSGEVENT_LOG_HEARTBEAT_RCVD
 *      Received a heartbeat.
 * - OTRL_MSGEVENT_LOG_HEARTBEAT_SENT
 *      Sent a heartbeat.
 * - OTRL_MSGEVENT_RCVDMSG_GENERAL_ERR
 *      Received a general OTR error. The argument 'message' will
 *      also be passed and it will contain the OTR error message.
 * - OTRL_MSGEVENT_RCVDMSG_UNENCRYPTED
 *      Received an unencrypted message. The argument 'message' will
  *      also be passed and it will contain the plaintext message.
  * - OTRL_MSGEVENT_RCVDMSG_UNRECOGNIZED
  *      Cannot recognize the type of OTR message received.
  * - OTRL_MSGEVENT_RCVDMSG_FOR_OTHER_INSTANCE
  *      Received and discarded a message intended for another instance. */
@claucece claucece added discuss importance low An issue that is not very important. If we can have it, great - if not, it's OK too labels Nov 6, 2018
@claucece
Copy link
Member Author

Ok, so we don't necessarily need them; but message events are used, from a client level, to generate notifications to the user, like the ones stated here: otrv4/pidgin-otrng#49 We can also tweak what we have to generate the same thing, though...

@claucece
Copy link
Member Author

Any ideas? @olabini @DrWhax .. I can explain more in detail...

@olabini
Copy link
Contributor

olabini commented Dec 22, 2018

Ah, I see. Yeah, those notifications are useful to have, and events definitely feel like the cleanest way of doing them. Do you have alternatives? Otherwise I think this is good.

@claucece
Copy link
Member Author

Well, in libotr, message events are handled in the "top" sending and receiving funcs. So when, for example, a hearbeat message is sent, the event is also raised:

		    } else if (edata.ignore_message != 1 &&
			    context->context_priv->their_keyid > 0) {
			/* If it's *not* a heartbeat, and we haven't
			 * sent anything in a while, also send a
			 * heartbeat. */
			time_t now = time(NULL);
			if (context->context_priv->lastsent <
				(now - HEARTBEAT_INTERVAL)) {
			    char *heartbeat;

			    /* Create the heartbeat message */
			    err = otrl_proto_create_data(&heartbeat,
				    context, "", NULL,
				    OTRL_MSGFLAGS_IGNORE_UNREADABLE,
				    NULL);
			    if (!err) {
				/* Send it, and inject a debug message */
				if (ops->inject_message) {
				    ops->inject_message(opdata, accountname,
					    protocol, sender, heartbeat);
				}
				free(heartbeat);

				context->context_priv->lastsent = now;
				otrl_context_update_recent_child(context, 1);

				/* Signal an event for the heartbeat message */
				if (ops->handle_msg_event) {
				    ops->handle_msg_event(opdata,
					    OTRL_MSGEVENT_LOG_HEARTBEAT_SENT,
					    context, NULL,
					    gcry_error(GPG_ERR_NO_ERROR));
				}
			    }
			}
		    }

Our code is different, and we don't have major functions that handle everything; but rather small functions, like this:

if (otr->client->should_heartbeat(otr->last_sent)) {
      otrng_debug_enter("trying to send a heartbeat message");
      if (!otrng_send_message(&response->to_send, "", warn, NULL,
                              MSG_FLAGS_IGNORE_UNREADABLE, otr)) {
        otrng_secure_wipe(mac_key, MAC_KEY_BYTES);
        otrng_data_message_free(msg);
        return OTRNG_ERROR;
      }
      otrng_debug_exit("heartbeat message sent");
      otr->last_sent = time(NULL);
    }

    otrng_secure_wipe(mac_key, MAC_KEY_BYTES);
    otrng_data_message_free(msg);

my idea is that there we can create a callback handle_msg_event which does the same as in libotr, like this one:

tstatic void display_error_message_cb(const otrng_error_event event,
                                      string_p *to_display,
                                      const otrng_s *conv) {
  otrng_client_callbacks_display_error_message(
      conv->client->global_state->callbacks, event, to_display, conv);
}

which is used every time an error message is received:

tstatic otrng_result receive_error_message(otrng_response_s *response,
                                           const string_p msg, otrng_s *otr) {
  otrng_error_event error_event = OTRNG_ERROR_NONE;

  if (strncmp(msg, "ERROR_1:", 8) == 0) {
    error_event = OTRNG_ERROR_UNREADABLE_EVENT;
    display_error_message_cb(error_event, &response->to_display, otr);

    if (otr->policy_type & OTRNG_ERROR_START_DAKE) {
      return otrng_build_query_message(&response->to_send, "", otr);
    }

We can use the same callback for every event required:

  • OTRL_MSGEVENT_ENCRYPTION_REQUIRED (for the policy)
  • OTRL_MSGEVENT_ENCRYPTION_ERROR (encryption error)
  • OTRL_MSGEVENT_CONNECTION_ENDED (unsure if we need this one..)
  • OTRL_MSGEVENT_SETUP_ERROR (a gcry_error_t error)
  • OTRL_MSGEVENT_MSG_REFLECTED (unsure if we will need it)
  • OTRL_MSGEVENT_MSG_RESENT
  • OTRL_MSGEVENT_RCVDMSG_NOT_IN_PRIVATE (this is prob already covered by the error)
  • OTRL_MSGEVENT_RCVDMSG_UNREADABLE (this is prob already covered by the error)
  • OTRL_MSGEVENT_RCVDMSG_MALFORMED (this is prob already covered by the error)
  • OTRL_MSGEVENT_LOG_HEARTBEAT_RCVD
  • OTRL_MSGEVENT_LOG_HEARTBEAT_SENT
  • OTRL_MSGEVENT_RCVDMSG_GENERAL_ERR
  • OTRL_MSGEVENT_RCVDMSG_UNENCRYPTED
  • OTRL_MSGEVENT_RCVDMSG_UNRECOGNIZED (unsure of needed, as we default everything to plaintext)
  • OTRL_MSGEVENT_RCVDMSG_FOR_OTHER_INSTANCE

What do you think?

claucece added a commit that referenced this issue Jan 11, 2019
@claucece claucece removed the discuss label Jan 14, 2019
claucece added a commit that referenced this issue Jan 14, 2019
claucece added a commit that referenced this issue Jan 14, 2019
@claucece claucece self-assigned this Jan 14, 2019
claucece added a commit that referenced this issue Jan 15, 2019
@claucece
Copy link
Member Author

Ok, I have added most of them. I need to document and check if we will support the other ones.

claucece added a commit that referenced this issue Jan 15, 2019
@claucece
Copy link
Member Author

I'll stop the work on this until we have the groups of policies in place.

@claucece claucece removed their assignment Jan 16, 2019
@claucece claucece added this to the architecture milestone Oct 20, 2019
@claucece claucece modified the milestones: architecture, April Feb 6, 2020
@claucece
Copy link
Member Author

Finished now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API architecture importance low An issue that is not very important. If we can have it, great - if not, it's OK too
Projects
None yet
Development

No branches or pull requests

2 participants