[Pki-devel] [PATCH] PKI Deployment Framework (20120716)

[Ade Lee]

Initial Comments:

1. As specified on #irc, please move the spawn or destroy log
into /var/log/pki (and also remove the code that cleans this location
when the last instance has been removed).  This will make the selinux
rules cleaner.

2. Does CS.cfg include entries that are no longer needed because they
are used by pkicreate?  Please file a trac task to remove these
post-alpha.  Also, please check that the pkicreate* entries are not used
elsewhere in the code (and are correct if they are).

3. Catalina.properties contains commented out jars for each of the
subsystems in the common.loader.  These should be removed.

4. In server.xml:
After the line: <!-- DO NOT REMOVE - Begin define PKI secure port
there is a line with a single 1 on it (and nothing else)  What is that
for?

5. tomcat.conf.  Tomcat is currently configured to not run under a
security manager.  Please create a task to re-enable a security manager.

6. We are still creating a default web.xml at common/shared/web.xml.  Is
this something that is still needed?  Can it be removed? 

7. pkideployment.cfg contains an "[Optional]" section which contains
values which may be changed, but for which default values are provided
if needed.  The user though has no way of knowing what these defaults
are, and so cannot determine if they should override them.  Would it
make more sense to fill in these optional values with the defaults?
What distinguishes there parameters from other parameters in the file?

8. In pkispawn and pkidestroy, why do we get the domain name?  So, if
the domain name is not set, can we no longer do pkispawn/destroy?  Under
what conditions will the subprocess throw an exception and exit?

9. pkijython.py -- construct_pki_configuration_data:
   a) external CA does not require new security domain
   b) is the subsystem name not configurable?  The defaults should also 
      probably be more identifiable - say including host and port.
   c) clone may not require hierarchy to be set to "root"
   
In general though, I find the structure of this function very confusing.
It would be better - or more easily maintainable - to break this into
separate functions for each subsystem type.  
ie. configureRootCA(), configureSubordinateCA(),
configureClonedRootCA(), configureClonedSubordinateCA() etc.

and have them call functions like:
createSigningCertData(), createTrnasportCertData() and the like.

It may be more code to do this, but it will be clearer than a set of
nested if-then statements.

More comments after my lunch ..
Ade


 
On Mon, 2012-07-16 at 19:28 -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
> The following patch adds/corrects functionality of the existing PKI
> Deployment Framework including (but not limited to):
>       * Integration of Tomcat 7
>       * Introduction of dependency upon tomcatjss 7.0
>       * Removal of http filtering configuration mechanisms
>       * Introduction of additional slot substitution to support
>         revised filesystem layout
>       * Addition of 'pkiuser' uid:gid creation methods
>       * Inclusion of per instance '*.profile' files
>       * Introduction of configurable 'configurationRoot' parameter
>       * Introduction of default configuration of 'log4j' mechanism
>         (alee)
>       * Modify web.xml to use new Application classes to bootstrap
>         servers (alee)
>       * Introduction of "Wrapper" logic to support Tomcat 6 --> Tomcat
>         7 API change (jmagne)
>       * Added jython helper function to allow attaching a remote java
>         debugger (e. g. - eclipse)
> Additionally, this patch has been re-based against the current
> 'master' and has been successfully executed to completion with regards
> to installing a CA, enrolling for a certificate, and accepting a
> certificate on a 64-bit Fedora 17 installation.
> 
> -- Matt
> 
> 
> _______________________________________________
> Pki-devel mailing list
> Pki-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel