CryptoExtensions.findSignersByName broken for organizational certs

[GoogleCodeExporter]

What steps will reproduce the problem?
1. The fix for http://code.google.com/p/nhin-d/issues/detail?id=126 has exposed 
another issue. Attempting to enforce the trust model on an incoming message 
sign with an organization level (domain) certificate will fail.

What is the expected output? What do you see instead?

Specifically I believe the line: 
    if (certSubjectContainsName(cert, name) || containsEmailAddressInSubjectAltName(cert, name))
should check that if the cert is an organization (domain) cert and attempt to 
match just the domain portion of the name parameter rather than the entire 
address.

Please use labels and text to provide additional information.

Original issue reported on code.google.com by reidn...@gmail.com on 5 May 2011 at 8:34

[GoogleCodeExporter]

The problem, or maybe an additional problem, might be here
        for (String excludeStr : excludeNames)
                if (certSubjectContainsName(cert, excludeStr) || containsEmailAddressInSubjectAltName(cert, /*FIXME? should this be excludeStr*/ name))

the original code with a bit more context ...

// check if we need to exclude anything
if (excludeNames != null)
        for (String excludeStr : excludeNames)
                if (certSubjectContainsName(cert, excludeStr) || containsEmailAddressInSubjectAltName(cert, /*FIXME?*/name))
                {
                        exclude = true;
                        break;
                }

if (exclude)
        continue; // break out and don't include this cert

Original comment by NFinst...@gmail.com on 5 May 2011 at 11:25

[GoogleCodeExporter]

Do you have a sample public CERT to test this with?  I am running this code on 
my EC2 instance with and org level CERT without issue.

Original comment by gm2...@cerner.com on 6 May 2011 at 1:02

[GoogleCodeExporter]

OK, I went back and looked at this a little more closely.  The issue would 
appear to be in certSubjectContainsName (containsEmailAddressInSubjectAltName 
uses equals and not a substring search).

Could you please provide a little more context around your statement:
"Attempting to enforce the trust model on an incoming message sign with an 
organization level (domain) certificate will fail".

Do you mean it is allowing certificates to be deemed as trusted when they 
should not be trusted or vice versa?  Again, having your public CERT and anchor 
would be helpful in creating test cases.

-g

Original comment by gm2...@cerner.com on 6 May 2011 at 1:16

[GoogleCodeExporter]

FYI,

I very well may decide to completely re-implement the methods that match email 
using the DN.  I forgot that I wrote a new implementation in the config 
services using the X509Principal object and the classic email OID.  This is a 
better to way to match certificates not found in the alt subject section.

Original comment by gm2...@cerner.com on 6 May 2011 at 1:37

[GoogleCodeExporter]

The certSubjectContainsName has been re-written to use the getSubjectAddress 
method with searches first for the email in the alt subject names then the DN 
if the address cannot be found in the alt names.

    public static boolean certSubjectContainsName(X509Certificate cert, String name)
    {
        if (name == null || name.length() == 0)
        {
            throw new IllegalArgumentException("Name cannot be null or empty.");
        }

        if (cert == null)
        {
            throw new IllegalArgumentException("Certificate cannot be null.");
        }

        boolean searchingForEmailAddress = name.toLowerCase(Locale.getDefault()).startsWith("emailaddress=");
        name = searchingForEmailAddress ? name.toLowerCase().replaceFirst("^emailaddress=", "") : name;     

        String address = getSubjectAddress(cert);
        if (address == null || address.isEmpty())
            return false;

        return name.toLowerCase(Locale.getDefault()).equals(address.toLowerCase(Locale.getDefault()));
    }   

The containsEmailAddressInSubjectAltName method is being deprecated as the new 
implementation certSubjectContainsName handles both use cases of alt names and 
DN.  The methods using containsEmailAddressInSubjectAltName have been updated 
appropriately including the CertificateStore class.  All testing has been 
regression testing against the agent and gateway, and all unit tests have 
passed.  If I get your cert and anchor, I'll add new unit tests to validate 
your use cases.

This code is temporarily being released as agent-1.2-SNAPSHOT, but will 
probably be released as agent-1.1.5 once validated.

The code has been checked in and the snapshot deployed to the snapshot 
repository.

Original comment by gm2...@cerner.com on 6 May 2011 at 2:06

[GoogleCodeExporter]

What I meant is that the agent is currently rejecting messages that should be 
trusted. I can provide the public cert and CA cert - but not the private key 
because it's a real (not test) certificate. Would that still be helpful?

Original comment by reidn...@gmail.com on 6 May 2011 at 2:07

[GoogleCodeExporter]

I've confirmed that your change fixes the issue we were seeing.

Original comment by reidn...@gmail.com on 6 May 2011 at 2:27

[GoogleCodeExporter]

Absolutely.  Wasn't expecting your private key ;-)....  All I need if you 
public cert and anchor and I can derive tests based on the information in those 
certs.

Chances are I won't be able to do the 1.1.5 release for a couple weeks as I 
will be out on vacation next week.

Original comment by gm2...@cerner.com on 6 May 2011 at 2:36

[GoogleCodeExporter]

Original comment by bgran...@harris.com on 9 May 2011 at 10:08

• Added labels: Area-Agent, Project-Java
• Removed labels: Area-Common, Project-All

[GoogleCodeExporter]

Has this been resolved? Can we close this issue?

Original comment by bgran...@harris.com on 15 Jun 2011 at 5:51

[GoogleCodeExporter]

It has.  I forgot to add it to the release notes in version 1.1.4.

Original comment by gm2...@cerner.com on 15 Jun 2011 at 8:56

[GoogleCodeExporter]

Original comment by bgran...@harris.com on 15 Jun 2011 at 9:44

• Changed state: Fixed