Getting really close now, but there are some major issues still to deal with ...
zidel changes from bc67c3e4370ef584c58166cc4f6d326030dc83f2 to a0be689df7860dca5cc43b0ef949aeff7cb511c1 - NFPPacket: SortedSet not LinkedList of acks. - NFPPacket: Acks are relative to the previous ack, not relative to the first ack. This allows us to cover a much wider range. - Synchronization fix. - Make some inner classes static. - isShortMessage not isLongMessage in MessageWrapper, be consistent. - Remove dead code, imports. - Rewrite the fragment creation method. - Unit tests: Set up RTT before doing tests relying on it. - Fix a unit test range check. - Unit test: Use different bytes so we can detect byte order bugs in receive sequence number. - createPacket() doesn't account for HMAC, subtract from available size before entering. - Return SHA256 to the pool after use. - SparseBitmap: Throw on bogus range. - Fix losing messages on full packet. - Sanity checking for ranges when data is added rather than later on in SentPacket. - Fix recording that we've sent a packet when we haven't due to not having a key. - Use average RTT, not maximum RTT. - Fix RTT average calculation: Ignore RTT slots not used yet. - Return 0 as default RTT. - Trivial code tidyup/optimisations (use Map.Entry iterator). - Imports. - Store message ID as long not int. - Use 28 bit non-wrapping message ID's. After the first one, any positive delta of no more than 4096 is compressed to 2 bytes. We use 4 bits for flags including the indicator of a full message. Remove code related to message ID reuse, it appears it is not possible to make it reliable. - Predict the next 512 sequence numbers and ignore any packet not starting with an expected sequence number. Refreshed when we get 3/4 of way through the buffer. - Don't send packets which are outside the receiver's 512-slot window. - Use the ECB-encrypted packet number as IV for the packet. Uses a separate key, the IV key, generated by JFK. Unfortunately it does not use a nonce yet. - Use a real keyed HMAC rather than SHA256, using a separate key generated by JFK. - Generate extra keys for the HMAC and the IV from JFK using its key generation function. - On disconnect, tell all the MessageWrappers about the disconnect. - New negotiation type 5 uses the new packet format. - Don't create a new packetFormat when rekeying. The sequence of packets should not be affected, only the crypto keys, to ensure no disruption and no complicated and buggy handovers. - Sender keeps track of estimated buffer size on the receiver, and ensures we do not exceed the maximum 256KB. - Drop packets if we can't expand the buffer to cover the new data. - Minor code cleanups. - Logging 4f08a3e13de550f0c15f910a8e23b73ac18592f4 - should addAck() be able to throw an error if the ack can't be encoded? currently we don't even check at toBytes() time??? e498d8d502a64e1413ebdf69e05ce0940d140d02 - can item.buf.length be < end ? d49933fadd01ab44145dc8346fa43cb0d4812d76 - is a default of 0 a good idea?? ac10be3f2059c89e42e23de8613b86e46fd013ee - you should check to make sure that the first fragment ID is always the full 4 bytes - deltas are always positive. this is sensible but we should try to pack in order if possible for maximum efficiency? - you don't check for negative delta either in toBytes() or in addMessageFragment(). You need to afaics. - how long will it take for us to go through 268M message ID's? it should be safe to wrap as long as the packet-level delta is so large that there is no possibility of confusion, so it doesn't affect rekeying, correct? (Actually rekeying has to be separate from the message sequence anyway ...) 1ad9015e0b642d52ce5096a1452b23068aaea19a - packet sequence numbers appear to be per-SessionKey? are message sequence numbers global (for a peer) while packet sequence numbers are per-encryption key (and potentially per-transport as well)? - you didn't initialise the PCFB with an IV! I hope you don't actually use it like this to encrypt packets! Really we should make this impossible at the API level ... I guess you haven't implemented the IV key yet ... - IMHO this should be a rolling window, rather than being periodically refreshed. I.e. you maintain a pointer within the buffer, and when you accept a packet you move the pointer and update the slots behind it. And because the IV is generated from the packet number, you will need to use the offset of the match to tell you the packet number in order to do the decryption. But maybe I'm wrong? 9c43896c682115f47ea2fc669bc7f6d5cb08c2aa - HERE BE DRAGONS !!!! - I spent a significant amount of time on issues related to this on the old packet format. - On the old packet format, we could not send packet N+256 until packet N had been acknowledged. But we still needed to be able to send ack's, and we don't want to busy loop. Hence if we can't send a packet with a sequence number, we send one without a sequence number (seqno=-1), containing only acks. The problem is that we can't reuse the same seqno in the new packet format because the IV is derived from the seqno and the IV must be unique for a packet. - IMHO you do *not* need to take into account the receive window as you do here. Packets are generally delivered in order, so you can have a lot more packets in flight than the decryption window and still have all of them decrypt correctly; and this is likely to be the case anywhere where you have a high round trip time. - Is there any other reason to arbitrarily limit the number of packet sequence numbers in flight? If you could just ignore the issue it avoids a lot of complications, network deadlocks and similar problems. Can we? Retransmitting at the message level ensures we never lose any data. But on the other hand, we do need to maintain some state for every packet we send which has not yet been acknowledged. AFAICS there is no way around this, it will require a maximum window size between transmitted and acknowledged. - If we do have to address the issue, I guess the best option is to have two packet sequence numbers - one for packets with content and one for packets without content. We *MUST* be able to send ack's or we risk a network-level deadlock, with each side refusing to send packets until the other side does. - Simple fix: If you don't use the sequence number, don't increment it! Incrementing it makes the situation worse! - With regards to busy-looping, we solve this in the old packet format code by 1) when we realise we can't send packets with data, we set a flag so we are not considered in PacketSender in deciding when to wake up, and 2) when we receive an ack, if we are blocked, we wake up PacketSender in case we can unblock. See PacketTracker.wouldBlock(), FNPPacketMangler.processOutgoingOrRequeue handling of WouldBlockException etc. a1a17c4ffe8f3fe652f42d523b0a71934665eddf - you need to use a nonce as well as the IV key. Generate an extra key from JFK, use it to fill the 28 bytes that are not occupied by the packet number. This is a minor twiddle in CTR mode that I hadn't realised about until recently. 853ffa63d0b0c85b327be39787ee5f2d7d320310 - you need a separate key for the HMAC too. i'm sure you will implement this. d5d5d6c52d1690be2872f07502cef9a45058c16b - I don't understand why this commit is useful or necessary. MessageWrapper's are all wrapping MessageItem's, aren't they? So they will get told of disconnect anyway? 612bda3d1f87928f696d4e6e0acee0c7ec16adae - Busy looping issues here are similar to not being able to allocate a packet sequence number: Acknowledgements can result in us being able to send another packet, hence waking up the PacketSender. Does this make that mechanism unnecessary? I'm not sure - if the receiver doesn't ack all packets, we could be hanging on to a lot of packets even with a limited total data in flight.... 7d80c2ca4d28f7ae0a01bb515ad00543c2954c5d - We should not add the message ID if it is new and we don't have space for it. - Can we check whether the message length is the same as previous assertions? It should not change once specified. - Can we optimise the sender size for high packet loss by e.g. trying to complete existing fragmented sends, possibly one at a time, before starting new ones? -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 197 bytes Desc: This is a digitally signed message part. URL: <https://emu.freenetproject.org/pipermail/devl/attachments/20100806/48e15642/attachment.pgp>
