I'm not too sure that its going to be worth it to reuse KeyStoreManager. That code is for storage of certs for the systemvm. So you need to ensure that your changes don't overlap with the systemvm code. Additionally, I don't see that the code handles the chain also. I could be wrong, but just from reading the code it seems to assume the "cert" string produces a single cert. Correct me if I'm wrong.
The absolute key thing for this feature, in my mind, is getting the input validation right. If you don't give useful errors, you'll be handling requests from people not being able to insert a cert, and the default errors from java are typically not very useful. Regarding design. API Commands should synchronously call a Service class, this is the create() method of an async command or execute() of a non async command. That service method should do no more than input validation and saving things to the database. If you need to communicate to resources, then it should be an async api command. The async portion of the API command, this would be the execute() method, should also call the service class. Since you ideally did all the input validation in the sync portion, not much validation should happen at this point. But there may be some more intensive validation you want to do at this point. After validation, the service class should call the manager. The manager does the real business logic. So you have two groups of functionality. Managing SSL certs and then apply SSL to LB. For the SSL cert management, I'd probably create a new CertificateService. I think all the functionality is really just manipulating the DB, so all the calls can be sync. (Don't let people remove a cert that is currently used). For associating SSL cert to LB, you can probably piggy back on the existing LB services/managers. Now one of my pet peeves in ACS is that Service interface and Manager interface are always implemented by the same class. This is bad, you end up bluring the lines of the architecture and code becomes a big blob, so avoid doing that. I'm guessing you won't need to create any new managers, so that probably won't apply. Darren On Tue, Oct 8, 2013 at 4:44 PM, Syed Ahmed <sah...@cloudops.com> wrote: > Thanks Edison for the reply. > > I see that there is already an implementation of KeystoreManager which does > certificate validation and saves it in the keystore table. Also, the API > (UploadCustomCertificate) is only callable from admin. I could add > functionality to this class for handling certificate chain and also make > sure the table stores the account_id as well. We could reduce creating one > table by reusing the keystore table. > > I have a question about terminology. What is a service and a manager because > I see them both being used. In my case, I assume that my CertificateService > will have the KeystoreManager injected and the Service will serve as a proxy > between the Resource layer and the KeystoreManager which is the Db layer. > Will this approach work? > > Thanks > -Syed > > > > On Tue 08 Oct 2013 06:56:34 PM EDT, Edison Su wrote: >> >> There is command in ACS, UploadCustomCertificateCmd, which can receive ssl >> cert, key can chain as input. Maybe can share some code? >> >>> -----Original Message----- >>> From: Darren Shepherd [mailto:darren.s.sheph...@gmail.com] >>> Sent: Tuesday, October 08, 2013 1:54 PM >>> To: dev@cloudstack.apache.org >>> Subject: Re: [New Feature FS] SSL Offload Support for Cloudstack >>> >>> The API should do input validation on the SSL cert, key and chain. >>> Getting those three pieces of info is usually difficult for most people >>> to get >>> right as they don't really know what those three things are. There's >>> about a >>> 80% chance most calls will fail. If you rely on the provider it will >>> probably just >>> give back some general failure message that we won't be able to map back >>> to >>> the user as useful information. >>> >>> I would implement the cert management as a separate CertificateService. >>> >>> Darren >>> >>> On Tue, Oct 8, 2013 at 1:31 PM, Syed Ahmed <syed1.mush...@gmail.com> >>> wrote: >>>> >>>> A question about implementation. I was looking at other commands and >>>> the >>>> execute() method for each of the other commands seem to call a service >>>> ( _lbservice for example ) which takes care of updating the DB and >>>> calling the resource layer. Should the Certificate management be >>>> implemented as a service or is there something else that I can use? An >>>> example would be immensely helpful. >>>> >>>> Thanks >>>> -Syed >>>> >>>> >>>> >>>> On Tue 08 Oct 2013 03:22:14 PM EDT, Syed Ahmed wrote: >>>>> >>>>> >>>>> Thanks for the feedback guys. Really appreciate it. >>>>> >>>>> 1) Changing the name to SSL Termination. >>>>> >>>>> I don't have a problem with that. I was looking at Netscaler all the >>>>> time and they call it SSL offloading. But I agree that termination is >>>>> a more general term. >>>>> I have changed the name. The new page is at >>>>> >>>>> >>> https://cwiki.apache.org/confluence/display/CLOUDSTACK/SSL+Terminatio >>>>> >>>>> n+Support >>>>> >>>>> >>>>> 2) Specify the protocol type. >>>>> >>>>> Currently the protocol type of a loadbalncer gets set by checking the >>>>> source and destination port ( see getNetScalerProtocol() in >>>>> NetscalerResouce.java ) . So, we should change that and add another >>>>> optional field in the createLoadBalancerRule for protocol. >>>>> >>>>> 3) Certificate chain as a seperate parameter. >>>>> >>>>> Again, I was looking at Netscaler as an example but separating the >>>>> chain and certificate makes sense. I have updated the document >>>>> accordingly. >>>>> >>>>> I was assuming that the certificate parsing/validation would be done >>>>> by the device and we would just pass the certficate data as-is. But >>>>> if we are adding chains separately, we should have the ability to >>>>> parse and combine the chain and certificate for some devices as you >>> >>> mentioned. >>>>> >>>>> >>>>> Thanks >>>>> -Syed >>>>> >>>>> >>>>> On Tue 08 Oct 2013 02:49:52 PM EDT, Chip Childers wrote: >>>>>> >>>>>> >>>>>> >>>>>> On Tue, Oct 08, 2013 at 11:41:42AM -0700, Darren Shepherd wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> Technicality here, can we call the functionality SSL termination? >>>>>>> While technically we are "offloading" ssl from the VM, offloading >>>>>>> typically carries a connotation that its being done in hardware. So >>>>>>> we are really talking about SSL termination. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> +1 - completely agree. There's certainly the possibility of an >>>>>> *implementation* being true offloading, but I'd generalize to >>>>>> "termination" to account for a non-hardware offload of the crypto >>>>>> processing. >>>>>> >>>>>>> >>>>>>> >>>>>>> Couple comments. I wouldn't want to assume anything about SSL >>> >>> based >>>>>>> >>>>>>> on port numbers. So instead specify the protocol >>>>>>> (http/https/ssl/tcp) for the front and back side of the load >>>>>>> balancer. Additionally, I'd prefer the chain not be in the cert. >>>>>>> When configuring some backends you need the cert and chain >>>>>>> separate. It would be easier if they were stored that way. >>>>>>> Otherwise you have to do logic of parsing all the certs in the >>>>>>> "keystore" >>> >>> and look for the one that matches the key. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Also +1 to this. Cert chains may be optional, certainly, but should >>>>>> actually be separate from the actual cert in the configuration. The >>>>>> implementation may need to combine them into one document, but >>>>>> that's implementation specific. >>>>>> >>>>>>> >>>>>>> >>>>>>> Otherwise, awesome feature. I'll tell you, from an impl >>>>>>> perspective, parsing and validating the SSL certs is a pain. I can >>>>>>> probably find some java code to help out here on this as I've done >>>>>>> this before in the past. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Yes, this is a sorely needed feature. I'm happy to see this be added >>>>>> to the Netscaler plugin, and await a time when HA proxy has a stable >>>>>> release that includes SSL term. >>>>>> >>>>>>> >>>>>>> >>>>>>> Darren >>>>>>> >>>>>>> On Tue, Oct 8, 2013 at 11:14 AM, Syed Ahmed <sah...@cloudops.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> I have been working on adding SSL offload functionality to >>>>>>>> cloudstack and make it work for Netscaler. I have an initial >>>>>>>> design documented at >>>>>>>> >>>>>>>> >>> https://cwiki.apache.org/confluence/display/CLOUDSTACK/SSL+Offload >>>>>>>> >>>>>>>> ing+Support >>>>>>>> >>>>>>>> and I would really love your feedback. The bug for this is >>>>>>>> https://issues.apache.org/jira/browse/CLOUDSTACK-4821 . >>>>>>>> >>>>>>>> Thanks, >>>>>>>> -Syed >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>>> >>>> > >
