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

From: Elwyn Davies <[email-masked]>
Date: Tue, 24 Feb 2009 16:31:00 +0000

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/

---

From: Nicolas Williams <[email-masked]>
Date: Thu, 26 Feb 2009 14:03:59 -0600

> http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Thanks you for your review and comments.  Replies to the API issues are
below.  I'll reply to the other issues separately.

> which case their views will prevail.

I've answered the concern about the API separately:

 - In a reply to Chris Newman:

|I tend to agree, but note that lack of abstract APIs and programming
|language bindings hasn't stopped TLS nor SASL.
|
|Also, IMO there's enough of a sketch of an abstract API in this draft
|now.
|...
|
|I don't think this I-D should wait for an API I-D.

 - In a reply to Cullen Jennings:

|Solaris already has connection latching today.  It requires no APIs to
|use because it does it by default.  There are APIs available, but they
|are not complete enough.
|
|Solaris does not comply entirely because it only latches IPsec policy
|and does not latch IKE IDs (but it could).

I.e., it's not necessary to have an API in order to benefit from
connection latching.

It's necessary to have an API in order to:

 - do channel binding to IPsec
 - use IPsec for authentication (i.e., use the IDs authenticated by IKE
   for application purposes)

I should add that there are references in the I-D to both, proposed and
actual connection latching APIs.  If the IESG or IETF agree we could
always turn the reference to [I-D.ietf-btns-abstract-api] into a
normative reference and then we could let this I-D block on that (but I
don't think this is necessary).

> notification even when the ULP is doing the active control of the

That's an API issue.

> to control/monitor the usage of IPsec in creating 'channels'.  In

It was not my intention to imply that this I-D provides an abstract API
for applications, but it does actually outline what it looks like in
that there are plenty of requirements for what such an API must provide.

> in the kernel and the IPsec implementation in the kernel.  It is not

Interfaces for applications are described, just not in as much detail as
the interfaces between the ULPs and IPsec key management.

There's a good reason for this: this I-D focuses on correctness of the
connection latching concept, and that requires a focus on the interfaces
between ULPs and IPsec.  The application APIs follow from the
requirements in the I-D, but the actual specification of application
APIs is left for a different document.

> draft (and more mention of it) is what is needed.

Solaris has a socket option interface to connection latching.  This
shows that a a socket-based connection-latching API is feasible.  See
the [IP_SEC_OPT.man] reference.

Nico
--

---

From: Nicolas Williams <[email-masked]>
Date: Thu, 26 Feb 2009 15:31:45 -0600

> creation fails)?  If not what state is the latch in when the function

It's effectively synchronous, though if the SAs exist or if all the
latching parameters are provided then CREATE_CONNECTION_LATCH() can
return very quickly (i.e., without initiating any IKEv2 exchanges).

But even if I wrote that it's synchronous one could always implement it
in a way is not.  For example, one could add a state called "LARVAL" so
that CREATE_CONNECTION_LATCH() can return immediately, and with some
interface by which to subsequently notify the ULP of the ultimate
disposition of the latch.

The requirements about when packets can be sent/accepted take care of
this issue as meeting those requirements effectively requires making
CREATE_CONNECTION_LATCH() synchronous or splitting its work as described
above.

> application should be informed that a component connection has broken..

I don't see that implication.

> misleading.

It's passive in the sense that it does not modify the IPsec processing
model, for example.  And it's passive in that no additional control
messages of any sort go on the wire.  I see that "passive" could have
other meanings in this context, but I'm not sure what other word to use,
and, in any case, where it is used the context is quite clear:

  "At its bare minimum connection latching is a passive layer atop IPsec
   ..."

and

                                     "In this minimal form connection
   latching is a passive, local feature layered atop IPsec."

>              The security considerations needs to be absolutely clear

I don't believe there are any such opportunities.  That should be clear
from the facts that: a) the IPsec processing model is not modified, b)
the ULP protocols are not modified, c) application protocols are not
modified.  The way the ULPs interact with IPsec is modified, but that
only adds protections.  To the extent that there are DoSes in IPsec or
in the ULPs themselves, these remain, except where protecting the ULP
with IPsec makes the DoSes go away.

> case a forward ref would help.

Allowing applications to override this for connection-oriented ULPs
makes no sense at all.  Allowing them to override it for UDP used with
connect() would be OK, but the application might as well close() and
re-create the socket to get the same effect.

That said, I don't oppose your request.

> but it would be good to make it clear here.

The requirements and recommendations in section 2 apply regardless of
which implementation design model the implementor chooses.

We described two models to make it easier to reason about connection
latching.  Other designs might be possible, for all I know, as long as
they conform to the requirements and recommendations of section 2.

> The ULP/application would probably like to know what went wrong.

Indeed, in error cases they return errors.  I elided that because I
thought it would be obvious.  I'd be glad to add text to make this
explicit.

> notification?

Yes.

>                If so it needs noting in the spec.  See also the comment
> about callbacks below.

How much more explicit can it be than it already is?  To wit:

   The IPsec LD interface to the ULP is as follows:
   o  ALERT(latch object handle, 5-tuple, new state, [reason])
	 Alerts a ULP as to an asynchronous state change for the given
	 connection latch and, optionally, provides a reason for the
	 change.

And it's referenced in the state diagram in figure 1.

> indicating that ULPs/applications must be able to set themselves up yo

"...interface to register the callback needed to handle..." sounds like
a programming language binding of an abstract interface.  That's not for
this I-D to provide.

ALERT() can be called if the ULP has connection latches.  Creating a
latch is sufficient for registration from the point of view of the
abstract interface.

> mechanisms.  How does it propagate to the application as described

That's for the ULP to do.  There's a requirement that the ULP provide
such an interface, but the particulars of the interface are not
described.  A socket-based implementation might cause I/O system calls
to on the affected socket to return errors, or it might send a signal to
the process, or an implementation might have extended async signalling
interfaces (think of kqueue, or Solaris event ports).  These are all
details that don't really belong in this I-D.

> lapse.  I think a little explanation is needed.

It's a term from IPsecv3 (see RFC4301).  It's not important if you don't
know, since section 2.3 is informative.  I don't think it's important to
describe what "decorrelated SPD" means here.

> address associations.'  I cannot parse this phrase.  s/to/in/ maybe????

or s/to/and/.  Either way, this is a reference to the fact that most/all
IPsec deployments rely on IPsec configuration to authenticate IP
addresses, but that in practice this is hard, thus applications don't
have a strong tie between a peer's hostname or IP address as provided in
a user interface element (say) and the credentials used by IKE to
actually authenticate the peer.

> connect() from 'socket' interface'

Sure.  This is clarified elsewhere in the I-D where "connect()" (and
listen()) is referred to.

> would be more consistent with the style of the other bullets.

The word "identity" would be more likely to cause confusion here.
Perhaps you meant "name of cryptographic algorithm suites"?  Either way
I don't see how the addition of "name of" or "identity of" to
"cryptographic algorithm suites" clarifies anything -- what else could
have been meant?  What could a reader confuse "cryptographic algorithm
suites" with?  I can see "quality of protection" being cryptic, which is
why it's defined.

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

Good catch, thanks.

> as BTNS peer IDs/values that potentially have large representations,

OK.

> occurrence).  There may be a few more I missed.

Do we still expand "IP" in RFCs?  But, sure.

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

Good catch.

Thanks!

Nico
--