Re: [Pki-devel] [PATCH] 657 Refactored CA certificate generation.

Thanks for the feedback. New patch attached. On 11/16/2015 7:48 PM, Christina Fu wrote: > 1 in > base/server/python/pki/server/deployment/scriptlets/configuration.py > doesn't this just add the leaf cert rather than the whole chain? In > other words, if your chain contains 2 or more certs, only the leaf subca > cert is added, isn't it? > > + nssdb.add_cert( > + nickname=external_ca_nickname, > + cert_file=external_ca_cert_chain_file, > + trust_attributes='CTu,CTu,CTu') Fixed. The new patch now supports PKCS #7 file, a single PEM cert, and the base-64 PKCS #7 data generated by getCertChain servlet. > 2 Also in the same file > + # If specified, import externally-signed CA cert in NSS database. > ... > Shouldn't there be a case when the externally signed ca keys were > generated on the hsm, you'd then need to import the issued externally > signed ca cert into the hsm db as well? If the externally-signed CA cert is specified in pki_external_ca_cert_path parameter it will be imported into the NSS database, regardless whether HSM is used. The code now calls certutil with -h option to specify the target token. Alternatively, the certificate can be imported manually before starting step 2. I've updated the docs: http://pki.fedoraproject.org/wiki/Installing_with_Externaly-Signed_CA_Cer... > 3 base/common/src/com/netscape/certsrv/system/ConfigurationRequest.java > I"m not seeing the following method being called, yet the getExternal() > is being called...did I miss something? > > + public void setExternal(Boolean external) { > > + this.external = external; > + } The external attribute is set in Python in pkihelper.py:3983: data.external = self.external The value will be sent to the server via REST interface. The getExternal() will read that value. > 4. > base/server/cms/src/com/netscape/cms/servlet/csadmin/ConfigurationUtils.java > + public static void loadCert(Cert cert) throws Exception { > ... > + // create certificate record to reserve the serial number in > internal database > + ICertRecord record = cr.createCertRecord(serialNo, > x509CertImpl, meta); > + cr.addCertificateRecord(record); > > In case of an externally signed ca or existing ca, why would you need to > reserve the serial number or even add in the certificate repository? Fixed. This code is actually only needed when importing existing self-signed CA cert. This way when the code generates the system certificates it will not conflict with the CA cert's serial number both in NSS database and in internal LDAP database. For existing non-self-signed CA cert or externally signed CA cert the code will not create the LDAP record. > 5. > Finally, please add comments to explain the cases for clarification... > such as "stand-alone v.s. external; step 1, step 2, etc." For example, > it seems the "external" could imply "existing" as well in terms of ca > cert, you might want to put in comment. Yes, the "external" code handles both external CA and existing CA cases. I've added some inline comments. Please let me know if we need more.