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@redhat.com> To: pki-devel@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@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel

Pushed to 10.0.X branch and master. On Wed, 2013-06-26 at 21:10 -0400, John Magne wrote:

Thanks for info.

Therefore:

ACK

----- Original Message ----- From: "Ade Lee" <alee@redhat.com> To: "John Magne" <jmagne@redhat.com> Cc: pki-devel@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@redhat.com> To: pki-devel@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@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel