code review request: 6894072: always refresh keytab

[Valerie (Yu-Ching) Peng]

Sounds good to me. A word of caution: only *refresh* the set of keys 
that we put in and not those which other apps add.
Thanks,
Valerie

On 03/31/11 07:23 PM, Weijun Wang wrote:
>
>
> On 04/01/2011 10:09 AM, Valerie (Yu-Ching) Peng wrote:
>> Max,
>>
>> I like this new approach of yours better.
>> As for compatibility, you mentioned only one aspect, i.e. apps which put
>> KerberosKeys inside a subject's private cred set.
>> There is also a possibility that apps may read the subject's private
>> credentials set for KerberosKeys that we used to put in and they won't
>> find the keys anymore since it's the KeyTab objects that we put into the
>> private cred set after this dynamic keytab support. Thoughts?
>
> No, I haven't thought about it.
>
> We can put a snapshot of keys there at the beginning and refresh them 
> whenever a getKeys() is called. This should be harmless because we 
> don't really use the keys if keytab objects (not keytab files) exist. 
> I can do that.
>
> Thanks
> Max
>
>
>>
>> Valerie
>>
>>
>> On 03/31/11 03:41 AM, Weijun Wang wrote:
>>> Hi Valerie
>>>
>>> Sorry for the late reply. I've considered some alternatives.
>>>
>>> A "to-be-resolved" KerberosKey is quite difficult. When it's resolved,
>>> we have a list of keys with different etypes as the private
>>> credentials. If it's not resolved, we can only create one, whose
>>> encoding and etype are both unresolved, and when it finally gets
>>> resolved, it's resolved into multiple keys. Also, there was a simple
>>> mapping between KerberosKey and EncryptionKey. The resolving process
>>> is not always at the same time as the mapping (converting from one to
>>> another) time, so it seems EncryptionKey might also needs to be
>>> unresolved.
>>>
>>> Another solution is to revert back to my original KeyTab without an
>>> intended user. This means several changes:
>>>
>>> In my latest version of ServiceCreds, there were multiple keys and
>>> *one* keytab, now we also need multiple keytabs, because there might
>>> be multiple keytabs in the subject's private credentials set and we
>>> cannot tell which is for who. Therefore we collect all of them, when
>>> the keys are needed at AP-REP time, we call getKeys() on all of them,
>>> and return the combination. Hopefully there won't be two keys for the
>>> same principal with same kvno and same etype. I regard that as an 
>>> abuse.
>>>
>>> Currently when there is no serverPrincipal specified in the
>>> Krb5AcceptCredential construction, we pick a KerberosKey from the
>>> private credentials set and use the KerberosPrincipal info inside, and
>>> then get all KerberosKeys for the same principal. We have never really
>>> looked at any KerberosPrincipal objects in the subject's principal
>>> set. I had tried to do the same to KeyTabs in my webrev.02. Now this
>>> will have a big change: the first step is always finding a
>>> KerberosPrincipal in the principal sets first. If serverPrincipal is
>>> specified, we find a matched one, otherwise, we just pick one and use
>>> it. And then, from the private credentials set, we fetch all
>>> KerberosKeys for this principal and all KeyTabs.
>>>
>>> I think this is the correct way to go. Of course, for compatibility
>>> reason, we assume there are third party codes that put KerberosKeys
>>> inside a subject's private credentials set but hasn't put any
>>> KerberosPrincipal there. Our Krb5LoginModule will never do it, but we
>>> can accept it.
>>>
>>> Here is a partial webrev:
>>>
>>> http://cr.openjdk.java.net/~weijun/6894072/webrev.03
>>>
>>> It only contains changes for Krb5Util.java, and hasn't included the
>>> compatibility codes I mentioned above. As you can see, the KeyTab
>>> objects are now retrieved by
>>>
>>> + sc.ktabs = SubjectComber.findMany(
>>> + subj, null, null, KeyTab.class);
>>>
>>> so no principal name is used, and we retrive "many".
>>>
>>> If you think this is OK, I'll clean up other codes. One benefit is
>>> that we don't need to update CCC with this solution. Yes we do
>>> introduce new hashCode/equals/toString methods, but I think they do
>>> not deserve a CCC.
>>>
>>> Thanks
>>> Max
>>>
>>>
>>> On 03/26/2011 08:20 AM, Valerie (Yu-Ching) Peng wrote:
>>>> Max,
>>>>
>>>> Well, I find it a bit awkward that the KeyTab class has to have the
>>>> KerberosPrincipal info which "intends" to use it.
>>>> Have you considered a different approach like:
>>>> Instead of adding the whole KeyTab object into the Subject's private
>>>> credential set, we add a "to-be-resolved" KerberosKey object. When we
>>>> need to use this kind of key, we'd check the associated KeyTab 
>>>> object to
>>>> re-fresh its value if needed. This approach is conceptually closer to
>>>> what we had and the changes aren't as dramatic and seems to meet the
>>>> need required by 6894072.
>>>>
>>>> I'll continue to review your webrev, but just want to kick this 
>>>> idea off
>>>> w/ you and see if it may work.
>>>> Valerie
>>>>
>>>> On 03/23/11 02:00 AM, Weijun Wang wrote:
>>>>> Hi Valerie
>>>>>
>>>>> Updated webrev:
>>>>>
>>>>> http://cr.openjdk.java.net/~weijun/6894072/webrev.02
>>>>>
>>>>> Changes since last version:
>>>>>
>>>>> 1. A KerberosPrincipal inside javax..KeyTab class. New getInstance()
>>>>> arguments, new getPrincipal() method.
>>>>>
>>>>> It can only be non-null now, but I didn't say anything in the spec.
>>>>> I'm hoping it can be null in the future to support multiple service
>>>>> principal in a single service.
>>>>>
>>>>> 2. toString(), hashCode(), equals() for KeyTab, since it will be put
>>>>> inside private credentials set.
>>>>>
>>>>> 3. Enhancement to SubjectComber:
>>>>> a) Generics for find() and findMany()
>>>>> b) findAux() now support Krb5AcceptCredential
>>>>>
>>>>> 4. Krb5Util.ServiceCreds: since principal is already inside both
>>>>> KeyTab and KerberosKey, no more KerberosPrincipal argument in
>>>>> getInstance(), there is still a field inside to save the value.
>>>>>
>>>>> 5. sun..KeyTab and javax..KeyTab: isMissing==true is now valid.
>>>>> Changes to the javadoc of javax..KeyTab.getKeys().
>>>>>
>>>>> 6. New TwoPrinces.java test, a subject with 2 KerberosPrincipal after
>>>>> JAAS commit.
>>>>>
>>>>> This time I'd like to first make sure implementation is correct, and
>>>>> then I'll update the CCC. Is this OK?
>>>>>
>>>>> Thanks
>>>>> Max
>>>>
>>