Subject: Re: [kitten] I-D Action: draft-ietf-kitten-channel-bound-flag-00.txt

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 7 Aug 2013 17:09:04 -0400 (EDT)

On Sun, 7 Jul 2013, [email-masked] wrote:

>
> A New Internet-Draft is available from the on-line Internet-Drafts directories.
> This draft is a work item of the Common Authentication Technology Next Generation Working Group of the IETF.
>
> 	Title           : Channel Binding Signalling for the Generic Security Services Application Programming Interface
> 	Author(s)       : Nicolas Williams
> 	Filename        : draft-ietf-kitten-channel-bound-flag-00.txt
> 	Pages           : 13
> 	Date            : 2013-07-07
>
> Abstract:
>   Channel binding is a technique that allows applications to use a
>   secure channel at a lower layer without having to use authentication
>   at that lower layer.  The concept of channel binding comes from the
>   Generic Security Services Application Programming Interface (GSS-
>   API).  It turns out that the semantics commonly implemented are
>   different that those specified in the base GSS-API RFC (RFC2743), and
>   that that specification has a serious bug.  This document addresses
>   both, the inconsistency as-implemented and the specification bug.
>
>   This Internet-Draft proposes the addition of a "channel bound" return
>   flag for the GSS_Init_sec_context() and GSS_Accept_sec_context()
>   functions.  Two behaviors are specified: a default, safe behavior
>   reflecting existing implementation deployments, and a behavior that
>   is only safe when the application specifically tells the GSS-API that
>   it (the application) supports the new behavior.  Additional API
>   elements related to this are also added, including a new security
>   context establishment API.
>
>
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-kitten-channel-bound-flag
>
> There's also a htmlized version available at:
> http://tools.ietf.org/html/draft-ietf-kitten-channel-bound-flag-00

I have reviewed this document; the following comments are basically 
editorial in nature, with no substantial flaws in the technical proposal.

In the abstract, "Additional API elements related to this are also added", 
what is "this"?  channel binding?

Section 1.2 seems rather informal for a standards-track submission.  (The 
whole document's use of "we" is a bit dubious, too, but that's a harder 
change to make.)  Would 1.3 and 1.4 (and in general, all "future work" 
comments) be better in an appendix?

In the first sentence of 1.3, you have a comma that begins a 
parenthetical, but no comma to end the parenthetical.

In section 2.2, please make it more explicit that in order to use the 
req_flags given to GSS_Set_context_flags(), the input flags to 
GSS_Init_sec_context() must be all zero.  Introducing the new FLAGS type 
in a "NOTE:" is probably not appropriate for a standards-track document.

In 2.2.1, it's probably worth making a note that the flags are now 64-bit 
fields instead of 32-bit.

In 2.5, I don't think the first sentence provides sufficient description 
of the flag.  Please give a full paragraph with such a description, and 
split the rest of the (current) paragraph into a separate paragraph.

Section 2.5.1's use of the phrase "both flags" is a little vague; I'd like 
to see some text indicating why the two flags are related.

In 2.6, I would say "applied to an empty security context" instead of the 
plural form currently given.

In the first bullet point of section 3, the use of commas to group the 
clauses which "both" binds together scans pretty awkwardly.  Can't we just 
use the same (a) and (b) style as in the other bullet points?

Throughout section 3, I don't see much precedent for the use of 
"input_channel_bindings" as a variable name.  RFC2743 has "chan_bindings", 
the MIT krb5 gssapi.h has "/* input_chan_bindings */" and Heimdal's header 
matches MIT's.

I agree with the overall scheme, as mentioned when we were debating 
between alternate approaches.

-Ben

---

From: Nico Williams <[email-masked]>
Date: Wed, 7 Aug 2013 23:00:05 -0500

> editorial in nature, with no substantial flaws in the technical proposal.

Thanks!

> what is "this"?  channel binding?

Well, the whole subject of the I-D, yes.

> Section 1.2 seems rather informal for a standards-track submission.  (The

Suggestions welcomed.

> comments) be better in an appendix?

If it makes it onto the Standards-Track then it really is "we"/"us".
I don't want to speak in the first person just to create extra work
for the RFC-Editor.  Perhaps you can suggest better wording that
doesn't have this problem and doesn't feel more artificial still.

> In the first sentence of 1.3, you have a comma that begins a parenthetical,
> but no comma to end the parenthetical.

it's not a parenthetical, it's a list of adjectives ("existing",
"non-standard"); "and" might be better than a comma, but I'm all "meh"
about it.

> GSS_Init_sec_context() must be all zero.  Introducing the new FLAGS type in

Oh, that's probably better, yeah, and GSS_Init_sec_context() should
even return an error otherwise.

> a "NOTE:" is probably not appropriate for a standards-track document.

Sure, I should add a sub-section just to describe FLAGS.  But it will
be very brief :)

> fields instead of 32-bit.

Er, sure.  And that it's 64-bit, not "at least 64-bit".

> In 2.5, I don't think the first sentence provides sufficient description of
> the flag.  Please give a full paragraph with such a description, and split
> the rest of the (current) paragraph into a separate paragraph.

OK.

> to see some text indicating why the two flags are related.

I think it's clear enough.  The two flags need not be related at all
in order to share the same bit assignment -- as long as they can't be
used in the same context (one is a req_flag but not a ret_flag and
vice-versa).

> plural form currently given.

OK.

> use the same (a) and (b) style as in the other bullet points?

There's a missing comma before "do not match".  Would that help?  But,
sure, I can do the (a) and (b) thing.  (I must have switched in the
second bullet because the text for (b) there got unwieldy.)

> the MIT krb5 gssapi.h has "/* input_chan_bindings */" and Heimdal's header

The latter match RFC2744.

Hmm, well, I think it's clear enough.  And I prefer it this way.  If
it's really obnoxious I could change it, but I'll note that we're
already rather inconsistent about argument naming throughout the
GSS-API.

> I agree with the overall scheme, as mentioned when we were debating between
> alternate approaches.

Excellent!  I'll submit a new version.  We should implement and WGLC.

Nico
--

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 8 Aug 2013 13:18:44 -0400 (EDT)

>> Section 1.2 seems rather informal for a standards-track submission.  (The

I'd probably just skip the historical discussion and mailing list 
mention and go straight to a declarative statement:
A useful aspect of the Java Bindings of the GSS-API is appropriated for 
the abstract API, specifically the notion of creating [...]

> doesn't have this problem and doesn't feel more artificial still.

It sounds like my point came across badly -- it wasn't a question of "we" 
versus "I", but rather the "we propose", "we are likely to", etc..  I know 
that RFC stands for "request for comments" and thus proposal language 
should be fitting, but I'm more used to seeing declarative specification 
like "a new X is created, which Y".

> "non-standard"); "and" might be better than a comma, but I'm all "meh"

Hmm, okay.  It doesn't look as bad now, rereading it.

> even return an error otherwise.

Well, erroring out would remove the feature (allowed by the present text) 
of being able to override the flags from context-creation time at 
Init_sec_context time.  Either would be fine with me, so long as the text 
is clear.

> Sure, I should add a sub-section just to describe FLAGS.  But it will

Yup :)

> second bullet because the text for (b) there got unwieldy.)

The extra comma would help some, but I think the a/b would be 
substantially more clear (and would make the whole list consistent).

> The latter match RFC2744.

That would have been clever for me to look at, whoops.

> already rather inconsistent about argument naming throughout the

I don't really care, I was just noting the inconsistency.
It calls to mind the mandatory/required checksum type split I noted in a 
different thread.

-Ben

---

From: Nico Williams <[email-masked]>
Date: Thu, 8 Aug 2013 12:46:27 -0500

On Thu, Aug 08, 2013 at 01:18:44PM -0400, Benjamin Kaduk wrote:
> A useful aspect of the Java Bindings of the GSS-API is appropriated
> for the abstract API, specifically the notion of creating [...]

OK, thanks.

> It sounds like my point came across badly -- it wasn't a question of
> "we" versus "I", but rather the "we propose", "we are likely to",
> etc..  I know that RFC stands for "request for comments" and thus
> proposal language should be fitting, but I'm more used to seeing
> declarative specification like "a new X is created, which Y".

Ah, now I get it.  Sure, though as for futures, they help explain the
value of the new design (or is that just my perception?), so I don't
want to relegate that to an appendix.

> Well, erroring out would remove the feature (allowed by the present
> text) of being able to override the flags from context-creation time
> at Init_sec_context time.  Either would be fine with me, so long as
> the text is clear.

I prefer the error out method.

> It calls to mind the mandatory/required checksum type split I noted
> in a different thread.

I propose we continute this tradition of inconsistent argument naming ;)