Hi, Max,
KrbAsRep.java
=> 1) the javadoc of its decrypt method only documents the first two
arguments which seems incomplete. You meant to emphasize that there two
are user-provided? Maybe you can just enhance the method description. In
addition, only one of the keys and password arguments is used. Would it
be clearer to separate this into two methods, one uses keys and the
other uses password?
=> 2) one thing that I find somewhat confusing is the values of "creds".
The old model sets all of its fields in the constructor. With the new
model, creds is null until decrypt(...) is called.
KrbKdcReq.java
=> 1) for the if-block between line 229 and 232, is it possible for the
KRB_ERR_RESPONSE_TOO_BIG when 'useTCP' is true? And don't you have to
check ibuf again after line 231?
=> 2) I am open for a name change since the current naming seems to
imply an inheritance relationship which you've changed.
KrbAsReqBuilder.java
=> 1) line 72 "one and only is non-null" may be clearer as "one of them
must be null".
=> 2) keys(..) and pass(..) are initialization methods which must be
called before getKeys(), right? Can you rename them so it's clearer?
There is no checking in getKeys() and if called out of sequence, it
looks to me that it'll error out w/ NPE since eType is still null.
Thanks,
Valerie
On 07/21/10 12:32, Valerie (Yu-Ching) Peng wrote:
On 06/13/10 08:02, Weijun Wang wrote:
Hi Valerie and Andrew
Please review the following webrev:
http://cr.openjdk.java.net/~weijun/6960894/webrev.00
The major enhancement is KrbAsReqBuilder which generates AS-REQ, sends it,
parses any response, and returns a Credentials object. The other big change is
KrbKdcReq, it's no longer base class for KrbAsReq and KrbTgsReq, but mainly a
vehicle for both kinds of KDC-REQ messages. Maybe it needs a name change?
Most other changes are about removing duplicate lines.
Thanks
Max
Begin forwarded message:
*Change Request ID*: 6960894
*Synopsis*: Better AS-REQ creation and processing
=== *Description* ============================================================
The current AS-REQ creation and processing implementation:
1. spread into multiple source files and have duplicate codes
2. cannot deal with PA-DATA in AS-REP
3. only use a single salt, and write it into PrincipalName permanently
4. generate too many secret keys and have no consistent way to clear them
5. does not handle the preferences of PA-ETYPE-INFO2, PA-ETYPE-INFO correctly
