Skip to content

Commit

Permalink
Mqtt311 compliance
Browse files Browse the repository at this point in the history
* Adds outbound utf-8 validation to MQTT311
* Updates QoS 1 publish session resumption behavior to include dup flag
* Adds many additional MQTT311 compliance tests

Co-authored-by: Bret Ambrose <[email protected]>
  • Loading branch information
bretambrose and Bret Ambrose authored Aug 29, 2023
1 parent 6ee615f commit ee06ce3
Show file tree
Hide file tree
Showing 17 changed files with 944 additions and 278 deletions.
9 changes: 9 additions & 0 deletions include/aws/mqtt/mqtt.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,18 @@ AWS_EXTERN_C_BEGIN

AWS_MQTT_API
bool aws_mqtt_is_valid_topic(const struct aws_byte_cursor *topic);

AWS_MQTT_API
bool aws_mqtt_is_valid_topic_filter(const struct aws_byte_cursor *topic_filter);

/**
* Validate utf-8 string under mqtt specs
*
* @param text
* @return AWS_OP_SUCCESS if the text is validate, otherwise AWS_OP_ERR
*/
AWS_MQTT_API int aws_mqtt_validate_utf8_text(struct aws_byte_cursor text);

/**
* Initializes internal datastructures used by aws-c-mqtt.
* Must be called before using any functionality in aws-c-mqtt.
Expand Down
3 changes: 3 additions & 0 deletions include/aws/mqtt/private/packets.h
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@ int aws_mqtt_packet_publish_encode_headers(struct aws_byte_buf *buf, const struc
AWS_MQTT_API
int aws_mqtt_packet_publish_decode(struct aws_byte_cursor *cur, struct aws_mqtt_packet_publish *packet);

AWS_MQTT_API
void aws_mqtt_packet_publish_set_dup(struct aws_mqtt_packet_publish *packet);

AWS_MQTT_API
bool aws_mqtt_packet_publish_get_dup(const struct aws_mqtt_packet_publish *packet);

Expand Down
8 changes: 0 additions & 8 deletions include/aws/mqtt/private/v5/mqtt5_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,6 @@ AWS_EXTERN_C_BEGIN
*/
AWS_MQTT_API extern struct aws_byte_cursor g_aws_mqtt5_connect_protocol_cursor;

/**
* Validate utf-8 string under mqtt5 specs
*
* @param text
* @return AWS_OP_SUCCESS if the text is validate, otherwise AWS_OP_ERR
*/
AWS_MQTT_API int aws_mqtt5_validate_utf8_text(struct aws_byte_cursor text);

/**
* Simple helper function to compute the first byte of an MQTT packet encoding as a function of 4 bit flags
* and the packet type.
Expand Down
12 changes: 0 additions & 12 deletions include/aws/mqtt/v5/mqtt5_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -787,18 +787,6 @@ AWS_MQTT_API int aws_mqtt5_negotiated_settings_init(
struct aws_mqtt5_negotiated_settings *negotiated_settings,
const struct aws_byte_cursor *client_id);

/**
* Makes an owning copy of a negotiated settings structure
*
* @param source settings to copy from
* @param dest settings to copy into. Must be in a zeroed or initialized state because it gets clean up
* called on it as the first step of the copy process.
* @return success/failure
*/
AWS_MQTT_API int aws_mqtt5_negotiated_settings_copy(
const struct aws_mqtt5_negotiated_settings *source,
struct aws_mqtt5_negotiated_settings *dest);

/**
* Clean up owned memory in negotiated_settings
*
Expand Down
52 changes: 46 additions & 6 deletions source/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -942,18 +942,23 @@ static int s_aws_mqtt_client_connection_311_set_will(
return aws_raise_error(AWS_ERROR_INVALID_STATE);
}

if (!aws_mqtt_is_valid_topic(topic)) {
AWS_LOGF_ERROR(AWS_LS_MQTT_CLIENT, "id=%p: Will topic is invalid", (void *)connection);
return aws_raise_error(AWS_ERROR_MQTT_INVALID_TOPIC);
}

if (qos > AWS_MQTT_QOS_EXACTLY_ONCE) {
AWS_LOGF_ERROR(AWS_LS_MQTT_CLIENT, "id=%p: Will qos is invalid", (void *)connection);
return aws_raise_error(AWS_ERROR_MQTT_INVALID_QOS);
}

int result = AWS_OP_ERR;
AWS_LOGF_TRACE(
AWS_LS_MQTT_CLIENT,
"id=%p: Setting last will with topic \"" PRInSTR "\"",
(void *)connection,
AWS_BYTE_CURSOR_PRI(*topic));

if (!aws_mqtt_is_valid_topic(topic)) {
AWS_LOGF_ERROR(AWS_LS_MQTT_CLIENT, "id=%p: Will topic is invalid", (void *)connection);
return aws_raise_error(AWS_ERROR_MQTT_INVALID_TOPIC);
}

struct aws_byte_buf local_topic_buf;
struct aws_byte_buf local_payload_buf;
AWS_ZERO_STRUCT(local_topic_buf);
Expand Down Expand Up @@ -1007,6 +1012,12 @@ static int s_aws_mqtt_client_connection_311_set_login(
return aws_raise_error(AWS_ERROR_INVALID_STATE);
}

if (username != NULL && aws_mqtt_validate_utf8_text(*username) == AWS_OP_ERR) {
AWS_LOGF_DEBUG(
AWS_LS_MQTT_CLIENT, "id=%p: Invalid utf8 or forbidden codepoints in username", (void *)connection);
return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
}

int result = AWS_OP_ERR;
AWS_LOGF_TRACE(AWS_LS_MQTT_CLIENT, "id=%p: Setting username and password", (void *)connection);

Expand Down Expand Up @@ -1428,6 +1439,16 @@ static int s_aws_mqtt_client_connection_311_connect(

struct aws_mqtt_client_connection_311_impl *connection = impl;

if (connection_options == NULL) {
return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
}

if (aws_mqtt_validate_utf8_text(connection_options->client_id) == AWS_OP_ERR) {
AWS_LOGF_DEBUG(
AWS_LS_MQTT_CLIENT, "id=%p: Invalid utf8 or forbidden codepoints in client id", (void *)connection);
return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
}

/* TODO: Do we need to support resuming the connection if user connect to the same connection & endpoint and the
* clean_session is false?
* If not, the broker will resume the connection in this case, and we pretend we are making a new connection, which
Expand Down Expand Up @@ -1957,6 +1978,11 @@ static uint16_t s_aws_mqtt_client_connection_311_subscribe_multiple(

AWS_PRECONDITION(connection);

if (topic_filters == NULL || aws_array_list_length(topic_filters) == 0) {
aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
return 0;
}

struct subscribe_task_arg *task_arg = aws_mem_calloc(connection->allocator, 1, sizeof(struct subscribe_task_arg));
if (!task_arg) {
return 0;
Expand Down Expand Up @@ -2804,6 +2830,8 @@ static enum aws_mqtt_client_request_state s_publish_send(uint16_t packet_id, boo

return AWS_MQTT_CLIENT_REQUEST_ERROR;
}
} else {
aws_mqtt_packet_publish_set_dup(&task_arg->publish);
}

struct aws_io_message *message = mqtt_get_message_for_packet(task_arg->connection, &task_arg->publish.fixed_header);
Expand Down Expand Up @@ -2919,6 +2947,11 @@ static uint16_t s_aws_mqtt_client_connection_311_publish(
return 0;
}

if (qos > AWS_MQTT_QOS_EXACTLY_ONCE) {
aws_raise_error(AWS_ERROR_MQTT_INVALID_QOS);
return 0;
}

struct publish_task_arg *arg = aws_mem_calloc(connection->allocator, 1, sizeof(struct publish_task_arg));
if (!arg) {
return 0;
Expand All @@ -2929,7 +2962,14 @@ static uint16_t s_aws_mqtt_client_connection_311_publish(
arg->topic = aws_byte_cursor_from_string(arg->topic_string);
arg->qos = qos;
arg->retain = retain;
if (aws_byte_buf_init_copy_from_cursor(&arg->payload_buf, connection->allocator, *payload)) {

struct aws_byte_cursor payload_cursor;
AWS_ZERO_STRUCT(payload_cursor);
if (payload != NULL) {
payload_cursor = *payload;
}

if (aws_byte_buf_init_copy_from_cursor(&arg->payload_buf, connection->allocator, payload_cursor)) {
goto handle_error;
}
arg->payload = aws_byte_cursor_from_buf(&arg->payload_buf);
Expand Down
43 changes: 42 additions & 1 deletion source/mqtt.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,28 @@

#include <aws/mqtt/mqtt.h>

#include <aws/common/encoding.h>
#include <aws/http/http.h>

#include <aws/io/logging.h>

/*******************************************************************************
* Topic Validation
******************************************************************************/

static bool s_is_valid_topic(const struct aws_byte_cursor *topic, bool is_filter) {
if (topic == NULL) {
return false;
}

/* [MQTT-4.7.3-1] Check existance and length */
if (!topic->ptr || !topic->len) {
return false;
}

if (aws_mqtt_validate_utf8_text(*topic) == AWS_OP_ERR) {
return false;
}

/* [MQTT-4.7.3-2] Check for the null character */
if (memchr(topic->ptr, 0, topic->len)) {
return false;
Expand Down Expand Up @@ -286,3 +293,37 @@ void aws_mqtt_fatal_assert_library_initialized(void) {
AWS_FATAL_ASSERT(s_mqtt_library_initialized);
}
}

/* UTF-8 encoded string validation respect to [MQTT-1.5.3-2]. */
static int aws_mqtt_utf8_decoder(uint32_t codepoint, void *user_data) {
(void)user_data;
/* U+0000 - A UTF-8 Encoded String MUST NOT include an encoding of the null character U+0000. [MQTT-1.5.4-2]
* U+0001..U+001F control characters are not valid
*/
if (AWS_UNLIKELY(codepoint <= 0x001F)) {
return aws_raise_error(AWS_ERROR_MQTT5_INVALID_UTF8_STRING);
}

/* U+007F..U+009F control characters are not valid */
if (AWS_UNLIKELY((codepoint >= 0x007F) && (codepoint <= 0x009F))) {
return aws_raise_error(AWS_ERROR_MQTT5_INVALID_UTF8_STRING);
}

/* Unicode non-characters are not valid: https://www.unicode.org/faq/private_use.html#nonchar1 */
if (AWS_UNLIKELY((codepoint & 0x00FFFF) >= 0x00FFFE)) {
return aws_raise_error(AWS_ERROR_MQTT5_INVALID_UTF8_STRING);
}
if (AWS_UNLIKELY(codepoint >= 0xFDD0 && codepoint <= 0xFDEF)) {
return aws_raise_error(AWS_ERROR_MQTT5_INVALID_UTF8_STRING);
}

return AWS_ERROR_SUCCESS;
}

static struct aws_utf8_decoder_options s_aws_mqtt_utf8_decoder_options = {
.on_codepoint = aws_mqtt_utf8_decoder,
};

int aws_mqtt_validate_utf8_text(struct aws_byte_cursor text) {
return aws_decode_utf8(text, &s_aws_mqtt_utf8_decoder_options);
}
4 changes: 4 additions & 0 deletions source/packets.c
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,10 @@ bool aws_mqtt_packet_publish_get_dup(const struct aws_mqtt_packet_publish *packe
return packet->fixed_header.flags & (1 << 3); /* bit 3 */
}

void aws_mqtt_packet_publish_set_dup(struct aws_mqtt_packet_publish *packet) {
packet->fixed_header.flags |= 0x08;
}

enum aws_mqtt_qos aws_mqtt_packet_publish_get_qos(const struct aws_mqtt_packet_publish *packet) {
return (packet->fixed_header.flags >> 1) & 0x3; /* bits 2,1 */
}
Expand Down
22 changes: 11 additions & 11 deletions source/v5/mqtt5_options_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ static int s_aws_mqtt5_user_property_set_validate(
return aws_raise_error(AWS_ERROR_MQTT5_USER_PROPERTY_VALIDATION);
}

if (aws_mqtt5_validate_utf8_text(property->name)) {
if (aws_mqtt_validate_utf8_text(property->name)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL, "id=%p: %s - user property #%zu name not valid UTF8", log_context, log_prefix, i);
return aws_raise_error(AWS_ERROR_MQTT5_USER_PROPERTY_VALIDATION);
Expand All @@ -187,7 +187,7 @@ static int s_aws_mqtt5_user_property_set_validate(
property->value.len);
return aws_raise_error(AWS_ERROR_MQTT5_USER_PROPERTY_VALIDATION);
}
if (aws_mqtt5_validate_utf8_text(property->value)) {
if (aws_mqtt_validate_utf8_text(property->value)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL,
"id=%p: %s - user property #%zu value not valid UTF8",
Expand Down Expand Up @@ -332,7 +332,7 @@ int aws_mqtt5_packet_connect_view_validate(const struct aws_mqtt5_packet_connect
return aws_raise_error(AWS_ERROR_MQTT5_CONNECT_OPTIONS_VALIDATION);
}

if (aws_mqtt5_validate_utf8_text(connect_options->client_id)) {
if (aws_mqtt_validate_utf8_text(connect_options->client_id)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL,
"id=%p: aws_mqtt5_packet_connect_view - client id not valid UTF-8",
Expand All @@ -349,7 +349,7 @@ int aws_mqtt5_packet_connect_view_validate(const struct aws_mqtt5_packet_connect
return aws_raise_error(AWS_ERROR_MQTT5_CONNECT_OPTIONS_VALIDATION);
}

if (aws_mqtt5_validate_utf8_text(*connect_options->username)) {
if (aws_mqtt_validate_utf8_text(*connect_options->username)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL,
"id=%p: aws_mqtt5_packet_connect_view - username not valid UTF-8",
Expand Down Expand Up @@ -1259,7 +1259,7 @@ int aws_mqtt5_packet_disconnect_view_validate(const struct aws_mqtt5_packet_disc
return aws_raise_error(AWS_ERROR_MQTT5_DISCONNECT_OPTIONS_VALIDATION);
}

if (aws_mqtt5_validate_utf8_text(*disconnect_view->reason_string)) {
if (aws_mqtt_validate_utf8_text(*disconnect_view->reason_string)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL,
"id=%p: aws_mqtt5_packet_disconnect_view - reason string not valid UTF-8",
Expand Down Expand Up @@ -1591,7 +1591,7 @@ int aws_mqtt5_packet_publish_view_validate(const struct aws_mqtt5_packet_publish
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL, "id=%p: aws_mqtt5_packet_publish_view - missing topic", (void *)publish_view);
return aws_raise_error(AWS_ERROR_MQTT5_PUBLISH_OPTIONS_VALIDATION);
} else if (aws_mqtt5_validate_utf8_text(publish_view->topic)) {
} else if (aws_mqtt_validate_utf8_text(publish_view->topic)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL, "id=%p: aws_mqtt5_packet_publish_view - topic not valid UTF-8", (void *)publish_view);
return aws_raise_error(AWS_ERROR_MQTT5_PUBLISH_OPTIONS_VALIDATION);
Expand Down Expand Up @@ -1626,7 +1626,7 @@ int aws_mqtt5_packet_publish_view_validate(const struct aws_mqtt5_packet_publish

// Make sure the payload data is UTF-8 if the payload_format set to UTF8
if (*publish_view->payload_format == AWS_MQTT5_PFI_UTF8) {
if (aws_mqtt5_validate_utf8_text(publish_view->payload)) {
if (aws_mqtt_validate_utf8_text(publish_view->payload)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL,
"id=%p: aws_mqtt5_packet_publish_view - payload value is not valid UTF-8 while payload format "
Expand All @@ -1646,7 +1646,7 @@ int aws_mqtt5_packet_publish_view_validate(const struct aws_mqtt5_packet_publish
return aws_raise_error(AWS_ERROR_MQTT5_PUBLISH_OPTIONS_VALIDATION);
}

if (aws_mqtt5_validate_utf8_text(*publish_view->response_topic)) {
if (aws_mqtt_validate_utf8_text(*publish_view->response_topic)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL,
"id=%p: aws_mqtt5_packet_publish_view - response topic not valid UTF-8",
Expand Down Expand Up @@ -1692,7 +1692,7 @@ int aws_mqtt5_packet_publish_view_validate(const struct aws_mqtt5_packet_publish
return aws_raise_error(AWS_ERROR_MQTT5_PUBLISH_OPTIONS_VALIDATION);
}

if (aws_mqtt5_validate_utf8_text(*publish_view->content_type)) {
if (aws_mqtt_validate_utf8_text(*publish_view->content_type)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL,
"id=%p: aws_mqtt5_packet_publish_view - content type not valid UTF-8",
Expand Down Expand Up @@ -2332,7 +2332,7 @@ int aws_mqtt5_packet_unsubscribe_view_validate(const struct aws_mqtt5_packet_uns

for (size_t i = 0; i < unsubscribe_view->topic_filter_count; ++i) {
const struct aws_byte_cursor *topic_filter = &unsubscribe_view->topic_filters[i];
if (aws_mqtt5_validate_utf8_text(*topic_filter)) {
if (aws_mqtt_validate_utf8_text(*topic_filter)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL,
"id=%p: aws_mqtt5_packet_unsubscribe_view - topic filter not valid UTF-8: \"" PRInSTR "\"",
Expand Down Expand Up @@ -2603,7 +2603,7 @@ static int s_aws_mqtt5_validate_subscription(
const struct aws_mqtt5_subscription_view *subscription,
void *log_context) {

if (aws_mqtt5_validate_utf8_text(subscription->topic_filter)) {
if (aws_mqtt_validate_utf8_text(subscription->topic_filter)) {
AWS_LOGF_ERROR(
AWS_LS_MQTT5_GENERAL,
"id=%p: aws_mqtt5_packet_subscribe_view - topic filter \"" PRInSTR "\" not valid UTF-8 in subscription",
Expand Down
Loading

0 comments on commit ee06ce3

Please sign in to comment.