Subject: Re: [kitten] draft-ietf-kitten-rfc5653bis-02 review

From: Thomas Maslen <[email-masked]>
Date: Wed, 11 Feb 2015 03:54:17 +0000

On 10 Feb 2015 18:40:25 +0800, Wang Weijun <[email-masked]> wrote
(in response to one of Ben's review comments):
> [...]
> I will spend some time talking with my colleagues on your opinions on stream-based methods.

For whatever it's worth...

I agree with most of what Ben wrote in his review, particularly the long-standing text he flagged in RFC 2853 / 5653 / rfc5653bis which seems to assume that all tokens -- not just initial tokens -- received by GSSContext.initSecContext() and GSSContext.acceptSecContext() use the mechanism-independent token format.

However, I don't agree with this comment in the review:

>> [...]
>> Since this essentially involves generating byte-array tokens everywhere
>> and generating single-use streams from them, it makes me wonder what
>> utility the stream forms of these routines provide.

I believe that the "Since this essentially involves generating byte-array tokens everywhere" premise isn't really true, and I believe that the stream-based methods are definitely useful and don't need to be revisited.

(Aside:  now that I have read the sample code in RFC 2853 et seq for the stream-based methods, I see what led Ben to believe that this essentially involves generating byte-array tokens everywhere, but I believe that's just a problem with the usefulness of the sample code, not a problem with the usefulness of the stream-based methods).

To me the stream-based methods are the fundamental form, whereas the byte-array methods are just convenience methods:  they provide a convenient but less efficient alternative, and it wouldn't have been a tragedy if RFC 2853 had omitted the byte-array methods and included only the stream-based methods.

Since I regard the byte-array methods as just convenience methods, I think that it's nice if they can provide all the same functionality as the stream-based methods but it definitely isn't a requirement -- to me it's OK if the byte-array methods don't support some more arcane cases.

Given that, my reaction to the rfc5653bis drafts is similar (I think) to Greg Hudson's comment on 1 Feb 2015:  the stream-based methods already kinda sorta provide a way to do this -- write the error token to the OutputStream, then throw the GSSException.  For me it's fine that only the stream-based methods support this and the byte-array methods don't, but I understand that others may differ, including of course Weijun.

w.r.t. the main change in the rfc5653bis drafts (i.e. adding the getOutputToken() method to GSSException), ideally I would like to see some tweaks:

(1) In the updated example code for the stream-based methods, the "sendToken(new ByteArrayOutputStream(outTok));" code in the catch-block is bogus (and also won't compile), right?

(2) [Perhaps someone already mentioned this one, maybe Greg?]  The stream-based methods now have two ways that they could convey an error token:  either write it to the OutputStream or return it in the GSSException (but not both, I trust).  I didn't notice any text that specifies (a) what the JGSS implementation is required to do (and not do) w.r.t. this, and (b) what the calling code should / shouldn't expect, but perhaps I overlooked it.  Would the rule be "If the implementation is generating an error token, it must never write any bytes to the OutputStream, and must always return the token in the GSSException", or something else?

(3) GSSException is used throughout the JGSS API:

        http://docs.oracle.com/javase/7/docs/api/org/ietf/jgss/class-use/GSSException.html

but I assume that the only places where GSSException.getOutputToken() can be non-null (and calling code should handle it) are after the two initSecContext() methods and the two acceptSecContext() methods -- right?  That's more or less implicit in the current draft, but could the section 6.8 intro and / or section 6.8.7 be a bit more explicit that getOutputToke() is only pertinent to initSecContext() and acceptSecContext() ?

... but none of them are anywhere close to being "over my dead body" issues.

Overall, given a choice between RFC 5653 and the current rfc5653bis drafts, my 0.02 would be just to keep RFC 5653 as-is, but again that's just a mild preference.

---

From: Greg Hudson <[email-masked]>
Date: Wed, 11 Feb 2015 11:33:23 -0500

[...]
[...]
> To me the stream-based methods are the fundamental form, whereas the byte-array methods are just convenience methods:  they provide a convenient but less efficient alternative, and it wouldn't have…

I am having trouble reconciling these statements.

The issue with token framing is not just an issue with the wording in
the RFC; it calls into question the whole mode of operation.  An
implementation of init/accept_sec_context cannot know how much data to
read from the input stream in order to read a token, since GSS tokens do
not necessarily have self-describing lengths.  The only way this can
really work is if the application cooperates by creating a single-use
input stream which ends at the end of the token.

Similarly, output tokens must be externally framed by the protocol.  An
application cannot simply provide its outgoing network buffer to
init/accept_sec_context; it must package up the data written to the
output stream and frame it.  In effect, the output stream must be
transformed into a byte array.

---

From: Thomas Maslen <[email-masked]>
Date: Wed, 11 Feb 2015 19:34:06 +0000

> input stream which ends at the end of the token.

Yes but no:

I agree that the application must create a single-use input stream which ends at the end of the token.

But you can do that without having to read and buffer the token.

It's easy enough to write a new subclass (I may as well call it BoundedInputStream) that takes an underlying InputStream and a length count, and produces an InputStream which lets you read that number of octets but no more (unless you go around it and get to the underlying InputStream).

This is a pretty normal sort of thing to do in Java, and it's normal enough that even JDK 1.0 had a convenience class -- http://docs.oracle.com/javase/7/docs/api/java/io/FilterInputStream.html -- that acts as a helpful base class for this sort of thing.  (The standard subclasses of FilterInputStream are mostly designed to munge the sequence of octets in the stream, not to virtualize end-of-stream, but the general approach is similar).

> transformed into a byte array.

I agree that the argument is weaker for output tokens, because the majority of application protocols that convey GSS tokens probably use simple framing where the token is preceded by a length count for the entire token -- so yeah, in that case you're pretty much stuck with buffering the token in order to determine its length, so you may as well have used the byte array instead.  Hypothetically an application protocol could use a chunked encoding, e.g. ASN.1 indefinite-length or HTTP/1.1 chunking, and then you could write a subclass of java.io.FilterOutputStream that only had to buffer up to some chunk size, not the whole token...  but likely all non-hypothetical protocols that use GSS tokens just use the simple whole-token length count?

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 12 Feb 2015 17:39:09 -0500 (EST)

On Tue, 10 Feb 2015, Thomas Maslen wrote:

> I believe that the "Since this essentially involves generating
> byte-array tokens everywhere" premise isn't really true, and I believe
> that the stream-based methods are definitely useful and don't need to be
> revisited.

My statement was intentionally strong and intended to spark discussion :)
It may well be that they should remain, but I do think that what we are
doing right now in revisiting whether they have utility, is a discussion
worth having.

> (Aside:  now that I have read the sample code in RFC 2853 et seq for the
> stream-based methods, I see what led Ben to believe that this
> essentially involves generating byte-array tokens everywhere, but I
> believe that's just a problem with the usefulness of the sample code,
> not a problem with the usefulness of the stream-based methods).

This seems like a fine opportunity to improve the sample code, since we
are doing the bis document.  (ter, really, I guess...)

> To me the stream-based methods are the fundamental form, whereas the
> byte-array methods are just convenience methods:  they provide a
> convenient but less efficient alternative, and it wouldn't have been a
> tragedy if RFC 2853 had omitted the byte-array methods and included only
> the stream-based methods.
>
> Since I regard the byte-array methods as just convenience methods, I
> think that it's nice if they can provide all the same functionality as
> the stream-based methods but it definitely isn't a requirement -- to me
> it's OK if the byte-array methods don't support some more arcane cases.

I think that your sentiment is too strong to be supportable.

While there is a strong sequencing requirement for context-level tokens
(the negotiation loop proceeds in lockstep), and so a stream-based method
would definitely be workaboe there, the per-message operations do not have
that ordering requirement.  I think it would be an unreasonable burden on
an application to expect it to supply input streams to the per-message
GSS operations which are guaranteed to only supply a single token.  Even
though it is possible to do, it requires a lot of effort to do correctly.

Regarding whether either the byte-array or stream methods are just
"convenience methods" around the other, I believe that since both are
listed in the RFC, both must be considered as first-class methods that
provide all the functionality of the underlying abstract GSS-API (RFC
2743) operations.  I would not find it reasonable to provide reduced
functionality in a standardized function like this.  (Some implmentation
could provide its own extension that has reduced functionality, of course,
and that would be fine.)

> Given that, my reaction to the rfc5653bis drafts is similar (I think) to
> Greg Hudson's comment on 1 Feb 2015:  the stream-based methods already
> kinda sorta provide a way to do this -- write the error token to the
> OutputStream, then throw the GSSException.  For me it's fine that only
> the stream-based methods support this and the byte-array methods don't,
> but I understand that others may differ, including of course Weijun.

My ideological preference would be consistency between the flavors, but I
would accept a solution that explicitly spelled out that the stream-based
methods handled error tokens in one way and the byte-array methods handled
error tokens in a different way.  The reason that I am arguing for the
exception to carry the error token in the stream method is that it seems
the best way to clearly spell out how to generate error tokens in the way
which is most likely to be backwards-compatible.  Having the
implementation automatically insert the error token into the stream seems
like it introduces a risk of duplicating the token (albeit a very small
one), and I don't see that risk if the exception carries the token.

> w.r.t. the main change in the rfc5653bis drafts (i.e. adding the
> getOutputToken() method to GSSException), ideally I would like to see
> some tweaks:
>
> (1) In the updated example code for the stream-based methods, the
> "sendToken(new ByteArrayOutputStream(outTok));" code in the catch-block
> is bogus (and also won't compile), right?
>
> (2) [Perhaps someone already mentioned this one, maybe Greg?] The
> stream-based methods now have two ways that they could convey an error
> token:  either write it to the OutputStream or return it in the
> GSSException (but not both, I trust).  I didn't notice any text that

Greg did mention this, yes.

> specifies (a) what the JGSS implementation is required to do (and not
> do) w.r.t. this, and (b) what the calling code should / shouldn't
> expect, but perhaps I overlooked it.  Would the rule be "If the
> implementation is generating an error token, it must never write any
> bytes to the OutputStream, and must always return the token in the
> GSSException", or something else?

I think the document would benefit from some more explicit text about what
the implementation is expected to do, yes.

> but I assume that the only places where GSSException.getOutputToken()
> can be non-null (and calling code should handle it) are after the two
> initSecContext() methods and the two acceptSecContext() methods --
> right?  That's more or less implicit in the current draft, but could the
> section 6.8 intro and / or section 6.8.7 be a bit more explicit that
> getOutputToke() is only pertinent to initSecContext() and
> acceptSecContext() ?

I agree that the text should be more explicit about this, yes.

Thank you for the review; these are definitely useful comments.

-Ben

---

From: Thomas Maslen <[email-masked]>
Date: Sun, 15 Feb 2015 01:22:55 +0000

> though it is possible to do, it requires a lot of effort to do correctly.

Quite possibly I'm missing something obvious or just haven't thought this through, but:

Even in really complex scenarios, e.g. running with requestSequenceDet(false) + concurrent / unordered delivery of application messages from the peer (e.g. using UDP or a separate TCP connection for each) + multiple threads operating concurrently to receive these application messages and present their per-message tokens to the GSSContext instance, I believe that it's as easy (or as difficult) to use the stream-based methods as it is to use the byte-array methods.

Possibly relevant:  note that the spec (2853, 5653, 5653bis) doesn't promise anything about concurrency w.r.t. the methods of a GSSContext instance (nor about anything else in the API, for that matter) -- in particular, the Java keyword "synchronized" doesn't appear anywhere.  [Yes, it might be a good thing if this were more explicit in the spec, whereas at present it's very very implicit;  by contrast, GSSContext's Javadoc in the JDK

    http://docs.oracle.com/javase/1.5.0/docs/api/org/ietf/jgss/GSSContext.html

comes right out and says "Also note that none of the methods in this interface are synchronized. Therefore, it is not advisable to share a GSSContext among several threads unless some application level synchronization is in place"].

The byte-array methods may intuitively seem to be kinda sorta "more atomic" than the stream-based methods, but they aren't;  if you're using multiple threads to feed per-message tokens to a GSSContext instance then you're responsible for serializing the calls, regardless of whether you're using the byte-array methods or the stream-based methods.

[Skipping...]

> one), and I don't see that risk if the exception carries the token.

I agree;  if, per the rfc5653bis drafts, GSSException is extended to carry the error token, then I think it's reasonable (and probably a good idea) to say that the implementation should always convey the error token in the GSSException, never in the OutputStream.

[One caveat, but it's probably not worth worrying about because this may be strictly hypothetical:  if there are any cases where an existing JGSS implementation has already been using the OutputStream to convey an error token (because that was the only approach that was available in 2853 or 5653) and real application code has been making use of this, then we might be outlawing something that has worked up until now...  but my guess is that this is only a hypothetical concern].

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 18 Feb 2015 16:45:09 -0500 (EST)

On Sat, 14 Feb 2015, Thomas Maslen wrote:

> The byte-array methods may intuitively seem to be kinda sorta "more
> atomic" than the stream-based methods, but they aren't;  if you're using
> multiple threads to feed per-message tokens to a GSSContext instance
> then you're responsible for serializing the calls, regardless of whether
> you're using the byte-array methods or the stream-based methods.

I suspect you are right, and I am just having a hard time getting out of
my array-based mindset instilled from the C bindings.

Sorry about that,

Ben