Re: [kitten] On stream-based GSSContext methods in RFC 5653

[Thomas Maslen <[e-mail]>]

My personal 0.02.

First, my overall reaction to this is pretty much the same as my reaction to the GSSException change proposed in the rfc5653bis draft:  I believe that this aspect of 2853 / 5653 isn't broken and so I'm leery of "fixing" it -- but I don't feel strongly about it and I don't really object to it (much), I mostly just want to make sure that we look before we leap.

[But I do agree that if (a) we deprecate the stream-based GSSContext methods and (b) we care about being able to both throw a GSSException and return an error token, then (c) yes, in that case the GSSException change would then be necessary].

To me, neither item 1 nor item 2 below is problematic for the stream-based methods.  Perhaps the assumptions that I'm using are different than yours?

What I mean by that is:

I agree that if you took the raw InputStream from e.g. Socket.getInputStream() and passed it to a stream-based GSSContext method then yes, it would be problematic.

But don't do that.  Instead:

    (1) Read the token-length field from the raw InputStream.  (There must be a token-length field, or something equivalent to it -- if there wasn't, you wouldn't be able to use the byte[]-based GSSContext methods either)

    (2) Use that length field to construct a wrapper InputStream (probably a subclass of FilterInputStream) that lets you read that number of bytes and then returns EOF (even though the underlying raw InputStream probably still has data available).  Yes, I'm definitely glossing over some details about ensuring that you always advance the underlying stream to the end of the token, but that's just a SMOP.

    (3) Pass that wrapper InputStream (not the underlying raw InputStream) to the stream-based GSSContext method.

The advantage of doing this, c.f. just using the byte[]-based methods, is efficiency:  with the byte[]-based methods you have to read the token length, then allocate a byte[] that's at least that large and System.arraycopy() all those bytes into it.  [Unless, of course, you have already paid the price to read the entire application-protocol message into a byte[] -- in that case, sure, it makes sense to use the byte[]-based GSSContext methods].

So I believe that the InputStream parameters are useful because they allow a more efficient implementation than using byte[].

I admit that the argument for the OutputStream parameters is much weaker (hypothetically they could be useful if the application protocol used chunked framing such as ASN.1 indefinite-length encoding, but in reality application protocols probably just use simple framing with a token-length field at the beginning and so yeah, the OutputStream doesn't buy you anything, you may as well just go ahead and return a byte[]).


I agree that the "GSS-API authentication tokens contain a definite start and end" wording in RFC 2853 / 5653 makes it sound as though perhaps the original authors mistakenly believed that all GSSAPI context tokens included their own framing (if so, then the stream-based methods were just a mistake caused by a flawed assumption, and it would be good to correct that mistake).  I'm a Johnny-come-lately to this and I don't know what the original authors actually thought / meant, but I choose to believe that no, they weren't confused about the (non-)framing of GSSAPI context tokens and so no, the stream-based GSSContext methods weren't a mistake;  rather, I believe that they were thinking about InputStream wrappers such as the one I outlined above, so the stream-based GSSContext methods are valid (and shouldn't be deprecated), and the only thing that needs to be fixed is the misleading wording, not the Java methods.

If we do go ahead with deprecating the stream-based methods then yes, I'm fine with the changes that you outlined below:

> [...] I propose to deprecate these methods in an I-D.
> [...] In Oracle JDK and OpenJDK, we would like to mark these methods as @deprecated. The existing implementations will be still supported.

Good.

Incidentally, w.r.t. Oracle JDK / OpenJDK:  the current implementation assumes that all context-initiation tokens (not just the initial token but also all subsequent context-initiation tokens) _do_ include framing (a GSSHeader).  There are a couple of special-case exceptions (for the SPNEGO mechanism and for the SunNativeGSS provider), but in general the code requires that all input tokens have a GSSHeader (and will error out, probably with DEFECTIVE_TOKEN, if they don't), and always prepends a GSSHeader to any outgoing tokens that GSSContextSpi returns.

So, even if the stream-based methods are deprecated, the current implementation will still only support GSSAPI mechanisms where every context-initiation token has GSSHeader framing.  My guess is that removing that limitation would entail breaking the sun.security.jgss.spi (again).



________________________________________
From: Wang Weijun [[e-mail]]
Sent: Sunday, March 15, 2015 20:46
To: [e-mail]; OpenJDK Dev list
Cc: Thomas Maslen; Benjamin Kaduk
Subject: On stream-based GSSContext methods in RFC 5653

Hi All

I discussed with my colleagues on the stream-based methods and we think they are not well-designed:

1. A library should not define a wire protocol and assume the peer is using the same. http://tools.ietf.org/html/draft-ietf-kitten-gss-loop-05 requires the application to define it.

2. It's impossible to implement these methods correctly when the mechanism token has no self-framing or the library has no knowledge of the token format (for example,
as a bridge talking to another GSS library).

Therefore, I propose to deprecate these methods in an I-D. By deprecation, I'd like to remove the methods from the main content and describe the reason in a "Changes since RFC 5653" section. The specification for the methods will be moved to an appendix or the "Changes since RFC 5653" section will contain links to sections inside RFC 5653.

In Oracle JDK and OpenJDK, we would like to mark these methods as @deprecated. The existing implementations will be still supported.

Thanks
Max