Subject: Re: [kitten] review of draft-ietf-kitten-gssapi-extensions-iana-07

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 5 Aug 2013 14:47:43 -0400 (EDT)

On Tue, 6 Nov 2012, Leif Johansson wrote:

>
> Folks,
>
> I've reviewed version -07. I general I think the document is in good shape.
> Reading through it I've found a small set of relatively minor nits.
>
> - Object Type: A long list of things like "Function" and "Integer" is given
> but  the description text basically sais these are just examples. I suggest
> replacing the list with the following text
>
> <Symbol> defined by the binding language
>
> - Registration Rules: Suggest change to say:
>
> "<Reference> to Policy defined by [RFC5226] or an RFC that
> updates [RFC5226], for instance ..."
>
> Finally, I  find the guidelines to Expert reviewers to be somewhat hard to
> follow. The final paragraph in section 8.2.2 to me basically reads as "use
> your good judgement".
>
> Personally I would find it difficult to draw any guidance from that text
> but I
> fully understand if this horse has been flogged enough.

I've also reviewed version -07, and don't see anything particularly 
objectionable.

Minor nits:

In the table in section 7, in the entry for "Registration Rules", my first 
reading left me confused as to which sub-namespace "items that fall in 
this sub-namespace" referred to.  Re-reading makes it clear that the 
sub-namespace is the one which has this Registration Rules attribute, and 
I don't have any ideas for how to reword the text to be clearer.  The 
capitalization of "sub-namespace" is a bit inconsistent throughout the 
document.

The table entry for "Reference" should add an 'a' in "Reference to a 
document".

The "Expert Reviewer" entry is internally inconsistent about how many 
reviewers can be listed in a single field.  Multiple instances are 
allowed, which would seem to indicate that one-reviewer-per-instance is 
the correct disambiguation.

In section 8, the meaning might be more clear with a comma between 
"multiple registries" and "each".

In the first paragraph of 8.2.2, "there is are any IETF Working Groups" 
has an unnecessary "is".

-Ben

---

From: Nico Williams <[email-masked]>
Date: Mon, 5 Aug 2013 14:54:10 -0500

On Thu, Nov 8, 2012 at 9:36 AM, Leif Johansson <[email-masked]> wrote:
>> The short answer to this is: if we know what the exact guidance is, we
>> wouldn't need to use the Expert Review. So this is by design.
>
> I object to that answer.

Hmm, well, we could certainly spend some time on providing GSS API
design perls of wisdom.

But then, if we proceed with the channel bound thing and async I/O...
well, we'll  be rototilling the whole thing.  That will be great
because the API will get easier to use, but it may also change some of
those perls of wisdom.

> Expert review should absolutely have guidance. I'm fine saying "use your
> good
> judgement" or not even having a guidance section and relying on implicit
> good sense.

"Take API design 101, and preferably 201 too".

> However, the current text claims to provide guidance to reviewers but
> probably
> won't help anyone who doesn't already understand what to do.
>
> I say skip it, but if ppl are really married to it I'm fine with that
> too :-)

I'll provide a handful of perls now:

 - *always* include an OM_uint32 *minor_status argument in the C
bindings of any new GSS functions

 - always specify all major status codes a new function may return,
and if it may return as-yet-undefined errors, then say so (or maybe
that should always be allowed for new functions?)

 - if you add any new opaque types always add a GSS_Duplicate_<type>() function

 - wherever possible use the naming attributes API for extensions

You already have to follow the naming conventions (GSS_S_.*, GSS_C_.*,
...), so no need to go there.

I could add more.  Someone else take a turn first.

Nico
--