You are viewing a plain text version of this content. The canonical link for it is here.
Posted to kerby@directory.apache.org by Steve Moyer <sm...@psu.edu> on 2016/02/12 22:34:20 UTC

Re: Kerby client library refactoring

All, 

I've finally found the time to really dig into the Kerby client code with an eye towards adding remote kpasswd and kadmin functionality. Today, we found what I believe to be the last area where the Kerby client generates different packets than the MIT CLI tools. I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types. I also think there's some refactoring to do around how the client builds the request - the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client. 

In any case, adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way. I'd propose the following hierarchy: 

                                      +----------------+
                                      | KrbClient (AS) |
                                      +----------------+
                                         ^          ^
                                         |          |
                        +----------------+          +-------------------+
                        |                                               |
                  +-----------+                                +-----------------+
                  | KdcClient |                                | <Protocol> (AP) |
                  +-----------+                                +-----------------+
                     ^  ^  ^                                      ^           ^
                     |  |  |                                      |           |
      +--------------+  |  +-------------+                   +----+           +----+
      |                 |                |                   |                     |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
| KinitTool |     | KvnoTool |     | KlistTool |     | KpasswdClient |     | KadminClient |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
                                                             ^                     ^
                                                             |                     |
                                                      +-------------+       +------------+
                                                      | KpasswdTool |       | KadminTool |
                                                      +-------------+       +------------+

Some important notes about where we would go from here:

1)  The kpasswd and kadmin specifications share a protocol but don't share commands.  I haven't thought of a good name for the class yet so <Protocol> is the placeholder.  These commands <use> a ticket retrieved via an AsRequest but the commands themselves are sent via ApRequests.

2)  The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.

3)  Any class above that includes the word "Tool" uses the class above it (sorry about the ASCII art).  Currently, the KrbKdcOption enum include flags for the Tool but SoC would suggest that the arguments are a concern of the Tool itself.  I think it would also be easier to manage argument parsing with args4j or commons-cli and that the English title of the flag should also be moved out of the enum.

4)  We might want a base class for some of the tools as they do have some commonality.

5)  There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?

6)  We don't have an ApRequest in the client yet, if we share the AsRequest with the server I'd suggest the same be done for ApRequest.

We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.

Have a great weekend,

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Zheng, Kai" <ka...@intel.com>
To: kerby@directory.apache.org
Sent: Monday, January 11, 2016 7:29:20 AM
Subject: FW: Kerby client library refactoring



Sorry, was working on some other projects. My thought was instead of 
code that looks like this: 

requestOptions = new KOptions(); 
requestOptions.add(KrbOption.USE_TGT, tgt); 
//requestOptions.add(KrbOption.SERVER_PRINCIPAL, 
"HTTP/freeipa.rhelent.lan"); 
requestOptions.add(KrbOption.SERVER_PRINCIPAL, new 
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO 
WN)); requestOptions.add(KrbOption.FORWARDABLE,true); 
requestOptions.add(KrbOption.PROXIABLE,false); 
requestOptions.add(KrbOption.RENEWABLE_OK,false); 

I would think this would be more OO: 

requestOptions = new KOptions(); 
requestOptions.setTgt(tgt); 
//requestOptions.setServerPrincipal("HTTP/freeipa.rhelent.lan"); 
requestOptions.setServerPrincipal(new 
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO 
WN)); 
requestOptions.setForwardable(true); 
requestOptions.setProxiable(false); 
requestOptions.setRenewable(false); 

Could keep it backed by a set of options 




Agreed. This is fully compatible with the definition of all the KrbOptions enums, except thay will not be visible by the end user. 

RE: Kerby client library refactoring

Posted by "Li, Jiajia" <ji...@intel.com>.
Thanks for Yan's great work on XDR, I will review the new JIRAs.

Regards,
Jiajia

-----Original Message-----
From: Yan, Yan A [mailto:yan.a.yan@intel.com] 
Sent: Friday, February 19, 2016 2:13 PM
To: kerby@directory.apache.org
Subject: RE: Kerby client library refactoring

Thanks for Kai's advice. I will do that.

Best regards,
Yan


-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com] 
Sent: Friday, February 19, 2016 2:06 PM
To: kerby@directory.apache.org
Subject: RE: Kerby client library refactoring

Thanks Yan for working on this. I understand you're saving your codes in your github, but still better to create JIRA entries for your target tasks. When your partly work is in good shape, please push request for the review and get the codes in timely. Thanks.

Regards,
Kai

-----Original Message-----
From: Yan, Yan A [mailto:yan.a.yan@intel.com] 
Sent: Friday, February 19, 2016 1:52 PM
To: kerby@directory.apache.org; Steve Moyer <sm...@psu.edu>
Subject: RE: Kerby client library refactoring

Hi, all

I was reading your discussion about adding the remote kadmin and kpasswd functionality, and I hope I can help with these modules.

In fact, I have read codes on kadmin-remote branch which has two new different modules from trunk branch (kerb-admin and kerb-admin-server).

If there is no other suggestions, I can help work on the kadmin-remote branch as you discussed before.

If someone has also started working on this issue, please let me know, and I'm glad to help share some parts.

Best regards,
Yan

-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com] 
Sent: Tuesday, February 16, 2016 2:46 PM
To: kerby@directory.apache.org; Steve Moyer <sm...@psu.edu>
Subject: RE: Kerby client library refactoring

Sorry for the late reply.

>> I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types
Sounds good to me. A question is, in your case, where do you get the encryption types list? If you get them from a configuration, then this might not be needed because KrbClient can pick up them by itself.

>> the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client.
Did you find any issue about reading them back out? Can we enhance the codes to allow you to use KrbClient as flexible as you would build the request yourself? Not sure what's exactly needed here. Exposing AsRequest level is a little tricky because it's still in fast evolving.

>> adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way.
Agree. We can add kpasswd and kadmin similarly as KrbClient. We had made some initial work in kadmin-remote branch for the prototype, as Jiajia mentioned. To avoid big impact for existing codes, we simply duplicated the codes and adapt accordingly. Eventually we may refactor the codes and avoid the duplication codes after we make it work first and have the unit tests.  Note there're some differences though: 1) KrbClient is against KDC, kpasswd and kadmin are against kadmind; 2) kadmin needs to authenticate via GSSAPI; 3) kadmin uses XDR for the RPC; 4) they may use different configurations. Considering such, it may sound reasonable to develop the new functionalities separately, instead of redesign the whole architecture first.

>> The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.
I thought it would be enough for kpasswd to support the change password protocol right, since all vendors use the spec?

>> We might want a base class for some of the tools as they do have some commonality.
Yeah, agree. We'll need add unit tests for the tools as well.

>>There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?
AsRequest is a helper class to build or handle the wire message type of AsReq. It looks roughly same in client and KDC sides, but still quite much difference, not to share allows the codes can evolve independently in the two sides.

>>We don't have an ApRequest in the client yet
Ah right, good catch!! We didn't have it because it's not needed yet. Do you have any cases to use it? We have one, and actually we're implementing a new GSSAPI mechanism based on Kerby, which will need ApReqeust and some APIs. 

>>We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.
Definitely you're very welcome. Thanks for the communication and sync to avoid possible conflict. Focusing on the client side sounds reasonable as it's essentially needed. We'll also made the server side work as well because it can help to do the unit tests. For small changes, how about doing in trunk targeting the RC2 release? And for the kpasswd and kadmin stuffs, how about developing them in the kadmin-remote branch? Would you check the branch, looking at the related client side modules? Our side may focus on the server side. If sounds good, I guess we can make the branch committable to you. If you have other ideas about how to collaborate, please let me know. Thanks.

Regards,
Kai

-----Original Message-----
From: Steve Moyer [mailto:smoyer@psu.edu] 
Sent: Saturday, February 13, 2016 5:34 AM
To: kerby@directory.apache.org
Subject: Re: Kerby client library refactoring

All, 

I've finally found the time to really dig into the Kerby client code with an eye towards adding remote kpasswd and kadmin functionality. Today, we found what I believe to be the last area where the Kerby client generates different packets than the MIT CLI tools. I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types. I also think there's some refactoring to do around how the client builds the request - the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client. 

In any case, adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way. I'd propose the following hierarchy: 

                                      +----------------+
                                      | KrbClient (AS) |
                                      +----------------+
                                         ^          ^
                                         |          |
                        +----------------+          +-------------------+
                        |                                               |
                  +-----------+                                +-----------------+
                  | KdcClient |                                | <Protocol> (AP) |
                  +-----------+                                +-----------------+
                     ^  ^  ^                                      ^           ^
                     |  |  |                                      |           |
      +--------------+  |  +-------------+                   +----+           +----+
      |                 |                |                   |                     |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
| KinitTool |     | KvnoTool |     | KlistTool |     | KpasswdClient |     | KadminClient |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
                                                             ^                     ^
                                                             |                     |
                                                      +-------------+       +------------+
                                                      | KpasswdTool |       | KadminTool |
                                                      +-------------+       +------------+

Some important notes about where we would go from here:

1)  The kpasswd and kadmin specifications share a protocol but don't share commands.  I haven't thought of a good name for the class yet so <Protocol> is the placeholder.  These commands <use> a ticket retrieved via an AsRequest but the commands themselves are sent via ApRequests.

2)  The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.

3)  Any class above that includes the word "Tool" uses the class above it (sorry about the ASCII art).  Currently, the KrbKdcOption enum include flags for the Tool but SoC would suggest that the arguments are a concern of the Tool itself.  I think it would also be easier to manage argument parsing with args4j or commons-cli and that the English title of the flag should also be moved out of the enum.

4)  We might want a base class for some of the tools as they do have some commonality.

5)  There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?

6)  We don't have an ApRequest in the client yet, if we share the AsRequest with the server I'd suggest the same be done for ApRequest.

We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.

Have a great weekend,

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Zheng, Kai" <ka...@intel.com>
To: kerby@directory.apache.org
Sent: Monday, January 11, 2016 7:29:20 AM
Subject: FW: Kerby client library refactoring



Sorry, was working on some other projects. My thought was instead of code that looks like this: 

requestOptions = new KOptions();
requestOptions.add(KrbOption.USE_TGT, tgt); //requestOptions.add(KrbOption.SERVER_PRINCIPAL,
"HTTP/freeipa.rhelent.lan");
requestOptions.add(KrbOption.SERVER_PRINCIPAL, new PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN)); requestOptions.add(KrbOption.FORWARDABLE,true);
requestOptions.add(KrbOption.PROXIABLE,false);
requestOptions.add(KrbOption.RENEWABLE_OK,false); 

I would think this would be more OO: 

requestOptions = new KOptions();
requestOptions.setTgt(tgt);
//requestOptions.setServerPrincipal("HTTP/freeipa.rhelent.lan");
requestOptions.setServerPrincipal(new
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN));
requestOptions.setForwardable(true);
requestOptions.setProxiable(false);
requestOptions.setRenewable(false); 

Could keep it backed by a set of options 




Agreed. This is fully compatible with the definition of all the KrbOptions enums, except thay will not be visible by the end user. 

RE: Kerby client library refactoring

Posted by "Yan, Yan A" <ya...@intel.com>.
Thanks for Kai's advice. I will do that.

Best regards,
Yan


-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com] 
Sent: Friday, February 19, 2016 2:06 PM
To: kerby@directory.apache.org
Subject: RE: Kerby client library refactoring

Thanks Yan for working on this. I understand you're saving your codes in your github, but still better to create JIRA entries for your target tasks. When your partly work is in good shape, please push request for the review and get the codes in timely. Thanks.

Regards,
Kai

-----Original Message-----
From: Yan, Yan A [mailto:yan.a.yan@intel.com] 
Sent: Friday, February 19, 2016 1:52 PM
To: kerby@directory.apache.org; Steve Moyer <sm...@psu.edu>
Subject: RE: Kerby client library refactoring

Hi, all

I was reading your discussion about adding the remote kadmin and kpasswd functionality, and I hope I can help with these modules.

In fact, I have read codes on kadmin-remote branch which has two new different modules from trunk branch (kerb-admin and kerb-admin-server).

If there is no other suggestions, I can help work on the kadmin-remote branch as you discussed before.

If someone has also started working on this issue, please let me know, and I'm glad to help share some parts.

Best regards,
Yan

-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com] 
Sent: Tuesday, February 16, 2016 2:46 PM
To: kerby@directory.apache.org; Steve Moyer <sm...@psu.edu>
Subject: RE: Kerby client library refactoring

Sorry for the late reply.

>> I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types
Sounds good to me. A question is, in your case, where do you get the encryption types list? If you get them from a configuration, then this might not be needed because KrbClient can pick up them by itself.

>> the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client.
Did you find any issue about reading them back out? Can we enhance the codes to allow you to use KrbClient as flexible as you would build the request yourself? Not sure what's exactly needed here. Exposing AsRequest level is a little tricky because it's still in fast evolving.

>> adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way.
Agree. We can add kpasswd and kadmin similarly as KrbClient. We had made some initial work in kadmin-remote branch for the prototype, as Jiajia mentioned. To avoid big impact for existing codes, we simply duplicated the codes and adapt accordingly. Eventually we may refactor the codes and avoid the duplication codes after we make it work first and have the unit tests.  Note there're some differences though: 1) KrbClient is against KDC, kpasswd and kadmin are against kadmind; 2) kadmin needs to authenticate via GSSAPI; 3) kadmin uses XDR for the RPC; 4) they may use different configurations. Considering such, it may sound reasonable to develop the new functionalities separately, instead of redesign the whole architecture first.

>> The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.
I thought it would be enough for kpasswd to support the change password protocol right, since all vendors use the spec?

>> We might want a base class for some of the tools as they do have some commonality.
Yeah, agree. We'll need add unit tests for the tools as well.

>>There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?
AsRequest is a helper class to build or handle the wire message type of AsReq. It looks roughly same in client and KDC sides, but still quite much difference, not to share allows the codes can evolve independently in the two sides.

>>We don't have an ApRequest in the client yet
Ah right, good catch!! We didn't have it because it's not needed yet. Do you have any cases to use it? We have one, and actually we're implementing a new GSSAPI mechanism based on Kerby, which will need ApReqeust and some APIs. 

>>We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.
Definitely you're very welcome. Thanks for the communication and sync to avoid possible conflict. Focusing on the client side sounds reasonable as it's essentially needed. We'll also made the server side work as well because it can help to do the unit tests. For small changes, how about doing in trunk targeting the RC2 release? And for the kpasswd and kadmin stuffs, how about developing them in the kadmin-remote branch? Would you check the branch, looking at the related client side modules? Our side may focus on the server side. If sounds good, I guess we can make the branch committable to you. If you have other ideas about how to collaborate, please let me know. Thanks.

Regards,
Kai

-----Original Message-----
From: Steve Moyer [mailto:smoyer@psu.edu] 
Sent: Saturday, February 13, 2016 5:34 AM
To: kerby@directory.apache.org
Subject: Re: Kerby client library refactoring

All, 

I've finally found the time to really dig into the Kerby client code with an eye towards adding remote kpasswd and kadmin functionality. Today, we found what I believe to be the last area where the Kerby client generates different packets than the MIT CLI tools. I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types. I also think there's some refactoring to do around how the client builds the request - the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client. 

In any case, adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way. I'd propose the following hierarchy: 

                                      +----------------+
                                      | KrbClient (AS) |
                                      +----------------+
                                         ^          ^
                                         |          |
                        +----------------+          +-------------------+
                        |                                               |
                  +-----------+                                +-----------------+
                  | KdcClient |                                | <Protocol> (AP) |
                  +-----------+                                +-----------------+
                     ^  ^  ^                                      ^           ^
                     |  |  |                                      |           |
      +--------------+  |  +-------------+                   +----+           +----+
      |                 |                |                   |                     |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
| KinitTool |     | KvnoTool |     | KlistTool |     | KpasswdClient |     | KadminClient |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
                                                             ^                     ^
                                                             |                     |
                                                      +-------------+       +------------+
                                                      | KpasswdTool |       | KadminTool |
                                                      +-------------+       +------------+

Some important notes about where we would go from here:

1)  The kpasswd and kadmin specifications share a protocol but don't share commands.  I haven't thought of a good name for the class yet so <Protocol> is the placeholder.  These commands <use> a ticket retrieved via an AsRequest but the commands themselves are sent via ApRequests.

2)  The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.

3)  Any class above that includes the word "Tool" uses the class above it (sorry about the ASCII art).  Currently, the KrbKdcOption enum include flags for the Tool but SoC would suggest that the arguments are a concern of the Tool itself.  I think it would also be easier to manage argument parsing with args4j or commons-cli and that the English title of the flag should also be moved out of the enum.

4)  We might want a base class for some of the tools as they do have some commonality.

5)  There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?

6)  We don't have an ApRequest in the client yet, if we share the AsRequest with the server I'd suggest the same be done for ApRequest.

We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.

Have a great weekend,

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Zheng, Kai" <ka...@intel.com>
To: kerby@directory.apache.org
Sent: Monday, January 11, 2016 7:29:20 AM
Subject: FW: Kerby client library refactoring



Sorry, was working on some other projects. My thought was instead of code that looks like this: 

requestOptions = new KOptions();
requestOptions.add(KrbOption.USE_TGT, tgt); //requestOptions.add(KrbOption.SERVER_PRINCIPAL,
"HTTP/freeipa.rhelent.lan");
requestOptions.add(KrbOption.SERVER_PRINCIPAL, new PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN)); requestOptions.add(KrbOption.FORWARDABLE,true);
requestOptions.add(KrbOption.PROXIABLE,false);
requestOptions.add(KrbOption.RENEWABLE_OK,false); 

I would think this would be more OO: 

requestOptions = new KOptions();
requestOptions.setTgt(tgt);
//requestOptions.setServerPrincipal("HTTP/freeipa.rhelent.lan");
requestOptions.setServerPrincipal(new
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN));
requestOptions.setForwardable(true);
requestOptions.setProxiable(false);
requestOptions.setRenewable(false); 

Could keep it backed by a set of options 




Agreed. This is fully compatible with the definition of all the KrbOptions enums, except thay will not be visible by the end user. 

RE: Kerby client library refactoring

Posted by "Zheng, Kai" <ka...@intel.com>.
Thanks Yan for working on this. I understand you're saving your codes in your github, but still better to create JIRA entries for your target tasks. When your partly work is in good shape, please push request for the review and get the codes in timely. Thanks.

Regards,
Kai

-----Original Message-----
From: Yan, Yan A [mailto:yan.a.yan@intel.com] 
Sent: Friday, February 19, 2016 1:52 PM
To: kerby@directory.apache.org; Steve Moyer <sm...@psu.edu>
Subject: RE: Kerby client library refactoring

Hi, all

I was reading your discussion about adding the remote kadmin and kpasswd functionality, and I hope I can help with these modules.

In fact, I have read codes on kadmin-remote branch which has two new different modules from trunk branch (kerb-admin and kerb-admin-server).

If there is no other suggestions, I can help work on the kadmin-remote branch as you discussed before.

If someone has also started working on this issue, please let me know, and I'm glad to help share some parts.

Best regards,
Yan

-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com] 
Sent: Tuesday, February 16, 2016 2:46 PM
To: kerby@directory.apache.org; Steve Moyer <sm...@psu.edu>
Subject: RE: Kerby client library refactoring

Sorry for the late reply.

>> I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types
Sounds good to me. A question is, in your case, where do you get the encryption types list? If you get them from a configuration, then this might not be needed because KrbClient can pick up them by itself.

>> the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client.
Did you find any issue about reading them back out? Can we enhance the codes to allow you to use KrbClient as flexible as you would build the request yourself? Not sure what's exactly needed here. Exposing AsRequest level is a little tricky because it's still in fast evolving.

>> adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way.
Agree. We can add kpasswd and kadmin similarly as KrbClient. We had made some initial work in kadmin-remote branch for the prototype, as Jiajia mentioned. To avoid big impact for existing codes, we simply duplicated the codes and adapt accordingly. Eventually we may refactor the codes and avoid the duplication codes after we make it work first and have the unit tests.  Note there're some differences though: 1) KrbClient is against KDC, kpasswd and kadmin are against kadmind; 2) kadmin needs to authenticate via GSSAPI; 3) kadmin uses XDR for the RPC; 4) they may use different configurations. Considering such, it may sound reasonable to develop the new functionalities separately, instead of redesign the whole architecture first.

>> The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.
I thought it would be enough for kpasswd to support the change password protocol right, since all vendors use the spec?

>> We might want a base class for some of the tools as they do have some commonality.
Yeah, agree. We'll need add unit tests for the tools as well.

>>There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?
AsRequest is a helper class to build or handle the wire message type of AsReq. It looks roughly same in client and KDC sides, but still quite much difference, not to share allows the codes can evolve independently in the two sides.

>>We don't have an ApRequest in the client yet
Ah right, good catch!! We didn't have it because it's not needed yet. Do you have any cases to use it? We have one, and actually we're implementing a new GSSAPI mechanism based on Kerby, which will need ApReqeust and some APIs. 

>>We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.
Definitely you're very welcome. Thanks for the communication and sync to avoid possible conflict. Focusing on the client side sounds reasonable as it's essentially needed. We'll also made the server side work as well because it can help to do the unit tests. For small changes, how about doing in trunk targeting the RC2 release? And for the kpasswd and kadmin stuffs, how about developing them in the kadmin-remote branch? Would you check the branch, looking at the related client side modules? Our side may focus on the server side. If sounds good, I guess we can make the branch committable to you. If you have other ideas about how to collaborate, please let me know. Thanks.

Regards,
Kai

-----Original Message-----
From: Steve Moyer [mailto:smoyer@psu.edu] 
Sent: Saturday, February 13, 2016 5:34 AM
To: kerby@directory.apache.org
Subject: Re: Kerby client library refactoring

All, 

I've finally found the time to really dig into the Kerby client code with an eye towards adding remote kpasswd and kadmin functionality. Today, we found what I believe to be the last area where the Kerby client generates different packets than the MIT CLI tools. I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types. I also think there's some refactoring to do around how the client builds the request - the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client. 

In any case, adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way. I'd propose the following hierarchy: 

                                      +----------------+
                                      | KrbClient (AS) |
                                      +----------------+
                                         ^          ^
                                         |          |
                        +----------------+          +-------------------+
                        |                                               |
                  +-----------+                                +-----------------+
                  | KdcClient |                                | <Protocol> (AP) |
                  +-----------+                                +-----------------+
                     ^  ^  ^                                      ^           ^
                     |  |  |                                      |           |
      +--------------+  |  +-------------+                   +----+           +----+
      |                 |                |                   |                     |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
| KinitTool |     | KvnoTool |     | KlistTool |     | KpasswdClient |     | KadminClient |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
                                                             ^                     ^
                                                             |                     |
                                                      +-------------+       +------------+
                                                      | KpasswdTool |       | KadminTool |
                                                      +-------------+       +------------+

Some important notes about where we would go from here:

1)  The kpasswd and kadmin specifications share a protocol but don't share commands.  I haven't thought of a good name for the class yet so <Protocol> is the placeholder.  These commands <use> a ticket retrieved via an AsRequest but the commands themselves are sent via ApRequests.

2)  The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.

3)  Any class above that includes the word "Tool" uses the class above it (sorry about the ASCII art).  Currently, the KrbKdcOption enum include flags for the Tool but SoC would suggest that the arguments are a concern of the Tool itself.  I think it would also be easier to manage argument parsing with args4j or commons-cli and that the English title of the flag should also be moved out of the enum.

4)  We might want a base class for some of the tools as they do have some commonality.

5)  There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?

6)  We don't have an ApRequest in the client yet, if we share the AsRequest with the server I'd suggest the same be done for ApRequest.

We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.

Have a great weekend,

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Zheng, Kai" <ka...@intel.com>
To: kerby@directory.apache.org
Sent: Monday, January 11, 2016 7:29:20 AM
Subject: FW: Kerby client library refactoring



Sorry, was working on some other projects. My thought was instead of code that looks like this: 

requestOptions = new KOptions();
requestOptions.add(KrbOption.USE_TGT, tgt); //requestOptions.add(KrbOption.SERVER_PRINCIPAL,
"HTTP/freeipa.rhelent.lan");
requestOptions.add(KrbOption.SERVER_PRINCIPAL, new PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN)); requestOptions.add(KrbOption.FORWARDABLE,true);
requestOptions.add(KrbOption.PROXIABLE,false);
requestOptions.add(KrbOption.RENEWABLE_OK,false); 

I would think this would be more OO: 

requestOptions = new KOptions();
requestOptions.setTgt(tgt);
//requestOptions.setServerPrincipal("HTTP/freeipa.rhelent.lan");
requestOptions.setServerPrincipal(new
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN));
requestOptions.setForwardable(true);
requestOptions.setProxiable(false);
requestOptions.setRenewable(false); 

Could keep it backed by a set of options 




Agreed. This is fully compatible with the definition of all the KrbOptions enums, except thay will not be visible by the end user. 

RE: Kerby client library refactoring

Posted by "Yan, Yan A" <ya...@intel.com>.
Hi, all

I was reading your discussion about adding the remote kadmin and kpasswd functionality, and I hope I can help with these modules.

In fact, I have read codes on kadmin-remote branch which has two new different modules from trunk branch (kerb-admin and kerb-admin-server).

If there is no other suggestions, I can help work on the kadmin-remote branch as you discussed before.

If someone has also started working on this issue, please let me know, and I'm glad to help share some parts.

Best regards,
Yan

-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com] 
Sent: Tuesday, February 16, 2016 2:46 PM
To: kerby@directory.apache.org; Steve Moyer <sm...@psu.edu>
Subject: RE: Kerby client library refactoring

Sorry for the late reply.

>> I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types
Sounds good to me. A question is, in your case, where do you get the encryption types list? If you get them from a configuration, then this might not be needed because KrbClient can pick up them by itself.

>> the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client.
Did you find any issue about reading them back out? Can we enhance the codes to allow you to use KrbClient as flexible as you would build the request yourself? Not sure what's exactly needed here. Exposing AsRequest level is a little tricky because it's still in fast evolving.

>> adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way.
Agree. We can add kpasswd and kadmin similarly as KrbClient. We had made some initial work in kadmin-remote branch for the prototype, as Jiajia mentioned. To avoid big impact for existing codes, we simply duplicated the codes and adapt accordingly. Eventually we may refactor the codes and avoid the duplication codes after we make it work first and have the unit tests.  Note there're some differences though: 1) KrbClient is against KDC, kpasswd and kadmin are against kadmind; 2) kadmin needs to authenticate via GSSAPI; 3) kadmin uses XDR for the RPC; 4) they may use different configurations. Considering such, it may sound reasonable to develop the new functionalities separately, instead of redesign the whole architecture first.

>> The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.
I thought it would be enough for kpasswd to support the change password protocol right, since all vendors use the spec?

>> We might want a base class for some of the tools as they do have some commonality.
Yeah, agree. We'll need add unit tests for the tools as well.

>>There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?
AsRequest is a helper class to build or handle the wire message type of AsReq. It looks roughly same in client and KDC sides, but still quite much difference, not to share allows the codes can evolve independently in the two sides.

>>We don't have an ApRequest in the client yet
Ah right, good catch!! We didn't have it because it's not needed yet. Do you have any cases to use it? We have one, and actually we're implementing a new GSSAPI mechanism based on Kerby, which will need ApReqeust and some APIs. 

>>We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.
Definitely you're very welcome. Thanks for the communication and sync to avoid possible conflict. Focusing on the client side sounds reasonable as it's essentially needed. We'll also made the server side work as well because it can help to do the unit tests. For small changes, how about doing in trunk targeting the RC2 release? And for the kpasswd and kadmin stuffs, how about developing them in the kadmin-remote branch? Would you check the branch, looking at the related client side modules? Our side may focus on the server side. If sounds good, I guess we can make the branch committable to you. If you have other ideas about how to collaborate, please let me know. Thanks.

Regards,
Kai

-----Original Message-----
From: Steve Moyer [mailto:smoyer@psu.edu] 
Sent: Saturday, February 13, 2016 5:34 AM
To: kerby@directory.apache.org
Subject: Re: Kerby client library refactoring

All, 

I've finally found the time to really dig into the Kerby client code with an eye towards adding remote kpasswd and kadmin functionality. Today, we found what I believe to be the last area where the Kerby client generates different packets than the MIT CLI tools. I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types. I also think there's some refactoring to do around how the client builds the request - the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client. 

In any case, adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way. I'd propose the following hierarchy: 

                                      +----------------+
                                      | KrbClient (AS) |
                                      +----------------+
                                         ^          ^
                                         |          |
                        +----------------+          +-------------------+
                        |                                               |
                  +-----------+                                +-----------------+
                  | KdcClient |                                | <Protocol> (AP) |
                  +-----------+                                +-----------------+
                     ^  ^  ^                                      ^           ^
                     |  |  |                                      |           |
      +--------------+  |  +-------------+                   +----+           +----+
      |                 |                |                   |                     |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
| KinitTool |     | KvnoTool |     | KlistTool |     | KpasswdClient |     | KadminClient |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
                                                             ^                     ^
                                                             |                     |
                                                      +-------------+       +------------+
                                                      | KpasswdTool |       | KadminTool |
                                                      +-------------+       +------------+

Some important notes about where we would go from here:

1)  The kpasswd and kadmin specifications share a protocol but don't share commands.  I haven't thought of a good name for the class yet so <Protocol> is the placeholder.  These commands <use> a ticket retrieved via an AsRequest but the commands themselves are sent via ApRequests.

2)  The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.

3)  Any class above that includes the word "Tool" uses the class above it (sorry about the ASCII art).  Currently, the KrbKdcOption enum include flags for the Tool but SoC would suggest that the arguments are a concern of the Tool itself.  I think it would also be easier to manage argument parsing with args4j or commons-cli and that the English title of the flag should also be moved out of the enum.

4)  We might want a base class for some of the tools as they do have some commonality.

5)  There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?

6)  We don't have an ApRequest in the client yet, if we share the AsRequest with the server I'd suggest the same be done for ApRequest.

We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.

Have a great weekend,

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Zheng, Kai" <ka...@intel.com>
To: kerby@directory.apache.org
Sent: Monday, January 11, 2016 7:29:20 AM
Subject: FW: Kerby client library refactoring



Sorry, was working on some other projects. My thought was instead of code that looks like this: 

requestOptions = new KOptions();
requestOptions.add(KrbOption.USE_TGT, tgt); //requestOptions.add(KrbOption.SERVER_PRINCIPAL,
"HTTP/freeipa.rhelent.lan");
requestOptions.add(KrbOption.SERVER_PRINCIPAL, new PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN)); requestOptions.add(KrbOption.FORWARDABLE,true);
requestOptions.add(KrbOption.PROXIABLE,false);
requestOptions.add(KrbOption.RENEWABLE_OK,false); 

I would think this would be more OO: 

requestOptions = new KOptions();
requestOptions.setTgt(tgt);
//requestOptions.setServerPrincipal("HTTP/freeipa.rhelent.lan");
requestOptions.setServerPrincipal(new
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN));
requestOptions.setForwardable(true);
requestOptions.setProxiable(false);
requestOptions.setRenewable(false); 

Could keep it backed by a set of options 




Agreed. This is fully compatible with the definition of all the KrbOptions enums, except thay will not be visible by the end user. 

RE: Kerby client library refactoring

Posted by "Zheng, Kai" <ka...@intel.com>.
Steve,

Might I have your further thoughts? I really wish the communication could be responsive and fruitful. Thanks.

Regards,
Kai

-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com] 
Sent: Wednesday, February 17, 2016 11:13 AM
To: kerby@directory.apache.org; Steve Moyer <sm...@psu.edu>
Subject: RE: Kerby client library refactoring

After having an off-line discussion with Jiajia, it seems to be a doable idea to share AsRequest or the like between client and server, and the impact might be not so big as I thought before. In this direction, the newly coming up ApRequest and ApResponse will be hosted in kerb-common module. Later we can migrate AsRequest/Response and TgsRequest/Response into the same place. As to the client or server specific codes, we can have KrbAsRequest/Response, KdcAsRequest/Response, KrbTgsRequest/Response, KdcTgsRequest/response as subclasses to the common ones.

Regards,
Kai

-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com] 
Sent: Tuesday, February 16, 2016 2:46 PM
To: kerby@directory.apache.org; Steve Moyer <sm...@psu.edu>
Subject: RE: Kerby client library refactoring

Sorry for the late reply.

>> I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types
Sounds good to me. A question is, in your case, where do you get the encryption types list? If you get them from a configuration, then this might not be needed because KrbClient can pick up them by itself.

>> the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client.
Did you find any issue about reading them back out? Can we enhance the codes to allow you to use KrbClient as flexible as you would build the request yourself? Not sure what's exactly needed here. Exposing AsRequest level is a little tricky because it's still in fast evolving.

>> adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way.
Agree. We can add kpasswd and kadmin similarly as KrbClient. We had made some initial work in kadmin-remote branch for the prototype, as Jiajia mentioned. To avoid big impact for existing codes, we simply duplicated the codes and adapt accordingly. Eventually we may refactor the codes and avoid the duplication codes after we make it work first and have the unit tests.  Note there're some differences though: 1) KrbClient is against KDC, kpasswd and kadmin are against kadmind; 2) kadmin needs to authenticate via GSSAPI; 3) kadmin uses XDR for the RPC; 4) they may use different configurations. Considering such, it may sound reasonable to develop the new functionalities separately, instead of redesign the whole architecture first.

>> The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.
I thought it would be enough for kpasswd to support the change password protocol right, since all vendors use the spec?

>> We might want a base class for some of the tools as they do have some commonality.
Yeah, agree. We'll need add unit tests for the tools as well.

>>There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?
AsRequest is a helper class to build or handle the wire message type of AsReq. It looks roughly same in client and KDC sides, but still quite much difference, not to share allows the codes can evolve independently in the two sides.

>>We don't have an ApRequest in the client yet
Ah right, good catch!! We didn't have it because it's not needed yet. Do you have any cases to use it? We have one, and actually we're implementing a new GSSAPI mechanism based on Kerby, which will need ApReqeust and some APIs. 

>>We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.
Definitely you're very welcome. Thanks for the communication and sync to avoid possible conflict. Focusing on the client side sounds reasonable as it's essentially needed. We'll also made the server side work as well because it can help to do the unit tests. For small changes, how about doing in trunk targeting the RC2 release? And for the kpasswd and kadmin stuffs, how about developing them in the kadmin-remote branch? Would you check the branch, looking at the related client side modules? Our side may focus on the server side. If sounds good, I guess we can make the branch committable to you. If you have other ideas about how to collaborate, please let me know. Thanks.

Regards,
Kai

-----Original Message-----
From: Steve Moyer [mailto:smoyer@psu.edu] 
Sent: Saturday, February 13, 2016 5:34 AM
To: kerby@directory.apache.org
Subject: Re: Kerby client library refactoring

All, 

I've finally found the time to really dig into the Kerby client code with an eye towards adding remote kpasswd and kadmin functionality. Today, we found what I believe to be the last area where the Kerby client generates different packets than the MIT CLI tools. I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types. I also think there's some refactoring to do around how the client builds the request - the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client. 

In any case, adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way. I'd propose the following hierarchy: 

                                      +----------------+
                                      | KrbClient (AS) |
                                      +----------------+
                                         ^          ^
                                         |          |
                        +----------------+          +-------------------+
                        |                                               |
                  +-----------+                                +-----------------+
                  | KdcClient |                                | <Protocol> (AP) |
                  +-----------+                                +-----------------+
                     ^  ^  ^                                      ^           ^
                     |  |  |                                      |           |
      +--------------+  |  +-------------+                   +----+           +----+
      |                 |                |                   |                     |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
| KinitTool |     | KvnoTool |     | KlistTool |     | KpasswdClient |     | KadminClient |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
                                                             ^                     ^
                                                             |                     |
                                                      +-------------+       +------------+
                                                      | KpasswdTool |       | KadminTool |
                                                      +-------------+       +------------+

Some important notes about where we would go from here:

1)  The kpasswd and kadmin specifications share a protocol but don't share commands.  I haven't thought of a good name for the class yet so <Protocol> is the placeholder.  These commands <use> a ticket retrieved via an AsRequest but the commands themselves are sent via ApRequests.

2)  The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.

3)  Any class above that includes the word "Tool" uses the class above it (sorry about the ASCII art).  Currently, the KrbKdcOption enum include flags for the Tool but SoC would suggest that the arguments are a concern of the Tool itself.  I think it would also be easier to manage argument parsing with args4j or commons-cli and that the English title of the flag should also be moved out of the enum.

4)  We might want a base class for some of the tools as they do have some commonality.

5)  There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?

6)  We don't have an ApRequest in the client yet, if we share the AsRequest with the server I'd suggest the same be done for ApRequest.

We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.

Have a great weekend,

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Zheng, Kai" <ka...@intel.com>
To: kerby@directory.apache.org
Sent: Monday, January 11, 2016 7:29:20 AM
Subject: FW: Kerby client library refactoring



Sorry, was working on some other projects. My thought was instead of code that looks like this: 

requestOptions = new KOptions();
requestOptions.add(KrbOption.USE_TGT, tgt); //requestOptions.add(KrbOption.SERVER_PRINCIPAL,
"HTTP/freeipa.rhelent.lan");
requestOptions.add(KrbOption.SERVER_PRINCIPAL, new PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN)); requestOptions.add(KrbOption.FORWARDABLE,true);
requestOptions.add(KrbOption.PROXIABLE,false);
requestOptions.add(KrbOption.RENEWABLE_OK,false); 

I would think this would be more OO: 

requestOptions = new KOptions();
requestOptions.setTgt(tgt);
//requestOptions.setServerPrincipal("HTTP/freeipa.rhelent.lan");
requestOptions.setServerPrincipal(new
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN));
requestOptions.setForwardable(true);
requestOptions.setProxiable(false);
requestOptions.setRenewable(false); 

Could keep it backed by a set of options 




Agreed. This is fully compatible with the definition of all the KrbOptions enums, except thay will not be visible by the end user. 

RE: Kerby client library refactoring

Posted by "Zheng, Kai" <ka...@intel.com>.
After having an off-line discussion with Jiajia, it seems to be a doable idea to share AsRequest or the like between client and server, and the impact might be not so big as I thought before. In this direction, the newly coming up ApRequest and ApResponse will be hosted in kerb-common module. Later we can migrate AsRequest/Response and TgsRequest/Response into the same place. As to the client or server specific codes, we can have KrbAsRequest/Response, KdcAsRequest/Response, KrbTgsRequest/Response, KdcTgsRequest/response as subclasses to the common ones.

Regards,
Kai

-----Original Message-----
From: Zheng, Kai [mailto:kai.zheng@intel.com] 
Sent: Tuesday, February 16, 2016 2:46 PM
To: kerby@directory.apache.org; Steve Moyer <sm...@psu.edu>
Subject: RE: Kerby client library refactoring

Sorry for the late reply.

>> I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types
Sounds good to me. A question is, in your case, where do you get the encryption types list? If you get them from a configuration, then this might not be needed because KrbClient can pick up them by itself.

>> the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client.
Did you find any issue about reading them back out? Can we enhance the codes to allow you to use KrbClient as flexible as you would build the request yourself? Not sure what's exactly needed here. Exposing AsRequest level is a little tricky because it's still in fast evolving.

>> adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way.
Agree. We can add kpasswd and kadmin similarly as KrbClient. We had made some initial work in kadmin-remote branch for the prototype, as Jiajia mentioned. To avoid big impact for existing codes, we simply duplicated the codes and adapt accordingly. Eventually we may refactor the codes and avoid the duplication codes after we make it work first and have the unit tests.  Note there're some differences though: 1) KrbClient is against KDC, kpasswd and kadmin are against kadmind; 2) kadmin needs to authenticate via GSSAPI; 3) kadmin uses XDR for the RPC; 4) they may use different configurations. Considering such, it may sound reasonable to develop the new functionalities separately, instead of redesign the whole architecture first.

>> The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.
I thought it would be enough for kpasswd to support the change password protocol right, since all vendors use the spec?

>> We might want a base class for some of the tools as they do have some commonality.
Yeah, agree. We'll need add unit tests for the tools as well.

>>There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?
AsRequest is a helper class to build or handle the wire message type of AsReq. It looks roughly same in client and KDC sides, but still quite much difference, not to share allows the codes can evolve independently in the two sides.

>>We don't have an ApRequest in the client yet
Ah right, good catch!! We didn't have it because it's not needed yet. Do you have any cases to use it? We have one, and actually we're implementing a new GSSAPI mechanism based on Kerby, which will need ApReqeust and some APIs. 

>>We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.
Definitely you're very welcome. Thanks for the communication and sync to avoid possible conflict. Focusing on the client side sounds reasonable as it's essentially needed. We'll also made the server side work as well because it can help to do the unit tests. For small changes, how about doing in trunk targeting the RC2 release? And for the kpasswd and kadmin stuffs, how about developing them in the kadmin-remote branch? Would you check the branch, looking at the related client side modules? Our side may focus on the server side. If sounds good, I guess we can make the branch committable to you. If you have other ideas about how to collaborate, please let me know. Thanks.

Regards,
Kai

-----Original Message-----
From: Steve Moyer [mailto:smoyer@psu.edu] 
Sent: Saturday, February 13, 2016 5:34 AM
To: kerby@directory.apache.org
Subject: Re: Kerby client library refactoring

All, 

I've finally found the time to really dig into the Kerby client code with an eye towards adding remote kpasswd and kadmin functionality. Today, we found what I believe to be the last area where the Kerby client generates different packets than the MIT CLI tools. I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types. I also think there's some refactoring to do around how the client builds the request - the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client. 

In any case, adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way. I'd propose the following hierarchy: 

                                      +----------------+
                                      | KrbClient (AS) |
                                      +----------------+
                                         ^          ^
                                         |          |
                        +----------------+          +-------------------+
                        |                                               |
                  +-----------+                                +-----------------+
                  | KdcClient |                                | <Protocol> (AP) |
                  +-----------+                                +-----------------+
                     ^  ^  ^                                      ^           ^
                     |  |  |                                      |           |
      +--------------+  |  +-------------+                   +----+           +----+
      |                 |                |                   |                     |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
| KinitTool |     | KvnoTool |     | KlistTool |     | KpasswdClient |     | KadminClient |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
                                                             ^                     ^
                                                             |                     |
                                                      +-------------+       +------------+
                                                      | KpasswdTool |       | KadminTool |
                                                      +-------------+       +------------+

Some important notes about where we would go from here:

1)  The kpasswd and kadmin specifications share a protocol but don't share commands.  I haven't thought of a good name for the class yet so <Protocol> is the placeholder.  These commands <use> a ticket retrieved via an AsRequest but the commands themselves are sent via ApRequests.

2)  The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.

3)  Any class above that includes the word "Tool" uses the class above it (sorry about the ASCII art).  Currently, the KrbKdcOption enum include flags for the Tool but SoC would suggest that the arguments are a concern of the Tool itself.  I think it would also be easier to manage argument parsing with args4j or commons-cli and that the English title of the flag should also be moved out of the enum.

4)  We might want a base class for some of the tools as they do have some commonality.

5)  There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?

6)  We don't have an ApRequest in the client yet, if we share the AsRequest with the server I'd suggest the same be done for ApRequest.

We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.

Have a great weekend,

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Zheng, Kai" <ka...@intel.com>
To: kerby@directory.apache.org
Sent: Monday, January 11, 2016 7:29:20 AM
Subject: FW: Kerby client library refactoring



Sorry, was working on some other projects. My thought was instead of code that looks like this: 

requestOptions = new KOptions();
requestOptions.add(KrbOption.USE_TGT, tgt); //requestOptions.add(KrbOption.SERVER_PRINCIPAL,
"HTTP/freeipa.rhelent.lan");
requestOptions.add(KrbOption.SERVER_PRINCIPAL, new PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN)); requestOptions.add(KrbOption.FORWARDABLE,true);
requestOptions.add(KrbOption.PROXIABLE,false);
requestOptions.add(KrbOption.RENEWABLE_OK,false); 

I would think this would be more OO: 

requestOptions = new KOptions();
requestOptions.setTgt(tgt);
//requestOptions.setServerPrincipal("HTTP/freeipa.rhelent.lan");
requestOptions.setServerPrincipal(new
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN));
requestOptions.setForwardable(true);
requestOptions.setProxiable(false);
requestOptions.setRenewable(false); 

Could keep it backed by a set of options 




Agreed. This is fully compatible with the definition of all the KrbOptions enums, except thay will not be visible by the end user. 

RE: Kerby client library refactoring

Posted by "Zheng, Kai" <ka...@intel.com>.
Sorry for the late reply.

>> I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types
Sounds good to me. A question is, in your case, where do you get the encryption types list? If you get them from a configuration, then this might not be needed because KrbClient can pick up them by itself.

>> the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client.
Did you find any issue about reading them back out? Can we enhance the codes to allow you to use KrbClient as flexible as you would build the request yourself? Not sure what's exactly needed here. Exposing AsRequest level is a little tricky because it's still in fast evolving.

>> adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way.
Agree. We can add kpasswd and kadmin similarly as KrbClient. We had made some initial work in kadmin-remote branch for the prototype, as Jiajia mentioned. To avoid big impact for existing codes, we simply duplicated the codes and adapt accordingly. Eventually we may refactor the codes and avoid the duplication codes after we make it work first and have the unit tests.  Note there're some differences though: 1) KrbClient is against KDC, kpasswd and kadmin are against kadmind; 2) kadmin needs to authenticate via GSSAPI; 3) kadmin uses XDR for the RPC; 4) they may use different configurations. Considering such, it may sound reasonable to develop the new functionalities separately, instead of redesign the whole architecture first.

>> The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.
I thought it would be enough for kpasswd to support the change password protocol right, since all vendors use the spec?

>> We might want a base class for some of the tools as they do have some commonality.
Yeah, agree. We'll need add unit tests for the tools as well.

>>There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?
AsRequest is a helper class to build or handle the wire message type of AsReq. It looks roughly same in client and KDC sides, but still quite much difference, not to share allows the codes can evolve independently in the two sides.

>>We don't have an ApRequest in the client yet
Ah right, good catch!! We didn't have it because it's not needed yet. Do you have any cases to use it? We have one, and actually we're implementing a new GSSAPI mechanism based on Kerby, which will need ApReqeust and some APIs. 

>>We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.
Definitely you're very welcome. Thanks for the communication and sync to avoid possible conflict. Focusing on the client side sounds reasonable as it's essentially needed. We'll also made the server side work as well because it can help to do the unit tests. For small changes, how about doing in trunk targeting the RC2 release? And for the kpasswd and kadmin stuffs, how about developing them in the kadmin-remote branch? Would you check the branch, looking at the related client side modules? Our side may focus on the server side. If sounds good, I guess we can make the branch committable to you. If you have other ideas about how to collaborate, please let me know. Thanks.

Regards,
Kai

-----Original Message-----
From: Steve Moyer [mailto:smoyer@psu.edu] 
Sent: Saturday, February 13, 2016 5:34 AM
To: kerby@directory.apache.org
Subject: Re: Kerby client library refactoring

All, 

I've finally found the time to really dig into the Kerby client code with an eye towards adding remote kpasswd and kadmin functionality. Today, we found what I believe to be the last area where the Kerby client generates different packets than the MIT CLI tools. I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types. I also think there's some refactoring to do around how the client builds the request - the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client. 

In any case, adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way. I'd propose the following hierarchy: 

                                      +----------------+
                                      | KrbClient (AS) |
                                      +----------------+
                                         ^          ^
                                         |          |
                        +----------------+          +-------------------+
                        |                                               |
                  +-----------+                                +-----------------+
                  | KdcClient |                                | <Protocol> (AP) |
                  +-----------+                                +-----------------+
                     ^  ^  ^                                      ^           ^
                     |  |  |                                      |           |
      +--------------+  |  +-------------+                   +----+           +----+
      |                 |                |                   |                     |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
| KinitTool |     | KvnoTool |     | KlistTool |     | KpasswdClient |     | KadminClient |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
                                                             ^                     ^
                                                             |                     |
                                                      +-------------+       +------------+
                                                      | KpasswdTool |       | KadminTool |
                                                      +-------------+       +------------+

Some important notes about where we would go from here:

1)  The kpasswd and kadmin specifications share a protocol but don't share commands.  I haven't thought of a good name for the class yet so <Protocol> is the placeholder.  These commands <use> a ticket retrieved via an AsRequest but the commands themselves are sent via ApRequests.

2)  The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.

3)  Any class above that includes the word "Tool" uses the class above it (sorry about the ASCII art).  Currently, the KrbKdcOption enum include flags for the Tool but SoC would suggest that the arguments are a concern of the Tool itself.  I think it would also be easier to manage argument parsing with args4j or commons-cli and that the English title of the flag should also be moved out of the enum.

4)  We might want a base class for some of the tools as they do have some commonality.

5)  There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?

6)  We don't have an ApRequest in the client yet, if we share the AsRequest with the server I'd suggest the same be done for ApRequest.

We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.

Have a great weekend,

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Zheng, Kai" <ka...@intel.com>
To: kerby@directory.apache.org
Sent: Monday, January 11, 2016 7:29:20 AM
Subject: FW: Kerby client library refactoring



Sorry, was working on some other projects. My thought was instead of code that looks like this: 

requestOptions = new KOptions();
requestOptions.add(KrbOption.USE_TGT, tgt); //requestOptions.add(KrbOption.SERVER_PRINCIPAL,
"HTTP/freeipa.rhelent.lan");
requestOptions.add(KrbOption.SERVER_PRINCIPAL, new PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN)); requestOptions.add(KrbOption.FORWARDABLE,true);
requestOptions.add(KrbOption.PROXIABLE,false);
requestOptions.add(KrbOption.RENEWABLE_OK,false); 

I would think this would be more OO: 

requestOptions = new KOptions();
requestOptions.setTgt(tgt);
//requestOptions.setServerPrincipal("HTTP/freeipa.rhelent.lan");
requestOptions.setServerPrincipal(new
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN));
requestOptions.setForwardable(true);
requestOptions.setProxiable(false);
requestOptions.setRenewable(false); 

Could keep it backed by a set of options 




Agreed. This is fully compatible with the definition of all the KrbOptions enums, except thay will not be visible by the end user. 

RE: Kerby client library refactoring

Posted by "Zheng, Kai" <ka...@intel.com>.
Nice to see this after a long holiday. Thanks Steve! I would read closely into this and hope to address them well late of today in addition to others' comments if any.

Regards,
Kai

-----Original Message-----
From: Steve Moyer [mailto:smoyer@psu.edu] 
Sent: Saturday, February 13, 2016 5:34 AM
To: kerby@directory.apache.org
Subject: Re: Kerby client library refactoring

All, 

I've finally found the time to really dig into the Kerby client code with an eye towards adding remote kpasswd and kadmin functionality. Today, we found what I believe to be the last area where the Kerby client generates different packets than the MIT CLI tools. I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types. I also think there's some refactoring to do around how the client builds the request - the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client. 

In any case, adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way. I'd propose the following hierarchy: 

                                      +----------------+
                                      | KrbClient (AS) |
                                      +----------------+
                                         ^          ^
                                         |          |
                        +----------------+          +-------------------+
                        |                                               |
                  +-----------+                                +-----------------+
                  | KdcClient |                                | <Protocol> (AP) |
                  +-----------+                                +-----------------+
                     ^  ^  ^                                      ^           ^
                     |  |  |                                      |           |
      +--------------+  |  +-------------+                   +----+           +----+
      |                 |                |                   |                     |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
| KinitTool |     | KvnoTool |     | KlistTool |     | KpasswdClient |     | KadminClient |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
                                                             ^                     ^
                                                             |                     |
                                                      +-------------+       +------------+
                                                      | KpasswdTool |       | KadminTool |
                                                      +-------------+       +------------+

Some important notes about where we would go from here:

1)  The kpasswd and kadmin specifications share a protocol but don't share commands.  I haven't thought of a good name for the class yet so <Protocol> is the placeholder.  These commands <use> a ticket retrieved via an AsRequest but the commands themselves are sent via ApRequests.

2)  The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.

3)  Any class above that includes the word "Tool" uses the class above it (sorry about the ASCII art).  Currently, the KrbKdcOption enum include flags for the Tool but SoC would suggest that the arguments are a concern of the Tool itself.  I think it would also be easier to manage argument parsing with args4j or commons-cli and that the English title of the flag should also be moved out of the enum.

4)  We might want a base class for some of the tools as they do have some commonality.

5)  There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?

6)  We don't have an ApRequest in the client yet, if we share the AsRequest with the server I'd suggest the same be done for ApRequest.

We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.

Have a great weekend,

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Zheng, Kai" <ka...@intel.com>
To: kerby@directory.apache.org
Sent: Monday, January 11, 2016 7:29:20 AM
Subject: FW: Kerby client library refactoring



Sorry, was working on some other projects. My thought was instead of code that looks like this: 

requestOptions = new KOptions();
requestOptions.add(KrbOption.USE_TGT, tgt); //requestOptions.add(KrbOption.SERVER_PRINCIPAL,
"HTTP/freeipa.rhelent.lan");
requestOptions.add(KrbOption.SERVER_PRINCIPAL, new PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN)); requestOptions.add(KrbOption.FORWARDABLE,true);
requestOptions.add(KrbOption.PROXIABLE,false);
requestOptions.add(KrbOption.RENEWABLE_OK,false); 

I would think this would be more OO: 

requestOptions = new KOptions();
requestOptions.setTgt(tgt);
//requestOptions.setServerPrincipal("HTTP/freeipa.rhelent.lan");
requestOptions.setServerPrincipal(new
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN));
requestOptions.setForwardable(true);
requestOptions.setProxiable(false);
requestOptions.setRenewable(false); 

Could keep it backed by a set of options 




Agreed. This is fully compatible with the definition of all the KrbOptions enums, except thay will not be visible by the end user. 

RE: Kerby client library refactoring

Posted by "Li, Jiajia" <ji...@intel.com>.
Hi Steve,
I think your ideas are based on the trunk branch, and some initial work of remote kadmin and kpasswd can be found in kadmin-remote branch(https://github.com/apache/directory-kerby/tree/kadmin-remote)
Mainly on:
1. Split Kadmin codes into two parts: LocalKadmin(impl) and RemoteKadmin(impl)
2. Add the AdminServer
3. Add the PasswdServer and DefaultInternalPasswdClient
Maybe it will help you.

Thanks
Jiajia

-----Original Message-----
From: Steve Moyer [mailto:smoyer@psu.edu] 
Sent: Saturday, February 13, 2016 5:34 AM
To: kerby@directory.apache.org
Subject: Re: Kerby client library refactoring

All, 

I've finally found the time to really dig into the Kerby client code with an eye towards adding remote kpasswd and kadmin functionality. Today, we found what I believe to be the last area where the Kerby client generates different packets than the MIT CLI tools. I've added a couple more sub-tasks to DIRKRB-440 which will allow the KrbClient to set the list of encryption types. I also think there's some refactoring to do around how the client builds the request - the KOption and KrbKdcOptions are added into the request's options (which does make them easier to pass) but then the requests need to read them back out. I think I'd prefer that (at least as an option) we be able to build the request we want and call something like an execute() method on the client. 

In any case, adding the remote kpasswd and kadmin functionality will require a small amount of refactoring within the client code as well as setting up the class hierarchy in a convenient way. I'd propose the following hierarchy: 

                                      +----------------+
                                      | KrbClient (AS) |
                                      +----------------+
                                         ^          ^
                                         |          |
                        +----------------+          +-------------------+
                        |                                               |
                  +-----------+                                +-----------------+
                  | KdcClient |                                | <Protocol> (AP) |
                  +-----------+                                +-----------------+
                     ^  ^  ^                                      ^           ^
                     |  |  |                                      |           |
      +--------------+  |  +-------------+                   +----+           +----+
      |                 |                |                   |                     |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
| KinitTool |     | KvnoTool |     | KlistTool |     | KpasswdClient |     | KadminClient |
+-----------+     +----------+     +-----------+     +---------------+     +--------------+
                                                             ^                     ^
                                                             |                     |
                                                      +-------------+       +------------+
                                                      | KpasswdTool |       | KadminTool |
                                                      +-------------+       +------------+

Some important notes about where we would go from here:

1)  The kpasswd and kadmin specifications share a protocol but don't share commands.  I haven't thought of a good name for the class yet so <Protocol> is the placeholder.  These commands <use> a ticket retrieved via an AsRequest but the commands themselves are sent via ApRequests.

2)  The Kpasswd client should actually understand both the Microsoft and the "standard" variant of the protocol, so there are actually two different request formats represent by kpasswd.

3)  Any class above that includes the word "Tool" uses the class above it (sorry about the ASCII art).  Currently, the KrbKdcOption enum include flags for the Tool but SoC would suggest that the arguments are a concern of the Tool itself.  I think it would also be easier to manage argument parsing with args4j or commons-cli and that the English title of the flag should also be moved out of the enum.

4)  We might want a base class for some of the tools as they do have some commonality.

5)  There's an AsRequest class defined in the client code as well as in the server code - is there some reason these classes aren't shared?  If they simply represent the packet on the wire, could they be shared?

6)  We don't have an ApRequest in the client yet, if we share the AsRequest with the server I'd suggest the same be done for ApRequest.

We're planning to continue work on the Kerby Client on Monday but would like some input before we "perform major surgery".  If you're interested, we could take on responsibility for the client if that helps.

Have a great weekend,

Steve

--

“The mark of the immature man is that he wants to die nobly for a cause, while the mark of the mature man is that he wants to live humbly for one.” - Wilhelm Stekel

----- Original Message -----
From: "Zheng, Kai" <ka...@intel.com>
To: kerby@directory.apache.org
Sent: Monday, January 11, 2016 7:29:20 AM
Subject: FW: Kerby client library refactoring



Sorry, was working on some other projects. My thought was instead of code that looks like this: 

requestOptions = new KOptions();
requestOptions.add(KrbOption.USE_TGT, tgt); //requestOptions.add(KrbOption.SERVER_PRINCIPAL,
"HTTP/freeipa.rhelent.lan");
requestOptions.add(KrbOption.SERVER_PRINCIPAL, new PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN)); requestOptions.add(KrbOption.FORWARDABLE,true);
requestOptions.add(KrbOption.PROXIABLE,false);
requestOptions.add(KrbOption.RENEWABLE_OK,false); 

I would think this would be more OO: 

requestOptions = new KOptions();
requestOptions.setTgt(tgt);
//requestOptions.setServerPrincipal("HTTP/freeipa.rhelent.lan");
requestOptions.setServerPrincipal(new
PrincipalName("HTTP/freeipa.rhelent.lan@RHELENT.LAN",NameType.NT_UNKNO
WN));
requestOptions.setForwardable(true);
requestOptions.setProxiable(false);
requestOptions.setRenewable(false); 

Could keep it backed by a set of options 




Agreed. This is fully compatible with the definition of all the KrbOptions enums, except thay will not be visible by the end user.