Subject: [kitten] New Version Notification for draft-kaduk-kitten-gss-loop-01.txt (fwd)

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 20 Nov 2013 14:09:00 -0500 (EST)

I submitted an -01 of the gss-loop document, with sample code.
There's a duplicate line of source XML in section 2.5 which causes some 
duplicate text, it's fixed in my git already.  Sorry about that.

Comments welcome :)

-Ben

---------- Forwarded message ----------
Date: Wed, 20 Nov 2013 11:03:24 -0800
From: [email-masked]
To: Benjamin Kaduk <[email-masked]>
Subject: New Version Notification for draft-kaduk-kitten-gss-loop-01.txt

A new version of I-D, draft-kaduk-kitten-gss-loop-01.txt
has been successfully submitted by Benjamin Kaduk and posted to the
IETF repository.

Filename:	 draft-kaduk-kitten-gss-loop
Revision:	 01
Title:		 Structure of the GSS Negotiation Loop
Creation date:	 2013-11-20
Group:		 Individual Submission
Number of pages: 17
URL:             http://www.ietf.org/internet-drafts/draft-kaduk-kitten-gss-loop-01.txt
Status:          http://datatracker.ietf.org/doc/draft-kaduk-kitten-gss-loop
Htmlized:        http://tools.ietf.org/html/draft-kaduk-kitten-gss-loop-01
Diff:            http://www.ietf.org/rfcdiff?url2=draft-kaduk-kitten-gss-loop-01

Abstract:
    This document specifies the generic structure of the negotiation loop
    to establish a GSS security context between initiator and acceptor.
    The control flow of the loop is indicated for both parties, including
    error conditions, and indications are given for where application-
    specific behavior must be specified.

Please note that it may take a couple of minutes from the time of submission
until the htmlized version and diff are available at tools.ietf.org.

The IETF Secretariat

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 20 Nov 2013 17:45:26 -0500 (EST)

> duplicate text, it's fixed in my git already.  Sorry about that.

We had some discussion on IRC, with Nico, Simo, Tom, Greg, and me.

It's unclear whether the src_name output of gss_accept_sec_context() is 
only intended to be populated on a GSS_S_COMPLETE return; if not, it is 
very easy to leak storage from earlier iterations when multiple calls are 
needed.

We discussed whether (either at the abstract level or the C bindings) it 
is guaranteed to be safe to call gss_release_buffer() twice on the same 
buffer.  It is required to set the length of the buffer to zero (but not 
to zero the value field), but also "Any buffer object returned by a 
GSS-API routine may be passed to gss_release_buffer (even if there is no 
storage associated with the buffer)."  What exactly constitutes a "buffer
object returned by a GSS-API routine" seems subject to interpretation.
(There are many things that could have been done to make this situation 
better, but we don't hvae a time machine.)  Buffers with length zero can 
be generated in lots of ways other than gss_release_buffer() 
(gss_init_sec_context(), gss_accept_sec_context(), gss_unwrap()).

We're pretty sure that GSS_S_CONTINUE_NEEDED will not be set with any 
other major status bits (in the C API).

Nico wanted sample Java code, too (anything we have bindings for, really). 
I don't think I can contribute such code, but would probably take it if 
someone else did.

The send_token()/receive_token() routines are not very good examples.  It 
is probably better to rewrite them to use sendmsg/recvmsg with socketpair, 
which also eliminates the need to add framing with the token length.
(Other stuff about the current implenetation is bad, too, but that's not 
really relevant if it gets rewritten.)

-Ben

---

From: Greg Hudson <[email-masked]>
Date: Wed, 20 Nov 2013 18:17:48 -0500

On 11/20/2013 05:45 PM, Benjamin Kaduk wrote:
> buffer.

I think that unfortunately it is not safe per the spec, although it is
probably safe in every GSSAPI implementation.

> The send_token()/receive_token() routines are not very good examples. 
> It is probably better to rewrite them to use sendmsg/recvmsg with
> socketpair, which also eliminates the need to add framing with the token
> length.

I think the RFC should only include the do_initiator() and do_acceptor()
functions.  The sample code does not need to compile and run, only
demonstrate how the API should be used and be a reasonable starting
point for cut and paste.  Adding questionable framing code runs the risk
of people actually using it, and adding solid framing code would be too
much verbiage.  release_buffer() is needed, of course, unless we get rid
of it somehow (perhaps by assuming that receive_token will free what's
already there, and then just calling free() in cleanup).

I would rather see anonymous handled as a runtime parameter than an
#ifdef, which is distracting.

"if ((ret_flags & GSS_C_ANON_FLAG) != GSS_C_ANON_FLAG)" can be
simplified to "if (!(ret_flags & GSS_C_ANON_FLAG))", and similarly
elsewhere.

---

From: [email-masked] (Martin Rex)
Date: Thu, 21 Nov 2013 00:51:29 +0100 (CET)

Benjamin Kaduk wrote:
> Benjamin Kaduk wrote:

It is up to the mechanism implementation at which point during the
handshake the src_name output parameter from gss_accept_sec_context()
is populated.  The application caller MUST provide the exact same
parameter as src_name to *ALL* iteration calls of gss_accept_sec_context()
of a single handshake, and the gssapi mechanism implementation ought
to actually "create" output values at most once during a handshake,
independent of when it is returned.

I actually had to look at the source to figure out what I had implemented
in my gsskrb5.dll wrapper for the Microsoft Kerberos SSP.  My implementation
clears all output parameters on entry to gss_accept_sec_context(),
returns scalars (ints) whenever they become available and memory-managed
objects (src_name, delegated_cred) only on the final interation that
returns GSS_S_COMPLETE.

I believe that to be conservative and simple behaviour with the least
potential for problems.

An application caller to the context iterator functions ought to
allocate the storage for all necessary/desired input and output
parameters _before_ the first iterator call, and use that exact
same storage for the entire handshake, i.e. until the iterator
either completes (GSS_S_COMPLETE)
or fails (everything else than COMPLETE and CONTINUE_NEEDED).

Creating the storage for input or output parameters only temporarily
for a singular invocation of either of the context iterator calls
is dangerous, unsafe and will result in undefined behaviour
and might result in unexpected handshakes or crashes.

> object returned by a GSS-API routine" seems subject to interpretation.

What in particular causes your notion "seems subject to interprettion"?

IMO, the specifications (rfc2743,rfc2744) are perfectly clear on this point.

In the high-level GSS-API spec (rfc2743), output parameters
are those parameters listed under "Outputs:",
e.g. GSS_Init_sec_context:

   http://tools.ietf.org/html/rfc2743#page-42

In the GSS-API C-Bindings spec (rfc2744), output parameters
are those that are tagged as "modify", whereas input parameters
are those tagged as "read" in the first line of each parameter description,
e.g. for gss_init_sec_context:

   http://tools.ietf.org/html/rfc2744#page-60

> (gss_init_sec_context(), gss_accept_sec_context(), gss_unwrap()).

Where do you see the problem?
An output buffer with a zero length is allowed to have a NULL value pointer,
and as an alternative, the gssapi mechanism could use a pointer to
one central, static empty string.  The one thing that a gssapi mechanism
obviously ought to NOT do is invoking the implementation-specific behaviour
of malloc(0).

When the gssapi mechanism does not allocate memory for zero-length output
buffers, then robust gss_release_buffer() is trivial to achieve,
if the length is 0, do nothing and return GSS_S_COMPLETE.

> other major status bits (in the C API).

This is my understanding as well (Microsoft's SSPI might differ,
they have an additional COMPLETE status beyond GSS-API for DCE interop).

-Martin

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 20 Nov 2013 19:40:28 -0500 (EST)

> parameter as src_name to *ALL* iteration calls of gss_accept_sec_context()

Having recently completed a re-read of RFC 2743 and RFC 2744 (*), I'm sad 
to say that I do not believe there is a normative reference to support the 
MUST, there.  I really wish there was, but RFC 2743 only calls out the 
acceptor_cred_handle as being fixed between iterations.
Hmm, I guess this is relevant, though:
    [...] verifies the incoming input_token and
    (following the successful completion of a context establishment
    sequence) returns the authenticated src_name and the mech_type used.
So, that would indicate that src_name is only valid after successful 
context establishment.

> independent of when it is returned.

Yes, it really ought to.  We'll have to remember that for the next refresh 
of the spec.

> and might result in unexpected handshakes or crashes.

I agree.  I do not think the current document mandates such sane behavior, 
though.

> What in particular causes your notion "seems subject to interprettion"?

Nico was making arguments about how it is "torturing the language and 
common sense" to claim that gss_release_buffer() is "notionally 
output[ting] a buffer".  I was claiming that (in the C bindings at least), 
the buffer parameter is an output parameter of gss_release_buffer().

Apparently this has been discussed previously, before I joined the list. 
If someone had a pointer to that handy, I would appreciate it

>   http://tools.ietf.org/html/rfc2744#page-60

I'm not sure why the particular page numbers linked are relevant.

In any case, RFC 2743's description of GSS_Release_buffer() lists the 
'buffer' argument as an Input, whereas RFC 2744's description of 
gss_release_buffer() lists the 'buffer' argument as 'modify' (an output).

The matter seemed subject to interpretation because we could not come to 
an agreement about what the correct interpretation is.

> if the length is 0, do nothing and return GSS_S_COMPLETE.

It is trivial to achieve, yes.  Would an implementation that failed to do 
so be non-compliant with the RFC?  Not everyone seems to agree.  As Greg 
notes, all known implementations seem to be safe.

-Ben

(*) I also noticed that ~all of Appendix A of RFC 2744 (the sample 
gssapi.h header) has incorrect signatures for the listed functions. 
Basically, any pointer argument which is not in the final position, is not 
listed as a pointer type.  It seems likely that a script to remove 
argument names was buggy.  I'm not sure that I'm up to submitting a full 
erratum to note all the changes, though.

---

From: [email-masked] (Martin Rex)
Date: Thu, 21 Nov 2013 02:14:47 +0100 (CET)

Benjamin Kaduk wrote:
> >>       It is required to set the length of the buffer to zero (but not

The GSS-API C-Bindings decided to design the "gss_buffer_t" in a fashion
that the container structure (gss_buffer_desc) is always owned by the
application caller (_never_ by the gssapi mechanism).  This is something
that the high-level GSS-API does not imply, but allows. 

> If someone had a pointer to that handy, I would appreciate it

I'm sorry, I don't have a pointer handy (and do not not consciously
remember such a discussion at the moment).

> I'm not sure why the particular page numbers linked are relevant.

Just an example for comparison.  rfc2743 is abstract. rfc2744 is one
possible incarnation of a language binding.

> gss_release_buffer() lists the 'buffer' argument as 'modify' (an output).

Well, OK, my rule-of-thumb fails in this special case for rfc2744.

The abstract semantics of rfc2743 for GSS_Release_buffer() _still_
apply to the rfc2744 gss_release_buffer() language bindings.

'buffer' is an input parameter, and gss_release_buffer() is allowed
(and typically will) modify the contents of the caller-owned gss_buffer_desc

gss_release_buffer() will NEVER return a buffer.
What gss_release_buffer() MUST do on success is zero the length field.
What gss_release_buffer() is permitted (but not required) to do, is
additionally set the value to NULL when zeroing the length field.

> an agreement about what the correct interpretation is.

I'm slightly lost here.

> notes, all known implementations seem to be safe.

While I do not remember any problems with gss_release_buffer(),
some MIT Kerberos releases, most Solaris kerberos libs and some AIX
kerberos libs crash with my gsstest tool, which indicates both, a lack
of testing, a lack of robustness and a lack of conformance with rfc2744.

100% RFC-conformaning implementations are pretty rare for non-trivial
specifications--and since most specifications do contain defects and/or
inappropriate requirements, 100% conformance might not always be
desirable.  The difficulty is that different implementors typically
choose to deliberately ignore different requirements, and the number
of differences typically increases with the complexity of features,
the number of defects in the spec and the number of invalid requirements
that are in clear conflict with rfc2119 Section 6.

> erratum to note all the changes, though.

I'm not aware of any problem with that header file (aside from the
invalid use of const for custom pointer types).

-Martin

---

From: [email-masked] (Martin Rex)
Date: Thu, 21 Nov 2013 02:23:42 +0100 (CET)

> invalid use of const for custom pointer types).

Ooops, you're correct.

Admittedly, I never tried to extract and use the header from rfc2744,
and just looking at it again, I agree, it seems to have a few errors
(lacking "*") in many of the output/modify function parameters
--seemingly except for the last function parameter, where the "*"
is usually present.

-Martin

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 20 Nov 2013 20:37:41 -0500 (EST)

> --seemingly except for the last function parameter, where the "*"

There is no comma following the name of the last parameter.  That's the 
best we came up with on IRC, at least.

-Ben

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 20 Nov 2013 20:45:10 -0500 (EST)

> remember such a discussion at the moment).

My summary of the IRC discussion may have been inaccurate, too; the 
previous discussions may have been about things like using different C 
types for input and output buffers, or indicating the provenance of a 
buffer within the structure (or by some other means).

> apply to the rfc2744 gss_release_buffer() language bindings.

I guess that's compelling, the 2744 text describing gss_release_buffer's 
applicability notwithstanding.

So, we're left with Greg's assessment, "it is not safe per the spec, 
although it is probably safe in every GSSAPI implementation."

Do we want to change the sample code to not rely on this behavior, or just 
note that it does so rely?

> I'm slightly lost here.

Tom and I had argued one way, and Nico the other.  Neither side won the 
other over, in the IRC discussion.  We've moved on since then, so it's 
probably not worth dwelling on.

-Ben

---

From: [email-masked] (Martin Rex)
Date: Thu, 21 Nov 2013 03:35:12 +0100 (CET)

> --seemingly except for the last function parameter, where the "*"

The "*" on the output parameters in the Appendix A sample header file
seem to have disappeared during I-D revision 05->06 of the C-bindings
draft, they were present during -00 through -05:

  http://tools.ietf.org/rfcdiff?url2=rfc2744.txt&url1=draft-ietf-cat-gssv2-cbind-05.txt#diff0410

I put together my gssapi_2.h header in 1995 (-00) and didn't notice
the corruption in Appendix A starting in cbind-06.

A "fix" of that size appears somewhat huge for the errata process.
Filing an errata that describes the nature of the problem--maybe including
a suitable URL to rfcdiff like the one above for visualizing the problem,
might be preferable.

The formatting of the C-Bindings I-D changed considerably from 05->06,
and apparently the gssapi.h sample header file in Appendix A got corrupted.
Scanning my archives, the C-Bindings author even indicated that he
had changed his I-D tooling.  Unfortunately http://tools.ietf.org/rfcdiff
didn't exist back then (and previous versions of drafts were also no
longer accessible in the download directories (so unless you happened
to have a previous copy...).

:  From:     John Wray
:  Subject:  New GSSAPI C-bindings draft
:  Date:     Mon, 13 Jul 1998 10:41:05 -0400
:  To:       [email-masked]
:
:  I've just submitted an updated C-bindings document, so expect
:  draft-ietf-cat-gssv2-cbind-06.txt to show up on the repositories in the
:  next couple of days.
:
:  Changes from the previous version:
:
:     Reformatting.  Changing jobs meant I no longer have all
:        the nroff macros I used to use, so I took the opportunity
:        to migrate to a slightly more modern formatting program.
:        One unfortunate result of this is that diff-ing this version
:        against the previous one won't give much useful
:        information.
:
:     Incorporation of the 2078bis alignments posted to the
:        list by Martin.
:
:     A number of typos and clarification requests received
:        by e-mail (also from Martin).
:
: John

Btw. there is another problem with the gssapi.h sample header file
in rfc2744:  for 64-bit, there exists a binary-incompatibility
between the X/Open defined GSS-API C-bindings and the IETF-defined
rfc2744 C-Bindings for "count" member of the gss_OID_set type:

rfc2744:  http://tools.ietf.org/html/rfc2744#page-84

     typedef struct gss_OID_set_desc_struct  {
         size_t     count;
         gss_OID    elements;
     } gss_OID_set_desc, *gss_OID_set;

X/Open: http://archive.opengroup.org/publications/archive/CDROM/c441.pdf
Page 45:

     typedef struct gss_OID_set_desc_struct{
         int count;
         gss_OID elements;
     } gss_OID_set_desc, *gss_OID_set;

-Martin

---

From: Nico Williams <[email-masked]>
Date: Wed, 20 Nov 2013 23:01:40 -0600

On Wed, Nov 20, 2013 at 07:40:28PM -0500, Benjamin Kaduk wrote:
> Having recently completed a re-read of RFC 2743 and RFC 2744 (*),
> I'm sad to say that I do not believe there is a normative reference
> to support the MUST, there.  I really wish there was, but RFC 2743
> only calls out the acceptor_cred_handle as being fixed between
> iterations.

Well, no other interpretation is sensible but the one that Martin gave.
Anything else surely would have been mentioned, and, at any rate, would
not be sensible.

Why would the name of the initiator *as output to the application* vary
as the security context token exchange progresses?

Clearly allowing that makes no sense.  That's not to say that the
mechanism might not learn the initiator's name piecemeal, just that it
can't output it piecemeal.

> So, that would indicate that src_name is only valid after successful

Authentication is only complete when security context establishment
completes.  Until then anything that may be available (e.g., PROT_READY,
channel binding state -- any and all ret_flags, src_name) is not
authenticated.  (The one exception that might make sense might be
deleg_cred, but there's no purpose to treating it as an exception.)

> Nico was making arguments about how it is "torturing the language
> and common sense" to claim that gss_release_buffer() is "notionally
> output[ting] a buffer".  I was claiming that (in the C bindings at
> least), the buffer parameter is an output parameter of
> gss_release_buffer().

"Output" is an [notional] attribute of the function argument in the GSS
abstract API and in the C bindings, not an attribute of the buffer type
(sadly).  The buffer is only an output of gss_release_buffer() in that
this notionally permits gss_release_buffer() to modify the buffer.

IF RFC2744 had specified proper const typedefs then we'd not say that
the buffer is an output of that function, just that the input to it is
not of a const type.  And then the const-/not-const-ness of an argument
would not merely be notional; it'd be specified in a way that the C
tooling can understand.

It is torturing the language to say that gss_release_buffer() outputs a
token.  It does not.  It modifies a buffer structure to mark it as
"empty", but that's it.

There is a problem in that RFC2744 says that gss_release_buffer() must
set the buffer's length to zero but only that it may set the pointer to
NULL.  This is a problem because malloc(0) may return a non-NULL pointer
that must be free()ed, and so gss_release_buffer() called with a
zero-length, non-NULL value buffer might well call free() on that value!

Now, obviously the foregoing is not a problem because a) apps shouldn't
be double-releasing things, and b) because providers shouldn't be
idiotic: they must either set the value to NULL on buffer release or not
free() it when length is zero (preferably both).

Nico
--

---

From: Nico Williams <[email-masked]>
Date: Wed, 20 Nov 2013 23:08:31 -0600

> >apply to the rfc2744 gss_release_buffer() language bindings.

I agree.

Some details might naturally differ from RFC2743 for other language
bindings, but at the end of the day they must roughly agree.  In
particular there are some important aspects:

 - outputs of security context establishment are only valid or
   authenticated when the context establishment completes

 - no language binding can change the synchronous nature of security
   context token exchanges

 - all the base features must be there (some mechs/providers might not
   provide some features, such as, e.g., confidentiality protection, but
   the programming language bindings of the abstract API  must make it
   possible to provide those)

 - the providers own output buffers

 - the app owns input buffers

There's probably more.

Things are are not obviously required of bindings include things like
manual memory management.  Indeed, automatic memory management is not
incompatible with the abstract API.

> I guess that's compelling, the 2744 text describing
> gss_release_buffer's applicability notwithstanding.

Yes.

> although it is probably safe in every GSSAPI implementation."

And: apps shouldn't be double-releasing anything.

> Do we want to change the sample code to not rely on this behavior,
> or just note that it does so rely?

Either way is fine with me.

Nico
--

---

From: Jeffrey Hutzelman <[email-masked]>
Date: Thu, 21 Nov 2013 10:45:28 -0500

On Wed, 2013-11-20 at 18:17 -0500, Greg Hudson wrote:
> point for cut and paste.

Agree.

-- Jeff

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 21 Nov 2013 11:02:17 -0500 (EST)

[changed subject]

>  http://tools.ietf.org/rfcdiff?url2=rfc2744.txt&url1=draft-ietf-cat-gssv2-cbind-05.txt#diff0410

Thanks for tracking down the version with the change (and the diff).

> A "fix" of that size appears somewhat huge for the errata process.

I agree.
However, it seems that replacing " ," with " *," (all 68 occurrences) 
suffices to fix almost all of the problem.  gss_export_name()'s 
minor_status parameter would not be caught by this replacement, but could 
be mentioned separately.

> a suitable URL to rfcdiff like the one above for visualizing the problem,

I'll probably file an erratum with the old/new text I mentioned above and 
your rfcdiff URL at some point, maybe later today.

>     } gss_OID_set_desc, *gss_OID_set;

Ugh.  Thanks for mentioning it.

-Ben

---

From: [email-masked] (Martin Rex)
Date: Thu, 21 Nov 2013 17:08:31 +0100 (CET)

> Ugh.  Thanks for mentioning it.

I forgot:

MIT Kerberos, HP-UX Kerberos, Solaris Kerberos, AIX Kerberos and
our own ABI uses the gss_OID_set definition from rfc2744 
(or the cbind-00 draft).

In the /usr/include/xom.h header file on HP-UX 11.31 I no longer see
any OID_set definition, and the /usr/include/gssapi/gssapi.h header
file contains the rfc2744 variant with "size_t count".

-Martin

---

From: Jeffrey Hutzelman <[email-masked]>
Date: Thu, 21 Nov 2013 11:09:46 -0500

> as the security context token exchange progresses?

Perhaps there's some stackable thing during which the notion of
initiator name is refined as control passes from one mech to another?

That said, since we are writing advice to application implementors here,
and not a mechanism, the question is not the wide range of what
applications _might_ be allowed to do, but rather what we tell them they
_should_ do.

For initiators, the answer is relatively simple: of course you should
allocate everything up front, pass the same arguments on every call, and
not look at the outputs until context establishment is complete.  Of
course, there's the exception of prot_ready, which is AFAIK the only
output whose semantics are explicitly defined even when the context is
incomplete.

For acceptors, things get a bit dicier, because you have acceptors like
RXGK_GSSNegotiate, which get called once per round trip and want to be
completely stateless in between.  Fortunately for us, actually achieving
that would require the ability to export partially-established contexts,
which is already not well-defined.  Therefore, we can advise that
implementations have little choice but to retain some state, and thus
might as well follow the same conservative approach: allocate everything
up front, and don't look at anything other than prot_ready until the
end.

> token.  It does not.  It modifies a buffer structure to mark it as

Indeed.  At the abstract level, the buffer is not an output.  In the
specific case of the C bindings in RFC2744, the gss_buffer_t is
modified, but that's not the same thing as the buffer being an output;
it's just an artifact of the language bindings.

-- Jeff

---

From: Nico Williams <[email-masked]>
Date: Thu, 21 Nov 2013 12:30:50 -0600

> initiator name is refined as control passes from one mech to another?

I addressed that immediately after asking the question :)  I believe
the correct answer for a mechanism that builds knowledge of the
initiator progressively (with each security context token round trip)
is to output the name only when that process is complete.  As for how
to represent all the details learned about the initiator, that's what
display syntax and name attributes are for.

> applications _might_ be allowed to do, but rather what we tell them they

Yes, but that requires considering the constraints on the
mechglue/provider.  In this case they must not output multiple
src_names -- that's just not something existing apps are prepared to
handle and makes no sense.

> completely stateless in between.  Fortunately for us, actually achieving

The mechglue/provider should either a) not output the src_name (and
deleg cred) until complete, or b) once they output either *keep*
outputing it until complete.  Nothing else is safe w.r.t. existing
applications.

> up front, and don't look at anything other than prot_ready until the

No, the mechglue/provider must keep state as described above.

Also, src_name can be left NULL -- the application can use
gss_inquire_context() instead.  So the only real problem here is
deleg_cred_handle.

Nico
--

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 21 Nov 2013 13:34:10 -0500 (EST)

> gss_inquire_context() instead.  So the only real problem here is

That's a good point.  My sample code doesn't even use the returned value, 
just requested it on the grounds that it is a common thing to want.

I'm somewhat inclined to modify the sample code to not request src_name.

-Ben

---

From: Nico Williams <[email-masked]>
Date: Thu, 21 Nov 2013 13:59:45 -0600

> I'm somewhat inclined to modify the sample code to not request src_name.

That's fine.  But we want devs who look at this code to know that they
must provide an authorization function, so a bit of code inquiring the
src name and a comment about the required authz check would be nice.

Nico
--

---

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 25 Nov 2013 22:22:52 -0500 (EST)

> already there, and then just calling free() in cleanup).

How would you feel about having stub routines send_token()/receive_token() 
which are just comments indicating what an implementation of them should 
do?  My original intention in including these routines was in fact to have 
sample code that would compile (and maybe run, if the mechanism supports 
nameless initiators), to make it as clear as possible what was necessary 
for an application implementation.  Subsequent discussion has convinced me 
that (as you say) including an actual implementation of those routines is 
unwise, but I would still like to be as explicit as possible what is 
necessary for applications.  A stub routine with comment seems like it 
could be more clear than just a call to an unimplemented routine with an 
argument 'readfd' or 'writefd'.

I will still have an implementation locally, so that I can test the sample 
code -- it wouldn't do to include buggy sample code.

> #ifdef, which is distracting.

We had previously discussed having separate sample code for the anonymous 
and non-anonymous case, but as I was writing it seemed like the 
differences were so small that duplicating everything was not worth it. 
However, I still wanted to make clear that if an application did not care 
about supporting anonymous initiators, the code in question could be 
removed; using a preprocessor conditional seemed the clearest way to do 
so.  I don't think the sense of "this code can be entirely removed" is as 
clear with a C-level conditional.

> simplified to "if (!(ret_flags & GSS_C_ANON_FLAG))", and similarly

This is true.  If you think it is more clear to write it that way, I guess 
I can change it for the next revision.

-Ben