Subject: [secdir] SecDir and AppsDir review of draft-ietf-storm-iscsi-cons-06

From: Alexey Melnikov <[email-masked]>
Date: Wed, 03 Oct 2012 14:54:50 +0100

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the IESG.
These comments were written primarily for the benefit of the security
area directors.  Document editors and WG chairs should treat these
comments just like any other last call comments.

This document describes a transport protocol for SCSI that works
on top of TCP.

Summary

Overall this is a very well written document. I also trust that all 
options were implemented by at least 1 implementation, so the document 
reflects implementation experience.

The Security Considerations section is thorough. iSCSI security 
partially relies on security provided by IPSec. Unfortunately I do not 
have the IPSec expertise to determine if all relevant cases were 
covered, although the text looked plausible.

Security considerations for CHAP and SRP authentication mechanisms are 
discussed in details and seem to be adequate. I've not seen any 
discussion of KRB5 (mentioned in 12.1) related issues though.

I also have other more detailed comments (some of them are security 
related). They range between minor and nits, although some of them can 
be reclassified as a bit more important:

In Section 2.2:

 >- SCSI Port Name: A name made up as UTF-8 characters and includes
 >  the iSCSI Name + 'i' or 't' + ISID or Portal Group Tag.

The first mention of UTF-8 needs a reference.

"UTF-8 characters" should be something like "UTF-8 [UTF-8] encoding of 
Unicode characters.
(There is no such thing as "UTF-8 character")

In Section 4.2, 2nd paragraph:

 >  For the remainder of this document, the terms "initiator" and
 >  "target" refer to "iSCSI initiator node" and "iSCSI target node",
 >  respectively (see iSCS) unless otherwise qualified.

I didn't find iSCS in the list of acronyms.

In 4.2.2.3.3:

   Whenever the TaskReporting key (Section 13.23"Task Reporting") is
   negotiated to ResponseFence or FastAbort for an iSCSI session and
   the Response Fence behavior is required for a SCSI response
   message,

The last part: how can this be known?

   the target iSCSI layer MUST perform the actions described
   in this Section for that session.

In 4.2.2.3.4:

  Note:
     - Due to the absence of ACA-related fencing requirements in
       [RFC3720], initiator implementations SHOULD NOT use ACA on
        multi-connection iSCSI sessions with targets complying only
        with [RFC3720].

How can the initiator know whether the target only complies with RFC 
3720 or not?

4.2.3.2. Notion of Affected Tasks

   LOGICAL UNIT RESET: All outstanding tasks from all initiators for
   the LU identified by the LUN field in the LOGICAL UNIT RESET
   Request PDU.

Are there any access control facilities for this operation?

4.2.3.4. FastAbort Multi-task Abort Semantics

   Protocol behavior defined in this Section SHOULD be implemented by

Why is this a SHOULD (and not a MUST)? What are possible alternatives?

   all iSCSI implementations complying with this document. Protocol
   behavior defined in this Section MUST be exhibited by iSCSI
   implementations on an iSCSI session when they negotiate the
   TaskReporting (Section 13.23) key to "FastAbort" on that session.
   The execution of ABORT TASK SET, CLEAR TASK SET, LOGICAL UNIT
   RESET, TARGET WARM RESET, and TARGET COLD RESET TMF Requests
   consists of the following sequence of actions in the specified
   order on the specified party.

In Section 4.2.7.1

      iSCSI names are composed only of displayable characters.

What does "displayable" means here? "ASCII printable"? "Unicode printable"?

         iSCSI
         names allow the use of international character sets but are
         not case sensitive.

What does this mean exactly? Do you mean ASCII case sensitivity? Unicode 
case sensitivity?

         No whitespace characters are used in
         iSCSI names.

What is "whitespace". U+0020? (ASCII space) Something else?

4.2.7.2. iSCSI Name Encoding

   The stringprep process is described in [RFC3454]; iSCSI's use of
   the stringprep process is described in [RFC3722]. Stringprep is a
   method designed by the Internationalized Domain Name (IDN) working
   group to translate human-typed strings into a format that can be
   compared as opaque strings. Strings MUST NOT include punctuation,
   spacing, diacritical marks, or other characters that could get in
   the way of readability.

This MUST NOT is not well defined. You need to define specific Unicode 
codepoints or character classes.

   The stringprep process also converts
   strings into equivalent strings of lower-case characters.

   The stringprep process does not need to be implemented if the
   names are only generated using numeric and lower-case (any
   character set) alphabetic characters.

Lower-case is not well defined, unless you say you mean ASCII or 
something else.
Also, "any character set"? This really doesn't look correct.

   Once iSCSI names encoded in UTF-8 are "normalized" they may be
   safely compared byte-for-byte.

Nit: I suggest adding "as described in this section" after "normalized" 
to distinguish this from various forms of Unicode Normalization, etc.

In Section 4.2.7.4:

   To generate names of this type, the person or organization
   generating the name must own a registered domain name. This domain
   name does not have to be active,

IMHO, the meaning of "active" is not clear here.

   and does not have to resolve to
   an address; it just needs to be reserved to prevent others from
   generating iSCSI names using the same domain name.

Every time the document is referring to "hexadecimal" - is the case 
important or not?

In 4.2.9:

   In situations where IP packets are delivered in order from the
   network, iSCSI message framing is not an issue and messages are
   processed one after the other. In the presence of IP packet
   reordering (i.e., frames being dropped), legacy TCP

Nit: What is so "legacy" about these implementations? Please explain or 
drop "legacy" here.

   implementations store the "out of order" TCP segments in temporary
   buffers until the missing TCP segments arrive, upon which the data
   must be copied to the application buffers. In iSCSI, it is
   desirable to steer the SCSI data within these out of order TCP
   segments into the pre-allocated SCSI buffers rather than store
   them in temporary buffers. This decreases the need for dedicated
   reassembly buffers as well as the latency and bandwidth related to
   extra copies.

4.3. iSCSI Session Types

   The session type is defined during login with key=value parameter
   in the login command.

Using which key?

In 4.6.1.5:

   To enable a SCSI command to be processed while involving a minimum
   number of messages, the last SCSI Data-in PDU passed for a command
   may also contain the status if the status indicates termination
   with no exceptions (no sense or response involved).

A forward reference to "sense" would be good here.

In 6.1:

  base64-constant: base64 constant encoded as a string that
        starts with "0b" or "0B" followed by 1 or more digits or
        letters or plus or slash or equal.

I think you need to include ASCII codes for the above, just to be clear.

Also, this is not quite correct, as leading "=" is not valid in base64 
encoding, etc.

        The encoding is done
        according to [RFC2045]

Please reference "Section 4 of [RFC4648]" instead of RFC 2045. RFC 4648 
is the most recent
RFC for base64 definition.

6.2.1. List negotiations

   In list negotiation, the originator sends a list of values (which
   may include "None") in its order of preference.

Is "None" a special value that designates an empty list? Or am I 
misintepreting what it is used for?

   If an acceptor does not understand any particular value in a list,
   it MUST ignore it. If an acceptor does not support, does not
   understand, or is not allowed to use any of the proposed options
   with a specific originator, it may use the constant "Reject" or
   terminate the negotiation. The selection of a value not proposed
   MUST be handled as a protocol error.

I suggest inserting "by the initiator" before "as a protocol error".

6.2.2. Simple-value Negotiations

   Specifically, the two cases in which answers are OPTIONAL are:

      - The Boolean function is "AND" and the value "No" is
        received. The outcome of the negotiation is "No".

      - The Boolean function is "OR" and the value "Yes" is
        received. The outcome of the negotiation is "Yes".

You lost me here, can you provide an example or two?

In 6.3:

   The Login Phase MAY include a SecurityNegotiation stage and a
   LoginOperationalNegotiation stage and MUST include at least one of
   them, but the included stage MAY be empty except for the mandatory
   names.

Again, an example (or pointer) here would be useful, as I am not 
entirely clear what this mean.

   Security MUST be completely negotiated within the Login Phase. The

What is the meaning of the word "Security" in this case?

   use of underlying IPsec security is specified in Chapter 8 and in
   [RFC3723]. iSCSI support for security within the protocol only
   consists of authentication in the Login Phase.

   In some environments, a target or an initiator is not interested
   in authenticating its counterpart. It is possible to bypass

How?

   authentication through the Login request and response.

In 6.3.1:

       -Login Response with Login reject. This is an immediate
        rejection from the target that causes the connection to
        terminate and the session to terminate if this is the first
        (or only) connection of a new session. The T bit and the CSG
        and NSG fields are reserved.

What does "reserved" mean in this case?

In 6.3.2:

   It should be noted that the negotiation might also be directed by
   the target if the initiator does support security, but is not
   ready to direct the negotiation (propose options).

How can this be done?

In 6.3.5:

   Session timeout is an event defined to occur when the last
   connection state timeout expires and no tasks are waiting for
   reassignment. This takes the session to the FREE state (N6
   transition in the session state diagram).

Is the timeout implied or negotiated?

In 7.1.4:

   In the detailed description of the recover classes the
   mandating terms (MUST, SHOULD, MAY, etc.) indicate normative
   actions to be executed if the recovery class is supported and
   used.

How is such support shown/negotiated?

7.1.4.1. Recovery Within-command

      Header digest error, which manifests as a sequence reception
         timeout or a sequence error - dealt with as specified in
         Section 7.9, using the option of a recovery R2T.

Did you mean 7.9 or 7.8?
(Also check elsewhere in the document.)

7.1.4.2. Recovery Within-connection

   At the initiator, the following cases lend themselves to within-
   connection recovery:

      - Requests not acknowledged for a long time. Requests are
        acknowledged explicitly through ExpCmdSN or implicitly by
        receiving data and/or status. The initiator MAY retry non-
        acknowledged commands as specified in Retry an.

Sorry, I didn't get what "Retry an" is.

In 7.2.2:

   In reassigning connection allegiance for a command, the targets
   SHOULD continue the command from its current state. For example,
   when reassigning read commands, the target SHOULD take advantage
   of the ExpDataSN field provided by the Task Management function
   request (which must be set to zero if there was no data transfer)
   and bring the read command to completion by sending the remaining
   data and sending (or resending) the status. ExpDataSN
   acknowledges all data sent up to, but not including, the Data-In
   PDU and or R2T with DataSN (or R2TSN) equal to ExpDataSN. However,
   targets may choose to send/receive all unacknowledged data or all
   of the data on a reassignment of connection allegiance if unable
   to recover or maintain accurate an state.

Nit: Should "an" be dropped?

7.14. Connection Failures

   iSCSI can keep a session in operation if it is able to
   keep/establish at least one TCP connection between the initiator
   and the target in a timely fashion. Targets and/or initiators may
   recognize a failing connection by either transport level means
   (TCP), a gap in the command sequence number, a response stream
   that is not filled for a long time, or by a failing iSCSI NOP
   (acting as a ping). The latter MAY be used periodically to
   increase the speed and likelihood of detecting connection
   failures. Initiators and targets MAY also use the keep-alive
   option on the TCP connection

I think TCP keep-alive option needs a reference here.

   to enable early link failure
   detection on otherwise idle links.

In 9.2.1:

The first mentioning of RADIUS needs an Informative reference.

In 9.3.1:

- HMAC-SHA1 MUST be implemented [RFC2404].

RFC 2404 seems to define HMAC-SHA-1-96, not HMAC-SHA1.

9.3.2. Confidentiality

   The NULL encryption algorithm MUST also be implemented.

I find it odd that the section talks about how weak DES is and then 
requires NULL encryption to be supported. What is the reason for this?

9.3.3. Policy, Security Associations, and Cryptographic Key
         Management

      - When digital signatures are used to achieve authentication,
        an IKE negotiator SHOULD use IKE Certificate Request
        Payload(s) to specify the certificate authority. IKE
        negotiators SHOULD check the pertinent Certificate
        Revocation List (CRL) before accepting a PKI certificate for
        use in IKE authentication procedures.

What are the reasons for these requirements being SHOULD level (as 
opposed to MUST level)?

   - The following identification type requirements apply to IKEv1.
     ID_IPV4_ADDR, ID_IPV6_ADDR (if the protocol stack supports
     IPv6) and ID_FQDN Identification Types MUST be supported;
     ID_USER_FQDN SHOULD be supported. The IP Subnet, IP Address
     Range, ID_DER_ASN1_DN, and ID_DER_ASN1_GN Identification Types
     SHOULD NOT be used. The ID_KEY_ID Identification Type MUST NOT
     be used.

It would be good to know the reason for the last SHOULD NOT and the last 
MUST NOT.

10.5. Synch and Steering Layer and Performance

   While a synch and steering layer is optional,

Is this section defining "synch and steering layer"? I found the use of 
this term confusing.

   an initiator/target
   that does not have it working against a target/initiator that
   demands synch and steering may experience performance degradation
   caused by packet reordering and loss. Providing a synch and
   steering mechanism is recommended for all high-speed
   implementations.

10.6.1. Determining the Proper ErrorRecoveryLevel

      Probability of transport layer "checksum escape".

Is this a well known term and if it is, what does it mean?

11.3.1. Flags and Task Attributes (byte 1)

      bit 3-4 Reserved.

Here and everywhere else in the document: what does it mean "reserved"?
Do you mean "MUST be set to zero when sent, MUST be ignored when received"?

11.4.4. SNACK Tag

    For a detailed discussion on R-Data SNACK see SNACK.

Is the last "SNACK" supposed to be a proper section reference?

11.4.5.3. SCSI REPORT LUNS and Residual Overflow

   If the Expected Data Transfer Length (EDTL) in the iSCSI header of
   the SCSI Command PDU for a REPORT LUNS command is set to at least
   as large as that ALLOCATION LENGTH, the SCSI layer truncation
   prevents an iSCSI Residual Overflow from occurring. A SCSI
   initiator can detect that such truncation has occurred via other
   information at theS CSI layer. The rest of the Section elaborates

s/theS CSI/the SCSI

   this required behavior.

11.9.2. AsyncVCode

    AsyncVCode is a vendor specific detail code that is only valid if
    the AsyncEvent field indicates a vendor specific event. Otherwise,
    it is reserved.

Is there an IANA registry for these?

11.13.8. Login Parameters

   Keys in Section 12, only need to be
   supported when the function to which they refer is mandatory to
   implement.

How is this decided?

12.1. AuthMethod

    Use: During Login - Security Negotiation
    Senders: Initiator and Target
    Scope: connection

    AuthMethod = <list-of-values>

    The main item of security negotiation is the authentication method
    (AuthMethod).

   The authentication methods that can be used (appear in the list-
   of-values) are either those listed in the following table or are
   vendor-unique methods:

Is there an IANA registry for this?

12.1.2. Secure Remote Password (SRP)

   For SRP [RFC2945], the initiator MUST use:

       SRP_U=<U> TargetAuth=Yes     /* or TargetAuth=No */

   The target MUST answer with a Login reject with the "Authorization
   Failure" status or reply with:

   SRP_GROUP=<G1,G2...> SRP_s=<s>

How are elements delemited on the wire?

12.1.3. Challenge Handshake Authentication Protocol (CHAP)

   For CHAP [RFC1994], the initiator MUST use:

      CHAP_A=<A1,A2...>

As above: How are elements delemited on the wire?

   To guarantee interoperability, initiators MUST always offer it as
   one of the proposed algorithms.

"Offer" or "support"? Most Apps protocols describe support, not 
necessarily requiring that
the mandatory-to-implement is enabled in a particular server.

---

From: Alexey Melnikov <[email-masked]>
Date: Tue, 09 Oct 2012 12:13:07 +0100

On 09/10/2012 04:42, Mallikarjun Chadalapaka wrote:
> Alexey, thanks for the review again. My responses in line.
Hi,
Thanks for the response.
> Couple of quick comments:
>
> - I am attaching a candidate -08 revision addressing most of your comments.  See the change bars/red text to see what's changed.
>
> - I think some of your comments are more about the current style of iSCSI spec. The spec by design introduces a concept in non-normative text to illustrate the idea, and then in a subsequent section/paragraph proceeds to specify normative requirements. I see you have a bunch of comments that you noted on the non-normative text, and almost all of them are addressed in subsequent normative sections. You'll find my responses on each in line, when that is the case.
Yeah, I think you are lacking lots of forward references.
> - I am spinning off your comments on security sections into a separate thread, just to keep it easier to track.
Ok. I will move my response about StringPrep/normalization into another 
thread.
> Thanks, and sorry for the delayed response on my end.
>
> Mallikarjun
>
>
>> -----Original Message-----
>> From: Alexey Melnikov [mailto:[email-masked]]
>> Sent: Wednesday, October 03, 2012 6:55 AM
>> To:[email-masked];[email-masked];
>> [email-masked]
>> Subject: SecDir and AppsDir review of draft-ietf-storm-iscsi-cons-06
>>
  [...]
> [Mallikarjun:] See section 13.23. TaskReporting was negotiated to ResponseFence or FastAbort.
Ok. No change is needed.
> [Mallikarjun:] See section 13.23. TaskReporting was negotiated to "RFC3720" or "NotUnderstood"
I think a forward reference here would be good.
> [Mallikarjun:] LU Reset is a SCSI operation that iSCSI simply acts as a transport for. So as such, iSCSI does not define an access control mechanism for the SCSI-level operation. SCSI specs ([SPC4]) define ACCESS CONTROL OUT and ACCESS CONTROL IN commands to set such access controls.
This sounds like a security consideration to me. At least it seems worth 
pointing this out.
> [Mallikarjun:] Because some steps may not be backwards-compatible with RFC 3720-compliant implementations. That's the reason or the immediately following "MUST" statement - to make the distinction between the two clear.
Personally I am a big fan of explaining SHOULDs. If you can add your 
explanation, that would be great.
>> What does "displayable" means here? "ASCII printable"? "Unicode
  [Moving this to another thread.]In Section 4.2.7.4: To generate names 
of this type, the person or organization generating the name must own a 
registered domain name. This domain name does not have to be active, 
IMHO, the meaning of "active" is not clear here.

  [...]
>   [Mallikarjun:] "active" as in "not expired registration"
If it is expired, it is no longer owned, which conflicts with the 
previous sentence ;-).
>   [Mallikarjun:] Not important, it's normalized to lower-case via stringprep for key names, and for key values, we explicitly allow upper and lower case.
I don't mind either way, I am just missing where this is explicitly said.
  [...]
>   [Mallikarjun:] Agreed that the key could have been specified here as well, but note that this is the non-normative descriptive text covering the concept (Section 6.3 covers it). We could add it here as well.
Or just add a forward reference.
>   [Mallikarjun:] Section 4.6.1.2 already introduces sense data (which is a SCSI concept, not iSCSI's)
Ah, Ok.
>   [Mallikarjun:] IMHO, the correct reference to RFC 2045 (which already exists) should address this comment. None of the preceding sections cite the respective ASCII codes either, e.g. hex-constant, decimal-constant.... While I suppose we could reference ASCII codes everywhere, I do not honestly see the necessity in this case - as readability will be impacted with lots of such ASCII code references on these pages (FYI, no changes have been made from RFC 3720 here).
Well, when I read this I need to know if you are talking about ASCII or 
Unicode. The text is written as if it is talking about ASCII and I don't 
think it was updated properly.
>   [Mallikarjun:] Sorry, I do not follow this. Leading "=" is not required for base64-constant by iSCSI. Are you referring to the usage of "key=value"? When used as a key value, the "=" sign is interpreted by iSCSI key parsing logic as for other keys, it's not considered as part of a base64 value.
I was trying to say that in a valid base64 encoded string something like 
AAAA==BB is not valid, as nothing can follow trailing "=".
Basically I would like for your document to be more strict about 
defining which data is valid.
>   [Mallikarjun:] Sure, will do.
>
>> 6.2.1. List negotiations
>>
>   [Mallikarjun:] Nope, it is not an empty list. When both sides agree to it, it means that no key-specific semantics are operational for the functionality represented by the key. Look at 12.1 (AuthMethod), and 13.1 (Digests) for examples.
Ok. It would be helpful to explain this earlier.
>   [Mallikarjun:] I assume you mean "by the originator".
Ok.
> Yes, we could add it, although IMHO the context makes it obvious.
Wasn't clear to me, that is why I've asked (I've seen other protocols 
that do that differently.)
>   [Mallikarjun:] Sure, look at Section 13.10 (InitialR2T), as an example. For this key, if one side proposes "Yes", the result is guaranteed to be "Yes", so the responder does not need to respond.
Ok.
>> What is the meaning of the word "Security" in this case?
FYI, unqualified word "security" is a red flag during SecDir review ;-).
>   [Mallikarjun:] But if you skip ahead just a few sentences, you see what is meant:
>
> The SecurityNegotiation keys appear in Section 12 and the LoginOperationalNegotiation keys appear in Section 13. Only a limited set of keys (marked as Any-Stage in Section 13) may be used in any of the two stages.
>
> In general, the login semantics of iSCSI are fairly complex, and the team of authors has re-written/re-ordered this text a handful of times during the original RFC 3720 work (with a *lot of help* from WG, :) ) So I have taken care not to preserve the original RFC 3720 text verbatim here.
>
>   [Mallikarjun:] This is also specified after a few sentences, :-)
>
> Stage transition is performed through a command exchange (request/response) that carries the T bit and the same CSG code. During this exchange, the next stage is selected by the target through the "next stage" code (NSG). The selected NSG MUST NOT exceed the value stated by the initiator. The initiator can request a transition whenever it is ready, but a target can only respond with a transition after one is proposed by the initiator.
Ok.
>   [Mallikarjun:] MUST be ignored on reception and SHOULD be set to 0 when sending. Sections 11.13.6 and 11.13.7 outline the precise normative semantics for setting these fields.
>
>> In 6.3.2:
>>
>   [Mallikarjun:]  Target can propose a new security key while responding to a Login Request by setting CSG,NSG=0,0 T=0 (See Appendix B where a precise example for this scenario is included).
A forward reference to the example would help.
>   [Mallikarjun:] Negotiated or specified (see the DefaultTime2Retain text key, and the Time2Retain field in Login Response PDU).
Ok.
>   [Mallikarjun:] By negotiating for ErrorRecoveryLevel key. See section 7.1.5 (error recovery hierarchy).
Forward reference is missing :-).
> [Mallikarjun:] I did mean section 7.9, :-) Section 7.9 discusses the meta problem of a sequence error which is the right reference here. Note that handling sequence errors may require digest error recovery, so it defers to Section 7.8 as appropriate.
Ok, never mind then.
  [...]
> <snip of security sections related comments>
>   [Mallikarjun:] I agree this usage reads a little broken, and this is because we have removed some sections that were normative in RFC 3720, and I should have included a bridging sentence in section 4.2.9, e.g.
>
> Different schemes can be used to recover synchronization. The details of any such schemes are beyond this protocol specification, but it suffices to note that [RFC4297] provides an overview of the direct data placement problem on IP networks, and [RFC5046] specifies a protocol extension for iSCSI that facilitates this direct data placement objective. Rest of this document refers to any such direct data placement protocol usage as an example of a "Synch and Steering layer".
Looks good to me.
>   [Mallikarjun:] This refers to the message corruptions undetected by the TCP checksum algorithm. "checksum escape" is a term at least the ips WG had used extensively in discussion, e.g.http://www.pdl.cmu.edu/mailinglists/ips/mail/msg04095.html  
> http://www.ietf.org/proceedings/51/slides/ips-6.pdf  
>
> IPS WG also had published an informational RFC (RFC 3385) to cover this entire area of checksums and CRCs and undetected errors.
>
> Having said that, it is probably a good idea to make this explicit in the doc, e.g. " "checksum escape" (message error undetected by TCP checksum)."
Yes. And add a reference.
>   [Mallikarjun:] As responded to earlier, it's a SHOULD-MUST. I think it's worth adding the following at the end of section 4.2.2.1 :
>
> In the rest of the document, unless otherwise explicitly stated, we use the term "reserved" for a PDU field to indicate that the field MUST be ignored on reception and SHOULD be set to 0 when sending.
Yes!
>   [Mallikarjun:] It's a reference to section 4.6. 3.4 as you can see in pdf, but it isn't proper, :-) Requires fixing, actually 11.16.3 is a better reference.
Ok, great.
>   [Mallikarjun:] No, it's a vendor-unique usage field that by definition may not work across vendors, and that fact should not affect interoperability.
Ok.
  [...]
>   [Mallikarjun:]Yes.http://www.ietf.org/assignments/iscsi-parameters  
>   [Mallikarjun:] As a comma-separated string (it's the value of the text key).
It wasn't entirely clear to me. Adding an example here (or pointing to 
one elsewhere in the document) would help.
>   [Mallikarjun:] Same as above.
As above.
>   [Mallikarjun:] "Offer" has a stronger implication for iSCSI than "support" -e.g. an implementation may support a method, but not necessarily offer it. OTOH, when an implementation offers a semantic, it means it's supported and is ready to use it for the specific session.
Well, Ok. As I said, in other protocols the requirement is to support. 
For example an IMAP server would support PLAIN authentication, but it 
might be turned off (via configuration). But anyway, if you sure this is 
what you want for your protocol, then Ok.

---

From: Alexey Melnikov <[email-masked]>
Date: Thu, 11 Oct 2012 12:34:51 +0100

Hi,
Sorry for the delay with this reply.

On 09/10/2012 04:42, Mallikarjun Chadalapaka wrote:
> [Mallikarjun:] The latter. Note that all normative statements make it clear that we refer to Unicode-encoding - especially look at Section 4.2.7.2 where it discusses the stringprep-normalization. This section (4.2.7.1) is just a non-normative discussion of the user-visible characteristics.
At this point I haven't read 4.2.7.2 and it is not clear that this 
section is non normative. I wish you would use more standard terminology 
and/or forward references.
>   [Mallikarjun:] See above
Any comments on this?
> [Mallikarjun:] Given the number of scripts that Unicode supports, this does not strike me a trivial exercise. I suspect that's why the original ips WG left it as such, and this level of specificity has worked well for over a decade. However, I am not a Unicode expert at all, so I may be overlooking something obvious you have in mind?
Unicode Consortium has defined classes of characters (e.g. "letters", 
"digits", "punctuation", etc.). You can just reference such classes.
Happy to try to suggest some text or find a person who can help you 
reword this.

Without specific details your MUST NOT is not implementable and not 
enforceable. Worse, I suspect nobody has implemented it anyway. I think 
your choices are: (a) delete the sentence as it is not useful or (b) fix 
it so that it actually ensures interoperability.
>   [Mallikarjun:] The entire iSCSI Name discussion is in the context of Unicode, and it is called out in normative sentences in appropriate places. This is non-normative descriptive text. OTOH, if you have specific suggestions, they are welcome.
I think you meant to write:

    The stringprep process does not need to be implemented if the
    names are only generated using ASCII numeric and Unicode characters with "Lowercase" property.

---

From: Alexey Melnikov <[email-masked]>
Date: Thu, 11 Oct 2012 14:57:10 +0100

On 11/10/2012 14:35, Black, David wrote:
> Alexey,
Hi David,
> Thanks for splitting this out into a separate thread.  More comments inline.
>
>>> [Mallikarjun:] The latter. Note that all normative statements make it clear
>> that we refer to Unicode-encoding - especially look at Section 4.2.7.2 where
>> it discusses the stringprep-normalization. This section (4.2.7.1) is just a
>> non-normative discussion of the user-visible characteristics.
>>
> "Unicode printable" is intended - the details are in 4.2.7.2 and the
> stringprep profile in RFC 3722.  I would change "displayable characters"
> to "printable ASCII and Unicode characters".  Yes, I know that ASCII is a
> subset of Unicode, but not every reader of this RFC will have that level
> of Unicode familiarity.
Ok.
> There's been a separate email exchange with Pete Resnick about this text.
> The conclusion is that the words "case sensitive" above are misleading,
> so the above sentence needs to be rewritten and expanded as follows:
>
> 	iSCSI names allow the use of international characters
> 	but uppercase characters are prohibited.  The iSCSI stringprep
>        profile [RFC3722] maps uppercase characters to lowercase and
>        SHOULD be used to prepare iSCSI names from input that may
>        include uppercase characters.
This would be Ok, if you update the above to say ASCII or Unicode.
> Something else - see RFC 3722, which specifies the explicit prohibitions
> beyond U+0020, specifically it prohibits the space characters in tables
> C.1.1 and C.1.2 from RFC 3454.  If it helps, we could add ", see [RFC3722]
> for details" at the end of that sentence.

Sounds good.

>>> [Mallikarjun:] Given the number of scripts that Unicode supports, this does
>> not strike me a trivial exercise. I suspect that's why the original ips WG
>> left it as such, and this level of specificity has worked well for over a
>> decade. However, I am not a Unicode expert at all, so I may be overlooking
>> something obvious you have in mind?
> Alexey is correct about this one - that "MUST NOT" is too strong and not
> implementable, even though it's a "good idea" in principle.
>
> This text was originally intended as usage guidance on avoidance of
> characters that result in different strings that look the same or similar
> to humans.  This is already a concern in ASCII (e.g., courtesy of l, 1, 0
> and O), but is much more of a concern for Unicode.
>
> The sentence needs a rewrite as guidance that avoids using "MUST NOT", e.g.:
>
> 	iSCSI names are expected to be used by administrators for purposes
> 	such as system configuration - for this reason, characters that may
> 	lead to human confusion among different iSCSI names (e.g., punctuation,
> 	spacing, diacritical marks) should be avoided, even when such
> 	characters are allowed as stringprep processing output by [RFC3722].

Ok. This is not what I expected the replacement text to say, but this is 
better.

> The mandatory requirements on allowed vs. prohibited characters are in
> RFC 3722, and (IMHO) it's best to not expand on them here.
>
> Actually it is well-defined in RFC 3722 via its reference to table B.2
> in RFC 3454.
Well, it is only well defined if you say what it means ;-). Adding a 
reference to B.2 from RFC 3454 would address this.
> OTOH, that's definitely not correct - "(any character set)" should just
> be deleted.  This text was intended to refer to what Unicode calls a
> "script" and it's probably better to just not mention that concept here.
>
>>>    [Mallikarjun:] The entire iSCSI Name discussion is in the context of
>> Unicode, and it is called out in normative sentences in appropriate places.
>> This is non-normative descriptive text. OTOH, if you have specific
>> suggestions, they are welcome.
> Unfortunately, that's not quite right, because the actual specification
> is what RFC 3722 allows via the tables in RFC 3454.  Those RFC 3454 are
> definitive, as opposed to the Unicode specifications of character
> properties (which were used to generate those tables).
>
> The following text should get this right:
>
>       names are generated using only characters allowed as output by
>       the stringprep processing specified in [RFC3722].  Those allowed
>       characters include all ASCII lowercase and numeric characters,
>       as well as lowercase Unicode characters as specified in [RFC3722].
This new text is better and addresses my concern. (The whole paragraph 
is sort of "if you are too lazy to implement StringPrep, this is what 
you can safely use", but as long as this is correct, I don't mind.)
> The PRECIS work should allow a future update that uses the Unicode
> "Lowercase" property (and other Unicode) properties.
Right.

---

From: Mallikarjun Chadalapaka <[email-masked]>
Date: Thu, 4 Oct 2012 17:34:17 +0000

Thanks Alexey for the careful review. I will study your comments and respond in a day or two.

BTW, I just published a -07 spec two days ago addressing other IESG Last Call comments - the new revision may (or may not) have addressed some of your comments. I'll let you know.

Mallikarjun

-----Original Message-----
From: Alexey Melnikov [mailto:[email-masked]] 
Sent: Wednesday, October 03, 2012 6:55 AM
To: [email-masked]; [email-masked]; [email-masked]
Subject: SecDir and AppsDir review of draft-ietf-storm-iscsi-cons-06

I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG.
These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat these comments just like any other last call comments.

This document describes a transport protocol for SCSI that works on top of TCP.

Summary

Overall this is a very well written document. I also trust that all options were implemented by at least 1 implementation, so the document reflects implementation experience.

The Security Considerations section is thorough. iSCSI security partially relies on security provided by IPSec. Unfortunately I do not have the IPSec expertise to determine if all relevant cases were covered, although the text looked plausible.

Security considerations for CHAP and SRP authentication mechanisms are discussed in details and seem to be adequate. I've not seen any discussion of KRB5 (mentioned in 12.1) related issues though.

I also have other more detailed comments (some of them are security related). They range between minor and nits, although some of them can be reclassified as a bit more important:

In Section 2.2:

 >- SCSI Port Name: A name made up as UTF-8 characters and includes  >  the iSCSI Name + 'i' or 't' + ISID or Portal Group Tag.

"UTF-8 characters" should be something like "UTF-8 [UTF-8] encoding of Unicode characters.
(There is no such thing as "UTF-8 character")

In 4.2.2.3.3:

In 4.2.2.3.4:

  Note:

In Section 4.2.7.1

         iSCSI

In Section 4.2.7.4:

In 4.2.9:

4.3. iSCSI Session Types

Using which key?

In 4.6.1.5:

In 6.1:

        The encoding is done
        according to [RFC2045]

6.2.1. List negotiations

In 6.3:

How?

In 6.3.1:

In 6.3.2:

How can this be done?

In 6.3.5:

In 7.1.4:

Did you mean 7.9 or 7.8?

In 7.2.2:

In 9.2.1:

In 9.3.1:

9.3.2. Confidentiality

   an initiator/target

      bit 3-4 Reserved.

11.4.4. SNACK Tag

s/theS CSI/the SCSI

   this required behavior.

11.9.2. AsyncVCode

How is this decided?

12.1. AuthMethod

      CHAP_A=<A1,A2...>

---

From: Mallikarjun Chadalapaka <[email-masked]>
Date: Tue, 9 Oct 2012 03:42:55 +0000

Mallikarjun

> (There is no such thing as "UTF-8 character")

[Mallikarjun:] OK, fixed it.

> I didn't find iSCS in the list of acronyms.

 [Mallikarjun:] Typo, already fixed in -07.

 [Mallikarjun:] See above

> to distinguish this from various forms of Unicode Normalization, etc.

 [Mallikarjun:] Sounds good

> Nit: What is so "legacy" about these implementations? Please explain or

 [Mallikarjun:] Sure, but that explanation follows after a few sentences, once we set the non-RDMA context here though. If you skip ahead a few sentences, it clearly describes what is *not* a "legacy" implementation as:

Different schemes can be used to recover synchronization. The details of any such schemes are beyond this protocol specification, but it suffices to note that [RFC4297] provides an overview of the direct data placement problem on IP networks, and [RFC5046] specifies a protocol extension for iSCSI that facilitates this direct data placement objective.

 

> I suggest inserting "by the initiator" before "as a protocol error".

 [Mallikarjun:] I assume you mean "by the originator". Yes, we could add it, although IMHO the context makes it obvious.

> Sorry, I didn't get what "Retry an" is.

 [Mallikarjun:] it's already fixed in -07 version (it was a broken cross-ref).

> Nit: Should "an" be dropped?

 [Mallikarjun:] Sure.

> I think TCP keep-alive option needs a reference here.

 [Mallikarjun:] Yes, a reference to RFC 1122 would be good here.

> The first mentioning of RADIUS needs an Informative reference.

 [Mallikarjun:] Sure, it's good.
<snip of security sections related comments>

> Is this a well known term and if it is, what does it mean?

 [Mallikarjun:] This refers to the message corruptions undetected by the TCP checksum algorithm. "checksum escape" is a term at least the ips WG had used extensively in discussion, e.g. http://www.pdl.cmu.edu/mailinglists/ips/mail/msg04095.html 
http://www.ietf.org/proceedings/51/slides/ips-6.pdf 

>    information at theS CSI layer. The rest of the Section elaborates

 [Mallikarjun:] Yes, fixed.

>    supported when the function to which they refer is mandatory to

 [Mallikarjun:] It's called out in respective sections, e.g. for AuthMethod:

"The initiator and target MUST implement CHAP. All other authentication methods are OPTIONAL."
> Is there an IANA registry for this?

 [Mallikarjun:]Yes.  http://www.ietf.org/assignments/iscsi-parameters

---

From: "Black, David" <[email-masked]>
Date: Thu, 11 Oct 2012 09:35:03 -0400

Alexey,

Thanks,
--David
----------------------------------------------------
David L. Black, Distinguished Engineer
EMC Corporation, 176 South St., Hopkinton, MA  01748
+1 (508) 293-7953             FAX: +1 (508) 293-7786
[email-masked]        Mobile: +1 (978) 394-7754
----------------------------------------------------

---

From: "Black, David" <[email-masked]>
Date: Thu, 11 Oct 2012 11:08:13 -0400

Alexey - thank you for the quick response.

I think we're aligned on these stringprep/Unicode concerns,
and this should all get dealt with in the next version of the draft.

I'm working through the rest of your review ... but (IMHO) a serious draft
revision will be needed before it'll make sense for any of the Discuss
positions to be cleared.

Thanks,
--David

> Sent: Thursday, October 11, 2012 9:57 AM
> To: Black, David
> Cc: Mallikarjun Chadalapaka; [email-masked]; draft-ietf-storm-iscsi-
> [email-masked]; [email-masked]
> Subject: Re: Use of StringPrep/Unicode (was Re: SecDir and AppsDir review of
> draft-ietf-storm-iscsi-cons-06)
> 

No problem.

> Ok. This is not what I expected the replacement text to say, but this is

Right - that was a good catch, thank you.

Ok.

> > is what RFC 3722 allows via the tables in RFC 3454.  Those RFC 3454 tables are

Thank you for understanding that.

---

From: "Black, David" <[email-masked]>
Date: Thu, 11 Oct 2012 11:58:57 -0400

I'm picking up three items from your review that still appear to need some
discussion.  We'll plan to add something for most of your requests for
explanations, forward references, and the like that aren't part of these
three.  In addition, the stringprep/Unicode items have been resolved in
a separate email thread.

[1]
>>> discussion of KRB5 (mentioned in 12.1) related issues though.

Ok, we'll do something about KRB5 security considerations, although only
CHAP is implemented and used in practice - I'll respond separately to Sean's
comment about needing to indicate that CHAP will be replaced.  The summary
there that an SRP-like secure password mechanism is probably what's wanted,
but I watched the ipsecme WG completely fail to select a single such
mechanism in the recent past - that's an experience that I don't care to
repeat in the storm WG, especially as storm is not a Security WG ;-).

[2]
>> [Mallikarjun:] LU Reset is a SCSI operation that iSCSI simply acts as
>> a transport for. So as such, iSCSI does not define an access control
>> mechanism for the SCSI-level operation. SCSI specs ([SPC4]) define
>> ACCESS CONTROL OUT and ACCESS CONTROL IN commands to set such access controls.

> This sounds like a security consideration to me. At least it seems worth

Yes - I'll work w/Mallikarjun to add Security Considerations text.
The two ACCESS CONTROL commands that he mentioned are rarely
implemented in practice, so the text will probably be more general
about SCSI-level access control mechanisms and indicate that they need to
cover task management operations such as Logical Unit Reset.

[3]
>   [Mallikarjun:] IMHO, the correct reference to RFC 2045 (which already exists)
> should address this comment. None of the preceding sections cite the respective
> ASCII codes either, e.g. hex-constant, decimal-constant.... While I suppose we
> could reference ASCII codes everywhere, I do not honestly see the necessity in
> this case - as readability will be impacted with lots of such ASCII code
> references on these pages (FYI, no changes have been made from RFC 3720 here).

> think it was updated properly.

It doesn't matter; all the characters involved here are ASCII, hence have the
same Unicode codepoints as ASCII.  I really don't see the need to expand strings
like this by listing the codepoints.  We could say that the entire constant
is ASCII if that helps.

>> Also, this is not quite correct, as leading "=" is not valid in base64

I believe this comment is off the mark, but I don't see a problem here.

What's going on here is that when a base64-constant occurs as the right hand
side of a key=value construct, one can wind up with multiple instances of
'='in that construct:

        - one to separate the key and the value; and
        - one or two at the end of the base64 constant.

The parsers get this right because the "0b" or "0B" after the first '='
indicates that everything that follows is base64, e.g., in
"example=0bZm9vYmE=", the first '=' introduces the value, "0b" flags
the value as base64-encoded, and the second '=' is part of the base64
encoding of the value.  This is unambiguous and works in practice.

Thanks,
--David

> Sent: Tuesday, October 09, 2012 7:13 AM
> To: Mallikarjun Chadalapaka
> Cc: [email-masked]; [email-masked];
> [email-masked]
> Subject: Re: SecDir and AppsDir review of draft-ietf-storm-iscsi-cons-06
>
>
> > - I am attaching a candidate -08 revision addressing most of your comments.
> See the change bars/red text to see what's changed.
> >
> > - I think some of your comments are more about the current style of iSCSI
> spec. The spec by design introduces a concept in non-normative text to
> illustrate the idea, and then in a subsequent section/paragraph proceeds to
> specify normative requirements. I see you have a bunch of comments that you
> noted on the non-normative text, and almost all of them are addressed in
> subsequent normative sections. You'll find my responses on each in line, when
> that is the case.
> > [Mallikarjun:] LU Reset is a SCSI operation that iSCSI simply acts as a
> transport for. So as such, iSCSI does not define an access control mechanism
> for the SCSI-level operation. SCSI specs ([SPC4]) define ACCESS CONTROL OUT
> and ACCESS CONTROL IN commands to set such access controls.
> > [Mallikarjun:] Because some steps may not be backwards-compatible with RFC
> 3720-compliant implementations. That's the reason or the immediately following
> "MUST" statement - to make the distinction between the two clear.
> >   [Mallikarjun:] Not important, it's normalized to lower-case via stringprep
> for key names, and for key values, we explicitly allow upper and lower case.
> >   [Mallikarjun:] Agreed that the key could have been specified here as well,
> but note that this is the non-normative descriptive text covering the concept
> (Section 6.3 covers it). We could add it here as well.
> >   [Mallikarjun:] IMHO, the correct reference to RFC 2045 (which already
> exists) should address this comment. None of the preceding sections cite the
> respective ASCII codes either, e.g. hex-constant, decimal-constant.... While I
> suppose we could reference ASCII codes everywhere, I do not honestly see the
> necessity in this case - as readability will be impacted with lots of such
> ASCII code references on these pages (FYI, no changes have been made from RFC
> 3720 here).
> >   [Mallikarjun:] Sorry, I do not follow this. Leading "=" is not required
> for base64-constant by iSCSI. Are you referring to the usage of "key=value"?
> When used as a key value, the "=" sign is interpreted by iSCSI key parsing
> logic as for other keys, it's not considered as part of a base64 value.
> >   [Mallikarjun:] Nope, it is not an empty list. When both sides agree to it,
> it means that no key-specific semantics are operational for the functionality
> represented by the key. Look at 12.1 (AuthMethod), and 13.1 (Digests) for
> examples.
> >   [Mallikarjun:] Sure, look at Section 13.10 (InitialR2T), as an example.
> For this key, if one side proposes "Yes", the result is guaranteed to be
> "Yes", so the responder does not need to respond.
> Ok.
> >> In 6.3:
> >>
> > The SecurityNegotiation keys appear in Section 12 and the
> LoginOperationalNegotiation keys appear in Section 13. Only a limited set of
> keys (marked as Any-Stage in Section 13) may be used in any of the two stages.
> >
> > In general, the login semantics of iSCSI are fairly complex, and the team of
> authors has re-written/re-ordered this text a handful of times during the
> original RFC 3720 work (with a *lot of help* from WG, :) ) So I have taken
> care not to preserve the original RFC 3720 text verbatim here.
> >
> > Stage transition is performed through a command exchange (request/response)
> that carries the T bit and the same CSG code. During this exchange, the next
> stage is selected by the target through the "next stage" code (NSG). The
> selected NSG MUST NOT exceed the value stated by the initiator. The initiator
> can request a transition whenever it is ready, but a target can only respond
> with a transition after one is proposed by the initiator.
> Ok.
> >   [Mallikarjun:] MUST be ignored on reception and SHOULD be set to 0 when
> sending. Sections 11.13.6 and 11.13.7 outline the precise normative semantics
> for setting these fields.
> >
> >> In 6.3.2:
> >>
> >   [Mallikarjun:]  Target can propose a new security key while responding to
> a Login Request by setting CSG,NSG=0,0 T=0 (See Appendix B where a precise
> example for this scenario is included).
> >   [Mallikarjun:] Negotiated or specified (see the DefaultTime2Retain text
> key, and the Time2Retain field in Login Response PDU).
> Ok.
> >> In 7.1.4:
> >>
> > [Mallikarjun:] I did mean section 7.9, :-) Section 7.9 discusses the meta
> problem of a sequence error which is the right reference here. Note that
> handling sequence errors may require digest error recovery, so it defers to
> Section 7.8 as appropriate.
> Ok, never mind then.
>   [...]
> >   [Mallikarjun:] I agree this usage reads a little broken, and this is
> because we have removed some sections that were normative in RFC 3720, and I
> should have included a bridging sentence in section 4.2.9, e.g.
> >
> > Different schemes can be used to recover synchronization. The details of any
> such schemes are beyond this protocol specification, but it suffices to note
> that [RFC4297] provides an overview of the direct data placement problem on IP
> networks, and [RFC5046] specifies a protocol extension for iSCSI that
> facilitates this direct data placement objective. Rest of this document refers
> to any such direct data placement protocol usage as an example of a "Synch and
> Steering layer".
> Looks good to me.
> >>     an initiator/target
> >   [Mallikarjun:] This refers to the message corruptions undetected by the
> TCP checksum algorithm. "checksum escape" is a term at least the ips WG had
> used extensively in discussion,
> e.g.http://www.pdl.cmu.edu/mailinglists/ips/mail/msg04095.html
> > IPS WG also had published an informational RFC (RFC 3385) to cover this
> entire area of checksums and CRCs and undetected errors.
> >
> > Having said that, it is probably a good idea to make this explicit in the
> doc, e.g. " "checksum escape" (message error undetected by TCP checksum)."
> >   [Mallikarjun:] As responded to earlier, it's a SHOULD-MUST. I think it's
> worth adding the following at the end of section 4.2.2.1 :
> >
> > In the rest of the document, unless otherwise explicitly stated, we use the
> term "reserved" for a PDU field to indicate that the field MUST be ignored on
> reception and SHOULD be set to 0 when sending.
> Yes!
> >> 11.4.4. SNACK Tag
> >>
> >   [Mallikarjun:] It's a reference to section 4.6. 3.4 as you can see in pdf,
> but it isn't proper, :-) Requires fixing, actually 11.16.3 is a better
> reference.
> Ok, great.
> >>     this required behavior.
> >>
> >> 11.9.2. AsyncVCode
> >>
> >   [Mallikarjun:] No, it's a vendor-unique usage field that by definition may
> not work across vendors, and that fact should not affect interoperability.
> Ok.
>   [...]
> >> 12.1. AuthMethod
> >>
> >   [Mallikarjun:] "Offer" has a stronger implication for iSCSI than "support"
> -e.g. an implementation may support a method, but not necessarily offer it.
> OTOH, when an implementation offers a semantic, it means it's supported and is
> ready to use it for the specific session.

---

From: Alexey Melnikov <[email-masked]>
Date: Fri, 12 Oct 2012 15:53:29 +0100

Hi David,
> repeat in the storm WG, especially as storm is not a Security WG ;-).
Understood. I just pointed out that you said nothing about KRB5, while 
you said something about CHAP & SRP.
Ok.
> like this by listing the codepoints.  We could say that the entire constant
I was speaking more generally about this section of the document.
> encoding of the value.  This is unambiguous and works in practice.
"example=0bZm9vYmE=" is valid, but something like 
"example=0bZm9vYmE=AAAA" is not (because the base64 value is not 
syntactically valid). Anyway, my point was that you should point to the 
RFC defining base64 and don't repeat information from it here.

---

From: Mallikarjun Chadalapaka <[email-masked]>
Date: Sun, 14 Oct 2012 23:12:40 +0000

> I think a forward reference here would be good.

Done.

> > This sounds like a security consideration to me. At least it seems
> > worth pointing this out.
> 
> implemented in practice, so the text will probably be more general about
> SCSI-level access control mechanisms and indicate that they need to cover
> task management operations such as Logical Unit Reset.

Tend to agree with David.  Here's some new text that might work.

9.5. SCSI Access Control Considerations
iSCSI is a SCSI transport protocol and as such does not apply any access controls on SCSI-level operations such as SCSI task management functions (e.g. LU Reset, see Section 11.5.1). SCSI-level access controls (e.g. ACCESS CONTROL OUT, see [SPC3]) have to be appropriately deployed in practice to address SCSI-level security considerations, in addition to security at iSCSI connection and packet protection mechanisms that were already discussed in preceding Sections.

> > [Mallikarjun:] Because some steps may not be backwards-compatible with
> RFC 3720-compliant implementations. That's the reason or the immediately
> following "MUST" statement - to make the distinction between the two
> clear.

Protocol behavior defined in this Section SHOULD be implemented by all iSCSI implementations complying with this document, noting that some steps below may not be compatible with [RFC3720] semantics. However, protocol behavior defined in this Section MUST be exhibited by iSCSI implementations on an iSCSI session when they negotiate the TaskReporting (Section 13.23) key to "FastAbort" on that session.

> If it is expired, it is no longer owned, which conflicts with the previous
> sentence ;-).

Fair enough. I removed this phrase now, so the sentence reads: 

This domain name does not have to resolve to an address; it just needs to be reserved to prevent others from generating iSCSI names using the same domain name.

> I don't mind either way, I am just missing where this is explicitly said.

Hexadecimal values can occur in "iSCSI names" - e.g. "naa" names. (section  4.2.7.3).
Section 4.2.7.2 - iSCSI name encoding section covers this.

When hexadecimal values show up in key values, the text format discussion (section 6.1) makes it clear that either upper case or lower case is fine - see the definition for hex-constant.

> >   [Mallikarjun:] Nope, it is not an empty list. When both sides agree to it, it
> means that no key-specific semantics are operational for the functionality

Sure, here's text I have added now to complete that paragraph:

The constant "None" MUST always be used to indicate a missing function. However, "None" is only a valid selection if it is explicitly proposed. When "None" is proposed as a selection item in a negotiation for a key, it indicates to the responder that not supporting any functionality related to that key is legal, and if "None" is the negotiation result for such a key, it means that key-specific semantics are not operational for the negotiation scope (connection or session) of that key.

> FYI, unqualified word "security" is a red flag during SecDir review ;-).

Fair enough, I have re-crafted that sentence to the following:

Authentication-related security keys (Section 12 ) MUST be completely negotiated within the Login Phase.

> a Login Request by setting CSG,NSG=0,0 T=0 (See Appendix B where a
> precise example for this scenario is included).

Done

> Forward reference is missing :-).

OK

> > Having said that, it is probably a good idea to make this explicit in the doc,
> e.g. " "checksum escape" (message error undetected by TCP checksum)."

Done.

> one elsewhere in the document) would help.
Sure, for SRP and CHAP.

> Sent: Tuesday, October 09, 2012 4:13 AM
> > - I am attaching a candidate -08 revision addressing most of your
> comments.  See the change bars/red text to see what's changed.
> >
> specify normative requirements. I see you have a bunch of comments that
> you noted on the non-normative text, and almost all of them are addressed
> in subsequent normative sections. You'll find my responses on each in line,
> when that is the case.
> >> options were implemented by at least 1 implementation, so the
> >> document reflects implementation experience.
> >>
> >> partially relies on security provided by IPSec. Unfortunately I do
> >> not have the IPSec expertise to determine if all relevant cases were
> >> Security considerations for CHAP and SRP authentication mechanisms
> >> are discussed in details and seem to be adequate. I've not seen any
> >> related). They range between minor and nits, although some of them
> >> can be reclassified as a bit more important:
>   [...]
> >> In 4.2.2.3.3:
> >>
> for the SCSI-level operation. SCSI specs ([SPC4]) define ACCESS CONTROL
> OUT and ACCESS CONTROL IN commands to set such access controls.
> > [Mallikarjun:] Because some steps may not be backwards-compatible with
> RFC 3720-compliant implementations. That's the reason or the immediately
> following "MUST" statement - to make the distinction between the two
> clear.
>   [Moving this to another thread.]In Section 4.2.7.4: To generate names of
> this type, the person or organization generating the name must own a
> If it is expired, it is no longer owned, which conflicts with the previous
> sentence ;-).
> suppose we could reference ASCII codes everywhere, I do not honestly see
> the necessity in this case - as readability will be impacted with lots of such
> ASCII code references on these pages (FYI, no changes have been made
> from RFC 3720 here).
> Unicode. The text is written as if it is talking about ASCII and I don't think it
> was updated properly.
> >> Also, this is not quite correct, as leading "=" is not valid in
> >> base64 encoding, etc.
> >   [Mallikarjun:] Sorry, I do not follow this. Leading "=" is not required for
> base64-constant by iSCSI. Are you referring to the usage of "key=value"?
> Basically I would like for your document to be more strict about defining
> which data is valid.
> >>          The encoding is done
> >>          according to [RFC2045]
> >>
> >> Please reference "Section 4 of [RFC4648]" instead of RFC 2045. RFC
> >> 4648 is the most recent RFC for base64 definition.
> >   [Mallikarjun:] Nope, it is not an empty list. When both sides agree to it, it
> means that no key-specific semantics are operational for the functionality
> >   [Mallikarjun:] Sure, look at Section 13.10 (InitialR2T), as an example. For
> this key, if one side proposes "Yes", the result is guaranteed to be "Yes", so
> the responder does not need to respond.
> Ok.
> >> In 6.3:
> >>
> original RFC 3720 work (with a *lot of help* from WG, :) ) So I have taken care
> not to preserve the original RFC 3720 text verbatim here.
> >
> > Stage transition is performed through a command exchange
> (request/response) that carries the T bit and the same CSG code. During this
> exchange, the next stage is selected by the target through the "next stage"
> code (NSG). The selected NSG MUST NOT exceed the value stated by the
> initiator. The initiator can request a transition whenever it is ready, but a
> target can only respond with a transition after one is proposed by the
> initiator.
> Ok.
> >   [Mallikarjun:] MUST be ignored on reception and SHOULD be set to 0
> when sending. Sections 11.13.6 and 11.13.7 outline the precise normative
> semantics for setting these fields.
> >
> >> In 6.3.2:
> >>
> a Login Request by setting CSG,NSG=0,0 T=0 (See Appendix B where a
> precise example for this scenario is included).
> >   [Mallikarjun:] I agree this usage reads a little broken, and this is because
> we have removed some sections that were normative in RFC 3720, and I
> > Different schemes can be used to recover synchronization. The details of
> any such schemes are beyond this protocol specification, but it suffices to
> note that [RFC4297] provides an overview of the direct data placement
> problem on IP networks, and [RFC5046] specifies a protocol extension for
> iSCSI that facilitates this direct data placement objective. Rest of this
> document refers to any such direct data placement protocol usage as an
> example of a "Synch and Steering layer".
> Looks good to me.
> >>     an initiator/target
> TCP checksum algorithm. "checksum escape" is a term at least the ips WG
> had used extensively in discussion,
> > Having said that, it is probably a good idea to make this explicit in the doc,
> e.g. " "checksum escape" (message error undetected by TCP checksum)."
> >   [Mallikarjun:] It's a reference to section 4.6. 3.4 as you can see in pdf, but
> it isn't proper, :-) Requires fixing, actually 11.16.3 is a better reference.
> Ok, great.
> >>     this required behavior.
> >>
> >> 11.9.2. AsyncVCode
> >>
> >   [Mallikarjun:] "Offer" has a stronger implication for iSCSI than "support" -
> e.g. an implementation may support a method, but not necessarily offer it.
> OTOH, when an implementation offers a semantic, it means it's supported
> and is ready to use it for the specific session.

---

From: Alexey Melnikov <[email-masked]>
Date: Tue, 16 Oct 2012 19:06:11 +0100

Hi,
Pretty much agreeing with all you proposed.

Ok.
> Protocol behavior defined in this Section SHOULD be implemented by all iSCSI implementations complying with this document, noting that some steps below may not be compatible with [RFC3720] semantic…
Looks good.
> This domain name does not have to resolve to an address; it just needs to be reserved to prevent others from generating iSCSI names using the same domain name.
Yes, this is good. Thanks.
Ok.
> The constant "None" MUST always be used to indicate a missing function. However, "None" is only a valid selection if it is explicitly proposed. When "None" is proposed as a selection item in a nego…
Perfect, thanks :-).
> Authentication-related security keys (Section 12 ) MUST be completely negotiated within the Login Phase.
Much better, thanks!