DTLS recv() handling of short buffer loses message boundaries

RESOLVED FIXED in 3.21

Status

NSS
Libraries
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: mcmanus, Unassigned, NeedInfo)

Tracking

trunk
3.21

Firefox Tracking Flags

(firefox42 affected)

Details

(Reporter)

Description

3 years ago
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.

Comment 1

3 years ago
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)

Updated

3 years ago
Flags: needinfo?(wtc)
(Reporter)

Comment 2

3 years ago
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)

Comment 3

3 years ago
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.

Comment 4

2 years ago
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/

Comment 5

2 years ago
r=mt on Rietveld.

Committed as: http://hg.mozilla.org/projects/nss/rev/1c297c82ad9f

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED

Updated

2 years ago
Target Milestone: --- → 3.21
You need to log in before you can comment on or make changes to this bug.