RE: Re: SecDir review assignment

Jim,  Looks good. I think the text suggested for 6.3.2 is a bit awkward. I've suggested a change below marked with my initials, PAT. Pat

-----Original Message-----
From: Jim Pinkerton [mailto:[email protected]] 
Sent: Friday, April 21, 2006 1:57 PM
To: Derek Atkins; [email protected]
Cc: Sam Hartman; Pat Cain; Russ Housley; [email protected]
Subject: RE: [rddp] Re: SecDir review assignment

 
Derek, thanks for the detailed review. Comments/updated text enclosed below. I should have a revised ID by end of next, pending any feedback you have on my revisions.
 
 
Jim
 
> -----Original Message-----
> From: Derek Atkins [mailto:[email protected]]
> Sent: Wednesday, April 19, 2006 2:58 PM
> To: [email protected]
> Cc: Russ Housley; Pat Cain; Sam Hartman; [email protected]
> Subject: [rddp] Re: SecDir review assignment
> 
   <snip>
 
> ----
> 
> 6.3.2  Using RDMA Read to Access Stale Data
> 
> It says:
> 
>    Because of this, the local ULP SHOULD ensure that no stale data
>    is contained in the buffer before remote read access rights are
>    granted (this can be done by zeroing the contents of the memory,
>    for example).
> 
> This reduces the threat to a race condition, but the threat still 
> remains.  It is still possible for an attacker to read this buffer, 
> only the time window is reduced before the data is zero'd.  A better 
> approach would be a means to allow the ULP to inform the DDP/RDMAP 
> that the buffer is available.  E.g., allow even an Non-Privileged ULP 
> to notify the engine that a buffer is "in process" and not available..
> Sort of like marking the buffer (or stag) as "Busy".  That way the 
> Stream can only make buffers available that have valid data and you 
> don't get invalid data buffers.
> 
[<jim>]
David's comment was right on, in terms of the intent - apologies for not making this clear that with this solution the race is eliminated. Revised text:
 
If a buffer is being used for some combination of reads and writes (either remote or local), and is exposed to a Remote Peer with at least remote read access rights, the Remote Peer may be able to examine the contents of the buffer before it is initialized with the correct data. Because of this potential race condition, whatever contents were present in the buffer before the buffer is advertised can be viewed by the Remote Peer, if the Remote Peer performs an RDMA Read. This becomes a security issue if the prior contents of the buffer were not intended to be shared with the Remote Peer.
To eliminate this race condition, the local ULP SHOULD ensure that no stale data is contained in the buffer before remote read access rights are granted (this can be done by zeroing the contents of the memory, for example). This ensures that the Remote Peer can not access the buffer until the stale data has been removed.
 
<PAT> I find the rewrite confusing because of excessive redundancy. The sentence starting "Because of this potential race condition" just repeats the same information as the sentence before. I find it more clear with that sentence deleted. If you want to get the words "race condition" in there the first sentence could be reworded:
If a buffer is being used for some combination of reads and writes (either remote or local) and is exposed to a Remote Peer with at least remote read access rights before it is initialized with the correct data, there is a potential race condition where the Remote Peer can view the prior contents of the buffer. <PAT>