[Pki-devel] [PATCH] pki-core-issuerDN-encoding.patch

[Christina Fu]

forgot to mention...Pushed to master.

commit 5bbd06e6e77729c63d65b77445f71f63ea0cdd1f 
<https://fedorahosted.org/pki/changeset/5bbd06e6e77729c63d65b77445f71f63ea0cdd1f/> 
Author: Christina Fu <cfu@…> Date: Wed Oct 15 10:30:31 2014 -0700

    Bug1151147 issuerDN encoding correction


On 10/24/2014 11:25 AM, John Magne wrote:
> OK, all this sounds reasonable.
>
> If the common cases have been tested to work.
>
> ACK
>
>
> ----- Original Message -----
>> From: "Christina Fu" <cfu at redhat.com>
>> To: "John Magne" <jmagne at redhat.com>
>> Cc: pki-devel at redhat.com
>> Sent: Friday, October 24, 2014 11:11:33 AM
>> Subject: Re: [Pki-devel] [PATCH] pki-core-issuerDN-encoding.patch
>>
>> Jack,
>> thanks for the review.  Please see response below.
>>
>> On 10/24/2014 11:02 AM, John Magne wrote:
>>> Christina:
>>>
>>> Looks good and glad to hear its tested to work, which solves a hairy
>>> problem.
>>>
>>> Just a couple of questions about a few places in the code:
>>>
>>>
>>> 1. In diff --git a/base/ca/src/com/netscape/ca/CertificateAuthority.java
>>> b/base/ca/src/com/netscape/ca/CertificateAuthority.java
>>>
>>> We have this block:
>>>
>>> +            String caSigningCertStr = caSigningCfg.getString("cert", "");
>>> +            if (caSigningCertStr.equals("")) {
>>> +                CMS.debug("CertificateAuthority:initSigUnit:
>>> ca.signing.cert not found");
>>> +            } else { //ca cert found
>>> +                CMS.debug("CertificateAuthority:initSigUnit: ca cert
>>> found");
>>> +                mCaCert = new X509CertImpl(CMS.AtoB(caSigningCertStr));
>>> +                // this ensures the isserDN and subjectDN have the same
>>> encoding
>>> +                // as that of the CA signing cert
>>> +                CMS.debug("CertificateAuthority: initSigUnit 1- setting
>>> mIssuerObj and mSubjectObj");
>>> +                mSubjectObj = mCaCert.getSubjectObj();
>>> +                // this mIssuerObj is the "issuerDN" obj for the certs
>>> this CA
>>> +                // issues, NOT necessarily the isserDN obj of the CA
>>> signing cert
>>> +                mIssuerObj = new
>>> CertificateIssuerName((X500Name)mSubjectObj.get(CertificateIssuerName.DN_NAME));
>>> +            }
>>> +
>>>
>>> Looks like you create a member variable mSubjectObj and an associated
>>> getter method.
>>> It seems that perhaps this is only used locally in this method to help
>>> create mIssuerObj, which is accessed later.
>>> Do we need this or did I miss something?
>> I decided to make it available though you are right that it is not
>> needed at this point.  I just figured since it's there, I will make it
>> available for future references.
>>> Also, what is supposed to happen when caSigningCertStr == "" ??
>>>
>>> Later on we have this in the same method:
>>>
>>> +            mSubjectObj = mCaCert.getSubjectObj();
>>> +            if (mSubjectObj != null) {
>>> +                // this ensures the isserDN and subjectDN have the same
>>> encoding
>>> +                // as that of the CA signing cert
>>> +                CMS.debug("CertificateAuthority: initSigUnit - setting
>>> mIssuerObj and mSubjectObj");
>>> +                // this mIssuerObj is the "issuerDN" obj for the certs
>>> this CA
>>> +                // issues, NOT necessarily the isserDN obj of the CA
>>> signing cert
>>> +                // unless the CA is self-signed
>>> +                mIssuerObj =
>>> +                        new
>>> CertificateIssuerName((X500Name)mSubjectObj.get(CertificateIssuerName.DN_NAME));
>>> +            }
>>>
>>> Question about this and other similar NULL checks for mSubjectObj. Can this
>>> really be null if everything was set up
>>> during the initialization phase of the CertificateAuthority?
>>>
>>> Also here if it is NULL, what happens, or is the intent to just keep going?
>> The code took different passes during installation and startup.  At very
>> early stage of the installation, the value is not expected to be there,
>> so we do want it to pass, and at later times, it will become available.
>> That is why we don't want to do anything if it's null.
>> I was being extra careful not to change the code path behavior and only
>> focus on getting the subject DN setup for later use.
>>>
>>> I was curious about those two since this appears to be pretty invasive to
>>> the system.
>>>
>>>
>>> thanks,
>>> jack
>>>
>>>
>>> ----- Original Message -----
>>>> From: "Christina Fu" <cfu at redhat.com>
>>>> To: pki-devel at redhat.com
>>>> Sent: Friday, October 24, 2014 9:25:22 AM
>>>> Subject: [Pki-devel] [PATCH] pki-core-issuerDN-encoding.patch
>>>>
>>>> Attached please find the fix for the following ticket:
>>>>
>>>> https://fedorahosted.org/pki/ticket/1190 CA: issuer DN encoding not
>>>> preserved at issuance with signing cert signed by an external CA
>>>>
>>>> thanks in advance for a review.
>>>> Christina
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Pki-devel mailing list
>>>> Pki-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/pki-devel
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20141027/bb086f1d/attachment.htm>