UDP proxy: Don't attempt to dissect dgram into records when dropping
To prevent dropping the same message over and over again, the UDP proxy test application programs/test/udp_proxy _logically_ maintains a mapping from records to the number of times the record has already been dropped, and stops dropping once a configurable threshold (currently 2) is passed. However, the actual implementation deviates from this logical view in two crucial respects: - To keep the implementation simple and independent of implementations of suitable map interfaces, it only counts how many times a record of a given _size_ has been dropped, and stops dropping further records of that size once the configurable threshold is passed. Of course, this is not fail-proof, but a good enough approximation for the proxy, and it allows to use an inefficient but simple array for the required map. - The implementation mixes datagram lengths and record lengths: When deciding whether it is allowed to drop a datagram, it uses the total datagram size as a lookup index into the map counting the number of times a package has been dropped. However, when updating this map, the UDP proxy traverses the datagram record by record, and updates the mapping at the level of record lengths. Apart from this inconsistency, the introduction of the Connection ID feature leads to yet another problem: The CID length is not part of the record header but dynamically negotiated during (potentially encrypted!) handshakes, and it is hence impossible for a passive traffic analyzer (in this case our UDP proxy) to reliably parse record headers; especially, it isn't possible to reliably infer the length of a record, nor to dissect a datagram into records. The previous implementation of the UDP proxy was not CID-aware and assumed that the record length would always reside at offsets 11, 12 in the DTLS record header, which would allow it to iterate through the datagram record by record. As mentioned, this is no longer possible for CID-based records, and the current implementation can run into a buffer overflow in this case (because it doesn't validate that the record length is not larger than what remains in the datagram). This commit removes the inconsistency in datagram vs. record length and resolves the buffer overflow issue by not attempting any dissection of datagrams into records, and instead only counting how often _datagrams_ of a particular size have been dropped. There is only one practical situation where this makes a difference: If datagram packing is used by default but disabled on retransmission (which OpenSSL has been seen to do), it can happen that we drop a datagram in its initial transmission, then also drop some of its records when they retransmitted one-by-one afterwards, yet still keeping the drop-counter at 1 instead of 2. However, even in this situation, we'll correctly count the number of droppings from that point on and eventually stop dropping, because the peer will not fall back to using packing and hence use stable record lengths.
This commit is contained in:
parent
a3daa21d8c
commit
961e677fe3
1 changed files with 7 additions and 22 deletions
|
@ -637,32 +637,17 @@ int send_delayed()
|
|||
static unsigned char dropped[2048] = { 0 };
|
||||
#define DROP_MAX 2
|
||||
|
||||
/*
|
||||
* OpenSSL groups packets in a datagram the first time it sends them, but not
|
||||
* when it resends them. Count every record as seen the first time.
|
||||
*/
|
||||
/* We only drop packets at the level of entire datagrams, not at the level
|
||||
* of records. In particular, if the peer changes the way it packs multiple
|
||||
* records into a single datagram, we don't necessarily count the number of
|
||||
* times a record has been dropped correctly. However, the only known reason
|
||||
* why a peer would change datagram packing is disabling the latter on
|
||||
* retransmission, in which case we'd drop involved records at most
|
||||
* DROP_MAX + 1 times. */
|
||||
void update_dropped( const packet *p )
|
||||
{
|
||||
size_t id = p->len % sizeof( dropped );
|
||||
const unsigned char *end = p->buf + p->len;
|
||||
const unsigned char *cur = p->buf;
|
||||
size_t len = ( ( cur[11] << 8 ) | cur[12] ) + 13;
|
||||
|
||||
++dropped[id];
|
||||
|
||||
/* Avoid counting single record twice */
|
||||
if( len == p->len )
|
||||
return;
|
||||
|
||||
while( cur < end )
|
||||
{
|
||||
len = ( ( cur[11] << 8 ) | cur[12] ) + 13;
|
||||
|
||||
id = len % sizeof( dropped );
|
||||
++dropped[id];
|
||||
|
||||
cur += len;
|
||||
}
|
||||
}
|
||||
|
||||
int handle_message( const char *way,
|
||||
|
|
Loading…
Reference in a new issue