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

[Ade Lee]

ACK.

Only caveat - the commit message is badly formatted -- shows up in one
long line.

Ade

On Thu, 2012-08-16 at 20:13 -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 issues listed below as well as the following
> issues:
>       * TRAC Ticket #266 - for non-master CA subsystems, pkidestroy
>         needs to contact the security domain to update the domain
>       * Made Fedora 17 rely upon tomcatjss 7.0.0 or later
>       * Changed Dogtag 10 build-time and runtime requirements for
>         'pki-deploy'
>       * Altered PKI Package Dependency Chain (top-to-bottom):  pki-ca,
>         pki-kra, pki-ocsp, pki-tks --> pki-deploy --> pki-common
>       * Changed TPS to require a build-time dependency of 'httpd-devel
>         >= 2.4.2'
>         
> It has been tested and proven to work successfully to
> spawn/destroy/spawn a KRA as a separate instance on a 64-bit Fedora 17
> machine (using the appropriate 'tomcatjss.jar').
> 
> On 08/15/12 12:50, Ade Lee wrote:
> 
> > 1. As discussed on #irc, the correct fix is to add null as the last
> > argument for the outputError() function calls when status is sent in.
> > Please fix this for all of these calls.
> > 
> > 2. Use dict function get(foo, default) rather than setdefault(foo,
> > default)
> > 
> > 3. The line : nick = subsystemnick.split(' ', 2) is confusing and not
> > necessary.  Its better to use code like this:
> > 
> > if ':' in subsystemnick: 
> >     token_name = subsystemnick.split(':')[0]
> > else: 
> >     token_name = "internal"
> > 
> > 4. Please use str.format() when constructing big strings like the sslget
> > command.
> > 
> > 5. In the case where you check if the security domain is defined, you
> > should log that it does not and then return (NOT exit).
> > 
> > 6. We should not exit in any cases here except if the sslget call has an
> > invocation error.  If there is an error, it should be prominently logged
> > but it should not stop the pkidestroy.
> > 
> > 7. Check what happens if sslget fails to reach the server.  In this
> > case, it is likely that status will be set to None (along with error).
> > If this is the case, right now your code will throw an exception.
> > 
> > Ade
> > 
> > 
> > On Tue, 2012-08-14 at 18:21 -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 #266 - for non-master CA subsystems, pkidestroy
> > >         needs to contact the security domain to update the domain
> > >       * Made Fedora 17 rely upon tomcatjss 7.0.0 or later
> > > It has been tested and proven to work successfully to
> > > spawn/destroy/spawn a KRA as a separate instance on a 64-bit Fedora 17
> > > machine (using the appropriate 'tomcatjss.jar').
> > > 
> > > P. S. - While fixing the parameters passed via "outputError()" in
> > > 'base/common/src/com/netscape/cms/servlet/csadmin/UpdateDomainXML.java', I noticed that several of the other servlets in this directory also utilized the "AUTH_FAILURE" error value for the second argument of "outputError()" which gets passed as the string "2" --- while this string is technically acceptable, I believe that this may be old usage of some legacy parent method since "outputError()" is currently defined in "base/common/src/com/netscape/cms/servlet/base/CMSServlet.java" as:
> > >       * protected void outputError(HttpServletResponse httpResp,
> > >         String errorString)
> > >       * protected void outputError(HttpServletResponse httpResp,
> > >         String errorString, String requestId)
> > >       * protected void outputError(HttpServletResponse httpResp,
> > >         String status, String errorString, String requestId)
> > > so for all of my changes to "outputError()" in "UpdateDomainXML.java",
> > > I merely changed these incorrect three parameter call versions to the
> > > two parameter call version by removing the second parameter
> > > ("AUTH_FAILURE").  If I am correct about this seemingly legacy usage,
> > > please let me know if I need to file a TRAC ticket for this issue.
> > > 
> > > Thanks,
> > > -- Matt 
> > > _______________________________________________
> > > Pki-devel mailing list
> > > Pki-devel at redhat.com
> > > https://www.redhat.com/mailman/listinfo/pki-devel
> > 
>