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

[Wang Weijun <[e-mail]>]

Hi Thomas

On Mar 20, 2015, at 10:15, Thomas Maslen <[e-mail]> wrote:
> 
> 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.

The GSSException case is not broken. It's just that the Java binding does not have the same feature as the C binding, at least for the token-based methods. You may say people can find a way to send the error token using the stream-based methods without any API change, but since we never documented anything before it could bring unexpected behavior to current users. Also, it is too automatic and a user might want to send a normal token but not send an error token.

> 
> [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?

For me, the main reason of (a) is that GSS-API is defined around tokens and sending/receiving these tokens should be left to applications. Using them, documenting them, and implementing them, are all unnecessarily complicated.

> 
> 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.

The javadoc for the methods seems to suggest that

*     while (!context.isEstablished()) {
*
*         context.initSecContext(is, os);
*
*         // send output token if generated
*         os.flush();
*     }

> 
> 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[].

This is somehow what the example inside RFC 5653 7.4.8 is suggesting

      ByteArrayOutputStream os = new ByteArrayOutputStream();
      ByteArrayInputStream is = null;

          do {

              // Re-create os?
              context.initSecContext(is, os);

              // send token if present
              if (os.size() > 0)
                  sendToken(os);

              // check if we should expect more tokens
              if (context.isEstablished())
                  break;

              // another token expected from peer
              is = recvToken();

          } while (true);

but then the recvToken in the example above should not be defined as a ByteArrayInputStream.

> 
> 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[]).

Exactly, I don't know how to emit that length field before sending the bytes into the stream. Creating a method like the following is ugly.

  byte[] initSecContext(InputStream ins)

> 
> 
> 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.

I don't know now. I am also late to this.

> 
> 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).

We don't plan to make any change. What we can say now is that they have a problem and could go wrong. If we add a new mechanism later that does not have a self-framing, we might have to consume all bytes available and treat it as the input token.

--Max

> 
> 
> 
> ________________________________________
> 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