[Pki-devel] [PATCH] pki-core-issuerDN-encoding.patch
John Magne
jmagne at redhat.com
Fri Oct 24 18:25:41 UTC 2014
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
>
>
More information about the Pki-devel
mailing list