> 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.
You are right. This just handles a single certificate. It does not handle trust chains. > (Don't let people remove a cert that is currently used). You bring up an good point. I assumed that if the user deletes a certificate, we internally go ahead and remove all the bindings from LB rules. This happens with VM instances. The difference that I see is if you remove the certificate, the LB goes down but if you remove an instance the LB may still work. > I'm guessing you won't need to create any > new managers, so that probably won't apply. If I am giving the functionality of updating a certificate then my CertificateService should call the resource layer for updating on the device as well. Now from you mail what I understand is this has to be done by creating a manager. So either we go ahead and drop the updateCertificate call or we will have to add a manager for this. I prefer dropping the updateCert call as I don't see anyone updating cert contents frequently. Thanks -Syed On Wed, Oct 9, 2013 at 1:02 AM, Darren Shepherd <darren.s.sheph...@gmail.com> wrote: > 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). > 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. > > 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 >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>>> >> >>
