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

[John Magne]

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
> 
>