[Pki-devel] [PATCH] PKI Deployment Framework PKI TRAC issues (08/09/2012)

[Ade Lee]

I'm not sure where the idea that you do not need a security domain for
an external CA came from.  You always need a security domain - either
new or existing.  I'm going to push the patch as-is for now, but this
will be one of the things that needs to be fixed for external installs.

I would have hoped that you would define the set_default() function or
similar.  Without it, the code is still very difficult to parse and that
slows down reviews and makes it likely for errors to creep in.  Also, to
make it more concise, we can replace pki_master_dict with just master.
We can leave this for a future patch, but I would like to see it soon.

Compare: 
if not len(config.pki_master_dict['pki_subsystem_name']):
            config.pki_master_dict['pki_subsystem_name'] =\
                config.pki_subsystem + " " +\
                config.pki_master_dict['pki_hostname'] + " " +\
                config.pki_master_dict['pki_https_port']

to

set_default('pki_subsystem_name', str.format("{} {} {}",\
    (config.pki_subsystem, master['pki_hostname'], master['pki_https_port']))

Pushed to master.

On Fri, 2012-08-10 at 22:15 -0700, Matthew Harmsen wrote:
> The attached patch (new code which has been combined with the previous 
> patch) addresses Comments 1 and 2 below.
> 
> This code has been successfully tested on a 64-bit Fedora 17 machine for 
> a CA, a Cloned CA, and a KRA.
> 
> NOTE:  While the logic in comment 2 is correct from a "configuration" 
> viewpoint, the "security_domain_name" is also required by other sections 
> of the "installation" code in
>         generating various certificate subject DNs for non-root CA 
> subsystems and will generate a "java.io.IOException: Bad AVA format: 
> Missing attribute value" exception
>         during "configuration" if it is not defined, thus why the 
> altered code in 'pkiparser.py' does not match the proposed logic verbatim.
> 
> -- Matt
> 
> P. S. -- Ade, once again, as you are the most probable reviewer of this patch, please feel free to 'push' it to 'master' if you find it in order.
> 
> On 08/10/12 07:50, Ade Lee wrote:
> > The patch works.  I was able to get a KRA installed.
> >
> > Comments:
> > 1. The logic in pkijython for determining whether to send the issuing CA
> > information is incorrect.  Specifically, all clones still need to
> > contact a CA to generate their server cert.
> >
> > 2.  The same logic applies to the code in pkiparser.py.  In fact, I
> > think we can simplify the logic there significantly.  There is no need
> > to distinguish in the subsystem name whether the server is a clone or
> > subordinate or external.  Just use "{subsystem_type} {hostname} {port}"
> > for all subsystems (apache and tomcat).
> >
> > Its very difficult to follow the logic in that section. For the
> > parameters in that section the logic should be:
> >
> > # for all subsystems
> > set_default(subsystem_name, "{subsystem_type} {hostname} {port}")
> >
> > if (root ca) {
> >     security_domain_type = "new"
> >     set_default(security_domain_name, "{dnsname} Security Domain")
> > } else {
> >     security_domain_type = "existing"
> >     set_default(security_domain_host, "{pki_hostname}")
> >     set_default(security_domain_uri, "https:// {security_domain_host}:{security)_domain_port}")
> > }
> >
> > where set_default() is defined as :
> >
> > set_default(x, y) {
> >     if not len (master_dict[x]) {
> >         master_dict[x] = y
> >     }
> > }
> >
> > I need to think about the conditional a bit to decide when we can say we
> > need a new vs. existing security domain.
> >
> > Ade
> >
> > On Thu, 2012-08-09 at 17:29 -0700, Matthew Harmsen wrote:
> >> This patch documents continued implementation of the PKI Deployment
> >> Framework based upon the revised filesystem layout documented here:
> >>        * http://pki.fedoraproject.org/wiki/PKI_Instance_Deployment#CA_.2F_KRA_.2F_OCSP_.2F_RA_.2F_TKS_.2F_TPS
> >> This patch addresses the following issues:
> >>        * TRAC Ticket #184 - Dogtag 10: Update PKI Deployment to handle
> >>          cloning CA/KRA/OCSP/TKS . . .
> >>        * TRAC Ticket #285 - Dogtag 10: Fix installation issues for KRA,
> >>          OCSP, and TKS
> >> It has been tested and proven to work successfully to deploy a KRA as
> >> a separate instance on a 64-bit Fedora 17 machine (using the
> >> appropriate 'tomcatjss.jar').
> >>
> >> P. S. -- Ade, as you are the most probable reviewer of this patch,
> >> please feel free to 'push' it to 'master' if you find it in order.
> >> _______________________________________________
> >> Pki-devel mailing list
> >> Pki-devel at redhat.com
> >> https://www.redhat.com/mailman/listinfo/pki-devel
> >
>