[OTR-dev] Freeze, .NET, RPC, and other weirdness

[chris.tuchs at hushmail.com]

On Sun, 20 Dec 2009 06:55:22 -0800 Ian Goldberg 
<ian at cypherpunks.ca> wrote:
>On Fri, Dec 18, 2009 at 10:21:20AM -0800, chris.tuchs at hushmail.com 
>wrote:
>> [...] just "Emerald Viewer - a Second Life Client" [...]
>Done.
Thanks!

>> 2: I still have sitting around as a patch the minor change I
>> made to the fragmentation code to support Second Life's
>> surprising out of order delivery of IM.  The format, syntax and
>> semantics of the fragments remain the same, I just changed the
>> logic in the fragment reassembly to better handle out of order
>> delivery.  I would be happy to submit the patch, let me know.

>The semantics can't be *quite* the same, right?  Since there's no
>ID number in the fragments (unnecessary if the packets are
>delivered in order, possibly with drops), how do you know if:
>
>1/2 1/2 2/2 2/2 
>
>is A B A B or A B B A?  Current OTR would treat it as:
>
>A B(dropping the partial A) B ignore


My patch would interpret this as 1/2A (missing 2/2A) 1/2B 2/2B
(missing 1/2C) 2/2C.  There is of course no way to know without
adding something like message ID's.  In this example my code ends
up assembling the same fragments that the existing OTR does, and
holding onto the final fragment unlike the existing OTR.  The
place where mine "does better" is 2/2A 1/2A where it correctly
assembles A, and the existing OTR code throws away the second half
of A, keeps the first half of A, and waits until another message
arrives.

>I guess "message ids in fragments" should be in the next version
>of the wire protocol.  But that opens up a potential DoS attack
>where you send packets with holes in them, forcing the other side
>to keep state around (forever?).  When does your patch decide
>that it's time to expire old partially completed messages?

My code has an additional vulnerability not in the existing OTR.
If my code sees 1/99999, 1/99999, 1/99999 it will free, malloc,
initialize, free, malloc, initialize ... the same approximately
half meg of memory.  When it sees x/Y it allocates a array of Y
char*'s, sets all to null, then set's [x] to point at the message
fragment just received.  There are no timers, and my code make
decisions when it sees fragments.  As soon as it sees x/Y it
decides: if Y is the same, and x is a duplicate; or if Y is
different from last time, throw away the old message, and start
assembling a new one.  After adding [x], check the count of
received fragments, if it is Y we just got the last fragment,
whatever number x happens to be.

Something like message ID's is probably a good idea, though I
would do something more like TCP.  Message ID's would force use to
deal with 1/99999id1, 1/99999id2, 1/99999id3... as a DOS type
attack.  I think looking at TCP's sliding window and not using
message ID's but rather byte numbers would be the way to go.  Each
fragment would ACK the last byte correctly received from the other
end, and indicate the offset of the first byte of this message.

We could define a well known buffer size all OTR's implement,
something reasonable like 32K maybe, and solve both my new
vulnerability, and the existing vulnerability of having to
assemble huge but undecipherable messages.

But it really bugs me to be adding (parts of) TCP in the guts of a
cryptographic protocol of an IM client.  What is the least we can
get away with?  Lost messages and out of order messages are not
uncommon in Second Life.  I may have seen duplicate messages, it's
hard to tell, but mangled messages I have never seen.  I don't
know the character of the many other protocols OTR has to live
with, though I will probably look at IRC soon.

This is a slightly edited excerpt from the diff I have laying
around.  This is just the code that decides what to do with a
fragment k/n.

if (k > 0 && n > 0 && k <= n && start > 0 && end > 0 && start < end)
{
    k--;
    if (!context->fragments)
    {
        /* starting a new fragmented message */
        err = otrl_malloc_fragments(context, n);
        if (err) return OTRL_FRAGMENT_INCOMPLETE; /* $TODO$ */
    }
    else if (n != context->fragment_n)
    {
        /* must be starting a new message, but we didn't get
         * all of the old one $TODO$ send a NAK */
        otrl_free_fragments(context);
        err = otrl_malloc_fragments(context, n);
        if (err) return OTRL_FRAGMENT_INCOMPLETE; /* $TODO$ */
    }
    else if (context->fragments[k])
    {
        /* We already got fragment K.  Not likely to be a
         * duplicate, so this must be for a new message.  But
         * we didn't get all of the old one $TODO$ send a
         * NAK */
        otrl_free_fragments(context);
        err = otrl_malloc_fragments(context, n);
        if (err) return OTRL_FRAGMENT_INCOMPLETE; /* $TODO$ */
    }
    err = otrl_add_fragment(context, k, end - start - 1, tag + 
start);
    if (err) return OTRL_FRAGMENT_INCOMPLETE; /* $TODO$ */
    if (context->fragment_k == context->fragment_n)
    {
        err = otrl_assemble_fragments(unfragmessagep, context);
        if (err) return OTRL_FRAGMENT_INCOMPLETE; /* $TODO$ */
        else     return OTRL_FRAGMENT_COMPLETE;
    }
    else
    {
        res = OTRL_FRAGMENT_INCOMPLETE;
    }
}

Chris