[security-dev 00756]: Re: Code review request: Undefined requesting URL in "java.net.Authenticator.getPasswordAuthentication()"

Weijun Wang Weijun.Wang at Sun.COM
Thu Apr 16 08:46:54 UTC 2009


Hi Chris/Valerie

Can you take a review on a related bug. I found it when I wrote the test
for the previous one.

6829283: HTTP/Negotiate: Authenticator triggered again when user cancels
the first one
http://cr.openjdk.java.net/~weijun/6829283/webrev.00/

Basically, it's because for HTTP/Negotiate, it's

   ... -> Callback -> Authenticator

We have 2 callbacks (user and pass) in JAAS, but there's only 1
Authenticator (doing user and pass at the same time). If user cancels
the first call, we shouldn't bother her again.

Thanks
Max

Max Wang (Weijun) wrote:
> Hi Chris
> 
> A new webrev is created at
>     http://cr.openjdk.java.net/~weijun/6578647/webrev.01
> 
> Now all HttpCallerInfo creations are inline, so the diff is much
> clearer. There's one place I didn't call toLowerCase(), the call is
> moved into NegotiatorImpl right before the service principal name is
> created.
> 
> I also add a test, putting two Kerberos KDC, one HTTP server, one proxy
> server in a single regression test is fun!
> 
> Thanks
> Mx
> 
> On Apr 14, 2009, at 8:55 PM, Max (Weijun) Wang wrote:
> 
>>
>> On Apr 14, 2009, at 5:59 PM, Christopher Hegarty - Sun Microsystems
>> Ireland wrote:
>>
>>> Hi Max,
>>>
>>> I only looked at the networking part of the changes. They look fine,
>>> I just have a few questions/comments:
>>>
>>> 1) sun.net.www.protocol.http.HttpURLConnection
>>>  Can you use the same HttpCallerInfo instance for proxy authentication
>>>  at line 1108? This instance has been created using the single arg
>>>  constructor therefore it is has authType = RequestorType.SERVER,
>>>  right?
>>
>> Yes, you're right. Will update tomorrow.
>>
>>>
>>> 2) sun.net.www.protocol.http.HttpCallerInfo
>>>  It is just my preference, but I would prefer to see all the fields of
>>>  HttpCallerInfo private and have simple accessors:
>>>      private final String host;
>>>      ......
>>>
>>>      public String host() {
>>>          return host;
>>>      }
>>>      ......
>>
>> Your suggestion is more formal. But I think making all fields final is
>> also sufficient to make it immutable.
>>
>>>
>>> 3) Are the changes to use HttpCallerInfo in AuthenticationHeader,
>>>  HttpURLConnection, NegotiateAuthentication and NegotiatorImpl
>>>  strictly necessary? They seem to be changed just for consistency of
>>>  using the new class. I only see that NegotiateCallbackHandler is
>>>  required to use this new class on the networking side.
>>
>> There needs a way to transfer these info into the JGSS underneath (so
>> that NegotiateCallbackHandler has a chance to know them), and the only
>> bridge is inside NegotiatorImpl. I don't know if there's a better way
>> to do this. The HttpClient class seems having similar info but
>> sometimes it's null and I don't know why. Sorry if I reinvent a
>> wheel-cart to carry these info.
>>
>> Thanks
>> Max
>>
>>>
>>>  This is not a problem just a question to see if I understand
>>>  correctly the changes.
>>>
>>> -Chris.
>>>
>>>
>>>
>>>
>>>
>>> On 04/13/09 03:27, Weijun Wang wrote:
>>>> Hi Valerie and Networking guys
>>>> Please take a review at this bug fix:
>>>>   http://cr.openjdk.java.net/~weijun/6578647/webrev.00/
>>>> The bug is
>>>>   http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6578647
>>>> The bug report says that no URL-related info is available in
>>>> Authenticator when using HTTP/Negotiate. The reason is that in the long
>>>> stack of
>>>>  HTTP/Negotiate -> JGSS -> JAAS -> Krb5LoginModule
>>>>      -> Callback -> Authenticator
>>>> The URL info is lost. In order to support special actions for
>>>> HTTP/Negotiate calls in JGSS (say, using Authenticator instead of
>>>> text-based callback, honor the OK-AS-DELEGATE flag...), we already used
>>>> an integer field (caller) to tell the codes deep below who initiates
>>>> the
>>>> JGSS calls. It seems an integer is not enough to carry too much
>>>> information. (oh, I love the C void*)
>>>> The fix is simple: change the caller from integer to a Java class:
>>>> GSSCaller, which includes as much as info it likes. For HTTP/Negotiate,
>>>> a child class HttpCaller, encapsulates all info an Authenticator needs.
>>>> The fix includes three parts:
>>>> 1. Three new classes:
>>>>  sun.sec.jgss.GSSCaller:
>>>>      the new caller
>>>>  sun.sec.jgss.HttpCaller:
>>>>      a child of GSSCaller, knows everything about HTTP
>>>>  sun.net.www.protocol.http.HttpCallerInfo:
>>>>      the info GSSCaller knows, this class is created on the
>>>>      network side so that no sun.security.jgss.* codes are
>>>>      dragged into the bootstrap building process.
>>>> 2. On the network side:
>>>>  Refactoring HTTP codes in sun.net.www.protocol.http.* to fill info
>>>>  into the HttpCallerInfo class.
>>>> 3. On the JGSS side:
>>>>  Multiple changes in sun.security.jgss.* classes. *All* the
>>>>  code changes are simply s/int/GSSCaller/g changes.
>>>>  I also moved the pre-defined callers from GSSUtil to
>>>>  GSSCaller.
>>>> Thanks
>>>> Max
>>
> 



More information about the security-dev mailing list