[PATCH] 657 Refactored CA certificate generation.
The CA certificate request and signing processes have been moved from the configuration servlet into the deployment scriptlet. This way the admin will have the option to: * generate self-signed CA certificate * import externally-signed CA certificate * import existing CA certificate before the server is started for the first time. https://fedorahosted.org/pki/ticket/456 Note: This is a preliminary patch. There are some unfinished works. -- Endi S. Dewata
On 11/9/2015 1:59 PM, Endi Sukma Dewata wrote:
The CA certificate request and signing processes have been moved from the configuration servlet into the deployment scriptlet. This way the admin will have the option to:
* generate self-signed CA certificate * import externally-signed CA certificate * import existing CA certificate
before the server is started for the first time.
https://fedorahosted.org/pki/ticket/456
Note: This is a preliminary patch. There are some unfinished works.
Attached is the actual patch. -- Endi S. Dewata
This is what I have so far. Just a few comments on the overall logic. I'm not making any Python coding-specific comments. 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') 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? 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; + } 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? 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. Christina On 11/16/2015 09:24 AM, Endi Sukma Dewata wrote:
On 11/9/2015 1:59 PM, Endi Sukma Dewata wrote:
The CA certificate request and signing processes have been moved from the configuration servlet into the deployment scriptlet. This way the admin will have the option to:
* generate self-signed CA certificate * import externally-signed CA certificate * import existing CA certificate
before the server is started for the first time.
https://fedorahosted.org/pki/ticket/456
Note: This is a preliminary patch. There are some unfinished works.
Attached is the actual patch.
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
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_Certif...
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. -- Endi S. Dewata
The fixed areas look good. If tested to work, ACK. Christina On 11/23/2015 11:54 AM, Endi Sukma Dewata wrote:
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_Certif...
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.
Looks ok to me, ACK but will defer more strongly to cfu on this one. One quick thing: The routine that creates the cert request doesn't appear to massage the key related params much. For instance if someone would give the RSA key sizes and an ECC curve name, the responsibility to check this would move down to the system call. Not sure this is worth fixing so just making it optional. ----- Original Message ----- From: "Christina Fu" <cfu@redhat.com> To: "Endi Sukma Dewata" <edewata@redhat.com>, pki-devel@redhat.com Sent: Monday, November 23, 2015 4:24:51 PM Subject: Re: [Pki-devel] [PATCH] 657 Refactored CA certificate generation. The fixed areas look good. If tested to work, ACK. Christina On 11/23/2015 11:54 AM, Endi Sukma Dewata wrote:
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_Certif...
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.
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
On 11/23/2015 6:43 PM, John Magne wrote:
Looks ok to me, ACK but will defer more strongly to cfu on this one.
One quick thing:
The routine that creates the cert request doesn't appear to massage the key related params much. For instance if someone would give the RSA key sizes and an ECC curve name, the responsibility to check this would move down to the system call.
Not sure this is worth fixing so just making it optional.
Yes, the code is intentionally doing just the minimal checking for key-related parameters such that if NSS introduces a new behavior (e.g. supporting new curve) PKI will automatically pick it up without any modification. They key size & curve name are passed directly to certutil assuming that NSS will do the validation and will fail if the values aren't valid. It's doing a little bit more checking on the key algorithm because it needs to parse the hash algorithm out of it, but the hash algorithm itself is passed directly to certutil, PKI doesn't validate it. Is this ok? -- Endi S. Dewata
Sounds fine. Like I said ACK, and my comments were optional, and with your explanation, sounds good. ----- Original Message ----- From: "Endi Sukma Dewata" <edewata@redhat.com> To: "John Magne" <jmagne@redhat.com>, "Christina Fu" <cfu@redhat.com> Cc: pki-devel@redhat.com Sent: Tuesday, 24 November, 2015 6:20:54 AM Subject: Re: [Pki-devel] [PATCH] 657 Refactored CA certificate generation. On 11/23/2015 6:43 PM, John Magne wrote:
Looks ok to me, ACK but will defer more strongly to cfu on this one.
One quick thing:
The routine that creates the cert request doesn't appear to massage the key related params much. For instance if someone would give the RSA key sizes and an ECC curve name, the responsibility to check this would move down to the system call.
Not sure this is worth fixing so just making it optional.
Yes, the code is intentionally doing just the minimal checking for key-related parameters such that if NSS introduces a new behavior (e.g. supporting new curve) PKI will automatically pick it up without any modification. They key size & curve name are passed directly to certutil assuming that NSS will do the validation and will fail if the values aren't valid. It's doing a little bit more checking on the key algorithm because it needs to parse the hash algorithm out of it, but the hash algorithm itself is passed directly to certutil, PKI doesn't validate it. Is this ok? -- Endi S. Dewata
On 11/23/2015 6:24 PM, Christina Fu wrote:
The fixed areas look good. If tested to work, ACK.
On 11/24/2015 11:45 AM, John Magne wrote:
Sounds fine. Like I said ACK, and my comments were optional, and with your explanation, sounds good.
Thanks! Pushed to master. -- Endi S. Dewata
participants (3)
-
Christina Fu -
Endi Sukma Dewata -
John Magne