Ade: This looks good but I have a question. Looking at the function you added: private static boolean importRequired(ArrayList<String> masterList, String nickname) { + if (masterList.contains(nickname)) + return true; + try { + X500Name xname = new X500Name(nickname); + for (String key: masterList) { + try { + X500Name xkey = new X500Name(key); + if (xkey.equals(xname)) return true; + } catch (IOException e) { + // xkey not an X500Name + } + } + + } catch (IOException e) { + // nickname is not a x500Name + return false; + } + return false; + } It looks like the top of this function does a String comparison just like the code you had in there but commented out already: if (masterList.contains(nickname)) + return true; As I understand the List contains method calls the equals method of the objects involved. Subsequently it looks like you rifle through the whole list and do a comparison between X500Name objects, which represent distinguished names. Why is this done? There are cases where the DN's are equivalent but their raw Strings may differ?

thanks, jack ----- Original Message ----- > From: "Ade Lee" <alee(a)redhat.com&gt; > To: pki-devel(a)redhat.com > Sent: Wednesday, June 26, 2013 11:28:42 AM > Subject: [Pki-devel] [PATCH] 0134-Make-sure-only-the-master-keys-and-certs-are-imported > > Make sure only the master keys and certs are imported. > > The key import code was written for when there was only one > subsystem per tomcat instance, and only one subsystems certs > and keys per p12 file. We need to ensure that only the master's > subsystem keys and certs are imported. Otherwise, unpredictable > behavior happens, like in Ticket 665. > > Please review, > > Thanks, > Ade > > > _______________________________________________ > Pki-devel mailing list > Pki-devel(a)redhat.com > https://www.redhat.com/mailman/listinfo/pki-devel

Thanks for info. Therefore: ACK ----- Original Message ----- From: "Ade Lee" <alee(a)redhat.com&gt; To: "John Magne" <jmagne(a)redhat.com&gt; Cc: pki-devel(a)redhat.com Sent: Wednesday, June 26, 2013 6:06:44 PM Subject: Re: [Pki-devel] [PATCH] 0134-Make-sure-only-the-master-keys-and-certs-are-imported On Wed, 2013-06-26 at 19:03 -0400, John Magne wrote: > Ade: > > This looks good but I have a question. > > Looking at the function you added: > > private static boolean importRequired(ArrayList<String> masterList, String nickname) { > + if (masterList.contains(nickname)) > + return true; > + try { > + X500Name xname = new X500Name(nickname); > + for (String key: masterList) { > + try { > + X500Name xkey = new X500Name(key); > + if (xkey.equals(xname)) return true; > + } catch (IOException e) { > + // xkey not an X500Name > + } > + } > + > + } catch (IOException e) { > + // nickname is not a x500Name > + return false; > + } > + return false; > + } > > It looks like the top of this function does a String comparison just like the code you had in there but commented out already: > > if (masterList.contains(nickname)) > + return true; > > As I understand the List contains method calls the equals method of the objects involved. > > Subsequently it looks like you rifle through the whole list and do a comparison between X500Name objects, which represent distinguished names. > Why is this done? There are cases where the DN's are equivalent but their raw Strings may differ? > The list of names consists of two types of strings - nicknames like "auditSigningCert pki-tomcat CA" and subject names like "CN= CA Audit Singing Cert, O=redhat domain". The masterList also contains similar names. The first call of the contains() method does a string comparison and so handles the cases where the nicknames are the same. For the subject names, I found that this was insufficient because the strings were not exactly the same. In particular, the masterList contained entries like: "cn= CA Audit Singing Cert, o=redhat domain", while the list of names from the pk12 file contained the following: "CN= CA Audit Singing Cert, O=redhat domain" Notice the difference in case for the field names. Parsing the name as an X500Name and using the equals() method for those objects eliminates those discrepancies. Ade > thanks, > jack > > ----- Original Message ----- > > From: "Ade Lee" <alee(a)redhat.com&gt; > > To: pki-devel(a)redhat.com > > Sent: Wednesday, June 26, 2013 11:28:42 AM > > Subject: [Pki-devel] [PATCH] 0134-Make-sure-only-the-master-keys-and-certs-are-imported > > > > Make sure only the master keys and certs are imported. > > > > The key import code was written for when there was only one > > subsystem per tomcat instance, and only one subsystems certs > > and keys per p12 file. We need to ensure that only the master's > > subsystem keys and certs are imported. Otherwise, unpredictable > > behavior happens, like in Ticket 665. > > > > Please review, > > > > Thanks, > > Ade > > > > > > _______________________________________________ > > Pki-devel mailing list > > Pki-devel(a)redhat.com > > https://www.redhat.com/mailman/listinfo/pki-devel