diff options
author | Alexandre Erwin Ittner <alexandre@ittner.com.br> | 2014-09-21 11:37:49 -0300 |
---|---|---|
committer | Alexandre Erwin Ittner <alexandre@ittner.com.br> | 2014-09-21 11:37:49 -0300 |
commit | b52eb48c15a06fe198c7c5e0d765cfef8ff4fbdf (patch) | |
tree | b203648e6df5bd825461e8c09b45a10f874d55ae | |
parent | 70b4018069c0bbfa7ac2cf907a76a693e78e2947 (diff) |
Remove chattiness from avatar data transfers
The chatty approach for the avatar data transfer was intended as a
security feature to add explicit delays to the transfer and prevent
amplification attacks among authenticated friends. This was deemed
unnecessary in the code review and, therefore, replaced by a simpler
approach that sends all data in a single burst.
-rw-r--r-- | docs/Avatars.md | 81 | ||||
-rw-r--r-- | toxcore/Messenger.c | 133 | ||||
-rw-r--r-- | toxcore/Messenger.h | 5 |
3 files changed, 85 insertions, 134 deletions
diff --git a/docs/Avatars.md b/docs/Avatars.md index f0d7a1eb..97b65c10 100644 --- a/docs/Avatars.md +++ b/docs/Avatars.md | |||
@@ -474,19 +474,15 @@ types. | |||
474 | - Packet `PACKET_ID_AVATAR_DATA_CONTROL` have the format: | 474 | - Packet `PACKET_ID_AVATAR_DATA_CONTROL` have the format: |
475 | 475 | ||
476 | PACKET_ID_AVATAR_DATA_CONTROL (54) | 476 | PACKET_ID_AVATAR_DATA_CONTROL (54) |
477 | Packet data size: 5 bytes | 477 | Packet data size: 1 byte |
478 | [1: uint8_t op][1: uint32_t bytes_received] | 478 | [1: uint8_t op] |
479 | 479 | ||
480 | where 'op' is a code signaling both an operation request or a status | 480 | where 'op' is a code signaling both an operation request or a status |
481 | return, which semantics are explained bellow. The following values are | 481 | return, which semantics are explained bellow. The following values are |
482 | defined: | 482 | defined: |
483 | 483 | ||
484 | 0 = AVATARDATACONTROL_REQ | 484 | 0 = AVATARDATACONTROL_REQ |
485 | 1 = AVATARDATACONTROL_MORE | 485 | 1 = AVATARDATACONTROL_ERROR |
486 | 2 = AVATARDATACONTROL_ERROR | ||
487 | |||
488 | and 'bytes_received' is the number of bytes already received by the | ||
489 | client when the operation is `MORE` or zero otherwise. | ||
490 | 486 | ||
491 | 487 | ||
492 | - Packet `PACKET_ID_AVATAR_DATA_START` have the following format: | 488 | - Packet `PACKET_ID_AVATAR_DATA_START` have the following format: |
@@ -503,7 +499,9 @@ types. | |||
503 | 499 | ||
504 | 500 | ||
505 | - Packet `PACKET_ID_AVATAR_DATA_PUSH` have no format structure, just up | 501 | - Packet `PACKET_ID_AVATAR_DATA_PUSH` have no format structure, just up |
506 | to `AVATAR_DATA_MAX_CHUNK_SIZE` (56) bytes of raw avatar image data. | 502 | to `AVATAR_DATA_MAX_CHUNK_SIZE` bytes of raw avatar image data; this |
503 | value is defined according to the maximum amount of data a Tox crypted | ||
504 | packet can hold. | ||
507 | 505 | ||
508 | 506 | ||
509 | 507 | ||
@@ -513,8 +511,7 @@ from a client "B": | |||
513 | - "A" must initialize its control structures and mark its data transfer | 511 | - "A" must initialize its control structures and mark its data transfer |
514 | as not yet started. Then it requests avatar data from "B" by sending a | 512 | as not yet started. Then it requests avatar data from "B" by sending a |
515 | packet `PACKET_ID_AVATAR_DATA_CONTROL` with 'op' set to | 513 | packet `PACKET_ID_AVATAR_DATA_CONTROL` with 'op' set to |
516 | `AVATARDATACONTROL_REQ`. The field 'bytes_received' must be present, but | 514 | `AVATARDATACONTROL_REQ`. |
517 | should be set to zero and is ignored in this step. | ||
518 | 515 | ||
519 | - If "B" accepts this transfer, it answers by sending an | 516 | - If "B" accepts this transfer, it answers by sending an |
520 | `PACKET_ID_AVATAR_DATA_START` with the fields 'format', 'hash' and | 517 | `PACKET_ID_AVATAR_DATA_START` with the fields 'format', 'hash' and |
@@ -527,42 +524,21 @@ from a client "B": | |||
527 | `AVATARDATACONTROL_ERROR` or simply ignore this request. "A" must cope | 524 | `AVATARDATACONTROL_ERROR` or simply ignore this request. "A" must cope |
528 | with this. | 525 | with this. |
529 | 526 | ||
527 | If "B" have an avatar, it sends a variable number of | ||
528 | `PACKET_ID_AVATAR_DATA_PUSH` packets with the avatar data in a single | ||
529 | shot. | ||
530 | |||
530 | - Upon receiving a `PACKET_ID_AVATAR_DATA_START`, "A" checks if it | 531 | - Upon receiving a `PACKET_ID_AVATAR_DATA_START`, "A" checks if it |
531 | has sent a data request to "B". If not, just ignores the packet. | 532 | has sent a data request to "B". If not, just ignores the packet. |
532 | 533 | ||
533 | If "A" really sent a request to "B", checks if the request was already | 534 | If "A" really requested avatar data and the format is `AVATARFORMAT_NONE`, |
534 | started. If true, it is an error and it just ignores the request. | 535 | it triggers the avatar data callback, and clears all the temporary data, |
535 | 536 | finishing the process. For other formats, "A" just waits for packets | |
536 | Otherwise, "A" decodes the message data and checks if the avatar data | 537 | of type `PACKET_ID_AVATAR_DATA_PUSH`. |
537 | length stated in the field 'data_length' is acceptable (ie. less or | ||
538 | equal than `TOX_MAX_AVATAR_DATA_LENGTH`). If not, it replies with an | ||
539 | `PACKET_ID_AVATAR_DATA_CONTROL` with the field 'op' set to | ||
540 | `AVATARDATACONTROL_ERROR` (or just ignores this, "A" holds no state). | ||
541 | |||
542 | If the size is acceptable, "A" marks the request as stated, stores the | ||
543 | format, hash, and data length in the local state for user "B", sets a | ||
544 | counter for the number of bytes received from the peer and replies with | ||
545 | a `PACKET_ID_AVATAR_DATA_CONTROL` with the field 'op' set to | ||
546 | `AVATARDATACONTROL_MORE` and 'bytes_received' set to zero (as no data | ||
547 | was received yet). | ||
548 | |||
549 | - Upon receiving a `PACKET_ID_AVATAR_DATA_CONTROL` with op | ||
550 | `AVATARDATACONTROL_MORE`, "B" sends an `PACKET_ID_AVATAR_DATA_PUSH` | ||
551 | with up to `AVATAR_DATA_MAX_CHUNK_SIZE` bytes of data from the avatar, | ||
552 | starting from the offset stated in the field 'bytes_received'. | ||
553 | 538 | ||
554 | If the requested offset is invalid, "B" replies with a | 539 | - Upon receiving a `PACKET_ID_AVATAR_DATA_PUSH`, "A" checks if it really |
555 | `PACKET_ID_AVATAR_DATA_CONTROL` with the field 'op' set to | 540 | sent an avatar data request and if the `PACKET_ID_AVATAR_DATA_START` was |
556 | `AVATARDATACONTROL_ERROR`. | 541 | already received. If this conditions are valid, it checks if the total |
557 | |||
558 | "B" must have full control of the amount of data it sends to "A" and | ||
559 | may, at any time, abort the transfer by sending a | ||
560 | `PACKET_ID_AVATAR_DATA_CONTROL` with the field 'op' set to | ||
561 | `AVATARDATACONTROL_ERROR`. This may happens, for example, if some limit | ||
562 | was hit or a network data usage throttle enabled. A rationale for this | ||
563 | procedures is available in section "Security considerations". | ||
564 | |||
565 | - Upon receiving a `PACKET_ID_AVATAR_DATA_PUSH`, "A" checks if the total | ||
566 | length of the data already stored in the receiving buffer plus the data | 542 | length of the data already stored in the receiving buffer plus the data |
567 | present in the push packet is still less or equal than | 543 | present in the push packet is still less or equal than |
568 | `TOX_MAX_AVATAR_DATA_LENGTH`. If invalid, it replies with a | 544 | `TOX_MAX_AVATAR_DATA_LENGTH`. If invalid, it replies with a |
@@ -582,22 +558,13 @@ from a client "B": | |||
582 | triggers the avatar data callback, and clears all the temporary data, | 558 | triggers the avatar data callback, and clears all the temporary data, |
583 | finishing the process. | 559 | finishing the process. |
584 | 560 | ||
585 | If not all data was received, "A" requests more data by sending a | 561 | If not all data was received, "A" simply waits for more data. |
586 | `PACKET_ID_AVATAR_DATA_CONTROL` with the field 'op' set to | ||
587 | `AVATARDATACONTROL_MORE` and 'bytes_received' set to the new offset. | ||
588 | 562 | ||
589 | Client "A" is always responsible for controlling the transfer and | 563 | Client "A" is always responsible for controlling the transfer and |
590 | validating the data received. "B" don't need to keep any state for the | 564 | validating the data received. "B" don't need to keep any state for the |
591 | protocol, have full control over the data sent and should implement | 565 | protocol, have full control over the data sent and should implement |
592 | some transfer limit for the data it sends. | 566 | some transfer limit for the data it sends. |
593 | 567 | ||
594 | This "chatty" protocol mitigates a potential amplification attack, | ||
595 | i.e., a malicious friend sending a very small data packet that causes | ||
596 | another user to send a larger amount of data. The hash validation | ||
597 | ensures that the avatar data is correct even if "B" changed its avatar | ||
598 | data in the middle of the transfer. A rationale for this procedures is | ||
599 | available in section "Security considerations". | ||
600 | |||
601 | - Any peer receiving a `PACKET_ID_AVATAR_DATA_CONTROL` with the field 'op' | 568 | - Any peer receiving a `PACKET_ID_AVATAR_DATA_CONTROL` with the field 'op' |
602 | set to `AVATARDATACONTROL_ERROR` clears any existing control state and | 569 | set to `AVATARDATACONTROL_ERROR` clears any existing control state and |
603 | finishes sending or receiving data. | 570 | finishes sending or receiving data. |
@@ -624,12 +591,6 @@ The present proposal mitigates this situation by: | |||
624 | - Providing an alternate, smaller, message to cooperative users refresh | 591 | - Providing an alternate, smaller, message to cooperative users refresh |
625 | avatar information when nothing has changed (`PACKET_ID_AVATAR_INFO`); | 592 | avatar information when nothing has changed (`PACKET_ID_AVATAR_INFO`); |
626 | 593 | ||
627 | - Making the avatar data transfer chatty: The user requesting avatar data | ||
628 | can not force a peer to send large amounts of data in a single shot and | ||
629 | must request new chunks as needed. The sender will never send more that | ||
630 | 1 kB of data in a single push and have ultimate control over the amount | ||
631 | of data sent in a chunk; | ||
632 | |||
633 | - Having per-friend data transfer limit. As the current protocol still | 594 | - Having per-friend data transfer limit. As the current protocol still |
634 | allows an user to request an infinite data stream by asking the the | 595 | allows an user to request an infinite data stream by asking the the |
635 | same offset of the avatar again and again, the implementation limits | 596 | same offset of the avatar again and again, the implementation limits |
@@ -640,10 +601,6 @@ The present proposal mitigates this situation by: | |||
640 | - Making the requester responsible for storing partial data and state | 601 | - Making the requester responsible for storing partial data and state |
641 | information; | 602 | information; |
642 | 603 | ||
643 | - (currently not implemented) Treating avatar data transfers as a low | ||
644 | priority operation, handled only if no other packets are being | ||
645 | processed. | ||
646 | |||
647 | Another problem present in the avatars is the possibility of a friend send | 604 | Another problem present in the avatars is the possibility of a friend send |
648 | a maliciously crafted image intended to exploit vulnerabilities in image | 605 | a maliciously crafted image intended to exploit vulnerabilities in image |
649 | decoders. Without an intermediate server to recompress and validate and | 606 | decoders. Without an intermediate server to recompress and validate and |
diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 4f8eee6b..fb6aafa5 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c | |||
@@ -42,7 +42,7 @@ | |||
42 | static void set_friend_status(Messenger *m, int32_t friendnumber, uint8_t status); | 42 | static void set_friend_status(Messenger *m, int32_t friendnumber, uint8_t status); |
43 | static int write_cryptpacket_id(const Messenger *m, int32_t friendnumber, uint8_t packet_id, const uint8_t *data, | 43 | static int write_cryptpacket_id(const Messenger *m, int32_t friendnumber, uint8_t packet_id, const uint8_t *data, |
44 | uint32_t length, uint8_t congestion_control); | 44 | uint32_t length, uint8_t congestion_control); |
45 | static int send_avatar_data_control(const Messenger *m, const uint32_t friendnumber, uint8_t op, uint32_t bytes_received); | 45 | static int send_avatar_data_control(const Messenger *m, const uint32_t friendnumber, uint8_t op); |
46 | 46 | ||
47 | // friend_not_valid determines if the friendnumber passed is valid in the Messenger object | 47 | // friend_not_valid determines if the friendnumber passed is valid in the Messenger object |
48 | static uint8_t friend_not_valid(const Messenger *m, int32_t friendnumber) | 48 | static uint8_t friend_not_valid(const Messenger *m, int32_t friendnumber) |
@@ -672,6 +672,8 @@ int m_request_avatar_data(const Messenger *m, const int32_t friendnumber) | |||
672 | avrd = malloc(sizeof(AVATARRECEIVEDATA)); | 672 | avrd = malloc(sizeof(AVATARRECEIVEDATA)); |
673 | if (avrd == NULL) | 673 | if (avrd == NULL) |
674 | return -1; | 674 | return -1; |
675 | memset(avrd, 0, sizeof(AVATARRECEIVEDATA)); | ||
676 | avrd->started = 0; | ||
675 | m->friendlist[friendnumber].avatar_recv_data = avrd; | 677 | m->friendlist[friendnumber].avatar_recv_data = avrd; |
676 | } | 678 | } |
677 | 679 | ||
@@ -680,13 +682,12 @@ int m_request_avatar_data(const Messenger *m, const int32_t friendnumber) | |||
680 | "friendnumber == %u", friendnumber); | 682 | "friendnumber == %u", friendnumber); |
681 | } | 683 | } |
682 | 684 | ||
683 | memset(avrd, 0, sizeof(AVATARRECEIVEDATA)); | ||
684 | avrd->started = 0; | 685 | avrd->started = 0; |
685 | avrd->bytes_received = 0; | 686 | avrd->bytes_received = 0; |
686 | avrd->total_length = 0; | 687 | avrd->total_length = 0; |
687 | avrd->format = AVATARFORMAT_NONE; | 688 | avrd->format = AVATARFORMAT_NONE; |
688 | 689 | ||
689 | return send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_REQ, 0); | 690 | return send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_REQ); |
690 | } | 691 | } |
691 | 692 | ||
692 | 693 | ||
@@ -2118,18 +2119,12 @@ static int handle_status(void *object, int i, uint8_t status) | |||
2118 | * values or request data. | 2119 | * values or request data. |
2119 | */ | 2120 | */ |
2120 | static int send_avatar_data_control(const Messenger *m, const uint32_t friendnumber, | 2121 | static int send_avatar_data_control(const Messenger *m, const uint32_t friendnumber, |
2121 | uint8_t op, uint32_t bytes_received) | 2122 | uint8_t op) |
2122 | { | 2123 | { |
2123 | uint8_t data[1 + sizeof(uint32_t)]; | ||
2124 | data[0] = op; | ||
2125 | uint32_t tmp_recv = htonl(bytes_received); | ||
2126 | memcpy(data+1, &tmp_recv, sizeof(uint32_t)); | ||
2127 | |||
2128 | int ret = write_cryptpacket_id(m, friendnumber, PACKET_ID_AVATAR_DATA_CONTROL, | 2124 | int ret = write_cryptpacket_id(m, friendnumber, PACKET_ID_AVATAR_DATA_CONTROL, |
2129 | data, sizeof(data), 0); | 2125 | &op, sizeof(op), 0); |
2130 | 2126 | LOGGER_DEBUG("friendnumber = %u, op = %u, ret = %d", | |
2131 | LOGGER_DEBUG("friendnumber = %u, op = %u, bytes_received = %u, ret = %d", | 2127 | friendnumber, op, ret); |
2132 | friendnumber, op, bytes_received, ret); | ||
2133 | return (ret >= 0) ? 0 : -1; | 2128 | return (ret >= 0) ? 0 : -1; |
2134 | } | 2129 | } |
2135 | 2130 | ||
@@ -2137,11 +2132,11 @@ static int send_avatar_data_control(const Messenger *m, const uint32_t friendnum | |||
2137 | static int handle_avatar_data_control(Messenger *m, uint32_t friendnumber, | 2132 | static int handle_avatar_data_control(Messenger *m, uint32_t friendnumber, |
2138 | uint8_t *data, uint32_t data_length) | 2133 | uint8_t *data, uint32_t data_length) |
2139 | { | 2134 | { |
2140 | if (data_length != 1 + sizeof(uint32_t)) { | 2135 | if (data_length != 1) { |
2141 | LOGGER_DEBUG("Error: PACKET_ID_AVATAR_DATA_CONTROL with bad " | 2136 | LOGGER_DEBUG("Error: PACKET_ID_AVATAR_DATA_CONTROL with bad " |
2142 | "data_length = %u, friendnumber = %u", | 2137 | "data_length = %u, friendnumber = %u", |
2143 | data_length, friendnumber); | 2138 | data_length, friendnumber); |
2144 | send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_ERROR, 0); | 2139 | send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_ERROR); |
2145 | return -1; /* Error */ | 2140 | return -1; /* Error */ |
2146 | } | 2141 | } |
2147 | 2142 | ||
@@ -2149,6 +2144,25 @@ static int handle_avatar_data_control(Messenger *m, uint32_t friendnumber, | |||
2149 | 2144 | ||
2150 | switch (data[0]) { | 2145 | switch (data[0]) { |
2151 | case AVATARDATACONTROL_REQ: { | 2146 | case AVATARDATACONTROL_REQ: { |
2147 | |||
2148 | /* Check data transfer limits for this friend */ | ||
2149 | AVATARSENDDATA * const avsd = &(m->friendlist[friendnumber].avatar_send_data); | ||
2150 | if (avsd->bytes_sent >= AVATAR_DATA_TRANSFER_LIMIT) { | ||
2151 | /* User reached data limit. Check timeout */ | ||
2152 | uint64_t now = unix_time(); | ||
2153 | if (avsd->last_reset > 0 | ||
2154 | && (avsd->last_reset + AVATAR_DATA_TRANSFER_TIMEOUT < now)) { | ||
2155 | avsd->bytes_sent = 0; | ||
2156 | avsd->last_reset = now; | ||
2157 | } else { | ||
2158 | /* Friend still rate-limitted. Send an error and stops. */ | ||
2159 | LOGGER_DEBUG("Avatar data transfer limit reached. " | ||
2160 | "friendnumber = %u", friendnumber); | ||
2161 | send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_ERROR); | ||
2162 | return 0; | ||
2163 | } | ||
2164 | } | ||
2165 | |||
2152 | /* Start the transmission with a DATA_START message. Format: | 2166 | /* Start the transmission with a DATA_START message. Format: |
2153 | * uint8_t format | 2167 | * uint8_t format |
2154 | * uint8_t hash[AVATAR_HASH_LENGTH] | 2168 | * uint8_t hash[AVATAR_HASH_LENGTH] |
@@ -2164,64 +2178,46 @@ static int handle_avatar_data_control(Messenger *m, uint32_t friendnumber, | |||
2164 | memcpy(start_data + 1, m->avatar_hash, AVATAR_HASH_LENGTH); | 2178 | memcpy(start_data + 1, m->avatar_hash, AVATAR_HASH_LENGTH); |
2165 | memcpy(start_data + 1 + AVATAR_HASH_LENGTH, &avatar_len, sizeof(uint32_t)); | 2179 | memcpy(start_data + 1 + AVATAR_HASH_LENGTH, &avatar_len, sizeof(uint32_t)); |
2166 | 2180 | ||
2181 | avsd->bytes_sent += sizeof(start_data); /* For rate limit */ | ||
2182 | |||
2167 | int ret = write_cryptpacket_id(m, friendnumber, PACKET_ID_AVATAR_DATA_START, | 2183 | int ret = write_cryptpacket_id(m, friendnumber, PACKET_ID_AVATAR_DATA_START, |
2168 | start_data, sizeof(start_data), 0); | 2184 | start_data, sizeof(start_data), 0); |
2169 | if (ret >= 0) | ||
2170 | return 0; /* Ok */ | ||
2171 | 2185 | ||
2172 | return -1; | 2186 | if (ret < 0) { |
2173 | } | 2187 | /* Something went wrong, try to signal the error so the friend |
2174 | 2188 | * can clear up the state. */ | |
2175 | case AVATARDATACONTROL_MORE: { | 2189 | send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_ERROR); |
2176 | /* Format: [uint8_t op][uint32_t position] | ||
2177 | * Friend received a chunk of data and asked for more. | ||
2178 | */ | ||
2179 | uint32_t bytes_received; | ||
2180 | memcpy(&bytes_received, data+1, sizeof(uint32_t)); | ||
2181 | bytes_received = ntohl(bytes_received); | ||
2182 | |||
2183 | if (m->avatar_format == AVATARFORMAT_NONE | ||
2184 | || m->avatar_data == NULL | ||
2185 | || m->avatar_data_length == 0 | ||
2186 | || m->avatar_data_length < bytes_received) { | ||
2187 | /* Avatar changed, or position became invalid, or we | ||
2188 | * received a incorrect or malicious request. Abort. */ | ||
2189 | LOGGER_DEBUG("Aborting. AVATARDATACONTROL_MORE with bad " | ||
2190 | "offset. bytes_received = %u, friendnumber = %u", | ||
2191 | bytes_received, friendnumber); | ||
2192 | send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_ERROR, 0); | ||
2193 | return 0; | 2190 | return 0; |
2194 | } | 2191 | } |
2195 | 2192 | ||
2196 | /* Check data transfer limits for this friend */ | 2193 | /* User have no avatar data, nothing more to do. */ |
2197 | AVATARSENDDATA * const avsd = &(m->friendlist[friendnumber].avatar_send_data); | 2194 | if (m->avatar_format == AVATARFORMAT_NONE) |
2198 | if (avsd->bytes_sent >= AVATAR_DATA_TRANSFER_LIMIT) { | 2195 | return 0; |
2199 | /* User reached data limit. Check timeout */ | 2196 | |
2200 | uint64_t now = unix_time(); | 2197 | /* Send the actual avatar data. */ |
2201 | if (avsd->last_reset > 0 | 2198 | uint32_t offset = 0; |
2202 | && (avsd->last_reset + AVATAR_DATA_TRANSFER_TIMEOUT < now)) { | 2199 | while (offset < m->avatar_data_length) { |
2203 | avsd->bytes_sent = 0; | 2200 | uint32_t chunk_len = m->avatar_data_length - offset; |
2204 | avsd->last_reset = now; | 2201 | if (chunk_len > AVATAR_DATA_MAX_CHUNK_SIZE) |
2205 | } else { | 2202 | chunk_len = AVATAR_DATA_MAX_CHUNK_SIZE; |
2206 | /* Friend still rate-limitted. Send an error ad stops. */ | 2203 | |
2207 | LOGGER_DEBUG("Avatar data transfer limit reached. " | 2204 | uint8_t chunk[AVATAR_DATA_MAX_CHUNK_SIZE]; |
2208 | "friendnumber = %u", friendnumber); | 2205 | memcpy(chunk, m->avatar_data + offset, chunk_len); |
2209 | send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_ERROR, 0); | 2206 | offset += chunk_len; |
2210 | return 0; | 2207 | avsd->bytes_sent += chunk_len; /* For rate limit */ |
2208 | |||
2209 | int ret = write_cryptpacket_id(m, friendnumber, | ||
2210 | PACKET_ID_AVATAR_DATA_PUSH, | ||
2211 | chunk, chunk_len, 0); | ||
2212 | if (ret < 0) { | ||
2213 | LOGGER_DEBUG("write_cryptpacket_id failed. ret = %d, " | ||
2214 | "friendnumber = %u, offset = %u", | ||
2215 | ret, friendnumber, offset); | ||
2216 | send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_ERROR); | ||
2217 | return -1; | ||
2211 | } | 2218 | } |
2212 | } | 2219 | } |
2213 | 2220 | ||
2214 | /* Send another chunk of data */ | ||
2215 | uint32_t chunk_len = m->avatar_data_length - bytes_received; | ||
2216 | if (chunk_len > AVATAR_DATA_MAX_CHUNK_SIZE) | ||
2217 | chunk_len = AVATAR_DATA_MAX_CHUNK_SIZE; | ||
2218 | |||
2219 | uint8_t chunk[AVATAR_DATA_MAX_CHUNK_SIZE]; | ||
2220 | memcpy(chunk, m->avatar_data + bytes_received, chunk_len); | ||
2221 | avsd->bytes_sent += chunk_len; /* For rate limit */ | ||
2222 | |||
2223 | write_cryptpacket_id(m, friendnumber, PACKET_ID_AVATAR_DATA_PUSH, | ||
2224 | chunk, chunk_len, 0); | ||
2225 | return 0; | 2221 | return 0; |
2226 | } | 2222 | } |
2227 | 2223 | ||
@@ -2305,8 +2301,8 @@ static int handle_avatar_data_start(Messenger *m, uint32_t friendnumber, | |||
2305 | return 0; | 2301 | return 0; |
2306 | } | 2302 | } |
2307 | 2303 | ||
2308 | LOGGER_DEBUG("requesting more data, friendnumber = %u", friendnumber); | 2304 | /* Waits for more data to be received */ |
2309 | return send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_MORE, 0); | 2305 | return 0; |
2310 | } | 2306 | } |
2311 | 2307 | ||
2312 | 2308 | ||
@@ -2371,9 +2367,8 @@ static int handle_avatar_data_push(Messenger *m, uint32_t friendnumber, | |||
2371 | return 0; | 2367 | return 0; |
2372 | } | 2368 | } |
2373 | 2369 | ||
2374 | /* Request more data */ | 2370 | /* Waits for more data to be received */ |
2375 | return send_avatar_data_control(m, friendnumber, AVATARDATACONTROL_MORE, | 2371 | return 0; |
2376 | avrd->bytes_received); | ||
2377 | } | 2372 | } |
2378 | 2373 | ||
2379 | 2374 | ||
diff --git a/toxcore/Messenger.h b/toxcore/Messenger.h index 499964a7..ee8aaa5f 100644 --- a/toxcore/Messenger.h +++ b/toxcore/Messenger.h | |||
@@ -117,8 +117,8 @@ enum { | |||
117 | /* If no packets are received from friend in this time interval, kill the connection. */ | 117 | /* If no packets are received from friend in this time interval, kill the connection. */ |
118 | #define FRIEND_CONNECTION_TIMEOUT (FRIEND_PING_INTERVAL * 3) | 118 | #define FRIEND_CONNECTION_TIMEOUT (FRIEND_PING_INTERVAL * 3) |
119 | 119 | ||
120 | /* Must be <= MAX_CRYPTO_DATA_SIZE */ | 120 | /* Must be < MAX_CRYPTO_DATA_SIZE */ |
121 | #define AVATAR_DATA_MAX_CHUNK_SIZE 1024 | 121 | #define AVATAR_DATA_MAX_CHUNK_SIZE (MAX_CRYPTO_DATA_SIZE-1) |
122 | 122 | ||
123 | /* Per-friend data limit for avatar data requests */ | 123 | /* Per-friend data limit for avatar data requests */ |
124 | #define AVATAR_DATA_TRANSFER_LIMIT (10*MAX_AVATAR_DATA_LENGTH) | 124 | #define AVATAR_DATA_TRANSFER_LIMIT (10*MAX_AVATAR_DATA_LENGTH) |
@@ -151,7 +151,6 @@ AVATARFORMAT; | |||
151 | */ | 151 | */ |
152 | typedef enum { | 152 | typedef enum { |
153 | AVATARDATACONTROL_REQ, | 153 | AVATARDATACONTROL_REQ, |
154 | AVATARDATACONTROL_MORE, | ||
155 | AVATARDATACONTROL_ERROR | 154 | AVATARDATACONTROL_ERROR |
156 | } | 155 | } |
157 | AVATARDATACONTROL; | 156 | AVATARDATACONTROL; |