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(a)redhat.com>
> To: pki-devel(a)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(a)redhat.com
>
https://www.redhat.com/mailman/listinfo/pki-devel