Dr. Richardson et al.: I was asked by the Applications area to review
draft-ietf-btns-c-api-03.

Here is a review of it, following a similar one I did last week for
draft-ietf-btns-abstract-api-01.

draft-ietf-btns-c-api-03
-------------------------------
As a follow-up draft to draft-ietf-btns-abstract-api, the reasons
why this document is important are much the same as why the latter
one was important.  I do note that this document appears to be
more fleshed out than its abstract-api counterpart, whereas I would
have expected the opposite.  For instance, the Security Considerations
section of the abstract-api I-D was empty, and the one for c-api
is filled out.  Ostensibly, the Security Considerations section
for the abstract-api should be the one that could be filled out
and "inherited", if you will, by a language-specific binding
document that fulfills the abstract-api one.

Furthermore, and I think this is important, the c-api document
should do more to align itself with the abstract-api document
since the latter document asserts the shape of the information
in the c-api document.  For instance, S2.3.2 of c-api contains
the attributes of a pToken.  Clearly, these are related to
S7 of abstract-api (Properties of pToken objects).  Likewise,
S2.2.2 of c-api is similarly related to S8 of abstract-api.
Currently, there is a lack of such close cross referencing
between these documents.  I suspect that the c-api is the
first language binding document; thus, any time spent now in
such cross-referencing will pay for itself later when c-api
is used as a template for a Java-api or c++-api document.

Here are more focused comments:

- S1, end of first paragraph: (1) may be a good idea to provide
  a reference for HIP (also expand the acronym).  SIP (RFC3261)
  may be another protocol that could use this API; S26.3.1 of
  rfc3261 does state that "Proxy servers, redirect servers,
  registrars, and UAs MAY also implement IPSec or other lower-
  layer security protocols."  So conceivably, a user agent
  can create a connection to its proxy and use the IPSec APIs to
  figure out whether that connection is integrity and confidentiality
  protected.  In certain networks (like IMS), IPSec is used
  fairly heavily between the UA and the proxy.

- S1, third paragraph:
  (1) s/we defined APIs/we define APIs/
  (2) s/document fulfisl/document fulfills/
  (3) s/requirements presented in/requirements in/
  (4) s/and present C-bindings/and presents C-bindings/

- S1, paragraph under Figure 1: s/'The third/The third

- IMHO, it would be good to move S2.1 to *after* the
  attributes for pToken and iToken are defined.
  That way, the reader can make more sense of these APIs.  As
  the draft currently stands, the reader does not know
  anything about the attributes when he or she
  reaches S2.1, so the discussion there appears out of place,
  without a good context.

- S2.1, second paragraph: s/using malloc/from heap.

- S2.1, second paragraph, last sentence: "When attr_val is NULL ...
  of the attr_val."  I do not understand the need for this.
  If attr_val is NULL, why would it have a non-zero attr_len?
  Maybe I am missing somethng?

- S2.1, last paragraph: You may want to mention that attr_val
  must not be NULL, and neither should attr_len.

- S2.2.1, text under Figure 3: what does the size have to do
  with a c'tor and d'tor?  I would suggest a re-write as follows:

    Operating environments that support the IPSec API will
    provide appropriate constructor and destructor for the
    iToken objects.  Because applications will often not be
    aware of the byte-representation of the iToken object, nor
    will it know which attributes to initialize upon construction,
    applications MUST only use the provided constructor to
    create an iToken object, and when no longer needed, use the
    provided destructor to destroy it.  Figure 4 shows the
    APIs:

- S2.2.2, Figure 5: s/LEAFOFFAITH/LEAPOFFAITH
  Also, you may want to tie these attributes to S8 in abstract-api
  document.

- S2.2.2, text under Figure 5: consider organizing the text in
  this paragraph to have hanging indents and such.  It will
  make it much more readable.

- S2.2.2, I do not understand the contents of the paragraph
  "The attributes for the second ... should be used."

- S2.3.1, Figure 7:
       s/ipsec_create_pToken()/ipsec_create_pToken(void)/
  The reason is that a func() is different than a func(void) under
  ANSI C.

- S2.3.1, last paragraph of the section: what would cause
  ipsec_free_pToken() to return non-zero?  About the only thing I
  can think of is if the parameter passed in is null.  I believe
  we can in fact get by with having ipsec_free_pToken() return a
  void, but I will leave it to your good judgement.

- S2.3.2: same comments as S2.3.1 about tying the attributes
  of pToken to abstract-api draft, and having hanging indents
  for readability.

- S2.3.4, the text in first paragraph: I am curious why the
  decision to only support the IPSec APIs on sendmsg() and
  recvmsg()?  I am sure you had a technical reason, but I
  cannot think of any.  It may be a good idea to list the reason
  as a note in the draft.

- S2.3.5: the text under Figure 11 mentions "ipsec_cmp_policy()".
  Did you mean "ipsec_cmp_pToken()" instead?

- S2.3.5: in the text under Figure 11, you may want to mention
  that ipsec_cmp_pToken() returns non-zero if p1 != p2.

- S2.3.6, Figure 12: should the type of "p" not be ipsec_pToken
  only?  "p" is the source of the copy, and as such only a
  pointer to it is required, not a **.  Plus, qualifying it with
  a const will probably not hurt.

Hope that helps!

Thank you.

- vijay
-- 
Vijay K. Gurbani, Bell Laboratories, Alcatel-Lucent
2701 Lucent Lane, Rm. 9F-546, Lisle, Illinois 60532 (USA)
Email: vkg@{alcatel-lucent.com,bell-labs.com,acm.org}
WWW:   http://www.alcatel-lucent.com/bell-labs
_______________________________________________
APPS-REVIEW mailing list
APPS-REVIEW@ietf.org
https://www.ietf.org/mailman/listinfo/apps-review