Closed Bug 1183827 Opened 9 years ago Closed 9 years ago

DTLS recv() handling of short buffer loses message boundaries

Categories

(NSS :: Libraries, defect)

defect
Not set
normal

Tracking

(firefox42 affected)

RESOLVED FIXED
Tracking Status
firefox42 --- affected

People

(Reporter: mcmanus, Unassigned, NeedInfo)

Details

STR
1] make a PRFileDesc from DTLS_ImportFD 
2] send it a DTLS message with a N byte cleartext payload from some other endpoint
3] PR_Recv() with a buffer sized for N/2

PR_Recv() will return the first N/2 bytes. Call PR_Recv() again to get the next N/2 bytes and it will do so as if this were a byte stream.

DTLS should preserve message boundaries, more or less in the same way that UDP does.
Thanks for this report. I agree that this behavior is wrong.

There are effectively two available options it his situation.

1. Truncate the read (i.e., flush the buffer).
2. Return an error at any attempt to read less than the full
buffered amount.

With an ordinary socket, PR_Recv() calls recv, which does #1,

https://dxr.mozilla.org/mozilla-central/source/nsprpub/pr/src/md/unix/unix.c#777

so I suggest that #1 would be right for consistency, though I prefer #2
as an API.

needinfoing mcmanus and wtc for opinions
Flags: needinfo?(mcmanus)
Flags: needinfo?(wtc)
Intuition would expect 1. Callers that were concerned with 2 could implement those semantics if we generally implemented MSG_TRUNC - I'd like to see that.
Flags: needinfo?(mcmanus)
Chrome bug - and the behaviour we adopted - is https://code.google.com/p/chromium/issues/detail?id=447431 , if that helps any. For our WebRTC API, we preserve boundaries and message sizes - effectively, Option #2 IIRC.
Offline conversation came to the conclusion that we should return an error
and flush the input buffer. 

Patch up on Rietveld
https://codereview.appspot.com/258990043/
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.21
You need to log in before you can comment on or make changes to this bug.