We've been chasing a weird bug on VikingMUD where TLS connections would end up permanently "one command behind". Every keypress executed the previous line the player sent, and it got worse over time. We think we've tracked it down to kf_tls_receive() in src/kfun/tls/tls.c.
The loop exits once all the raw input has been fed into the BIO:
} while (len != 0 && progress != 0 && buflen != sizeof(buffer) &&
outbuflen != sizeof(outbuf));As far as I can tell, SSL_read_ex() only ever returns one record's worth of plaintext per call. So if one tls_receive() call gets handed ciphertext containing two application records, which happens in practice when a client writes each command separately and the records coalesce into one TCP segment (TinTin++ splitting on ; does this constantly), then the first pass writes all the raw bytes, reads record one, and the loop stops because len == 0.
Record two is sitting fully decryptable inside the SSL object, but nothing ever goes back for it. The bytes are already off the wire, so from the driver's point of view there's no more data, and tls_receive() doesn't get called again until the next packet arrives. At that point it returns the old buffered record instead of the new one, and the connection is now permanently behind, one more record per multi-record packet, until the player reconnects.
We reproduced it deterministically by doing one SSL_write() per command and delivering the ciphertext in a single buffer. Same behaviour on 1.5 and current master.
I think the fix is just to keep looping while SSL_read_ex() is making progress, i.e. drop the len != 0 conjunct:
} while (progress != 0 && buflen != sizeof(buffer) &&
outbuflen != sizeof(outbuf));That way the loop only ends on a pass where the read comes back with WANT_READ. We're currently working around it at the LPC level by calling tls_receive("") in a loop after each receive; each empty call drains exactly one buffered record, which also seems to confirm the diagnosis. But it'd be nicer to have it fixed at the source, if I'm reading it correctly.
We've been chasing a weird bug on VikingMUD where TLS connections would end up permanently "one command behind". Every keypress executed the previous line the player sent, and it got worse over time. We think we've tracked it down to
kf_tls_receive()insrc/kfun/tls/tls.c.The loop exits once all the raw input has been fed into the BIO:
As far as I can tell,
SSL_read_ex()only ever returns one record's worth of plaintext per call. So if onetls_receive()call gets handed ciphertext containing two application records, which happens in practice when a client writes each command separately and the records coalesce into one TCP segment (TinTin++ splitting on;does this constantly), then the first pass writes all the raw bytes, reads record one, and the loop stops becauselen == 0.Record two is sitting fully decryptable inside the SSL object, but nothing ever goes back for it. The bytes are already off the wire, so from the driver's point of view there's no more data, and
tls_receive()doesn't get called again until the next packet arrives. At that point it returns the old buffered record instead of the new one, and the connection is now permanently behind, one more record per multi-record packet, until the player reconnects.We reproduced it deterministically by doing one
SSL_write()per command and delivering the ciphertext in a single buffer. Same behaviour on 1.5 and current master.I think the fix is just to keep looping while
SSL_read_ex()is making progress, i.e. drop thelen != 0conjunct:That way the loop only ends on a pass where the read comes back with
WANT_READ. We're currently working around it at the LPC level by callingtls_receive("")in a loop after each receive; each empty call drains exactly one buffered record, which also seems to confirm the diagnosis. But it'd be nicer to have it fixed at the source, if I'm reading it correctly.