<html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 04/08/13 17:02, Matthew Harmsen wrote: </div> <blockquote cite="mid:51635A8C.1060202@redhat.com" type="cite"> <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1"> <tt>Please perform an initial code review on the attached patches (only applicable for RHCS 8.1 on RHEL 5).</tt><tt> </tt><tt> </tt></blockquote> <tt>Three new patches (two which are revisions to the previous patches, and one which represents a simple recursive diffs bet</tt><tt>ween the two 'pki' trees which contain the code changes) </tt><tt>have been attached with address the following issues raised during code review (also see inline comments regarding other issues):</tt><tt> </tt> <ul> <li><tt>base/common/src/com/netscape/cms/authentication/TokenAuthentication.java</tt><tt>:</tt></li> <ul> <li><tt><strike>remove CMS.debug("TokenAuthentication: givenHost=" + givenHost);</strike></tt></li> </ul> <li><tt>base/common/src/com/netscape/cms/servlet/csadmin/*Panel.java:</tt></li> <ul> <li><tt><strike>rename 'buildSANsslserverURLextension' to 'buildSANSSLserverURLExtension'</strike></tt></li> <li><tt><strike>fix preop.ca.hostname (be explicit as to which host this refers to)</strike></tt></li> </ul> <li><tt>base/common/src/com/netscape/cms/servlet/csadmin/ImportAdminCertPanel.java:</tt></li> <ul> <li><tt><strike>try to make them all use EE host and EE port (which did not work as the EE connection is unavailable during installation of a CA)</strike></tt></li> <li><tt><strike>since that did not work for all cases, fixed all cases to utilize Admin host and Admin port as requested</strike></tt></li> </ul> <li><tt>base/common/src/com/netscape/cms/servlet/csadmin/WizardPanelBase.java:</tt></li> <ul> <li><tt><strike>break line </strike></tt><tt><strike>CMS.debug("WizardPanelBase updateDomainXML start hostname=" + hostname + " port=" + port + " url=" + servlet + " content=" + uri);</strike></tt></li> <li><tt><strike>change 'Vector v_admin_host = parser.getValuesFromContainer( nodeList.item(i), "Host" );' to 'Vector v_admin_host = parser.getValuesFromContainer( nodeList.item(i), "AdminHost" );'</strike></tt></li> </ul> <li><tt>base/pkisilent/templates:</tt></li> <ul> <li><tt><strike>fixed fa</strike></tt><tt><strike>ilure of </strike></tt><tt><strike>pkisilent to </strike></tt><tt><strike>successfully con</strike></tt><tt><strike>figu</strike></tt><tt><strike>re a PKI instance</strike></tt></li> <li><tt><strike>New IP Port Separation pkisilent templates have been created for CA, KRA, OCSP, and TKS</strike></tt></li> <li><tt><strike>New pkisilent templates for CA and KRA utilizing IP Port Separation were successfully executed</strike></tt></li> </ul> <li><tt>base/setup/pkicommon:</tt></li> <ul> <li><tt><strike>make 'addr' a local variable rather than global variable</strike></tt></li> <li><tt><strike><strike>used join() for SAN uniqueness routine</strike></strike></tt></li> <li><tt><strike>renamed '</strike></tt><tt><strike>IsPortConfigurationModeValid' to 'get_port_configuration_mode' and changed it to return strings rather than integers</strike></tt></li> <li><tt><strike>added logic to check for unlabeled ports being defined on installation host primarily to support IP Separation (e. g. - all interfaces distinguishable by unique IPs using a common port)</strike></tt></li> </ul> </ul> <tt>The lone remaining item that MUST be addressed (besides any additional feedback associated with these revised patches) is:</tt><tt> </tt> <ul> <li><tt>reported concerns regarding the ability to install</tt><tt>/</tt><tt>configure an RA/TPS instance </tt><tt>which uses the </tt><tt>existing code changes </tt><tt>required</tt><tt> for in</tt><tt>teraction with </tt><tt>the revised security domain</tt></li> <li><tt>will be </tt><tt>investigat</tt><tt>ed starting on 4/11/2013</tt></li> </ul> <tt> </tt><tt>The new patches do not address the following items from the previous code review, and may not be addressed due to schedule and resources:</tt><tt> </tt> <ul> <li><tt>base/setup/pkiremove:</tt></li> <ul> <li><tt>revive 'use strict' - was removed since 'pkiremove' now references variables from the 'require pkicommon' file; this was probably the cause for 'use strict' not being a part of 'pkicreate' </tt></li> <li><tt>in pkiremove, in the function where is is determined which selinux ports to remove, the $i variable is used to track the index of the array - no need to do that -- just use append()</tt></li> </ul> <li><tt>base/setup/pkicommon:</tt></li> <ul> <li><tt>modularization of IsPortConfigurationModeValid() - e. g. - uniqueness helper functions to replace large conditional blocks</tt></li> <li><tt>refactor IsPortConfigurationModeValid() - rejected as it was discussed that since the code has been tested numerous times, and while this may help with maintainability, this code is only used for the 8.1 code base errata process</tt></li> <li><tt>standardize coding style - rejected for the 8.1 code base -- this has already been addressed in the Dogtag 10 code base</tt></li> </ul> </ul> <tt>-- Matt </tt> <blockquote cite="mid:51635A8C.1060202@redhat.com" type="cite"><tt> </tt><tt>The following two patches address:</tt><tt> </tt> <ul> <li><tt>'pkicreate' now does three types of port configuration:</tt></li> <ul> <li><tt>IP Port Separation</tt></li> <li><tt>Port Separation</tt></li> <li><tt>Shared Ports (deprecated)</tt></li> </ul> <li><tt>security manager issue was fixed</tt><tt> </tt></li> <li><tt>new security domain schema is complete</tt></li> <li><tt>the security domain has been implemented</tt><tt> to c</tt><tt>omply with this new schema</tt><tt> </tt></li> <li><tt>generated a multi-host CA complete with an SSL Server Certificate containing SAN information (</tt><tt>utilizes profile framework)</tt><tt> </tt> </li> <li> <tt>generated a multi-host KRA complete with an SSL Server Certificate containing SAN information (utilizes name/value pairs passed in via the enrollment URL which are processed via the profile framework)</tt></li> <li><tt>addressed 'TokenAuthenticate' SSL_ForceHandshake issue by utilizing DNSName instead of DirectoryName attributes in the </tt><tt>SSL Server certificate SAN extensions</tt></li> <li><tt>applied the checkIP() feature described in </tt><tt>'</tt><tt>Bugzilla Bug #708075 -</tt><tt> Clone installation does not work over NAT'</tt><tt></tt><tt></tt></li> <li><tt>applied substitution of raw IP addresses from 'pkicreate' into the 'server.xml' to support the new IP Port Separation mode</tt></li> </ul> <tt>Dev</tt><tt>elopment test info:</tt><tt> </tt> <ul> <li><tt>pki-ip-host (installation host - RHEL 5.9 x86_64)</tt></li> <ul> <li><tt>pki-ca-agent (CA agent interface - virtual IP)</tt></li> <li><tt>pki-ca-ee (CA EE interface</tt><tt><tt> - virtual IP</tt>)</tt></li> <li><tt>pki-ca-ee-ca (CA EE clientauth interface</tt><tt><tt> - virtual IP</tt>)</tt></li> <li><tt>pki-ca-admin (CA admin interface</tt><tt><tt> - virtual IP</tt>)</tt></li> <li><tt>pki-kra-agent (KRA agent interface</tt><tt><tt> - virtual IP</tt>)</tt></li> <li><tt>pki-kra-ee (KRA EE interface</tt><tt><tt> - virtual IP</tt>)</tt></li> <li><tt>pki-kra-admin (KRA admin interface</tt><tt><tt> - virtual IP</tt>)</tt></li> </ul> <li><tt>pki-rhel6 (RHDS 9.1 - RHEL 6.3 x86_64 which uses a different domain) </tt></li> </ul> <tt>Thus far, only </tt><tt>t</tt><tt>he following tests have been run against th</tt><tt>ese patches</tt><tt>:</tt><tt> </tt> <ul> <li><tt></tt><tt>successfully tested regression case of CA and KRA installed using Port Separation</tt><tt></tt></li> <li><tt>successfully tested sanity case of CA and KRA installed using IP Port Separation</tt></li> <li><tt>successfully tested mixed mode deployment case of a CA installed using Port Separation and a KRA installed using IP Port Separation</tt></li> <li><tt>successfully tested mixed mode deployment case of a CA installed using IP Port Separation and a KRA installed using Port Separation</tt></li> <li><tt>successfully tested miscellaneous case of specifying a CA with four virtual IPs (none of which belonged to the host that the server was being installed upon) using IP Port Separation</tt></li> <li><tt>successfully tested miscellaneous case of </tt><tt>CA and KRA installed using IP Port Separation utilizing unique IP addresses for each interface (none of which specified the installation host IP), but specifying the same HTTP/HTTPS port numbers (e. g. - 19080/19443) and unique ports for Tomcat (9701/10701)</tt></li> <ul> <li><tt>NOTE: I managed to successfully test this case with SELinux in Enforcing mode -- this is because the only ports that would be labeled are the Tomcat ports which exist on the installation machine (which do not in this case, as they are the default cases for pki_ca_port_t and pki_kra_port_t). In this test case, since none of the interfaces refer to the installation machine IP, none of these ports are labeled by SELinux. The 'pkicreate' executable enforces unique <hostname:port> entries. While a second instance (e. g. - KRA) could be installed re-using the <hostname:port> entries specified (e. g. - CA), the two instances could not be started simultaneously due to an inability to bind (java.net.BindException: Address already in use) - see 'netstat -a | grep <host>' or 'netstat -a | grep <port>'. </tt></li> </ul> <li><tt>successfully tested miscellaneous case of installing a CA using IP Port Separation which was configured using a customized SAN 'serverCert.profile' which included two additional SAN entries on top of the entries computed for IP Port Separation</tt></li> </ul> <tt>The following issues are still activ</tt><tt>ely </tt><tt>being addressed:</tt><tt> </tt><tt></tt> <ul> <li><tt>f</tt><tt>ailure of java security manager to </tt><tt>allow server to start when specifying </tt><tt>non-installation host </tt><tt>ports 80/443 (SELinux in permissive mode) results in (</tt><tt>java.net.BindException: Permission denied:80) - (i. e. - see <a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://www.jvmhost.com/articles/java-net-bindexception-permisssion-denied-operation-not-permitted">http://www.jvmhost.com/articles/java-net-bindexception-permisssion-denied-operation-not-permitted</a>) </tt></li> </ul> </blockquote> <tt>This issue will be documented, and does not block the release of this patch.</tt> <blockquote cite="mid:51635A8C.1060202@redhat.com" type="cite"> <ul> <li><tt> </tt></li> <li><tt>fa</tt><tt>ilure of </tt><tt>pkisilent to </tt><tt>successfully con</tt><tt>figu</tt><tt>re a PKI instance</tt></li> </ul> </blockquote> <tt>Fixed -- new pkisilent templates for CA and KRA utilizing IP Port Separation were successfully executed. New IP Port Separation pkisilent templates have been created for CA, KRA, OCSP, and TKS.</tt> <blockquote cite="mid:51635A8C.1060202@redhat.com" type="cite"> <ul> <li><tt>reported concerns regarding the ability to install</tt><tt>/</tt><tt>configure an RA/TPS instance </tt><tt>which uses the </tt><tt>existing code changes </tt><tt>required</tt><tt> for in</tt><tt>teraction with </tt><tt>the revised security domain</tt><tt> </tt></li> </ul> </blockquote> <tt>This last remaining issue will be investigated starting on 4/11/2013. </tt> <blockquote cite="mid:51635A8C.1060202@redhat.com" type="cite"> <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ Pki-devel mailing list <a class="moz-txt-link-abbreviated" href="mailto:Pki-devel@redhat.com">Pki-devel@redhat.com</a> <a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/pki-devel">https://www.redhat.com/mailman/listinfo/pki-devel</a></pre> </blockquote> </body> </html>