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

[Benjamin Kaduk <[e-mail]>]

On Tue, 10 Feb 2015, Thomas Maslen wrote:

> On 10 Feb 2015 18:40:25 +0800, Wang Weijun <[e-mail]> wrote
> (in response to one of Ben's review comments):
>
> 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.

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.

> (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() ?

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

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

-Ben