[Gen-art] Gen-art review of draft-ietf-btns-connection-latching-08.txt

[Elwyn Davies <[e-mail]>]

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-btns-connection-latching-08.txt
Reviewer: Elwyn Davies
Review Date: 24 February 2009
IETF LC End Date: 24 February 2009
IESG Telechat date: 26 February 2009

Summary:
Not ready.  I don't have any problems with the latching concept and how
it is set out here.  I think the major concern is that the draft seems
to be saying that it is addressing the application API needed to support
the connection whereas it only addresses linkage between the ULP and
IPsec within the protocol stack.  If I understand correctly it is
effectively supporting the real application API defined in
draft-ietf-btns-abstract-api, but this is only mentioned in the very
last lines of the draft, and there is no apparent attempt to coordinate
with that draft.  At the moment that draft is in a very rudimentary
form, and I feel that it would be extremely premature to turn this draft
into an RFC: of course the WG and the ADs may have different ideas, in 
which case their views will prevail.

In relation to coordination I think that it is necessary to examine
more closely how the application and the ULP work together to control
and integrate with the IPsec component - at the moment there are some
rather vague statements that indicate that the application will get some
notification even when the ULP is doing the active control of the
connection.


Major issues
General: The abstract claims that the draft abstractly  defines how to
interface ULPs *and applications* to IPsec.  Para 3 of s1 seems to imply
that we should expect to see here defined an abstract interface that
could be converted into a implementation that an *application* could use
to control/monitor the usage of IPsec in creating 'channels'.  In
actuality, the interfaces that are defined are linkages between the ULPs
in the kernel and the IPsec implementation in the kernel.  It is not
clear how this would be extended to the application itself, although
some of the definitions imply that the notifications will be propagated
to the application and the latch creation functions pass security
parameters to the latch machine.  The existing (basic) socket AP doesn't
provide any help here. The implication is that the semantics of some of
the socket interface will be extended, but I believe this draft misses
its avowed aim of defining/improving the API for *applications* to make
use of IPsec.  But then I read right to the end and find that maybe this
isn't the aim after all, but just to support the main API  document in
draft-ietf-btns-abstract-api.  It maybe that coordination with this
draft (and more mention of it) is what is needed.

s2.2, CREATE_CONNECTION_LATCH:  Is it intended that this function should
operate synchronously.. i.e., it doesn't return until the child SAs are
in place and the latch is in ESTABLISHED state (or BROKEN if the
creation fails)?  If not what state is the latch in when the function
returns? BROKEN, maybe?

s5.4:  If I read this correctly, it implies that any one of the
connection latches becoming BROKEN triggers is likely to trigger
termination of the whole SCTP conection, and at the very least the
application should be informed that a component connection has broken..
This seems to run counter to the idea of SCTP, that it should continue
to function with the remaining intact connections until it runs out of 
connections, and that the multiplicity of connections is hidden from the 
application.

s6.1 (and elsewhere):  Connection latching in its basic form is
described as a 'passive feature'.  This is true in the sense that the
interaction with the IPsec components is passive monitoring, but it is
not passive as regards packet processing.  Packets can be held up or
dropped if the latch becomes BROKEN. Thus this term may be a little
misleading.  The security considerations needs to be absolutely clear
about any additional opportunities that the breaking of a latch might
give for DoS attacks.  I think the answer is none (apart from possibly
when using SCTP), but I am not a security expert.

Minor issues:
s2, requirements bullets:
    o  Implementations SHOULD create IPsec channels automatically by
       default when the application does not explicitly request an IPsec
       channel.
Is this something an application ought to be able to override? Maybe
needs something like 'If the implementation automatically creates IPsec
channels, then the implementation SHOULD provide a mechanism that allows
an application to override this provided that the Security Policy allows
it.' Maybe this is covereed by the optional features later.. in which 
case a forward ref would help.

s2, next to last para:  It is unclear which of the two models the
various constraints apply to.  I think I might be able to work it out
with a deal of thought, and I am sure I will know by the end of the doc
but it would be good to make it clear here.

s2.2, CREATE_LISTENER_LATCH and CREATE_CONNECTION_LATCH: What is
returned if the creation fails?  There is no success/error value return.
The ULP/application would probably like to know what went wrong.

s2.2, CREATE_LISTENER_LATCH: does the ALERT function provide the
notification?  If so it needs noting in the spec.  See also the comment
about callbacks below.

s2.2, ALERT: Should there be an explicit interface to register the
callback needed to handle the ALERT?  It may just need some weasel words
indicating that ULPs/applications must be able to set themselves up yo
receive the ALERTs.

s2.2, ALERT:  This is an interface between the ULP and the IPsec
mechanisms.  How does it propagate to the application as described
previously.

S2.3: 'as well as effectively caching a subset of the decorrelated SPD
in ULP TCBs.'  I have no idea what is meant by a decorrelated SPD.  It
is not a term I remember from IPsec or IKE, but that may be a memory
lapse.  I think a little explanation is needed.

Nits/editorial comments:
Abstract, para 2: 'as the result of using weak peer identity to peer
address associations.'  I cannot parse this phrase.  s/to/in/ maybe????

s2, para after 1st set of bullets: 'This can be the result of a local
operation (e.g., a connect())'  Probably good to clarify this is
connect() from 'socket' interface'

s2, 2nd set of bullets:
'  o  Quality of protection: cryptographic algorithm suites, key
       lengths, and replay protection;'
This is a bit cryptic.  Something like
'   o  Quality of protection: identity of cryptographic algorithm
        suites used, key lengths, and parameters assocoiated with
        replay protection;'
would be more consistent with the style of the other bullets.

s2, para after 2nd set of bullets: s/The set SAs that protect/The set of
SAs that protect/

s2, next to last requirements bullet: s/potentially large values, such
as BTNS peer IDs/values that potentially have large representations,
such as BTNS peer IDs/

General: Expand NIC, ESP, IKE, AH. SG, IP (3-tuples, 5-tuples - on first
occurrence).  There may be a few more I missed.

s2.1, para 3: s/These states represents an/These states represent an/

s2.1, para 4: s/ownser/owner/