A fundamental JGSS-API bug?

[Xuelei Fan]

On 6/8/2013 9:31 AM, Weijun Wang wrote:
> 
> 
> On 6/7/13 11:49 PM, Sean Mullan wrote:
>>>          try {
>>>              send(initSecContext(inToken));
>>>          } catch (GSSException e) {
>>>              if (e.getResidue() != null) {
>>>                  send(e.getResidue());
>>>              }
>>>              throw e;
>>>          }
>>
>> That doesn't seem too complicated to me, all things considered. I think
>> this would be a reasonable solution. I would simply name the method
>> getToken instead of getResidue.
> 
> Or getOutputToken()? If the incoming token is a KRB_ERROR,
> initSecContext(token) will also throw a GSSException. Although I don't
> plan to provide a method to retrieve the incoming token, getToken()
> might be confusing.
> 
>>
>>> As for the overloaded initSecContext(InputStream, OutputStream) style,
>>> it looks easier to update this method to do the correct thing without
>>> any new API. However, the change will be very confusing because there is
>>> no more number of written bytes to return. More importantly, if it's
>>> just a silent behavior change, we'll have to care about compatibility
>>> (Maybe someone already added his/her own KRB-ERROR sending codes?),
>>> which makes the situation much tougher.
>>
>> Can you describe how the application code would use this method like you
>> did above for the other initSecContext method?
> 
> The recommended way was
> 
>   while (!context.isEstablished()) {
>      context.initSecContext(is, os);
>      os.flush();
>   }
> 
> When I say "easier", I mean it looks like there is no need to make any
> application change and this method can just write KRB_ERROR into os and
> *then* throw the exception.
> 
> But this is a behavior change and can be quite dangerous.
Can the scenarios look like:
1. need to reject the negotiation in this round to call initSecContext;
2. generate a reject message, and mark that the negotiation is
terminated, return as normal(no exception);
3. next round to call initSecContext(), as the negotiation has
terminated, throw exception accordingly.

Is the InputStream a potential problem that we cannot do above scenarios?

> I'd rather also write
> 
>   while (!context.isEstablished()) {
>      try {
>         context.initSecContext(is, os);
>      } catch (GSSException e) {
>         if (e.getOutputToken() != null) {
>            os.write(e.getOutputToken());
>            os.flush();  // in finally? not sure.
>            throw e;
>         }
>      }
>      os.flush();
>   }
> 
The coding style looks really strange to me.  If need to send something,
the initSecContext() should not throw exceptions.

Per RFC 2853, a common usage of initSecContext is as:

   do {
       byte[] outTok = context.initSecContext(inTok, 0, inTok.length);

       // send the token if present
       if (outTok != null)
           sendToken(outTok);


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

       // another token expected from peer
       inTok = readToken();
   } while (true);


I would like another approach if there is strong motivation to make the
change.  That's, the initSecContext() does not throw exception when a
reject message is needed (it should not, because reject message is the
right message of a negotiation process). But we add a new method to
identify the statue, isTerminated() or something similar.  Then the code
is updated to:

   do {
       byte[] outTok = context.initSecContext(inTok, 0, inTok.length);

       // send the token if present
       if (outTok != null)
           sendToken(outTok);


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

+      // check if we should expect more tokens
+      if (context.isTerminated())
+          break;  // fail break;

       // another token expected from peer
       inTok = readToken();
   } while (true);

Xuelei

> and add a clarification to the method saying no bytes will be written to
> os if there is a GSSException.
> 
> Thanks
> Max
> 
>>
>> --Sean
>>
>>>
>>> Thanks
>>> Max
>>>
>>> [1] http://tools.ietf.org/html/rfc2743#page-46
>>