Subject: Re: [secdir] secdir review of draft-ietf-storm-iser-12.txt

From: Stephen Kent <[email-masked]>
Date: Mon, 05 Nov 2012 17:33:25 -0500

David,

> Steve,
>
> Thank you for this review.
>
> > RFC 5042 analyzes security issues associated with RDMA, so it is an
>
> > appropriate reference here. As an aside, I'm surprised to see that 
> RFC 5042
>
> > refers to the old IPsec RFCs (2401 series), instead of the newer 
> IPsec RFCs
>
> > (4301 series). RFC 5042 was published in October 2007, almost 2 years 
> after
>
> > the later set of IPsec RFCs, so there was plenty of time to update the
>
> > references! I didn't see any rationale in 5042 for why the earlier IPsec
>
> > RFCs were cited. (OK, time to fess up, which SECDIR member reviewed 
> 5042?)
>
> A conscious and deliberate decision was made at that time to continue 
> to use
>
> the IP Storage profile of the older IPsec RFCs (2401 series) as 
> specified in
>
> RFC 3723 for consistency.  While that decision was apparently not recorded
>
> in RFC 5042, another RFC in that series, RFC 5040 does have this to say in
>
> its Security Considerations (Section 8):
>
> The IPsec requirements for RDDP are based on the version of IPsec
>
> specified in RFC 2401 [RFC2401] and related RFCs, as profiled by RFC
>
> 3723 [RFC3723], despite the existence of a newer version of IPsec
>
> specified in RFC 4301 [RFC4301] and related RFCs [RFC4303],
>
> [RFC4306], [RFC4835].  One of the important early applications of the
>
> RDDP protocols is their use with iSCSI [iSER]; RDDP's IPsec
>
> requirements follow those of IPsec in order to facilitate that usage
>
> by allowing a common profile of IPsec to be used with iSCSI and the
>
> RDDP protocols.  In the future, RFC 3723 may be updated to the newer
>
> version of IPsec, and the IPsec security requirements of any such
>
> update should apply uniformly to iSCSI and the RDDP protocols.
>
> So, I think the (anonymous) SECDIR reviewer is off the hook on this one,
>
> which is just as well as (IIRC), at least one of the SEC ADs at the time
>
> was also involved :-).
>
> The consolidated iSCSI draft updates RFC 3723 to extend that IPsec profile
>
> to the 4301 series RFCs (as well as to the current IKEv2 specification,
>
> RFC 5996), although the 2401 series RFCs are still the "MUST implement"
>
> requirements based on what has been widely implemented.  I will check with
>
> the current security ADs (both of whom just happen to be holding Discuss
>
> positions on that iSCSI draft for good reasons) about whether to go ahead
>
> and use that iSCSI draft to update the 5040 series RFCs - I think that
>
> should probably be done.
>
Thanks for the explanation. If the WG and ADs choose to stick with the 
3401 series,
I think it would be useful to readers to note the rationale for this in 
the SC section.
>
> > The document states that "iSER requires no changes to iSCSI 
> authentication,
>
> > security and text mode negotiation mechanisms." (The wording here is 
> a bit
>
> > worrisome as suggests that the authors equate security with 
> confidentiality,
>
> > since the more general, and accurate, use of the term "security" 
> encompasses
>
> > authentication.)
>
> You could have just called that an editorial nit :-) - we'll fix it, 
> regardless.
>
Too often I review docs where the authors don't know the difference, 
which is why
I flag issues like this. but, in your case I'll chalk it up as a typo :-).
>
> > This section states plainly that iSER is "functionally iSCSI" and thus
>
> > all of the iSCSI security considerations apply. In particular, when iSER
>
> > is carried over IP networks, IPsec MUST be employed.
>
> Not exactly ;-).  The IPsec requirements are consistently "MUST implement,
>
> MAY use".
>
Right. sorry for my sloppy characterization.
>
> > There is an overly-long, single sentence paragraph discussing how to
>
> > minimize the potential for DoS attacks. The wording is a bit vague, but
>
> > the text refers  to the discussion of initiator and target behaviors,
>
> > which provide the relevant details. This is a very narrow focus on DoS,
>
> > and I suggest that the paragraph in question be augmented to state that
>
> > the only DoS attacks addressed here are ones that could cause resource
>
> > exhaustion at a target.
>
> Will edit and clarify as suggested.
>
Thanks.
>
> > The Security Considerations section concludes with a paragraph that is
>
> > a bit mysterious.
>
> What's Halloween time without a good mystery :-) :-) ?
>
good timing!
>
> > It seems to warn implementers of a residual vulnerability associated
>
> > with the use of a buffer identifier. However, the section to which this
>
> > paragraph refers contains the following sentence:
>
> >
>
> > "That iSER layer MUST check that STag invalidation has occurred whenever
>
> > receipt of a Send with Invalidate message is the expected means of 
> causing
>
> > an STag to be invalidated, and MUST perform the STag invalidation if the
>
> > STag has not already been invalidated (e.g., because a Send message was
>
> > used instead of Send with Invalidate)."
>
> >
>
> > This sort of writing does not explain complex issues to a reader.
>
> We'll provide a better explanation in the Security Considerations section.
>
> What happened here is that  the original specification of iSER (RFC 5046)
>
> required (MUST) use of a Send with Invalidate RCaP primitive in some 
> cases.
>
> That primitive has the side effect of invalidating a buffer identifier,
>
> preventing future use (as the buffer identifier enables remote direct 
> access
>
> to memory), and that requirement allowed the implementation on the 
> receiving
>
> end to rely on use of that primitive to invalidate the buffer identifier.
>
> This draft allows use of a Send RCaP primitive that does not 
> invalidate the
>
> buffer identifier in all cases, and that exposes the buffer identifier to
>
> future use, thereby creating a potential security issue if the identifier
>
> was supposed to be invalidated.  Hence when invalidation of the buffer
>
> identifier is intended or required (e.g., it's not being cached for future
>
> reuse), an iSER implementation can no longer rely on the underlying use of
>
> an RCaP Send with Invalidate primitive, but has to instead explicitly 
> check
>
> the state of the buffer identifier and/or perform a local invalidation 
> if the
>
> identifier is still valid.
>
>
Thanks for the explanation. I;m confident that your revision will make 
this OK.

No need for me to re-review.

Steve