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

[John Magne]

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 at redhat.com>
To: "Endi Sukma Dewata" <edewata at redhat.com>, pki-devel at 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_Certificate
>
>> 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 at redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel