FCIP Last Call technical comments

[Black_David@emc.com]

[T] Technical comments only.  Editorial comments have been sent directly
to the document editor.

----- FC over TCP/IP -09 -----

-- Section 6.6 - FCIP Data Engine (FCIP_DE)

   Table 2 shows the SOF and EOF code values that are legal in FCIP
   Frames. This list may be a subset of the SOF and EOF codes listed in
   the FC Frame Encapsulation [27].

[T] This is a problem because these codes are being specified in more
than one place.  I think the FC Frame Encapsulation document is the right
place for the normative specification of these codes (and see my comments
against it on the need to get IANA involved).  This would be ok as a list
of codes that are currently valid, but the specification authority needs
to be in one place.

-- Section 6.6.2.2 - Errors in FCIP Headers and Discarding FCIP Frames

   In addition to the tests above, the validity and positioning of the
   following FCIP Frame information SHOULD be used to detect
   encapsulation errors that may or may not affect synchronization:
 
   a)  Protocol # field and its ones complement (2 tests);
   b)  Version field and its ones complement (2 tests);
[... list continues, snipped ...]

[T] I think at least these two are MUSTs.  At a minimum, the Protocol#
and Version field must be checked against expected values - I might accept
the checks against their ones complements being SHOULDs.  Same comment
applies to the Flags field and SOF.  Someone MUST check the FC frame CRC
before forwarding the frame, but that responsibility could be assigned
to the FC Entity.

   At least 5 of the 18 tests listed above SHALL be performed. Failure
   of any of the above tests actually performed SHALL indicate an
   encapsulation error and the frame SHALL NOT be forwarded on to the
   FC Entity. Further, such errors SHOULD be considered carefully,
   since some may be synchronization errors.

[T] There aren't 18 tests, and I think some of the individual tests (or
subsets) are MUSTs (see previous comment).  This needs to be gone over
carefully - in essence a MUST is only appropriate where failure to apply
the test carries a non-negligible risk of forwarding a bad frame to FC.
The authors are the experts on this and need to do this analysis.  I
don't understand the last "SHOULD" - what is the (testable) requirement
on an implementation?

-- Section 6.6.2.3 - Synchronization Failures

   If the FCIP_DE attempts to recover synchronization, the
   resynchronization algorithm used SHALL meet the following
   requirements:

[T] One requirement is missing, which may be part of b):

   b)  return to sending valid FC Frames only after synchronization has
       been verified; and

[T] Verification of synchronization MUST exclude any possibility of
forwarding an FC frame that was not sent by the transmitting FCIP Entity.
This includes the scenario in which a valid encapsulated
FCIP frame occurs in the payload of an FC frame that is encapsulated
and sent over FCIP; excluding this scenario is necessary but not
sufficient to meet the requirement.

   An example algorithm meeting these requirements can be found in
   annex C.

[T] That doesn't meet the missing requirement that my
above comment wants to add.  The problem is at step 2 of the algorithm
description.

   2)  Use multiple strong candidate headers to locate a verified
       candidate header:
 
       The Frame Length in one strong candidate header is used to skip
       incoming bytes until the expected location of the next strong
       candidate header is reached. Then the tests described in step 1)
       are applied to see if another strong candidate header has
       successfully been located.

The "skip incoming bytes" step makes it possible that a set of valid
FC headers is interlaced in the payloads of FC frames in a fashion
that causes all the real headers to be skipped.  This is a rather
unlikely, but not impossible scenario.  This could be dealt with by
searching for headers instead of skipping data and aborting if a
header is found where data should have been or carrying on and
aborting if an interlaced header chain scenario arises.  The algorithm
in Annex C does address the scenario of FCIP frames occurring in
FC frame payloads, but it doesn't meet the "can't be fooled"
requirement that I think should be added.

Unfortunately, this issue appears to not be resolvable within the WG.
I have had heated and lengthy offline discussions with the FCIP Authors
in which they have made clear their strong opposition to the "missing
requirement" and resulting scan rather than skip of data, and have
argued that the algorithm in Annex C produces a long enough chain of
headers that the odds of having followed a chain that is interlaced
through frame payloads is small enough to be ignored.  I will have to
consult the Area Directors.

-- Section 7 - Checking FC Frame Transit Times in the IP Network
 
[T] I don't believe that this paragraph meets the requirements in the
FC Frame Encapsulation specification for processing transit time
information.  Punting this up to the FC Entity is problematic -
the minimum functional requirements on the FC Entity to meet the
FC Frame Encapsulation requirements will need to be spelled out here
in detail (i.e., a single sentence saying "must meet requirements in
Section 4 of the FC Frame Encapsulation document" is probably not
going to fly).  Mallikarjun caught some issues in this area also.

-- Section 8 - The FCIP Special Frame

[T] How is the FCIP Special Frame payload protected?  I don't see the
equivalent
of an FC Frame CRC.

   The Connection Usage Code field contains Fibre Channel defined
   information regarding the intended usage of the connection as
   specified in FC-BB-2 [4].

[T] The authors need to talk to me about this, so that I can understand
what's going on here, and whether we need another IANA registry, as is
the case for the SOF and EOF values.

[T] This section needs to describe the usage of the FCIP Special Frame,
including the structure of the interaction between the two FCIP Entities,
and how that establishes the security and connection usage properties
that are desired.  The descriptions in Section 9 contain a great deal
of detail that mixes several mechanisms together - a high level guide
to what's going on is necessary to understand them.

-- Section 9.1.2.1 - Non-Dynamic Creation of New TCP Connections

   If the TCP connect request is rejected, the FCIP Entity SHALL act to
   limit unnecessary repetition of attempts to establish similar
   connections.

[T] That's a little vague.  How about specifying a minimum time period
that MUST elapse before retry?

   It is recommended that an FCIP Entity not initiate TCP connect
   requests to another FCIP Entity if incoming TCP connect requests
   from that FCIP Entity have already been accepted.

[T] Needs an upper case "SHOULD" or "RECOMMENDED".  This also needs
a MUST or SHOULD on the configuration mechanism to indicate in which
direction connections are to be established between a pair of FCIP
Entities in order to deal with a problem that turns up near the end
of Section 9.1.3.  This is related to Mallikarjun's issue about
handling of simultaneous opens.

-- Section 9.1.2.3 - Connection Setup After a Successful TCP Connect Request

[T] This dives into the details of FCIP Special Frame handling without
explaining
the overall structure and goals of the use, and is unclear as a result.  For
example, For example, on p. 29, after constructing the FCIP Special Frame,
the
text says:

   After the FCIP Special Frame bytes are sent on the newly formed
   connection, the FCIP Entity SHALL wait for the FCIP Special Frame to
   be echoed as the first bytes received on the newly formed connection.

But one has to wade all the way down to p.33 towards the end of Section
9.1.3
to discover that the other FCIP Entity is expected to perform validation
actions
before echoing the frame.  The structural outline of the usage of the FCIP
Special Frame (without the blow by blow details) needs to be described in
Section 8.

   The remaining steps in this section SHALL be performed only if the
   echoed FCIP Special Frame bytes exactly match the FCIP Special Frame
   bytes sent (words 7 through 17 inclusive).

-- Section 9.1.3 - Processing Incoming TCP Connect Requests

   If the requested connection is
   allowed, the FC Entity SHALL ensure that required IP security
   features are enabled and accept the TCP connect request.

[T] As written, this appears to require a dynamic interrogation of the IPsec
Security Association Database, and possibly the IPsec Security Policy
Database.
I suspect that this is in excess of what was intended, and suspect a longer
description is needed.

   If the Destination FC Fabric Entity World Wide Name contains 0, the
   FCIP Entity SHALL take one of the following three actions:
 
   1)  Leave the Destination FC Fabric Entity World Wide Name field and
       Ch bit both 0;
   2)  Change the Destination FC Fabric Entity World Wide Name field to
       match FC Fabric Entity World Wide Name associated with the FCIP
       Entity that received the TCP connect request and change the Ch
       bit to 1; or
   3)  Close the TCP Connection without sending any response.
 
   The choice between the above actions depends on the anticipated
   usage of the FCIP Entity and is outside the scope of this
   specification.  The FCIP Entity may consult the "shared" database
   when choosing between the above actions.

[T] I'm not buying the "outside the scope" disclaimer.  Some more
description
of why the three choices are available is in order even if the normative
criteria for choosing among them are specified in FC-BB-2.  If my assumption
about FC-BB-2 is correct, the last sentence is almost certainly too weak -
it needs to refer to consulting the FC Entity to determine what to do.

   If:
   a)  The Destination FC Fabric Entity World Wide Name contains a non-
       zero value that does not match the FC Fabric Entity World Wide
       Name associated with the FCIP Entity that received the TCP
       connect request, or
   b)  The contents of the Connection Usage Flags, and Connection Usage
       Code fields is not acceptable to the FCIP Entity that received
       the TCP connect request,
   then the FCIP Entity SHALL take one of the following two actions:
   1)  Change the contents of the unacceptable fields to correct/
       acceptable values and set the Ch bit to 1; or
   2)  Close the TCP Connection without sending any response.

[T] 1) looks like a security hole that discloses information an attacker
may find useful in establishing an undesired connection to FCIP.  What's
the motivation/purpose for this?

   If the Source FC Fabric Entity World Wide Name and Source FC/FCIP
   Entity Identifier field values in the FCIP Special Frame match the
   Source FC Fabric Entity World Wide Name and Source FC/FCIP Entity
   Identifier associated with an existing FCIP_LEP, the FCIP Entity
   SHALL:
 
   1)  Request that the FC Entity authenticate the source of TCP
       connect request, providing the following information to the FC
       Entity for authentication purposes:

[T] Need to say more about what the FC Entity MUST do to "authenticate the
source".
I realize that the details are specified in FC-BB-2, but the functional
requirements on the FC Entity can be specified here.

       The FCIP Entity SHALL wait indefinitely for the FC Entity to
       authenticate source of the TCP connect request and SHALL not use
       the new TCP Connection for any purpose until the FC Entity
       completes the authentication.

[T] "wait indefinitely" creates an obvious denial of service attack
vulnerability.  Try something else.

   Note that the originator of TCP connect requests uses IP Address and
   TCP Port to identify which TCP Connections belong to which FCIP_LEPs
   while the recipient of TCP connect requests uses the Source FC
   Fabric Entity World Wide Name, Source FC/FCIP Entity Identifier
   fields from the FCIP Special Frame to identify which TCP Connection
   belong to which FCIP_LEPs.  For this reason, an FCIP Entity that both
   originates and receives TCP connect requests is unable to match the
   FCIP_LEPs associated with originated TCP connect requests to the
   FCIP_LEPs associated with received TCP connect requests.

[T] That's a problem.  See comment against Section 9.1.2.1 for one
suggestion
for how to fix this, but some sort of fix appears necessary to me.

-- Section 9.3.4 TCP_NODELAY Option
 
   FCIP Entities SHALL set the TCP_NODELAY option to one. This will
   disable the Nagle Algorithm that is designed for usage in a telnet
   environment.

[T] I believe that "SHALL" should be a lower case "should".  This is a local
performance optimization that has no other effects.

-- Section 9.5 - Flow Control Mapping between TCP and FC

   Coordination of these flow control mechanisms one of which is credit
   based and the other of which is window based depends on painstaking
   design that is outside the scope of this specification.

[T] This is content-free.  At least warn about some of the things that
can go wrong, in particular BB-credit starvation and the potential to
really screw up a Fibre Channel fabric if this is long-lived.

-- Section 10 Security

[T] Need to get this whole section aligned with the latest (currently -11)
version of the IPS Security draft.

-- Section 10.3.1 - IPSec ESP Authentication and Confidentiality
 
   FCIP Entities MUST implement IPSec ESP [16] in Tunnel Mode for
   providing Data Integrity and Confidentiality. FCIP Entities MAY
   implement IPSec ESP in Transport Mode, if deployment considerations
   require use of Transport Mode.

[T] Those deployment considerations include RFC 2401 requirement for
Transport mode because the IPsec implementation is a host implementation
rather than a security gateway.  I thought this was understood by the FCIP
authors, and it needs to be explicit here including an appropriate reference
to RFC 2401.  I expect to be able to double-check this requirement with the
IETF Security Area in the next few days.

-- Section 10.3.2 - Key Management

   When pre-shared keys are used, IKE Aggressive Mode SHOULD be used
   and Main Mode SHOULD NOT be used.

[T] I think this is too strong and will cause problems.  Pre-Shared keys
are a "MUST", Aggressive Mode is a "MAY", so this "SHOULD" is inconsistent.
The issue with Main Mode arises when dynamically assigned IP addresses are
used (and hence Main Mode can't figure out which pre-shared key to use).
The escape from this box appears to be that Aggressive Mode is a MUST
(SHOULD?) when dynamic assignment of IP addresses to FCIP implementations
is used, but support for dynamic assignment of IP addresses is NOT REQUIRED.

The problem with this approach is that one gets into trouble on one end
of an FCIP Link when the *other* end has its IP address dynamically
assigned.  The obvious solutions to this issue are to forbid (MUST NOT)
dynamic IP address assignment (which has no chance of making it through
the IESG) or to REQUIRE Aggressive Mode.  In addition, the IPS Security
draft appears to need some editing to allow Aggressive Mode to be a "MAY"
for FCIP (and iFCP).  Darn - I thought we had this issue closed in
Huntington
Beach -- I didn't pay enough attention to the details of what we did at
the very end of the security session.

	Support for IKE Oakley Groups is not required.

[T] What does this refer to?  At a minimum, it needs a reference.

   For the purposes of establishing a secure FCIP Link, the two
   participating FCIP Entities consult a Security Policy Database
   (SPD).

-- Section 10.4.2 - TCP Connection Security Associations (SAs)
 
   For a TCP Connection establishment, IKE Phase 2 is employed,
   resulting in an SA, identified by an SPI. All IP datagrams of the
   TCP Connection MUST carry an ESP header with a valid SPI and
   Sequence Number to be accepted as valid by the receiving peer.

[T] The requirement for a phase 2 SA per TCP connection has been removed.
This text (and possibly the rest of this section) need some editing to
reflect that.

-- Section 10.4.3 - Handling data integrity and confidentiality violations
 
   Confidentiality checks MUST be performed if Confidentiality is
   enabled.

[T] And what would those be, please?  Replace this with a prohibition on
use of Confidentiality without Integrity.

-- Section 10.4.4 - Handling SA parameter mismatches
 
   When SA parameters do not match, the TCP Connection may reach a
   point where no traffic moves, or there are excessive TCP
   retransmissions. In such a case, either side MAY take one of the
   following actions:
   a)  Reestablish another set of SA parameters; or
   b)  Close the TCP Connection and notify the FC Entity with the
       reason for the closure.

[T/E] Needs some more explanation, including an example of the sort of
mismatch that could cause this problem.  Recall that IKE negotiates SA
parameters, and this fact needs to be included in the explanation and
example.

-- Section 11.2 - IP Quality of Service (QoS) Support

   If diffserv/PHB QoS is NOT implemented, the DSCP field for all IP
   packets SHALL be set to '000000'.

[T] Not a good idea.  That's not consistent with RFC 2474.  Best
bet is to drop this sentence, but if the authors want to say something
here, they should contact me directly to discuss/vet appropriate text

-- Section 12 - Normative References

[E] This needs to be carefully checked to minimize normative references.
[7] is definitely non-normative.  Most of the QoS references are or can
be non-normative, specifically [11], [22], [23], [24]).  [22] is
an Informational RFC and hence has to be referenced in a non-normative
fashion, and I really want to see [23] and [24] be non-normative (else
any FCIP implementation will have to implement both EF and AF, which
is surely not what was intended).  Need to add a non-normative MPLS
reference.

-- Section 14 - Acknowledgments
 
[E] This is a good place to thank everyone who's reviewed the document,
commented on ideas in it, or helped in other ways.
 
-- Section 15 - Contributors' Addresses
 
We'll try this structure of not separating the folks listed on p.1 from
the other contributors and see if there are any procedural objections.
It's unusual to say the least.

-- Annex A - IANA Considerations

[T] Instruct IANA to change the authority for those port allocations to
reference
this RFC when it is approved.  Add a sentence forbidding the use of UDP with
FCIP even though IANA has allocated a port.

[T] Will need to reference the SOF/EOF registry that the FC Frame
Encapsulation
Draft will need to set up.  Point to the Protocol# allocation done by that
draft also.  If a connection usage registry is needed (see comment against
Section 8, details of that will have to go here).

[E] Should the ANNEXes be APPENDIXes instead?

-- Annex C - Example of synchronization recovery algorithm

[T] See comment on this Annex under Section 6.6.2.3 above.

-- Annexes E, F, and G

I skipped reviewing these three annexes based on the assumption that they
correctly reproduce information specified elsewhere, and the amount of time
I've already spent on this review.


---------------------------------------------------
David L. Black, Senior Technologist
EMC Corporation, 42 South St., Hopkinton, MA  01748
+1 (508) 249-6449 *NEW*      FAX: +1 (508) 497-8500
black_david@emc.com         Cell: +1 (978) 394-7754
---------------------------------------------------